fix: store filtered scopes on tokens so refresh can skip the client metadata check

Signed-off-by: Trezy <tre@trezy.com>

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<String> = 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<String> = 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(

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() {

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"]
+    );
+}

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<String> {
+        self.passthrough
+            .iter()
+            .cloned()
+            .chain(self.sets.iter().map(ResolvedSetGroup::include_token))
+            .collect()
+    }
 }
 
 #[derive(Debug, Deserialize)]