From 8eb4b5516f07c091df26b15efc4a23854136094b Mon Sep 17 00:00:00 2001 From: lewis Date: Sat, 24 Jan 2026 12:36:43 +0200 Subject: [PATCH] fix: match ref pds permission-levels for some endpoints --- .../tranquil-pds/src/api/actor/preferences.rs | 6 +- crates/tranquil-pds/src/api/error.rs | 1 - .../src/api/identity/plc/request.rs | 4 +- .../tranquil-pds/src/api/identity/plc/sign.rs | 4 +- .../src/api/identity/plc/submit.rs | 4 +- .../src/api/server/account_status.rs | 8 +- .../src/api/server/app_password.rs | 8 +- crates/tranquil-pds/src/api/server/email.rs | 10 +- crates/tranquil-pds/src/api/server/invite.rs | 4 +- crates/tranquil-pds/src/api/server/session.rs | 4 +- crates/tranquil-pds/src/auth/extractor.rs | 103 ++++-------------- crates/tranquil-pds/src/auth/mod.rs | 3 +- crates/tranquil-pds/tests/actor.rs | 102 +++++++++++++++++ crates/tranquil-pds/tests/auth_extractor.rs | 65 +++++++++++ 14 files changed, 216 insertions(+), 110 deletions(-) diff --git a/crates/tranquil-pds/src/api/actor/preferences.rs b/crates/tranquil-pds/src/api/actor/preferences.rs index 3b8f345..ef705b7 100644 --- a/crates/tranquil-pds/src/api/actor/preferences.rs +++ b/crates/tranquil-pds/src/api/actor/preferences.rs @@ -1,5 +1,5 @@ use crate::api::error::ApiError; -use crate::auth::{Active, Auth}; +use crate::auth::{Auth, NotTakendown, Permissive}; use crate::state::AppState; use axum::{ Json, @@ -32,7 +32,7 @@ fn get_age_from_datestring(birth_date: &str) -> Option { pub struct GetPreferencesOutput { pub preferences: Vec, } -pub async fn get_preferences(State(state): State, auth: Auth) -> Response { +pub async fn get_preferences(State(state): State, auth: Auth) -> Response { let has_full_access = auth.permissions().has_full_access(); let user_id: uuid::Uuid = match state.user_repo.get_id_by_did(&auth.did).await { Ok(Some(id)) => id, @@ -89,7 +89,7 @@ pub struct PutPreferencesInput { } pub async fn put_preferences( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Response { let has_full_access = auth.permissions().has_full_access(); diff --git a/crates/tranquil-pds/src/api/error.rs b/crates/tranquil-pds/src/api/error.rs index 3afe821..09dda55 100644 --- a/crates/tranquil-pds/src/api/error.rs +++ b/crates/tranquil-pds/src/api/error.rs @@ -546,7 +546,6 @@ impl From for ApiError { crate::auth::extractor::AuthError::ServiceAuthNotAllowed => Self::AuthenticationFailed( Some("Service authentication not allowed for this endpoint".to_string()), ), - crate::auth::extractor::AuthError::SigningKeyRequired => Self::InvalidSigningKey, crate::auth::extractor::AuthError::InsufficientScope(msg) => { Self::InsufficientScope(Some(msg)) } diff --git a/crates/tranquil-pds/src/api/identity/plc/request.rs b/crates/tranquil-pds/src/api/identity/plc/request.rs index 9bba943..de5f71e 100644 --- a/crates/tranquil-pds/src/api/identity/plc/request.rs +++ b/crates/tranquil-pds/src/api/identity/plc/request.rs @@ -1,6 +1,6 @@ use crate::api::EmptyResponse; use crate::api::error::ApiError; -use crate::auth::{Auth, NotTakendown}; +use crate::auth::{Auth, Permissive}; use crate::state::AppState; use axum::{ extract::State, @@ -15,7 +15,7 @@ fn generate_plc_token() -> String { pub async fn request_plc_operation_signature( State(state): State, - auth: Auth, + auth: Auth, ) -> Result { if let Err(e) = crate::auth::scope_check::check_identity_scope( auth.is_oauth(), diff --git a/crates/tranquil-pds/src/api/identity/plc/sign.rs b/crates/tranquil-pds/src/api/identity/plc/sign.rs index ed13e89..3b97e79 100644 --- a/crates/tranquil-pds/src/api/identity/plc/sign.rs +++ b/crates/tranquil-pds/src/api/identity/plc/sign.rs @@ -1,5 +1,5 @@ use crate::api::ApiError; -use crate::auth::{Auth, NotTakendown}; +use crate::auth::{Auth, Permissive}; use crate::circuit_breaker::with_circuit_breaker; use crate::plc::{PlcClient, PlcError, PlcService, create_update_op, sign_operation}; use crate::state::AppState; @@ -40,7 +40,7 @@ pub struct SignPlcOperationOutput { pub async fn sign_plc_operation( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { if let Err(e) = crate::auth::scope_check::check_identity_scope( diff --git a/crates/tranquil-pds/src/api/identity/plc/submit.rs b/crates/tranquil-pds/src/api/identity/plc/submit.rs index 0d432fb..adf6ee8 100644 --- a/crates/tranquil-pds/src/api/identity/plc/submit.rs +++ b/crates/tranquil-pds/src/api/identity/plc/submit.rs @@ -1,5 +1,5 @@ use crate::api::{ApiError, EmptyResponse}; -use crate::auth::{Auth, NotTakendown}; +use crate::auth::{Auth, Permissive}; use crate::circuit_breaker::with_circuit_breaker; use crate::plc::{PlcClient, signing_key_to_did_key, validate_plc_operation}; use crate::state::AppState; @@ -20,7 +20,7 @@ pub struct SubmitPlcOperationInput { pub async fn submit_plc_operation( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { if let Err(e) = crate::auth::scope_check::check_identity_scope( diff --git a/crates/tranquil-pds/src/api/server/account_status.rs b/crates/tranquil-pds/src/api/server/account_status.rs index 98ea29c..75eca55 100644 --- a/crates/tranquil-pds/src/api/server/account_status.rs +++ b/crates/tranquil-pds/src/api/server/account_status.rs @@ -1,6 +1,6 @@ use crate::api::EmptyResponse; use crate::api::error::ApiError; -use crate::auth::{Active, Auth, NotTakendown}; +use crate::auth::{Auth, NotTakendown, Permissive}; use crate::cache::Cache; use crate::plc::PlcClient; use crate::state::AppState; @@ -41,7 +41,7 @@ pub struct CheckAccountStatusOutput { pub async fn check_account_status( State(state): State, - auth: Auth, + auth: Auth, ) -> Result { let did = &auth.did; let user_id = state @@ -306,7 +306,7 @@ async fn assert_valid_did_document_for_service( pub async fn activate_account( State(state): State, - auth: Auth, + auth: Auth, ) -> Result { info!("[MIGRATION] activateAccount called"); info!( @@ -470,7 +470,7 @@ pub struct DeactivateAccountInput { pub async fn deactivate_account( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { if let Err(e) = crate::auth::scope_check::check_account_scope( diff --git a/crates/tranquil-pds/src/api/server/app_password.rs b/crates/tranquil-pds/src/api/server/app_password.rs index a374ac8..a2a9010 100644 --- a/crates/tranquil-pds/src/api/server/app_password.rs +++ b/crates/tranquil-pds/src/api/server/app_password.rs @@ -1,6 +1,6 @@ use crate::api::EmptyResponse; use crate::api::error::ApiError; -use crate::auth::{Active, Auth, generate_app_password}; +use crate::auth::{Auth, NotTakendown, Permissive, generate_app_password}; use crate::delegation::{DelegationActionType, intersect_scopes}; use crate::state::{AppState, RateLimitKind}; use axum::{ @@ -33,7 +33,7 @@ pub struct ListAppPasswordsOutput { pub async fn list_app_passwords( State(state): State, - auth: Auth, + auth: Auth, ) -> Result { let user = state .user_repo @@ -90,7 +90,7 @@ pub struct CreateAppPasswordOutput { pub async fn create_app_password( State(state): State, headers: HeaderMap, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { let client_ip = crate::rate_limit::extract_client_ip(&headers, None); @@ -227,7 +227,7 @@ pub struct RevokeAppPasswordInput { pub async fn revoke_app_password( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { let user = state diff --git a/crates/tranquil-pds/src/api/server/email.rs b/crates/tranquil-pds/src/api/server/email.rs index 3403715..bf3f118 100644 --- a/crates/tranquil-pds/src/api/server/email.rs +++ b/crates/tranquil-pds/src/api/server/email.rs @@ -1,6 +1,6 @@ use crate::api::error::ApiError; use crate::api::{EmptyResponse, TokenRequiredResponse, VerifiedResponse}; -use crate::auth::{Active, Auth}; +use crate::auth::{Auth, NotTakendown}; use crate::state::{AppState, RateLimitKind}; use axum::{ Json, @@ -45,7 +45,7 @@ pub struct RequestEmailUpdateInput { pub async fn request_email_update( State(state): State, headers: axum::http::HeaderMap, - auth: Auth, + auth: Auth, input: Option>, ) -> Result { let client_ip = crate::rate_limit::extract_client_ip(&headers, None); @@ -140,7 +140,7 @@ pub struct ConfirmEmailInput { pub async fn confirm_email( State(state): State, headers: axum::http::HeaderMap, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { let client_ip = crate::rate_limit::extract_client_ip(&headers, None); @@ -233,7 +233,7 @@ pub struct UpdateEmailInput { pub async fn update_email( State(state): State, - auth: Auth, + auth: Auth, Json(input): Json, ) -> Result { if let Err(e) = crate::auth::scope_check::check_account_scope( @@ -500,7 +500,7 @@ pub async fn authorize_email_update( pub async fn check_email_update_status( State(state): State, headers: axum::http::HeaderMap, - auth: Auth, + auth: Auth, ) -> Result { let client_ip = crate::rate_limit::extract_client_ip(&headers, None); if !state diff --git a/crates/tranquil-pds/src/api/server/invite.rs b/crates/tranquil-pds/src/api/server/invite.rs index 8b00719..1ff6f42 100644 --- a/crates/tranquil-pds/src/api/server/invite.rs +++ b/crates/tranquil-pds/src/api/server/invite.rs @@ -1,5 +1,5 @@ use crate::api::ApiError; -use crate::auth::{Active, Admin, Auth}; +use crate::auth::{Admin, Auth, NotTakendown}; use crate::state::AppState; use crate::types::Did; use axum::{ @@ -193,7 +193,7 @@ pub struct GetAccountInviteCodesOutput { pub async fn get_account_invite_codes( State(state): State, - auth: Auth, + auth: Auth, axum::extract::Query(params): axum::extract::Query, ) -> Result { let include_used = params.include_used.unwrap_or(true); diff --git a/crates/tranquil-pds/src/api/server/session.rs b/crates/tranquil-pds/src/api/server/session.rs index 3dc5e9b..414aed9 100644 --- a/crates/tranquil-pds/src/api/server/session.rs +++ b/crates/tranquil-pds/src/api/server/session.rs @@ -1,6 +1,6 @@ use crate::api::error::ApiError; use crate::api::{EmptyResponse, SuccessResponse}; -use crate::auth::{Active, Auth, NotTakendown}; +use crate::auth::{Active, Auth, Permissive}; use crate::state::{AppState, RateLimitKind}; use crate::types::{AccountState, Did, Handle, PlainPassword}; use axum::{ @@ -279,7 +279,7 @@ pub async fn create_session( pub async fn get_session( State(state): State, - auth: Auth, + auth: Auth, ) -> Result { let permissions = auth.permissions(); let can_read_email = permissions.allows_email_read(); diff --git a/crates/tranquil-pds/src/auth/extractor.rs b/crates/tranquil-pds/src/auth/extractor.rs index 9ea40ff..98e8f8b 100644 --- a/crates/tranquil-pds/src/auth/extractor.rs +++ b/crates/tranquil-pds/src/auth/extractor.rs @@ -27,7 +27,6 @@ pub enum AuthError { AccountTakedown, AdminRequired, ServiceAuthNotAllowed, - SigningKeyRequired, InsufficientScope(String), OAuthExpiredToken(String), UseDpopNonce(String), @@ -430,6 +429,28 @@ impl FromRequestParts for ServiceAuth { } } +impl OptionalFromRequestParts for ServiceAuth { + type Rejection = AuthError; + + async fn from_request_parts( + parts: &mut Parts, + state: &AppState, + ) -> Result, Self::Rejection> { + match extract_auth_internal(parts, state).await { + Ok(ExtractedAuth::Service(claims)) => { + let did: Did = claims + .iss + .parse() + .map_err(|_| AuthError::AuthenticationFailed)?; + Ok(Some(ServiceAuth { did, claims })) + } + Ok(ExtractedAuth::User(_)) => Err(AuthError::AuthenticationFailed), + Err(AuthError::MissingToken) => Ok(None), + Err(e) => Err(e), + } + } +} + pub enum AuthAny { User(Auth

), Service(ServiceAuth), @@ -517,86 +538,6 @@ impl OptionalFromRequestParts for AuthAny

{ } } -pub struct SigningAuth { - pub did: Did, - pub key_bytes: Vec, - pub is_admin: bool, - pub status: AccountStatus, - pub scope: Option, - pub controller_did: Option, - is_oauth: bool, - _policy: PhantomData

, -} - -impl SigningAuth

{ - pub fn needs_scope_check(&self) -> bool { - self.is_oauth - } - - pub fn permissions(&self) -> ScopePermissions { - if let Some(ref scope) = self.scope - && scope != super::SCOPE_ACCESS - { - return ScopePermissions::from_scope_string(Some(scope)); - } - if !self.is_oauth { - return ScopePermissions::from_scope_string(Some("atproto")); - } - ScopePermissions::from_scope_string(self.scope.as_deref()) - } - - #[allow(clippy::result_large_err)] - pub fn check_repo_scope(&self, action: RepoAction, collection: &str) -> Result<(), Response> { - if !self.needs_scope_check() { - return Ok(()); - } - self.permissions() - .assert_repo(action, collection) - .map_err(|e| ApiError::InsufficientScope(Some(e.to_string())).into_response()) - } -} - -impl FromRequestParts for SigningAuth

{ - type Rejection = AuthError; - - async fn from_request_parts( - parts: &mut Parts, - state: &AppState, - ) -> Result { - let user = extract_user_auth_internal(parts, state).await?; - P::validate(&user)?; - - let key_bytes = match user.key_bytes { - Some(kb) => kb, - None => { - let user_with_key = state - .user_repo - .get_with_key_by_did(&user.did) - .await - .ok() - .flatten() - .ok_or(AuthError::SigningKeyRequired)?; - crate::config::decrypt_key( - &user_with_key.key_bytes, - user_with_key.encryption_version, - ) - .map_err(|_| AuthError::SigningKeyRequired)? - } - }; - - Ok(SigningAuth { - did: user.did, - key_bytes, - is_admin: user.is_admin, - status: user.status, - scope: user.scope, - controller_did: user.controller_did, - is_oauth: user.auth_source.is_oauth(), - _policy: PhantomData, - }) - } -} - #[cfg(test)] fn extract_bearer_token(auth_header: &str) -> Result<&str, AuthError> { let auth_header = auth_header.trim(); diff --git a/crates/tranquil-pds/src/auth/mod.rs b/crates/tranquil-pds/src/auth/mod.rs index 1f07eb3..ef693f3 100644 --- a/crates/tranquil-pds/src/auth/mod.rs +++ b/crates/tranquil-pds/src/auth/mod.rs @@ -18,8 +18,7 @@ pub mod webauthn; pub use extractor::{ Active, Admin, AnyUser, Auth, AuthAny, AuthError, AuthPolicy, ExtractedToken, NotTakendown, - Permissive, ServiceAuth, SigningAuth, extract_auth_token_from_header, - extract_bearer_token_from_header, + Permissive, ServiceAuth, extract_auth_token_from_header, extract_bearer_token_from_header, }; pub use service::{ServiceTokenClaims, ServiceTokenVerifier, is_service_token}; diff --git a/crates/tranquil-pds/tests/actor.rs b/crates/tranquil-pds/tests/actor.rs index d1bae1b..753d904 100644 --- a/crates/tranquil-pds/tests/actor.rs +++ b/crates/tranquil-pds/tests/actor.rs @@ -436,3 +436,105 @@ async fn test_declared_age_pref_computed_under_18() { assert_eq!(declared_age["isOverAge16"], false); assert_eq!(declared_age["isOverAge18"], false); } + +#[tokio::test] +async fn test_deactivated_account_can_get_preferences() { + let client = client(); + let base = base_url().await; + let (token, _did) = create_account_and_login(&client).await; + + let prefs = json!({ + "preferences": [ + { + "$type": "app.bsky.actor.defs#adultContentPref", + "enabled": true + } + ] + }); + let put_resp = client + .post(format!("{}/xrpc/app.bsky.actor.putPreferences", base)) + .header("Authorization", format!("Bearer {}", token)) + .json(&prefs) + .send() + .await + .unwrap(); + assert_eq!(put_resp.status(), 200); + + let deactivate = client + .post(format!( + "{}/xrpc/com.atproto.server.deactivateAccount", + base + )) + .header("Authorization", format!("Bearer {}", token)) + .json(&json!({})) + .send() + .await + .unwrap(); + assert_eq!(deactivate.status(), 200); + + let get_resp = client + .get(format!("{}/xrpc/app.bsky.actor.getPreferences", base)) + .header("Authorization", format!("Bearer {}", token)) + .send() + .await + .unwrap(); + assert_eq!( + get_resp.status(), + 200, + "Deactivated account should still be able to get preferences" + ); + let body: Value = get_resp.json().await.unwrap(); + let prefs_arr = body["preferences"].as_array().unwrap(); + assert_eq!(prefs_arr.len(), 1); +} + +#[tokio::test] +async fn test_deactivated_account_can_put_preferences() { + let client = client(); + let base = base_url().await; + let (token, _did) = create_account_and_login(&client).await; + + let deactivate = client + .post(format!( + "{}/xrpc/com.atproto.server.deactivateAccount", + base + )) + .header("Authorization", format!("Bearer {}", token)) + .json(&json!({})) + .send() + .await + .unwrap(); + assert_eq!(deactivate.status(), 200); + + let prefs = json!({ + "preferences": [ + { + "$type": "app.bsky.actor.defs#adultContentPref", + "enabled": true + } + ] + }); + let put_resp = client + .post(format!("{}/xrpc/app.bsky.actor.putPreferences", base)) + .header("Authorization", format!("Bearer {}", token)) + .json(&prefs) + .send() + .await + .unwrap(); + assert_eq!( + put_resp.status(), + 200, + "Deactivated account should still be able to put preferences" + ); + + let get_resp = client + .get(format!("{}/xrpc/app.bsky.actor.getPreferences", base)) + .header("Authorization", format!("Bearer {}", token)) + .send() + .await + .unwrap(); + assert_eq!(get_resp.status(), 200); + let body: Value = get_resp.json().await.unwrap(); + let prefs_arr = body["preferences"].as_array().unwrap(); + assert_eq!(prefs_arr.len(), 1); +} diff --git a/crates/tranquil-pds/tests/auth_extractor.rs b/crates/tranquil-pds/tests/auth_extractor.rs index fae4214..fb1faaf 100644 --- a/crates/tranquil-pds/tests/auth_extractor.rs +++ b/crates/tranquil-pds/tests/auth_extractor.rs @@ -581,3 +581,68 @@ fn generate_dpop_proof(method: &str, uri: &str, nonce: Option<&str>) -> (Value, let proof = format!("{}.{}", signing_input, sig_b64); (jwk, proof) } + +#[tokio::test] +async fn test_optional_service_auth_extractor_behavior() { + let url = base_url().await; + let http_client = client(); + let (access_jwt, did) = create_account_and_login(&http_client).await; + + let service_auth_res = http_client + .get(format!("{}/xrpc/com.atproto.server.getServiceAuth", url)) + .bearer_auth(&access_jwt) + .query(&[("aud", "did:web:test.example")]) + .send() + .await + .unwrap(); + assert_eq!(service_auth_res.status(), StatusCode::OK); + let service_body: Value = service_auth_res.json().await.unwrap(); + let service_token = service_body["token"].as_str().unwrap(); + + let no_auth_res = http_client + .get(format!( + "{}/xrpc/com.atproto.sync.getBlob?did={}&cid=bafyreifakecidfornowfakecidfornow1234567", + url, did + )) + .send() + .await + .unwrap(); + assert!( + no_auth_res.status() == StatusCode::NOT_FOUND + || no_auth_res.status() == StatusCode::BAD_REQUEST, + "getBlob with no auth should reach handler (AuthAny optional path) - got {}", + no_auth_res.status() + ); + + let service_auth_blob_res = http_client + .get(format!( + "{}/xrpc/com.atproto.sync.getBlob?did={}&cid=bafyreifakecidfornowfakecidfornow1234567", + url, did + )) + .bearer_auth(service_token) + .send() + .await + .unwrap(); + assert!( + service_auth_blob_res.status() == StatusCode::NOT_FOUND + || service_auth_blob_res.status() == StatusCode::BAD_REQUEST, + "getBlob with service auth should reach handler (AuthAny service path) - got {}", + service_auth_blob_res.status() + ); + + let user_auth_blob_res = http_client + .get(format!( + "{}/xrpc/com.atproto.sync.getBlob?did={}&cid=bafyreifakecidfornowfakecidfornow1234567", + url, did + )) + .bearer_auth(&access_jwt) + .send() + .await + .unwrap(); + assert!( + user_auth_blob_res.status() == StatusCode::NOT_FOUND + || user_auth_blob_res.status() == StatusCode::BAD_REQUEST, + "getBlob with user auth should reach handler (AuthAny user path) - got {}", + user_auth_blob_res.status() + ); +}