From dae3cc7e081c8bb5d73f53c4c2c29d1008694ed3 Mon Sep 17 00:00:00 2001 From: Louis Escher Date: Wed, 19 Aug 2026 21:19:20 +0200 Subject: [PATCH] fix: aud fragment matching --- crates/tranquil-pds/tests/oauth.rs | 47 ++++++++++++------- crates/tranquil-pds/tests/scope_edge_cases.rs | 32 +++++++++---- crates/tranquil-scopes/src/permissions.rs | 41 +++++++++++----- 3 files changed, 80 insertions(+), 40 deletions(-) diff --git a/crates/tranquil-pds/tests/oauth.rs b/crates/tranquil-pds/tests/oauth.rs index 2a05cd3..8d9bcfd 100644 --- a/crates/tranquil-pds/tests/oauth.rs +++ b/crates/tranquil-pds/tests/oauth.rs @@ -1274,8 +1274,10 @@ async fn test_granular_scope_rpc_specific_method() { async fn test_granular_scope_rpc_aud_with_service_id() { let url = base_url().await; let http_client = client(); - let (token, _, _) = - get_oauth_token_with_scope("rpc:app.bsky.feed.getTimeline?aud=did:web:api.bsky.app").await; + let (token, _, _) = get_oauth_token_with_scope( + "rpc:app.bsky.feed.getTimeline?aud=did:web:api.bsky.app#bsky_appview", + ) + .await; let allowed_res = http_client .get(format!("{}/xrpc/com.atproto.server.getServiceAuth", url)) .bearer_auth(&token) @@ -1289,7 +1291,7 @@ async fn test_granular_scope_rpc_aud_with_service_id() { assert_eq!( allowed_res.status(), StatusCode::OK, - "A scope granted for a service must cover a request naming one of its service ids" + "the granted service id must cover a request naming it" ); let body: Value = allowed_res.json().await.unwrap(); let service_token = body["token"].as_str().unwrap(); @@ -1299,21 +1301,30 @@ async fn test_granular_scope_rpc_aud_with_service_id() { claims["aud"], "did:web:api.bsky.app#bsky_appview", "the service id must reach the signed claim even on the granular scope path" ); - let blocked_res = http_client - .get(format!("{}/xrpc/com.atproto.server.getServiceAuth", url)) - .bearer_auth(&token) - .query(&[ - ("aud", "did:web:other.example#bsky_appview"), - ("lxm", "app.bsky.feed.getTimeline"), - ]) - .send() - .await - .unwrap(); - assert_eq!( - blocked_res.status(), - StatusCode::FORBIDDEN, - "A service id must not smuggle in a different audience" - ); + + for (aud, reason) in [ + ( + "did:web:api.bsky.app#atproto_labeler", + "a scope for the appview must not mint tokens for the labeler on the same DID", + ), + ( + "did:web:api.bsky.app", + "a scope for one service must not widen to the whole DID", + ), + ( + "did:web:other.example#bsky_appview", + "a service id must not smuggle in a different audience", + ), + ] { + let blocked_res = http_client + .get(format!("{}/xrpc/com.atproto.server.getServiceAuth", url)) + .bearer_auth(&token) + .query(&[("aud", aud), ("lxm", "app.bsky.feed.getTimeline")]) + .send() + .await + .unwrap(); + assert_eq!(blocked_res.status(), StatusCode::FORBIDDEN, "{reason}"); + } } #[tokio::test] diff --git a/crates/tranquil-pds/tests/scope_edge_cases.rs b/crates/tranquil-pds/tests/scope_edge_cases.rs index 9cffed4..74ac84f 100644 --- a/crates/tranquil-pds/tests/scope_edge_cases.rs +++ b/crates/tranquil-pds/tests/scope_edge_cases.rs @@ -182,19 +182,19 @@ fn test_permissions_rpc_lxm_wildcard_prefix() { } #[test] -fn test_permissions_rpc_aud_service_id_is_normalized() { +fn test_permissions_rpc_aud_service_id_must_match_verbatim() { let perms = ScopePermissions::from_scope_string(Some("rpc:app.bsky.feed.*?aud=did:web:api.bsky.app")); assert!( - perms.allows_rpc( + perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getTimeline")), + "the granted audience must cover itself" + ); + assert!( + !perms.allows_rpc( "did:web:api.bsky.app#bsky_appview", &c("app.bsky.feed.getTimeline") ), - "a scope granted for a service must cover a request naming one of its service ids" - ); - assert!( - perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getTimeline")), - "the bare form must keep working" + "a bare DID grants nothing to the services listed under it" ); assert!( !perms.allows_rpc( @@ -208,8 +208,22 @@ fn test_permissions_rpc_aud_service_id_is_normalized() { "rpc:app.bsky.feed.*?aud=did:web:api.bsky.app%23bsky_appview", )); assert!( - fragment_scope.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getTimeline")), - "a scope granted with a service id must still cover the bare audience" + fragment_scope.allows_rpc( + "did:web:api.bsky.app#bsky_appview", + &c("app.bsky.feed.getTimeline") + ), + "the granted service id must cover itself" + ); + assert!( + !fragment_scope.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getTimeline")), + "a scope granted for one service must not widen to the whole DID" + ); + assert!( + !fragment_scope.allows_rpc( + "did:web:api.bsky.app#atproto_labeler", + &c("app.bsky.feed.getTimeline") + ), + "the appview and the labeler are different audiences even on one DID" ); } diff --git a/crates/tranquil-scopes/src/permissions.rs b/crates/tranquil-scopes/src/permissions.rs index 519fa26..7fb4a74 100644 --- a/crates/tranquil-scopes/src/permissions.rs +++ b/crates/tranquil-scopes/src/permissions.rs @@ -177,8 +177,6 @@ impl ScopePermissions { return Ok(()); } - let aud_base = aud.split('#').next().unwrap_or(aud); - let has_permission = self.find_rpc_scopes().any(|rpc_scope| { let lxm_matches = match &rpc_scope.lxm { None => true, @@ -193,10 +191,7 @@ impl ScopePermissions { let aud_matches = match &rpc_scope.aud { None => true, Some(scope_aud) if scope_aud == "*" => true, - Some(scope_aud) => { - let scope_aud_base = scope_aud.split('#').next().unwrap_or(scope_aud); - scope_aud_base == aud_base - } + Some(scope_aud) => scope_aud == aud, }; lxm_matches && aud_matches @@ -558,27 +553,47 @@ mod tests { let perms = ScopePermissions::from_scope_string(Some( "rpc:app.bsky.feed.getAuthorFeed?aud=did:web:api.bsky.app#bsky_appview", )); - assert!(perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getAuthorFeed"))); assert!(perms.allows_rpc( "did:web:api.bsky.app#bsky_appview", &c("app.bsky.feed.getAuthorFeed") )); - assert!(perms.allows_rpc( - "did:web:api.bsky.app#other_service", - &c("app.bsky.feed.getAuthorFeed") - )); + assert!( + !perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getAuthorFeed")), + "a scope naming one service must not cover the whole DID" + ); + assert!( + !perms.allows_rpc( + "did:web:api.bsky.app#atproto_labeler", + &c("app.bsky.feed.getAuthorFeed") + ), + "a scope naming one service must not cover a sibling service on the same DID" + ); assert!(!perms.allows_rpc("did:web:other.app", &c("app.bsky.feed.getAuthorFeed"))); assert!(!perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getTimeline"))); } #[test] - fn test_rpc_scope_without_fragment_matches_with_fragment() { + fn test_rpc_scope_without_fragment_does_not_cover_service_ids() { let perms = ScopePermissions::from_scope_string(Some( "rpc:app.bsky.feed.getAuthorFeed?aud=did:web:api.bsky.app", )); assert!(perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getAuthorFeed"))); + assert!( + !perms.allows_rpc( + "did:web:api.bsky.app#bsky_appview", + &c("app.bsky.feed.getAuthorFeed") + ), + "the audience is compared verbatim, so a bare DID grants nothing to its services" + ); + } + + #[test] + fn test_rpc_scope_aud_wildcard_covers_service_ids() { + let perms = + ScopePermissions::from_scope_string(Some("rpc:app.bsky.feed.getAuthorFeed?aud=*")); + assert!(perms.allows_rpc("did:web:api.bsky.app", &c("app.bsky.feed.getAuthorFeed"))); assert!(perms.allows_rpc( - "did:web:api.bsky.app#bsky_appview", + "did:web:api.bsky.app#atproto_labeler", &c("app.bsky.feed.getAuthorFeed") )); }