mirrors/tranquil-pds
1
0
Fork 0
mirror of https://tangled.org/tranquil.farm/tranquil-pds synced 2026-09-03 08:46:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(oauth): gc tokens in pg in the right order, more exposure of dpop err

Browse Source 
Lewis: May this revision serve well! <lu5a@proton.me>
This commit is contained in:
Lewis
2026-05-14 16:51:53 +03:00
committed by Tangled
parent 60e10af4aa
commit a13343e1de
7 changed files with 201 additions and 192 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.sqlx/query-8047fda41bd94f819213decb8b3e0aba49a8dbdb10217eefd77e3567f8c9694a.json
Generated
-158
View File
@@ -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"
}
.sqlx/query-56cd24903171eddc2ededd9079ffe10937c34e99b0305f25c980ca754da44625.json → .sqlx/query-8f4357f7a18ddcf6b686a4555f244d37c35917364b8f917ca6ee2d4030ace742.json
Generated
+2 -2
View File
@@ -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"
}
crates/tranquil-db/src/postgres/oauth.rs
+1 -1
View File
@@ -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
)
"#,
crates/tranquil-pds/src/api/error.rs
+43 -3
View File
@@ -21,6 +21,8 @@ pub enum ApiError {
InvalidToken(Option<String>),
ExpiredToken(Option<String>),
OAuthExpiredToken(Option<String>),
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<crate::auth::TokenValidationError> 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<crate::auth::extractor::AuthError> 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)
}
}
}
crates/tranquil-pds/src/auth/extractor.rs
+2 -27
View File
@@ -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()
}
}
crates/tranquil-pds/src/auth/mod.rs
+11 -1
View File
@@ -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),
}
}
crates/tranquil-pds/tests/oauth_token_eviction.rs
+142
View File
@@ -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<Utc>,
) {
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<String> = (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<String> =
remaining.iter().map(|t| t.token_id.0.clone()).collect();
let expected_ids: std::collections::HashSet<String> =
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);
}