diff --git a/.sqlx/query-168e205e3eb832b78d085b48281a1bae74d5a0e64c4c793c18a6400605bacf76.json b/.sqlx/query-168e205e3eb832b78d085b48281a1bae74d5a0e64c4c793c18a6400605bacf76.json new file mode 100644 index 0000000..be9ecfe --- /dev/null +++ b/.sqlx/query-168e205e3eb832b78d085b48281a1bae74d5a0e64c4c793c18a6400605bacf76.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "DELETE FROM refresh_tokens WHERE jti = ?", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "168e205e3eb832b78d085b48281a1bae74d5a0e64c4c793c18a6400605bacf76" +} diff --git a/.sqlx/query-893c45c27854ba15ea611e8254ef980ced21dc64e6ca95fde56af513f86ffa46.json b/.sqlx/query-893c45c27854ba15ea611e8254ef980ced21dc64e6ca95fde56af513f86ffa46.json new file mode 100644 index 0000000..daea20c --- /dev/null +++ b/.sqlx/query-893c45c27854ba15ea611e8254ef980ced21dc64e6ca95fde56af513f86ffa46.json @@ -0,0 +1,20 @@ +{ + "db_name": "SQLite", + "query": "SELECT jti FROM refresh_tokens WHERE jti = ?", + "describe": { + "columns": [ + { + "name": "jti", + "ordinal": 0, + "type_info": "Text" + } + ], + "parameters": { + "Right": 1 + }, + "nullable": [ + true + ] + }, + "hash": "893c45c27854ba15ea611e8254ef980ced21dc64e6ca95fde56af513f86ffa46" +} diff --git a/.sqlx/query-c9ee5c64ca547f0c730379919a642bd649cbf81fc1804159101a70efabf08b33.json b/.sqlx/query-c9ee5c64ca547f0c730379919a642bd649cbf81fc1804159101a70efabf08b33.json new file mode 100644 index 0000000..4fcbfbc --- /dev/null +++ b/.sqlx/query-c9ee5c64ca547f0c730379919a642bd649cbf81fc1804159101a70efabf08b33.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "INSERT INTO refresh_tokens (jti, user_id, expires_at) VALUES (?, ?, ?)", + "describe": { + "columns": [], + "parameters": { + "Right": 3 + }, + "nullable": [] + }, + "hash": "c9ee5c64ca547f0c730379919a642bd649cbf81fc1804159101a70efabf08b33" +} diff --git a/.sqlx/query-ef7af925a8715c329dcafca5257c691e6bca31755eb5f54be47114f21fc04c8c.json b/.sqlx/query-ef7af925a8715c329dcafca5257c691e6bca31755eb5f54be47114f21fc04c8c.json new file mode 100644 index 0000000..b045281 --- /dev/null +++ b/.sqlx/query-ef7af925a8715c329dcafca5257c691e6bca31755eb5f54be47114f21fc04c8c.json @@ -0,0 +1,12 @@ +{ + "db_name": "SQLite", + "query": "DELETE FROM refresh_tokens WHERE expires_at < ?", + "describe": { + "columns": [], + "parameters": { + "Right": 1 + }, + "nullable": [] + }, + "hash": "ef7af925a8715c329dcafca5257c691e6bca31755eb5f54be47114f21fc04c8c" +} diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 462d3ca..96ff23f 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -660,7 +660,7 @@ All endpoints are under the base URL configured by the user (e.g., `https://soli |---|---|---|---|---| | POST | `/api/auth/register` | None | `{username, password}` | `{access_token, refresh_token}` | | POST | `/api/auth/login` | None | `{username, password}` | `{access_token, refresh_token}` | -| POST | `/api/auth/refresh` | None | `{refresh_token}` | `{access_token}` | +| POST | `/api/auth/refresh` | None | `{refresh_token}` | `{access_token, refresh_token}` (rotated) | ### Sync @@ -1020,6 +1020,7 @@ Migrations run automatically on startup via `sqlx::migrate!()`. | Password storage | bcrypt, cost factor 12 — never stored in plaintext | | Token security | JWTs signed with HS256, stored in OS keychain via `keyring` crate | | Token expiry | Access: 24h, Refresh: 30d | +| Refresh token rotation | Each `/api/auth/refresh` call consumes the incoming refresh token (deletes its jti row) and issues a new one. Reuse of a consumed token returns 401. Expired rows are pruned inline. | | Brute force | `tower-governor`: 10 req/min per IP on `/api/auth/*` | | Payload abuse | 1MB max request body, enforced by Axum middleware | | Data deletion | `DELETE /api/account` removes all rows via `ON DELETE CASCADE` | diff --git a/solitaire_data/src/sync_client.rs b/solitaire_data/src/sync_client.rs index f365eb5..4fd064a 100644 --- a/solitaire_data/src/sync_client.rs +++ b/solitaire_data/src/sync_client.rs @@ -162,16 +162,17 @@ impl SolitaireServerClient { /// Attempt to refresh the access token using the stored refresh token. /// - /// On success the new access token is persisted to the OS keychain, - /// replacing the previous one. The refresh token itself is unchanged. + /// The server rotates refresh tokens on each call: the response includes a + /// new refresh token that replaces the old one. Both tokens are persisted + /// to the OS keychain on success. async fn refresh_token(&self) -> Result<(), SyncError> { - let refresh = load_refresh_token(&self.username) + let old_refresh = load_refresh_token(&self.username) .map_err(|e| SyncError::Auth(e.to_string()))?; let resp = self .client .post(format!("{}/api/auth/refresh", self.base_url)) - .json(&serde_json::json!({ "refresh_token": refresh })) + .json(&serde_json::json!({ "refresh_token": old_refresh })) .send() .await .map_err(|e| SyncError::Network(e.to_string()))?; @@ -189,9 +190,11 @@ impl SolitaireServerClient { .as_str() .ok_or_else(|| SyncError::Serialization("missing access_token in refresh response".into()))?; - // store_tokens replaces both access and refresh; we keep the old - // refresh token unchanged so its 30-day TTL is preserved. - store_tokens(&self.username, new_access, &refresh) + // Server rotates refresh tokens — store the new one. + // Fall back to the old token if the field is absent (pre-rotation server). + let new_refresh = body["refresh_token"].as_str().unwrap_or(&old_refresh); + + store_tokens(&self.username, new_access, new_refresh) .map_err(|e| SyncError::Auth(e.to_string())) } diff --git a/solitaire_server/migrations/003_refresh_tokens.sql b/solitaire_server/migrations/003_refresh_tokens.sql new file mode 100644 index 0000000..96e7feb --- /dev/null +++ b/solitaire_server/migrations/003_refresh_tokens.sql @@ -0,0 +1,17 @@ +-- Migration 003: refresh token rotation table +-- +-- One row per live refresh token. Issued at login/register and rotated +-- (old row deleted, new row inserted) on every POST /api/auth/refresh call. +-- Cascade on user deletion means no manual cleanup is needed when an +-- account is removed. + +CREATE TABLE IF NOT EXISTS refresh_tokens ( + jti TEXT PRIMARY KEY, -- UUID v4 embedded in the JWT + user_id TEXT NOT NULL REFERENCES users(id) ON DELETE CASCADE, + expires_at TEXT NOT NULL -- ISO 8601, mirrors the JWT exp claim +); + +-- Expired-row pruning (done inline in the refresh handler) uses this index +-- to avoid a full table scan on every refresh call. +CREATE INDEX IF NOT EXISTS refresh_tokens_expires_at_idx + ON refresh_tokens(expires_at); diff --git a/solitaire_server/src/auth.rs b/solitaire_server/src/auth.rs index 7b7941c..c545846 100644 --- a/solitaire_server/src/auth.rs +++ b/solitaire_server/src/auth.rs @@ -37,10 +37,13 @@ pub struct AuthResponse { pub refresh_token: String, } -/// Successful refresh response — contains only the new access token. +/// Successful refresh response — contains the new access token and the rotated +/// refresh token. The refresh token is always rotated: the client must store +/// the new value and discard the old one. #[derive(Debug, Serialize)] pub struct RefreshResponse { pub access_token: String, + pub refresh_token: String, } // --------------------------------------------------------------------------- @@ -73,21 +76,47 @@ pub fn make_access_token(user_id: &str, secret: &str) -> Result Result { +/// +/// Returns `(jwt_string, jti)`. The caller must insert the jti into +/// `refresh_tokens` before returning the JWT to the client. +pub fn make_refresh_token(user_id: &str, secret: &str) -> Result<(String, String), AppError> { + let jti = Uuid::new_v4().to_string(); let exp = (Utc::now() + chrono::Duration::days(30)).timestamp() as usize; let claims = Claims { sub: user_id.to_string(), exp, kind: "refresh".to_string(), + jti: Some(jti.clone()), }; - encode(&Header::default(), &claims, &EncodingKey::from_secret(secret.as_bytes())) - .map_err(|e| AppError::Internal(e.to_string())) + let token = encode(&Header::default(), &claims, &EncodingKey::from_secret(secret.as_bytes())) + .map_err(|e| AppError::Internal(e.to_string()))?; + Ok((token, jti)) +} + +/// Insert a jti row into `refresh_tokens`. Must be called immediately after +/// [`make_refresh_token`] and before the token is sent to the client. +async fn store_refresh_jti( + pool: &sqlx::SqlitePool, + jti: &str, + user_id: &str, +) -> Result<(), AppError> { + let expires_at = (Utc::now() + chrono::Duration::days(30)).to_rfc3339(); + sqlx::query!( + "INSERT INTO refresh_tokens (jti, user_id, expires_at) VALUES (?, ?, ?)", + jti, + user_id, + expires_at + ) + .execute(pool) + .await?; + Ok(()) } // --------------------------------------------------------------------------- @@ -160,9 +189,13 @@ pub async fn register( .execute(&state.pool) .await?; + let access_token = make_access_token(&user_id, &state.jwt_secret)?; + let (refresh_token, refresh_jti) = make_refresh_token(&user_id, &state.jwt_secret)?; + store_refresh_jti(&state.pool, &refresh_jti, &user_id).await?; + Ok(Json(AuthResponse { - access_token: make_access_token(&user_id, &state.jwt_secret)?, - refresh_token: make_refresh_token(&user_id, &state.jwt_secret)?, + access_token, + refresh_token, })) } @@ -190,27 +223,74 @@ pub async fn login( return Err(AppError::InvalidCredentials); } + let access_token = make_access_token(&row_id, &state.jwt_secret)?; + let (refresh_token, refresh_jti) = make_refresh_token(&row_id, &state.jwt_secret)?; + store_refresh_jti(&state.pool, &refresh_jti, &row_id).await?; + Ok(Json(AuthResponse { - access_token: make_access_token(&row_id, &state.jwt_secret)?, - refresh_token: make_refresh_token(&row_id, &state.jwt_secret)?, + access_token, + refresh_token, })) } -/// `POST /api/auth/refresh` — exchange a refresh token for a new access token. +/// `POST /api/auth/refresh` — exchange a valid refresh token for a new token pair. +/// +/// The incoming refresh token is consumed (its jti row is deleted) and a new +/// refresh token is issued. Using a consumed token returns 401. Tokens issued +/// before rotation was enabled (no `jti` claim) are also rejected with 401 — +/// the player must re-login once after upgrading the server. +/// +/// Expired rows from other sessions are pruned on each successful call. pub async fn refresh( State(state): State, Json(body): Json, ) -> Result, AppError> { let claims = validate_refresh_token(&body.refresh_token, &state.jwt_secret)?; + // Tokens without jti predate rotation — require re-login. + let jti = claims.jti.ok_or(AppError::Unauthorized)?; + + // Verify this jti is still live (not yet consumed or from a deleted account). + // SQLite TEXT columns are always nullable in sqlx; flatten the double-Option. + let exists: Option = sqlx::query_scalar!( + "SELECT jti FROM refresh_tokens WHERE jti = ?", + jti + ) + .fetch_optional(&state.pool) + .await? + .flatten(); + + if exists.is_none() { + return Err(AppError::Unauthorized); + } + + // Consume the old token before issuing new ones. If the insert below + // fails, the user loses this session (must re-login) — safe by design. + sqlx::query!("DELETE FROM refresh_tokens WHERE jti = ?", jti) + .execute(&state.pool) + .await?; + + let new_access = make_access_token(&claims.sub, &state.jwt_secret)?; + let (new_refresh, new_jti) = make_refresh_token(&claims.sub, &state.jwt_secret)?; + store_refresh_jti(&state.pool, &new_jti, &claims.sub).await?; + + // Prune expired rows from all sessions on each successful rotation. + // The expires_at index makes this a cheap index-backed scan. + let now = Utc::now().to_rfc3339(); + sqlx::query!("DELETE FROM refresh_tokens WHERE expires_at < ?", now) + .execute(&state.pool) + .await?; + Ok(Json(RefreshResponse { - access_token: make_access_token(&claims.sub, &state.jwt_secret)?, + access_token: new_access, + refresh_token: new_refresh, })) } /// `DELETE /api/account` — permanently delete the authenticated user's account. /// -/// All related rows are removed via `ON DELETE CASCADE` in the schema. +/// All related rows (sync_state, refresh_tokens, leaderboard) are removed +/// via `ON DELETE CASCADE` in the schema. pub async fn delete_account( State(state): State, user: AuthenticatedUser, @@ -229,7 +309,7 @@ mod tests { const TEST_SECRET: &str = "test_secret_for_unit_tests_only"; - fn decode_token(token: &str) -> Claims { + fn decode_claims(token: &str) -> Claims { let mut validation = Validation::default(); validation.leeway = 60; decode::( @@ -244,27 +324,39 @@ mod tests { #[test] fn make_access_token_decodes_with_correct_claims() { let token = make_access_token("user-123", TEST_SECRET).unwrap(); - let claims = decode_token(&token); + let claims = decode_claims(&token); assert_eq!(claims.sub, "user-123"); assert_eq!(claims.kind, "access"); + assert!(claims.jti.is_none(), "access token must not carry a jti"); let now = Utc::now().timestamp() as usize; - // expiry should be roughly 24 hours in the future (allow ±60s for test execution) assert!(claims.exp > now + 86_400 - 60); assert!(claims.exp < now + 86_400 + 60); } #[test] fn make_refresh_token_decodes_with_correct_claims() { - let token = make_refresh_token("user-456", TEST_SECRET).unwrap(); - let claims = decode_token(&token); + let (token, jti) = make_refresh_token("user-456", TEST_SECRET).unwrap(); + let claims = decode_claims(&token); assert_eq!(claims.sub, "user-456"); assert_eq!(claims.kind, "refresh"); + assert_eq!( + claims.jti.as_deref(), + Some(jti.as_str()), + "jti in JWT must match returned jti" + ); + assert!(!jti.is_empty(), "jti must be non-empty"); let now = Utc::now().timestamp() as usize; - // expiry should be roughly 30 days in the future (allow ±60s for test execution) assert!(claims.exp > now + 30 * 86_400 - 60); assert!(claims.exp < now + 30 * 86_400 + 60); } + #[test] + fn make_refresh_token_generates_unique_jtis() { + let (_, jti1) = make_refresh_token("u", TEST_SECRET).unwrap(); + let (_, jti2) = make_refresh_token("u", TEST_SECRET).unwrap(); + assert_ne!(jti1, jti2, "each call must produce a unique jti"); + } + #[test] fn make_access_token_wrong_secret_fails_decode() { let token = make_access_token("user-789", TEST_SECRET).unwrap(); @@ -279,9 +371,9 @@ mod tests { #[test] fn access_and_refresh_tokens_have_different_kinds() { let access = make_access_token("u", TEST_SECRET).unwrap(); - let refresh = make_refresh_token("u", TEST_SECRET).unwrap(); - let a_claims = decode_token(&access); - let r_claims = decode_token(&refresh); + let (refresh, _jti) = make_refresh_token("u", TEST_SECRET).unwrap(); + let a_claims = decode_claims(&access); + let r_claims = decode_claims(&refresh); assert_ne!(a_claims.kind, r_claims.kind); } diff --git a/solitaire_server/src/middleware.rs b/solitaire_server/src/middleware.rs index bf1b682..46c8a9f 100644 --- a/solitaire_server/src/middleware.rs +++ b/solitaire_server/src/middleware.rs @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize}; use crate::{error::AppError, AppState}; -/// The claims encoded in our JWT access tokens. +/// The claims encoded in our JWTs. #[derive(Debug, Serialize, Deserialize)] pub struct Claims { /// Subject — the user's UUID string. @@ -24,6 +24,10 @@ pub struct Claims { pub exp: usize, /// Token kind: `"access"` or `"refresh"`. pub kind: String, + /// JWT ID — UUID v4 embedded in refresh tokens for rotation tracking. + /// Access tokens omit this field (`None`). + #[serde(skip_serializing_if = "Option::is_none")] + pub jti: Option, } /// The authenticated user identity injected into request extensions after @@ -135,6 +139,7 @@ mod tests { sub: user_id.to_string(), exp, kind: kind.to_string(), + jti: None, }; encode(&Header::default(), &claims, &EncodingKey::from_secret(SECRET.as_bytes())).unwrap() } diff --git a/solitaire_server/tests/server_tests.rs b/solitaire_server/tests/server_tests.rs index df1a29e..1f259e6 100644 --- a/solitaire_server/tests/server_tests.rs +++ b/solitaire_server/tests/server_tests.rs @@ -347,9 +347,10 @@ async fn login_with_unknown_username_returns_401() { ); } -/// `POST /api/auth/refresh` with a valid refresh token returns 200 with a new access token. +/// `POST /api/auth/refresh` with a valid refresh token returns 200 with both +/// a new access token and a rotated refresh token. #[tokio::test] -async fn refresh_returns_new_access_token() { +async fn refresh_returns_new_access_and_refresh_tokens() { let app = build_test_router(test_pool().await); @@ -368,6 +369,80 @@ async fn refresh_returns_new_access_token() { body["access_token"].is_string(), "refresh must return a new access_token" ); + assert!( + body["refresh_token"].is_string(), + "refresh must return a rotated refresh_token" + ); + let rotated = body["refresh_token"].as_str().unwrap(); + assert_ne!( + rotated, refresh, + "rotated refresh token must differ from the original" + ); +} + +/// After a successful rotation, the old refresh token must be rejected (consumed). +#[tokio::test] +async fn consumed_refresh_token_is_rejected() { + let app = build_test_router(test_pool().await); + + let (_access, original_refresh) = + register_user(app.clone(), "grace_rot", "rotation_pass").await; + + // First refresh — consumes original_refresh, returns a new one. + let resp1 = post_json( + app.clone(), + "/api/auth/refresh", + serde_json::json!({ "refresh_token": original_refresh }), + ) + .await; + assert_eq!(resp1.status(), StatusCode::OK, "first rotation must succeed"); + + // Second attempt with the now-consumed original token must fail. + let resp2 = post_json( + app, + "/api/auth/refresh", + serde_json::json!({ "refresh_token": original_refresh }), + ) + .await; + assert_eq!( + resp2.status(), + StatusCode::UNAUTHORIZED, + "consumed refresh token must return 401" + ); +} + +/// The rotated refresh token must be usable for a subsequent refresh. +#[tokio::test] +async fn rotated_refresh_token_can_be_used_again() { + let app = build_test_router(test_pool().await); + + let (_access, refresh) = register_user(app.clone(), "helen_rot", "pass_word_1").await; + + // First rotation. + let resp1 = post_json( + app.clone(), + "/api/auth/refresh", + serde_json::json!({ "refresh_token": refresh }), + ) + .await; + assert_eq!(resp1.status(), StatusCode::OK); + let rotated = body_json(resp1).await; + let second_refresh = rotated["refresh_token"].as_str().unwrap().to_string(); + + // Second rotation using the first rotated token. + let resp2 = post_json( + app, + "/api/auth/refresh", + serde_json::json!({ "refresh_token": second_refresh }), + ) + .await; + assert_eq!( + resp2.status(), + StatusCode::OK, + "rotated token must work for a second rotation" + ); + let body2 = body_json(resp2).await; + assert!(body2["access_token"].is_string()); } /// Supplying an access token to `POST /api/auth/refresh` must be rejected because