diff --git a/crates/tranquil-pds/src/delegation/mod.rs b/crates/tranquil-pds/src/delegation/mod.rs index 3bf6e5f..19a7201 100644 --- a/crates/tranquil-pds/src/delegation/mod.rs +++ b/crates/tranquil-pds/src/delegation/mod.rs @@ -5,8 +5,9 @@ pub use roles::{ CanAddControllers, CanControlAccounts, verify_can_add_controllers, verify_can_control_accounts, }; pub use scopes::{ - EDITOR_FULL_SCOPES, GrantCoverage, InvalidDelegationScopeError, OWNER_FULL_SCOPES, - SCOPE_PRESETS, ScopePreset, ValidatedDelegationScope, grant_coverage, intersect_scopes, + ADMIN_FULL_SCOPES, EDITOR_FULL_SCOPES, GrantCoverage, InvalidDelegationScopeError, + OWNER_FULL_SCOPES, SCOPE_PRESETS, ScopePreset, ValidatedDelegationScope, grant_coverage, + intersect_scopes, }; pub use tranquil_db_traits::DelegationActionType; diff --git a/crates/tranquil-pds/src/delegation/scopes.rs b/crates/tranquil-pds/src/delegation/scopes.rs index 008d33a..5b36d87 100644 --- a/crates/tranquil-pds/src/delegation/scopes.rs +++ b/crates/tranquil-pds/src/delegation/scopes.rs @@ -14,10 +14,13 @@ pub struct ScopePreset { pub scopes: &'static str, } -pub const OWNER_FULL_SCOPES: &str = "atproto repo:* blob:*/* identity:* account:*?action=manage"; +pub const OWNER_FULL_SCOPES: &str = + "atproto repo:* blob:*/* rpc:* identity:* account:*?action=manage"; + +pub const ADMIN_FULL_SCOPES: &str = "atproto repo:* blob:*/* rpc:* account:*?action=manage"; pub const EDITOR_FULL_SCOPES: &str = - "atproto repo:*?action=create repo:*?action=update repo:*?action=delete blob:*/*"; + "atproto repo:*?action=create repo:*?action=update repo:*?action=delete blob:*/* rpc:*"; pub const SCOPE_PRESETS: &[ScopePreset] = &[ ScopePreset { @@ -30,7 +33,7 @@ pub const SCOPE_PRESETS: &[ScopePreset] = &[ name: "admin", label: "Admin", description: "Manage account settings, post content, upload media", - scopes: "atproto repo:* blob:*/* account:*?action=manage", + scopes: ADMIN_FULL_SCOPES, }, ScopePreset { name: "editor", @@ -330,4 +333,94 @@ mod tests { GrantCoverage::Narrowed("repo:io.atcr.manifest?action=create".to_string()) ); } + + // Tracks all known scope prefixes + const GRANULAR_SCOPE_TAXONOMY: &[(&str, &str)] = &[ + ("repo", "repo:app.bsky.feed.post?action=create"), + ("blob", "blob:image/png"), + ("rpc", "rpc:app.bsky.actor.getProfile?aud=*"), + ("account", "account:email?action=manage"), + ("identity", "identity:handle"), + ]; + + // Verifies every scope prefix is reachable through at least one preset. + #[allow(dead_code)] + fn assert_taxonomy_is_exhaustive(scope: ParsedScope) { + match scope { + // Granular capabilities: each must appear in GRANULAR_SCOPE_TAXONOMY. + ParsedScope::Repo(_) + | ParsedScope::Blob(_) + | ParsedScope::Rpc(_) + | ParsedScope::Account(_) + | ParsedScope::Identity(_) => {} + ParsedScope::Atproto => {} + ParsedScope::TransitionGeneric + | ParsedScope::TransitionChat + | ParsedScope::TransitionEmail => {} + ParsedScope::Include(_) => {} + ParsedScope::Unknown(_) => {} + } + } + + fn coverage_matrix() -> String { + GRANULAR_SCOPE_TAXONOMY + .iter() + .map(|(label, scope)| { + let granting: Vec<&str> = SCOPE_PRESETS + .iter() + .filter(|p| grant_coverage(p.scopes, scope) != GrantCoverage::Withheld) + .map(|p| p.name) + .collect(); + match granting.is_empty() { + true => format!(" {:<9} ({}) -> NONE", label, scope), + false => format!(" {:<9} ({}) -> {}", label, scope, granting.join(", ")), + } + }) + .collect::>() + .join("\n") + } + + #[test] + fn test_every_granular_scope_type_is_reachable_through_some_preset() { + let unreachable: Vec<&str> = GRANULAR_SCOPE_TAXONOMY + .iter() + .filter(|(_, scope)| { + SCOPE_PRESETS + .iter() + .all(|p| grant_coverage(p.scopes, scope) == GrantCoverage::Withheld) + }) + .map(|(label, _)| *label) + .collect(); + + assert!( + unreachable.is_empty(), + "no delegation preset confers any `{}` scope, so delegated accounts cannot use \ + that capability at all.\ncoverage by preset:\n{}", + unreachable.join("`, `"), + coverage_matrix() + ); + } + + #[test] + fn test_forbidden_rpc_wildcard_is_not_a_usable_grant() { + // `rpc:*?aud=*` wildcards both lxm and aud, which the spec forbids, so it parses to + // Unknown and confers nothing. A preset reaching for it to mean "all rpc" would look + // right and silently grant nothing -- `rpc:*` is the form that works. + assert_eq!( + grant_coverage("atproto rpc:*?aud=*", "rpc:app.bsky.actor.getProfile?aud=*"), + GrantCoverage::Withheld + ); + assert_eq!( + grant_coverage("atproto rpc:*", "rpc:app.bsky.actor.getProfile?aud=*"), + GrantCoverage::Full + ); + } + + #[test] + fn test_forbidden_rpc_wildcard_request_stays_denied() { + assert_eq!( + grant_coverage("atproto rpc:*", "rpc:*?aud=*"), + GrantCoverage::Withheld + ); + } } diff --git a/crates/tranquil-pds/src/state.rs b/crates/tranquil-pds/src/state.rs index c83c889..6aaefd7 100644 --- a/crates/tranquil-pds/src/state.rs +++ b/crates/tranquil-pds/src/state.rs @@ -521,42 +521,68 @@ struct TranquilStoreWiring { } fn migrate_delegation_preset_scopes(metastore: &tranquil_store::metastore::Metastore) { - const MARKER_KEY: &str = "migration:delegation_preset_scopes_v1"; - const LEGACY_EDITOR_SCOPES: &str = + const V1_MARKER: &str = "migration:delegation_preset_scopes_v1"; + const V1_LEGACY_OWNER: &str = "atproto"; + const V1_LEGACY_EDITOR: &str = "repo:*?action=create repo:*?action=update repo:*?action=delete blob:*/*"; + // v2 adds `rpc:*` to the writing presets + // Without it delegated sessions can't hold rpc scopes + const V2_MARKER: &str = "migration:delegation_preset_scopes_v2"; + const V2_LEGACY_OWNER: &str = "atproto repo:* blob:*/* identity:* account:*?action=manage"; + const V2_LEGACY_ADMIN: &str = "atproto repo:* blob:*/* account:*?action=manage"; + const V2_LEGACY_EDITOR: &str = + "atproto repo:*?action=create repo:*?action=update repo:*?action=delete blob:*/*"; + + let passes: [(&str, &[(&str, &str)]); 2] = [ + ( + V1_MARKER, + &[ + (V1_LEGACY_OWNER, crate::delegation::OWNER_FULL_SCOPES), + (V1_LEGACY_EDITOR, crate::delegation::EDITOR_FULL_SCOPES), + ], + ), + ( + V2_MARKER, + &[ + (V2_LEGACY_OWNER, crate::delegation::OWNER_FULL_SCOPES), + (V2_LEGACY_ADMIN, crate::delegation::ADMIN_FULL_SCOPES), + (V2_LEGACY_EDITOR, crate::delegation::EDITOR_FULL_SCOPES), + ], + ), + ]; + let infra = metastore.infra_ops(); - if infra.get_server_config(MARKER_KEY).ok().flatten().is_some() { - return; - } - let ops = metastore.delegation_ops(); - let owners = match ops.remap_grant_scopes("atproto", crate::delegation::OWNER_FULL_SCOPES) { - Ok(n) => n, - Err(e) => { - tracing::error!(error = ?e, "delegation owner-scope migration failed, will retry on next start"); - return; - } - }; - let editors = match ops - .remap_grant_scopes(LEGACY_EDITOR_SCOPES, crate::delegation::EDITOR_FULL_SCOPES) - { - Ok(n) => n, - Err(e) => { - tracing::error!(error = ?e, "delegation editor-scope migration failed, will retry on next start"); - return; - } - }; - if owners + editors > 0 { - tracing::info!( - owners, - editors, - "upgraded legacy delegation grants to preset scopes" - ); - } - if let Err(e) = infra.upsert_server_config(MARKER_KEY, "done") { - tracing::error!(error = ?e, "failed to record delegation scope migration marker, will retry"); + for (marker, remaps) in passes { + if infra.get_server_config(marker).ok().flatten().is_some() { + continue; + } + + let mut migrated = 0usize; + for (from, to) in remaps { + match ops.remap_grant_scopes(from, to) { + Ok(n) => migrated += n, + Err(e) => { + tracing::error!(error = ?e, marker, from, "delegation scope migration failed, will retry on next start"); + return; + } + } + } + + if migrated > 0 { + tracing::info!( + marker, + migrated, + "upgraded legacy delegation grants to preset scopes" + ); + } + + if let Err(e) = infra.upsert_server_config(marker, "done") { + tracing::error!(error = ?e, marker, "failed to record delegation scope migration marker, will retry"); + return; + } } } diff --git a/crates/tranquil-pds/tests/oauth_permission_sets.rs b/crates/tranquil-pds/tests/oauth_permission_sets.rs index 5df9999..168f470 100644 --- a/crates/tranquil-pds/tests/oauth_permission_sets.rs +++ b/crates/tranquil-pds/tests/oauth_permission_sets.rs @@ -280,10 +280,11 @@ async fn test_delegated_consent_marks_restricted_scopes() { seed_permission_set(PERMISSION_SET_NSID, PERMISSION_SET_GRANULAR_SCOPE).await; let scope = format!("atproto include:{}", PERMISSION_SET_NSID); - let (_session, consent_body, _mock) = create_delegated_session_with_scope( + let (_session, consent_body, _mock) = create_delegated_session_with_grant( "psr", "https://example.com/permset-restricted-callback", &scope, + "atproto repo:*", ) .await; @@ -327,7 +328,53 @@ async fn test_delegated_consent_marks_restricted_scopes() { assert_eq!( rpc["restricted"].as_bool(), Some(true), - "rpc scope is not conferred by the OWNER grant and must be restricted" + "rpc scope is not conferred by a repo-only grant and must be restricted" + ); +} + +#[tokio::test] +async fn test_delegated_owner_grant_confers_rpc_scopes() { + seed_permission_set(PERMISSION_SET_NSID, PERMISSION_SET_GRANULAR_SCOPE).await; + + let scope = format!("atproto include:{}", PERMISSION_SET_NSID); + let (_session, consent_body, _mock) = create_delegated_session_with_scope( + "pso", + "https://example.com/permset-owner-rpc-callback", + &scope, + ) + .await; + + let set_entry = consent_body["permission_sets"] + .as_array() + .and_then(|sets| { + sets.iter() + .find(|s| s["nsid"].as_str() == Some(PERMISSION_SET_NSID)) + }) + .unwrap_or_else(|| { + panic!( + "expected a permission_sets entry for '{}'. Got: {:?}", + PERMISSION_SET_NSID, consent_body + ) + }); + + let expanded = set_entry["expanded"] + .as_array() + .expect("permission_sets entry should have an expanded array"); + + let rpc = expanded + .iter() + .find(|s| s["scope"].as_str() == Some("rpc:io.atcr.getManifest?aud=*")) + .expect("expanded[] should list the rpc scope"); + assert_eq!( + rpc["restricted"].as_bool(), + Some(false), + "the OWNER grant includes rpc:* and must confer rpc scopes" + ); + + assert_eq!( + set_entry["restricted"].as_bool(), + Some(false), + "a fully-covered set must not be flagged restricted" ); }