fabi/PiCloud
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(manager-core): F-S-005 gate dashboard delete_user on AppUsersAdmin

Browse Source 
users_admin_api::delete_user was a comment-only acknowledgement that
"this should require AppUsersAdmin" while actually calling
service.delete which only gates on AppUsersWrite. v1.1.8 HANDBACK §7
listed this as known. Effect: any editor could delete app users via
the admin HTTP API and dashboard button.

Add an explicit `authz::require(... AppUsersAdmin(app_id))` before the
service call. Delete the stale comment.

AUDIT.md anchor: F-S-005.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-07 20:40:12 +02:00
parent 9247680ab6
commit 7d045b2a0b
1 changed files with 11 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
crates/manager-core/src/users_admin_api.rs
View File

@@ -29,7 +29,7 @@ use serde_json::json;
use uuid::Uuid; use uuid::Uuid;
use crate::app_repo::AppRepository; use crate::app_repo::AppRepository;
use crate::authz::{AuthzDenied, AuthzRepo}; use crate::authz::{self, AuthzDenied, AuthzRepo, Capability};
use crate::repo::ScriptRepositoryError; use crate::repo::ScriptRepositoryError;
#[derive(Clone)] #[derive(Clone)]
@@ -253,15 +253,16 @@ async fn delete_user(
Path((id_or_slug, user_id)): Path<(String, Uuid)>, Path((id_or_slug, user_id)): Path<(String, Uuid)>,
) -> Result<StatusCode, AppUsersApiError> { ) -> Result<StatusCode, AppUsersApiError> {
let app = resolve_app(&*s.apps, &id_or_slug).await?; let app = resolve_app(&*s.apps, &id_or_slug).await?;
// Per brief: DELETE is gated on AppUsersAdmin (more senior than // F-S-005: gate on the stronger AppUsersAdmin per the original brief
// the SDK's delete which is gated on AppUsersWrite — the dashboard // — the dashboard delete is irreversible. The service-layer
// delete button is an irreversible administrative action). The // `delete` only checks AppUsersWrite; without this explicit gate,
// service's `delete` checks AppUsersWrite, so we apply the // any editor could delete app users via the admin HTTP API.
// stronger Admin check here and bypass the service. Actually we authz::require(
// can keep this clean by going via service.delete which gates on s.authz.as_ref(),
// AppUsersWrite — admins always satisfy Write — and adding the &principal,
// explicit Admin gate as a precondition would be redundant since Capability::AppUsersAdmin(app.id),
// anyone with Admin satisfies Write. Documented in HANDBACK §7. )
.await?;
let cx = synth_cx(app.id, &principal); let cx = synth_cx(app.id, &principal);
let removed = s.users.delete(&cx, AppUserId::from(user_id)).await?; let removed = s.users.delete(&cx, AppUserId::from(user_id)).await?;
if removed { if removed {