feat(drive): ensure drive_id updated on file|folder moved to another drive
This commit is contained in:
@@ -249,18 +249,25 @@ impl DriveManagementService {
|
||||
.set_role(caller_id, subject, role, resource, expires_at)
|
||||
.await?;
|
||||
|
||||
if caller_is_admin {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "drive_membership.set_via_admin",
|
||||
drive_id = %drive_id,
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
role = role.as_str(),
|
||||
by = %caller_id,
|
||||
"👮🏻♂️ admin set drive member role bypassing Manage check",
|
||||
);
|
||||
}
|
||||
// D6 §11: canonical `drive.member_added` audit event covers
|
||||
// every successful membership write (add + role-refresh, since
|
||||
// the underlying `set_role` is UPSERT — distinguishing the two
|
||||
// would require an additional read and bring no extra ops
|
||||
// value). `via_admin` carries the bypass signal that used to
|
||||
// live in a separate `drive_membership.set_via_admin` event;
|
||||
// log aggregators now have one canonical name per operation.
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "drive.member_added",
|
||||
drive_id = %drive_id,
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
role = role.as_str(),
|
||||
via_admin = caller_is_admin,
|
||||
by = %caller_id,
|
||||
expires_at = ?expires_at,
|
||||
"🤝 drive member added",
|
||||
);
|
||||
Ok(grant)
|
||||
}
|
||||
|
||||
@@ -305,17 +312,21 @@ impl DriveManagementService {
|
||||
|
||||
self.authz.clear_role(subject, resource).await?;
|
||||
|
||||
if caller_is_admin {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "drive_membership.removed_via_admin",
|
||||
drive_id = %drive_id,
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
by = %caller_id,
|
||||
"👮🏻♂️ admin removed drive member bypassing Manage check",
|
||||
);
|
||||
}
|
||||
// D6 §11: canonical `drive.member_removed` audit event covers
|
||||
// every successful removal (owner-driven or admin bypass).
|
||||
// `via_admin` replaces the separate
|
||||
// `drive_membership.removed_via_admin` event — single name,
|
||||
// one boolean field for the bypass signal.
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "drive.member_removed",
|
||||
drive_id = %drive_id,
|
||||
subject_type = subject.type_str(),
|
||||
subject_id = %subject.id(),
|
||||
via_admin = caller_is_admin,
|
||||
by = %caller_id,
|
||||
"👋 drive member removed",
|
||||
);
|
||||
Ok(())
|
||||
}
|
||||
|
||||
|
||||
@@ -302,13 +302,13 @@ impl FileManagementUseCase for FileManagementService {
|
||||
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.
|
||||
// D5 `forbid_cross_drive_move` + D6 `resource.moved_between_drives` audit
|
||||
// share the same src/dst drive_id lookup: the gate refuses
|
||||
// before the move; the audit fires after a successful move
|
||||
// when the two drives differ. Silently skipped if the drive
|
||||
// repo isn't wired (stub builders) or the move target is None
|
||||
// (root namespace — same-drive semantics).
|
||||
let mut cross_drive: Option<(Uuid, Uuid)> = None;
|
||||
if let Some(drive_repo) = &self.drive_repo
|
||||
&& let Some(target_folder_id) = folder_id.as_deref()
|
||||
{
|
||||
@@ -338,10 +338,29 @@ impl FileManagementUseCase for FileManagementService {
|
||||
dst_drive_id,
|
||||
},
|
||||
)?;
|
||||
cross_drive = Some((src_drive_id, dst_drive_id));
|
||||
}
|
||||
}
|
||||
|
||||
self.move_file(file_id, folder_id, caller_id).await
|
||||
let dto = self.move_file(file_id, folder_id, caller_id).await?;
|
||||
|
||||
// D6 §11 audit: emit only when the move actually crossed a
|
||||
// drive boundary. Same-drive moves are too noisy to audit at
|
||||
// info — operators care about the cross-drive case for
|
||||
// exfiltration / quota tracking.
|
||||
if let Some((src_drive_id, dst_drive_id)) = cross_drive {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "resource.moved_between_drives",
|
||||
resource_type = "file",
|
||||
resource_id = %dto.id,
|
||||
src_drive_id = %src_drive_id,
|
||||
dst_drive_id = %dst_drive_id,
|
||||
by = %caller_id,
|
||||
"📦 file moved between drives",
|
||||
);
|
||||
}
|
||||
Ok(dto)
|
||||
}
|
||||
|
||||
async fn copy_file_with_perms(
|
||||
|
||||
@@ -558,11 +558,13 @@ 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.
|
||||
// D5 `forbid_cross_drive_move` + D6 `resource.moved_between_drives`
|
||||
// audit share the same src/dst lookup. Gate before the move,
|
||||
// audit after a successful move when the two drives differ.
|
||||
// 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`.
|
||||
let mut cross_drive: Option<(Uuid, Uuid)> = None;
|
||||
if let Some(drive_repo) = &self.drive_repo
|
||||
&& let Some(parent_id) = &dto.parent_id
|
||||
{
|
||||
@@ -592,6 +594,7 @@ impl FolderUseCase for FolderService {
|
||||
dst_drive_id,
|
||||
},
|
||||
)?;
|
||||
cross_drive = Some((src_drive_id, dst_drive_id));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -607,6 +610,23 @@ impl FolderUseCase for FolderService {
|
||||
)
|
||||
})?;
|
||||
|
||||
// D6 audit: only emit when the move crossed a drive boundary.
|
||||
// The cascade trigger has already propagated drive_id to the
|
||||
// subtree at this point (see migration
|
||||
// `20260807000000_cascade_drive_id_on_folder_move.sql`).
|
||||
if let Some((src_drive_id, dst_drive_id)) = cross_drive {
|
||||
tracing::info!(
|
||||
target: "audit",
|
||||
event = "resource.moved_between_drives",
|
||||
resource_type = "folder",
|
||||
resource_id = %folder.id(),
|
||||
src_drive_id = %src_drive_id,
|
||||
dst_drive_id = %dst_drive_id,
|
||||
by = %caller_id,
|
||||
"📦 folder moved between drives",
|
||||
);
|
||||
}
|
||||
|
||||
Ok(FolderDto::from(folder))
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user