feat(manager-core): admin is implicit app_admin; delete-script needs AppAdmin
Aligns the canonical capability rules with how the dashboard now shadows its UI. Instance admins become implicit app_admin on every app (only InstanceManageSettings stays owner-only), and the script-delete handler moves from AppWriteScript to AppAdmin so editors can save but not delete. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -270,10 +270,13 @@ async fn delete_script<R: ScriptRepository, L: ExecutionLogRepository>(
|
|||||||
Path(id): Path<ScriptId>,
|
Path(id): Path<ScriptId>,
|
||||||
) -> Result<StatusCode, ApiError> {
|
) -> Result<StatusCode, ApiError> {
|
||||||
let script = state.repo.get(id).await?.ok_or(ApiError::NotFound(id))?;
|
let script = state.repo.get(id).await?.ok_or(ApiError::NotFound(id))?;
|
||||||
|
// Delete is gated tighter than Save: editors can edit scripts but
|
||||||
|
// only app_admin / instance admin / owner can remove them. See
|
||||||
|
// blueprint §11.6.
|
||||||
require(
|
require(
|
||||||
state.authz.as_ref(),
|
state.authz.as_ref(),
|
||||||
&principal,
|
&principal,
|
||||||
Capability::AppWriteScript(script.app_id),
|
Capability::AppAdmin(script.app_id),
|
||||||
)
|
)
|
||||||
.await?;
|
.await?;
|
||||||
state.repo.delete(id).await?;
|
state.repo.delete(id).await?;
|
||||||
|
|||||||
@@ -143,8 +143,8 @@ pub struct AppLookupResponse {
|
|||||||
pub redirect_to: Option<String>,
|
pub redirect_to: Option<String>,
|
||||||
/// The caller's role on this app, used by the dashboard to decide
|
/// The caller's role on this app, used by the dashboard to decide
|
||||||
/// whether to render admin-only surfaces (Members tab, settings).
|
/// whether to render admin-only surfaces (Members tab, settings).
|
||||||
/// `Owner` maps to `app_admin`, `Admin` to `editor` (both implicit
|
/// `Owner` and `Admin` both map to `app_admin` (implicit per
|
||||||
/// per blueprint §11.6); `Member` carries its explicit
|
/// blueprint §11.6); `Member` carries its explicit
|
||||||
/// `app_members.role`.
|
/// `app_members.role`.
|
||||||
pub my_role: Option<AppRole>,
|
pub my_role: Option<AppRole>,
|
||||||
}
|
}
|
||||||
@@ -226,16 +226,15 @@ async fn get_app(
|
|||||||
/// Compute the caller's effective `AppRole` on a specific app. Mirrors
|
/// Compute the caller's effective `AppRole` on a specific app. Mirrors
|
||||||
/// the implicit-grant logic in `authz::role_grants` but returns the
|
/// the implicit-grant logic in `authz::role_grants` but returns the
|
||||||
/// role itself (for UI gating) rather than a yes/no decision. `Owner`
|
/// role itself (for UI gating) rather than a yes/no decision. `Owner`
|
||||||
/// is implicit `AppAdmin` everywhere; `Admin` is implicit `Editor`
|
/// and `Admin` are both implicit `AppAdmin` everywhere; `Member`
|
||||||
/// everywhere; `Member` consults `app_members`.
|
/// consults `app_members`.
|
||||||
async fn compute_my_role(
|
async fn compute_my_role(
|
||||||
authz: &dyn AuthzRepo,
|
authz: &dyn AuthzRepo,
|
||||||
principal: &Principal,
|
principal: &Principal,
|
||||||
app_id: AppId,
|
app_id: AppId,
|
||||||
) -> Result<Option<AppRole>, AppsApiError> {
|
) -> Result<Option<AppRole>, AppsApiError> {
|
||||||
match principal.instance_role {
|
match principal.instance_role {
|
||||||
InstanceRole::Owner => Ok(Some(AppRole::AppAdmin)),
|
InstanceRole::Owner | InstanceRole::Admin => Ok(Some(AppRole::AppAdmin)),
|
||||||
InstanceRole::Admin => Ok(Some(AppRole::Editor)),
|
|
||||||
InstanceRole::Member => Ok(authz.membership(principal.user_id, app_id).await?),
|
InstanceRole::Member => Ok(authz.membership(principal.user_id, app_id).await?),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -199,21 +199,14 @@ async fn role_grants(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Admin is implicit `editor` on every app (per blueprint §11.6). They
|
/// Admin is implicit `app_admin` on every app (per blueprint §11.6).
|
||||||
/// can create apps and manage users, but NOT touch instance-wide
|
/// They can create apps, manage users, and take any app-scoped action
|
||||||
/// settings or take app-admin-only actions on apps they're not
|
/// on any app without an explicit `app_members` row — single-human
|
||||||
/// explicitly app_admin of. Everything not in this set falls through
|
/// installs would otherwise need to add themselves to every new app.
|
||||||
/// to deny (`InstanceManageSettings`, `AppManageDomains`, `AppAdmin`).
|
/// Only `InstanceManageSettings` (sandbox ceiling, etc.) stays
|
||||||
|
/// owner-only.
|
||||||
const fn admin_grants(cap: Capability) -> bool {
|
const fn admin_grants(cap: Capability) -> bool {
|
||||||
matches!(
|
!matches!(cap, Capability::InstanceManageSettings)
|
||||||
cap,
|
|
||||||
Capability::InstanceCreateApp
|
|
||||||
| Capability::InstanceManageUsers
|
|
||||||
| Capability::AppRead(_)
|
|
||||||
| Capability::AppWriteScript(_)
|
|
||||||
| Capability::AppWriteRoute(_)
|
|
||||||
| Capability::AppLogRead(_)
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Member has zero instance authority. App authority requires an
|
/// Member has zero instance authority. App authority requires an
|
||||||
@@ -357,10 +350,23 @@ mod tests {
|
|||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn admin_cannot_manage_instance_settings_or_app_admin_actions() {
|
async fn admin_cannot_manage_instance_settings() {
|
||||||
|
let repo = InMemoryAuthzRepo::default();
|
||||||
|
let p = principal(InstanceRole::Admin);
|
||||||
|
assert_eq!(
|
||||||
|
can(&repo, &p, Capability::InstanceManageSettings)
|
||||||
|
.await
|
||||||
|
.unwrap(),
|
||||||
|
Decision::Deny,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[tokio::test]
|
||||||
|
async fn admin_is_implicit_app_admin_on_every_app() {
|
||||||
let repo = InMemoryAuthzRepo::default();
|
let repo = InMemoryAuthzRepo::default();
|
||||||
let p = principal(InstanceRole::Admin);
|
let p = principal(InstanceRole::Admin);
|
||||||
let app = AppId::new();
|
let app = AppId::new();
|
||||||
|
// Instance-scoped allowances.
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
can(&repo, &p, Capability::InstanceCreateApp).await.unwrap(),
|
can(&repo, &p, Capability::InstanceCreateApp).await.unwrap(),
|
||||||
Decision::Allow,
|
Decision::Allow,
|
||||||
@@ -371,36 +377,22 @@ mod tests {
|
|||||||
.unwrap(),
|
.unwrap(),
|
||||||
Decision::Allow,
|
Decision::Allow,
|
||||||
);
|
);
|
||||||
assert_eq!(
|
// Editor-like + app-admin grants both succeed without any
|
||||||
can(&repo, &p, Capability::InstanceManageSettings)
|
// app_members row.
|
||||||
.await
|
for cap in [
|
||||||
.unwrap(),
|
Capability::AppRead(app),
|
||||||
Decision::Deny,
|
Capability::AppWriteScript(app),
|
||||||
);
|
Capability::AppWriteRoute(app),
|
||||||
// Editor-like grants succeed
|
Capability::AppLogRead(app),
|
||||||
assert_eq!(
|
Capability::AppManageDomains(app),
|
||||||
can(&repo, &p, Capability::AppWriteScript(app))
|
Capability::AppAdmin(app),
|
||||||
.await
|
] {
|
||||||
.unwrap(),
|
assert_eq!(
|
||||||
Decision::Allow,
|
can(&repo, &p, cap).await.unwrap(),
|
||||||
);
|
Decision::Allow,
|
||||||
assert_eq!(
|
"admin denied app-scoped capability {cap:?}"
|
||||||
can(&repo, &p, Capability::AppWriteRoute(app))
|
);
|
||||||
.await
|
}
|
||||||
.unwrap(),
|
|
||||||
Decision::Allow,
|
|
||||||
);
|
|
||||||
// App-admin grants do not
|
|
||||||
assert_eq!(
|
|
||||||
can(&repo, &p, Capability::AppManageDomains(app))
|
|
||||||
.await
|
|
||||||
.unwrap(),
|
|
||||||
Decision::Deny,
|
|
||||||
);
|
|
||||||
assert_eq!(
|
|
||||||
can(&repo, &p, Capability::AppAdmin(app)).await.unwrap(),
|
|
||||||
Decision::Deny,
|
|
||||||
);
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
@@ -474,6 +466,29 @@ mod tests {
|
|||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Editors hold `AppWriteScript` (Save) but **not** `AppAdmin`
|
||||||
|
/// (Delete). The script-delete handler gates on the latter so the
|
||||||
|
/// API can't be tricked into letting an editor remove the script
|
||||||
|
/// they were only allowed to edit.
|
||||||
|
#[tokio::test]
|
||||||
|
async fn editor_can_write_scripts_but_not_delete_them() {
|
||||||
|
let repo = InMemoryAuthzRepo::default();
|
||||||
|
let p = principal(InstanceRole::Member);
|
||||||
|
let app = AppId::new();
|
||||||
|
repo.grant(p.user_id, app, AppRole::Editor).await;
|
||||||
|
|
||||||
|
assert!(can(&repo, &p, Capability::AppWriteScript(app))
|
||||||
|
.await
|
||||||
|
.unwrap()
|
||||||
|
.is_allow());
|
||||||
|
// Delete is gated on AppAdmin in the handler — editors must be
|
||||||
|
// denied here for that gate to bite.
|
||||||
|
assert_eq!(
|
||||||
|
can(&repo, &p, Capability::AppAdmin(app)).await.unwrap(),
|
||||||
|
Decision::Deny,
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[tokio::test]
|
#[tokio::test]
|
||||||
async fn member_with_app_admin_role_can_do_app_admin_actions() {
|
async fn member_with_app_admin_role_can_do_app_admin_actions() {
|
||||||
let repo = InMemoryAuthzRepo::default();
|
let repo = InMemoryAuthzRepo::default();
|
||||||
|
|||||||
@@ -293,7 +293,7 @@ async fn owner_access_matrix(pool: PgPool) {
|
|||||||
|
|
||||||
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
||||||
#[sqlx::test(migrations = "../manager-core/migrations")]
|
#[sqlx::test(migrations = "../manager-core/migrations")]
|
||||||
async fn admin_can_manage_users_but_not_app_admin_settings(pool: PgPool) {
|
async fn admin_is_implicit_app_admin_on_every_app(pool: PgPool) {
|
||||||
let s = boot(pool.clone()).await;
|
let s = boot(pool.clone()).await;
|
||||||
seed_user(&s.pool, "alice", "alice-pw", InstanceRole::Admin).await;
|
seed_user(&s.pool, "alice", "alice-pw", InstanceRole::Admin).await;
|
||||||
let token = login_token(&s.server, "alice", "alice-pw").await;
|
let token = login_token(&s.server, "alice", "alice-pw").await;
|
||||||
@@ -305,24 +305,32 @@ async fn admin_can_manage_users_but_not_app_admin_settings(pool: PgPool) {
|
|||||||
.await
|
.await
|
||||||
.assert_status_ok();
|
.assert_status_ok();
|
||||||
|
|
||||||
// Allowed: read default app (admin is implicit editor everywhere).
|
// Allowed: read default app — admin is implicit app_admin
|
||||||
|
// everywhere (per blueprint §11.6).
|
||||||
s.server
|
s.server
|
||||||
.get("/api/v1/admin/apps/default")
|
.get("/api/v1/admin/apps/default")
|
||||||
.add_header("authorization", format!("Bearer {token}"))
|
.add_header("authorization", format!("Bearer {token}"))
|
||||||
.await
|
.await
|
||||||
.assert_status_ok();
|
.assert_status_ok();
|
||||||
|
|
||||||
// Allowed: write scripts (implicit editor).
|
// Allowed: write scripts.
|
||||||
let script = create_script_via_api(&s.server, &token, s.default_app, "admin-write").await;
|
let script = create_script_via_api(&s.server, &token, s.default_app, "admin-write").await;
|
||||||
assert!(script["id"].is_string());
|
assert!(script["id"].is_string());
|
||||||
|
|
||||||
// Denied: delete the default app (AppAdmin only).
|
// Allowed: list app members (AppAdmin gate). Pre-3.5.x this
|
||||||
let denied = s
|
// 403'd; now it's the same allow as the owner sees.
|
||||||
.server
|
s.server
|
||||||
|
.get("/api/v1/admin/apps/default/members")
|
||||||
|
.add_header("authorization", format!("Bearer {token}"))
|
||||||
|
.await
|
||||||
|
.assert_status_ok();
|
||||||
|
|
||||||
|
// Allowed: delete the default app (AppAdmin).
|
||||||
|
s.server
|
||||||
.delete("/api/v1/admin/apps/default")
|
.delete("/api/v1/admin/apps/default")
|
||||||
.add_header("authorization", format!("Bearer {token}"))
|
.add_header("authorization", format!("Bearer {token}"))
|
||||||
.await;
|
.await
|
||||||
assert_eq!(denied.status_code(), axum::http::StatusCode::FORBIDDEN);
|
.assert_status(axum::http::StatusCode::NO_CONTENT);
|
||||||
}
|
}
|
||||||
|
|
||||||
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
#[ignore = "needs DATABASE_URL pointing at a running Postgres"]
|
||||||
@@ -735,7 +743,7 @@ async fn my_role_field_matches_caller_role(pool: PgPool) {
|
|||||||
"owner reports app_admin"
|
"owner reports app_admin"
|
||||||
);
|
);
|
||||||
|
|
||||||
// Admin → implicit editor everywhere.
|
// Admin → implicit app_admin everywhere (post-§11.6 update).
|
||||||
seed_user(&s.pool, "alice", "alice-pw", InstanceRole::Admin).await;
|
seed_user(&s.pool, "alice", "alice-pw", InstanceRole::Admin).await;
|
||||||
let admin_token = login_token(&s.server, "alice", "alice-pw").await;
|
let admin_token = login_token(&s.server, "alice", "alice-pw").await;
|
||||||
let r = s
|
let r = s
|
||||||
@@ -746,8 +754,8 @@ async fn my_role_field_matches_caller_role(pool: PgPool) {
|
|||||||
r.assert_status_ok();
|
r.assert_status_ok();
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
r.json::<Value>()["my_role"].as_str(),
|
r.json::<Value>()["my_role"].as_str(),
|
||||||
Some("editor"),
|
Some("app_admin"),
|
||||||
"admin reports editor"
|
"admin reports app_admin"
|
||||||
);
|
);
|
||||||
|
|
||||||
// Member with explicit `viewer` membership → viewer.
|
// Member with explicit `viewer` membership → viewer.
|
||||||
|
|||||||
@@ -1049,7 +1049,7 @@ pub struct Principal {
|
|||||||
| Role | Powers |
|
| Role | Powers |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `owner` | full instance control, manage other owners, implicit `app_admin` on every app. Multiple owners allowed. |
|
| `owner` | full instance control, manage other owners, implicit `app_admin` on every app. Multiple owners allowed. |
|
||||||
| `admin` | create apps, invite users, implicit `editor` on every app. Cannot manage instance-wide settings or other owners. |
|
| `admin` | create apps, invite users, implicit `app_admin` on every app. Cannot manage instance-wide settings (sandbox ceiling, etc.) or other owners. |
|
||||||
| `member` | invited into specific apps only. Cannot create apps, cannot invite. **Strict isolation enforced at SQL** — list endpoints `WHERE app_id IN (SELECT app_id FROM app_members WHERE user_id = $1)`; the API never returns apps a member isn't part of. |
|
| `member` | invited into specific apps only. Cannot create apps, cannot invite. **Strict isolation enforced at SQL** — list endpoints `WHERE app_id IN (SELECT app_id FROM app_members WHERE user_id = $1)`; the API never returns apps a member isn't part of. |
|
||||||
|
|
||||||
The current Phase 3a `admin_users` rows all become `owner` via `DEFAULT 'owner'` on the new column. Multi-owner installs get a startup `tracing::warn!` listing the active owner usernames so the operator can demote extras via `PATCH /api/v1/admin/admins/{id}`.
|
The current Phase 3a `admin_users` rows all become `owner` via `DEFAULT 'owner'` on the new column. Multi-owner installs get a startup `tracing::warn!` listing the active owner usernames so the operator can demote extras via `PATCH /api/v1/admin/admins/{id}`.
|
||||||
@@ -1058,11 +1058,13 @@ The current Phase 3a `admin_users` rows all become `owner` via `DEFAULT 'owner'`
|
|||||||
|
|
||||||
| Role | Grants |
|
| Role | Grants |
|
||||||
|---|---|
|
|---|---|
|
||||||
| `app_admin` | settings, domain claims, delete app |
|
| `app_admin` | settings, domain claims, delete app, **delete scripts** |
|
||||||
| `editor` | CRUD on scripts, routes, sandbox config |
|
| `editor` | create + edit scripts, routes, sandbox config (no script delete) |
|
||||||
| `viewer` | read scripts + execution logs |
|
| `viewer` | read scripts + execution logs |
|
||||||
|
|
||||||
Implicit grants from instance role: every `owner` is `app_admin` on every app; every `admin` is `editor` on every app. Explicit `app_members` rows are the only path for `member` users.
|
Implicit grants from instance role: every `owner` and every `admin` is `app_admin` on every app — a single-human install would otherwise have to add itself to each new app's `app_members`. Explicit `app_members` rows are the only path for `member` users.
|
||||||
|
|
||||||
|
Script **save** uses `AppWriteScript` (editor+); script **delete** uses `AppAdmin` (app_admin+). Editors can iterate on a script's source freely but cannot remove it — destructive cleanup stays with the role that also owns the app.
|
||||||
|
|
||||||
### Auth Methods — Same Principal, Different Extractor
|
### Auth Methods — Same Principal, Different Extractor
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user