diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs index 336abc7..174fd9f 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs @@ -30,7 +30,7 @@ pub struct FailedSetInfo { pub nsid: String, #[serde(skip_serializing_if = "Option::is_none")] pub aud: Option, - pub reason: String, + pub reason: tranquil_scopes::ResolveFailure, } #[derive(Debug, Serialize)] @@ -142,7 +142,7 @@ pub async fn consent_get( }; let authority = match delegation_grant.as_ref() { - Some(grant) => scope_resolution::Authority::Delegated(grant.granted_scopes.as_str()), + Some(grant) => scope_resolution::Authority::Delegated(&grant.granted_scopes), None => scope_resolution::Authority::FullSelf, }; let effective = scope_resolution::resolve_effective_scopes( @@ -263,12 +263,7 @@ pub async fn consent_get( .map(|f| FailedSetInfo { nsid: f.nsid.clone(), aud: f.aud.clone(), - reason: match f.reason { - tranquil_scopes::ResolveFailure::NotFound => "not_found", - tranquil_scopes::ResolveFailure::NetworkError => "unreachable", - tranquil_scopes::ResolveFailure::Invalid => "invalid", - } - .to_string(), + reason: f.reason.clone(), }) .collect(); @@ -410,7 +405,7 @@ pub async fn consent_post( }; let authority = match delegation_grant.as_ref() { - Some(grant) => scope_resolution::Authority::Delegated(grant.granted_scopes.as_str()), + Some(grant) => scope_resolution::Authority::Delegated(&grant.granted_scopes), None => scope_resolution::Authority::FullSelf, }; let effective = scope_resolution::resolve_effective_scopes( diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/scope_resolution.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/scope_resolution.rs index e09089e..bb20ca7 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/scope_resolution.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/scope_resolution.rs @@ -1,3 +1,4 @@ +use tranquil_db_traits::DbScope; use tranquil_pds::cache::Cache; use tranquil_pds::delegation::intersect_scopes; use tranquil_pds::oauth::permission_set_resolver::expand_scopes; @@ -5,7 +6,7 @@ use tranquil_scopes::ExpansionOutcome; pub enum Authority<'a> { FullSelf, - Delegated(&'a str), + Delegated(&'a DbScope), } pub struct EffectiveScopes { @@ -22,7 +23,7 @@ pub async fn resolve_effective_scopes( let expanded = outcome.to_scope_string(); let resolved = match authority { Authority::FullSelf => expanded, - Authority::Delegated(granted) => intersect_scopes(&expanded, granted), + Authority::Delegated(granted) => intersect_scopes(&expanded, granted.as_str()), }; EffectiveScopes { resolved, outcome } } @@ -50,7 +51,7 @@ mod tests { fn cache_with(nsid: &str, scopes: &str) -> MapCache { let c = MapCache::default(); - let key = tranquil_pds::cache_keys::permission_set_key(nsid, None); + let key = tranquil_pds::cache_keys::permission_set_key(&tranquil_types::Nsid::new(nsid).unwrap(), None); let json = serde_json::json!({ "scope": scopes, "title": null, "detail": null }).to_string(); c.0.lock().unwrap().insert(key, json); c @@ -69,9 +70,10 @@ mod tests { #[tokio::test] async fn delegated_intersects_expanded_against_grant() { let c = cache_with("io.atcr.authFullApp", "repo:io.atcr.manifest?action=create identity:*"); + let granted = DbScope::new("atproto repo:* blob:*/* account:*?action=manage").unwrap(); let eff = resolve_effective_scopes( &c, "atproto include:io.atcr.authFullApp", - Authority::Delegated("atproto repo:* blob:*/* account:*?action=manage"), + Authority::Delegated(&granted), ).await; assert!(eff.resolved.contains("atproto")); assert!(eff.resolved.contains("repo:io.atcr.manifest?action=create")); diff --git a/crates/tranquil-oauth-server/src/endpoints/token/grants.rs b/crates/tranquil-oauth-server/src/endpoints/token/grants.rs index 8359359..8bc7c5b 100644 --- a/crates/tranquil-oauth-server/src/endpoints/token/grants.rs +++ b/crates/tranquil-oauth-server/src/endpoints/token/grants.rs @@ -133,7 +133,7 @@ pub async fn handle_authorization_code_grant( let controller_did = authorized.controller_did.clone(); let requested_scope = authorized.parameters.scope.clone(); - let granted_scopes: Option = if let Some(ref controller) = controller_did { + let granted_scopes: Option = if let Some(ref controller) = controller_did { let grant = state .repos .delegation @@ -144,11 +144,11 @@ pub async fn handle_authorization_code_grant( .ok_or_else(|| { OAuthError::InvalidGrant("Delegation grant not found or revoked".to_string()) })?; - Some(grant.granted_scopes.as_str().to_string()) + Some(grant.granted_scopes.clone()) } else { None }; - let authority = match granted_scopes.as_deref() { + let authority = match granted_scopes.as_ref() { Some(g) => crate::endpoints::authorize::scope_resolution::Authority::Delegated(g), None => crate::endpoints::authorize::scope_resolution::Authority::FullSelf, }; @@ -254,7 +254,7 @@ async fn recompute_resolved_scope( token_data: &TokenData, ) -> Result { let requested = token_data.scope.as_deref().unwrap_or("atproto"); - let granted_scopes: Option = if let Some(ref controller) = token_data.controller_did { + let granted_scopes: Option = if let Some(ref controller) = token_data.controller_did { let grant = state .repos .delegation @@ -265,11 +265,11 @@ async fn recompute_resolved_scope( .ok_or_else(|| { OAuthError::InvalidGrant("Delegation grant not found or revoked".to_string()) })?; - Some(grant.granted_scopes.as_str().to_string()) + Some(grant.granted_scopes.clone()) } else { None }; - let authority = match granted_scopes.as_deref() { + let authority = match granted_scopes.as_ref() { Some(g) => crate::endpoints::authorize::scope_resolution::Authority::Delegated(g), None => crate::endpoints::authorize::scope_resolution::Authority::FullSelf, }; diff --git a/crates/tranquil-pds/src/cache_keys.rs b/crates/tranquil-pds/src/cache_keys.rs index 6625577..3a148ce 100644 --- a/crates/tranquil-pds/src/cache_keys.rs +++ b/crates/tranquil-pds/src/cache_keys.rs @@ -40,7 +40,7 @@ pub fn auto_verify_sent_key(did: &Did) -> String { format!("auto_verify_sent:{}", did) } -pub fn permission_set_key(nsid: &str, aud: Option<&str>) -> String { +pub fn permission_set_key(nsid: &tranquil_types::Nsid, aud: Option<&str>) -> String { match aud { Some(a) => format!("permset:{}:{}", nsid, a), None => format!("permset:{}", nsid), diff --git a/crates/tranquil-pds/src/oauth/permission_set_resolver.rs b/crates/tranquil-pds/src/oauth/permission_set_resolver.rs index 6c5062f..2044aee 100644 --- a/crates/tranquil-pds/src/oauth/permission_set_resolver.rs +++ b/crates/tranquil-pds/src/oauth/permission_set_resolver.rs @@ -43,8 +43,8 @@ async fn resolve_one( nsid: &str, aud: Option<&str>, ) -> Result { - let parsed = Nsid::new(nsid).map_err(|_| ResolveFailure::Invalid)?; - let key = permission_set_key(nsid, aud); + let parsed = Nsid::new(nsid).map_err(|_| ResolveFailure::Malformed)?; + let key = permission_set_key(&parsed, aud); if let Some(json) = cache.get(&key).await && let Ok(v) = serde_json::from_str::(&json) @@ -89,10 +89,12 @@ fn group_from( fn map_err(e: &ScopeExpansionError) -> ResolveFailure { use ScopeExpansionError as E; match e { - E::InvalidNsid(_) | E::UnexpectedType(_) | E::EmptyPermissions | E::MissingDefinition(_) => { - ResolveFailure::Invalid - } - E::DnsResolution(_) | E::HttpFailed(_) | E::DidResolution(_) => ResolveFailure::NetworkError, + E::InvalidNsid(_) => ResolveFailure::Malformed, + E::RecordNotFound => ResolveFailure::NotFound, + E::UnexpectedType(_) => ResolveFailure::NotAPermissionSet, + E::MissingDefinition(_) => ResolveFailure::MalformedLexicon, + E::EmptyPermissions => ResolveFailure::EmptyPermissions, + E::DnsResolution(_) | E::HttpFailed(_) | E::DidResolution(_) => ResolveFailure::Unreachable, } } @@ -132,7 +134,7 @@ mod tests { } fn seed(cache: &MapCache, nsid: &str, scope: &str) { - let key = crate::cache_keys::permission_set_key(nsid, None); + let key = crate::cache_keys::permission_set_key(&tranquil_types::Nsid::new(nsid).unwrap(), None); let val = serde_json::to_string(&CachedPermissionSet { scope: scope.to_string(), title: Some("Basic".into()), diff --git a/crates/tranquil-pds/tests/oauth_permission_sets.rs b/crates/tranquil-pds/tests/oauth_permission_sets.rs index cc2b7f4..ce8aeaf 100644 --- a/crates/tranquil-pds/tests/oauth_permission_sets.rs +++ b/crates/tranquil-pds/tests/oauth_permission_sets.rs @@ -53,7 +53,7 @@ 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(nsid, None); + let key = tranquil_pds::cache_keys::permission_set_key(&tranquil_types::Nsid::new(nsid).unwrap(), None); let val = json!({ "scope": granular_scope, "title": "Basic", @@ -536,7 +536,7 @@ async fn test_consent_post_errors_when_set_unresolvable() { ); let state = common::get_test_app_state().await; - let key = tranquil_pds::cache_keys::permission_set_key(UNRESOLVABLE_NSID, None); + let key = tranquil_pds::cache_keys::permission_set_key(&tranquil_types::Nsid::new(UNRESOLVABLE_NSID).unwrap(), None); state.cache.delete(&key).await.unwrap(); let approved_scopes: Vec<&str> = scope.split_whitespace().collect(); diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index afd6282..7225016 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -1,7 +1,7 @@ use hickory_resolver::TokioAsyncResolver; use hickory_resolver::config::{ResolverConfig, ResolverOpts}; use reqwest::Client; -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use std::collections::HashMap; use tracing::debug; use tranquil_types::{Did, Nsid}; @@ -20,15 +20,27 @@ pub enum ScopeExpansionError { HttpFailed(String), #[error("DID resolution failed: {0}")] DidResolution(String), + #[error("Lexicon record not found")] + RecordNotFound, #[error("No valid permissions found in permission-set")] EmptyPermissions, } -#[derive(Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] pub enum ResolveFailure { + // Couldn't connect to PDS + Unreachable, + // Connected to PDS, but the lexicon record doesn't exist NotFound, - NetworkError, - Invalid, + // The NSID is malformed + Malformed, + // A lexicon exists, but it isn't a `permission-set` (e.g. a query or record type). + NotAPermissionSet, + // Lexicon doc was malformed (no main, no permissions) + MalformedLexicon, + // The doc is valid, but grants nothing usable (i.e. empty permissions, or permissions are from a different namespace) + EmptyPermissions, } #[derive(Debug, Clone)] @@ -87,6 +99,11 @@ struct PlcService { service_endpoint: String, } +#[derive(Debug, Deserialize)] +struct XrpcError { + error: String, +} + #[derive(Debug, Deserialize)] struct GetRecordResponse { value: LexiconDoc, @@ -200,16 +217,25 @@ async fn fetch_lexicon_via_atproto(nsid: &Nsid) -> Result(&body) + .map(|e| e.error.eq_ignore_ascii_case("RecordNotFound")) + .unwrap_or(false); + return Err(if not_found { + ScopeExpansionError::RecordNotFound + } else { + ScopeExpansionError::HttpFailed(format!("HTTP {}", status)) + }); } - let record: GetRecordResponse = response - .json() - .await + let record: GetRecordResponse = serde_json::from_str(&body) .map_err(|e| ScopeExpansionError::HttpFailed(e.to_string()))?; Ok(record.value) diff --git a/frontend/src/locales/en.json b/frontend/src/locales/en.json index 609ff60..7941a88 100644 --- a/frontend/src/locales/en.json +++ b/frontend/src/locales/en.json @@ -603,7 +603,10 @@ "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.", - "invalid": "This bundle is invalid and cannot be granted." + "malformed": "This bundle's name is not a valid identifier, so it cannot be granted.", + "not_a_permission_set": "This identifier does not refer to a permission bundle, so it cannot be granted.", + "malformed_lexicon": "This bundle is published incorrectly and cannot be granted.", + "empty_permissions": "This bundle does not currently grant any permissions." }, "permTable": { "data": "Data",