diff --git a/crates/common/src/limits/mod.rs b/crates/common/src/limits/mod.rs index 7c39b9a5..c996fe3e 100644 --- a/crates/common/src/limits/mod.rs +++ b/crates/common/src/limits/mod.rs @@ -870,12 +870,24 @@ pub async fn upsert_rule(pool: &PgPool, req: UpsertRule) -> Result Result { - let n = sqlx::query("DELETE FROM rate_limit_rules WHERE id = $1") - .bind(id) - .execute(pool) - .await? - .rows_affected(); +/// Delete one rule, but only if it belongs to the given subject. The +/// caller has authorized the subject, not the row id, so a row id +/// that belongs to someone else must not match. +pub async fn delete_rule( + pool: &PgPool, + id: Uuid, + subject_kind: RateLimitSubject, + subject_id: Uuid, +) -> Result { + let n = sqlx::query( + "DELETE FROM rate_limit_rules WHERE id = $1 AND subject_kind = $2 AND subject_id = $3", + ) + .bind(id) + .bind(subject_kind.as_str()) + .bind(subject_id) + .execute(pool) + .await? + .rows_affected(); Ok(n > 0) } @@ -970,12 +982,23 @@ pub async fn upsert_cap(pool: &PgPool, req: UpsertCap) -> Result Result { - let n = sqlx::query("DELETE FROM budget_caps WHERE id = $1") - .bind(id) - .execute(pool) - .await? - .rows_affected(); +/// Delete one cap, but only if it belongs to the given subject — see +/// [`delete_rule`]. +pub async fn delete_cap( + pool: &PgPool, + id: Uuid, + subject_kind: BudgetSubject, + subject_id: Uuid, +) -> Result { + let n = sqlx::query( + "DELETE FROM budget_caps WHERE id = $1 AND subject_kind = $2 AND subject_id = $3", + ) + .bind(id) + .bind(subject_kind.as_str()) + .bind(subject_id) + .execute(pool) + .await? + .rows_affected(); Ok(n > 0) } diff --git a/crates/server/src/handlers/limits.rs b/crates/server/src/handlers/limits.rs index c0d5ad92..1d49b64f 100644 --- a/crates/server/src/handlers/limits.rs +++ b/crates/server/src/handlers/limits.rs @@ -23,10 +23,11 @@ // containing the target subject) that covers `(kind, id)` from // the URL path. So a team_manager scoped to team:engineering // can edit limits on api_keys belonging to engineering members -// but gets 403 trying to touch marketing's keys. -// - Provider / mcp_server subjects always require global scope -// because they're platform-wide resources — see -// `AuthUser::assert_scope_for_subject`'s polymorphic dispatch. +// but gets 403 trying to touch marketing's keys. A team-scoped +// grant never covers the caller's own user or keys — see +// `AuthUser::assert_scope_for_subject`. +// - Deletes by row id are bound to the subject in the path, so an +// authorized subject can't be paired with someone else's row id. // ============================================================================ use axum::Json; @@ -439,11 +440,12 @@ pub async fn delete_rule( auth_user .assert_scope_for_subject(&state.db, "rate_limits:write", &kind, subject_id) .await?; - // Validate the kind even though we don't actually need it for - // the delete — keeps the URL shape consistent with the rest of - // the surface. - parse_rate_subject(&kind)?; - let removed = limits::delete_rule(&state.db, rule_id).await?; + // The scope check above authorized the subject in the URL, so the + // delete is bound to that subject: a rule id that belongs to + // someone else is "not found" rather than deleted. + let subject_kind = parse_rate_subject(&kind)?; + let storage_id = resolve_subject_id(&state.db, &kind, subject_id).await?; + let removed = limits::delete_rule(&state.db, rule_id, subject_kind, storage_id).await?; if !removed { return Err(AppError::NotFound("Rate limit rule not found".into())); } @@ -593,8 +595,10 @@ pub async fn delete_cap( auth_user .assert_scope_for_subject(&state.db, "rate_limits:write", &kind, subject_id) .await?; - parse_budget_subject(&kind)?; - let removed = limits::delete_cap(&state.db, cap_id).await?; + // Bound to the authorized subject — see `delete_rule`. + let subject_kind = parse_budget_subject(&kind)?; + let storage_id = resolve_subject_id(&state.db, &kind, subject_id).await?; + let removed = limits::delete_cap(&state.db, cap_id, subject_kind, storage_id).await?; if !removed { return Err(AppError::NotFound("Budget cap not found".into())); } diff --git a/crates/server/src/handlers/roles.rs b/crates/server/src/handlers/roles.rs index 5be21010..60b6d112 100644 --- a/crates/server/src/handlers/roles.rs +++ b/crates/server/src/handlers/roles.rs @@ -96,16 +96,14 @@ pub const PERMISSIONS: &[PermissionDef] = &[ // Teams are the unit of "scoped admin": a custom role with // `team_members:write` granted in scope `team:` lets the // holder add/remove members of that team without touching any - // other team. The CRUD perms below operate on the team - // catalog itself (rename, delete) and live at global scope - // because they're platform-wide bookkeeping. + // other team. `teams:read` at team scope shows that one team + // and its roster (the seeded team_manager holds both); at + // global scope it lists every team. Create / delete operate on + // the team catalog itself and need global scope. p("teams:read", "teams", "read"), p("teams:create", "teams", "create"), p("teams:update", "teams", "update"), d("teams:delete", "teams", "delete"), - // Membership management is the only perm intended to be - // granted at team scope. The handler accepts it at global - // scope too for super_admin convenience. d("team_members:write", "team_members", "write"), // --- Providers (AI upstream) --- p("providers:read", "providers", "read"), @@ -130,8 +128,6 @@ pub const PERMISSIONS: &[PermissionDef] = &[ p("users:create", "users", "create"), p("users:update", "users", "update"), d("users:delete", "users", "delete"), - p("team:read", "team", "read"), - p("team:write", "team", "write"), // --- Sessions (revoke other users) --- d("sessions:revoke", "sessions", "revoke"), // --- Roles & permissions (self-modifying — always dangerous) --- @@ -149,12 +145,9 @@ pub const PERMISSIONS: &[PermissionDef] = &[ p("analytics:read_own", "analytics", "read_own"), p("analytics:read_team", "analytics", "read_team"), p("analytics:read_all", "analytics", "read_all"), - p("audit_logs:read_own", "audit_logs", "read_own"), - p("audit_logs:read_team", "audit_logs", "read_team"), - p("audit_logs:read_all", "audit_logs", "read_all"), - // --- Gateway logs (raw request bodies — sensitive) --- - p("logs:read_own", "logs", "read_own"), - p("logs:read_team", "logs", "read_team"), + // --- Logs (gateway, MCP, audit, access and app logs) --- + // Every log endpoint is platform-wide and needs this at global + // scope; there is no per-user or per-team log view. d("logs:read_all", "logs", "read_all"), // Reading the raw request/response payload (prompts, completions, // tool arguments, tool results) is a strictly stronger right than @@ -194,6 +187,53 @@ pub(super) fn is_known_permission(key: &str) -> bool { PERMISSIONS.iter().any(|p| p.key == key) } +/// Keys that earlier releases put in the catalog and in the seeded +/// system roles, but that no handler ever checked: +/// +/// - `team:read` / `team:write` predate the `teams:*` and +/// `team_members:write` permissions the team handlers enforce. +/// - `logs:read_own` / `logs:read_team` and `audit_logs:*` — every +/// log endpoint (audit logs included) is gated on `logs:read_all` +/// at global scope; no own- or team-filtered log view exists. +/// +/// They are gone from the catalog, so the role editor no longer +/// offers them. Stored policies may still name them (the seeds only +/// insert missing roles, and custom roles could grant them), so the +/// startup check tolerates them with a warning instead of refusing to +/// boot. `db/release_migrations/2026-09-30_retire_unchecked_permissions.sql` +/// strips them. +pub(super) const RETIRED_PERMISSIONS: &[&str] = &[ + "team:read", + "team:write", + "logs:read_own", + "logs:read_team", + "audit_logs:read_own", + "audit_logs:read_team", + "audit_logs:read_all", +]; + +/// Split the permissions named by stored role policies into +/// `(unknown, retired)` entries, each formatted `"role: perm"`. +fn classify_role_permissions(rows: &[(String, serde_json::Value)]) -> (Vec, Vec) { + let all_perm_keys: Vec<&str> = PERMISSIONS.iter().map(|p| p.key).collect(); + let mut unknown = Vec::new(); + let mut retired = Vec::new(); + for (role_name, doc) in rows { + for perm in think_watch_common::limits::extract_permissions(doc, &all_perm_keys) { + if is_known_permission(&perm) { + continue; + } + let entry = format!("{role_name}: {perm}"); + if RETIRED_PERMISSIONS.contains(&perm.as_str()) { + retired.push(entry); + } else { + unknown.push(entry); + } + } + } + (unknown, retired) +} + /// Default policy document for each seeded system role. /// /// This is the **single source of truth** for "what should this @@ -212,19 +252,19 @@ pub const SYSTEM_ROLE_DEFAULTS: &[(&str, &str)] = &[ ), ( "admin", - r#"{"Version":"2024-01-01","Statement":[{"Sid":"AdminAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","api_keys:delete","api_keys:admin","providers:read","providers:create","providers:update","providers:delete","providers:rotate_key","models:read","models:write","mcp_servers:read","mcp_servers:create","mcp_servers:update","mcp_servers:delete","users:read","users:create","users:update","teams:read","teams:create","teams:update","teams:delete","team_members:write","team:read","team:write","sessions:revoke","roles:read","roles:create","roles:update","roles:delete","analytics:read_all","audit_logs:read_all","logs:read_all","log_forwarders:read","log_forwarders:write","webhooks:read","webhooks:write","content_filter:read","content_filter:write","pii_redactor:read","pii_redactor:write","rate_limits:read","rate_limits:write","settings:read","settings:write"],"Resource":"*"}]}"#, + r#"{"Version":"2024-01-01","Statement":[{"Sid":"AdminAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","api_keys:delete","api_keys:admin","providers:read","providers:create","providers:update","providers:delete","providers:rotate_key","models:read","models:write","mcp_servers:read","mcp_servers:create","mcp_servers:update","mcp_servers:delete","users:read","users:create","users:update","teams:read","teams:create","teams:update","teams:delete","team_members:write","sessions:revoke","roles:read","roles:create","roles:update","roles:delete","analytics:read_all","logs:read_all","log_forwarders:read","log_forwarders:write","webhooks:read","webhooks:write","content_filter:read","content_filter:write","pii_redactor:read","pii_redactor:write","rate_limits:read","rate_limits:write","settings:read","settings:write"],"Resource":"*"}]}"#, ), ( "team_manager", - r#"{"Version":"2024-01-01","Statement":[{"Sid":"TeamManagement","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","providers:read","models:read","mcp_servers:read","users:read","users:update","team_members:write","team:read","team:write","analytics:read_team","audit_logs:read_team","logs:read_team","rate_limits:read","rate_limits:write"],"Resource":"*"}]}"#, + r#"{"Version":"2024-01-01","Statement":[{"Sid":"TeamManagement","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","providers:read","models:read","mcp_servers:read","users:read","users:update","team_members:write","teams:read","analytics:read_team","rate_limits:read","rate_limits:write"],"Resource":"*"}]}"#, ), ( "developer", - r#"{"Version":"2024-01-01","Statement":[{"Sid":"DeveloperAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","providers:read","models:read","mcp_servers:read","analytics:read_own","audit_logs:read_own","logs:read_own"],"Resource":"*"}]}"#, + r#"{"Version":"2024-01-01","Statement":[{"Sid":"DeveloperAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","providers:read","models:read","mcp_servers:read","analytics:read_own"],"Resource":"*"}]}"#, ), ( "viewer", - r#"{"Version":"2024-01-01","Statement":[{"Sid":"ViewerAccess","Effect":"Allow","Action":["api_keys:read","providers:read","models:read","mcp_servers:read","analytics:read_own","audit_logs:read_own","logs:read_own"],"Resource":"*"}]}"#, + r#"{"Version":"2024-01-01","Statement":[{"Sid":"ViewerAccess","Effect":"Allow","Action":["api_keys:read","providers:read","models:read","mcp_servers:read","analytics:read_own"],"Resource":"*"}]}"#, ), ]; @@ -246,15 +286,14 @@ fn system_role_default_policy(name: &str) -> Option { /// fail-fast. pub async fn validate_seeded_roles(pool: &sqlx::PgPool) -> anyhow::Result<()> { let rows = repo::policy_documents(pool).await?; - let all_perm_keys: Vec<&str> = PERMISSIONS.iter().map(|p| p.key).collect(); - let mut unknown: Vec = Vec::new(); - for (role_name, doc) in &rows { - let perms = think_watch_common::limits::extract_permissions(doc, &all_perm_keys); - for perm in &perms { - if !is_known_permission(perm) { - unknown.push(format!("{role_name}: {perm}")); - } - } + let (unknown, retired) = classify_role_permissions(&rows); + if !retired.is_empty() { + tracing::warn!( + "Roles still grant retired permissions that nothing checks: {}. \ + Apply db/release_migrations/2026-09-30_retire_unchecked_permissions.sql \ + to remove them.", + retired.join(", "), + ); } if !unknown.is_empty() { anyhow::bail!( @@ -903,6 +942,73 @@ mod tests { assert!(is_known_permission("settings:write")); } + #[test] + fn unchecked_log_and_team_permissions_are_not_in_catalog() { + // No handler checks these; the catalog must not offer them. + for key in [ + "team:read", + "team:write", + "logs:read_own", + "logs:read_team", + "audit_logs:read_own", + "audit_logs:read_team", + "audit_logs:read_all", + ] { + assert!(!is_known_permission(key), "{key} is still in the catalog"); + } + } + + #[test] + fn team_manager_default_grants_the_team_read_the_handlers_check() { + // The team handlers gate listing a team, its roster and its + // roles on `teams:read`; the seeded team_manager must hold it. + let policy = system_role_default_policy("team_manager").unwrap(); + let actions: Vec<&str> = policy["Statement"][0]["Action"] + .as_array() + .unwrap() + .iter() + .filter_map(|v| v.as_str()) + .collect(); + assert!(actions.contains(&"teams:read"), "{actions:?}"); + assert!(actions.contains(&"team_members:write"), "{actions:?}"); + } + + #[test] + fn stored_roles_naming_retired_permissions_still_validate() { + // An install upgraded without the release migration still has + // the old seeds; that must warn, not refuse to boot. + let rows = vec![ + ( + "team_manager".to_string(), + serde_json::json!({"Statement": [{"Effect": "Allow", + "Action": ["team:read", "team:write", "logs:read_team", "teams:read"], + "Resource": "*"}]}), + ), + ( + "custom".to_string(), + serde_json::json!({"Statement": [{"Effect": "Allow", + "Action": ["audit_logs:read_all", "nope:read"], "Resource": "*"}]}), + ), + ]; + let (unknown, retired) = classify_role_permissions(&rows); + assert_eq!(unknown, vec!["custom: nope:read".to_string()]); + assert_eq!( + retired, + vec![ + "team_manager: logs:read_team".to_string(), + "team_manager: team:read".to_string(), + "team_manager: team:write".to_string(), + "custom: audit_logs:read_all".to_string(), + ] + ); + for key in RETIRED_PERMISSIONS { + assert!( + !is_known_permission(key), + "{key} is retired but in the catalog" + ); + } + } + #[test] fn is_known_permission_rejects_unknown() { assert!(!is_known_permission("")); diff --git a/crates/server/src/handlers/teams.rs b/crates/server/src/handlers/teams.rs index a71fc387..ff9cb92f 100644 --- a/crates/server/src/handlers/teams.rs +++ b/crates/server/src/handlers/teams.rs @@ -8,7 +8,8 @@ // // Auth model: // - `teams:read` — list / get a team's metadata + member list. -// Required at GLOBAL scope to list ALL teams. Members of a +// Required at GLOBAL scope to list ALL teams; scoped to a team +// it covers that one team (the seeded `team_manager`). Members of a // team can ALSO read their own team's metadata + roster as a // baseline knowledge right (no perm needed) — see // `caller_can_view_team`. @@ -95,9 +96,9 @@ async fn caller_is_team_member( Ok(exists) } -/// Allow a team read if caller has `teams:read` globally OR is a -/// member of the team in question. Members of a team always have -/// read access to their own team's metadata + roster. +/// Allow a team read if caller holds `teams:read` globally or scoped +/// to this team, OR is a member of the team in question. Members of a +/// team always have read access to their own team's metadata + roster. async fn assert_can_view_team( auth_user: &AuthUser, pool: &sqlx::PgPool, @@ -106,7 +107,9 @@ async fn assert_can_view_team( if caller_is_team_member(pool, auth_user.claims.sub, team_id).await? { return Ok(()); } - auth_user.assert_scope_global(pool, "teams:read").await + auth_user + .assert_scope_for_team(pool, "teams:read", team_id) + .await } // ---------------------------------------------------------------------------- diff --git a/crates/server/src/middleware/auth_guard.rs b/crates/server/src/middleware/auth_guard.rs index d585a521..22fca6cc 100644 --- a/crates/server/src/middleware/auth_guard.rs +++ b/crates/server/src/middleware/auth_guard.rs @@ -371,25 +371,56 @@ impl AuthUser { } /// Polymorphic scope check for the limits engine. The limits - /// CRUD endpoints are keyed on `(subject_kind, subject_id)` - /// where `subject_kind ∈ {user, api_key, role}`. All three are - /// admin-level writes and, for now, require the perm at global - /// scope — team-scoped grants are not enough to mutate another - /// team's user or key. A future revision could relax `user` / - /// `api_key` to allow team_manager-style scoping by looking up - /// the subject's team membership, but that's not needed today. + /// endpoints are keyed on `(subject_kind, subject_id)`: + /// + /// - `user` — global scope, or `perm` scoped to a team the user + /// belongs to. + /// - `api_key` (an `api_keys.id`) / `api_key_lineage` (a + /// `lineage_id`, as stored on override rows) — same rule, + /// applied to the key's owner. A service-account key (no + /// owner) or an id that doesn't resolve needs global scope. + /// - `role` — global scope only; roles are platform-wide. + /// + /// A team-scoped grant never covers the caller's own user or own + /// keys: otherwise a team manager, who is usually a member of the + /// team they manage, could lift their own rate limits and budget + /// caps. Only a global grant reaches the caller's own subject. pub async fn assert_scope_for_subject( &self, pool: &sqlx::PgPool, perm: &str, subject_kind: &str, - _subject_id: uuid::Uuid, + subject_id: uuid::Uuid, ) -> Result<(), AppError> { - match subject_kind { - "role" | "user" | "api_key" => self.assert_scope_global(pool, perm).await, - other => Err(AppError::BadRequest(format!( - "unknown subject_kind '{other}' (expected: user, api_key, role)" - ))), + let owner: Option = match subject_kind { + "role" => return self.assert_scope_global(pool, perm).await, + "user" => Some(subject_id), + "api_key" | "api_key_lineage" => { + let column = if subject_kind == "api_key" { + "id" + } else { + "lineage_id" + }; + let owner: Option> = sqlx::query_scalar(&format!( + "SELECT user_id FROM api_keys WHERE {column} = $1 LIMIT 1" + )) + .bind(subject_id) + .fetch_optional(pool) + .await + .map_err(|e| AppError::Internal(anyhow::anyhow!("scope check failed: {e}")))?; + owner.flatten() + } + other => { + return Err(AppError::BadRequest(format!( + "unknown subject_kind '{other}' (expected: user, api_key, role)" + ))); + } + }; + match owner { + Some(user_id) if user_id != self.claims.sub => { + self.assert_scope_for_user(pool, perm, user_id).await + } + _ => self.assert_scope_global(pool, perm).await, } } diff --git a/crates/test-support/tests/authz_horizontal.rs b/crates/test-support/tests/authz_horizontal.rs index 7573e1fb..76a615f5 100644 --- a/crates/test-support/tests/authz_horizontal.rs +++ b/crates/test-support/tests/authz_horizontal.rs @@ -359,9 +359,10 @@ async fn team_manager_a_can_add_member_to_own_team() { #[ignore = "integration test — run via `make test-it`"] #[tokio::test] async fn team_manager_a_cannot_write_limits_for_team_b_user() { - // Single-row limits writes go through `assert_scope_for_subject` - // which (for "user" subjects) requires global scope. A - // team-scoped manager should never reach the SQL path. + // Single-row limits writes go through `assert_scope_for_subject`, + // which for "user" subjects needs global scope or a team scope + // containing the user. A user outside manager A's team must never + // reach the SQL path. let app = TestApp::spawn().await; let team_a = make_team(&app.db, "team-a").await; let manager_a = fixtures::create_user_with_role(&app.db, "team_manager", "team", Some(team_a)) diff --git a/crates/test-support/tests/team_rbac.rs b/crates/test-support/tests/team_rbac.rs new file mode 100644 index 00000000..89b7f12e --- /dev/null +++ b/crates/test-support/tests/team_rbac.rs @@ -0,0 +1,413 @@ +//! What the seeded `team_manager` role can do when it is granted at +//! team scope — the shape its description says it is meant for. +//! +//! - read the team it manages: the team list, the team, its roster +//! and its roles (`teams:read`); +//! - manage rate limits and budget caps for the other members of that +//! team and their API keys (`rate_limits:read` / `rate_limits:write`), +//! but not for itself, not for anyone outside the team, and not by +//! pairing an in-scope subject with someone else's row id. +//! +//! The last test pins how gateway permissions from a team-scoped role +//! behave today: scope is not consulted on the gateway path. + +use serde_json::Value; +use think_watch_test_support::prelude::*; +use uuid::Uuid; + +async fn login(app: &TestApp, user: &fixtures::SeededUser) -> TestClient { + let con = app.console_client(); + con.post( + "/api/auth/login", + json!({"email": user.user.email, "password": user.plaintext_password}), + ) + .await + .unwrap() + .assert_ok(); + con +} + +async fn make_team(db: &sqlx::PgPool, prefix: &str) -> Uuid { + sqlx::query_scalar("INSERT INTO teams (name, description) VALUES ($1, 'rbac') RETURNING id") + .bind(unique_name(prefix)) + .fetch_one(db) + .await + .unwrap() +} + +async fn add_to_team(db: &sqlx::PgPool, team_id: Uuid, user_id: Uuid) { + sqlx::query("INSERT INTO team_members (team_id, user_id) VALUES ($1, $2)") + .bind(team_id) + .bind(user_id) + .execute(db) + .await + .unwrap(); +} + +async fn team_manager_of(app: &TestApp, team_id: Uuid) -> fixtures::SeededUser { + fixtures::create_user_with_role(&app.db, "team_manager", "team", Some(team_id)) + .await + .unwrap() +} + +fn rule_body(max_count: i64) -> Value { + json!({ + "surface": "ai_gateway", + "metric": "requests", + "window_secs": 60, + "max_count": max_count, + }) +} + +async fn rule_count(db: &sqlx::PgPool, subject_id: Uuid) -> i64 { + sqlx::query_scalar("SELECT count(*) FROM rate_limit_rules WHERE subject_id = $1") + .bind(subject_id) + .fetch_one(db) + .await + .unwrap() +} + +// --------------------------------------------------------------------------- +// teams:read +// --------------------------------------------------------------------------- + +#[ignore = "integration test — run via `make test-it`"] +#[tokio::test] +async fn team_manager_can_read_the_team_it_manages() { + let app = TestApp::spawn().await; + let team_a = make_team(&app.db, "tm-read-a").await; + let team_b = make_team(&app.db, "tm-read-b").await; + let member = fixtures::create_random_user(&app.db).await.unwrap(); + add_to_team(&app.db, team_a, member.user.id).await; + // The manager is NOT a member of team A: every read below has to + // come from the team-scoped grant, not the members' baseline right. + let manager = team_manager_of(&app, team_a).await; + let con = login(&app, &manager).await; + + let resp = con.get("/api/admin/teams").await.unwrap(); + resp.assert_ok(); + let teams: Vec = resp.json().unwrap(); + let ids: Vec<&str> = teams.iter().filter_map(|t| t["id"].as_str()).collect(); + assert_eq!(ids, vec![team_a.to_string()], "team list: {teams:?}"); + + con.get(&format!("/api/admin/teams/{team_a}")) + .await + .unwrap() + .assert_ok(); + + let resp = con + .get(&format!("/api/admin/teams/{team_a}/members")) + .await + .unwrap(); + resp.assert_ok(); + let members: Vec = resp.json().unwrap(); + assert_eq!(members.len(), 1); + assert_eq!(members[0]["user_id"], json!(member.user.id)); + + con.get(&format!("/api/admin/teams/{team_a}/roles")) + .await + .unwrap() + .assert_ok(); + + // Another team stays out of reach. + for path in [ + format!("/api/admin/teams/{team_b}"), + format!("/api/admin/teams/{team_b}/members"), + format!("/api/admin/teams/{team_b}/roles"), + ] { + con.get(&path).await.unwrap().assert_status(403); + } +} + +// --------------------------------------------------------------------------- +// rate_limits:read / rate_limits:write +// --------------------------------------------------------------------------- + +#[ignore = "integration test — run via `make test-it`"] +#[tokio::test] +async fn team_manager_can_manage_limits_for_members_of_its_team() { + let app = TestApp::spawn().await; + let team = make_team(&app.db, "tm-limits").await; + let member = fixtures::create_random_user(&app.db).await.unwrap(); + add_to_team(&app.db, team, member.user.id).await; + let key = fixtures::create_api_key( + &app.db, + member.user.id, + &unique_name("tm-key"), + &["ai_gateway"], + None, + None, + ) + .await + .unwrap(); + let manager = team_manager_of(&app, team).await; + let con = login(&app, &manager).await; + + // User subject: write, read, delete. + let resp = con + .post( + &format!("/api/admin/limits/user/{}/rules", member.user.id), + rule_body(10), + ) + .await + .unwrap(); + resp.assert_ok(); + let rule: Value = resp.json().unwrap(); + let rule_id = rule["id"].as_str().unwrap().to_string(); + let resp = con + .get(&format!("/api/admin/limits/user/{}/rules", member.user.id)) + .await + .unwrap(); + resp.assert_ok(); + let listed: Value = resp.json().unwrap(); + assert_eq!(listed["items"].as_array().unwrap().len(), 1); + con.post( + &format!("/api/admin/limits/user/{}/budgets", member.user.id), + json!({"period": "daily", "limit_tokens": 1000}), + ) + .await + .unwrap() + .assert_ok(); + con.delete(&format!( + "/api/admin/limits/user/{}/rules/{rule_id}", + member.user.id + )) + .await + .unwrap() + .assert_ok(); + assert_eq!(rule_count(&app.db, member.user.id).await, 0); + + // API key subject (persisted against the key's lineage). + con.post( + &format!("/api/admin/limits/api_key/{}/rules", key.row.id), + rule_body(5), + ) + .await + .unwrap() + .assert_ok(); + assert_eq!(rule_count(&app.db, key.row.lineage_id).await, 1); + + // Bulk apply across the team. + let resp = con + .post( + "/api/admin/limits/bulk/rules", + json!({ + "targets": [{"kind": "user", "id": member.user.id}], + "surface": "ai_gateway", + "metric": "requests", + "window_secs": 3600, + "max_count": 100, + }), + ) + .await + .unwrap(); + resp.assert_ok(); + let body: Value = resp.json().unwrap(); + assert_eq!(body["success_count"], json!(1), "{body}"); +} + +#[ignore = "integration test — run via `make test-it`"] +#[tokio::test] +async fn team_scoped_limits_grant_does_not_cover_the_manager_itself() { + // A team manager is usually a member of the team it manages. The + // team-scoped grant must not let it lift its own limits. + let app = TestApp::spawn().await; + let team = make_team(&app.db, "tm-self").await; + let manager = team_manager_of(&app, team).await; + add_to_team(&app.db, team, manager.user.id).await; + let own_key = fixtures::create_api_key( + &app.db, + manager.user.id, + &unique_name("tm-own-key"), + &["ai_gateway"], + None, + None, + ) + .await + .unwrap(); + let con = login(&app, &manager).await; + + con.post( + &format!("/api/admin/limits/user/{}/rules", manager.user.id), + rule_body(1_000_000), + ) + .await + .unwrap() + .assert_status(403); + con.post( + &format!("/api/admin/limits/api_key/{}/rules", own_key.row.id), + rule_body(1_000_000), + ) + .await + .unwrap() + .assert_status(403); + let resp = con + .post( + "/api/admin/limits/bulk/rules", + json!({ + "targets": [{"kind": "user", "id": manager.user.id}], + "surface": "ai_gateway", + "metric": "requests", + "window_secs": 60, + "max_count": 1_000_000, + }), + ) + .await + .unwrap(); + let body: Value = resp.json().unwrap(); + assert_eq!(body["success_count"], json!(0), "{body}"); + assert_eq!(rule_count(&app.db, manager.user.id).await, 0); + assert_eq!(rule_count(&app.db, own_key.row.lineage_id).await, 0); +} + +#[ignore = "integration test — run via `make test-it`"] +#[tokio::test] +async fn limits_delete_is_bound_to_the_subject_in_the_path() { + // Scope is checked on the subject in the URL; the row id must + // belong to that subject, or a manager could pair its own member's + // id with an outsider's rule id. + let app = TestApp::spawn().await; + let team = make_team(&app.db, "tm-bound").await; + let member = fixtures::create_random_user(&app.db).await.unwrap(); + add_to_team(&app.db, team, member.user.id).await; + let outsider = fixtures::create_random_user(&app.db).await.unwrap(); + let outsider_rule = fixtures::create_rate_limit_rule( + &app.db, + "user", + outsider.user.id, + "ai_gateway", + "requests", + 60, + 100, + ) + .await + .unwrap(); + let outsider_cap = + fixtures::create_budget_cap(&app.db, "user", outsider.user.id, "daily", 1000) + .await + .unwrap(); + let manager = team_manager_of(&app, team).await; + let con = login(&app, &manager).await; + + con.delete(&format!( + "/api/admin/limits/user/{}/rules/{outsider_rule}", + member.user.id + )) + .await + .unwrap() + .assert_status(404); + con.delete(&format!( + "/api/admin/limits/user/{}/budgets/{outsider_cap}", + member.user.id + )) + .await + .unwrap() + .assert_status(404); + assert_eq!(rule_count(&app.db, outsider.user.id).await, 1); + let caps: i64 = sqlx::query_scalar("SELECT count(*) FROM budget_caps WHERE id = $1") + .bind(outsider_cap) + .fetch_one(&app.db) + .await + .unwrap(); + assert_eq!(caps, 1); +} + +// --------------------------------------------------------------------------- +// Gateway permissions from a team-scoped role (current behaviour) +// --------------------------------------------------------------------------- + +#[ignore = "integration test — run via `make test-it`"] +#[tokio::test] +async fn team_scoped_role_widens_gateway_model_access_platform_wide() { + // Pins today's behaviour: gateway requests carry no team, and the + // model allow-list is the union of every role the user holds, + // whatever its scope. A role granted at scope `team:` therefore + // widens model access for every request the user makes — the user + // does not even have to be a member of that team. + let app = TestApp::spawn().await; + let upstream = MockProvider::openai_chat_ok("tm-model-a").await; + let provider = fixtures::create_provider( + &app.db, + &unique_name("tm-prov"), + "openai", + &upstream.uri(), + None, + ) + .await + .unwrap(); + fixtures::create_model_and_route(&app.db, provider.id, "tm-model-a") + .await + .unwrap(); + fixtures::create_model_and_route(&app.db, provider.id, "tm-model-b") + .await + .unwrap(); + app.rebuild_gateway_router().await; + + let only_a: Uuid = sqlx::query_scalar( + r#"INSERT INTO rbac_roles (name, is_system, policy_document) + VALUES ($1, FALSE, '{"Version":"2024-01-01","Statement":[{"Effect":"Allow", + "Action":["ai_gateway:use"],"Resource":["model:tm-model-a"]}]}') + RETURNING id"#, + ) + .bind(unique_name("only-model-a")) + .fetch_one(&app.db) + .await + .unwrap(); + let user = fixtures::create_user(&app.db, &unique_email(), "Scoped", "ScopedPwd_12345!") + .await + .unwrap(); + sqlx::query( + "INSERT INTO rbac_role_assignments (user_id, role_id, scope_kind, assigned_by) + VALUES ($1, $2, 'global', $1)", + ) + .bind(user.user.id) + .bind(only_a) + .execute(&app.db) + .await + .unwrap(); + let key = fixtures::create_api_key( + &app.db, + user.user.id, + &unique_name("tm-gw-key"), + &["ai_gateway"], + None, + None, + ) + .await + .unwrap(); + let gw = app.gateway_client(); + gw.set_bearer(&key.plaintext); + let call = |model: &'static str| json!({"model": model, "messages": [{"role": "user", "content": "hi"}]}); + + gw.post("/v1/chat/completions", call("tm-model-a")) + .await + .unwrap() + .assert_ok(); + let denied = gw + .post("/v1/chat/completions", call("tm-model-b")) + .await + .unwrap(); + assert!( + !denied.status.is_success(), + "model-b must be refused under the global model-a-only role: {}", + denied.text() + ); + + // Grant `developer` (Resource "*") scoped to a team the user is + // not a member of. + let team = make_team(&app.db, "tm-gw").await; + sqlx::query( + r#"INSERT INTO rbac_role_assignments (user_id, role_id, scope_kind, scope_id, assigned_by) + SELECT $1, id, 'team', $2, $1 FROM rbac_roles WHERE name = 'developer'"#, + ) + .bind(user.user.id) + .bind(team) + .execute(&app.db) + .await + .unwrap(); + + gw.post("/v1/chat/completions", call("tm-model-b")) + .await + .unwrap() + .assert_ok(); +} diff --git a/db/release_migrations/2026-09-30_retire_unchecked_permissions.sql b/db/release_migrations/2026-09-30_retire_unchecked_permissions.sql new file mode 100644 index 00000000..709ce9b7 --- /dev/null +++ b/db/release_migrations/2026-09-30_retire_unchecked_permissions.sql @@ -0,0 +1,104 @@ +-- 2026-09-30: team_manager gets teams:read; retire permissions nothing checks +-- +-- What changed: +-- * The seeded `team_manager` role granted `team:read` / `team:write`, +-- but the team handlers check `teams:read` (list a team, its roster, +-- its roles). A team manager could not open the team they manage. +-- The seed now grants `teams:read` instead. +-- * `team:read`, `team:write`, `logs:read_own`, `logs:read_team`, +-- `audit_logs:read_own`, `audit_logs:read_team` and +-- `audit_logs:read_all` were in the permission catalog and in the +-- seeded roles, but no handler ever checked them (every log endpoint, +-- audit logs included, is gated on `logs:read_all` at global scope). +-- They are gone from the catalog and from the seeds. +-- +-- Why this file: +-- `db/seeds.sql` only inserts missing roles, so an existing database +-- keeps the old system-role policies. The server still boots with them +-- (the startup check logs a warning for retired keys instead of +-- failing), but team managers stay unable to read their team until the +-- policy is updated. This file: +-- 1. adds `teams:read` to `team_manager`'s Allow statements that still +-- carry the legacy `team:read` (a role an operator already edited +-- to drop `team:read` is left alone); +-- 2. removes the retired keys from every role, system and custom. +-- Removing them changes no access — nothing checked them. +-- Clicking "Reset to defaults" on a system role in the console has the +-- same effect for that role. +-- +-- Re-running is a no-op: after step 2 no role names `team:read`, so +-- step 1 matches nothing, and step 2 matches nothing. +-- +-- The rewrite does not add rows to rbac_role_history. Each user's +-- permission set is cached in Redis for 60 seconds, so team managers +-- see `teams:read` within a minute of the commit. +-- +-- Applied to: +-- - dev: pending +-- - stage: pending +-- - prod: pending + +BEGIN; + +-- Step 1. team_manager: team:read -> teams:read. +UPDATE rbac_roles r + SET policy_document = jsonb_set( + r.policy_document, + '{Statement}', + (SELECT jsonb_agg( + CASE + WHEN stmt->>'Effect' = 'Allow' + AND jsonb_typeof(stmt->'Action') = 'array' + AND stmt->'Action' ? 'team:read' + AND NOT stmt->'Action' ? 'teams:read' + THEN jsonb_set(stmt, '{Action}', (stmt->'Action') || '["teams:read"]'::jsonb) + ELSE stmt + END + ORDER BY ord) + FROM jsonb_array_elements(r.policy_document->'Statement') + WITH ORDINALITY AS s(stmt, ord))), + updated_at = now() + WHERE r.name = 'team_manager' + AND r.is_system + AND jsonb_typeof(r.policy_document->'Statement') = 'array' + AND jsonb_path_exists(r.policy_document, '$.Statement[*].Action[*] ? (@ == "team:read")'); + +-- Step 2. Strip the retired keys from every role's Action arrays. +UPDATE rbac_roles r + SET policy_document = jsonb_set( + r.policy_document, + '{Statement}', + (SELECT jsonb_agg( + CASE + WHEN jsonb_typeof(stmt->'Action') = 'array' + THEN jsonb_set( + stmt, + '{Action}', + (SELECT COALESCE(jsonb_agg(a ORDER BY o), '[]'::jsonb) + FROM jsonb_array_elements(stmt->'Action') + WITH ORDINALITY AS x(a, o) + WHERE a #>> '{}' NOT IN ( + 'team:read', 'team:write', + 'logs:read_own', 'logs:read_team', + 'audit_logs:read_own', 'audit_logs:read_team', + 'audit_logs:read_all'))) + ELSE stmt + END + ORDER BY ord) + FROM jsonb_array_elements(r.policy_document->'Statement') + WITH ORDINALITY AS s(stmt, ord))), + updated_at = now() + WHERE jsonb_typeof(r.policy_document->'Statement') = 'array' + AND jsonb_path_exists( + r.policy_document, + '$.Statement[*].Action[*] ? (@ == "team:read" || @ == "team:write" + || @ == "logs:read_own" || @ == "logs:read_team" + || @ == "audit_logs:read_own" || @ == "audit_logs:read_team" + || @ == "audit_logs:read_all")'); + +-- Check: no role should list here. +SELECT name, stmt->'Action' AS actions + FROM rbac_roles, jsonb_array_elements(policy_document->'Statement') AS stmt + WHERE jsonb_path_exists(stmt, '$.Action[*] ? (@ like_regex "^(team:(read|write)|logs:read_(own|team)|audit_logs:)")'); + +COMMIT; diff --git a/db/seeds.sql b/db/seeds.sql index f81ea791..66dd1f92 100644 --- a/db/seeds.sql +++ b/db/seeds.sql @@ -22,22 +22,22 @@ INSERT INTO rbac_roles (name, description, is_system, policy_document) VALUES ('admin', 'Administrative access. Manages providers, MCP servers, API keys, and users.', TRUE, - '{"Version":"2024-01-01","Statement":[{"Sid":"AdminAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","api_keys:delete","api_keys:admin","providers:read","providers:create","providers:update","providers:delete","providers:rotate_key","models:read","models:write","mcp_servers:read","mcp_servers:create","mcp_servers:update","mcp_servers:delete","users:read","users:create","users:update","teams:read","teams:create","teams:update","teams:delete","team_members:write","team:read","team:write","sessions:revoke","roles:read","roles:create","roles:update","roles:delete","analytics:read_all","audit_logs:read_all","logs:read_all","log_forwarders:read","log_forwarders:write","webhooks:read","webhooks:write","content_filter:read","content_filter:write","pii_redactor:read","pii_redactor:write","rate_limits:read","rate_limits:write","settings:read","settings:write"],"Resource":"*"}]}' + '{"Version":"2024-01-01","Statement":[{"Sid":"AdminAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","api_keys:delete","api_keys:admin","providers:read","providers:create","providers:update","providers:delete","providers:rotate_key","models:read","models:write","mcp_servers:read","mcp_servers:create","mcp_servers:update","mcp_servers:delete","users:read","users:create","users:update","teams:read","teams:create","teams:update","teams:delete","team_members:write","sessions:revoke","roles:read","roles:create","roles:update","roles:delete","analytics:read_all","logs:read_all","log_forwarders:read","log_forwarders:write","webhooks:read","webhooks:write","content_filter:read","content_filter:write","pii_redactor:read","pii_redactor:write","rate_limits:read","rate_limits:write","settings:read","settings:write"],"Resource":"*"}]}' ), ('team_manager', 'Team-level management. Manages members, API keys, and rate limits for the team it''s assigned to. Intended to be granted with scope_kind = team.', TRUE, - '{"Version":"2024-01-01","Statement":[{"Sid":"TeamManagement","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","providers:read","models:read","mcp_servers:read","users:read","users:update","team_members:write","team:read","team:write","analytics:read_team","audit_logs:read_team","logs:read_team","rate_limits:read","rate_limits:write"],"Resource":"*"}]}' + '{"Version":"2024-01-01","Statement":[{"Sid":"TeamManagement","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","api_keys:rotate","providers:read","models:read","mcp_servers:read","users:read","users:update","team_members:write","teams:read","analytics:read_team","rate_limits:read","rate_limits:write"],"Resource":"*"}]}' ), ('developer', 'Standard developer. Uses the gateway, manages own API keys, sees own usage.', TRUE, - '{"Version":"2024-01-01","Statement":[{"Sid":"DeveloperAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","providers:read","models:read","mcp_servers:read","analytics:read_own","audit_logs:read_own","logs:read_own"],"Resource":"*"}]}' + '{"Version":"2024-01-01","Statement":[{"Sid":"DeveloperAccess","Effect":"Allow","Action":["ai_gateway:use","mcp_gateway:use","mcp:connect","api_keys:read","api_keys:create","api_keys:update","providers:read","models:read","mcp_servers:read","analytics:read_own"],"Resource":"*"}]}' ), ('viewer', 'Read-only access. Can browse providers and analytics but not modify anything.', TRUE, - '{"Version":"2024-01-01","Statement":[{"Sid":"ViewerAccess","Effect":"Allow","Action":["api_keys:read","providers:read","models:read","mcp_servers:read","analytics:read_own","audit_logs:read_own","logs:read_own"],"Resource":"*"}]}' + '{"Version":"2024-01-01","Statement":[{"Sid":"ViewerAccess","Effect":"Allow","Action":["api_keys:read","providers:read","models:read","mcp_servers:read","analytics:read_own"],"Resource":"*"}]}' ) ON CONFLICT (name) DO NOTHING; INSERT INTO api_key_surface_kinds (name, display_name, description) VALUES diff --git a/web/scripts/check-i18n.mjs b/web/scripts/check-i18n.mjs index 98e4085b..ce34507b 100644 --- a/web/scripts/check-i18n.mjs +++ b/web/scripts/check-i18n.mjs @@ -42,8 +42,8 @@ const DYNAMIC_ENUMS = { // through to the raw key (shown uppercased) in the permission tree. 'permissions.resource.${_}': [ 'ai_gateway', 'mcp_gateway', 'api_keys', 'providers', 'mcp_servers', - 'models', 'users', 'team', 'teams', 'team_members', 'sessions', - 'roles', 'analytics', 'audit_logs', 'logs', 'log_forwarders', + 'models', 'users', 'teams', 'team_members', 'sessions', + 'roles', 'analytics', 'logs', 'log_forwarders', 'webhooks', 'content_filter', 'pii_redactor', 'rate_limits', 'settings', 'system', ], diff --git a/web/src/i18n/en.json b/web/src/i18n/en.json index 2ce9d718..f7b2673b 100644 --- a/web/src/i18n/en.json +++ b/web/src/i18n/en.json @@ -1214,15 +1214,13 @@ "providers": "Providers", "mcp_servers": "MCP servers", "users": "Users", - "team": "Team", "teams": "Teams", "team_members": "Team members", "models": "Models", "sessions": "Sessions", "roles": "Roles", "analytics": "Analytics", - "audit_logs": "Audit logs", - "logs": "Gateway logs", + "logs": "Logs", "log_forwarders": "Log forwarders", "webhooks": "Webhooks", "content_filter": "Content filter", diff --git a/web/src/i18n/zh.json b/web/src/i18n/zh.json index 0cf22ce1..3deb184e 100644 --- a/web/src/i18n/zh.json +++ b/web/src/i18n/zh.json @@ -1214,15 +1214,13 @@ "providers": "提供商", "mcp_servers": "MCP 服务器", "users": "用户", - "team": "所属团队", "teams": "团队", "team_members": "团队成员", "models": "模型", "sessions": "会话", "roles": "角色", "analytics": "分析", - "audit_logs": "审计日志", - "logs": "网关日志", + "logs": "日志", "log_forwarders": "日志转发器", "webhooks": "Webhook", "content_filter": "内容过滤", diff --git a/web/src/routes/admin/roles/types.ts b/web/src/routes/admin/roles/types.ts index 52e5a4c2..55f6c015 100644 --- a/web/src/routes/admin/roles/types.ts +++ b/web/src/routes/admin/roles/types.ts @@ -204,8 +204,6 @@ export const SIMPLE_TEMPLATES: SimpleTemplate[] = [ 'providers:read', 'mcp_servers:read', 'analytics:read_own', - 'audit_logs:read_own', - 'logs:read_own', ], }, // Read-only across the surface a non-admin can browse. @@ -219,8 +217,6 @@ export const SIMPLE_TEMPLATES: SimpleTemplate[] = [ 'mcp_servers:read', 'roles:read', 'analytics:read_own', - 'audit_logs:read_own', - 'logs:read_own', 'settings:read', 'log_forwarders:read', 'webhooks:read', @@ -253,7 +249,6 @@ export const SIMPLE_TEMPLATES: SimpleTemplate[] = [ 'mcp_servers:update', 'mcp_servers:delete', 'analytics:read_all', - 'audit_logs:read_all', 'logs:read_all', 'log_forwarders:read', 'log_forwarders:write', @@ -269,7 +264,7 @@ export const SIMPLE_TEMPLATES: SimpleTemplate[] = [ // Analytics-only viewer (e.g. an SRE dashboard or finance owner). { id: 'analytics_only', - permissions: ['analytics:read_all', 'audit_logs:read_all', 'logs:read_all'], + permissions: ['analytics:read_all', 'logs:read_all'], }, ];