diff --git a/crates/tranquil-pds/src/oauth/permission_set_resolver.rs b/crates/tranquil-pds/src/oauth/permission_set_resolver.rs index 2f51d8a..20366f2 100644 --- a/crates/tranquil-pds/src/oauth/permission_set_resolver.rs +++ b/crates/tranquil-pds/src/oauth/permission_set_resolver.rs @@ -44,13 +44,13 @@ async fn resolve_one( aud: Option<&str>, ) -> Result { let key = permission_set_key(nsid, aud); - // Cache hit + if let Some(json) = cache.get(&key).await && let Ok(v) = serde_json::from_str::(&json) { return Ok(group_from(nsid, aud, v.scope, v.title, v.detail)); } - // Miss → network fetch → cache + let parsed = Nsid::new(nsid).map_err(|_| ResolveFailure::Invalid)?; match fetch_and_expand(&parsed, aud).await { Ok(fetched) => { @@ -173,7 +173,7 @@ mod tests { #[tokio::test] async fn cache_miss_unresolvable_is_a_failure() { - let cache = MapCache::default(); // empty → miss → network fetch of a fake NSID fails fast + let cache = MapCache::default(); let out = expand_scopes(&cache, "include:nonexistent.fake.permissionSet").await; assert_eq!(out.sets.len(), 0); assert_eq!(out.failures.len(), 1); diff --git a/crates/tranquil-pds/src/oauth/scopes/mod.rs b/crates/tranquil-pds/src/oauth/scopes/mod.rs index 944eda7..f19b655 100644 --- a/crates/tranquil-pds/src/oauth/scopes/mod.rs +++ b/crates/tranquil-pds/src/oauth/scopes/mod.rs @@ -1,7 +1,6 @@ pub use tranquil_scopes::{ AccountAction, AccountAttr, AccountScope, BlobScope, IdentityAttr, IdentityScope, IncludeScope, ParsedScope, RepoAction, RepoScope, RpcScope, SCOPE_DEFINITIONS, ScopeCategory, - ScopeDefinition, ScopeError, ScopeExpansionError, ScopePermissions, expand_include_scopes, - format_scope_for_display, get_required_scopes, get_scope_definition, is_valid_scope, - parse_scope, parse_scope_string, + ScopeDefinition, ScopeError, ScopeExpansionError, ScopePermissions, format_scope_for_display, + get_required_scopes, get_scope_definition, is_valid_scope, parse_scope, parse_scope_string, }; diff --git a/crates/tranquil-scopes/src/coverage.rs b/crates/tranquil-scopes/src/coverage.rs index 9a4c1af..3dcb013 100644 --- a/crates/tranquil-scopes/src/coverage.rs +++ b/crates/tranquil-scopes/src/coverage.rs @@ -3,8 +3,6 @@ use crate::parser::{ RepoScope, RpcScope, }; -/// Returns true if the `granted` scope authorizes everything the `requested` scope asks for. -/// Typed replacement for the old string-prefix `scope_covers`. pub fn covers(granted: &ParsedScope, requested: &ParsedScope) -> bool { use ParsedScope::*; match (granted, requested) { @@ -25,16 +23,15 @@ pub fn covers(granted: &ParsedScope, requested: &ParsedScope) -> bool { fn repo_covers(g: &RepoScope, r: &RepoScope) -> bool { let collection_ok = match &g.collection { - None => true, // repo:* — any collection + None => true, Some(gc) => match &r.collection { - None => false, // a specific grant cannot cover a wildcard request + None => false, Some(rc) => match gc.strip_suffix(".*") { Some(prefix) => rc.starts_with(prefix) && rc.as_bytes().get(prefix.len()) == Some(&b'.'), None => gc == rc, }, }, }; - // parse_scope expands a missing ?action to all three actions, so subset is exact. collection_ok && r.actions.is_subset(&g.actions) } @@ -42,7 +39,6 @@ fn blob_covers(g: &BlobScope, r: &BlobScope) -> bool { if g.accept.is_empty() || g.accept.contains("*/*") { return true; } - // every accept pattern the request wants must be matched by the grant !r.accept.is_empty() && r.accept.iter().all(|pat| g.matches_mime(pat)) } @@ -63,7 +59,7 @@ fn rpc_covers(g: &RpcScope, r: &RpcScope) -> bool { fn account_covers(g: &AccountScope, r: &AccountScope) -> bool { let attr_ok = g.attr == AccountAttr::Wildcard || g.attr == r.attr; let action_ok = match (g.action, r.action) { - (AccountAction::Manage, _) => true, // manage ⊇ read + (AccountAction::Manage, _) => true, (AccountAction::Read, AccountAction::Read) => true, (AccountAction::Read, AccountAction::Manage) => false, }; @@ -96,7 +92,6 @@ mod tests { #[test] fn repo_action_subset_required() { - // granted no ?action == all three actions assert!(c("repo:*", "repo:*?action=create")); assert!(!c("repo:*?action=create", "repo:*?action=delete")); assert!(c("repo:*?action=create&action=delete", "repo:*?action=create")); @@ -105,8 +100,6 @@ mod tests { #[test] fn repo_partial_action_grant_does_not_cover_actionless_request() { - // SECURITY: actionless requested repo == all three actions; a create+delete - // grant must NOT cover it (previously the string engine wrongly did). assert!(!c("repo:*?action=create&action=delete", "repo:app.bsky.feed.post")); } @@ -166,7 +159,7 @@ mod tests { fn repo_prefix_wildcard_respects_dot_boundary() { assert!(!c("repo:app.bsky.*", "repo:app.bskyEXTRA")); assert!(c("repo:app.bsky.*", "repo:app.bsky.feed.post")); - assert!(!c("repo:app.bsky.*", "repo:app.bsky")); // no trailing segment + assert!(!c("repo:app.bsky.*", "repo:app.bsky")); } #[test] diff --git a/crates/tranquil-scopes/src/lib.rs b/crates/tranquil-scopes/src/lib.rs index ec35354..50439a3 100644 --- a/crates/tranquil-scopes/src/lib.rs +++ b/crates/tranquil-scopes/src/lib.rs @@ -17,6 +17,6 @@ pub use parser::{ }; pub use permission_set::{ ExpansionOutcome, FailedSet, FetchedSet, ResolveFailure, ResolvedSetGroup, - ScopeExpansionError, expand_include_scopes, fetch_and_expand, parse_include_scope, + ScopeExpansionError, fetch_and_expand, parse_include_scope, }; pub use permissions::ScopePermissions; diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index a19b342..46b7639 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -3,8 +3,6 @@ use hickory_resolver::config::{ResolverConfig, ResolverOpts}; use reqwest::Client; use serde::Deserialize; use std::collections::HashMap; -use std::sync::LazyLock; -use tokio::sync::RwLock; use tracing::debug; use tranquil_types::{Did, Nsid}; @@ -58,29 +56,25 @@ pub struct ExpansionOutcome { impl ExpansionOutcome { pub fn flat_scopes(&self) -> Vec { - let mut out = self.passthrough.clone(); - for s in &self.sets { - out.extend(s.expanded.iter().cloned()); + let mut out = Vec::new(); + let mut seen = std::collections::HashSet::new(); + for s in self.passthrough.iter().chain( + self.sets + .iter() + .flat_map(|group| group.expanded.iter()), + ) { + if seen.insert(s.as_str()) { + out.push(s.clone()); + } } out } - /// Space-joined `flat_scopes`. + pub fn to_scope_string(&self) -> String { self.flat_scopes().join(" ") } } -static LEXICON_CACHE: LazyLock>> = - LazyLock::new(|| RwLock::new(HashMap::new())); - -#[derive(Clone)] -struct CachedLexicon { - expanded_scope: String, - cached_at: std::time::Instant, -} - -const CACHE_TTL_SECS: u64 = 3600; - #[derive(Debug, Deserialize)] struct PlcDocument { service: Vec, @@ -123,29 +117,6 @@ struct PermissionEntry { aud: Option, } -pub async fn expand_include_scopes(scope_string: &str) -> Result { - let futures: Vec<_> = scope_string - .split_whitespace() - .map(|scope| async move { - match scope.strip_prefix("include:") { - Some(rest) => { - let (nsid_base, aud) = parse_include_scope(rest); - let nsid = Nsid::new(nsid_base) - .map_err(|_| ScopeExpansionError::InvalidNsid(nsid_base.to_string()))?; - expand_permission_set(&nsid, aud).await - } - None => Ok(scope.to_string()), - } - }) - .collect(); - - futures::future::join_all(futures) - .await - .into_iter() - .collect::, ScopeExpansionError>>() - .map(|v| v.join(" ")) -} - pub fn parse_include_scope(rest: &str) -> (&str, Option<&str>) { rest.split_once('?') .map(|(nsid, params)| { @@ -155,43 +126,6 @@ pub fn parse_include_scope(rest: &str) -> (&str, Option<&str>) { .unwrap_or((rest, None)) } -async fn expand_permission_set( - nsid: &Nsid, - aud: Option<&str>, -) -> Result { - let cache_key = match aud { - Some(a) => format!("{}?aud={}", nsid, a), - None => nsid.to_string(), - }; - - { - let cache = LEXICON_CACHE.read().await; - if let Some(cached) = cache.get(&cache_key) - && cached.cached_at.elapsed().as_secs() < CACHE_TTL_SECS - { - debug!(nsid = %nsid, "Using cached permission set expansion"); - return Ok(cached.expanded_scope.clone()); - } - } - - let fetched = fetch_and_expand(nsid, aud).await?; - let expanded = fetched.expanded; - - { - let mut cache = LEXICON_CACHE.write().await; - cache.insert( - cache_key, - CachedLexicon { - expanded_scope: expanded.clone(), - cached_at: std::time::Instant::now(), - }, - ); - } - - debug!(nsid = %nsid, expanded = %expanded, "Successfully expanded permission set"); - Ok(expanded) -} - pub struct FetchedSet { pub expanded: String, pub title: Option, @@ -609,117 +543,6 @@ mod tests { assert!(expanded.is_empty()); } - #[tokio::test] - async fn test_expand_include_scopes_passthrough_non_include() { - let result = expand_include_scopes("atproto transition:generic") - .await - .unwrap(); - assert_eq!(result, "atproto transition:generic"); - } - - #[tokio::test] - async fn test_expand_include_scopes_mixed_with_regular() { - let result = expand_include_scopes("atproto repo:app.bsky.feed.post?action=create") - .await - .unwrap(); - assert!(result.contains("atproto")); - assert!(result.contains("repo:app.bsky.feed.post?action=create")); - } - - #[tokio::test] - async fn test_expand_include_scopes_fails_on_unresolvable_nsid() { - let result = expand_include_scopes("atproto include:nonexistent.fake.permissionSet").await; - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_expand_include_scopes_fails_even_with_valid_scopes_present() { - let result = expand_include_scopes( - "atproto include:nonexistent.fake.permissionSet repo:app.bsky.feed.post?action=create", - ) - .await; - assert!(result.is_err()); - } - - #[tokio::test] - async fn test_cache_population_and_retrieval() { - let cache_key = "test.cached.scope"; - let cached_value = "repo:test.cached.collection?action=create"; - - { - let mut cache = LEXICON_CACHE.write().await; - cache.insert( - cache_key.to_string(), - CachedLexicon { - expanded_scope: cached_value.to_string(), - cached_at: std::time::Instant::now(), - }, - ); - } - - let result = expand_permission_set(&nsid(cache_key), None).await; - assert!(result.is_ok()); - assert_eq!(result.unwrap(), cached_value); - - { - let mut cache = LEXICON_CACHE.write().await; - cache.remove(cache_key); - } - } - - #[tokio::test] - async fn test_cache_with_aud_parameter() { - let nsid = "test.aud.scope"; - let aud = "did:web:example.com"; - let cache_key = format!("{}?aud={}", nsid, aud); - let cached_value = "rpc:test.aud.method?aud=did:web:example.com"; - - { - let mut cache = LEXICON_CACHE.write().await; - cache.insert( - cache_key.clone(), - CachedLexicon { - expanded_scope: cached_value.to_string(), - cached_at: std::time::Instant::now(), - }, - ); - } - - let result = expand_permission_set(&nsid.parse().unwrap(), Some(aud)).await; - assert!(result.is_ok()); - assert_eq!(result.unwrap(), cached_value); - - { - let mut cache = LEXICON_CACHE.write().await; - cache.remove(&cache_key); - } - } - - #[tokio::test] - async fn test_expired_cache_triggers_refresh() { - let cache_key = "test.expired.scope"; - - { - let mut cache = LEXICON_CACHE.write().await; - cache.insert( - cache_key.to_string(), - CachedLexicon { - expanded_scope: "old_value".to_string(), - cached_at: std::time::Instant::now() - - std::time::Duration::from_secs(CACHE_TTL_SECS + 1), - }, - ); - } - - let result = expand_permission_set(&nsid(cache_key), None).await; - assert!(result.is_err()); - - { - let mut cache = LEXICON_CACHE.write().await; - cache.remove(cache_key); - } - } - fn dns_authority(nsid: &str) -> String { let parts: Vec<&str> = nsid.split('.').collect(); parts[..parts.len() - 1] @@ -775,6 +598,24 @@ mod tests { ); } + #[test] + fn flat_scopes_dedupes() { + let out = ExpansionOutcome { + passthrough: vec!["repo:x".into()], + sets: vec![ResolvedSetGroup { + nsid: "io.atcr.authFullApp".into(), + aud: None, + title: None, + detail: None, + expanded: vec!["repo:x".into(), "rpc:io.atcr.getManifest".into()], + }], + failures: vec![], + }; + let flat = out.flat_scopes(); + assert_eq!(flat, vec!["repo:x", "rpc:io.atcr.getManifest"]); + assert_eq!(out.to_scope_string(), "repo:x rpc:io.atcr.getManifest"); + } + #[test] fn test_lexicon_def_captures_title_and_detail() { let json = serde_json::json!({