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 !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(); - if existing.is_none() { - ungrouped_repo_scopes - .insert(coll.to_string(), vec![action.to_string()]); - } else { - existing.unwrap().push(action.to_string()); - } + 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]