From 156066fe1b67b57ef7a8f7f44f2c249f83f71601 Mon Sep 17 00:00:00 2001 From: Trezy Date: Tue, 15 Sep 2026 14:37:23 -0500 Subject: [PATCH] fix: reject PAR requests missing the `atproto` scope Signed-off-by: Trezy --- .../src/endpoints/par.rs | 47 +++++++++++++++++++ crates/tranquil-pds/tests/oauth.rs | 13 ++--- crates/tranquil-pds/tests/oauth_scopes.rs | 29 ++++++++++++ 3 files changed, 83 insertions(+), 6 deletions(-) diff --git a/crates/tranquil-oauth-server/src/endpoints/par.rs b/crates/tranquil-oauth-server/src/endpoints/par.rs index f218d14..cf00666 100644 --- a/crates/tranquil-oauth-server/src/endpoints/par.rs +++ b/crates/tranquil-oauth-server/src/endpoints/par.rs @@ -173,6 +173,11 @@ fn normalize_scope(requested_scope: &Option) -> Result, O if requested_scopes.is_empty() { return Ok(Some("atproto".to_string())); } + if !requested_scopes.contains(&"atproto") { + return Err(OAuthError::InvalidScope( + "The atproto scope is required".to_string(), + )); + } Ok(Some(requested_scopes.join(" "))) } @@ -225,3 +230,45 @@ fn parse_prompt(value: Option<&str>) -> Result, OAuthError> { ))), } } + +#[cfg(test)] +mod tests { + use super::*; + + fn normalized(scope: Option<&str>) -> Result, OAuthError> { + normalize_scope(&scope.map(str::to_string)) + } + + #[test] + fn absent_or_blank_scope_defaults_to_atproto() { + assert_eq!(normalized(None).unwrap().as_deref(), Some("atproto")); + assert_eq!(normalized(Some("")).unwrap().as_deref(), Some("atproto")); + assert_eq!(normalized(Some(" ")).unwrap().as_deref(), Some("atproto")); + } + + #[test] + fn scope_without_atproto_is_invalid() { + assert!(matches!( + normalized(Some("repo:*?action=create blob:*/*")), + Err(OAuthError::InvalidScope(_)) + )); + } + + #[test] + fn atproto_need_not_come_first() { + assert_eq!( + normalized(Some("repo:*?action=create atproto")) + .unwrap() + .as_deref(), + Some("repo:*?action=create atproto") + ); + } + + #[test] + fn unrecognized_scopes_still_pass_par() { + assert_eq!( + normalized(Some("atproto chat")).unwrap().as_deref(), + Some("atproto chat") + ); + } +} diff --git a/crates/tranquil-pds/tests/oauth.rs b/crates/tranquil-pds/tests/oauth.rs index 8d9bcfd..8c179d3 100644 --- a/crates/tranquil-pds/tests/oauth.rs +++ b/crates/tranquil-pds/tests/oauth.rs @@ -1057,7 +1057,7 @@ async fn test_granular_scope_repo_create_only() { let url = base_url().await; let http_client = client(); let (token, did, _) = - get_oauth_token_with_scope("repo:app.bsky.feed.post?action=create blob:*/*").await; + get_oauth_token_with_scope("atproto repo:app.bsky.feed.post?action=create blob:*/*").await; let now = chrono::Utc::now().to_rfc3339(); let create_res = http_client .post(format!("{}/xrpc/com.atproto.repo.createRecord", url)) @@ -1111,7 +1111,7 @@ async fn test_granular_scope_wildcard_collection() { let url = base_url().await; let http_client = client(); let (token, did, _) = get_oauth_token_with_scope( - "repo:app.bsky.*?action=create&action=update&action=delete blob:*/*", + "atproto repo:app.bsky.*?action=create&action=update&action=delete blob:*/*", ) .await; let now = chrono::Utc::now().to_rfc3339(); @@ -1168,7 +1168,7 @@ async fn test_granular_scope_wildcard_collection() { async fn test_granular_scope_email_read() { let url = base_url().await; let http_client = client(); - let (token, did, _) = get_oauth_token_with_scope("account:email?action=read").await; + let (token, did, _) = get_oauth_token_with_scope("atproto account:email?action=read").await; let session_res = http_client .get(format!("{}/xrpc/com.atproto.server.getSession", url)) .bearer_auth(&token) @@ -1189,7 +1189,7 @@ async fn test_granular_scope_email_read() { async fn test_granular_scope_no_email_access() { let url = base_url().await; let http_client = client(); - let (token, did, _) = get_oauth_token_with_scope("repo:*?action=create blob:*/*").await; + let (token, did, _) = get_oauth_token_with_scope("atproto repo:*?action=create blob:*/*").await; let session_res = http_client .get(format!("{}/xrpc/com.atproto.server.getSession", url)) .bearer_auth(&token) @@ -1210,7 +1210,8 @@ async fn test_granular_scope_no_email_access() { async fn test_granular_scope_rpc_specific_method() { let url = base_url().await; let http_client = client(); - let (token, _, _) = get_oauth_token_with_scope("rpc:app.bsky.feed.getTimeline?aud=*").await; + let (token, _, _) = + get_oauth_token_with_scope("atproto rpc:app.bsky.feed.getTimeline?aud=*").await; let allowed_res = http_client .get(format!("{}/xrpc/com.atproto.server.getServiceAuth", url)) .bearer_auth(&token) @@ -1275,7 +1276,7 @@ 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#bsky_appview", + "atproto rpc:app.bsky.feed.getTimeline?aud=did:web:api.bsky.app#bsky_appview", ) .await; let allowed_res = http_client diff --git a/crates/tranquil-pds/tests/oauth_scopes.rs b/crates/tranquil-pds/tests/oauth_scopes.rs index e9d2d9a..89e844c 100644 --- a/crates/tranquil-pds/tests/oauth_scopes.rs +++ b/crates/tranquil-pds/tests/oauth_scopes.rs @@ -959,6 +959,35 @@ fn has_scope(scope_str: &str, scope: &str) -> bool { scope_str.split_whitespace().any(|s| s == scope) } +#[tokio::test] +async fn test_par_rejects_scope_without_atproto() { + let url = base_url().await; + let mock = setup_mock_client_metadata(REDIRECT_URI).await; + let client_id = mock.uri(); + let (_, code_challenge) = generate_pkce(); + let par_res = client() + .post(format!("{}/oauth/par", url)) + .form(&[ + ("response_type", "code"), + ("client_id", &client_id), + ("redirect_uri", REDIRECT_URI), + ("code_challenge", &code_challenge), + ("code_challenge_method", "S256"), + ("scope", "repo:*?action=create"), + ]) + .send() + .await + .expect("PAR failed"); + assert_eq!(par_res.status(), StatusCode::BAD_REQUEST); + let body: Value = par_res.json().await.unwrap(); + assert_eq!( + body["error"].as_str(), + Some("invalid_scope"), + "got {:?}", + body + ); +} + #[tokio::test] async fn test_scope_missing_from_client_metadata_is_not_registered_on_consent() { let pending = par_and_login(