diff --git a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs index 0f6a11c..b9ceea8 100644 --- a/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs +++ b/crates/tranquil-oauth-server/src/endpoints/authorize/consent.rs @@ -182,13 +182,7 @@ pub async fn consent_get( .iter() .map(|p| (p.scope.as_str(), p.granted)) .collect(); - let presented_item_strings: Vec = effective - .outcome - .passthrough - .iter() - .cloned() - .chain(effective.outcome.sets.iter().map(|g| g.include_token())) - .collect(); + let presented_item_strings = effective.outcome.unexpanded_scopes(); let show_consent = should_show_consent( state.repos.oauth.as_ref(), &did, @@ -504,13 +498,7 @@ pub async fn consent_post( ), ); } - let presented_items: Vec = effective - .outcome - .passthrough - .iter() - .cloned() - .chain(effective.outcome.sets.iter().map(|g| g.include_token())) - .collect(); + let presented_items = effective.outcome.unexpanded_scopes(); let atproto_was_requested = presented_items.iter().any(|s| s == "atproto"); if atproto_was_requested && !form.approved_scopes.contains(&"atproto".to_string()) { return json_error( diff --git a/crates/tranquil-oauth-server/src/endpoints/token/grants.rs b/crates/tranquil-oauth-server/src/endpoints/token/grants.rs index 9360c92..3e63e52 100644 --- a/crates/tranquil-oauth-server/src/endpoints/token/grants.rs +++ b/crates/tranquil-oauth-server/src/endpoints/token/grants.rs @@ -202,7 +202,10 @@ pub async fn handle_authorization_code_grant( details: None, code: None, current_refresh_token: Some(refresh_token.clone()), - scope: requested_scope.clone(), + // Filtered but unexpanded: a remembered consent skips the consent screen, so the raw + // request can still hold scopes the client no longer registers. Sets stay as `include:` + // tokens so refresh re-resolves them. + scope: Some(effective.outcome.unexpanded_scopes().join(" ")), controller_did: controller_did.clone(), }; state @@ -275,17 +278,13 @@ async fn recompute_resolved_scope( Some(g) => crate::endpoints::authorize::scope_resolution::Authority::Delegated(g), None => crate::endpoints::authorize::scope_resolution::Authority::FullSelf, }; - let client_scope = state - .client_metadata_cache - .get(&token_data.client_id) - .await - .ok() - .and_then(|m| m.scope); + // No client metadata check here: `token_data.scope` was already filtered against it when + // the token was issued, so there is nothing for a re-check to remove. let effective = crate::endpoints::authorize::scope_resolution::resolve_effective_scopes( &*state.cache, requested, authority, - client_scope.as_deref(), + None, ) .await; if !effective.outcome.failures.is_empty() { diff --git a/crates/tranquil-pds/tests/oauth_scopes.rs b/crates/tranquil-pds/tests/oauth_scopes.rs index 89e844c..7ede0bd 100644 --- a/crates/tranquil-pds/tests/oauth_scopes.rs +++ b/crates/tranquil-pds/tests/oauth_scopes.rs @@ -3,7 +3,7 @@ mod helpers; use base64::{Engine as _, engine::general_purpose::URL_SAFE_NO_PAD}; use chrono::Utc; -use common::{base_url, client}; +use common::{base_url, client, get_test_repos}; use helpers::verify_new_account; use reqwest::StatusCode; use serde_json::{Value, json}; @@ -1033,3 +1033,84 @@ async fn test_scope_missing_from_client_metadata_is_not_registered_on_consent() let granted = token["scope"].as_str().unwrap(); assert!(!has_scope(granted, "identity:*"), "granted {:?}", granted); } + +/// A remembered consent skips the consent screen, so the scope stored on the token must be +/// filtered at issuance rather than copied from the raw request. Otherwise a scope the client +/// has since dropped from its metadata survives in storage and comes back on refresh. +#[tokio::test] +async fn test_remembered_scope_later_unregistered_never_reaches_a_token() { + let pending = par_and_login( + "remember", + "atproto identity:*", + Some("atproto"), + async |did, client_id| { + let prefs = + ["atproto", "identity:*"].map(|scope| tranquil_pds::oauth::db::ScopePreference { + scope: scope.to_string(), + granted: true, + }); + get_test_repos() + .await + .oauth + .upsert_scope_preferences( + &did.parse().unwrap(), + &tranquil_types::ClientId::new(client_id.to_string()), + &prefs, + ) + .await + .expect("seeding scope preferences failed"); + }, + ) + .await; + assert!( + !pending.location.contains("/oauth/consent"), + "remembered consent should skip the consent screen, got {}", + pending.location + ); + + let token = exchange_code(&pending, &pending.location).await; + assert!(!has_scope(token["scope"].as_str().unwrap(), "identity:*")); + + let token_id = { + let payload = token["access_token"] + .as_str() + .unwrap() + .split('.') + .nth(1) + .unwrap(); + let claims: Value = + serde_json::from_slice(&URL_SAFE_NO_PAD.decode(payload).unwrap()).unwrap(); + tranquil_types::TokenId::new(claims["sid"].as_str().expect("sid claim")) + }; + let row = get_test_repos() + .await + .oauth + .get_token_by_id(&token_id) + .await + .expect("get_token_by_id query failed") + .expect("token row should exist"); + let row_scope = row.scope.expect("token row should have a scope"); + assert!( + !has_scope(&row_scope, "identity:*"), + "stored {:?}", + row_scope + ); + + let refresh_res = client() + .post(format!("{}/oauth/token", base_url().await)) + .form(&[ + ("grant_type", "refresh_token"), + ("refresh_token", token["refresh_token"].as_str().unwrap()), + ("client_id", &pending.client_id), + ]) + .send() + .await + .expect("Refresh request failed"); + assert_eq!(refresh_res.status(), StatusCode::OK); + let refreshed: Value = refresh_res.json().await.unwrap(); + assert!( + !has_scope(refreshed["scope"].as_str().unwrap(), "identity:*"), + "refresh granted {:?}", + refreshed["scope"] + ); +} diff --git a/crates/tranquil-scopes/src/permission_set.rs b/crates/tranquil-scopes/src/permission_set.rs index 5c530b4..743c361 100644 --- a/crates/tranquil-scopes/src/permission_set.rs +++ b/crates/tranquil-scopes/src/permission_set.rs @@ -110,6 +110,16 @@ impl ExpansionOutcome { pub fn to_scope_string(&self) -> String { self.flat_scopes().join(" ") } + + /// The scopes that survived filtering, as requested: passthrough scopes plus the `include:` + /// token of each resolved set, without expanding the sets. + pub fn unexpanded_scopes(&self) -> Vec { + self.passthrough + .iter() + .cloned() + .chain(self.sets.iter().map(ResolvedSetGroup::include_token)) + .collect() + } } #[derive(Debug, Deserialize)]