diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs index 174fd9f..a8a85aa 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs @@ -9,6 +9,7 @@ pub struct ScopeInfo { pub description: String, pub display_name: String, pub granted: Option, + pub restricted: bool, } #[derive(Debug, Serialize)] @@ -23,6 +24,7 @@ pub struct PermissionSetInfo { pub include_scope: String, pub expanded: Vec, pub granted: Option, + pub restricted: bool, } #[derive(Debug, Serialize)] @@ -182,6 +184,13 @@ pub async fn consent_get( .unwrap_or(true); let has_granular_scopes = requested_scopes.iter().any(|s| is_granular_scope(s)); + let grant_scope_str: Option<&str> = delegation_grant + .as_ref() + .map(|g| g.granted_scopes.as_str()); + let is_restricted = |scope: &str| -> bool { + grant_scope_str.is_some_and(|g| !tranquil_pds::delegation::grant_covers(g, scope)) + }; + let make_scope_info = |scope: &str| -> ScopeInfo { let (category, required, description, display_name) = if let Some(def) = tranquil_pds::oauth::scopes::SCOPE_DEFINITIONS.get(scope) { @@ -225,6 +234,7 @@ pub async fn consent_get( description, display_name, granted, + restricted: is_restricted(scope), } }; @@ -244,6 +254,9 @@ pub async fn consent_get( Some(a) => format!("include:{}?aud={}", g.nsid, a), None => format!("include:{}", g.nsid), }; + let expanded: Vec = + g.expanded.iter().map(|s| make_scope_info(s)).collect(); + let restricted = !expanded.is_empty() && expanded.iter().all(|s| s.restricted); PermissionSetInfo { nsid: g.nsid.clone(), aud: g.aud.clone(), @@ -251,7 +264,8 @@ pub async fn consent_get( detail: g.detail.clone(), granted: pref_map.get(include_scope.as_str()).copied(), include_scope, - expanded: g.expanded.iter().map(|s| make_scope_info(s)).collect(), + expanded, + restricted, } }) .collect(); diff --git a/crates/tranquil-pds/src/delegation/mod.rs b/crates/tranquil-pds/src/delegation/mod.rs index 37f144d..9ae5f3f 100644 --- a/crates/tranquil-pds/src/delegation/mod.rs +++ b/crates/tranquil-pds/src/delegation/mod.rs @@ -6,7 +6,7 @@ pub use roles::{ }; pub use scopes::{ EDITOR_FULL_SCOPES, InvalidDelegationScopeError, OWNER_FULL_SCOPES, SCOPE_PRESETS, ScopePreset, - ValidatedDelegationScope, intersect_scopes, + ValidatedDelegationScope, grant_covers, 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 0f857e8..6c0eba0 100644 --- a/crates/tranquil-pds/src/delegation/scopes.rs +++ b/crates/tranquil-pds/src/delegation/scopes.rs @@ -63,6 +63,15 @@ pub fn intersect_scopes(requested: &str, granted: &str) -> String { scopes.join(" ") } +pub fn grant_covers(granted: &str, scope: &str) -> bool { + if scope == "atproto" { + return true; + } + let granted_parsed: Vec = + granted.split_whitespace().map(parse_scope).collect(); + any_granted_covers(scope, &granted_parsed) +} + fn any_granted_covers(requested: &str, granted: &[tranquil_scopes::ParsedScope]) -> bool { let requested_parsed = parse_scope(requested); granted.iter().any(|g| covers(g, &requested_parsed)) @@ -251,4 +260,32 @@ mod tests { }); }); } + + #[test] + fn test_grant_covers_matches_intersection() { + let granted = "atproto repo:* blob:*/* account:*?action=manage"; + let intersected = intersect_scopes( + "repo:app.bsky.feed.post?action=create identity:* account:*?action=manage", + granted, + ); + assert!(grant_covers(granted, "repo:app.bsky.feed.post?action=create")); + assert!(grant_covers(granted, "account:*?action=manage")); + assert!(!grant_covers(granted, "identity:*")); + assert_eq!( + grant_covers(granted, "identity:*"), + intersected.contains("identity") + ); + } + + #[test] + fn test_grant_covers_atproto_always_true() { + assert!(grant_covers("", "atproto")); + assert!(grant_covers("repo:*", "atproto")); + } + + #[test] + fn test_grant_covers_empty_grant_covers_nothing_else() { + assert!(!grant_covers("", "repo:app.bsky.feed.post?action=create")); + assert!(!grant_covers("", "identity:*")); + } } diff --git a/crates/tranquil-pds/tests/oauth_permission_sets.rs b/crates/tranquil-pds/tests/oauth_permission_sets.rs index ce8aeaf..eff8453 100644 --- a/crates/tranquil-pds/tests/oauth_permission_sets.rs +++ b/crates/tranquil-pds/tests/oauth_permission_sets.rs @@ -54,10 +54,15 @@ async fn setup_mock_client_metadata(redirect_uri: &str) -> MockServer { async fn seed_permission_set(nsid: &str, granular_scope: &str) { let state = common::get_test_app_state().await; let key = tranquil_pds::cache_keys::permission_set_key(&tranquil_types::Nsid::new(nsid).unwrap(), None); + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_secs(); let val = json!({ "scope": granular_scope, "title": "Basic", - "detail": null + "detail": null, + "refreshed_at": now }) .to_string(); state @@ -245,6 +250,62 @@ async fn create_delegated_session_with_scope( (session, consent_get_body, mock_client) } +#[tokio::test] +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( + "psr", + "https://example.com/permset-restricted-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 + ) + }); + + assert_eq!( + set_entry["restricted"].as_bool(), + Some(false), + "a partially-covered set must not be flagged fully restricted" + ); + + let expanded = set_entry["expanded"] + .as_array() + .expect("permission_sets entry should have an expanded array"); + + let repo = expanded + .iter() + .find(|s| s["scope"].as_str() == Some("repo:io.atcr.manifest?action=create")) + .expect("expanded[] should list the repo scope"); + assert_eq!( + repo["restricted"].as_bool(), + Some(false), + "repo scope is covered by the repo:* grant and must not be restricted" + ); + + 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(true), + "rpc scope is not conferred by the OWNER grant and must be restricted" + ); +} + #[tokio::test] async fn test_delegated_include_scope_shows_granular_on_consent() { seed_permission_set(PERMISSION_SET_NSID, PERMISSION_SET_GRANULAR_SCOPE).await; diff --git a/frontend/src/locales/en.json b/frontend/src/locales/en.json index 7941a88..c9c7b8a 100644 --- a/frontend/src/locales/en.json +++ b/frontend/src/locales/en.json @@ -598,8 +598,11 @@ "required": "Required", "rememberChoiceLabel": "Remember my choice for this application", "permissionSets": "Permission bundles", - "unavailableSets": "Unavailable permission bundles", "showIncludedScopes": "Show included permissions ({count})", + "unavailablePermissions": "Unavailable permissions", + "unavailableLimited": "Limited by delegation", + "unavailableFailed": "Failed to load", + "setPartiallyLimited": "Some permissions in this bundle are limited by your delegation; see \"Unavailable Permissions\" to find out which permissions weren't allowed.", "setFailureReason": { "not_found": "This bundle could not be found and cannot be granted.", "unreachable": "The bundle's publisher could not be reached; it cannot be granted right now.", diff --git a/frontend/src/routes/OAuthConsent.svelte b/frontend/src/routes/OAuthConsent.svelte index d9fc834..e77ce97 100644 --- a/frontend/src/routes/OAuthConsent.svelte +++ b/frontend/src/routes/OAuthConsent.svelte @@ -9,6 +9,7 @@ description: string display_name: string granted: boolean | null + restricted?: boolean } const SCOPE_LOCALE_MAP: Record = { @@ -40,6 +41,7 @@ include_scope: string expanded: ScopeInfo[] granted: boolean | null + restricted?: boolean } interface FailedSetInfo { @@ -142,13 +144,16 @@ consentData = data scopeSelections = Object.fromEntries( - data.scopes.map((scope) => [ - scope.scope, - scope.required ? true : scope.granted ?? true, - ]) + data.scopes + .filter((scope) => !scope.restricted) + .map((scope) => [ + scope.scope, + scope.required ? true : scope.granted ?? true, + ]) ) for (const set of data.permission_sets ?? []) { + if (set.restricted) continue scopeSelections[set.include_scope] = set.granted ?? true } @@ -274,7 +279,19 @@ fetchConsentData() }) - let scopeGroups = $derived(consentData ? groupScopesByCategory(consentData.scopes) : []) + let grantedScopes = $derived(consentData ? consentData.scopes.filter(s => !s.restricted) : []) + let approvableSets = $derived(consentData ? (consentData.permission_sets ?? []).filter(s => !s.restricted) : []) + let scopeGroups = $derived(groupScopesByCategory(grantedScopes)) + + let restrictedScopes = $derived(consentData ? consentData.scopes.filter(s => s.restricted) : []) + let limitedBundles = $derived( + consentData ? (consentData.permission_sets ?? []).filter(s => s.expanded.some(e => e.restricted)) : [] + ) + let failedSets = $derived(consentData?.failed_sets ?? []) + let hasUnavailable = $derived( + restrictedScopes.length > 0 || limitedBundles.length > 0 || failedSets.length > 0 + ) + let hasGranularScopes = $derived( (consentData?.scopes.some(s => isGranularScope(s.scope)) ?? false) || ((consentData?.permission_sets?.length ?? 0) > 0) @@ -335,6 +352,20 @@ } return { repo: [...repo.values()], rpc, other } } + + const grantedExpanded = (set: PermissionSetInfo) => set.expanded.filter(s => !s.restricted) + const withheldExpanded = (set: PermissionSetInfo) => set.expanded.filter(s => s.restricted) + function scopeLabel(scope: string): string { + const base = scope.split('?')[0] + if (base.startsWith('repo:')) { + const params = new URLSearchParams(scope.split('?')[1] ?? '') + const actions = params.getAll('action') + const coll = base.slice('repo:'.length) || '*' + return actions.length ? `${coll} (${actions.join(', ')})` : coll + } + if (base.startsWith('rpc:')) return base.slice('rpc:'.length) + return base + }
@@ -445,11 +476,13 @@ {/each} {/if} - {#if consentData.permission_sets?.length} + {#if approvableSets.length}

{$_('oauth.consent.permissionSets')}

- {#each consentData.permission_sets as set} - {@const desc = describeExpanded(set.expanded)} + {#each approvableSets as set} + {@const granted = grantedExpanded(set)} + {@const desc = describeExpanded(granted)} + {@const partiallyLimited = set.expanded.some(s => s.restricted)}