An easy-to-host PDS on the ATProtocol, iPhone and MacOS. Maintain control of your keys and data, always.
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

fix: address PR review feedback for deleteSession

- Assert createSession returns 200 in test helper
- Add tracing::debug\! on idempotent not-found path
- Check rows_affected() on DELETE FROM sessions (warn, not error)
- Add test: rotated token still revokes entire session
- Add test: access JWT remains valid until expiry after session delete
- Add comment explaining why ExpiredSignature arm is absent

authored by

Malpercio and committed by
Tangled
1447ff18 fb1eb44e

+132 -1
+2
crates/relay/src/auth/jwt.rs
··· 237 237 validation.set_required_spec_claims(&["sub"]); 238 238 validation.leeway = 0; 239 239 240 + // Note: no ExpiredSignature arm — `validate_exp = false` means jsonwebtoken 241 + // never emits that error kind here. All failures are signature/structural. 240 242 decode::<RefreshTokenClaims>(token, &decoding_key, &validation) 241 243 .map(|data| data.claims) 242 244 .map_err(|e| {
+130 -1
crates/relay/src/routes/delete_session.rs
··· 57 57 58 58 // Idempotent: token not found means already revoked — logout already done. 59 59 let Some(session_id) = session_id else { 60 + tracing::debug!(jti = %jti, "deleteSession called with unknown jti; treating as already revoked"); 60 61 return Ok(StatusCode::OK); 61 62 }; 62 63 ··· 75 76 ApiError::new(ErrorCode::InternalError, "internal error") 76 77 })?; 77 78 78 - sqlx::query("DELETE FROM sessions WHERE id = ?") 79 + let deleted = sqlx::query("DELETE FROM sessions WHERE id = ?") 79 80 .bind(&session_id) 80 81 .execute(&mut *tx) 81 82 .await ··· 83 84 tracing::error!(error = %e, session_id = %session_id, "failed to delete session"); 84 85 ApiError::new(ErrorCode::InternalError, "internal error") 85 86 })?; 87 + 88 + if deleted.rows_affected() != 1 { 89 + tracing::warn!( 90 + session_id = %session_id, 91 + jti = %jti, 92 + rows = deleted.rows_affected(), 93 + "session DELETE affected unexpected row count; session may have been removed concurrently" 94 + ); 95 + } 86 96 87 97 tx.commit().await.map_err(|e| { 88 98 tracing::error!(error = %e, session_id = %session_id, "failed to commit revocation transaction"); ··· 132 142 ))) 133 143 .unwrap(); 134 144 let response = app(state.clone()).oneshot(request).await.unwrap(); 145 + assert_eq!(response.status(), StatusCode::OK, "createSession must succeed"); 135 146 body_json(response).await 136 147 } 137 148 ··· 454 465 response.status(), 455 466 StatusCode::OK, 456 467 "expired token not in DB must be idempotent 200" 468 + ); 469 + } 470 + 471 + // ── Rotated token revocation ────────────────────────────────────────────── 472 + 473 + #[tokio::test] 474 + async fn rotated_token_revokes_entire_session() { 475 + // After refreshSession, the old refresh token has next_jti set but is still 476 + // in the DB with a valid session_id. deleteSession with the old token must 477 + // still revoke the session and all its refresh tokens. 478 + let state = test_state().await; 479 + insert_account_with_password( 480 + &state.db, 481 + "did:plc:del8", 482 + "del8.test.example.com", 483 + "del8@example.com", 484 + "hunter2", 485 + ) 486 + .await; 487 + 488 + let db = state.db.clone(); 489 + let tokens = create_session_tokens(&state, "did:plc:del8", "hunter2").await; 490 + let original_refresh_jwt = tokens["refreshJwt"].as_str().unwrap().to_string(); 491 + 492 + // Rotate the token via refreshSession — old token now has next_jti set. 493 + let rotated = app(state.clone()) 494 + .oneshot( 495 + Request::builder() 496 + .method("POST") 497 + .uri("/xrpc/com.atproto.server.refreshSession") 498 + .header("Authorization", format!("Bearer {original_refresh_jwt}")) 499 + .body(Body::empty()) 500 + .unwrap(), 501 + ) 502 + .await 503 + .unwrap(); 504 + assert_eq!(rotated.status(), StatusCode::OK, "refreshSession must succeed"); 505 + 506 + let session_id: String = 507 + sqlx::query_scalar("SELECT id FROM sessions WHERE did = 'did:plc:del8'") 508 + .fetch_one(&db) 509 + .await 510 + .unwrap(); 511 + 512 + // Call deleteSession with the old (now-rotated) token. 513 + let del = app(state) 514 + .oneshot(post_delete_session(&original_refresh_jwt)) 515 + .await 516 + .unwrap(); 517 + assert_eq!( 518 + del.status(), 519 + StatusCode::OK, 520 + "deleteSession with rotated token must return 200" 521 + ); 522 + 523 + let session_count: i64 = 524 + sqlx::query_scalar("SELECT COUNT(*) FROM sessions WHERE id = ?") 525 + .bind(&session_id) 526 + .fetch_one(&db) 527 + .await 528 + .unwrap(); 529 + assert_eq!(session_count, 0, "session must be deleted"); 530 + 531 + let token_count: i64 = 532 + sqlx::query_scalar("SELECT COUNT(*) FROM refresh_tokens WHERE session_id = ?") 533 + .bind(&session_id) 534 + .fetch_one(&db) 535 + .await 536 + .unwrap(); 537 + assert_eq!(token_count, 0, "all refresh tokens (including rotated) must be deleted"); 538 + } 539 + 540 + // ── Access token boundary ───────────────────────────────────────────────── 541 + 542 + #[tokio::test] 543 + async fn access_token_from_deleted_session_is_still_valid_until_expiry() { 544 + // Access JWTs are stateless — deleteSession cannot invalidate them before 545 + // their natural expiry. This test documents that as an intentional contract: 546 + // getSession must still succeed with the access JWT after the session is deleted. 547 + // If a future change adds session-row validation to the access path, this test 548 + // will fail loudly. 549 + let state = test_state().await; 550 + insert_account_with_password( 551 + &state.db, 552 + "did:plc:del9", 553 + "del9.test.example.com", 554 + "del9@example.com", 555 + "hunter2", 556 + ) 557 + .await; 558 + 559 + let tokens = create_session_tokens(&state, "did:plc:del9", "hunter2").await; 560 + let access_jwt = tokens["accessJwt"].as_str().unwrap().to_string(); 561 + let refresh_jwt = tokens["refreshJwt"].as_str().unwrap(); 562 + 563 + // Delete the session. 564 + let del = app(state.clone()) 565 + .oneshot(post_delete_session(refresh_jwt)) 566 + .await 567 + .unwrap(); 568 + assert_eq!(del.status(), StatusCode::OK); 569 + 570 + // Access JWT is still valid — getSession must return 200. 571 + let get = app(state) 572 + .oneshot( 573 + Request::builder() 574 + .method("GET") 575 + .uri("/xrpc/com.atproto.server.getSession") 576 + .header("Authorization", format!("Bearer {access_jwt}")) 577 + .body(Body::empty()) 578 + .unwrap(), 579 + ) 580 + .await 581 + .unwrap(); 582 + assert_eq!( 583 + get.status(), 584 + StatusCode::OK, 585 + "access JWT must remain valid until expiry after session deletion (stateless)" 457 586 ); 458 587 } 459 588 }