Merge origin/main into webdav-litmus-compliance
This commit is contained in:
@@ -346,6 +346,18 @@ async fn handle_propfind(
|
||||
.header(header::CONTENT_TYPE, "application/xml; charset=utf-8")
|
||||
.body(Body::from(response_body))
|
||||
.unwrap())
|
||||
} else if first_is_uuid {
|
||||
// Path segment IS a UUID but the calendar isn't
|
||||
// accessible to the caller — could be another
|
||||
// owner's calendar or genuinely missing. Return
|
||||
// 404 (anti-enum, matches every other OxiCloud
|
||||
// surface post-D7). The pre-Round-3 fall-through
|
||||
// silently listed the caller's OWN calendars,
|
||||
// which was misleading (the URL claimed one calendar,
|
||||
// response returned unrelated ones) and violated
|
||||
// the anti-enumeration contract audited in
|
||||
// `docs/plan/authz_audit/caldav_carddav_wopi.md`.
|
||||
Err(AppError::not_found("Calendar not found"))
|
||||
} else {
|
||||
// Not a calendar ID — treat as user calendar home (e.g. /caldav/{username}/)
|
||||
// List all calendars for this user
|
||||
|
||||
@@ -33,8 +33,8 @@ use crate::application::adapters::webdav_adapter::{PropFindRequest, PropFindType
|
||||
use crate::application::dtos::address_book_dto::{CreateAddressBookDto, UpdateAddressBookDto};
|
||||
use crate::application::dtos::contact_dto::CreateContactVCardDto;
|
||||
use crate::application::ports::carddav_ports::{AddressBookUseCase, ContactUseCase};
|
||||
use crate::application::services::contact_service::ContactService;
|
||||
use crate::common::di::AppState;
|
||||
use crate::infrastructure::adapters::contact_storage_adapter::ContactStorageAdapter;
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::{AuthUser, CurrentUser};
|
||||
|
||||
@@ -177,7 +177,7 @@ fn extract_user(req: &Request<Body>) -> Result<AuthUser, AppError> {
|
||||
.ok_or_else(|| AppError::unauthorized("Authentication required"))
|
||||
}
|
||||
|
||||
fn get_addressbook_service(state: &AppState) -> Result<&Arc<ContactStorageAdapter>, AppError> {
|
||||
fn get_addressbook_service(state: &AppState) -> Result<&Arc<ContactService>, AppError> {
|
||||
state.addressbook_use_case.as_ref().ok_or_else(|| {
|
||||
AppError::new(
|
||||
StatusCode::NOT_IMPLEMENTED,
|
||||
@@ -187,7 +187,7 @@ fn get_addressbook_service(state: &AppState) -> Result<&Arc<ContactStorageAdapte
|
||||
})
|
||||
}
|
||||
|
||||
fn get_contact_service(state: &AppState) -> Result<&Arc<ContactStorageAdapter>, AppError> {
|
||||
fn get_contact_service(state: &AppState) -> Result<&Arc<ContactService>, AppError> {
|
||||
state.contact_use_case.as_ref().ok_or_else(|| {
|
||||
AppError::new(
|
||||
StatusCode::NOT_IMPLEMENTED,
|
||||
|
||||
@@ -19,8 +19,8 @@ use crate::application::dtos::contact_dto::{
|
||||
use crate::application::dtos::user_dto::UserDto;
|
||||
use crate::application::ports::carddav_ports::{AddressBookUseCase, ContactUseCase};
|
||||
use crate::application::services::auth_application_service::AuthApplicationService;
|
||||
use crate::application::services::contact_service::ContactService;
|
||||
use crate::domain::errors::ErrorKind;
|
||||
use crate::infrastructure::adapters::contact_storage_adapter::ContactStorageAdapter;
|
||||
use crate::interfaces::middleware::auth::AuthUser;
|
||||
|
||||
const SYSTEM_BOOK_ID: &str = "system";
|
||||
@@ -28,7 +28,7 @@ const SYSTEM_BOOK_ID: &str = "system";
|
||||
/// Combined state for the contacts REST API.
|
||||
#[derive(Clone)]
|
||||
pub struct ContactsApiState {
|
||||
pub contact_service: Arc<ContactStorageAdapter>,
|
||||
pub contact_service: Arc<ContactService>,
|
||||
pub auth_service: Option<Arc<AuthApplicationService>>,
|
||||
/// When false, the virtual "system" address book (OxiCloud users) is hidden.
|
||||
pub expose_system_users: bool,
|
||||
|
||||
@@ -48,23 +48,7 @@ pub async fn list_drives(
|
||||
) -> impl IntoResponse {
|
||||
let caller_id = auth_user.id;
|
||||
|
||||
let (subject_types, subject_ids) = match state
|
||||
.authorization
|
||||
.expand_subject_for_listing(Subject::User(caller_id))
|
||||
.await
|
||||
{
|
||||
Ok(pair) => pair,
|
||||
Err(e) => {
|
||||
error!("list_drives: subject expansion failed: {e}");
|
||||
return AppError::from(e).into_response();
|
||||
}
|
||||
};
|
||||
|
||||
match state
|
||||
.drive_repo
|
||||
.list_for_subjects(&subject_types, &subject_ids)
|
||||
.await
|
||||
{
|
||||
match state.drive_repo.list_readable_by(caller_id).await {
|
||||
Ok(drives) => {
|
||||
let dtos: Vec<DriveDto> = drives.into_iter().map(DriveDto::from).collect();
|
||||
(StatusCode::OK, Json(dtos)).into_response()
|
||||
@@ -416,6 +400,10 @@ pub struct UpdateDrivePoliciesDto {
|
||||
pub forbid_cross_drive_move: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub forbid_owner_role_change: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub include_in_photo_index: Option<bool>,
|
||||
#[serde(default, skip_serializing_if = "Option::is_none")]
|
||||
pub include_in_music_index: Option<bool>,
|
||||
}
|
||||
|
||||
/// `PATCH /api/drives/{id}/policies` — **OxiCloud-admin only** policy
|
||||
@@ -494,18 +482,22 @@ pub async fn update_drive_policies(
|
||||
serde_json::Value::Bool(v),
|
||||
);
|
||||
}
|
||||
if let Some(v) = dto.include_in_photo_index {
|
||||
partial_obj.insert("include_in_photo_index".into(), serde_json::Value::Bool(v));
|
||||
}
|
||||
if let Some(v) = dto.include_in_music_index {
|
||||
partial_obj.insert("include_in_music_index".into(), serde_json::Value::Bool(v));
|
||||
}
|
||||
// Pass the raw JSON straight through so the JSONB `||` merge in
|
||||
// the repo only touches keys the caller supplied. Round-tripping
|
||||
// via `DrivePolicies` (which has `#[serde(default)]`) would
|
||||
// silently fill every omitted field with `false` — the merge
|
||||
// would then clobber every unmentioned policy on the row.
|
||||
let partial_value = serde_json::Value::Object(partial_obj);
|
||||
let partial: crate::domain::entities::drive::DrivePolicies =
|
||||
match serde_json::from_value(partial_value) {
|
||||
Ok(p) => p,
|
||||
Err(e) => {
|
||||
return AppError::bad_request(format!("invalid policy body: {e}")).into_response();
|
||||
}
|
||||
};
|
||||
|
||||
match state
|
||||
.drive_management_service
|
||||
.update_policies(auth_user.id, drive_id, partial)
|
||||
.update_policies(auth_user.id, drive_id, partial_value)
|
||||
.await
|
||||
{
|
||||
Ok(merged) => (StatusCode::OK, axum::Json(merged)).into_response(),
|
||||
|
||||
@@ -6,7 +6,7 @@ use axum::{
|
||||
};
|
||||
use serde::Deserialize;
|
||||
use std::sync::Arc;
|
||||
use tracing::{error, info};
|
||||
use tracing::info;
|
||||
use utoipa::ToSchema;
|
||||
|
||||
use crate::application::dtos::display_helpers::{
|
||||
@@ -66,7 +66,8 @@ pub async fn add_favorite(
|
||||
Json(serde_json::json!({
|
||||
"error": "Item type must be 'file' or 'folder'"
|
||||
})),
|
||||
);
|
||||
)
|
||||
.into_response();
|
||||
}
|
||||
|
||||
match favorites_service
|
||||
@@ -81,16 +82,14 @@ pub async fn add_favorite(
|
||||
"message": "Item added to favorites"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error adding to favorites: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to add to favorites"
|
||||
})),
|
||||
)
|
||||
}
|
||||
// Route through AppError so the `DomainError::kind` maps to the
|
||||
// right status code (NotFound → 404 anti-enum for the pre-write
|
||||
// authz gate, InvalidInput → 400 for a malformed UUID, etc.).
|
||||
// A hardcoded 500 here would mask the 404 the Round 1 AuthZ
|
||||
// fix relies on.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -129,6 +128,7 @@ pub async fn remove_favorite(
|
||||
"message": "Item removed from favorites"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
} else {
|
||||
info!("Item {} '{}' was not in favorites", item_type, item_id);
|
||||
(
|
||||
@@ -137,17 +137,12 @@ pub async fn remove_favorite(
|
||||
"message": "Item was not in favorites"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error removing from favorites: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to remove from favorites"
|
||||
})),
|
||||
)
|
||||
}
|
||||
// Same rationale as `add_favorite` — preserve DomainError→HTTP
|
||||
// status mapping instead of collapsing every error to 500.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -215,7 +210,6 @@ pub async fn list_favorites_resources(
|
||||
name: row.name.clone(),
|
||||
path,
|
||||
parent_id: row.parent_id.map(|u| u.to_string()),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
drive_id: row.drive_id,
|
||||
created_at: row.resource_created_at.timestamp() as u64,
|
||||
modified_at: row.modified_at.timestamp() as u64,
|
||||
@@ -265,7 +259,6 @@ pub async fn list_favorites_resources(
|
||||
)),
|
||||
category: std::sync::Arc::from(category_for(&row.name, mime)),
|
||||
size_formatted: format_file_size(size_bytes),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
sort_date: None,
|
||||
content_hash,
|
||||
etag,
|
||||
@@ -349,15 +342,10 @@ pub async fn batch_add_favorites(
|
||||
);
|
||||
(StatusCode::OK, Json(serde_json::json!(result))).into_response()
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error in batch add favorites: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to batch add favorites"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
// Preserve DomainError→HTTP status mapping — the Round 1
|
||||
// AuthZ fix relies on a per-item NotFound propagating out
|
||||
// of the batch. A hardcoded 500 would mask the 404 that
|
||||
// signals a cross-tenant probe.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -106,9 +106,12 @@ impl FolderHandler {
|
||||
Self::list_folders_scoped(service, None, &auth_user).await
|
||||
}
|
||||
|
||||
/// Internal helper: lists folders scoped to the authenticated user.
|
||||
/// Uses `list_folders_for_owner` — the DB query filters by `user_id`,
|
||||
/// so no data from other users ever leaves the database.
|
||||
/// Internal helper: lists folders the authenticated caller can Read.
|
||||
/// Post-PR-B, `list_root_folders_for_caller` scopes via
|
||||
/// drive-membership grants (`role_grants` + group cascade via
|
||||
/// `storage.caller_group_ids`) instead of the legacy `folders.user_id`
|
||||
/// filter, so folders in shared drives the caller belongs to
|
||||
/// surface here too.
|
||||
async fn list_folders_scoped(
|
||||
service: AppState,
|
||||
parent_id: Option<&str>,
|
||||
@@ -501,7 +504,6 @@ pub async fn list_folder_resources(
|
||||
name: row.name.clone(),
|
||||
path: String::new(), // cleared — share recipients must not see hierarchy
|
||||
parent_id: row.parent_id.map(|u| u.to_string()),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
drive_id: row.drive_id,
|
||||
created_at: row.created_at.timestamp() as u64,
|
||||
modified_at: row.modified_at.timestamp() as u64,
|
||||
@@ -550,7 +552,6 @@ pub async fn list_folder_resources(
|
||||
icon_special_class: Arc::from(icon_special_class_for(&row.name, mime)),
|
||||
category: Arc::from(category_for(&row.name, mime)),
|
||||
size_formatted: format_file_size(size_bytes),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
sort_date: None,
|
||||
content_hash,
|
||||
etag,
|
||||
|
||||
@@ -113,6 +113,14 @@ pub async fn create_grant(
|
||||
.get_by_id(id)
|
||||
.await
|
||||
.map(|d| d.drive.typed_policies()),
|
||||
// Calendars, address books and playlists live outside the
|
||||
// drive hierarchy (top-level per user), so no drive-level
|
||||
// policy gates apply. If per-resource policies ever ship for
|
||||
// these kinds, they'll live on the resource itself, not on a
|
||||
// drive; the default-empty bag is the right no-op here.
|
||||
Resource::Calendar(_) | Resource::AddressBook(_) | Resource::Playlist(_) => {
|
||||
Ok(crate::domain::entities::drive::DrivePolicies::default())
|
||||
}
|
||||
};
|
||||
let drive_policies = match drive_policies {
|
||||
Ok(p) => p,
|
||||
|
||||
@@ -63,13 +63,13 @@ pub async fn list_photos(
|
||||
headers: HeaderMap,
|
||||
Query(params): Query<PhotosQueryParams>,
|
||||
) -> impl IntoResponse {
|
||||
let user_id = auth_user.id;
|
||||
let caller_id = auth_user.id;
|
||||
let limit = params.limit.unwrap_or(200).clamp(1, 500);
|
||||
|
||||
let file_read = &state.repositories.file_read_repository;
|
||||
|
||||
match file_read
|
||||
.list_media_files(user_id, params.before, limit)
|
||||
.list_media_files(caller_id, params.before, limit)
|
||||
.await
|
||||
{
|
||||
Ok((files, sort_dates, dims)) => {
|
||||
|
||||
@@ -5,7 +5,7 @@ use axum::{
|
||||
response::IntoResponse,
|
||||
};
|
||||
use std::sync::Arc;
|
||||
use tracing::{error, info};
|
||||
use tracing::info;
|
||||
|
||||
use crate::application::dtos::display_helpers::{
|
||||
category_for, format_file_size, icon_class_for, icon_special_class_for,
|
||||
@@ -70,16 +70,10 @@ pub async fn record_item_access(
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error recording access in recents: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to record access"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
// Preserve DomainError→HTTP status mapping — the Round 1
|
||||
// AuthZ fix relies on the NotFound from `authz.require`
|
||||
// propagating as 404 (anti-enum), not being masked as 500.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -130,16 +124,9 @@ pub async fn remove_from_recent(
|
||||
.into_response()
|
||||
}
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error removing from recents: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to remove from recents"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
// Same rationale as `record_item_access` — preserve the
|
||||
// DomainError→HTTP mapping instead of collapsing to 500.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -170,16 +157,9 @@ pub async fn clear_recent_items(
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
Err(err) => {
|
||||
error!("Error clearing recent items: {}", err);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
Json(serde_json::json!({
|
||||
"error": "Failed to clear recent items"
|
||||
})),
|
||||
)
|
||||
.into_response()
|
||||
}
|
||||
// Same rationale as `record_item_access` — preserve the
|
||||
// DomainError→HTTP mapping instead of collapsing to 500.
|
||||
Err(err) => AppError::from(err).into_response(),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -246,7 +226,6 @@ pub async fn list_recent_resources(
|
||||
name: row.name.clone(),
|
||||
path,
|
||||
parent_id: row.parent_id.map(|u| u.to_string()),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
drive_id: row.drive_id,
|
||||
created_at: row.resource_created_at.timestamp() as u64,
|
||||
modified_at: row.modified_at.timestamp() as u64,
|
||||
@@ -294,7 +273,6 @@ pub async fn list_recent_resources(
|
||||
)),
|
||||
category: std::sync::Arc::from(category_for(&row.name, mime)),
|
||||
size_formatted: format_file_size(size_bytes),
|
||||
owner_id: Some(row.owner_id.to_string()),
|
||||
sort_date: None,
|
||||
content_hash,
|
||||
etag,
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -20,10 +20,13 @@ use axum::{
|
||||
use serde::{Deserialize, Serialize};
|
||||
use std::sync::Arc;
|
||||
|
||||
use crate::application::ports::authorization_ports::AuthorizationEngine;
|
||||
use crate::application::ports::file_ports::{FileRetrievalUseCase, FileUploadUseCase};
|
||||
use crate::application::services::wopi_lock_service::WopiLockService;
|
||||
use crate::application::services::wopi_token_service::WopiTokenService;
|
||||
use crate::domain::repositories::drive_repository::DriveRepository;
|
||||
use crate::domain::services::authorization::{Permission, Resource, Subject};
|
||||
use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
|
||||
use crate::infrastructure::services::wopi_discovery_service::WopiDiscoveryService;
|
||||
|
||||
/// Shared state for WOPI handlers.
|
||||
@@ -64,6 +67,37 @@ pub struct CheckFileInfoResponse {
|
||||
pub close_url: String,
|
||||
}
|
||||
|
||||
/// Enforce that the WOPI caller (`claims.sub`) still has `perm` on the
|
||||
/// file at redemption time — not just at token-mint time.
|
||||
///
|
||||
/// **Why every verb needs this.** WOPI tokens are validated locally
|
||||
/// (HMAC over claims), so a token that was legitimately minted stays
|
||||
/// verify-able until its TTL. If a grant is revoked after mint, or the
|
||||
/// token was minted for view but is used to POST content, the token's
|
||||
/// signature alone doesn't catch it. This helper re-checks against the
|
||||
/// live authorization engine on every verb — the memory note
|
||||
/// `wopi-authz-bypass` calls out the class of bugs this fences.
|
||||
///
|
||||
/// Returns 404 (anti-enumeration — same shape as "file doesn't exist")
|
||||
/// on both bad UUID and authorization denial. The engine emits a
|
||||
/// structured `audit` line on denial internally, so ops sees the real
|
||||
/// reason without the attacker being able to distinguish "gone" from
|
||||
/// "revoked".
|
||||
async fn require_wopi_perm(
|
||||
authz: &PgAclEngine,
|
||||
caller_sub: &str,
|
||||
file_id: &str,
|
||||
perm: Permission,
|
||||
) -> Result<(uuid::Uuid, uuid::Uuid), StatusCode> {
|
||||
let caller_uuid = uuid::Uuid::parse_str(caller_sub).map_err(|_| StatusCode::UNAUTHORIZED)?;
|
||||
let file_uuid = uuid::Uuid::parse_str(file_id).map_err(|_| StatusCode::NOT_FOUND)?;
|
||||
authz
|
||||
.require(Subject::User(caller_uuid), perm, Resource::File(file_uuid))
|
||||
.await
|
||||
.map_err(|_| StatusCode::NOT_FOUND)?;
|
||||
Ok((caller_uuid, file_uuid))
|
||||
}
|
||||
|
||||
/// GET /wopi/files/{file_id} — CheckFileInfo
|
||||
async fn check_file_info(
|
||||
Path(file_id): Path<String>,
|
||||
@@ -82,6 +116,19 @@ async fn check_file_info(
|
||||
return StatusCode::UNAUTHORIZED.into_response();
|
||||
}
|
||||
|
||||
// Redemption-time authz: even with a valid token, the caller must
|
||||
// still hold Read on this file. Catches revoked-grant-mid-session.
|
||||
if let Err(status) = require_wopi_perm(
|
||||
state.app_state.authorization.as_ref(),
|
||||
&claims.sub,
|
||||
&file_id,
|
||||
Permission::Read,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return status.into_response();
|
||||
}
|
||||
|
||||
// Fetch file metadata
|
||||
let file = match state
|
||||
.app_state
|
||||
@@ -99,16 +146,40 @@ async fn check_file_info(
|
||||
.map(|dt| dt.to_rfc3339())
|
||||
.unwrap_or_default();
|
||||
|
||||
// `user_can_write` = actual current Update permission ∧ token's
|
||||
// can_write flag. If the caller's Update was revoked since the
|
||||
// token was minted (e.g. their grant was downgraded from Editor
|
||||
// to Viewer), the editor sees the file as read-only and won't
|
||||
// even attempt PutFile. The stricter `require_wopi_perm(Update)`
|
||||
// in put_file is the actual gate; this field is a UI hint.
|
||||
let can_write_now = claims.can_write
|
||||
&& state
|
||||
.app_state
|
||||
.authorization
|
||||
.check(
|
||||
Subject::User(uuid::Uuid::parse_str(&claims.sub).unwrap_or(uuid::Uuid::nil())),
|
||||
Permission::Update,
|
||||
Resource::File(uuid::Uuid::parse_str(&file_id).unwrap_or(uuid::Uuid::nil())),
|
||||
)
|
||||
.await
|
||||
.unwrap_or(false);
|
||||
|
||||
let response = CheckFileInfoResponse {
|
||||
base_file_name: file.name.clone(),
|
||||
owner_id: file.owner_id.clone().unwrap_or_else(|| claims.sub.clone()),
|
||||
// WOPI's `OwnerId` field is required. Post-D7 the DTO no
|
||||
// longer carries `owner_id`; fall back to `created_by`
|
||||
// (§14 provenance) with the requesting user as a final default.
|
||||
owner_id: file
|
||||
.created_by
|
||||
.map(|u| u.to_string())
|
||||
.unwrap_or_else(|| claims.sub.clone()),
|
||||
size: file.size,
|
||||
user_id: claims.sub.clone(),
|
||||
version: file.modified_at.to_string(),
|
||||
supports_locks: true,
|
||||
supports_update: claims.can_write,
|
||||
supports_update: can_write_now,
|
||||
supports_rename: false,
|
||||
user_can_write: claims.can_write,
|
||||
user_can_write: can_write_now,
|
||||
user_friendly_name: claims.username.clone(),
|
||||
post_message_origin: state.public_base_url.clone(),
|
||||
last_modified_time: last_modified,
|
||||
@@ -139,6 +210,18 @@ async fn get_file(
|
||||
return StatusCode::UNAUTHORIZED.into_response();
|
||||
}
|
||||
|
||||
// Redemption-time authz — see require_wopi_perm docstring.
|
||||
if let Err(status) = require_wopi_perm(
|
||||
state.app_state.authorization.as_ref(),
|
||||
&claims.sub,
|
||||
&file_id,
|
||||
Permission::Read,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return status.into_response();
|
||||
}
|
||||
|
||||
match state
|
||||
.app_state
|
||||
.applications
|
||||
@@ -178,6 +261,21 @@ async fn put_file(
|
||||
return StatusCode::UNAUTHORIZED.into_response();
|
||||
}
|
||||
|
||||
// Redemption-time authz: the token says the caller could write when
|
||||
// it was minted, but Update permission may have been revoked since.
|
||||
// Re-check now so a stale write-capable token can't survive a
|
||||
// downgrade / share removal / drive-membership change until its TTL.
|
||||
if let Err(status) = require_wopi_perm(
|
||||
state.app_state.authorization.as_ref(),
|
||||
&claims.sub,
|
||||
&file_id,
|
||||
Permission::Update,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return status.into_response();
|
||||
}
|
||||
|
||||
// Check lock
|
||||
let request_lock = headers
|
||||
.get("X-WOPI-Lock")
|
||||
@@ -258,7 +356,7 @@ async fn put_file(
|
||||
.app_state
|
||||
.applications
|
||||
.file_upload_service
|
||||
.update_file_streaming(
|
||||
.update_file_streaming_with_perms(
|
||||
&file.path,
|
||||
drive_id,
|
||||
ingested.stored(),
|
||||
@@ -296,6 +394,22 @@ async fn file_operations(
|
||||
return StatusCode::UNAUTHORIZED.into_response();
|
||||
}
|
||||
|
||||
// Every lock op mutates shared state (LOCK / UNLOCK / REFRESH_LOCK
|
||||
// change the lock; GET_LOCK reads it but the read is only useful
|
||||
// to a caller who could subsequently take a write action — so gate
|
||||
// on Update uniformly rather than splitting per-op). A Viewer with
|
||||
// a stale token must not be able to hold or contend for a lock.
|
||||
if let Err(status) = require_wopi_perm(
|
||||
state.app_state.authorization.as_ref(),
|
||||
&claims.sub,
|
||||
&file_id,
|
||||
Permission::Update,
|
||||
)
|
||||
.await
|
||||
{
|
||||
return status.into_response();
|
||||
}
|
||||
|
||||
let override_header = headers
|
||||
.get("X-WOPI-Override")
|
||||
.and_then(|v| v.to_str().ok())
|
||||
@@ -368,25 +482,71 @@ pub struct EditorUrlResponse {
|
||||
pub access_token_ttl: i64,
|
||||
}
|
||||
|
||||
/// Determines if `caller_id` can access `file_id` and with what permissions.
|
||||
/// Resolve the WOPI mint target: gate on real permissions and derive
|
||||
/// the `can_write` flag from the caller's ACTUAL Update rights.
|
||||
///
|
||||
/// Uses the SQL-level ownership check (`get_file_owned`) so that files
|
||||
/// belonging to other users — or non-existent files — both return `NOT_FOUND`,
|
||||
/// avoiding existence-leak oracles.
|
||||
/// Prior behaviour used a naive `requested_action != "view"` heuristic
|
||||
/// so a Viewer clicking "Edit in Collabora" received a write-capable
|
||||
/// token, promoting themselves to Editor for the token's TTL. The
|
||||
/// memory note `wopi-authz-bypass` fix #12 calls this out explicitly.
|
||||
///
|
||||
/// Returns `(FileDto, can_write)` on success.
|
||||
/// Contract:
|
||||
///
|
||||
/// 1. **Read** is the bar to open the file in any mode. If the caller
|
||||
/// has no Read grant, return 404 (anti-enum — same shape as "no such
|
||||
/// file").
|
||||
/// 2. **Update** determines the returned `can_write` bit — INDEPENDENT
|
||||
/// of what the client's `requested_action` said. A Viewer who
|
||||
/// requested `action=edit` gets `can_write=false` and Collabora
|
||||
/// opens in view mode; the token stays authorised for view-only
|
||||
/// ops and put_file will 404 at redemption regardless.
|
||||
/// 3. `requested_action == "view"` is respected as a downgrade — an
|
||||
/// Editor can explicitly request view mode (co-browsing a doc
|
||||
/// without accidentally editing) and get `can_write=false`.
|
||||
///
|
||||
/// The `PgAclEngine::require`/`check` calls emit structured audit
|
||||
/// lines on denial (`authz.denied` event), so a Viewer's "edit"
|
||||
/// attempt shows up in the audit stream as a rejected Update check.
|
||||
async fn authorize_wopi_access<S: FileRetrievalUseCase>(
|
||||
authz: &PgAclEngine,
|
||||
file_retrieval: &S,
|
||||
file_id: &str,
|
||||
caller_id: uuid::Uuid,
|
||||
requested_action: &str,
|
||||
) -> Result<(crate::application::dtos::file_dto::FileDto, bool), StatusCode> {
|
||||
let file = file_retrieval
|
||||
.get_file_with_perms(file_id, caller_id)
|
||||
let file_uuid = uuid::Uuid::parse_str(file_id).map_err(|_| StatusCode::NOT_FOUND)?;
|
||||
|
||||
// Step 1 — Read is required to even open the file.
|
||||
authz
|
||||
.require(
|
||||
Subject::User(caller_id),
|
||||
Permission::Read,
|
||||
Resource::File(file_uuid),
|
||||
)
|
||||
.await
|
||||
.map_err(|_| StatusCode::NOT_FOUND)?;
|
||||
// Owner verified — grant write unless explicitly requesting view-only.
|
||||
let can_write = requested_action != "view";
|
||||
|
||||
let file = file_retrieval
|
||||
.get_file(file_id)
|
||||
.await
|
||||
.map_err(|_| StatusCode::NOT_FOUND)?;
|
||||
|
||||
// Step 2 — can_write reflects real Update, not the client's
|
||||
// action-string. `check` returns bool without throwing; failure
|
||||
// just means the caller lacks Update, so we degrade the token to
|
||||
// read-only. Deliberately no `require` here — a Viewer opening
|
||||
// the file is legitimate; only the write claim is suppressed.
|
||||
let has_update = authz
|
||||
.check(
|
||||
Subject::User(caller_id),
|
||||
Permission::Update,
|
||||
Resource::File(file_uuid),
|
||||
)
|
||||
.await
|
||||
.unwrap_or(false);
|
||||
|
||||
// Step 3 — allow explicit view-mode downgrade for Editors.
|
||||
let can_write = has_update && requested_action != "view";
|
||||
Ok((file, can_write))
|
||||
}
|
||||
|
||||
@@ -403,6 +563,7 @@ pub async fn get_editor_url(
|
||||
let username = &auth_user.username;
|
||||
// Verify the caller owns the file (SQL-level check, no existence leak).
|
||||
let (file, can_write) = match authorize_wopi_access(
|
||||
state.app_state.authorization.as_ref(),
|
||||
state.app_state.applications.file_retrieval_service.as_ref(),
|
||||
¶ms.file_id,
|
||||
user_id,
|
||||
@@ -488,7 +649,8 @@ async fn host_page(
|
||||
Ok(u) => u,
|
||||
Err(_) => return StatusCode::UNAUTHORIZED.into_response(),
|
||||
};
|
||||
let file = match authorize_wopi_access(
|
||||
let (file, can_write_now) = match authorize_wopi_access(
|
||||
state.app_state.authorization.as_ref(),
|
||||
state.app_state.applications.file_retrieval_service.as_ref(),
|
||||
&file_id,
|
||||
caller_uuid,
|
||||
@@ -496,7 +658,7 @@ async fn host_page(
|
||||
)
|
||||
.await
|
||||
{
|
||||
Ok((f, _)) => f,
|
||||
Ok((f, cw)) => (f, cw),
|
||||
Err(status) => return status.into_response(),
|
||||
};
|
||||
|
||||
@@ -513,11 +675,15 @@ async fn host_page(
|
||||
_ => return StatusCode::INTERNAL_SERVER_ERROR.into_response(),
|
||||
};
|
||||
|
||||
// Use the freshly-computed `can_write_now` (real Update permission
|
||||
// ∧ requested_action) rather than the incoming token's `can_write`
|
||||
// flag. Otherwise a Viewer who somehow reached this host page with
|
||||
// a stale edit-capable token would get another one re-minted.
|
||||
let (token, ttl) = match state.token_service.generate_token(
|
||||
&file_id,
|
||||
&claims.sub,
|
||||
&claims.username,
|
||||
claims.can_write,
|
||||
can_write_now,
|
||||
) {
|
||||
Ok(t) => t,
|
||||
Err(_) => return StatusCode::INTERNAL_SERVER_ERROR.into_response(),
|
||||
|
||||
Reference in New Issue
Block a user