feat(drive): add drive policie
- add policy forbid_external_sharing
- add policy forbid_sharing
- add polocy forbid_cross_drive_move
- add policy forbid_owner_role_change
This commit is contained in:
@@ -29,6 +29,7 @@ use crate::domain::repositories::subject_group_repository::SubjectGroupRepositor
|
||||
use crate::domain::services::authorization::{Grant, Permission, Resource, Role, Subject};
|
||||
use crate::infrastructure::repositories::pg::DrivePgRepository;
|
||||
use crate::infrastructure::repositories::pg::SubjectGroupPgRepository;
|
||||
use crate::infrastructure::repositories::pg::UserPgRepository;
|
||||
use crate::infrastructure::services::pg_acl_engine::PgAclEngine;
|
||||
|
||||
pub struct DriveManagementService {
|
||||
@@ -39,6 +40,11 @@ pub struct DriveManagementService {
|
||||
/// constructing an orphan-owned drive (the "drive must always have
|
||||
/// ≥1 effective Owner-user" invariant from day one).
|
||||
group_repo: Arc<SubjectGroupPgRepository>,
|
||||
/// D5: `set_member_role` reads `users.is_external` to enforce
|
||||
/// `forbid_external_sharing` on the drive — closes the gap that the
|
||||
/// `POST /api/drives/{id}/members` route would otherwise open
|
||||
/// (the grant_handler check only catches `POST /api/grants`).
|
||||
user_repo: Arc<UserPgRepository>,
|
||||
}
|
||||
|
||||
impl DriveManagementService {
|
||||
@@ -46,11 +52,13 @@ impl DriveManagementService {
|
||||
drive_repo: Arc<DrivePgRepository>,
|
||||
authz: Arc<PgAclEngine>,
|
||||
group_repo: Arc<SubjectGroupPgRepository>,
|
||||
user_repo: Arc<UserPgRepository>,
|
||||
) -> Self {
|
||||
Self {
|
||||
drive_repo,
|
||||
authz,
|
||||
group_repo,
|
||||
user_repo,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -201,6 +209,30 @@ impl DriveManagementService {
|
||||
|
||||
self.refuse_if_personal(drive_id, "set_member_role").await?;
|
||||
|
||||
// D5: `forbid_external_sharing` on a shared drive — refuses
|
||||
// grant writes whose User subject is `is_external = true`.
|
||||
// Closes the `POST /api/drives/{id}/members` gap that
|
||||
// grant_handler's same-shaped check (covering `POST /api/grants`
|
||||
// only) doesn't reach. Group/Token subjects can't be external
|
||||
// by construction, so the lookup runs only for User subjects.
|
||||
// See `docs/plan/drive.md` §8.
|
||||
self.refuse_if_forbid_external_sharing(drive_id, subject, caller_id)
|
||||
.await?;
|
||||
|
||||
// D5: `forbid_owner_role_change` — locks the Owner roster
|
||||
// against non-admin callers. Fires when this write would add a
|
||||
// new Owner (role == Owner) OR demote a current Owner
|
||||
// (subject is currently Owner and role != Owner).
|
||||
self.refuse_if_forbid_owner_role_change(
|
||||
drive_id,
|
||||
subject,
|
||||
Some(role),
|
||||
caller_id,
|
||||
caller_is_admin,
|
||||
"set_member_role",
|
||||
)
|
||||
.await?;
|
||||
|
||||
// Demotion of the last owner = last-owner protection trips. A fresh
|
||||
// owner-role write or any non-owner subject is fine; only the case
|
||||
// "this subject is currently the only owner AND the new role is not
|
||||
@@ -255,6 +287,19 @@ impl DriveManagementService {
|
||||
|
||||
self.refuse_if_personal(drive_id, "remove_member").await?;
|
||||
|
||||
// D5: `forbid_owner_role_change` — locks the Owner roster
|
||||
// against non-admin callers. Fires when this would remove a
|
||||
// current Owner.
|
||||
self.refuse_if_forbid_owner_role_change(
|
||||
drive_id,
|
||||
subject,
|
||||
None, // None = removal, not a role write
|
||||
caller_id,
|
||||
caller_is_admin,
|
||||
"remove_member",
|
||||
)
|
||||
.await?;
|
||||
|
||||
self.refuse_if_last_owner_change(drive_id, subject, caller_id)
|
||||
.await?;
|
||||
|
||||
@@ -373,30 +418,27 @@ impl DriveManagementService {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// `PATCH /api/drives/{id}/policies`. Owner-only mutation of the
|
||||
/// drive's `policies` JSONB bag (§5 — "edit policies" is in the
|
||||
/// drive owner bundle, applies to personal AND shared drives).
|
||||
/// `PATCH /api/drives/{id}/policies`. OxiCloud-admin only.
|
||||
///
|
||||
/// The drive's `policies` JSONB bag is a compliance surface — same
|
||||
/// category as `drives.quota_bytes` and `users.storage_quota_bytes`
|
||||
/// (§7). Owner mutation would make the policies self-policing
|
||||
/// (an owner could disable `forbid_external_sharing`, share, and
|
||||
/// re-enable), so mutation is restricted to the tenant operator.
|
||||
/// The handler is the gate (refuses non-admin callers with 404 for
|
||||
/// anti-enumeration); this method trusts that gate and writes
|
||||
/// unconditionally.
|
||||
///
|
||||
/// JSONB-level merge preserves unknown keys; only the partial
|
||||
/// supplied is overwritten. Returns the post-merge typed view.
|
||||
///
|
||||
/// `caller_is_admin` mirrors the membership endpoints — skips the
|
||||
/// per-drive Manage check. Audit emits `drive.policy_changed` with
|
||||
/// the post-merge bag for steady-state observability; ops can grep
|
||||
/// for the specific keys that flipped against the prior values.
|
||||
/// Audit emits `drive.policy_changed` with the post-merge bag for
|
||||
/// steady-state observability.
|
||||
pub async fn update_policies(
|
||||
&self,
|
||||
caller_id: Uuid,
|
||||
caller_is_admin: bool,
|
||||
drive_id: Uuid,
|
||||
partial: crate::domain::entities::drive::DrivePolicies,
|
||||
) -> Result<crate::domain::entities::drive::DrivePolicies, DomainError> {
|
||||
let resource = Resource::Drive(drive_id);
|
||||
if !caller_is_admin {
|
||||
self.authz
|
||||
.require(Subject::User(caller_id), Permission::Manage, resource)
|
||||
.await?;
|
||||
}
|
||||
|
||||
let merged = self
|
||||
.drive_repo
|
||||
.update_policies(drive_id, &partial)
|
||||
@@ -413,22 +455,63 @@ impl DriveManagementService {
|
||||
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = if caller_is_admin {
|
||||
"drive.policy_changed_via_admin"
|
||||
} else {
|
||||
"drive.policy_changed"
|
||||
},
|
||||
event = "drive.policy_changed",
|
||||
drive_id = %drive_id,
|
||||
by = %caller_id,
|
||||
forbid_sharing = merged.forbid_sharing,
|
||||
forbid_external_sharing = merged.forbid_external_sharing,
|
||||
forbid_public_links = merged.forbid_public_links,
|
||||
forbid_cross_drive_move = merged.forbid_cross_drive_move,
|
||||
forbid_owner_role_change = merged.forbid_owner_role_change,
|
||||
"📜 drive policies updated",
|
||||
);
|
||||
Ok(merged)
|
||||
}
|
||||
|
||||
/// D5 `forbid_external_sharing` for `set_member_role`. Fetches the
|
||||
/// data this surface has but grant_handler doesn't (drive policies +
|
||||
/// user flags), then defers the decision + audit + canonical error
|
||||
/// to `DrivePolicies::refuse_external_sharing` — the same gate
|
||||
/// `grant_handler::create_grant` runs for File/Folder resources. One
|
||||
/// rejection shape across both entry points.
|
||||
///
|
||||
/// Group / Token subjects can't be external by construction, so the
|
||||
/// user lookup is skipped (the gate handles those branches too, but
|
||||
/// returning early avoids a wasted SELECT on the drive row).
|
||||
async fn refuse_if_forbid_external_sharing(
|
||||
&self,
|
||||
drive_id: Uuid,
|
||||
subject: Subject,
|
||||
caller_id: Uuid,
|
||||
) -> Result<(), DomainError> {
|
||||
let Subject::User(uid) = subject else {
|
||||
return Ok(());
|
||||
};
|
||||
let drive = self.drive_repo.get_by_id(drive_id).await.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("Failed to fetch drive: {e:?}"))
|
||||
})?;
|
||||
let policies = drive.drive.typed_policies();
|
||||
if !policies.forbid_external_sharing {
|
||||
return Ok(());
|
||||
}
|
||||
let flags = self
|
||||
.user_repo
|
||||
.get_user_flags(uid)
|
||||
.await
|
||||
.map_err(|e| DomainError::internal_error("User", format!("flags lookup: {e:?}")))?;
|
||||
policies.refuse_external_sharing(
|
||||
subject,
|
||||
flags.is_external,
|
||||
crate::domain::entities::drive::ExternalSharingGateContext {
|
||||
caller_id,
|
||||
stage: "drive_member",
|
||||
drive_id: Some(drive_id),
|
||||
resource_type: None,
|
||||
resource_id: None,
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
// ── Business rules ──────────────────────────────────────────────────────
|
||||
|
||||
/// Personal drives are single-user single-owner; any member mutation is
|
||||
@@ -454,6 +537,73 @@ impl DriveManagementService {
|
||||
Ok(())
|
||||
}
|
||||
|
||||
/// D5 `forbid_owner_role_change`. Fetches drive policies (one PK
|
||||
/// probe), bails out early when the policy is off or the caller is
|
||||
/// admin, then determines whether the requested op actually
|
||||
/// mutates the Owner roster:
|
||||
///
|
||||
/// - `new_role = Some(Role::Owner)` — Owner add or refresh. Owner
|
||||
/// roster mutation.
|
||||
/// - `new_role = Some(Role::X)` and subject is currently Owner —
|
||||
/// demotion. Owner roster mutation.
|
||||
/// - `new_role = None` (remove) and subject is currently Owner —
|
||||
/// removal. Owner roster mutation.
|
||||
///
|
||||
/// In any of those cases, defers to
|
||||
/// `DrivePolicies::refuse_owner_role_change` for the audit + error.
|
||||
async fn refuse_if_forbid_owner_role_change(
|
||||
&self,
|
||||
drive_id: Uuid,
|
||||
subject: Subject,
|
||||
new_role: Option<Role>,
|
||||
caller_id: Uuid,
|
||||
caller_is_admin: bool,
|
||||
operation: &'static str,
|
||||
) -> Result<(), DomainError> {
|
||||
// Fast bypass for the tenant operator.
|
||||
if caller_is_admin {
|
||||
return Ok(());
|
||||
}
|
||||
let drive = self.drive_repo.get_by_id(drive_id).await.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("Failed to fetch drive: {e:?}"))
|
||||
})?;
|
||||
let policies = drive.drive.typed_policies();
|
||||
if !policies.forbid_owner_role_change {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
// Determine whether this op touches the Owner roster. An Owner
|
||||
// add (role == Owner) always does; a non-Owner write or a
|
||||
// removal only does when the subject currently holds Owner —
|
||||
// fetched lazily on the second case to skip the round-trip
|
||||
// when we already know the answer.
|
||||
let touches_owner = if matches!(new_role, Some(Role::Owner)) {
|
||||
true
|
||||
} else {
|
||||
let grants = self
|
||||
.authz
|
||||
.list_grants_on_resource(Resource::Drive(drive_id))
|
||||
.await?;
|
||||
grants
|
||||
.iter()
|
||||
.any(|g| g.subject == subject && matches!(g.role, Role::Owner))
|
||||
};
|
||||
if !touches_owner {
|
||||
return Ok(());
|
||||
}
|
||||
|
||||
policies.refuse_owner_role_change(
|
||||
crate::domain::entities::drive::OwnerRoleChangeGateContext {
|
||||
caller_id,
|
||||
caller_is_admin,
|
||||
drive_id,
|
||||
operation,
|
||||
subject_type: subject.type_str(),
|
||||
subject_id: subject.id(),
|
||||
},
|
||||
)
|
||||
}
|
||||
|
||||
/// Refuse the change if `subject` is currently the sole `Owner` on the
|
||||
/// drive and the operation would remove or demote them. A shared drive
|
||||
/// must always have at least one Owner — otherwise it becomes orphaned
|
||||
|
||||
@@ -37,6 +37,12 @@ pub struct FileManagementService {
|
||||
/// downloads. Distinct from the lifecycle hook because lifecycle hooks
|
||||
/// don't carry the `caller_id` the recording side needs.
|
||||
resource_access_hook: Option<Arc<dyn ResourceAccessHook>>,
|
||||
/// Drive repository — used by D5's `forbid_cross_drive_move` gate
|
||||
/// on `move_file_with_perms`. Optional so stubs / test factories
|
||||
/// can build the service without wiring the full drive repo; in
|
||||
/// that case the cross-drive move check is skipped (the policy
|
||||
/// is silently off). Production DI wires it in.
|
||||
drive_repo: Option<Arc<dyn crate::domain::repositories::drive_repository::DriveRepository>>,
|
||||
}
|
||||
|
||||
impl FileManagementService {
|
||||
@@ -60,6 +66,7 @@ impl FileManagementService {
|
||||
authz,
|
||||
file_lifecycle_hook: None,
|
||||
resource_access_hook: None,
|
||||
drive_repo: None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -82,6 +89,17 @@ impl FileManagementService {
|
||||
}
|
||||
}
|
||||
|
||||
/// Wires the drive repository, enabling D5 `forbid_cross_drive_move`
|
||||
/// enforcement on `move_file_with_perms`. Without it, the gate is
|
||||
/// silently skipped.
|
||||
pub fn with_drive_repo(
|
||||
mut self,
|
||||
drive_repo: Arc<dyn crate::domain::repositories::drive_repository::DriveRepository>,
|
||||
) -> Self {
|
||||
self.drive_repo = Some(drive_repo);
|
||||
self
|
||||
}
|
||||
|
||||
/// Engine check for a file resource. Parses the id into a `Uuid` and
|
||||
/// requires the specified permission.
|
||||
async fn require_file_perm(
|
||||
@@ -283,6 +301,46 @@ impl FileManagementUseCase for FileManagementService {
|
||||
.await?;
|
||||
self.require_target_folder_perm(folder_id.as_deref(), Permission::Create, caller_id)
|
||||
.await?;
|
||||
|
||||
// D5 `forbid_cross_drive_move`: refuse when the destination
|
||||
// folder belongs to a different drive than the source file and
|
||||
// the source drive's policy is on. Silently skipped if the
|
||||
// drive repo isn't wired (stub builders) or the move target is
|
||||
// None (root namespace — same-drive semantics). Source policy
|
||||
// is canonical per §8: the drive that owns the content
|
||||
// controls outbound moves.
|
||||
if let Some(drive_repo) = &self.drive_repo
|
||||
&& let Some(target_folder_id) = folder_id.as_deref()
|
||||
{
|
||||
let file_uuid =
|
||||
Uuid::parse_str(file_id).map_err(|_| DomainError::not_found("File", file_id))?;
|
||||
let dst_folder_uuid = Uuid::parse_str(target_folder_id)
|
||||
.map_err(|_| DomainError::not_found("Folder", target_folder_id))?;
|
||||
let (src_drive_id, src_policies) = drive_repo
|
||||
.get_drive_id_and_policies_for_file(file_uuid)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("source drive lookup: {e:?}"))
|
||||
})?;
|
||||
let dst_drive_id = drive_repo
|
||||
.drive_id_for_folder(dst_folder_uuid)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("destination drive lookup: {e:?}"))
|
||||
})?;
|
||||
if src_drive_id != dst_drive_id {
|
||||
src_policies.refuse_cross_drive_move(
|
||||
crate::domain::entities::drive::CrossDriveMoveGateContext {
|
||||
caller_id,
|
||||
resource_type: "file",
|
||||
resource_id: file_uuid,
|
||||
src_drive_id,
|
||||
dst_drive_id,
|
||||
},
|
||||
)?;
|
||||
}
|
||||
}
|
||||
|
||||
self.move_file(file_id, folder_id, caller_id).await
|
||||
}
|
||||
|
||||
|
||||
@@ -25,6 +25,12 @@ pub struct FolderService {
|
||||
/// to reap. Always present — the dispatcher itself is a no-op when
|
||||
/// no hooks are registered, so callers don't need an Option branch.
|
||||
file_lifecycle: Arc<FileLifecycleService>,
|
||||
/// Drive repository — used by D5's `forbid_cross_drive_move` gate
|
||||
/// on `move_folder_with_perms`. Optional so stubs / test factories
|
||||
/// can build the service without wiring the full drive repo; in
|
||||
/// that case the cross-drive move check is skipped (the policy is
|
||||
/// silently off). Production DI wires it via `with_drive_repo`.
|
||||
drive_repo: Option<Arc<dyn crate::domain::repositories::drive_repository::DriveRepository>>,
|
||||
}
|
||||
|
||||
impl FolderService {
|
||||
@@ -38,9 +44,22 @@ impl FolderService {
|
||||
folder_storage,
|
||||
authz,
|
||||
file_lifecycle,
|
||||
drive_repo: None,
|
||||
}
|
||||
}
|
||||
|
||||
/// Wires the drive repository, enabling D5
|
||||
/// `forbid_cross_drive_move` enforcement on
|
||||
/// `move_folder_with_perms`. Without it, the gate is silently
|
||||
/// skipped.
|
||||
pub fn with_drive_repo(
|
||||
mut self,
|
||||
drive_repo: Arc<dyn crate::domain::repositories::drive_repository::DriveRepository>,
|
||||
) -> Self {
|
||||
self.drive_repo = Some(drive_repo);
|
||||
self
|
||||
}
|
||||
|
||||
/// Batch counterpart of `get_folder`: resolve many folder ids in ONE
|
||||
/// query instead of one per id. Like `get_folder` it performs no
|
||||
/// per-folder authorization — both current callers (ACL grant listing,
|
||||
@@ -539,6 +558,43 @@ impl FolderUseCase for FolderService {
|
||||
// TODO: full descendant-cycle check (moving a folder into one of its own descendants)
|
||||
}
|
||||
|
||||
// D5 `forbid_cross_drive_move`: refuse when src and dst sit in
|
||||
// different drives and the source drive's policy is on.
|
||||
// Skipped for parent_id=None (root namespace, same-drive
|
||||
// semantics) and when drive_repo isn't wired (stubs/tests) —
|
||||
// same shape as `move_file_with_perms`.
|
||||
if let Some(drive_repo) = &self.drive_repo
|
||||
&& let Some(parent_id) = &dto.parent_id
|
||||
{
|
||||
let src_folder_uuid =
|
||||
Uuid::parse_str(id).map_err(|_| DomainError::not_found("Folder", id))?;
|
||||
let dst_folder_uuid = Uuid::parse_str(parent_id)
|
||||
.map_err(|_| DomainError::not_found("Folder", parent_id.as_str()))?;
|
||||
let (src_drive_id, src_policies) = drive_repo
|
||||
.get_drive_id_and_policies_for_folder(src_folder_uuid)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("source drive lookup: {e:?}"))
|
||||
})?;
|
||||
let dst_drive_id = drive_repo
|
||||
.drive_id_for_folder(dst_folder_uuid)
|
||||
.await
|
||||
.map_err(|e| {
|
||||
DomainError::internal_error("Drive", format!("destination drive lookup: {e:?}"))
|
||||
})?;
|
||||
if src_drive_id != dst_drive_id {
|
||||
src_policies.refuse_cross_drive_move(
|
||||
crate::domain::entities::drive::CrossDriveMoveGateContext {
|
||||
caller_id,
|
||||
resource_type: "folder",
|
||||
resource_id: src_folder_uuid,
|
||||
src_drive_id,
|
||||
dst_drive_id,
|
||||
},
|
||||
)?;
|
||||
}
|
||||
}
|
||||
|
||||
let parent_ref = dto.parent_id.as_deref();
|
||||
let folder = self
|
||||
.folder_storage
|
||||
|
||||
@@ -247,9 +247,9 @@ impl ShareUseCase for ShareService {
|
||||
// disable anonymous-link creation on every resource in their
|
||||
// drive without per-resource intervention. Lookup is one JOIN
|
||||
// (`get_policies_for_file` / `_for_folder` — single round-trip);
|
||||
// a denial returns `OperationNotSupported` with an audit log
|
||||
// mirroring the per-drive membership refusal shape used in
|
||||
// `drive_management_service::refuse_if_personal`.
|
||||
// the decision + audit + canonical error live on
|
||||
// `DrivePolicies::refuse_public_links` so every public-link entry
|
||||
// point (future NC OCS share, etc.) refuses with the same shape.
|
||||
let item_uuid = Uuid::parse_str(&dto.item_id)
|
||||
.map_err(|_| ShareServiceError::Validation("Invalid item UUID".to_string()))?;
|
||||
let policies = match item_type {
|
||||
@@ -261,21 +261,15 @@ impl ShareUseCase for ShareService {
|
||||
}
|
||||
}
|
||||
.map_err(|e| ShareServiceError::Repository(e.to_string()))?;
|
||||
if policies.forbid_public_links {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "share.rejected",
|
||||
reason = "forbid_public_links",
|
||||
caller_id = %user_id,
|
||||
item_id = %dto.item_id,
|
||||
item_type = %dto.item_type,
|
||||
"👮🏻♂️ public-link creation refused: drive policy forbid_public_links",
|
||||
);
|
||||
return Err(DomainError::operation_not_supported(
|
||||
"Share",
|
||||
"This drive does not allow public links.",
|
||||
));
|
||||
}
|
||||
let item_type_str: &'static str = match item_type {
|
||||
ShareItemType::File => "file",
|
||||
ShareItemType::Folder => "folder",
|
||||
};
|
||||
policies.refuse_public_links(crate::domain::entities::drive::PublicLinkGateContext {
|
||||
caller_id: user_id,
|
||||
item_type: item_type_str,
|
||||
item_id: item_uuid,
|
||||
})?;
|
||||
|
||||
let password_hash = match dto.password {
|
||||
Some(p) => Some(self.hash_password_async(&p).await?),
|
||||
|
||||
Reference in New Issue
Block a user