diff --git a/.sqlx/query-8047fda41bd94f819213decb8b3e0aba49a8dbdb10217eefd77e3567f8c9694a.json b/.sqlx/query-8047fda41bd94f819213decb8b3e0aba49a8dbdb10217eefd77e3567f8c9694a.json deleted file mode 100644 index 70ae98c..0000000 --- a/.sqlx/query-8047fda41bd94f819213decb8b3e0aba49a8dbdb10217eefd77e3567f8c9694a.json +++ /dev/null @@ -1,158 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "UPDATE comms_queue\n SET status = 'processing', updated_at = NOW()\n WHERE id IN (\n SELECT id FROM comms_queue\n WHERE status = 'pending'\n AND scheduled_for <= $1\n AND attempts < max_attempts\n ORDER BY scheduled_for ASC\n LIMIT $2\n FOR UPDATE SKIP LOCKED\n )\n RETURNING\n id, user_id,\n channel as \"channel: CommsChannel\",\n comms_type as \"comms_type: CommsType\",\n status as \"status: CommsStatus\",\n recipient, subject, body, metadata,\n attempts, max_attempts, last_error,\n created_at, updated_at, scheduled_for, processed_at", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "id", - "type_info": "Uuid" - }, - { - "ordinal": 1, - "name": "user_id", - "type_info": "Uuid" - }, - { - "ordinal": 2, - "name": "channel: CommsChannel", - "type_info": { - "Custom": { - "name": "comms_channel", - "kind": { - "Enum": [ - "email", - "discord", - "telegram", - "signal" - ] - } - } - } - }, - { - "ordinal": 3, - "name": "comms_type: CommsType", - "type_info": { - "Custom": { - "name": "comms_type", - "kind": { - "Enum": [ - "welcome", - "email_verification", - "password_reset", - "email_update", - "account_deletion", - "admin_email", - "plc_operation", - "two_factor_code", - "channel_verification", - "passkey_recovery", - "legacy_login_alert", - "migration_verification", - "channel_verified" - ] - } - } - } - }, - { - "ordinal": 4, - "name": "status: CommsStatus", - "type_info": { - "Custom": { - "name": "comms_status", - "kind": { - "Enum": [ - "pending", - "processing", - "sent", - "failed" - ] - } - } - } - }, - { - "ordinal": 5, - "name": "recipient", - "type_info": "Text" - }, - { - "ordinal": 6, - "name": "subject", - "type_info": "Text" - }, - { - "ordinal": 7, - "name": "body", - "type_info": "Text" - }, - { - "ordinal": 8, - "name": "metadata", - "type_info": "Jsonb" - }, - { - "ordinal": 9, - "name": "attempts", - "type_info": "Int4" - }, - { - "ordinal": 10, - "name": "max_attempts", - "type_info": "Int4" - }, - { - "ordinal": 11, - "name": "last_error", - "type_info": "Text" - }, - { - "ordinal": 12, - "name": "created_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 13, - "name": "updated_at", - "type_info": "Timestamptz" - }, - { - "ordinal": 14, - "name": "scheduled_for", - "type_info": "Timestamptz" - }, - { - "ordinal": 15, - "name": "processed_at", - "type_info": "Timestamptz" - } - ], - "parameters": { - "Left": [ - "Timestamptz", - "Int8" - ] - }, - "nullable": [ - false, - false, - false, - false, - false, - false, - true, - false, - true, - false, - false, - true, - false, - false, - false, - true - ] - }, - "hash": "8047fda41bd94f819213decb8b3e0aba49a8dbdb10217eefd77e3567f8c9694a" -} diff --git a/.sqlx/query-56cd24903171eddc2ededd9079ffe10937c34e99b0305f25c980ca754da44625.json b/.sqlx/query-8f4357f7a18ddcf6b686a4555f244d37c35917364b8f917ca6ee2d4030ace742.json similarity index 59% rename from .sqlx/query-56cd24903171eddc2ededd9079ffe10937c34e99b0305f25c980ca754da44625.json rename to .sqlx/query-8f4357f7a18ddcf6b686a4555f244d37c35917364b8f917ca6ee2d4030ace742.json index 4bf02af..edb414b 100644 --- a/.sqlx/query-56cd24903171eddc2ededd9079ffe10937c34e99b0305f25c980ca754da44625.json +++ b/.sqlx/query-8f4357f7a18ddcf6b686a4555f244d37c35917364b8f917ca6ee2d4030ace742.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "\n DELETE FROM oauth_token\n WHERE id IN (\n SELECT id FROM oauth_token\n WHERE did = $1\n ORDER BY updated_at ASC\n OFFSET $2\n )\n ", + "query": "\n DELETE FROM oauth_token\n WHERE id IN (\n SELECT id FROM oauth_token\n WHERE did = $1\n ORDER BY created_at DESC\n OFFSET $2\n )\n ", "describe": { "columns": [], "parameters": { @@ -11,5 +11,5 @@ }, "nullable": [] }, - "hash": "56cd24903171eddc2ededd9079ffe10937c34e99b0305f25c980ca754da44625" + "hash": "8f4357f7a18ddcf6b686a4555f244d37c35917364b8f917ca6ee2d4030ace742" } diff --git a/crates/tranquil-db/src/postgres/oauth.rs b/crates/tranquil-db/src/postgres/oauth.rs index 948912f..c6a8d8e 100644 --- a/crates/tranquil-db/src/postgres/oauth.rs +++ b/crates/tranquil-db/src/postgres/oauth.rs @@ -374,7 +374,7 @@ impl OAuthRepository for PostgresOAuthRepository { WHERE id IN ( SELECT id FROM oauth_token WHERE did = $1 - ORDER BY updated_at ASC + ORDER BY created_at DESC OFFSET $2 ) "#, diff --git a/crates/tranquil-pds/src/api/error.rs b/crates/tranquil-pds/src/api/error.rs index 96a68e9..1ec5029 100644 --- a/crates/tranquil-pds/src/api/error.rs +++ b/crates/tranquil-pds/src/api/error.rs @@ -21,6 +21,8 @@ pub enum ApiError { InvalidToken(Option), ExpiredToken(Option), OAuthExpiredToken(Option), + UseDpopNonce(String), + InvalidDpopProof(String), TokenRequired, AccountDeactivated, AccountTakedown, @@ -137,6 +139,8 @@ impl ApiError { | Self::InvalidToken(_) | Self::PasskeyCounterAnomaly | Self::OAuthExpiredToken(_) + | Self::UseDpopNonce(_) + | Self::InvalidDpopProof(_) | Self::ReauthRequired { .. } => StatusCode::UNAUTHORIZED, Self::InvalidCode(_) => StatusCode::BAD_REQUEST, Self::ExpiredToken(_) => StatusCode::BAD_REQUEST, @@ -236,6 +240,8 @@ impl ApiError { Self::AuthenticationFailed(_) => Cow::Borrowed("AuthenticationFailed"), Self::InvalidToken(_) => Cow::Borrowed("InvalidToken"), Self::ExpiredToken(_) | Self::OAuthExpiredToken(_) => Cow::Borrowed("ExpiredToken"), + Self::UseDpopNonce(_) => Cow::Borrowed("use_dpop_nonce"), + Self::InvalidDpopProof(_) => Cow::Borrowed("invalid_dpop_proof"), Self::TokenRequired => Cow::Borrowed("TokenRequired"), Self::AccountDeactivated => Cow::Borrowed("AccountDeactivated"), Self::AccountTakedown => Cow::Borrowed("AccountTakedown"), @@ -335,6 +341,8 @@ impl ApiError { Self::ExpiredToken(msg) | Self::OAuthExpiredToken(msg) => { msg.clone().unwrap_or_else(|| "Token has expired".into()) } + Self::UseDpopNonce(_) => "DPoP nonce required".into(), + Self::InvalidDpopProof(msg) => msg.clone(), Self::RepoNotFound(msg) => msg .clone() .unwrap_or_else(|| "Repository not found".into()), @@ -560,6 +568,36 @@ impl IntoResponse for ApiError { ), ); } + Self::UseDpopNonce(nonce) => { + match HeaderValue::from_str(nonce) { + Ok(val) => { + response + .headers_mut() + .insert(crate::util::HEADER_DPOP_NONCE, val); + } + Err(err) => { + tracing::error!( + ?err, + nonce_len = nonce.len(), + "generated DPoP nonce is not a valid header value" + ); + } + } + response.headers_mut().insert( + http::header::WWW_AUTHENTICATE, + HeaderValue::from_static( + "DPoP error=\"use_dpop_nonce\", error_description=\"Resource server requires nonce in DPoP proof\"", + ), + ); + } + Self::InvalidDpopProof(_) => { + response.headers_mut().insert( + http::header::WWW_AUTHENTICATE, + HeaderValue::from_static( + "DPoP error=\"invalid_dpop_proof\", error_description=\"Invalid DPoP proof\"", + ), + ); + } _ => {} } response @@ -596,6 +634,8 @@ impl From for ApiError { crate::auth::TokenValidationError::InvalidToken => { Self::AuthenticationFailed(Some("Invalid token format".to_string())) } + crate::auth::TokenValidationError::UseDpopNonce(nonce) => Self::UseDpopNonce(nonce), + crate::auth::TokenValidationError::InvalidDpopProof(msg) => Self::InvalidDpopProof(msg), } } } @@ -625,9 +665,9 @@ impl From for ApiError { crate::auth::extractor::AuthError::OAuthExpiredToken(msg) => { Self::OAuthExpiredToken(Some(msg)) } - crate::auth::extractor::AuthError::UseDpopNonce(_) - | crate::auth::extractor::AuthError::InvalidDpopProof(_) => { - Self::AuthenticationFailed(None) + crate::auth::extractor::AuthError::UseDpopNonce(nonce) => Self::UseDpopNonce(nonce), + crate::auth::extractor::AuthError::InvalidDpopProof(msg) => { + Self::InvalidDpopProof(msg) } } } diff --git a/crates/tranquil-pds/src/auth/extractor.rs b/crates/tranquil-pds/src/auth/extractor.rs index 5f1573d..f999f9b 100644 --- a/crates/tranquil-pds/src/auth/extractor.rs +++ b/crates/tranquil-pds/src/auth/extractor.rs @@ -2,7 +2,7 @@ use std::marker::PhantomData; use axum::{ extract::{FromRequestParts, OptionalFromRequestParts, OriginalUri}, - http::{StatusCode, header::AUTHORIZATION, request::Parts}, + http::{header::AUTHORIZATION, request::Parts}, response::{IntoResponse, Response}, }; use tracing::{debug, error, info}; @@ -35,32 +35,7 @@ pub enum AuthError { impl IntoResponse for AuthError { fn into_response(self) -> Response { - match self { - Self::UseDpopNonce(nonce) => ( - StatusCode::UNAUTHORIZED, - [ - ("DPoP-Nonce", nonce.as_str()), - ("WWW-Authenticate", "DPoP error=\"use_dpop_nonce\""), - ], - axum::Json(serde_json::json!({ - "error": "use_dpop_nonce", - "message": "DPoP nonce required" - })), - ) - .into_response(), - Self::OAuthExpiredToken(msg) => ApiError::OAuthExpiredToken(Some(msg)).into_response(), - Self::InvalidDpopProof(msg) => ( - StatusCode::UNAUTHORIZED, - [("WWW-Authenticate", "DPoP error=\"invalid_dpop_proof\"")], - axum::Json(serde_json::json!({ - "error": "invalid_dpop_proof", - "message": msg - })), - ) - .into_response(), - Self::InsufficientScope(msg) => ApiError::InsufficientScope(Some(msg)).into_response(), - other => ApiError::from(other).into_response(), - } + ApiError::from(self).into_response() } } diff --git a/crates/tranquil-pds/src/auth/mod.rs b/crates/tranquil-pds/src/auth/mod.rs index 5329d81..09037bf 100644 --- a/crates/tranquil-pds/src/auth/mod.rs +++ b/crates/tranquil-pds/src/auth/mod.rs @@ -106,7 +106,7 @@ struct CachedUserStatus { is_admin: bool, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub enum TokenValidationError { AccountDeactivated, AccountTakedown, @@ -115,6 +115,8 @@ pub enum TokenValidationError { TokenExpired, OAuthTokenExpired, InvalidToken, + UseDpopNonce(String), + InvalidDpopProof(String), } impl fmt::Display for TokenValidationError { @@ -126,6 +128,8 @@ impl fmt::Display for TokenValidationError { Self::AuthenticationFailed => write!(f, "AuthenticationFailed"), Self::TokenExpired | Self::OAuthTokenExpired => write!(f, "ExpiredToken"), Self::InvalidToken => write!(f, "InvalidToken"), + Self::UseDpopNonce(_) => write!(f, "use_dpop_nonce"), + Self::InvalidDpopProof(_) => write!(f, "invalid_dpop_proof"), } } } @@ -613,6 +617,12 @@ pub async fn validate_token_with_dpop( Err(crate::oauth::OAuthError::ExpiredToken(_)) => { Err(TokenValidationError::OAuthTokenExpired) } + Err(crate::oauth::OAuthError::UseDpopNonce(nonce)) => { + Err(TokenValidationError::UseDpopNonce(nonce)) + } + Err(crate::oauth::OAuthError::InvalidDpopProof(msg)) => { + Err(TokenValidationError::InvalidDpopProof(msg)) + } Err(_) => Err(TokenValidationError::AuthenticationFailed), } } diff --git a/crates/tranquil-pds/tests/oauth_token_eviction.rs b/crates/tranquil-pds/tests/oauth_token_eviction.rs new file mode 100644 index 0000000..3b69c3c --- /dev/null +++ b/crates/tranquil-pds/tests/oauth_token_eviction.rs @@ -0,0 +1,142 @@ +mod common; +mod helpers; + +use chrono::{DateTime, Duration, Utc}; +use common::{base_url, client, get_test_db_pool, get_test_repos}; +use helpers::verify_new_account; +use reqwest::StatusCode; +use serde_json::{Value, json}; +use tranquil_types::Did; + +async fn create_account_and_get_did(handle: &str, email: &str, password: &str) -> Did { + let client = client(); + let res = client + .post(format!( + "{}/xrpc/com.atproto.server.createAccount", + base_url().await + )) + .json(&json!({ + "handle": handle, + "email": email, + "password": password, + })) + .send() + .await + .expect("createAccount request failed"); + assert_eq!(res.status(), StatusCode::OK, "createAccount failed"); + let body: Value = res.json().await.expect("invalid createAccount JSON"); + let did_str = body["did"].as_str().expect("no did in response").to_string(); + let _ = verify_new_account(&client, &did_str).await; + Did::new(did_str).expect("invalid DID format") +} + +async fn insert_token_with_created_at( + pool: &sqlx::PgPool, + did: &Did, + token_id: &str, + created_at: DateTime, +) { + sqlx::query( + r#" + INSERT INTO oauth_token ( + did, token_id, created_at, updated_at, expires_at, + client_id, client_auth, parameters + ) VALUES ($1, $2, $3, $3, $4, $5, $6::jsonb, $7::jsonb) + "#, + ) + .bind(did.as_str()) + .bind(token_id) + .bind(created_at) + .bind(created_at + Duration::hours(1)) + .bind("https://test.example/client") + .bind(r#"{"method":"none"}"#) + .bind( + r#"{"response_type":"code","client_id":"https://test.example/client","redirect_uri":"https://test.example/cb","code_challenge":"x","code_challenge_method":"S256"}"#, + ) + .execute(pool) + .await + .expect("token insert failed"); +} + +#[tokio::test] +async fn delete_oldest_tokens_evicts_lowest_created_at() { + let ts = Utc::now().timestamp_millis(); + let handle = format!("tok-evict-{}.test", ts); + let email = format!("tok-evict-{}@test.com", ts); + let did = create_account_and_get_did(&handle, &email, "EvictTest123!").await; + + let pool = get_test_db_pool().await; + let repos = get_test_repos().await; + + let base = Utc::now(); + let token_ids: Vec = (0..5) + .map(|i| format!("tok-{}-{}", ts, i)) + .collect(); + + for (i, tid) in token_ids.iter().enumerate() { + let created = base + Duration::seconds(i as i64); + insert_token_with_created_at(pool, &did, tid, created).await; + } + + let count_before = repos + .oauth + .count_tokens_for_user(&did) + .await + .expect("count failed"); + assert_eq!(count_before, 5, "all 5 tokens should be present"); + + let deleted = repos + .oauth + .delete_oldest_tokens_for_user(&did, 3) + .await + .expect("delete failed"); + assert_eq!(deleted, 2, "two oldest tokens should be deleted"); + + let remaining = repos + .oauth + .list_tokens_for_user(&did) + .await + .expect("list failed"); + assert_eq!(remaining.len(), 3, "three newest tokens should remain"); + + let remaining_ids: std::collections::HashSet = + remaining.iter().map(|t| t.token_id.0.clone()).collect(); + let expected_ids: std::collections::HashSet = + token_ids[2..].iter().cloned().collect(); + assert_eq!( + remaining_ids, expected_ids, + "surviving tokens must be the three newest by created_at" + ); +} + +#[tokio::test] +async fn delete_oldest_tokens_no_op_when_under_keep_count() { + let ts = Utc::now().timestamp_millis(); + let handle = format!("tok-evict-noop-{}.test", ts); + let email = format!("tok-evict-noop-{}@test.com", ts); + let did = create_account_and_get_did(&handle, &email, "EvictTest123!").await; + + let pool = get_test_db_pool().await; + let repos = get_test_repos().await; + + let base = Utc::now(); + for i in 0..2 { + let tid = format!("noop-tok-{}-{}", ts, i); + let created = base + Duration::seconds(i); + insert_token_with_created_at(pool, &did, &tid, created).await; + } + + let deleted = repos + .oauth + .delete_oldest_tokens_for_user(&did, 5) + .await + .expect("delete failed"); + assert_eq!(deleted, 0, "nothing to delete when count <= keep"); + + let remaining = repos + .oauth + .list_tokens_for_user(&did) + .await + .expect("list failed"); + assert_eq!(remaining.len(), 2); +}