From 20e8e38310042ec09b8003f09fe0095b64b2f36b Mon Sep 17 00:00:00 2001 From: Orual Date: Thu, 2 Apr 2026 21:47:09 -0400 Subject: [PATCH] [jacquard] migrate Vec> to Scopes container MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Replace all Vec> fields with Scopes across jacquard-oauth - Migrate OAuthState/OAuthSession persistence format (Vec → String) - Migrate main crate consumers and integration tests - Retire old utility methods (parse_multiple, serialize_multiple, remove_scope) - Use expect instead of unwrap_or_default for scope parsing in token refresh --- crates/jacquard-oauth/src/atproto.rs | 54 +- crates/jacquard-oauth/src/client.rs | 9 +- crates/jacquard-oauth/src/loopback.rs | 10 +- crates/jacquard-oauth/src/request.rs | 11 +- crates/jacquard-oauth/src/scopes.rs | 634 +------------------- crates/jacquard-oauth/src/session.rs | 33 +- crates/jacquard-oauth/src/types.rs | 18 +- crates/jacquard/src/client/token.rs | 36 +- crates/jacquard/tests/oauth_auto_refresh.rs | 12 +- crates/jacquard/tests/oauth_flow.rs | 6 +- 10 files changed, 88 insertions(+), 735 deletions(-) diff --git a/crates/jacquard-oauth/src/atproto.rs b/crates/jacquard-oauth/src/atproto.rs index d1ec67ce..e226c896 100644 --- a/crates/jacquard-oauth/src/atproto.rs +++ b/crates/jacquard-oauth/src/atproto.rs @@ -1,7 +1,7 @@ use std::str::FromStr; use crate::types::OAuthClientMetadata; -use crate::{keyset::Keyset, scopes::Scope}; +use crate::{keyset::Keyset, scopes::{Scope, Scopes}}; use jacquard_common::deps::fluent_uri::Uri; use jacquard_common::{BosStr, IntoStatic}; use serde::{Deserialize, Serialize}; @@ -145,7 +145,7 @@ where /// The grant types this client will use. pub grant_types: Vec, /// The OAuth scopes this client requests; must include `atproto`. - pub scopes: Vec>, + pub scopes: Scopes, /// URI pointing to the client's JWK Set; mutually exclusive with inline `jwks`. pub jwks_uri: Option>, /// Human-readable display name for the client. @@ -160,9 +160,9 @@ where impl IntoStatic for AtprotoClientMetadata where - S: BosStr + IntoStatic + Ord + FromStr, + S: BosStr + IntoStatic + Ord + FromStr + AsRef, ::Err: core::fmt::Debug, - S::Output: BosStr + FromStr + Ord, + S::Output: BosStr + FromStr + Ord + AsRef, ::Err: core::fmt::Debug, { type Output = AtprotoClientMetadata; @@ -212,14 +212,14 @@ where /// This is a convenience constructor for local development and CLI tools. The resulting /// metadata uses `http://localhost` as the `client_id` with both IPv4 and IPv6 loopback /// redirect URIs. - pub fn default_localhost() -> Self { - Self::new_localhost( - None, - Some(vec![ - Scope::Atproto, - Scope::Transition(crate::scopes::TransitionScope::Generic), - ]), - ) + pub fn default_localhost() -> Self + where + S: From + AsRef, + { + let scopes = Scopes::new(SmolStr::new_static("atproto transition:generic")) + .expect("valid scopes") + .convert(); + Self::new_localhost(None, Some(scopes)) } /// Create loopback client metadata with optional custom redirect URIs and scopes. @@ -230,8 +230,11 @@ where /// are used. pub fn new_localhost( redirect_uris: Option>>, - scopes: Option>>, - ) -> AtprotoClientMetadata { + scopes: Option>, + ) -> AtprotoClientMetadata + where + S: From + AsRef, + { // determine client_id #[derive(serde::Serialize)] struct Parameters { @@ -247,9 +250,7 @@ where }); let query = serde_html_form::to_string(Parameters { redirect_uri: redir_str, - scope: scopes - .as_ref() - .map(|s| SmolStr::from(Scope::serialize_multiple(s.as_slice()).as_str())), + scope: scopes.as_ref().map(|s| s.to_normalized_string()), }) .ok(); let mut client_id = String::from("http://localhost/"); @@ -258,6 +259,9 @@ where { client_id.push_str(&format!("?{query}")); } + let default_scopes: Scopes = Scopes::new(SmolStr::new_static("atproto")) + .expect("valid scopes") + .convert(); AtprotoClientMetadata { client_id: Uri::parse(client_id).unwrap(), client_uri: None, @@ -266,7 +270,7 @@ where Uri::parse("http://[::1]".to_string()).unwrap(), ]), grant_types: vec![GrantType::AuthorizationCode, GrantType::RefreshToken], - scopes: scopes.unwrap_or(vec![Scope::Atproto]), + scopes: scopes.unwrap_or(default_scopes), jwks_uri: None, client_name: None, logo_uri: None, @@ -303,7 +307,7 @@ where if !metadata.grant_types.contains(&GrantType::AuthorizationCode) { return Err(Error::InvalidGrantTypes); } - if !metadata.scopes.contains(&Scope::Atproto) { + if !metadata.scopes.grants(&Scope::::Atproto) { return Err(Error::InvalidScope); } let (auth_method, jwks_uri, jwks) = if let Some(keyset) = keyset { @@ -342,7 +346,7 @@ where ), response_types: vec![S::from_static("code")], scope: Some( - S::from_str(Scope::serialize_multiple(metadata.scopes.as_slice()).as_str()).unwrap(), + S::from_str(metadata.scopes.to_normalized_string().as_str()).unwrap(), ), dpop_bound_access_tokens: Some(true), jwks_uri, @@ -370,8 +374,6 @@ where #[cfg(test)] mod tests { - use crate::scopes::TransitionScope; - use super::*; use elliptic_curve::SecretKey; use jose_jwk::{Jwk, Key, Parameters}; @@ -424,11 +426,7 @@ gbGGr0pN+oSing7cZ0169JaRHTNh+0LNQXrFobInX6cj95FzEdRyT4T3 Uri::parse("http://127.0.0.1/callback".to_string()).unwrap(), Uri::parse("http://[::1]/callback".to_string()).unwrap(), ]), - Some(vec![ - Scope::Atproto, - Scope::Transition(TransitionScope::Generic), - Scope::parse("account:email").unwrap() - ]) + Some(Scopes::new(SmolStr::from("account:email atproto transition:generic")).unwrap()) ), &None ) @@ -586,7 +584,7 @@ gbGGr0pN+oSing7cZ0169JaRHTNh+0LNQXrFobInX6cj95FzEdRyT4T3 client_uri: Some(Uri::parse("https://example.com".to_string()).unwrap()), redirect_uris: vec![Uri::parse("https://example.com/callback".to_string()).unwrap()], grant_types: vec![GrantType::AuthorizationCode], - scopes: vec![Scope::Atproto], + scopes: Scopes::new(SmolStr::new_static("atproto")).unwrap(), jwks_uri: None, client_name: None, logo_uri: None, diff --git a/crates/jacquard-oauth/src/client.rs b/crates/jacquard-oauth/src/client.rs index 4296ea9b..ba59246b 100644 --- a/crates/jacquard-oauth/src/client.rs +++ b/crates/jacquard-oauth/src/client.rs @@ -5,7 +5,7 @@ use crate::{ error::{CallbackError, Result}, request::{OAuthMetadata, exchange_code, par}, resolver::OAuthResolver, - scopes::Scope, + scopes::Scopes, session::{ClientData, ClientSessionData, DpopClientData, SessionRegistry}, types::{AuthorizeOptions, CallbackParams}, }; @@ -291,11 +291,10 @@ where { Ok(token_set) => { let scopes = if let Some(scope) = &token_set.scope { - Scope::::parse_multiple_reduced(scope.as_str()) - .expect("Failed to parse scopes") - .into_static() + Scopes::new(SmolStr::from(scope.as_str())) + .expect("Failed to parse scopes from token response") } else { - vec![] + Scopes::empty() }; let client_data = ClientSessionData { account_did: token_set.sub.clone(), diff --git a/crates/jacquard-oauth/src/loopback.rs b/crates/jacquard-oauth/src/loopback.rs index 01e85ef9..73ed4d45 100644 --- a/crates/jacquard-oauth/src/loopback.rs +++ b/crates/jacquard-oauth/src/loopback.rs @@ -260,15 +260,7 @@ where let redirect = Uri::parse(redirect_uri).unwrap(); let scopes = if opts.scopes.is_empty() { - Some( - self.registry - .client_data - .config - .scopes - .iter() - .cloned() - .collect(), - ) + Some(self.registry.client_data.config.scopes.clone()) } else { Some(opts.scopes.clone()) }; diff --git a/crates/jacquard-oauth/src/request.rs b/crates/jacquard-oauth/src/request.rs index da858242..b4088b19 100644 --- a/crates/jacquard-oauth/src/request.rs +++ b/crates/jacquard-oauth/src/request.rs @@ -3,7 +3,7 @@ use std::str::FromStr; use chrono::{TimeDelta, Utc}; use http::{Method, Request, StatusCode}; use jacquard_common::{ - CowStr, IntoStatic, + CowStr, bos::{BosStr, DefaultStr}, http_client::HttpClient, session::SessionStoreError, @@ -26,7 +26,7 @@ use crate::{ jose::jwt::{RegisteredClaims, RegisteredClaimsAud}, keyset::Keyset, resolver::OAuthResolver, - scopes::Scope, + scopes::Scopes, session::{ AuthRequestData, ClientData, ClientSessionData, DpopClientData, DpopDataSource, DpopReqData, }, @@ -577,11 +577,10 @@ pub async fn par< .await?; let scopes = if let Some(scope) = &metadata.client_metadata.scope { - Scope::::parse_multiple_reduced(scope.as_ref()) + Scopes::new(SmolStr::from(scope.as_ref())) .expect("Failed to parse scopes") - .into_static() } else { - vec![] + Scopes::empty() }; let auth_req_data: AuthRequestData = AuthRequestData { state: state.into(), @@ -1081,7 +1080,7 @@ mod tests { authserver_url: SmolStr::new_static("https://issuer"), authserver_token_endpoint: SmolStr::new_static("https://issuer/token"), authserver_revocation_endpoint: None, - scopes: vec![], + scopes: Scopes::empty(), dpop_data: DpopClientData { dpop_key: crate::utils::generate_key(&[SmolStr::new_static("ES256")]).unwrap(), dpop_authserver_nonce: SmolStr::default(), diff --git a/crates/jacquard-oauth/src/scopes.rs b/crates/jacquard-oauth/src/scopes.rs index 997f2075..341bede0 100644 --- a/crates/jacquard-oauth/src/scopes.rs +++ b/crates/jacquard-oauth/src/scopes.rs @@ -33,7 +33,7 @@ use jacquard_common::{Bos, BorrowOrShare, FromStaticStr, IntoStatic}; use serde::de::{Error as DeError, Visitor}; use serde::{Deserialize, Serialize}; use smallvec::SmallVec; -use smol_str::{SmolStr, SmolStrBuilder, ToSmolStr, format_smolstr}; +use smol_str::{SmolStr, ToSmolStr, format_smolstr}; /// Represents an AT Protocol OAuth scope #[derive(Debug, Clone, PartialEq, Eq, Hash)] @@ -514,7 +514,7 @@ where // ============================================================================ /// Byte-range indices for a single scope within a `Scopes` buffer. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct ScopeIndices { pub(crate) start: u16, pub(crate) end: u16, @@ -522,7 +522,7 @@ pub(crate) struct ScopeIndices { } /// Pre-parsed structure of a scope, storing only byte-range indices into the buffer. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum ScopeInnerIndices { Account { resource: AccountResource, @@ -604,7 +604,7 @@ impl RepoActionFlags { /// Both variants store byte ranges into the buffer. The discriminant /// tells `grants()` whether to decode before comparing, and tells /// `to_string_normalized()` whether the raw form needs encoding. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum IncludeAudience { /// Audience in buffer is already decoded (no percent-encoding). /// `grants()` can compare directly. Serialisation must encode `#` → `%23`. @@ -659,7 +659,7 @@ impl<'i, 'o> std::iter::FusedIterator for ScopesIter<'i, 'o> {} /// Owns or borrows a single scope string and stores pre-computed byte-range /// indices. Typed `Scope<&str>` views are reconstructed on demand from the /// shared buffer. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct Scopes + AsRef = DefaultStr> { buffer: S, indices: Vec, @@ -1590,161 +1590,7 @@ impl Scope { } } - /// Parse multiple space-separated scopes from a string - /// - /// # Examples - /// ``` - /// # use jacquard_oauth::scopes::Scope; - /// # use smol_str::SmolStr; - /// let scopes = Scope::::parse_multiple("atproto repo:*").unwrap(); - /// assert_eq!(scopes.len(), 2); - /// ``` - pub fn parse_multiple<'a>(s: &'a str) -> Result, ParseError> - where - S: FromStr, - ::Err: core::fmt::Debug, - { - if s.trim().is_empty() { - return Ok(Vec::new()); - } - - let mut scopes = Vec::new(); - for scope_str in s.split_whitespace() { - scopes.push(Self::parse(scope_str)?); - } - - Ok(scopes) - } - - /// Parse multiple space-separated scopes and return the minimal set needed - /// - /// This method removes duplicate scopes and scopes that are already granted - /// by other scopes in the list, returning only the minimal set of scopes needed. - /// - /// # Examples - /// ``` - /// # use jacquard_oauth::scopes::Scope; - /// # use smol_str::SmolStr; - /// // repo:* grants repo:foo.bar, so only repo:* is kept - /// let scopes = Scope::::parse_multiple_reduced("atproto repo:app.bsky.feed.post repo:*").unwrap(); - /// assert_eq!(scopes.len(), 2); // atproto and repo:* - /// ``` - pub fn parse_multiple_reduced<'a>(s: &'a str) -> Result, ParseError> - where - S: FromStr, - ::Err: core::fmt::Debug, - { - let all_scopes = Self::parse_multiple(s)?; - - if all_scopes.is_empty() { - return Ok(Vec::new()); - } - - let mut result: Vec = Vec::new(); - - for scope in all_scopes { - // Check if this scope is already granted by something in the result - let mut is_granted = false; - for existing in &result { - if existing.grants(&scope) && existing != &scope { - is_granted = true; - break; - } - } - - if is_granted { - continue; // Skip this scope, it's already covered - } - - // Check if this scope grants any existing scopes in the result - let mut indices_to_remove = Vec::new(); - for (i, existing) in result.iter().enumerate() { - if scope.grants(existing) && &scope != existing { - indices_to_remove.push(i); - } - } - - // Remove scopes that are granted by the new scope (in reverse order to maintain indices) - for i in indices_to_remove.into_iter().rev() { - result.remove(i); - } - - // Add the new scope if it's not a duplicate - if !result.contains(&scope) { - result.push(scope); - } - } - - Ok(result) - } - - /// Serialize a list of scopes into a space-separated OAuth scopes string - /// - /// The scopes are sorted alphabetically by their string representation to ensure - /// consistent output regardless of input order. - /// - /// # Examples - /// ``` - /// # use jacquard_oauth::scopes::Scope; - /// # use smol_str::SmolStr; - /// let scopes = vec![ - /// Scope::::parse("repo:*").unwrap(), - /// Scope::::parse("atproto").unwrap(), - /// Scope::::parse("account:email").unwrap(), - /// ]; - /// let result = Scope::serialize_multiple(&scopes); - /// assert_eq!(result, "account:email atproto repo:*"); - /// ``` - pub fn serialize_multiple(scopes: &[Self]) -> SmolStr { - if scopes.is_empty() { - return SmolStr::new_static(""); - } - - let mut serialized: Vec = scopes - .iter() - .map(|scope| scope.to_string_normalized()) - .collect(); - serialized.sort(); - let mut builder = SmolStrBuilder::new(); - for (i, scope) in serialized.iter().enumerate() { - if i > 0 { - builder.push_str(" "); - } - builder.push_str(scope); - } - builder.finish() - } - - /// Remove a scope from a list of scopes - /// - /// Returns a new vector with all instances of the specified scope removed. - /// If the scope doesn't exist in the list, returns a copy of the original list. - /// - /// # Examples - /// ``` - /// # use jacquard_oauth::scopes::Scope; - /// # use smol_str::SmolStr; - /// let scopes = vec![ - /// Scope::::parse("repo:*").unwrap(), - /// Scope::::parse("atproto").unwrap(), - /// Scope::::parse("account:email").unwrap(), - /// ]; - /// let to_remove = Scope::::parse("atproto").unwrap(); - /// let result = Scope::remove_scope(&scopes, &to_remove); - /// assert_eq!(result.len(), 2); - /// assert!(!result.contains(&to_remove)); - /// ``` - pub fn remove_scope(scopes: &[Self], scope_to_remove: &Self) -> Vec - where - S: Clone, - { - scopes - .iter() - .filter(|s| *s != scope_to_remove) - .cloned() - .collect() - } /// Parse a scope from a string pub fn parse<'a>(s: &'a str) -> Result @@ -2969,231 +2815,6 @@ mod tests { assert!(!account.grants(&transition_email)); } - #[test] - fn test_parse_multiple() { - // Test parsing multiple scopes - let scopes = Scope::::parse_multiple("atproto repo:*").unwrap(); - assert_eq!(scopes.len(), 2); - assert_eq!(scopes[0], Scope::Atproto); - assert_eq!( - scopes[1], - Scope::Repo(RepoScope { - collection: RepoCollection::All, - actions: { - let mut actions = BTreeSet::new(); - actions.insert(RepoAction::Create); - actions.insert(RepoAction::Update); - actions.insert(RepoAction::Delete); - actions - } - }) - ); - - // Test with more scopes - let scopes = - Scope::::parse_multiple("account:email identity:handle blob:image/png") - .unwrap(); - assert_eq!(scopes.len(), 3); - assert!(matches!(scopes[0], Scope::Account(_))); - assert!(matches!(scopes[1], Scope::Identity(_))); - assert!(matches!(scopes[2], Scope::Blob(_))); - - // Test with complex scopes - let scopes = Scope::::parse_multiple( - "account:email?action=manage repo:app.bsky.feed.post?action=create transition:email", - ) - .unwrap(); - assert_eq!(scopes.len(), 3); - - // Test empty string - let scopes = Scope::::parse_multiple("").unwrap(); - assert_eq!(scopes.len(), 0); - - // Test whitespace only - let scopes = Scope::::parse_multiple(" ").unwrap(); - assert_eq!(scopes.len(), 0); - - // Test with extra whitespace - let scopes = Scope::::parse_multiple(" atproto repo:* ").unwrap(); - assert_eq!(scopes.len(), 2); - - // Test single scope - let scopes = Scope::::parse_multiple("atproto").unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!(scopes[0], Scope::Atproto); - - // Test error propagation - assert!(Scope::::parse_multiple("atproto invalid:scope").is_err()); - assert!(Scope::::parse_multiple("account:invalid repo:*").is_err()); - } - - #[test] - fn test_parse_multiple_reduced() { - // Test repo scope reduction - wildcard grants specific - let scopes = - Scope::::parse_multiple_reduced("atproto repo:app.bsky.feed.post repo:*") - .unwrap(); - assert_eq!(scopes.len(), 2); - assert!(scopes.contains(&Scope::Atproto)); - assert!(scopes.contains(&Scope::Repo(RepoScope { - collection: RepoCollection::All, - actions: { - let mut actions = BTreeSet::new(); - actions.insert(RepoAction::Create); - actions.insert(RepoAction::Update); - actions.insert(RepoAction::Delete); - actions - } - }))); - - // Test reverse order - should get same result - let scopes = - Scope::::parse_multiple_reduced("atproto repo:* repo:app.bsky.feed.post") - .unwrap(); - assert_eq!(scopes.len(), 2); - assert!(scopes.contains(&Scope::Atproto)); - assert!(scopes.contains(&Scope::Repo(RepoScope { - collection: RepoCollection::All, - actions: { - let mut actions = BTreeSet::new(); - actions.insert(RepoAction::Create); - actions.insert(RepoAction::Update); - actions.insert(RepoAction::Delete); - actions - } - }))); - - // Test account scope reduction - manage grants read - let scopes = - Scope::::parse_multiple_reduced("account:email account:email?action=manage") - .unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!( - scopes[0], - Scope::Account(AccountScope { - resource: AccountResource::Email, - action: AccountAction::Manage, - }) - ); - - // Test identity scope reduction - wildcard grants specific - let scopes = - Scope::::parse_multiple_reduced("identity:handle identity:*").unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!(scopes[0], Scope::Identity(IdentityScope::All)); - - // Test blob scope reduction - wildcard grants specific - let scopes = - Scope::::parse_multiple_reduced("blob:image/png blob:image/* blob:*/*") - .unwrap(); - assert_eq!(scopes.len(), 1); - let mut accept = BTreeSet::new(); - accept.insert(MimePattern::All); - assert_eq!(scopes[0], Scope::Blob(BlobScope { accept })); - - // Test no reduction needed - different scope types - let scopes = Scope::::parse_multiple_reduced( - "account:email identity:handle blob:image/png", - ) - .unwrap(); - assert_eq!(scopes.len(), 3); - - // Test repo action reduction - let scopes = Scope::::parse_multiple_reduced( - "repo:app.bsky.feed.post?action=create repo:app.bsky.feed.post", - ) - .unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!( - scopes[0], - Scope::Repo(RepoScope { - collection: RepoCollection::Nsid(Nsid::new_owned("app.bsky.feed.post").unwrap()), - actions: { - let mut actions = BTreeSet::new(); - actions.insert(RepoAction::Create); - actions.insert(RepoAction::Update); - actions.insert(RepoAction::Delete); - actions - } - }) - ); - - // Test RPC scope reduction - let scopes = Scope::::parse_multiple_reduced( - "rpc:com.example.service?aud=did:example:123 rpc:com.example.service rpc:*", - ) - .unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!( - scopes[0], - Scope::Rpc(RpcScope { - lxm: { - let mut lxm = BTreeSet::new(); - lxm.insert(RpcLexicon::All); - lxm - }, - aud: { - let mut aud = BTreeSet::new(); - aud.insert(RpcAudience::All); - aud - } - }) - ); - - // Test duplicate removal - let scopes = Scope::::parse_multiple_reduced("atproto atproto atproto").unwrap(); - assert_eq!(scopes.len(), 1); - assert_eq!(scopes[0], Scope::Atproto); - - // Test transition scopes - only grant themselves - let scopes = - Scope::::parse_multiple_reduced("transition:generic transition:email") - .unwrap(); - assert_eq!(scopes.len(), 2); - assert!(scopes.contains(&Scope::Transition(TransitionScope::Generic))); - assert!(scopes.contains(&Scope::Transition(TransitionScope::Email))); - - // Test empty input - let scopes = Scope::::parse_multiple_reduced("").unwrap(); - assert_eq!(scopes.len(), 0); - - // Test complex scenario with multiple reductions - let scopes = Scope::::parse_multiple_reduced( - "account:email?action=manage account:email account:repo account:repo?action=read identity:* identity:handle" - ).unwrap(); - assert_eq!(scopes.len(), 3); - // Should have: account:email?action=manage, account:repo, identity:* - assert!(scopes.contains(&Scope::Account(AccountScope { - resource: AccountResource::Email, - action: AccountAction::Manage, - }))); - assert!(scopes.contains(&Scope::Account(AccountScope { - resource: AccountResource::Repo, - action: AccountAction::Read, - }))); - assert!(scopes.contains(&Scope::Identity(IdentityScope::All))); - - // Test that atproto doesn't grant other scopes (per recent change) - let scopes = - Scope::::parse_multiple_reduced("atproto account:email repo:*").unwrap(); - assert_eq!(scopes.len(), 3); - assert!(scopes.contains(&Scope::Atproto)); - assert!(scopes.contains(&Scope::Account(AccountScope { - resource: AccountResource::Email, - action: AccountAction::Read, - }))); - assert!(scopes.contains(&Scope::Repo(RepoScope { - collection: RepoCollection::All, - actions: { - let mut actions = BTreeSet::new(); - actions.insert(RepoAction::Create); - actions.insert(RepoAction::Update); - actions.insert(RepoAction::Delete); - actions - } - }))); - } - #[test] fn test_openid_connect_scope_parsing() { // Test OpenID scope @@ -3260,251 +2881,6 @@ mod tests { assert!(!account.grants(&email)); } - #[test] - fn test_parse_multiple_with_openid_connect() { - let scopes = Scope::::parse_multiple("openid profile email atproto").unwrap(); - assert_eq!(scopes.len(), 4); - assert_eq!(scopes[0], Scope::OpenId); - assert_eq!(scopes[1], Scope::Profile); - assert_eq!(scopes[2], Scope::Email); - assert_eq!(scopes[3], Scope::Atproto); - - // Test with mixed scopes - let scopes = - Scope::::parse_multiple("openid account:email profile repo:*").unwrap(); - assert_eq!(scopes.len(), 4); - assert!(scopes.contains(&Scope::OpenId)); - assert!(scopes.contains(&Scope::Profile)); - } - - #[test] - fn test_parse_multiple_reduced_with_openid_connect() { - // OpenID Connect scopes don't grant each other, so no reduction - let scopes = - Scope::::parse_multiple_reduced("openid profile email openid").unwrap(); - assert_eq!(scopes.len(), 3); - assert!(scopes.contains(&Scope::OpenId)); - assert!(scopes.contains(&Scope::Profile)); - assert!(scopes.contains(&Scope::Email)); - - // Mixed with other scopes - let scopes = Scope::::parse_multiple_reduced( - "openid account:email account:email?action=manage profile", - ) - .unwrap(); - assert_eq!(scopes.len(), 3); - assert!(scopes.contains(&Scope::OpenId)); - assert!(scopes.contains(&Scope::Profile)); - assert!(scopes.contains(&Scope::Account(AccountScope { - resource: AccountResource::Email, - action: AccountAction::Manage, - }))); - } - - #[test] - fn test_serialize_multiple() { - // Test empty list - let scopes: Vec = vec![]; - assert_eq!(Scope::serialize_multiple(&scopes), ""); - - // Test single scope - let scopes = vec![Scope::Atproto]; - assert_eq!(Scope::::serialize_multiple(&scopes), "atproto"); - - // Test multiple scopes - should be sorted alphabetically - let scopes = vec![ - Scope::::parse("repo:*").unwrap(), - Scope::Atproto, - Scope::parse("account:email").unwrap(), - ]; - assert_eq!( - Scope::serialize_multiple(&scopes), - "account:email atproto repo:*" - ); - - // Test that sorting is consistent regardless of input order - let scopes = vec![ - Scope::::parse("identity:handle").unwrap(), - Scope::parse("blob:image/png").unwrap(), - Scope::parse("account:repo?action=manage").unwrap(), - ]; - assert_eq!( - Scope::serialize_multiple(&scopes), - "account:repo?action=manage blob:image/png identity:handle" - ); - - // Test with OpenID Connect scopes - let scopes = vec![Scope::Email, Scope::OpenId, Scope::Profile, Scope::Atproto]; - assert_eq!( - Scope::::serialize_multiple(&scopes), - "atproto email openid profile" - ); - - // Test with complex scopes including query parameters - let scopes = vec![ - Scope::::parse("rpc:com.example.service?aud=did:plc:yfvwmnlztr4dwkb7hwz55r2g&lxm=com.example.method") - .unwrap(), - Scope::parse("repo:app.bsky.feed.post?action=create&action=update").unwrap(), - Scope::parse("blob:image/*?accept=image/png&accept=image/jpeg").unwrap(), - ]; - let result = Scope::serialize_multiple(&scopes); - // The result should be sorted alphabetically - // Note: RPC scope with query params is serialized as "rpc?aud=...&lxm=..." - assert!(result.starts_with("blob:")); - assert!(result.contains(" repo:")); - assert!( - result.contains("rpc?aud=did:plc:yfvwmnlztr4dwkb7hwz55r2g&lxm=com.example.service") - ); - - // Test with transition scopes - let scopes = vec![ - Scope::Transition(TransitionScope::Email), - Scope::Transition(TransitionScope::Generic), - Scope::Atproto, - ]; - assert_eq!( - Scope::<&str>::serialize_multiple(&scopes), - "atproto transition:email transition:generic" - ); - - // Test duplicates - they remain in the output (caller's responsibility to dedupe if needed) - let scopes = vec![ - Scope::Atproto, - Scope::Atproto, - Scope::::parse("account:email").unwrap(), - ]; - assert_eq!( - Scope::serialize_multiple(&scopes), - "account:email atproto atproto" - ); - - // Test normalization is preserved in serialization - let scopes = - vec![Scope::::parse("blob?accept=image/png&accept=image/jpeg").unwrap()]; - // Should normalize query parameters alphabetically - assert_eq!( - Scope::serialize_multiple(&scopes), - "blob?accept=image/jpeg&accept=image/png" - ); - } - - #[test] - fn test_serialize_multiple_roundtrip() { - // Test that parse_multiple and serialize_multiple are inverses (when sorted) - let original = "account:email atproto blob:image/png identity:handle repo:*"; - let scopes = Scope::::parse_multiple(original).unwrap(); - let serialized = Scope::serialize_multiple(&scopes); - assert_eq!(serialized, original); - - // Test with complex scopes - let original = "account:repo?action=manage blob?accept=image/jpeg&accept=image/png rpc:*"; - let scopes = Scope::::parse_multiple(original).unwrap(); - let serialized = Scope::serialize_multiple(&scopes); - // Parse again to verify it's valid - let reparsed = Scope::parse_multiple(&serialized).unwrap(); - assert_eq!(scopes, reparsed); - - // Test with OpenID Connect scopes - let original = "email openid profile"; - let scopes = Scope::::parse_multiple(original).unwrap(); - let serialized = Scope::serialize_multiple(&scopes); - assert_eq!(serialized, original); - } - - #[test] - fn test_remove_scope() { - // Test removing a scope that exists - let scopes = vec![ - Scope::::parse("repo:*").unwrap(), - Scope::Atproto, - Scope::parse("account:email").unwrap(), - ]; - let to_remove = Scope::Atproto; - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert!(!result.contains(&to_remove)); - assert!(result.contains(&Scope::parse("repo:*").unwrap())); - assert!(result.contains(&Scope::parse("account:email").unwrap())); - - // Test removing a scope that doesn't exist - let scopes = vec![ - Scope::::parse("repo:*").unwrap(), - Scope::parse("account:email").unwrap(), - ]; - let to_remove = Scope::parse("identity:handle").unwrap(); - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert_eq!(result, scopes); - - // Test removing from empty list - let scopes: Vec = vec![]; - let to_remove = Scope::Atproto; - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 0); - - // Test removing all instances of a duplicate scope - let scopes = vec![ - Scope::Atproto, - Scope::::parse("account:email").unwrap(), - Scope::Atproto, - Scope::parse("repo:*").unwrap(), - Scope::Atproto, - ]; - let to_remove = Scope::Atproto; - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert!(!result.contains(&to_remove)); - assert!(result.contains(&Scope::parse("account:email").unwrap())); - assert!(result.contains(&Scope::parse("repo:*").unwrap())); - - // Test removing complex scopes with query parameters - let scopes = vec![ - Scope::::parse("account:email?action=manage").unwrap(), - Scope::parse("blob?accept=image/png&accept=image/jpeg").unwrap(), - Scope::parse("rpc:com.example.service?aud=did:example:123").unwrap(), - ]; - let to_remove = Scope::parse("blob?accept=image/jpeg&accept=image/png").unwrap(); // Note: normalized order - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert!(!result.contains(&to_remove)); - - // Test with OpenID Connect scopes - let scopes = vec![Scope::OpenId, Scope::Profile, Scope::Email, Scope::Atproto]; - let to_remove = Scope::Profile; - let result = Scope::<&str>::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 3); - assert!(!result.contains(&to_remove)); - assert!(result.contains(&Scope::OpenId)); - assert!(result.contains(&Scope::Email)); - assert!(result.contains(&Scope::Atproto)); - - // Test with transition scopes - let scopes = vec![ - Scope::Transition(TransitionScope::Generic), - Scope::Transition(TransitionScope::Email), - Scope::Atproto, - ]; - let to_remove = Scope::Transition(TransitionScope::Email); - let result = Scope::<&str>::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert!(!result.contains(&to_remove)); - assert!(result.contains(&Scope::Transition(TransitionScope::Generic))); - assert!(result.contains(&Scope::Atproto)); - - // Test that only exact matches are removed - let scopes = vec![ - Scope::::parse("account:email").unwrap(), - Scope::parse("account:email?action=manage").unwrap(), - Scope::parse("account:repo").unwrap(), - ]; - let to_remove = Scope::parse("account:email").unwrap(); - let result = Scope::remove_scope(&scopes, &to_remove); - assert_eq!(result.len(), 2); - assert!(!result.contains(&Scope::parse("account:email").unwrap())); - assert!(result.contains(&Scope::parse("account:email?action=manage").unwrap())); - assert!(result.contains(&Scope::parse("account:repo").unwrap())); - } - // ======================================================================== // Tests for Task 1: Scopes container and constructor // ======================================================================== diff --git a/crates/jacquard-oauth/src/session.rs b/crates/jacquard-oauth/src/session.rs index 794797ed..388b6380 100644 --- a/crates/jacquard-oauth/src/session.rs +++ b/crates/jacquard-oauth/src/session.rs @@ -9,7 +9,7 @@ use crate::{ keyset::Keyset, request::{OAuthMetadata, refresh}, resolver::OAuthResolver, - scopes::Scope, + scopes::Scopes, types::TokenSet, }; @@ -51,7 +51,7 @@ pub trait DpopDataSource { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(bound( serialize = "S: serde::Serialize + BosStr + Ord", - deserialize = "S: serde::Deserialize<'de> + BosStr, Scope: serde::Deserialize<'de>" + deserialize = "S: serde::Deserialize<'de> + BosStr + AsRef" ))] pub struct ClientSessionData { /// DID of the authenticated account; serves as the primary key for session storage @@ -77,7 +77,7 @@ pub struct ClientSessionData { pub authserver_revocation_endpoint: Option, /// The set of OAuth scopes approved for this session, as returned in the initial token response. - pub scopes: Vec>, + pub scopes: Scopes, /// DPoP key and nonce state for ongoing requests in this session. #[serde(flatten)] @@ -88,9 +88,9 @@ pub struct ClientSessionData { pub token_set: TokenSet, } -impl IntoStatic for ClientSessionData +impl> IntoStatic for ClientSessionData where - S::Output: BosStr + Ord, + S::Output: BosStr + Ord + AsRef, { type Output = ClientSessionData; @@ -111,7 +111,7 @@ where } } -impl ClientSessionData { +impl> ClientSessionData { /// Update this session's token set and, if the new token set includes scopes, replace the scope list. /// /// Called after a successful token refresh so that any scope changes returned by the server @@ -122,15 +122,14 @@ impl ClientSessionData { /// not be refreshed in place. pub fn update_with_tokens(&mut self, token_set: &TokenSet) where - S: FromStr + Clone, + S: FromStr + Clone + From + AsRef, S::Err: std::fmt::Debug, { - if let Some(Ok(scopes)) = token_set - .scope - .as_ref() - .map(|scope| Scope::::parse_multiple_reduced(scope.as_ref())) - { - self.scopes = scopes.into_iter().map(|s| s.convert()).collect(); + if let Some(scope_str) = token_set.scope.as_ref() { + // Parse scopes from the returned scope string, converting to the appropriate backing type + let scopes_smol = Scopes::new(SmolStr::from(scope_str.as_ref())) + .expect("server returned invalid scopes in token refresh"); + self.scopes = scopes_smol.convert(); } self.token_set = token_set.clone(); } @@ -180,7 +179,7 @@ impl DpopDataSource for DpopClientData { #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] #[serde(bound( serialize = "S: serde::Serialize + BosStr + Ord", - deserialize = "S: serde::Deserialize<'de> + BosStr, Scope: serde::Deserialize<'de>" + deserialize = "S: serde::Deserialize<'de> + BosStr + AsRef" ))] pub struct AuthRequestData { /// Random identifier generated for this authorization request; used as the primary key @@ -196,7 +195,7 @@ pub struct AuthRequestData { pub account_did: Option>, /// OAuth scopes requested for this authorization. - pub scopes: Vec>, + pub scopes: Scopes, /// The PAR `request_uri` returned by the authorization server; included in the redirect URL. pub request_uri: S, @@ -217,9 +216,9 @@ pub struct AuthRequestData { pub dpop_data: DpopReqData, } -impl IntoStatic for AuthRequestData +impl> IntoStatic for AuthRequestData where - S::Output: BosStr + Ord, + S::Output: BosStr + Ord + AsRef, { type Output = AuthRequestData; diff --git a/crates/jacquard-oauth/src/types.rs b/crates/jacquard-oauth/src/types.rs index a1be7f60..5b67fa46 100644 --- a/crates/jacquard-oauth/src/types.rs +++ b/crates/jacquard-oauth/src/types.rs @@ -4,7 +4,7 @@ mod request; mod response; mod token; -use crate::scopes::Scope; +use crate::scopes::Scopes; pub use self::client_metadata::*; pub use self::metadata::*; @@ -59,29 +59,33 @@ impl From for &'static str { /// Options for initiating an OAuth authorization request. #[derive(Debug)] -pub struct AuthorizeOptions { +pub struct AuthorizeOptions +where + S: AsRef, +{ /// Override the redirect URI registered in the client metadata. pub redirect_uri: Option>, /// Scopes to request. Defaults to an empty list (server-defined defaults apply). - pub scopes: Vec>, + pub scopes: Scopes, /// Optional prompt hint for the authorization server's UI. pub prompt: Option, /// Opaque client-provided state value, echoed back in the callback for CSRF protection. pub state: Option, } -impl Default for AuthorizeOptions { +impl + From> Default for AuthorizeOptions { fn default() -> Self { + let empty_scopes: Scopes = Scopes::empty().convert(); Self { redirect_uri: None, - scopes: vec![], + scopes: empty_scopes, prompt: None, state: None, } } } -impl AuthorizeOptions { +impl> AuthorizeOptions { /// Set the `prompt` parameter sent to the authorization server. pub fn with_prompt(mut self, prompt: AuthorizeOptionPrompt) -> Self { self.prompt = Some(prompt); @@ -101,7 +105,7 @@ impl AuthorizeOptions { } /// Set the OAuth scopes to request. - pub fn with_scopes(mut self, scopes: Vec>) -> Self { + pub fn with_scopes(mut self, scopes: Scopes) -> Self { self.scopes = scopes; self } diff --git a/crates/jacquard/src/client/token.rs b/crates/jacquard/src/client/token.rs index 9f792585..ed9e9c1d 100644 --- a/crates/jacquard/src/client/token.rs +++ b/crates/jacquard/src/client/token.rs @@ -1,7 +1,7 @@ use jacquard_common::deps::fluent_uri::Uri; use jacquard_common::session::{FileTokenStore, SessionStore, SessionStoreError}; use jacquard_common::types::string::{Datetime, Did}; -use jacquard_oauth::scopes::Scope; +use jacquard_oauth::scopes::Scopes; use jacquard_oauth::session::{AuthRequestData, ClientSessionData, DpopClientData, DpopReqData}; use jacquard_oauth::types::OAuthTokenType; use jose_jwk::Key; @@ -59,8 +59,8 @@ pub struct OAuthSession { #[serde(skip_serializing_if = "std::option::Option::is_none")] authserver_revocation_endpoint: Option, - /// Granted scopes - scopes: Vec, + /// Granted scopes (space-separated, normalized). + scopes: String, /// Client DPoP key material pub dpop_key: Key, @@ -101,11 +101,7 @@ impl From> for OAuth authserver_revocation_endpoint: data .authserver_revocation_endpoint .map(|s| AsRef::::as_ref(&s).to_owned()), - scopes: data - .scopes - .into_iter() - .map(|s| String::from(s.to_string_normalized())) - .collect(), + scopes: String::from(data.scopes.to_normalized_string()), dpop_key: data.dpop_data.dpop_key, dpop_authserver_nonce: AsRef::::as_ref(&data.dpop_data.dpop_authserver_nonce) .to_owned(), @@ -139,11 +135,8 @@ impl From for ClientSessionData { authserver_revocation_endpoint: session .authserver_revocation_endpoint .map(SmolStr::from), - scopes: session - .scopes - .into_iter() - .map(|s| Scope::parse(&s).unwrap()) - .collect(), + scopes: Scopes::new(SmolStr::from(session.scopes.as_str())) + .expect("stored scopes should be valid"), dpop_data: DpopClientData { dpop_key: session.dpop_key, dpop_authserver_nonce: SmolStr::from(session.dpop_authserver_nonce), @@ -176,8 +169,8 @@ pub struct OAuthState { #[serde(skip_serializing_if = "std::option::Option::is_none")] pub account_did: Option, - /// Requested scopes - pub scopes: Vec, + /// Requested scopes (space-separated, normalized). + pub scopes: String, /// Request URI for the authorization step pub request_uri: String, @@ -208,11 +201,7 @@ impl TryFrom> for OAut account_did: value .account_did .map(|s| AsRef::::as_ref(&s).to_owned()), - scopes: value - .scopes - .into_iter() - .map(|s| String::from(s.to_string_normalized())) - .collect(), + scopes: String::from(value.scopes.to_normalized_string()), request_uri: AsRef::::as_ref(&value.request_uri).to_owned(), authserver_token_endpoint: AsRef::::as_ref(&value.authserver_token_endpoint) .to_owned(), @@ -239,11 +228,8 @@ impl From for AuthRequestData { .account_did .map(|s| Did::new_owned(s).expect("stored DID should be valid")), authserver_revocation_endpoint: value.authserver_revocation_endpoint.map(SmolStr::from), - scopes: value - .scopes - .into_iter() - .map(|s| Scope::parse(&s).unwrap()) - .collect(), + scopes: Scopes::new(SmolStr::from(value.scopes.as_str())) + .expect("stored scopes should be valid"), request_uri: SmolStr::from(value.request_uri), authserver_token_endpoint: SmolStr::from(value.authserver_token_endpoint), pkce_verifier: SmolStr::from(value.pkce_verifier), diff --git a/crates/jacquard/tests/oauth_auto_refresh.rs b/crates/jacquard/tests/oauth_auto_refresh.rs index 25c3c831..5baabfca 100644 --- a/crates/jacquard/tests/oauth_auto_refresh.rs +++ b/crates/jacquard/tests/oauth_auto_refresh.rs @@ -12,7 +12,7 @@ use jacquard_common::http_client::HttpClient; use jacquard_oauth::atproto::AtprotoClientMetadata; use jacquard_oauth::client::OAuthSession; use jacquard_oauth::resolver::OAuthResolver; -use jacquard_oauth::scopes::Scope; +use jacquard_oauth::scopes::Scopes; use jacquard_oauth::session::SessionRegistry; use jacquard_oauth::session::{ClientData, ClientSessionData, DpopClientData}; use jacquard_oauth::types::{OAuthAuthorizationServerMetadata, OAuthTokenType, TokenSet}; @@ -211,7 +211,7 @@ async fn oauth_xrpc_invalid_token_triggers_refresh_and_retries() { let client_data = ClientData { keyset: None, - config: AtprotoClientMetadata::new_localhost(None, Some(vec![Scope::Atproto])), + config: AtprotoClientMetadata::new_localhost(None, Some(Scopes::new(SmolStr::new_static("atproto")).unwrap())), }; use jacquard::IntoStatic; let session_data = ClientSessionData { @@ -221,7 +221,7 @@ async fn oauth_xrpc_invalid_token_triggers_refresh_and_retries() { authserver_url: SmolStr::new_static("https://issuer"), authserver_token_endpoint: SmolStr::from("https://issuer/token"), authserver_revocation_endpoint: None, - scopes: vec![Scope::Atproto], + scopes: Scopes::new(SmolStr::new_static("atproto")).unwrap(), dpop_data: DpopClientData { dpop_key: jacquard_oauth::utils::generate_key(&[SmolStr::from("ES256")]).unwrap(), dpop_authserver_nonce: SmolStr::from(""), @@ -249,7 +249,7 @@ async fn oauth_xrpc_invalid_token_triggers_refresh_and_retries() { authserver_url: SmolStr::new_static("https://issuer"), authserver_token_endpoint: SmolStr::from("https://issuer/token"), authserver_revocation_endpoint: None, - scopes: vec![Scope::Atproto], + scopes: Scopes::new(SmolStr::new_static("atproto")).unwrap(), dpop_data: DpopClientData { dpop_key: jacquard_oauth::utils::generate_key(&[SmolStr::from("ES256")]).unwrap(), dpop_authserver_nonce: SmolStr::from(""), @@ -340,7 +340,7 @@ async fn oauth_xrpc_invalid_token_body_triggers_refresh_and_retries() { let client_data = ClientData { keyset: None, - config: AtprotoClientMetadata::new_localhost(None, Some(vec![Scope::Atproto])), + config: AtprotoClientMetadata::new_localhost(None, Some(Scopes::new(SmolStr::new_static("atproto")).unwrap())), }; use jacquard::IntoStatic; let session_data = ClientSessionData { @@ -350,7 +350,7 @@ async fn oauth_xrpc_invalid_token_body_triggers_refresh_and_retries() { authserver_url: SmolStr::new_static("https://issuer"), authserver_token_endpoint: SmolStr::from("https://issuer/token"), authserver_revocation_endpoint: None, - scopes: vec![Scope::Atproto], + scopes: Scopes::new(SmolStr::new_static("atproto")).unwrap(), dpop_data: DpopClientData { dpop_key: jacquard_oauth::utils::generate_key(&[SmolStr::from("ES256")]).unwrap(), dpop_authserver_nonce: SmolStr::from(""), diff --git a/crates/jacquard/tests/oauth_flow.rs b/crates/jacquard/tests/oauth_flow.rs index 0d236545..955ba2e2 100644 --- a/crates/jacquard/tests/oauth_flow.rs +++ b/crates/jacquard/tests/oauth_flow.rs @@ -11,7 +11,7 @@ use jacquard_oauth::atproto::AtprotoClientMetadata; use jacquard_oauth::authstore::ClientAuthStore; use jacquard_oauth::client::OAuthClient; use jacquard_oauth::resolver::OAuthResolver; -use jacquard_oauth::scopes::Scope; +use jacquard_oauth::scopes::Scopes; use jacquard_oauth::session::ClientData; use smol_str::SmolStr; @@ -215,7 +215,7 @@ async fn oauth_end_to_end_mock_flow() { let client_data: ClientData<_> = ClientData { keyset: None, - config: AtprotoClientMetadata::new_localhost(None, Some(vec![Scope::Atproto])), + config: AtprotoClientMetadata::new_localhost(None, Some(Scopes::new(SmolStr::new_static("atproto")).unwrap())), }; let client_arc = client.clone(); let oauth = OAuthClient::new_from_resolver(store, (*client_arc).clone(), client_data); @@ -225,7 +225,7 @@ async fn oauth_end_to_end_mock_flow() { let mut metadata = jacquard_oauth::request::OAuthMetadata { server_metadata, client_metadata: jacquard_oauth::atproto::atproto_client_metadata( - &AtprotoClientMetadata::new_localhost(None, Some(vec![Scope::Atproto])), + &AtprotoClientMetadata::new_localhost(None, Some(Scopes::new(SmolStr::new_static("atproto")).unwrap())), &None, ) .unwrap(), -- 2.51.2