diff --git a/crates/tranquil-api/src/server/passkey_account.rs b/crates/tranquil-api/src/server/passkey_account.rs index 33d96b9..2780da6 100644 --- a/crates/tranquil-api/src/server/passkey_account.rs +++ b/crates/tranquil-api/src/server/passkey_account.rs @@ -492,7 +492,7 @@ pub async fn complete_passkey_setup( } }; - let security_key = match webauthn.finish_registration(&credential, ®_state) { + let passkey = match webauthn.finish_registration(&credential, ®_state) { Ok(sk) => sk, Err(e) => { warn!("Passkey registration failed: {:?}", e); @@ -500,11 +500,11 @@ pub async fn complete_passkey_setup( } }; - let credential_id = security_key.cred_id().to_vec(); - let public_key = match serde_json::to_vec(&security_key) { + let credential_id = passkey.cred_id().to_vec(); + let public_key = match serde_json::to_vec(&passkey) { Ok(pk) => pk, Err(e) => { - error!("Error serializing security key: {:?}", e); + error!("Error serializing passkey: {:?}", e); return Err(ApiError::InternalError(None)); } }; diff --git a/crates/tranquil-api/src/server/passkeys.rs b/crates/tranquil-api/src/server/passkeys.rs index b683a9f..d79611c 100644 --- a/crates/tranquil-api/src/server/passkeys.rs +++ b/crates/tranquil-api/src/server/passkeys.rs @@ -104,11 +104,10 @@ pub async fn finish_passkey_registration( .log_db_err("loading registration state")? .ok_or(ApiError::NoRegistrationInProgress)?; - let reg_state: SecurityKeyRegistration = - serde_json::from_str(®_state_json).map_err(|e| { - error!("Failed to deserialize registration state: {:?}", e); - ApiError::InternalError(None) - })?; + let reg_state: PasskeyRegistration = serde_json::from_str(®_state_json).map_err(|e| { + error!("Failed to deserialize registration state: {:?}", e); + ApiError::InternalError(None) + })?; let credential: RegisterPublicKeyCredential = serde_json::from_value(input.credential) .map_err(|e| { diff --git a/crates/tranquil-api/src/server/reauth.rs b/crates/tranquil-api/src/server/reauth.rs index 154f958..b4ae82f 100644 --- a/crates/tranquil-api/src/server/reauth.rs +++ b/crates/tranquil-api/src/server/reauth.rs @@ -159,7 +159,7 @@ pub async fn reauth_passkey_start( return Err(ApiError::NoPasskeys); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -216,7 +216,7 @@ pub async fn reauth_passkey_finish( .log_db_err("loading authentication state")? .ok_or(ApiError::NoChallengeInProgress)?; - let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = + let auth_state: webauthn_rs::prelude::PasskeyAuthentication = serde_json::from_str(&auth_state_json).map_err(|e| { error!("Failed to deserialize authentication state: {:?}", e); ApiError::InternalError(None) diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs index d114c20..ad1aab3 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs @@ -327,7 +327,7 @@ async fn passkey_start_named( .into_response(); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -714,7 +714,7 @@ async fn passkey_finish_named( ).into_response() })?; - let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = + let auth_state: webauthn_rs::prelude::PasskeyAuthentication = serde_json::from_str(&auth_state_json).map_err(|e| { tracing::error!(error = %e, "Failed to deserialize authentication state"); ( @@ -972,7 +972,7 @@ pub async fn authorize_passkey_start( .into_response(); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -1146,7 +1146,7 @@ pub async fn authorize_passkey_finish( } }; - let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = match serde_json::from_str( + let auth_state: webauthn_rs::prelude::PasskeyAuthentication = match serde_json::from_str( &auth_state_json, ) { Ok(s) => s, diff --git a/crates/tranquil-pds/src/auth/webauthn.rs b/crates/tranquil-pds/src/auth/webauthn.rs index e255f25..3c25dc7 100644 --- a/crates/tranquil-pds/src/auth/webauthn.rs +++ b/crates/tranquil-pds/src/auth/webauthn.rs @@ -28,8 +28,7 @@ impl WebAuthnConfig { let builder = WebauthnBuilder::new(&rp_id, &rp_origin) .map_err(|e| WebauthnError::BuilderFailed(e.to_string()))? - .rp_name("Tranquil PDS") - .danger_set_user_presence_only_security_keys(true); + .rp_name("Tranquil PDS"); let webauthn = builder .build() @@ -44,11 +43,11 @@ impl WebAuthnConfig { username: &str, display_name: &str, exclude_credentials: Vec, - ) -> Result<(CreationChallengeResponse, SecurityKeyRegistration), WebauthnError> { + ) -> Result<(CreationChallengeResponse, PasskeyRegistration), WebauthnError> { let user_unique_id = Uuid::new_v5(&Uuid::NAMESPACE_OID, user_id.as_bytes()); self.webauthn - .start_securitykey_registration( + .start_passkey_registration( user_unique_id, username, display_name, @@ -57,8 +56,6 @@ impl WebAuthnConfig { } else { Some(exclude_credentials) }, - None, - None, ) .map(|(mut ccr, state)| { let sel = ccr @@ -75,29 +72,29 @@ impl WebAuthnConfig { pub fn finish_registration( &self, reg: &RegisterPublicKeyCredential, - state: &SecurityKeyRegistration, - ) -> Result { + state: &PasskeyRegistration, + ) -> Result { self.webauthn - .finish_securitykey_registration(reg, state) + .finish_passkey_registration(reg, state) .map_err(|e| WebauthnError::RegistrationFailed(e.to_string())) } pub fn start_authentication( &self, - credentials: Vec, - ) -> Result<(RequestChallengeResponse, SecurityKeyAuthentication), WebauthnError> { + credentials: Vec, + ) -> Result<(RequestChallengeResponse, PasskeyAuthentication), WebauthnError> { self.webauthn - .start_securitykey_authentication(&credentials) + .start_passkey_authentication(&credentials) .map_err(|e| WebauthnError::AuthenticationFailed(e.to_string())) } pub fn finish_authentication( &self, auth: &PublicKeyCredential, - state: &SecurityKeyAuthentication, + state: &PasskeyAuthentication, ) -> Result { self.webauthn - .finish_securitykey_authentication(auth, state) + .finish_passkey_authentication(auth, state) .map_err(|e| WebauthnError::AuthenticationFailed(e.to_string())) } -- 2.51.2 From 9e78206cf411e4e556160d58f0cb89871f83a889 Mon Sep 17 00:00:00 2001 From: Johanna Larsson Date: Sat, 8 Aug 2026 13:22:59 +0100 Subject: [PATCH 2/9] Switch back to SecurityKey, remove hint --- .../src/server/passkey_account.rs | 8 +++--- crates/tranquil-api/src/server/passkeys.rs | 9 ++++--- crates/tranquil-api/src/server/reauth.rs | 4 +-- .../src/endpoints/authorize/passkey.rs | 8 +++--- crates/tranquil-pds/src/auth/webauthn.rs | 27 ++++++++++++------- 5 files changed, 32 insertions(+), 24 deletions(-) diff --git a/crates/tranquil-api/src/server/passkey_account.rs b/crates/tranquil-api/src/server/passkey_account.rs index 2780da6..33d96b9 100644 --- a/crates/tranquil-api/src/server/passkey_account.rs +++ b/crates/tranquil-api/src/server/passkey_account.rs @@ -492,7 +492,7 @@ pub async fn complete_passkey_setup( } }; - let passkey = match webauthn.finish_registration(&credential, ®_state) { + let security_key = match webauthn.finish_registration(&credential, ®_state) { Ok(sk) => sk, Err(e) => { warn!("Passkey registration failed: {:?}", e); @@ -500,11 +500,11 @@ pub async fn complete_passkey_setup( } }; - let credential_id = passkey.cred_id().to_vec(); - let public_key = match serde_json::to_vec(&passkey) { + let credential_id = security_key.cred_id().to_vec(); + let public_key = match serde_json::to_vec(&security_key) { Ok(pk) => pk, Err(e) => { - error!("Error serializing passkey: {:?}", e); + error!("Error serializing security key: {:?}", e); return Err(ApiError::InternalError(None)); } }; diff --git a/crates/tranquil-api/src/server/passkeys.rs b/crates/tranquil-api/src/server/passkeys.rs index d79611c..b683a9f 100644 --- a/crates/tranquil-api/src/server/passkeys.rs +++ b/crates/tranquil-api/src/server/passkeys.rs @@ -104,10 +104,11 @@ pub async fn finish_passkey_registration( .log_db_err("loading registration state")? .ok_or(ApiError::NoRegistrationInProgress)?; - let reg_state: PasskeyRegistration = serde_json::from_str(®_state_json).map_err(|e| { - error!("Failed to deserialize registration state: {:?}", e); - ApiError::InternalError(None) - })?; + let reg_state: SecurityKeyRegistration = + serde_json::from_str(®_state_json).map_err(|e| { + error!("Failed to deserialize registration state: {:?}", e); + ApiError::InternalError(None) + })?; let credential: RegisterPublicKeyCredential = serde_json::from_value(input.credential) .map_err(|e| { diff --git a/crates/tranquil-api/src/server/reauth.rs b/crates/tranquil-api/src/server/reauth.rs index b4ae82f..154f958 100644 --- a/crates/tranquil-api/src/server/reauth.rs +++ b/crates/tranquil-api/src/server/reauth.rs @@ -159,7 +159,7 @@ pub async fn reauth_passkey_start( return Err(ApiError::NoPasskeys); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -216,7 +216,7 @@ pub async fn reauth_passkey_finish( .log_db_err("loading authentication state")? .ok_or(ApiError::NoChallengeInProgress)?; - let auth_state: webauthn_rs::prelude::PasskeyAuthentication = + let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = serde_json::from_str(&auth_state_json).map_err(|e| { error!("Failed to deserialize authentication state: {:?}", e); ApiError::InternalError(None) diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs index ad1aab3..d114c20 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/passkey.rs @@ -327,7 +327,7 @@ async fn passkey_start_named( .into_response(); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -714,7 +714,7 @@ async fn passkey_finish_named( ).into_response() })?; - let auth_state: webauthn_rs::prelude::PasskeyAuthentication = + let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = serde_json::from_str(&auth_state_json).map_err(|e| { tracing::error!(error = %e, "Failed to deserialize authentication state"); ( @@ -972,7 +972,7 @@ pub async fn authorize_passkey_start( .into_response(); } - let passkeys: Vec = stored_passkeys + let passkeys: Vec = stored_passkeys .iter() .filter_map(|sp| serde_json::from_slice(&sp.public_key).ok()) .collect(); @@ -1146,7 +1146,7 @@ pub async fn authorize_passkey_finish( } }; - let auth_state: webauthn_rs::prelude::PasskeyAuthentication = match serde_json::from_str( + let auth_state: webauthn_rs::prelude::SecurityKeyAuthentication = match serde_json::from_str( &auth_state_json, ) { Ok(s) => s, diff --git a/crates/tranquil-pds/src/auth/webauthn.rs b/crates/tranquil-pds/src/auth/webauthn.rs index 3c25dc7..e8ed4ec 100644 --- a/crates/tranquil-pds/src/auth/webauthn.rs +++ b/crates/tranquil-pds/src/auth/webauthn.rs @@ -43,11 +43,11 @@ impl WebAuthnConfig { username: &str, display_name: &str, exclude_credentials: Vec, - ) -> Result<(CreationChallengeResponse, PasskeyRegistration), WebauthnError> { + ) -> Result<(CreationChallengeResponse, SecurityKeyRegistration), WebauthnError> { let user_unique_id = Uuid::new_v5(&Uuid::NAMESPACE_OID, user_id.as_bytes()); self.webauthn - .start_passkey_registration( + .start_securitykey_registration( user_unique_id, username, display_name, @@ -56,6 +56,8 @@ impl WebAuthnConfig { } else { Some(exclude_credentials) }, + None, + None, ) .map(|(mut ccr, state)| { let sel = ccr @@ -64,6 +66,7 @@ impl WebAuthnConfig { .get_or_insert_with(AuthenticatorSelectionCriteria::default); sel.resident_key = Some(ResidentKeyRequirement::Required); sel.require_resident_key = true; + ccr.public_key.hints = None; (ccr, state) }) .map_err(|e| WebauthnError::RegistrationFailed(e.to_string())) @@ -72,29 +75,33 @@ impl WebAuthnConfig { pub fn finish_registration( &self, reg: &RegisterPublicKeyCredential, - state: &PasskeyRegistration, - ) -> Result { + state: &SecurityKeyRegistration, + ) -> Result { self.webauthn - .finish_passkey_registration(reg, state) + .finish_securitykey_registration(reg, state) .map_err(|e| WebauthnError::RegistrationFailed(e.to_string())) } pub fn start_authentication( &self, - credentials: Vec, - ) -> Result<(RequestChallengeResponse, PasskeyAuthentication), WebauthnError> { + credentials: Vec, + ) -> Result<(RequestChallengeResponse, SecurityKeyAuthentication), WebauthnError> { self.webauthn - .start_passkey_authentication(&credentials) + .start_securitykey_authentication(&credentials) + .map(|(mut rcr, state)| { + rcr.public_key.hints = None; + (rcr, state) + }) .map_err(|e| WebauthnError::AuthenticationFailed(e.to_string())) } pub fn finish_authentication( &self, auth: &PublicKeyCredential, - state: &PasskeyAuthentication, + state: &SecurityKeyAuthentication, ) -> Result { self.webauthn - .finish_passkey_authentication(auth, state) + .finish_securitykey_authentication(auth, state) .map_err(|e| WebauthnError::AuthenticationFailed(e.to_string())) } -- 2.51.2 From bc751b0ee2fef8dfebb5c36775b5aa672e5d086d Mon Sep 17 00:00:00 2001 From: Johanna Larsson Date: Sat, 8 Aug 2026 16:29:28 +0100 Subject: [PATCH 3/9] Bring back thing that made yubikey work --- crates/tranquil-pds/src/auth/webauthn.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/tranquil-pds/src/auth/webauthn.rs b/crates/tranquil-pds/src/auth/webauthn.rs index e8ed4ec..ea669b0 100644 --- a/crates/tranquil-pds/src/auth/webauthn.rs +++ b/crates/tranquil-pds/src/auth/webauthn.rs @@ -28,7 +28,8 @@ impl WebAuthnConfig { let builder = WebauthnBuilder::new(&rp_id, &rp_origin) .map_err(|e| WebauthnError::BuilderFailed(e.to_string()))? - .rp_name("Tranquil PDS"); + .rp_name("Tranquil PDS") + .danger_set_user_presence_only_security_keys(true); let webauthn = builder .build() -- 2.51.2 From dc2fbe665419961694ba7add4402e56dfebcc19e Mon Sep 17 00:00:00 2001 From: Jack Platten Date: Wed, 12 Aug 2026 14:04:25 -0700 Subject: [PATCH 4/9] chore: use JSON array for healthcheck now that container is distroless --- deploy/quadlets/tranquil-pds-app.container | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deploy/quadlets/tranquil-pds-app.container b/deploy/quadlets/tranquil-pds-app.container index 0b13b6a..3bf5c3b 100644 --- a/deploy/quadlets/tranquil-pds-app.container +++ b/deploy/quadlets/tranquil-pds-app.container @@ -10,7 +10,7 @@ Environment=SERVER_PORT=3000 Volume=/srv/tranquil-pds/config/config.toml:/etc/tranquil-pds/config.toml:ro,Z Volume=/srv/tranquil-pds/blobs:/var/lib/tranquil-pds/blobs:Z Volume=/srv/tranquil-pds/store:/var/lib/tranquil-pds/store:Z -HealthCmd=wget -q --spider http://localhost:3000/xrpc/_health +HealthCmd=["/usr/local/bin/tranquil-pds", "healthcheck"] HealthInterval=30s HealthTimeout=10s HealthRetries=3 -- 2.51.2 From a5a2f30bbe4adb6854d92e548830860c449c3a55 Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Fri, 7 Aug 2026 21:47:57 +0200 Subject: [PATCH 5/9] fix: Collapse action parameters for repo scopes TODO: Still missing tests! --- crates/tranquil-scopes/src/permission_set.rs | 26 +++++++++++++++++--- 1 file changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index 8897ef0..5fe4fec 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -339,7 +339,9 @@ fn build_expanded_scopes( default_aud: Option<&str>, namespace_authority: &str, ) -> String { - let mut scopes: Vec = Vec::new(); + // Key is `repo`, value is array of actions + let mut ungrouped_repo_scopes: HashMap> = HashMap::new(); + let mut rpc_scopes: Vec = Vec::new(); permissions .iter() @@ -357,7 +359,14 @@ fn build_expanded_scopes( .filter(|coll| is_under_authority(coll, namespace_authority)) .for_each(|coll| { actions.iter().for_each(|action| { - scopes.push(format!("repo:{}?action={}", coll, action)); + let existing = ungrouped_repo_scopes.get_mut(coll); + + if existing.is_none() { + ungrouped_repo_scopes + .insert(coll.to_string(), vec![action.to_string()]); + } else { + existing.unwrap().push(action.to_string()); + } }); }); } @@ -373,14 +382,23 @@ fn build_expanded_scopes( Some(aud) => format!("rpc:{}?aud={}", lxm, aud), None => format!("rpc:{}", lxm), }; - scopes.push(scope); + + rpc_scopes.push(scope); }); } } _ => {} }); - scopes.join(" ") + let grouped_repo_scopes: Vec = ungrouped_repo_scopes + .iter() + .map(|(repo, actions)| format!("repo:{}?action={}", repo, actions.join("&action="))) + .collect(); + + let combined_repo_scopes = grouped_repo_scopes.join(" "); + let combined_rpc_scopes = rpc_scopes.join(" "); + + format!("{} {}", combined_repo_scopes, combined_rpc_scopes) } #[cfg(test)] -- 2.51.2 From 434079a73294beb389822571f091f58183b686d3 Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Sat, 8 Aug 2026 00:44:57 +0200 Subject: [PATCH 6/9] feat: compress large token scopes with brotli --- CONTRIBUTING.md | 2 +- Cargo.lock | 37 +++++ crates/tranquil-auth/Cargo.toml | 1 + crates/tranquil-auth/src/compress.rs | 145 +++++++++++++++++++ crates/tranquil-auth/src/lib.rs | 3 + crates/tranquil-auth/src/token.rs | 6 +- crates/tranquil-auth/src/verify.rs | 16 +- crates/tranquil-scopes/src/permission_set.rs | 14 +- 8 files changed, 213 insertions(+), 11 deletions(-) create mode 100644 crates/tranquil-auth/src/compress.rs diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index ef6c728..8f69075 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -64,7 +64,7 @@ In order of importance the following rules describe what "correct" means for Tra and not something said application relies on for proper functioning. There is bound to be edge cases that these rules don't fully cover. -Here common sense, community sentiment, furthering the goals of atproto itself, and ultimately maintainer opinion take precedence over support for any individual applicaion. +Here common sense, community sentiment, furthering the goals of atproto itself, and ultimately maintainer opinion take precedence over support for any individual application. Even Bluesky. The rules above are meant to capture Tranquils goals of being correct while being community oriented and avoiding as much "Bluesky-defaultism" as possible. diff --git a/Cargo.lock b/Cargo.lock index 6a7c118..4679a5c 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -105,6 +105,21 @@ version = "0.1.3" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "250f629c0161ad8107cf89319e990051fae62832fd343083bea452d93e2205fd" +[[package]] +name = "alloc-no-stdlib" +version = "2.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cc7bb162ec39d46ab1ca8c77bf72e890535becd1751bb45f64c597edb4c8c6b3" + +[[package]] +name = "alloc-stdlib" +version = "0.2.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0e76a019e91224d279006ff972f1e984179a6e9feb050adba6ce8274aef23195" +dependencies = [ + "alloc-no-stdlib", +] + [[package]] name = "allocator-api2" version = "0.2.21" @@ -1250,6 +1265,27 @@ dependencies = [ "cfg_aliases", ] +[[package]] +name = "brotli" +version = "8.0.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5cc91aac060a7a1e25823bdccbfb6af1875b88f17c6daac97894eed8207166b3" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", + "brotli-decompressor", +] + +[[package]] +name = "brotli-decompressor" +version = "5.0.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3a32acac15fe1967bc3986b2a6347dffc965602354ea6f450ad07e8bfd253583" +dependencies = [ + "alloc-no-stdlib", + "alloc-stdlib", +] + [[package]] name = "bs58" version = "0.5.1" @@ -7682,6 +7718,7 @@ dependencies = [ "base32", "base64 0.22.1", "bcrypt", + "brotli", "chrono", "hmac", "k256", diff --git a/crates/tranquil-auth/Cargo.toml b/crates/tranquil-auth/Cargo.toml index 6a4c981..863c1d9 100644 --- a/crates/tranquil-auth/Cargo.toml +++ b/crates/tranquil-auth/Cargo.toml @@ -24,3 +24,4 @@ subtle = { workspace = true } totp-rs = { workspace = true } urlencoding = { workspace = true } uuid = { workspace = true } +brotli = "8.0.4" diff --git a/crates/tranquil-auth/src/compress.rs b/crates/tranquil-auth/src/compress.rs new file mode 100644 index 0000000..263e09d --- /dev/null +++ b/crates/tranquil-auth/src/compress.rs @@ -0,0 +1,145 @@ +use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; +use brotli::{CompressorWriter, Decompressor}; +use std::fmt; +use std::io::{Read, Write}; + +const COMPRESSED_PREFIX: &str = "$br$"; +const QUALITY: u32 = 9; +const WINDOW_BITS: u32 = 16; +const BUFFER_SIZE: usize = 4096; +const MAX_DECOMPRESSED_LEN: u64 = 64 * 1024; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopeDecodeError { + Base64DecodeFailed, + DecompressFailed, + TooLarge, +} + +impl fmt::Display for ScopeDecodeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Base64DecodeFailed => write!(f, "Base64 decode of compressed scope failed"), + Self::DecompressFailed => write!(f, "Brotli decompression of scope failed"), + Self::TooLarge => write!(f, "Decompressed scope exceeds maximum length"), + } + } +} + +impl std::error::Error for ScopeDecodeError {} + +fn brotli_compress(input: &str) -> Vec { + let mut writer = CompressorWriter::new(Vec::new(), BUFFER_SIZE, QUALITY, WINDOW_BITS); + + writer + .write_all(input.as_bytes()) + .expect("writing to a Vec cannot fail"); + + writer.into_inner() +} + +fn brotli_decompress(input: &[u8]) -> Result { + let mut output = String::new(); + + Decompressor::new(input, BUFFER_SIZE) + .take(MAX_DECOMPRESSED_LEN + 1) + .read_to_string(&mut output) + .map_err(|_| ScopeDecodeError::DecompressFailed)?; + + if output.len() as u64 > MAX_DECOMPRESSED_LEN { + return Err(ScopeDecodeError::TooLarge); + } + + Ok(output) +} + +pub fn encode_scope(scope: &str) -> String { + let encoded = URL_SAFE_NO_PAD.encode(brotli_compress(scope)); + + if COMPRESSED_PREFIX.len() + encoded.len() < scope.len() { + format!("{COMPRESSED_PREFIX}{encoded}") + } else { + scope.to_owned() + } +} + +pub fn decode_scope(scope: &str) -> Result { + let Some(encoded) = scope.strip_prefix(COMPRESSED_PREFIX) else { + return Ok(scope.to_owned()); + }; + + let compressed = URL_SAFE_NO_PAD + .decode(encoded) + .map_err(|_| ScopeDecodeError::Base64DecodeFailed)?; + + brotli_decompress(&compressed) +} + +#[cfg(test)] +mod tests { + use super::*; + + fn long_scope() -> String { + let mut scope = String::from("transition:generic transition:chat.bsky"); + for collection in [ + "social.colibri.message", + "social.colibri.community", + "social.colibri.reaction", + "social.colibri.member", + "social.colibri.channel.read", + ] { + scope.push_str(&format!(" repo:{collection}?action=create&action=delete")); + } + scope + } + + #[test] + fn long_scope_roundtrips_through_compression() { + let scope = long_scope(); + let encoded = encode_scope(&scope); + + assert!(encoded.starts_with(COMPRESSED_PREFIX)); + assert!(encoded.len() < scope.len()); + assert_eq!(decode_scope(&encoded).unwrap(), scope); + } + + #[test] + fn short_scope_stays_plaintext() { + let encoded = encode_scope("com.atproto.access"); + + assert_eq!(encoded, "com.atproto.access"); + assert_eq!(decode_scope(&encoded).unwrap(), "com.atproto.access"); + } + + #[test] + fn untagged_scope_passes_through() { + assert_eq!( + decode_scope("com.atproto.refresh").unwrap(), + "com.atproto.refresh" + ); + assert_eq!(decode_scope("").unwrap(), ""); + } + + #[test] + fn malformed_compressed_scope_errors_instead_of_panicking() { + assert_eq!( + decode_scope("$br$not valid base64!"), + Err(ScopeDecodeError::Base64DecodeFailed) + ); + assert_eq!( + decode_scope("$br$AAAAAAAAAAAAAAAA"), + Err(ScopeDecodeError::DecompressFailed) + ); + } + + #[test] + fn compression_bomb_is_rejected() { + let bomb = encode_scope(&"a".repeat(MAX_DECOMPRESSED_LEN as usize * 2)); + let encoded = bomb.strip_prefix(COMPRESSED_PREFIX).unwrap_or(&bomb); + + assert_eq!( + decode_scope(&format!("{COMPRESSED_PREFIX}{encoded}")), + Err(ScopeDecodeError::TooLarge) + ); + } +} diff --git a/crates/tranquil-auth/src/lib.rs b/crates/tranquil-auth/src/lib.rs index 1f9b8c6..611ab55 100644 --- a/crates/tranquil-auth/src/lib.rs +++ b/crates/tranquil-auth/src/lib.rs @@ -1,3 +1,4 @@ +mod compress; mod token; mod totp; mod types; @@ -12,6 +13,8 @@ pub use token::{ create_service_token_hs256, }; +pub use compress::{ScopeDecodeError, decode_scope, encode_scope}; + pub use totp::{ TotpError, decrypt_totp_secret, encrypt_totp_secret, generate_backup_codes, generate_qr_png_base64, generate_totp_secret, generate_totp_uri, hash_backup_code, diff --git a/crates/tranquil-auth/src/token.rs b/crates/tranquil-auth/src/token.rs index 9088b67..c6f66fc 100644 --- a/crates/tranquil-auth/src/token.rs +++ b/crates/tranquil-auth/src/token.rs @@ -1,3 +1,5 @@ +use crate::compress::encode_scope; + use super::types::{ ActClaim, Claims, Header, SigningAlgorithm, TokenScope, TokenType, TokenWithMetadata, }; @@ -205,7 +207,7 @@ fn create_signed_token_pinned( aud: format!("did:web:{}", aud_hostname), exp: expiration, iat: Utc::now().timestamp(), - scope: Some(scope.to_string()), + scope: Some(encode_scope(scope)), lxm: None, jti: jti.clone(), act, @@ -328,7 +330,7 @@ fn create_hs256_token_with_metadata( ), exp: expiration, iat: Utc::now().timestamp(), - scope: Some(scope.to_string()), + scope: Some(encode_scope(scope)), lxm: None, jti: jti.clone(), act: None, diff --git a/crates/tranquil-auth/src/verify.rs b/crates/tranquil-auth/src/verify.rs index fb59fc3..b0f74bf 100644 --- a/crates/tranquil-auth/src/verify.rs +++ b/crates/tranquil-auth/src/verify.rs @@ -1,3 +1,5 @@ +use crate::compress::decode_scope; + use super::types::{ Claims, Header, SigningAlgorithm, TokenData, TokenDecodeError, TokenScope, TokenType, TokenVerifyError, UnsafeClaims, @@ -164,9 +166,15 @@ pub fn verify_token_es256k( .decode(claims_b64) .map_err(|_| TokenVerifyError::Invalid("Base64 decode of claims failed"))?; - let claims: Claims = serde_json::from_slice(&claims_bytes) + let mut claims: Claims = serde_json::from_slice(&claims_bytes) .map_err(|_| TokenVerifyError::Invalid("JSON decode of claims failed"))?; + if let Some(scope) = &claims.scope { + claims.scope = Some( + decode_scope(scope).map_err(|_| TokenVerifyError::Invalid("Invalid token scope"))?, + ); + } + let now = Utc::now().timestamp(); if claims.exp < now { return Err(TokenVerifyError::Expired); @@ -244,9 +252,13 @@ fn verify_token_hs256_internal( .decode(claims_b64) .context("Base64 decode of claims failed")?; - let claims: Claims = + let mut claims: Claims = serde_json::from_slice(&claims_bytes).context("JSON decode of claims failed")?; + if let Some(scope) = &claims.scope { + claims.scope = Some(decode_scope(scope).context("Invalid scope claim encoding")?); + } + let now = Utc::now().timestamp(); if claims.exp < now { return Err(anyhow!("Token expired")); diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index 5fe4fec..b0b032e 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -399,6 +399,8 @@ fn build_expanded_scopes( let combined_rpc_scopes = rpc_scopes.join(" "); format!("{} {}", combined_repo_scopes, combined_rpc_scopes) + .trim() + .to_string() } #[cfg(test)] @@ -477,9 +479,8 @@ mod tests { }]; let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); - assert!(expanded.contains("repo:io.atcr.manifest?action=create")); - assert!(expanded.contains("repo:io.atcr.manifest?action=delete")); - assert!(expanded.contains("repo:io.atcr.sailor.star?action=create")); + assert!(expanded.contains("repo:io.atcr.manifest?action=create&action=delete")); + assert!(expanded.contains("repo:io.atcr.sailor.star?action=create&action=delete")); assert!(!expanded.contains("app.bsky.feed.post")); } @@ -494,9 +495,10 @@ mod tests { }]; let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); - assert!(expanded.contains("repo:io.atcr.manifest?action=create")); - assert!(expanded.contains("repo:io.atcr.manifest?action=update")); - assert!(expanded.contains("repo:io.atcr.manifest?action=delete")); + assert!(expanded.contains("repo:io.atcr.manifest?action=")); + assert!(expanded.contains("action=create")); + assert!(expanded.contains("action=update")); + assert!(expanded.contains("action=delete")); } #[test] -- 2.51.2 From b3c314ce669a4ad6d13aee4b39e823c19f2ed532 Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Sat, 8 Aug 2026 16:45:08 +0200 Subject: [PATCH 7/9] fix: Address review comments --- crates/tranquil-auth/src/compress.rs | 48 +++-- crates/tranquil-auth/src/lib.rs | 2 +- crates/tranquil-auth/src/token.rs | 6 +- .../src/endpoints/token/helpers.rs | 3 +- crates/tranquil-pds/src/auth/mod.rs | 17 +- crates/tranquil-pds/src/oauth/verify.rs | 4 +- .../tests/oauth_permission_sets.rs | 123 ++++++++++++ crates/tranquil-scopes/src/permission_set.rs | 177 +++++++++++++++--- 8 files changed, 330 insertions(+), 50 deletions(-) diff --git a/crates/tranquil-auth/src/compress.rs b/crates/tranquil-auth/src/compress.rs index 263e09d..1efc50b 100644 --- a/crates/tranquil-auth/src/compress.rs +++ b/crates/tranquil-auth/src/compress.rs @@ -7,7 +7,7 @@ const COMPRESSED_PREFIX: &str = "$br$"; const QUALITY: u32 = 9; const WINDOW_BITS: u32 = 16; const BUFFER_SIZE: usize = 4096; -const MAX_DECOMPRESSED_LEN: u64 = 64 * 1024; +const MAX_SCOPE_LEN: u64 = 64 * 1024; #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ScopeDecodeError { @@ -28,6 +28,21 @@ impl fmt::Display for ScopeDecodeError { impl std::error::Error for ScopeDecodeError {} +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ScopeEncodeError { + TooLarge, +} + +impl fmt::Display for ScopeEncodeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::TooLarge => write!(f, "Scope exceeds maximum length"), + } + } +} + +impl std::error::Error for ScopeEncodeError {} + fn brotli_compress(input: &str) -> Vec { let mut writer = CompressorWriter::new(Vec::new(), BUFFER_SIZE, QUALITY, WINDOW_BITS); @@ -42,24 +57,28 @@ fn brotli_decompress(input: &[u8]) -> Result { let mut output = String::new(); Decompressor::new(input, BUFFER_SIZE) - .take(MAX_DECOMPRESSED_LEN + 1) + .take(MAX_SCOPE_LEN + 1) .read_to_string(&mut output) .map_err(|_| ScopeDecodeError::DecompressFailed)?; - if output.len() as u64 > MAX_DECOMPRESSED_LEN { + if output.len() as u64 > MAX_SCOPE_LEN { return Err(ScopeDecodeError::TooLarge); } Ok(output) } -pub fn encode_scope(scope: &str) -> String { +pub fn encode_scope(scope: &str) -> Result { + if scope.len() as u64 > MAX_SCOPE_LEN { + return Err(ScopeEncodeError::TooLarge); + } + let encoded = URL_SAFE_NO_PAD.encode(brotli_compress(scope)); if COMPRESSED_PREFIX.len() + encoded.len() < scope.len() { - format!("{COMPRESSED_PREFIX}{encoded}") + Ok(format!("{COMPRESSED_PREFIX}{encoded}")) } else { - scope.to_owned() + Ok(scope.to_owned()) } } @@ -96,7 +115,7 @@ mod tests { #[test] fn long_scope_roundtrips_through_compression() { let scope = long_scope(); - let encoded = encode_scope(&scope); + let encoded = encode_scope(&scope).unwrap(); assert!(encoded.starts_with(COMPRESSED_PREFIX)); assert!(encoded.len() < scope.len()); @@ -105,7 +124,7 @@ mod tests { #[test] fn short_scope_stays_plaintext() { - let encoded = encode_scope("com.atproto.access"); + let encoded = encode_scope("com.atproto.access").unwrap(); assert_eq!(encoded, "com.atproto.access"); assert_eq!(decode_scope(&encoded).unwrap(), "com.atproto.access"); @@ -134,12 +153,19 @@ mod tests { #[test] fn compression_bomb_is_rejected() { - let bomb = encode_scope(&"a".repeat(MAX_DECOMPRESSED_LEN as usize * 2)); - let encoded = bomb.strip_prefix(COMPRESSED_PREFIX).unwrap_or(&bomb); + let bomb = URL_SAFE_NO_PAD.encode(brotli_compress(&"a".repeat(MAX_SCOPE_LEN as usize * 2))); assert_eq!( - decode_scope(&format!("{COMPRESSED_PREFIX}{encoded}")), + decode_scope(&format!("{COMPRESSED_PREFIX}{bomb}")), Err(ScopeDecodeError::TooLarge) ); } + + #[test] + fn encode_rejects_oversized_scope() { + let oversized = "a".repeat(MAX_SCOPE_LEN as usize + 1); + + assert_eq!(encode_scope(&oversized), Err(ScopeEncodeError::TooLarge)); + assert!(encode_scope(&"a".repeat(MAX_SCOPE_LEN as usize)).is_ok()); + } } diff --git a/crates/tranquil-auth/src/lib.rs b/crates/tranquil-auth/src/lib.rs index 611ab55..aed10ed 100644 --- a/crates/tranquil-auth/src/lib.rs +++ b/crates/tranquil-auth/src/lib.rs @@ -13,7 +13,7 @@ pub use token::{ create_service_token_hs256, }; -pub use compress::{ScopeDecodeError, decode_scope, encode_scope}; +pub use compress::{ScopeDecodeError, ScopeEncodeError, decode_scope, encode_scope}; pub use totp::{ TotpError, decrypt_totp_secret, encrypt_totp_secret, generate_backup_codes, diff --git a/crates/tranquil-auth/src/token.rs b/crates/tranquil-auth/src/token.rs index c6f66fc..9fee5fb 100644 --- a/crates/tranquil-auth/src/token.rs +++ b/crates/tranquil-auth/src/token.rs @@ -3,7 +3,7 @@ use crate::compress::encode_scope; use super::types::{ ActClaim, Claims, Header, SigningAlgorithm, TokenScope, TokenType, TokenWithMetadata, }; -use anyhow::Result; +use anyhow::{Context, Result}; use base64::Engine as _; use base64::engine::general_purpose::URL_SAFE_NO_PAD; use chrono::{DateTime, Duration, Utc}; @@ -207,7 +207,7 @@ fn create_signed_token_pinned( aud: format!("did:web:{}", aud_hostname), exp: expiration, iat: Utc::now().timestamp(), - scope: Some(encode_scope(scope)), + scope: Some(encode_scope(scope).context("Scope too large to encode")?), lxm: None, jti: jti.clone(), act, @@ -330,7 +330,7 @@ fn create_hs256_token_with_metadata( ), exp: expiration, iat: Utc::now().timestamp(), - scope: Some(encode_scope(scope)), + scope: Some(encode_scope(scope).context("Scope too large to encode")?), lxm: None, jti: jti.clone(), act: None, diff --git a/crates/tranquil-oauth-server/src/endpoints/token/helpers.rs b/crates/tranquil-oauth-server/src/endpoints/token/helpers.rs index ae0df2d..3a56e08 100644 --- a/crates/tranquil-oauth-server/src/endpoints/token/helpers.rs +++ b/crates/tranquil-oauth-server/src/endpoints/token/helpers.rs @@ -43,7 +43,8 @@ pub fn create_access_token_with_delegation( let issuer = format!("https://{}", pds_hostname); let now = Utc::now().timestamp(); let exp = now + ACCESS_TOKEN_EXPIRY_SECONDS; - let actual_scope = scope.unwrap_or("atproto"); + let actual_scope = tranquil_pds::auth::encode_scope(scope.unwrap_or("atproto")) + .map_err(|_| OAuthError::InvalidScope("Scope too large".to_string()))?; let mut payload = json!({ "iss": issuer, "sub": sub.as_str(), diff --git a/crates/tranquil-pds/src/auth/mod.rs b/crates/tranquil-pds/src/auth/mod.rs index 4d3c5b1..1995ca8 100644 --- a/crates/tranquil-pds/src/auth/mod.rs +++ b/crates/tranquil-pds/src/auth/mod.rs @@ -43,14 +43,15 @@ pub use scope_verified::{ pub use service::{ServiceTokenClaims, ServiceTokenError, ServiceTokenVerifier, is_service_token}; pub use tranquil_auth::{ - ActClaim, Claims, Header, SigningAlgorithm, TokenData, TokenDecodeError, TokenScope, TokenType, - TokenVerifyError, TokenWithMetadata, TotpError, UnsafeClaims, create_access_token, - create_access_token_hs256, create_access_token_hs256_with_metadata, - create_access_token_with_delegation, create_access_token_with_jti, - create_access_token_with_metadata, create_access_token_with_scope_metadata, - create_refresh_token, create_refresh_token_hs256, create_refresh_token_hs256_with_metadata, - create_refresh_token_with_jti, create_refresh_token_with_metadata, create_service_token, - create_service_token_hs256, generate_backup_codes, generate_qr_png_base64, + ActClaim, Claims, Header, ScopeDecodeError, ScopeEncodeError, SigningAlgorithm, TokenData, + TokenDecodeError, TokenScope, TokenType, TokenVerifyError, TokenWithMetadata, TotpError, + UnsafeClaims, create_access_token, create_access_token_hs256, + create_access_token_hs256_with_metadata, create_access_token_with_delegation, + create_access_token_with_jti, create_access_token_with_metadata, + create_access_token_with_scope_metadata, create_refresh_token, create_refresh_token_hs256, + create_refresh_token_hs256_with_metadata, create_refresh_token_with_jti, + create_refresh_token_with_metadata, create_service_token, create_service_token_hs256, + decode_scope, encode_scope, generate_backup_codes, generate_qr_png_base64, generate_totp_secret, generate_totp_uri, get_algorithm_from_token, get_did_from_token, get_jti_from_token, hash_backup_code, is_backup_code_format, verify_access_token, verify_access_token_hs256, verify_backup_code, verify_refresh_token, diff --git a/crates/tranquil-pds/src/oauth/verify.rs b/crates/tranquil-pds/src/oauth/verify.rs index da1b630..0d54d83 100644 --- a/crates/tranquil-pds/src/oauth/verify.rs +++ b/crates/tranquil-pds/src/oauth/verify.rs @@ -164,7 +164,9 @@ pub fn extract_oauth_token_info(token: &str) -> Result>() + .join(" "); + seed_permission_set(BIG_SET_NSID, &granular_scope).await; + + let scope = format!("atproto include:{}", BIG_SET_NSID); + let (session, _consent_body, _mock) = create_delegated_session_with_scope( + "psc", + "https://example.com/permset-compress-callback", + &scope, + ) + .await; + + let payload = decode_jwt_payload(&session.access_token); + let jwt_scope = payload["scope"] + .as_str() + .expect("access token JWT should have a scope claim"); + assert!( + jwt_scope.starts_with("$br$"), + "an expanded scope this long should be compressed in the JWT claim, got: {}", + jwt_scope + ); + + let decoded = tranquil_pds::auth::decode_scope(jwt_scope).expect("scope claim should decode"); + for coll in collections { + assert!( + decoded.contains(&format!("repo:{}?action=create", coll)), + "decoded scope should carry {}, got: {}", + coll, + decoded + ); + } + assert!( + !decoded.contains("include:"), + "decoded scope should not contain the raw include: token, got: {}", + decoded + ); + + let url = base_url().await; + let http_client = client(); + + let introspect_res = http_client + .post(format!("{}/oauth/introspect", url)) + .form(&[("token", session.access_token.as_str())]) + .send() + .await + .expect("introspect request failed"); + assert_eq!(introspect_res.status(), StatusCode::OK); + let introspect_body: Value = introspect_res.json().await.unwrap(); + let introspect_scope = introspect_body["scope"] + .as_str() + .expect("introspect response should have a scope string"); + assert_eq!( + introspect_scope, decoded, + "introspect should report the decoded scope" + ); + + let collection = collections[0]; + let create_res = http_client + .post(format!("{}/xrpc/com.atproto.repo.createRecord", url)) + .bearer_auth(&session.access_token) + .json(&json!({ + "repo": session.delegated_did, + "collection": collection, + "validate": false, + "record": { + "$type": collection, + "note": "compressed scope enforcement test", + "createdAt": Utc::now().to_rfc3339() + } + })) + .send() + .await + .expect("createRecord request failed"); + assert_ne!( + create_res.status(), + StatusCode::FORBIDDEN, + "a compressed scope claim must still authorize the collections it covers. Got body: {:?}", + create_res.text().await + ); + + let refresh_res = http_client + .post(format!("{}/oauth/token", url)) + .form(&[ + ("grant_type", "refresh_token"), + ("refresh_token", session.refresh_token.as_str()), + ("client_id", session.client_id.as_str()), + ]) + .send() + .await + .expect("Refresh request failed"); + assert_eq!(refresh_res.status(), StatusCode::OK); + let refresh_body: Value = refresh_res.json().await.unwrap(); + let refreshed_token = refresh_body["access_token"].as_str().unwrap(); + let refreshed_claim = decode_jwt_payload(refreshed_token)["scope"] + .as_str() + .expect("refreshed JWT should have a scope claim") + .to_string(); + assert!( + refreshed_claim.starts_with("$br$"), + "refreshed claim should also be compressed, got: {}", + refreshed_claim + ); + assert_eq!( + tranquil_pds::auth::decode_scope(&refreshed_claim).expect("refreshed scope should decode"), + decoded, + "refresh must yield a byte-identical decoded scope" + ); +} + #[tokio::test] async fn test_consent_post_errors_when_set_unresolvable() { const UNRESOLVABLE_NSID: &str = "io.atcr.authUnresolvableSet"; diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index b0b032e..ec7d14b 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -2,7 +2,7 @@ use hickory_resolver::TokioAsyncResolver; use hickory_resolver::config::{ResolverConfig, ResolverOpts}; use reqwest::Client; use serde::{Deserialize, Serialize}; -use std::collections::HashMap; +use std::collections::{BTreeMap, HashMap}; use tracing::debug; use tranquil_types::{Did, Nsid}; @@ -334,13 +334,20 @@ fn is_under_authority(target_nsid: &str, authority: &str) -> bool { const DEFAULT_ACTIONS: &[&str] = &["create", "update", "delete"]; +fn action_rank(action: &str) -> usize { + DEFAULT_ACTIONS + .iter() + .position(|known| *known == action) + .unwrap_or(DEFAULT_ACTIONS.len()) +} + fn build_expanded_scopes( permissions: &[PermissionEntry], default_aud: Option<&str>, namespace_authority: &str, ) -> String { // Key is `repo`, value is array of actions - let mut ungrouped_repo_scopes: HashMap> = HashMap::new(); + let mut ungrouped_repo_scopes: BTreeMap> = BTreeMap::new(); let mut rpc_scopes: Vec = Vec::new(); permissions @@ -354,21 +361,21 @@ fn build_expanded_scopes( .map(|a| a.iter().map(String::as_str).collect()) .unwrap_or_else(|| DEFAULT_ACTIONS.to_vec()); - collections - .iter() - .filter(|coll| is_under_authority(coll, namespace_authority)) - .for_each(|coll| { - actions.iter().for_each(|action| { - let existing = ungrouped_repo_scopes.get_mut(coll); - - if existing.is_none() { - ungrouped_repo_scopes - .insert(coll.to_string(), vec![action.to_string()]); - } else { - existing.unwrap().push(action.to_string()); - } + if !actions.is_empty() { + collections + .iter() + .filter(|coll| is_under_authority(coll, namespace_authority)) + .for_each(|coll| { + let existing = + ungrouped_repo_scopes.entry(coll.to_string()).or_default(); + + actions.iter().for_each(|action| { + if !existing.iter().any(|seen| seen == action) { + existing.push(action.to_string()); + } + }); }); - }); + } } } "rpc" => { @@ -383,7 +390,9 @@ fn build_expanded_scopes( None => format!("rpc:{}", lxm), }; - rpc_scopes.push(scope); + if !rpc_scopes.contains(&scope) { + rpc_scopes.push(scope); + } }); } } @@ -392,7 +401,12 @@ fn build_expanded_scopes( let grouped_repo_scopes: Vec = ungrouped_repo_scopes .iter() - .map(|(repo, actions)| format!("repo:{}?action={}", repo, actions.join("&action="))) + .map(|(repo, actions)| { + let mut actions = actions.clone(); + actions.sort_by(|a, b| action_rank(a).cmp(&action_rank(b)).then_with(|| a.cmp(b))); + + format!("repo:{}?action={}", repo, actions.join("&action=")) + }) .collect(); let combined_repo_scopes = grouped_repo_scopes.join(" "); @@ -479,9 +493,11 @@ mod tests { }]; let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); - assert!(expanded.contains("repo:io.atcr.manifest?action=create&action=delete")); - assert!(expanded.contains("repo:io.atcr.sailor.star?action=create&action=delete")); - assert!(!expanded.contains("app.bsky.feed.post")); + assert_eq!( + expanded, + "repo:io.atcr.manifest?action=create&action=delete \ + repo:io.atcr.sailor.star?action=create&action=delete" + ); } #[test] @@ -495,10 +511,121 @@ mod tests { }]; let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); - assert!(expanded.contains("repo:io.atcr.manifest?action=")); - assert!(expanded.contains("action=create")); - assert!(expanded.contains("action=update")); - assert!(expanded.contains("action=delete")); + assert_eq!( + expanded, + "repo:io.atcr.manifest?action=create&action=update&action=delete" + ); + } + + #[test] + fn test_build_expanded_scopes_repo_omitted_action_grants_all() { + let permissions = vec![PermissionEntry { + resource: "repo".to_string(), + action: None, + collection: Some(vec!["io.atcr.manifest".to_string()]), + lxm: None, + aud: None, + }]; + + let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); + assert_eq!( + expanded, "repo:io.atcr.manifest?action=create&action=update&action=delete", + "an omitted action list means all actions" + ); + } + + #[test] + fn test_build_expanded_scopes_repo_empty_action_list_skips_entry() { + let permissions = vec![PermissionEntry { + resource: "repo".to_string(), + action: Some(vec![]), + collection: Some(vec!["io.atcr.manifest".to_string()]), + lxm: None, + aud: None, + }]; + + let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); + assert!( + expanded.is_empty(), + "an explicitly empty action list is invalid, so the entry is skipped rather \ + than expanded to all actions or emitted as a bare `?action=`, got: {expanded}" + ); + } + + #[test] + fn test_build_expanded_scopes_is_deterministic() { + let permissions = vec![ + PermissionEntry { + resource: "repo".to_string(), + action: Some(vec!["create".to_string()]), + collection: Some(vec![ + "io.atcr.sailor.star".to_string(), + "io.atcr.manifest".to_string(), + "io.atcr.blob".to_string(), + ]), + lxm: None, + aud: None, + }, + PermissionEntry { + resource: "rpc".to_string(), + action: None, + collection: None, + lxm: Some(vec![ + "io.atcr.getManifest".to_string(), + "io.atcr.listTags".to_string(), + ]), + aud: Some("*".to_string()), + }, + ]; + + let first = build_expanded_scopes(&permissions, None, "io.atcr"); + assert_eq!( + first, + "repo:io.atcr.blob?action=create repo:io.atcr.manifest?action=create \ + repo:io.atcr.sailor.star?action=create \ + rpc:io.atcr.getManifest?aud=* rpc:io.atcr.listTags?aud=*" + ); + + for _ in 0..16 { + assert_eq!(build_expanded_scopes(&permissions, None, "io.atcr"), first); + } + } + + #[test] + fn test_build_expanded_scopes_dedupes_and_canonicalizes_actions() { + let permissions = vec![ + PermissionEntry { + resource: "repo".to_string(), + action: Some(vec!["delete".to_string(), "create".to_string()]), + collection: Some(vec!["io.atcr.manifest".to_string()]), + lxm: None, + aud: None, + }, + PermissionEntry { + resource: "repo".to_string(), + action: Some(vec!["create".to_string(), "update".to_string()]), + collection: Some(vec!["io.atcr.manifest".to_string()]), + lxm: None, + aud: None, + }, + PermissionEntry { + resource: "rpc".to_string(), + action: None, + collection: None, + lxm: Some(vec![ + "io.atcr.getManifest".to_string(), + "io.atcr.getManifest".to_string(), + ]), + aud: Some("*".to_string()), + }, + ]; + + let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); + assert_eq!( + expanded, + "repo:io.atcr.manifest?action=create&action=update&action=delete \ + rpc:io.atcr.getManifest?aud=*" + ); } #[test] -- 2.51.2 From c88f69f31d4b81aa89286398dafe1eb02294ea9f Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Sun, 9 Aug 2026 14:16:46 +0200 Subject: [PATCH 8/9] fix: Address PR review --- crates/tranquil-auth/src/compress.rs | 18 ++- .../src/endpoints/authorize/consent.rs | 2 +- crates/tranquil-pds/src/delegation/mod.rs | 2 +- crates/tranquil-pds/src/delegation/scopes.rs | 105 ++++++++++++------ .../tests/oauth_permission_sets.rs | 68 +++++++++++- crates/tranquil-pds/tests/scope_edge_cases.rs | 21 ++-- crates/tranquil-scopes/src/coverage.rs | 83 ++++++++++++-- crates/tranquil-scopes/src/lib.rs | 2 +- crates/tranquil-scopes/src/parser.rs | 86 +++++++++----- crates/tranquil-scopes/src/permission_set.rs | 89 +++++++++------ 10 files changed, 353 insertions(+), 123 deletions(-) diff --git a/crates/tranquil-auth/src/compress.rs b/crates/tranquil-auth/src/compress.rs index 1efc50b..6b75c8d 100644 --- a/crates/tranquil-auth/src/compress.rs +++ b/crates/tranquil-auth/src/compress.rs @@ -73,10 +73,13 @@ pub fn encode_scope(scope: &str) -> Result { return Err(ScopeEncodeError::TooLarge); } - let encoded = URL_SAFE_NO_PAD.encode(brotli_compress(scope)); + let tagged = format!( + "{COMPRESSED_PREFIX}{}", + URL_SAFE_NO_PAD.encode(brotli_compress(scope)) + ); - if COMPRESSED_PREFIX.len() + encoded.len() < scope.len() { - Ok(format!("{COMPRESSED_PREFIX}{encoded}")) + if tagged.len() < scope.len() || scope.starts_with(COMPRESSED_PREFIX) { + Ok(tagged) } else { Ok(scope.to_owned()) } @@ -161,6 +164,15 @@ mod tests { ); } + #[test] + fn plaintext_that_looks_compressed_roundtrips() { + let scope = "$br$repo:*"; + let encoded = encode_scope(scope).unwrap(); + + assert!(encoded.starts_with(COMPRESSED_PREFIX)); + assert_eq!(decode_scope(&encoded).unwrap(), scope); + } + #[test] fn encode_rejects_oversized_scope() { let oversized = "a".repeat(MAX_SCOPE_LEN as usize + 1); diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs index d459421..dae4d86 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs @@ -187,7 +187,7 @@ pub async fn consent_get( 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)) + grant_scope_str.is_some_and(|g| !tranquil_pds::delegation::grant_permits(g, scope)) }; let make_scope_info = |scope: &str| -> ScopeInfo { diff --git a/crates/tranquil-pds/src/delegation/mod.rs b/crates/tranquil-pds/src/delegation/mod.rs index 9ae5f3f..928fd2b 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, grant_covers, intersect_scopes, + ValidatedDelegationScope, grant_permits, 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 9ccaaaf..3aa007c 100644 --- a/crates/tranquil-pds/src/delegation/scopes.rs +++ b/crates/tranquil-pds/src/delegation/scopes.rs @@ -1,6 +1,6 @@ -use std::collections::HashSet; +use std::collections::BTreeSet; -use tranquil_scopes::{covers, parse_scope}; +use tranquil_scopes::{ParsedScope, narrow, parse_scope}; pub use tranquil_db_traits::{ DbScope as ValidatedDelegationScope, InvalidScopeError as InvalidDelegationScopeError, @@ -47,34 +47,40 @@ pub const SCOPE_PRESETS: &[ScopePreset] = &[ ]; pub fn intersect_scopes(requested: &str, granted: &str) -> String { - let requested_set: HashSet<&str> = requested.split_whitespace().collect(); - let granted_parsed: Vec = - granted.split_whitespace().map(parse_scope).collect(); - - let mut scopes: Vec<&str> = requested_set - .iter() - .filter(|requested_scope| { - **requested_scope != "atproto" && any_granted_covers(requested_scope, &granted_parsed) + let granted_parsed: Vec = granted.split_whitespace().map(parse_scope).collect(); + + let scopes: BTreeSet = requested + .split_whitespace() + .filter_map(|requested_scope| { + if requested_scope == "atproto" { + return Some(requested_scope.to_string()); + } + + let requested_parsed = parse_scope(requested_scope); + let narrowed = narrow(&granted_parsed, &requested_parsed)?; + + if narrowed == requested_parsed { + return Some(requested_scope.to_string()); + } + + match &narrowed { + ParsedScope::Repo(repo) => Some(repo.to_scope_string()), + _ => Some(requested_scope.to_string()), + } }) - .copied() - .chain(requested_set.contains("atproto").then_some("atproto")) .collect(); - scopes.sort(); - scopes.join(" ") + + scopes.into_iter().collect::>().join(" ") } -pub fn grant_covers(granted: &str, scope: &str) -> bool { +pub fn grant_permits(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)) + let granted_parsed: Vec = granted.split_whitespace().map(parse_scope).collect(); + + narrow(&granted_parsed, &parse_scope(scope)).is_some() } #[cfg(test)] @@ -220,12 +226,33 @@ mod tests { } #[test] - fn test_intersect_partial_action_grant_drops_actionless_request() { + fn test_intersect_partial_action_grant_narrows_actionless_request() { let result = intersect_scopes( "repo:app.bsky.feed.post", "repo:*?action=create&action=delete", ); - assert_eq!(result, ""); + assert_eq!( + result, + "repo:app.bsky.feed.post?action=create&action=delete" + ); + } + + #[test] + fn test_intersect_keeps_collapsed_request_under_split_action_grant() { + assert_eq!( + intersect_scopes( + "repo:io.atcr.manifest?action=create&action=delete", + EDITOR_FULL_SCOPES + ), + "repo:io.atcr.manifest?action=create&action=delete" + ); + assert_eq!( + intersect_scopes( + "repo:io.atcr.manifest?action=create&action=delete", + "repo:*?action=create" + ), + "repo:io.atcr.manifest?action=create" + ); } #[test] @@ -262,33 +289,41 @@ mod tests { } #[test] - fn test_grant_covers_matches_intersection() { + fn test_grant_permits_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( + assert!(grant_permits( granted, "repo:app.bsky.feed.post?action=create" )); - assert!(grant_covers(granted, "account:*?action=manage")); - assert!(!grant_covers(granted, "identity:*")); + assert!(grant_permits(granted, "account:*?action=manage")); + assert!(!grant_permits(granted, "identity:*")); assert_eq!( - grant_covers(granted, "identity:*"), + grant_permits(granted, "identity:*"), intersected.contains("identity") ); } #[test] - fn test_grant_covers_atproto_always_true() { - assert!(grant_covers("", "atproto")); - assert!(grant_covers("repo:*", "atproto")); + fn test_grant_permits_atproto_always_true() { + assert!(grant_permits("", "atproto")); + assert!(grant_permits("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:*")); + fn test_grant_permits_empty_grant_permits_nothing_else() { + assert!(!grant_permits("", "repo:app.bsky.feed.post?action=create")); + assert!(!grant_permits("", "identity:*")); + } + + #[test] + fn test_grant_permits_partially_granted_repo_scope() { + assert!(grant_permits( + EDITOR_FULL_SCOPES, + "repo:io.atcr.manifest?action=create&action=delete" + )); } } diff --git a/crates/tranquil-pds/tests/oauth_permission_sets.rs b/crates/tranquil-pds/tests/oauth_permission_sets.rs index 7cee66d..ad4ba22 100644 --- a/crates/tranquil-pds/tests/oauth_permission_sets.rs +++ b/crates/tranquil-pds/tests/oauth_permission_sets.rs @@ -15,6 +15,9 @@ use wiremock::{Mock, MockServer, ResponseTemplate}; const PERMISSION_SET_NSID: &str = "io.atcr.authFullApp"; const PERMISSION_SET_GRANULAR_SCOPE: &str = "repo:io.atcr.manifest?action=create rpc:io.atcr.getManifest?aud=*"; +const PERMISSION_SET_MULTI_ACTION_SCOPE: &str = + "repo:io.atcr.manifest?action=create&action=update&action=delete"; +const EDITOR_SET_NSID: &str = "io.atcr.authEditorApp"; fn disable_rate_limiting_once() { static ONCE: std::sync::Once = std::sync::Once::new(); @@ -105,6 +108,21 @@ async fn create_delegated_session_with_scope( handle_prefix: &str, redirect_uri: &str, scope: &str, +) -> (DelegatedSession, Value, MockServer) { + create_delegated_session_with_grant( + handle_prefix, + redirect_uri, + scope, + tranquil_pds::delegation::OWNER_FULL_SCOPES, + ) + .await +} + +async fn create_delegated_session_with_grant( + handle_prefix: &str, + redirect_uri: &str, + scope: &str, + controller_scopes: &str, ) -> (DelegatedSession, Value, MockServer) { let url = base_url().await; disable_rate_limiting_once(); @@ -119,7 +137,7 @@ async fn create_delegated_session_with_scope( .bearer_auth(&controller_jwt) .json(&json!({ "handle": delegated_handle, - "controllerScopes": tranquil_pds::delegation::OWNER_FULL_SCOPES + "controllerScopes": controller_scopes })) .send() .await @@ -375,6 +393,54 @@ async fn test_delegated_include_scope_shows_granular_on_consent() { ); } +#[tokio::test] +async fn test_delegated_editor_grant_keeps_collapsed_permission_set() { + seed_permission_set(EDITOR_SET_NSID, PERMISSION_SET_MULTI_ACTION_SCOPE).await; + + let scope = format!("atproto include:{}", EDITOR_SET_NSID); + let (session, consent_body, _mock) = create_delegated_session_with_grant( + "pse", + "https://example.com/permset-editor-callback", + &scope, + tranquil_pds::delegation::EDITOR_FULL_SCOPES, + ) + .await; + + let set_entry = consent_body["permission_sets"] + .as_array() + .expect("consent response should have a permission_sets array") + .iter() + .find(|s| s["nsid"].as_str() == Some(EDITOR_SET_NSID)) + .unwrap_or_else(|| { + panic!( + "permission_sets should contain an entry for nsid '{}'. Got: {:?}", + EDITOR_SET_NSID, consent_body + ) + }); + assert_eq!( + set_entry["restricted"].as_bool(), + Some(false), + "an editor grant spells its actions as separate tokens, but it still permits every \ + action in the collapsed set, so the set must not be marked restricted. Got: {:?}", + set_entry + ); + + let payload = decode_jwt_payload(&session.access_token); + let jwt_scope = tranquil_pds::auth::decode_scope( + payload["scope"] + .as_str() + .expect("access token JWT should have a scope claim"), + ) + .expect("JWT scope claim should decode"); + assert!( + jwt_scope.contains(PERMISSION_SET_MULTI_ACTION_SCOPE), + "delegated intersection must narrow the collapsed repo scope rather than discard it, \ + expected '{}' in decoded scope, got: {}", + PERMISSION_SET_MULTI_ACTION_SCOPE, + jwt_scope + ); +} + #[tokio::test] async fn test_grant_row_keeps_include_jwt_carries_expanded() { seed_permission_set(PERMISSION_SET_NSID, PERMISSION_SET_GRANULAR_SCOPE).await; diff --git a/crates/tranquil-pds/tests/scope_edge_cases.rs b/crates/tranquil-pds/tests/scope_edge_cases.rs index 4319939..bdca3bb 100644 --- a/crates/tranquil-pds/tests/scope_edge_cases.rs +++ b/crates/tranquil-pds/tests/scope_edge_cases.rs @@ -254,15 +254,18 @@ fn test_scope_with_multiple_params() { } #[test] -fn test_scope_invalid_action_ignored() { - let scope = parse_scope("repo:*?action=invalid"); - if let ParsedScope::Repo(repo) = scope { - assert!(repo.actions.contains(&RepoAction::Create)); - assert!(repo.actions.contains(&RepoAction::Update)); - assert!(repo.actions.contains(&RepoAction::Delete)); - } else { - panic!("Expected Repo scope"); - } +fn test_scope_invalid_action_rejects_whole_scope() { + assert!( + matches!( + parse_scope("repo:*?action=invalid"), + ParsedScope::Unknown(_) + ), + "an unrecognized action must not fall back to granting every action" + ); + assert!(matches!( + parse_scope("repo:*?action=create&action=invalid"), + ParsedScope::Unknown(_) + )); } #[test] diff --git a/crates/tranquil-scopes/src/coverage.rs b/crates/tranquil-scopes/src/coverage.rs index 716da42..54dcbec 100644 --- a/crates/tranquil-scopes/src/coverage.rs +++ b/crates/tranquil-scopes/src/coverage.rs @@ -1,7 +1,8 @@ use crate::parser::{ AccountAction, AccountAttr, AccountScope, BlobScope, IdentityAttr, IdentityScope, ParsedScope, - RepoScope, RpcScope, + RepoAction, RepoScope, RpcScope, }; +use std::collections::HashSet; pub fn covers(granted: &ParsedScope, requested: &ParsedScope) -> bool { use ParsedScope::*; @@ -21,8 +22,8 @@ pub fn covers(granted: &ParsedScope, requested: &ParsedScope) -> bool { } } -fn repo_covers(g: &RepoScope, r: &RepoScope) -> bool { - let collection_ok = match &g.collection { +fn repo_collection_covers(g: &RepoScope, r: &RepoScope) -> bool { + match &g.collection { None => true, Some(gc) => match &r.collection { None => false, @@ -33,8 +34,36 @@ fn repo_covers(g: &RepoScope, r: &RepoScope) -> bool { None => gc == rc, }, }, - }; - collection_ok && r.actions.is_subset(&g.actions) + } +} + +fn repo_covers(g: &RepoScope, r: &RepoScope) -> bool { + repo_collection_covers(g, r) && r.actions.is_subset(&g.actions) +} + +pub fn narrow(granted: &[ParsedScope], requested: &ParsedScope) -> Option { + if let ParsedScope::Repo(r) = requested { + let actions: HashSet = granted + .iter() + .filter_map(|g| match g { + ParsedScope::Repo(g) if repo_collection_covers(g, r) => Some(&g.actions), + _ => None, + }) + .flat_map(|granted_actions| granted_actions.intersection(&r.actions).copied()) + .collect(); + + return (!actions.is_empty()).then(|| { + ParsedScope::Repo(RepoScope { + collection: r.collection.clone(), + actions, + }) + }); + } + + granted + .iter() + .any(|g| covers(g, requested)) + .then(|| requested.clone()) } fn blob_covers(g: &BlobScope, r: &BlobScope) -> bool { @@ -74,13 +103,23 @@ fn identity_covers(g: &IdentityScope, r: &IdentityScope) -> bool { #[cfg(test)] mod tests { - use super::covers; - use crate::parser::parse_scope; + use super::{covers, narrow}; + use crate::parser::{ParsedScope, parse_scope}; fn c(granted: &str, requested: &str) -> bool { covers(&parse_scope(granted), &parse_scope(requested)) } + fn narrowed(granted: &str, requested: &str) -> Option { + let granted: Vec = granted.split_whitespace().map(parse_scope).collect(); + + match narrow(&granted, &parse_scope(requested)) { + Some(ParsedScope::Repo(repo)) => Some(repo.to_scope_string()), + Some(_) => Some(requested.to_string()), + None => None, + } + } + #[test] fn repo_wildcard_covers_specific() { assert!(c("repo:*", "repo:app.bsky.feed.post")); @@ -193,4 +232,34 @@ mod tests { assert!(c("weird:token", "weird:token")); assert!(!c("weird:token", "other:token")); } + + #[test] + fn narrow_intersects_repo_actions() { + assert_eq!( + narrowed( + "repo:*?action=create repo:*?action=update repo:*?action=delete", + "repo:io.atcr.manifest?action=create&action=delete" + ), + Some("repo:io.atcr.manifest?action=create&action=delete".to_string()) + ); + assert_eq!( + narrowed( + "repo:*?action=create", + "repo:io.atcr.manifest?action=create&action=delete" + ), + Some("repo:io.atcr.manifest?action=create".to_string()) + ); + assert_eq!( + narrowed( + "repo:*?action=create", + "repo:io.atcr.manifest?action=delete" + ), + None + ); + assert_eq!(narrowed("repo:app.bsky.*?action=create", "repo:*"), None); + assert_eq!( + narrowed("identity:*", "identity:handle"), + Some("identity:handle".to_string()) + ); + } } diff --git a/crates/tranquil-scopes/src/lib.rs b/crates/tranquil-scopes/src/lib.rs index 1160e56..e7b6861 100644 --- a/crates/tranquil-scopes/src/lib.rs +++ b/crates/tranquil-scopes/src/lib.rs @@ -5,7 +5,7 @@ mod parser; mod permission_set; mod permissions; -pub use coverage::covers; +pub use coverage::{covers, narrow}; pub use definitions::{ SCOPE_DEFINITIONS, ScopeCategory, ScopeDefinition, format_scope_for_display, get_required_scopes, get_scope_definition, is_valid_scope, diff --git a/crates/tranquil-scopes/src/parser.rs b/crates/tranquil-scopes/src/parser.rs index 9193bf7..191dae1 100644 --- a/crates/tranquil-scopes/src/parser.rs +++ b/crates/tranquil-scopes/src/parser.rs @@ -28,7 +28,22 @@ pub struct RepoScope { pub actions: HashSet, } -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Serialize, Deserialize)] +impl RepoScope { + pub fn to_scope_string(&self) -> String { + let mut actions: Vec = self.actions.iter().copied().collect(); + actions.sort(); + + let rendered: Vec<&str> = actions.iter().map(RepoAction::as_str).collect(); + + format!( + "repo:{}?action={}", + self.collection.as_deref().unwrap_or("*"), + rendered.join("&action=") + ) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash, Serialize, Deserialize)] #[serde(rename_all = "lowercase")] pub enum RepoAction { Create, @@ -37,6 +52,8 @@ pub enum RepoAction { } impl RepoAction { + pub const ALL: [RepoAction; 3] = [Self::Create, Self::Update, Self::Delete]; + pub fn parse_str(s: &str) -> Option { match s { "create" => Some(Self::Create), @@ -45,6 +62,14 @@ impl RepoAction { _ => None, } } + + pub fn as_str(&self) -> &'static str { + match self { + Self::Create => "create", + Self::Update => "update", + Self::Delete => "delete", + } + } } #[derive(Debug, Clone, PartialEq, Eq)] @@ -150,6 +175,14 @@ fn parse_query_params(query: &str) -> HashMap> { }) } +fn parse_repo_actions(params: &HashMap>) -> Option> { + match params.get("action") { + None => Some(RepoAction::ALL.into_iter().collect()), + Some(values) if values.is_empty() => None, + Some(values) => values.iter().map(|s| RepoAction::parse_str(s)).collect(), + } +} + pub fn parse_scope(scope: &str) -> ParsedScope { match scope { "atproto" => return ParsedScope::Atproto, @@ -169,20 +202,9 @@ pub fn parse_scope(scope: &str) -> ParsedScope { Some(rest.to_string()) }; - let actions: HashSet = params - .get("action") - .map(|action_values| { - action_values - .iter() - .filter_map(|s| RepoAction::parse_str(s)) - .collect() - }) - .filter(|set: &HashSet| !set.is_empty()) - .unwrap_or_else(|| { - [RepoAction::Create, RepoAction::Update, RepoAction::Delete] - .into_iter() - .collect() - }); + let Some(actions) = parse_repo_actions(¶ms) else { + return ParsedScope::Unknown(scope.to_string()); + }; return ParsedScope::Repo(RepoScope { collection, @@ -191,20 +213,10 @@ pub fn parse_scope(scope: &str) -> ParsedScope { } if base == "repo" { - let actions: HashSet = params - .get("action") - .map(|action_values| { - action_values - .iter() - .filter_map(|s| RepoAction::parse_str(s)) - .collect() - }) - .filter(|set: &HashSet| !set.is_empty()) - .unwrap_or_else(|| { - [RepoAction::Create, RepoAction::Update, RepoAction::Delete] - .into_iter() - .collect() - }); + let Some(actions) = parse_repo_actions(¶ms) else { + return ParsedScope::Unknown(scope.to_string()); + }; + return ParsedScope::Repo(RepoScope { collection: None, actions, @@ -340,6 +352,22 @@ mod tests { } } + #[test] + fn test_parse_repo_unrecognized_action_is_not_a_repo_scope() { + assert!(matches!( + parse_scope("repo:app.bsky.feed.post?action=read"), + ParsedScope::Unknown(_) + )); + assert!(matches!( + parse_scope("repo:app.bsky.feed.post?action="), + ParsedScope::Unknown(_) + )); + assert!(matches!( + parse_scope("repo?action=read"), + ParsedScope::Unknown(_) + )); + } + #[test] fn test_parse_blob_wildcard() { let scope = parse_scope("blob:*/*"); diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index ec7d14b..a5c07a5 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -1,9 +1,10 @@ +use crate::parser::RepoAction; use hickory_resolver::TokioAsyncResolver; use hickory_resolver::config::{ResolverConfig, ResolverOpts}; use reqwest::Client; use serde::{Deserialize, Serialize}; -use std::collections::{BTreeMap, HashMap}; -use tracing::debug; +use std::collections::{BTreeMap, BTreeSet, HashMap}; +use tracing::{debug, warn}; use tranquil_types::{Did, Nsid}; #[derive(Debug, thiserror::Error)] @@ -332,13 +333,23 @@ fn is_under_authority(target_nsid: &str, authority: &str) -> bool { .is_some_and(|c| c == '.') } -const DEFAULT_ACTIONS: &[&str] = &["create", "update", "delete"]; - -fn action_rank(action: &str) -> usize { - DEFAULT_ACTIONS - .iter() - .position(|known| *known == action) - .unwrap_or(DEFAULT_ACTIONS.len()) +fn parse_permission_actions(actions: Option<&Vec>) -> Option> { + match actions { + None => Some(RepoAction::ALL.into_iter().collect()), + Some(values) => values + .iter() + .map(|value| { + let parsed = RepoAction::parse_str(value); + if parsed.is_none() { + warn!( + action = %value, + "skipping permission entry with unrecognized repo action" + ); + } + parsed + }) + .collect(), + } } fn build_expanded_scopes( @@ -346,36 +357,26 @@ fn build_expanded_scopes( default_aud: Option<&str>, namespace_authority: &str, ) -> String { - // Key is `repo`, value is array of actions - let mut ungrouped_repo_scopes: BTreeMap> = BTreeMap::new(); + let mut ungrouped_repo_scopes: BTreeMap> = BTreeMap::new(); let mut rpc_scopes: Vec = Vec::new(); permissions .iter() .for_each(|perm| match perm.resource.as_str() { "repo" => { - if let Some(collections) = &perm.collection { - let actions: Vec<&str> = perm - .action - .as_ref() - .map(|a| a.iter().map(String::as_str).collect()) - .unwrap_or_else(|| DEFAULT_ACTIONS.to_vec()); - - if !actions.is_empty() { - collections - .iter() - .filter(|coll| is_under_authority(coll, namespace_authority)) - .for_each(|coll| { - let existing = - ungrouped_repo_scopes.entry(coll.to_string()).or_default(); - - actions.iter().for_each(|action| { - if !existing.iter().any(|seen| seen == action) { - existing.push(action.to_string()); - } - }); - }); - } + if let Some(collections) = &perm.collection + && let Some(actions) = parse_permission_actions(perm.action.as_ref()) + && !actions.is_empty() + { + collections + .iter() + .filter(|coll| is_under_authority(coll, namespace_authority)) + .for_each(|coll| { + ungrouped_repo_scopes + .entry(coll.to_string()) + .or_default() + .extend(actions.iter().copied()); + }); } } "rpc" => { @@ -402,10 +403,9 @@ fn build_expanded_scopes( let grouped_repo_scopes: Vec = ungrouped_repo_scopes .iter() .map(|(repo, actions)| { - let mut actions = actions.clone(); - actions.sort_by(|a, b| action_rank(a).cmp(&action_rank(b)).then_with(|| a.cmp(b))); + let rendered: Vec<&str> = actions.iter().map(RepoAction::as_str).collect(); - format!("repo:{}?action={}", repo, actions.join("&action=")) + format!("repo:{}?action={}", repo, rendered.join("&action=")) }) .collect(); @@ -628,6 +628,23 @@ mod tests { ); } + #[test] + fn test_build_expanded_scopes_repo_unrecognized_action_skips_entry() { + let permissions = vec![PermissionEntry { + resource: "repo".to_string(), + action: Some(vec!["read".to_string()]), + collection: Some(vec!["io.atcr.manifest".to_string()]), + lxm: None, + aud: None, + }]; + + let expanded = build_expanded_scopes(&permissions, None, "io.atcr"); + assert!( + expanded.is_empty(), + "an unrecognized repo action must not expand to all actions, got: {expanded}" + ); + } + #[test] fn test_build_expanded_scopes_rpc() { let permissions = vec![PermissionEntry { -- 2.51.2 From ce2f05b9d4c78c0c7fe130af7e36dce345c532de Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Thu, 13 Aug 2026 12:53:25 +0200 Subject: [PATCH 9/9] fix: make coverage triple state instead of boolean --- .../src/endpoints/authorize/consent.rs | 32 +++-- crates/tranquil-pds/src/delegation/mod.rs | 4 +- crates/tranquil-pds/src/delegation/scopes.rs | 120 +++++++++--------- .../tests/oauth_permission_sets.rs | 81 ++++++++++++ crates/tranquil-scopes/src/coverage.rs | 80 ++++++++++-- crates/tranquil-scopes/src/lib.rs | 2 +- frontend/src/routes/OAuthConsent.svelte | 7 +- 7 files changed, 242 insertions(+), 84 deletions(-) diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs index dae4d86..5743817 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs @@ -10,6 +10,8 @@ pub struct ScopeInfo { pub display_name: String, pub granted: Option, pub restricted: bool, + #[serde(skip_serializing_if = "Option::is_none")] + pub effective_scope: Option, } #[derive(Debug, Serialize)] @@ -186,20 +188,29 @@ pub async fn consent_get( 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_permits(g, scope)) + let coverage_of = |scope: &str| -> tranquil_pds::delegation::GrantCoverage { + match grant_scope_str { + Some(g) => tranquil_pds::delegation::grant_coverage(g, scope), + None => tranquil_pds::delegation::GrantCoverage::Full, + } }; let make_scope_info = |scope: &str| -> ScopeInfo { + let (restricted, effective_scope) = match coverage_of(scope) { + tranquil_pds::delegation::GrantCoverage::Full => (false, None), + tranquil_pds::delegation::GrantCoverage::Narrowed(narrowed) => (false, Some(narrowed)), + tranquil_pds::delegation::GrantCoverage::Withheld => (true, None), + }; + let described = effective_scope.as_deref().unwrap_or(scope); let (category, required, description, display_name) = - if let Some(def) = tranquil_pds::oauth::scopes::SCOPE_DEFINITIONS.get(scope) { - let desc = if scope == "atproto" && has_granular_scopes { + if let Some(def) = tranquil_pds::oauth::scopes::SCOPE_DEFINITIONS.get(described) { + let desc = if described == "atproto" && has_granular_scopes { "AT Protocol baseline scope (permissions determined by selected options below)" .to_string() } else { def.description.to_string() }; - let name = if scope == "atproto" && has_granular_scopes { + let name = if described == "atproto" && has_granular_scopes { "AT Protocol Access".to_string() } else { def.display_name.to_string() @@ -210,19 +221,19 @@ pub async fn consent_get( desc, name, ) - } else if scope.starts_with("ref:") { + } else if described.starts_with("ref:") { ( "Reference".to_string(), false, "Referenced scope".to_string(), - scope.to_string(), + described.to_string(), ) } else { ( "Other".to_string(), false, - format!("Access to {}", scope), - scope.to_string(), + format!("Access to {}", described), + described.to_string(), ) }; let granted = pref_map.get(scope).copied(); @@ -233,7 +244,8 @@ pub async fn consent_get( description, display_name, granted, - restricted: is_restricted(scope), + restricted, + effective_scope, } }; diff --git a/crates/tranquil-pds/src/delegation/mod.rs b/crates/tranquil-pds/src/delegation/mod.rs index 928fd2b..e793f8d 100644 --- a/crates/tranquil-pds/src/delegation/mod.rs +++ b/crates/tranquil-pds/src/delegation/mod.rs @@ -5,8 +5,8 @@ pub use roles::{ CanAddControllers, CanControlAccounts, verify_can_add_controllers, verify_can_control_accounts, }; pub use scopes::{ - EDITOR_FULL_SCOPES, InvalidDelegationScopeError, OWNER_FULL_SCOPES, SCOPE_PRESETS, ScopePreset, - ValidatedDelegationScope, grant_permits, intersect_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 3aa007c..008d33a 100644 --- a/crates/tranquil-pds/src/delegation/scopes.rs +++ b/crates/tranquil-pds/src/delegation/scopes.rs @@ -1,6 +1,6 @@ use std::collections::BTreeSet; -use tranquil_scopes::{ParsedScope, narrow, parse_scope}; +use tranquil_scopes::{Coverage, ParsedScope, coverage, parse_scope}; pub use tranquil_db_traits::{ DbScope as ValidatedDelegationScope, InvalidScopeError as InvalidDelegationScopeError, @@ -46,43 +46,53 @@ pub const SCOPE_PRESETS: &[ScopePreset] = &[ }, ]; +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum GrantCoverage { + Full, + Narrowed(String), + Withheld, +} + +fn scope_coverage(granted: &[ParsedScope], scope: &str) -> GrantCoverage { + if scope == "atproto" { + return GrantCoverage::Full; + } + + match coverage(granted, &parse_scope(scope)) { + Coverage::Full => GrantCoverage::Full, + Coverage::Narrowed(ParsedScope::Repo(repo)) => { + GrantCoverage::Narrowed(repo.to_scope_string()) + } + Coverage::Narrowed(_) => GrantCoverage::Full, + Coverage::Withheld => GrantCoverage::Withheld, + } +} + +pub fn grant_coverage(granted: &str, scope: &str) -> GrantCoverage { + scope_coverage(&parse_grant(granted), scope) +} + +fn parse_grant(granted: &str) -> Vec { + granted.split_whitespace().map(parse_scope).collect() +} + pub fn intersect_scopes(requested: &str, granted: &str) -> String { - let granted_parsed: Vec = granted.split_whitespace().map(parse_scope).collect(); + let granted_parsed = parse_grant(granted); let scopes: BTreeSet = requested .split_whitespace() - .filter_map(|requested_scope| { - if requested_scope == "atproto" { - return Some(requested_scope.to_string()); - } - - let requested_parsed = parse_scope(requested_scope); - let narrowed = narrow(&granted_parsed, &requested_parsed)?; - - if narrowed == requested_parsed { - return Some(requested_scope.to_string()); - } - - match &narrowed { - ParsedScope::Repo(repo) => Some(repo.to_scope_string()), - _ => Some(requested_scope.to_string()), - } - }) + .filter_map( + |requested_scope| match scope_coverage(&granted_parsed, requested_scope) { + GrantCoverage::Full => Some(requested_scope.to_string()), + GrantCoverage::Narrowed(narrowed) => Some(narrowed), + GrantCoverage::Withheld => None, + }, + ) .collect(); scopes.into_iter().collect::>().join(" ") } -pub fn grant_permits(granted: &str, scope: &str) -> bool { - if scope == "atproto" { - return true; - } - - let granted_parsed: Vec = granted.split_whitespace().map(parse_scope).collect(); - - narrow(&granted_parsed, &parse_scope(scope)).is_some() -} - #[cfg(test)] mod tests { use super::*; @@ -289,41 +299,35 @@ mod tests { } #[test] - fn test_grant_permits_matches_intersection() { + fn test_grant_coverage_full_and_withheld() { 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_eq!(grant_coverage(granted, "atproto"), GrantCoverage::Full); + assert_eq!( + grant_coverage(granted, "repo:app.bsky.feed.post?action=create"), + GrantCoverage::Full ); - assert!(grant_permits( - granted, - "repo:app.bsky.feed.post?action=create" - )); - assert!(grant_permits(granted, "account:*?action=manage")); - assert!(!grant_permits(granted, "identity:*")); assert_eq!( - grant_permits(granted, "identity:*"), - intersected.contains("identity") + grant_coverage(granted, "identity:*"), + GrantCoverage::Withheld ); + assert_eq!(grant_coverage("", "identity:*"), GrantCoverage::Withheld); } #[test] - fn test_grant_permits_atproto_always_true() { - assert!(grant_permits("", "atproto")); - assert!(grant_permits("repo:*", "atproto")); - } - - #[test] - fn test_grant_permits_empty_grant_permits_nothing_else() { - assert!(!grant_permits("", "repo:app.bsky.feed.post?action=create")); - assert!(!grant_permits("", "identity:*")); - } - - #[test] - fn test_grant_permits_partially_granted_repo_scope() { - assert!(grant_permits( - EDITOR_FULL_SCOPES, - "repo:io.atcr.manifest?action=create&action=delete" - )); + fn test_grant_coverage_narrowed_when_grant_is_a_strict_action_subset() { + assert_eq!( + grant_coverage( + EDITOR_FULL_SCOPES, + "repo:io.atcr.manifest?action=create&action=delete" + ), + GrantCoverage::Full + ); + assert_eq!( + grant_coverage( + "atproto repo:*?action=create blob:*/*", + "repo:io.atcr.manifest?action=create&action=delete" + ), + GrantCoverage::Narrowed("repo:io.atcr.manifest?action=create".to_string()) + ); } } diff --git a/crates/tranquil-pds/tests/oauth_permission_sets.rs b/crates/tranquil-pds/tests/oauth_permission_sets.rs index ad4ba22..5df9999 100644 --- a/crates/tranquil-pds/tests/oauth_permission_sets.rs +++ b/crates/tranquil-pds/tests/oauth_permission_sets.rs @@ -18,6 +18,10 @@ const PERMISSION_SET_GRANULAR_SCOPE: &str = const PERMISSION_SET_MULTI_ACTION_SCOPE: &str = "repo:io.atcr.manifest?action=create&action=update&action=delete"; const EDITOR_SET_NSID: &str = "io.atcr.authEditorApp"; +const SUBSET_SET_NSID: &str = "io.atcr.authSubsetApp"; +const PERMISSION_SET_CREATE_DELETE_SCOPE: &str = + "repo:io.atcr.manifest?action=create&action=delete"; +const CREATE_ONLY_GRANT: &str = "atproto repo:*?action=create blob:*/*"; fn disable_rate_limiting_once() { static ONCE: std::sync::Once = std::sync::Once::new(); @@ -441,6 +445,83 @@ async fn test_delegated_editor_grant_keeps_collapsed_permission_set() { ); } +#[tokio::test] +async fn test_delegated_consent_shows_the_scope_the_token_will_carry() { + seed_permission_set(SUBSET_SET_NSID, PERMISSION_SET_CREATE_DELETE_SCOPE).await; + + let scope = format!("atproto include:{}", SUBSET_SET_NSID); + let (session, consent_body, _mock) = create_delegated_session_with_grant( + "pss", + "https://example.com/permset-subset-callback", + &scope, + CREATE_ONLY_GRANT, + ) + .await; + + let set_entry = consent_body["permission_sets"] + .as_array() + .expect("consent response should have a permission_sets array") + .iter() + .find(|s| s["nsid"].as_str() == Some(SUBSET_SET_NSID)) + .unwrap_or_else(|| { + panic!( + "permission_sets should contain an entry for nsid '{}'. Got: {:?}", + SUBSET_SET_NSID, consent_body + ) + }); + assert_eq!( + set_entry["restricted"].as_bool(), + Some(false), + "the create action is still granted, so the set stays approvable. Got: {:?}", + set_entry + ); + + let repo = set_entry["expanded"] + .as_array() + .expect("permission_sets entry should have an expanded array") + .iter() + .find(|s| s["scope"].as_str() == Some(PERMISSION_SET_CREATE_DELETE_SCOPE)) + .unwrap_or_else(|| { + panic!( + "expanded[] should list the requested scope '{}'. Got: {:?}", + PERMISSION_SET_CREATE_DELETE_SCOPE, set_entry + ) + }); + assert_eq!( + repo["restricted"].as_bool(), + Some(false), + "a partially-covered scope is neither fully granted nor withheld. Got: {:?}", + repo + ); + let effective_scope = repo["effective_scope"].as_str().unwrap_or_else(|| { + panic!( + "a scope the grant narrows must report the actions it actually confers. Got: {:?}", + repo + ) + }); + assert_eq!(effective_scope, "repo:io.atcr.manifest?action=create"); + + let payload = decode_jwt_payload(&session.access_token); + let jwt_scope = tranquil_pds::auth::decode_scope( + payload["scope"] + .as_str() + .expect("access token JWT should have a scope claim"), + ) + .expect("JWT scope claim should decode"); + assert!( + jwt_scope.split_whitespace().any(|s| s == effective_scope), + "the consent screen must show the scope the token carries, expected '{}' in decoded \ + scope, got: {}", + effective_scope, + jwt_scope + ); + assert!( + !jwt_scope.contains("action=delete"), + "the grant confers no delete action, so the token must not carry one, got: {}", + jwt_scope + ); +} + #[tokio::test] async fn test_grant_row_keeps_include_jwt_carries_expanded() { seed_permission_set(PERMISSION_SET_NSID, PERMISSION_SET_GRANULAR_SCOPE).await; diff --git a/crates/tranquil-scopes/src/coverage.rs b/crates/tranquil-scopes/src/coverage.rs index 54dcbec..ea9e50e 100644 --- a/crates/tranquil-scopes/src/coverage.rs +++ b/crates/tranquil-scopes/src/coverage.rs @@ -41,7 +41,14 @@ fn repo_covers(g: &RepoScope, r: &RepoScope) -> bool { repo_collection_covers(g, r) && r.actions.is_subset(&g.actions) } -pub fn narrow(granted: &[ParsedScope], requested: &ParsedScope) -> Option { +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Coverage { + Full, + Narrowed(ParsedScope), + Withheld, +} + +pub fn coverage(granted: &[ParsedScope], requested: &ParsedScope) -> Coverage { if let ParsedScope::Repo(r) = requested { let actions: HashSet = granted .iter() @@ -52,18 +59,29 @@ pub fn narrow(granted: &[ParsedScope], requested: &ParsedScope) -> Option Coverage::Withheld, + _ if actions == r.actions => Coverage::Full, + _ => Coverage::Narrowed(ParsedScope::Repo(RepoScope { collection: r.collection.clone(), actions, - }) - }); + })), + }; } - granted - .iter() - .any(|g| covers(g, requested)) - .then(|| requested.clone()) + if granted.iter().any(|g| covers(g, requested)) { + Coverage::Full + } else { + Coverage::Withheld + } +} + +pub fn narrow(granted: &[ParsedScope], requested: &ParsedScope) -> Option { + match coverage(granted, requested) { + Coverage::Full => Some(requested.clone()), + Coverage::Narrowed(scope) => Some(scope), + Coverage::Withheld => None, + } } fn blob_covers(g: &BlobScope, r: &BlobScope) -> bool { @@ -103,7 +121,7 @@ fn identity_covers(g: &IdentityScope, r: &IdentityScope) -> bool { #[cfg(test)] mod tests { - use super::{covers, narrow}; + use super::{Coverage, coverage, covers, narrow}; use crate::parser::{ParsedScope, parse_scope}; fn c(granted: &str, requested: &str) -> bool { @@ -120,6 +138,16 @@ mod tests { } } + fn covered(granted: &str, requested: &str) -> Coverage { + let granted: Vec = granted.split_whitespace().map(parse_scope).collect(); + + coverage(&granted, &parse_scope(requested)) + } + + fn narrowed_to(scope: &str) -> Coverage { + Coverage::Narrowed(parse_scope(scope)) + } + #[test] fn repo_wildcard_covers_specific() { assert!(c("repo:*", "repo:app.bsky.feed.post")); @@ -262,4 +290,36 @@ mod tests { Some("identity:handle".to_string()) ); } + + #[test] + fn coverage_distinguishes_full_from_narrowed_repo_actions() { + assert_eq!( + covered( + "repo:*?action=create repo:*?action=update repo:*?action=delete", + "repo:io.atcr.manifest?action=create&action=delete" + ), + Coverage::Full + ); + assert_eq!( + covered( + "repo:*?action=create", + "repo:io.atcr.manifest?action=create&action=delete" + ), + narrowed_to("repo:io.atcr.manifest?action=create") + ); + assert_eq!( + covered( + "repo:*?action=create&action=delete", + "repo:io.atcr.manifest" + ), + narrowed_to("repo:io.atcr.manifest?action=create&action=delete") + ); + assert_eq!( + covered( + "repo:*?action=create", + "repo:io.atcr.manifest?action=delete" + ), + Coverage::Withheld + ); + } } diff --git a/crates/tranquil-scopes/src/lib.rs b/crates/tranquil-scopes/src/lib.rs index e7b6861..1a124e5 100644 --- a/crates/tranquil-scopes/src/lib.rs +++ b/crates/tranquil-scopes/src/lib.rs @@ -5,7 +5,7 @@ mod parser; mod permission_set; mod permissions; -pub use coverage::{covers, narrow}; +pub use coverage::{Coverage, coverage, covers, narrow}; pub use definitions::{ SCOPE_DEFINITIONS, ScopeCategory, ScopeDefinition, format_scope_for_display, get_required_scopes, get_scope_definition, is_valid_scope, diff --git a/frontend/src/routes/OAuthConsent.svelte b/frontend/src/routes/OAuthConsent.svelte index 30693cd..0860894 100644 --- a/frontend/src/routes/OAuthConsent.svelte +++ b/frontend/src/routes/OAuthConsent.svelte @@ -10,6 +10,7 @@ display_name: string granted: boolean | null restricted?: boolean + effective_scope?: string } const SCOPE_LOCALE_MAP: Record = { @@ -320,7 +321,7 @@ ) function getLocalizedScopeName(scope: ScopeInfo): string { - const localeKey = SCOPE_LOCALE_MAP[scope.scope] + const localeKey = SCOPE_LOCALE_MAP[scope.effective_scope ?? scope.scope] if (!localeKey) return scope.display_name if (scope.scope === 'atproto' && hasGranularScopes) { @@ -333,7 +334,7 @@ } function getLocalizedScopeDescription(scope: ScopeInfo): string { - const localeKey = SCOPE_LOCALE_MAP[scope.scope] + const localeKey = SCOPE_LOCALE_MAP[scope.effective_scope ?? scope.scope] if (!localeKey) return scope.description if (scope.scope === 'atproto' && hasGranularScopes) { @@ -355,7 +356,7 @@ const rpc: string[] = [] const other: ScopeInfo[] = [] for (const s of expanded) { - const [base, query = ''] = s.scope.split('?') + const [base, query = ''] = (s.effective_scope ?? s.scope).split('?') const params = new URLSearchParams(query) if (base.startsWith('repo:')) { const collection = base.slice('repo:'.length) || '*'