diff --git a/src/config.rs b/src/config.rs index 72472fd..5c680a9 100644 --- a/src/config.rs +++ b/src/config.rs @@ -8,11 +8,9 @@ use clap::Parser; use crate::atproto::lexicon::{ModListing, ModRelease, Record}; -// https://atproto.com/guides/permission-requests -// -// Widening this invalidates the tokens every signed-in user already holds: -// their PDS will refuse writes to a collection their session predates, and -// they have to sign in again to get it. +/// https://atproto.com/guides/permission-requests +/// +/// Changing this invalidates every session. pub static OAUTH_SCOPE: LazyLock = LazyLock::new(|| { format!( "atproto repo:{} repo:{} blob:*/*", diff --git a/src/oauth/callback.rs b/src/oauth/callback.rs index f2dca77..348fffb 100644 --- a/src/oauth/callback.rs +++ b/src/oauth/callback.rs @@ -13,6 +13,7 @@ use chrono::{Duration, Utc}; use serde::Deserialize; use crate::atproto::id::{Did, Handle}; +use crate::config::OAUTH_SCOPE; use crate::error::AppError; use crate::oauth::session::{ build_identity_cookie_header, build_session_cookie_header, encode_identity_cookie, @@ -199,6 +200,7 @@ pub async fn handle_get_callback( expires_at, dpop_private_key: persisted.dpop_private_key.clone(), pds_endpoint: pds_endpoint.clone(), + scope: OAUTH_SCOPE.clone(), }; let hint_verified = document.as_ref().is_some_and(|d| d.id == did.as_str()); @@ -216,6 +218,7 @@ pub async fn handle_get_callback( did: did.clone(), handle, pds_url: Some(pds_endpoint), + scope: OAUTH_SCOPE.clone(), }; let session_value = encode_session_cookie(&state.secrets.cookie_secret, &session) diff --git a/src/oauth/refresh.rs b/src/oauth/refresh.rs index 7fe6871..4518a27 100644 --- a/src/oauth/refresh.rs +++ b/src/oauth/refresh.rs @@ -119,6 +119,9 @@ pub async fn try_refresh_session( expires_at, dpop_private_key: session.dpop_private_key.clone(), pds_endpoint: session.pds_endpoint.clone(), + // Carried over, not re-stamped: a refresh returns a token with the + // same grant, so this stays what the user authorized. + scope: session.scope.clone(), }; let encoded = encode_session_cookie(&state.secrets.cookie_secret, &new_session) diff --git a/src/oauth/session.rs b/src/oauth/session.rs index c0f2239..8fe5224 100644 --- a/src/oauth/session.rs +++ b/src/oauth/session.rs @@ -17,6 +17,7 @@ use rand::RngCore as _; use serde::{Deserialize, Serialize}; use crate::atproto::id::{Did, Handle}; +use crate::config::OAUTH_SCOPE; use crate::error::AppError; use crate::state::AppState; @@ -40,6 +41,10 @@ pub struct SessionCookie { pub dpop_private_key: String, /// The user's PDS base URL, for authenticated writes. pub pds_endpoint: String, + /// The OAuth scope these tokens were granted under. Empty for a cookie + /// written before the field existed, which no current scope matches. + #[serde(default)] + pub scope: String, } /// Extracts the session cookie, rejecting with 401 if there isn't one. @@ -85,6 +90,10 @@ pub struct IdentityCookie { pub handle: Option, /// The user's PDS URL, if resolved. pub pds_url: Option, + /// The scope of the session this describes, repeated here because `Page` + /// has no cookie secret to decrypt that one with. + #[serde(default)] + pub scope: String, } impl IdentityCookie { @@ -286,14 +295,16 @@ pub fn extract_cookie_value<'a>(cookie_header: &'a str, name: &str) -> Option<&' pub fn get_session_from_headers(secret: &[u8; 32], headers: &HeaderMap) -> Option { let cookie_header = headers.get(header::COOKIE)?.to_str().ok()?; let session_value = extract_cookie_value(cookie_header, SESSION_COOKIE_NAME)?; - decode_session_cookie(secret, session_value).ok() + let session = decode_session_cookie(secret, session_value).ok()?; + (session.scope == *OAUTH_SCOPE).then_some(session) } /// Extract and decode the identity cookie from request headers. pub fn get_identity_from_headers(headers: &HeaderMap) -> Option { let cookie_header = headers.get(header::COOKIE)?.to_str().ok()?; let identity_value = extract_cookie_value(cookie_header, IDENTITY_COOKIE_NAME)?; - decode_identity_cookie(identity_value).ok() + let identity = decode_identity_cookie(identity_value).ok()?; + (identity.scope == *OAUTH_SCOPE).then_some(identity) } #[cfg(test)] @@ -310,6 +321,7 @@ mod tests { expires_at: Utc::now() + Duration::minutes(10), dpop_private_key: "did:key:example".to_string(), pds_endpoint: "https://pds.example".to_string(), + scope: OAUTH_SCOPE.clone(), }; let encoded = encode_session_cookie(&secret, &session).unwrap(); @@ -332,6 +344,7 @@ mod tests { expires_at: Utc::now(), dpop_private_key: "did:key:example".to_string(), pds_endpoint: "https://pds.example".to_string(), + scope: OAUTH_SCOPE.clone(), }; let encoded = encode_session_cookie(&secret, &session).unwrap(); @@ -344,6 +357,7 @@ mod tests { did: Did::new("did:plc:example"), handle: Some(Handle::new("alice.test").unwrap()), pds_url: Some("https://pds.example".to_string()), + scope: OAUTH_SCOPE.clone(), }; let encoded = encode_identity_cookie(&identity).unwrap(); @@ -354,6 +368,63 @@ mod tests { assert_eq!(decoded.pds_url, identity.pds_url); } + /// Headers carrying `session`/`identity` cookies with the given scope. + fn headers_for_scope(secret: &[u8; 32], scope: &str) -> HeaderMap { + let session = SessionCookie { + did: Did::new("did:plc:example"), + access_token: "access-token".to_string(), + refresh_token: None, + expires_at: Utc::now() + Duration::minutes(10), + dpop_private_key: "did:key:example".to_string(), + pds_endpoint: "https://pds.example".to_string(), + scope: scope.to_string(), + }; + let identity = IdentityCookie { + did: Did::new("did:plc:example"), + handle: Some(Handle::new("alice.test").unwrap()), + pds_url: Some("https://pds.example".to_string()), + scope: scope.to_string(), + }; + + let value = format!( + "{SESSION_COOKIE_NAME}={}; {IDENTITY_COOKIE_NAME}={}", + encode_session_cookie(secret, &session).unwrap(), + encode_identity_cookie(&identity).unwrap(), + ); + let mut headers = HeaderMap::new(); + headers.insert(header::COOKIE, HeaderValue::from_str(&value).unwrap()); + headers + } + + #[test] + fn a_session_granted_under_the_current_scope_is_accepted() { + let secret = [3u8; 32]; + let headers = headers_for_scope(&secret, &OAUTH_SCOPE); + + assert!(get_session_from_headers(&secret, &headers).is_some()); + assert!(get_identity_from_headers(&headers).is_some()); + } + + /// Widening the scope logs everyone out, even though their tokens still + /// decrypt and have not expired. + #[test] + fn a_session_granted_under_a_narrower_scope_is_refused() { + let secret = [3u8; 32]; + let headers = headers_for_scope(&secret, "atproto repo:dev.starhaven.mod.listing"); + + assert!(get_session_from_headers(&secret, &headers).is_none()); + assert!(get_identity_from_headers(&headers).is_none()); + } + + #[test] + fn a_session_from_before_the_scope_was_recorded_is_refused() { + let secret = [3u8; 32]; + let headers = headers_for_scope(&secret, ""); + + assert!(get_session_from_headers(&secret, &headers).is_none()); + assert!(get_identity_from_headers(&headers).is_none()); + } + #[test] fn extracts_named_cookie_from_header() { let header = "foo=bar; session=abc123; identity=xyz789";