diff --git a/lib/apps/fabro-server/src/auth/cli_flow.rs b/lib/apps/fabro-server/src/auth/cli_flow.rs index d3242084e..af8598405 100644 --- a/lib/apps/fabro-server/src/auth/cli_flow.rs +++ b/lib/apps/fabro-server/src/auth/cli_flow.rs @@ -28,8 +28,8 @@ use url::{Host, Url}; use crate::auth::browser_shell::browser_shell; use crate::auth::{ - self, AuthCode, AuthErrorCode, AuthSessionRecord, JwtSubject, REFRESH_TOKEN_PREFIX, - RefreshToken, RotateOutcome, + self, AuthCode, AuthErrorCode, AuthSessionRecord, InitialRefreshToken, JwtSubject, + REFRESH_TOKEN_PREFIX, RotateOutcome, }; use crate::jwt_auth::{AuthMode, ConfiguredAuth, bearer_token_from_headers}; use crate::principal_middleware::{ @@ -478,12 +478,10 @@ async fn token( created_at: now, last_used_at: now, }; - let refresh_row = RefreshToken { + let refresh_row = InitialRefreshToken { token_hash: hash_refresh_secret(&refresh_secret), - session_id: session.id, issued_at: now, expires_at: refresh_expires_at, - used_at: None, }; if let Err(err) = state .stores @@ -1187,7 +1185,7 @@ mod tests { CliFlowCookie, DEV_TOKEN_LOGIN_INSTRUCTIONS, add_cli_flow_cookie, read_private_cli_flow, user_agent_fingerprint, web_routes, }; - use crate::auth::{self, AuthCode, AuthErrorCode, AuthSessionRecord, RefreshToken}; + use crate::auth::{self, AuthCode, AuthErrorCode, AuthSessionRecord, InitialRefreshToken}; use crate::jwt_auth::{AuthMode, ConfiguredAuth}; use crate::principal_middleware::{AuthStatus, RequestAuthContext}; use crate::server::AppState; @@ -1353,7 +1351,7 @@ client_id = "github-client-id" Sha256::digest(secret.as_bytes()).into() } - fn session_and_token(secret: &str) -> (AuthSessionRecord, RefreshToken) { + fn session_and_token(secret: &str) -> (AuthSessionRecord, InitialRefreshToken) { let now = chrono::Utc::now(); let session = AuthSessionRecord { id: Uuid::new_v4(), @@ -1367,12 +1365,10 @@ client_id = "github-client-id" created_at: now, last_used_at: now, }; - let token = RefreshToken { + let token = InitialRefreshToken { token_hash: hash_refresh_secret(secret), - session_id: session.id, issued_at: now, expires_at: now + chrono::Duration::days(30), - used_at: None, }; (session, token) } diff --git a/lib/apps/fabro-server/src/auth/mod.rs b/lib/apps/fabro-server/src/auth/mod.rs index da4823f5c..b9b0244b1 100644 --- a/lib/apps/fabro-server/src/auth/mod.rs +++ b/lib/apps/fabro-server/src/auth/mod.rs @@ -31,7 +31,9 @@ pub(crate) const REFRESH_TOKEN_PREFIX: &str = "fabro_refresh_"; pub(crate) use browser_shell::browser_shell; pub(crate) use cli_flow::web_routes; pub(crate) use fabro_store::AuthCode; -pub(crate) use fabro_store::auth_session_store::{AuthSessionRecord, RefreshToken, RotateOutcome}; +pub(crate) use fabro_store::auth_session_store::{ + AuthSessionRecord, InitialRefreshToken, RotateOutcome, +}; pub use github_endpoints::GithubEndpoints; pub(crate) use jwt::{JwtError, JwtSubject, issue, verify}; pub(crate) use keys::{ diff --git a/lib/apps/fabro-server/tests/it/api/auth_sessions.rs b/lib/apps/fabro-server/tests/it/api/auth_sessions.rs index f5b08f662..be8300a23 100644 --- a/lib/apps/fabro-server/tests/it/api/auth_sessions.rs +++ b/lib/apps/fabro-server/tests/it/api/auth_sessions.rs @@ -9,7 +9,7 @@ use fabro_server::jwt_auth::resolve_auth_mode_with_lookup; use fabro_server::server::{AppState, RouterOptions, build_router_with_options}; use fabro_server::test_support::{TEST_SESSION_SECRET, TestAppStateBuilder}; use fabro_server::web_auth::{SESSION_COOKIE_NAME, SessionCookie}; -use fabro_store::auth_session_store::{AuthSessionRecord, RefreshToken}; +use fabro_store::auth_session_store::{AuthSessionRecord, InitialRefreshToken}; use fabro_store::{ArtifactStore, Database}; use hkdf::Hkdf; use object_store::memory::InMemory; @@ -134,18 +134,16 @@ fn cli_session(id: Uuid, identity: fabro_types::IdpIdentity) -> AuthSessionRecor } } -fn refresh_token(hash: [u8; 32], session_id: Uuid) -> RefreshToken { +fn initial_refresh_token(hash: [u8; 32]) -> InitialRefreshToken { let now = chrono::Utc::now(); - RefreshToken { + InitialRefreshToken { token_hash: hash, - session_id, - issued_at: now - chrono::Duration::days(1), + issued_at: now - chrono::Duration::days(1), expires_at: now + chrono::Duration::days(30), - used_at: None, } } -async fn seed_session(state: &AppState, session: AuthSessionRecord, token: RefreshToken) { +async fn seed_session(state: &AppState, session: AuthSessionRecord, token: InitialRefreshToken) { state .test_auth_session_store() .create_session(&session, &token) @@ -196,7 +194,7 @@ async fn active_cli_refresh_token_chains_for_identity_appear_in_unified_list() { seed_session( &state, cli_session(session_id, github_identity()), - refresh_token([1_u8; 32], session_id), + initial_refresh_token([1_u8; 32]), ) .await; @@ -227,33 +225,41 @@ async fn inactive_and_other_identity_cli_tokens_are_excluded() { let now = chrono::Utc::now(); let expired_id = Uuid::new_v4(); - let mut expired = refresh_token([2_u8; 32], expired_id); + let mut expired = initial_refresh_token([2_u8; 32]); expired.expires_at = now - chrono::Duration::seconds(1); - // A chain whose only token has already been rotated away has nothing left - // to spend, so it is inactive even though the token has not expired. - let used_id = Uuid::new_v4(); - let mut used = refresh_token([3_u8; 32], used_id); - used.used_at = Some(now); + // A rotated chain whose successor has expired is inactive even though its + // original token remains as a live-but-spent replay marker. + let spent_id = Uuid::new_v4(); for (session, token) in [ ( cli_session(active_session_id, github_identity()), - refresh_token([1_u8; 32], active_session_id), + initial_refresh_token([1_u8; 32]), ), (cli_session(expired_id, github_identity()), expired), - (cli_session(used_id, github_identity()), used), + ( + cli_session(spent_id, github_identity()), + initial_refresh_token([3_u8; 32]), + ), ( cli_session(Uuid::new_v4(), other_identity()), - refresh_token([4_u8; 32], Uuid::new_v4()), + initial_refresh_token([4_u8; 32]), ), ] { - let token = RefreshToken { - session_id: session.id, - ..token - }; seed_session(&state, session, token).await; } + state + .test_auth_session_store() + .rotate( + &[3_u8; 32], + &[5_u8; 32], + now - chrono::Duration::hours(1), + "fabro-cli/it", + now - chrono::Duration::hours(2), + ) + .await + .expect("rotation should succeed"); let body = get_sessions(app, &session_cookie()).await; let session_ids = body["sessions"] @@ -281,7 +287,7 @@ async fn deleting_cli_session_removes_refresh_token_chain() { seed_session( &state, cli_session(session_id, github_identity()), - refresh_token([1_u8; 32], session_id), + initial_refresh_token([1_u8; 32]), ) .await; // Rotate once so the chain holds a spent token alongside its live one. diff --git a/lib/apps/fabro-server/tests/it/api/cli_auth_token.rs b/lib/apps/fabro-server/tests/it/api/cli_auth_token.rs index b985f85b5..fc6ded632 100644 --- a/lib/apps/fabro-server/tests/it/api/cli_auth_token.rs +++ b/lib/apps/fabro-server/tests/it/api/cli_auth_token.rs @@ -7,7 +7,7 @@ use base64::Engine; use fabro_server::jwt_auth::resolve_auth_mode_with_lookup; use fabro_server::server::{AppState, RouterOptions, build_router_with_options}; use fabro_server::test_support::test_app_state_with_store_and_runtime_settings; -use fabro_store::auth_session_store::{AuthSessionRecord, RefreshToken}; +use fabro_store::auth_session_store::{AuthSessionRecord, InitialRefreshToken}; use fabro_store::{ArtifactStore, AuthCode, Database}; use object_store::memory::InMemory; use sha2::{Digest, Sha256}; @@ -153,12 +153,10 @@ client_id = "Iv1.test" }; state .test_auth_session_store() - .create_session(&session, &RefreshToken { + .create_session(&session, &InitialRefreshToken { token_hash: hash_refresh_secret("integration-refresh"), - session_id: session.id, issued_at: now, expires_at: now + chrono::Duration::days(30), - used_at: None, }) .await .unwrap(); diff --git a/lib/components/fabro-store/src/auth_session_store.rs b/lib/components/fabro-store/src/auth_session_store.rs index 00619bcbd..935a72f94 100644 --- a/lib/components/fabro-store/src/auth_session_store.rs +++ b/lib/components/fabro-store/src/auth_session_store.rs @@ -27,15 +27,22 @@ pub struct AuthSessionRecord { pub last_used_at: DateTime, } -/// One refresh token within a session. `used_at` is set when the token is -/// rotated away; the row is kept until expiry so a replay stays recognisable. +/// Token-specific facts needed to open a session with its first refresh +/// token. The store derives the owning session and unused state. #[derive(Debug, Clone, PartialEq, Eq)] -pub struct RefreshToken { +pub struct InitialRefreshToken { pub token_hash: [u8; 32], - pub session_id: Uuid, pub issued_at: DateTime, pub expires_at: DateTime, - pub used_at: Option>, +} + +/// Complete refresh-token row stored in SQLite. +struct RefreshTokenRecord { + token_hash: [u8; 32], + session_id: Uuid, + issued_at: DateTime, + expires_at: DateTime, + used_at: Option>, } /// A session with a spendable token, as returned by the session listing. @@ -102,7 +109,7 @@ impl AuthSessionStore { pub async fn create_session( &self, session: &AuthSessionRecord, - token: &RefreshToken, + token: &InitialRefreshToken, ) -> Result<()> { let mut tx = self.pool.begin().await?; sqlx::query( @@ -125,7 +132,14 @@ INSERT INTO auth_sessions ( .bind(session.last_used_at.timestamp_millis()) .execute(&mut *tx) .await?; - insert_token(&mut tx, token).await?; + insert_token(&mut tx, &RefreshTokenRecord { + token_hash: token.token_hash, + session_id: session.id, + issued_at: token.issued_at, + expires_at: token.expires_at, + used_at: None, + }) + .await?; tx.commit().await?; Ok(()) } @@ -241,7 +255,7 @@ RETURNING session_id }; let session_id = parse_uuid(&session_id)?; - insert_token(&mut tx, &RefreshToken { + insert_token(&mut tx, &RefreshTokenRecord { token_hash: *new_token_hash, session_id, issued_at: now, @@ -330,7 +344,7 @@ WHERE NOT EXISTS (SELECT 1 FROM refresh_tokens t WHERE t.session_id = auth_sessi async fn insert_token( tx: &mut sqlx::Transaction<'_, sqlx::Sqlite>, - token: &RefreshToken, + token: &RefreshTokenRecord, ) -> Result<()> { sqlx::query( r" @@ -404,7 +418,7 @@ mod tests { use tokio::task::JoinSet; use uuid::Uuid; - use super::{AuthSessionRecord, AuthSessionStore, RefreshToken, RotateOutcome}; + use super::{AuthSessionRecord, AuthSessionStore, InitialRefreshToken, RotateOutcome}; use crate::test_support::sqlite_auth_session_store; fn identity(subject: &str) -> IdpIdentity { @@ -428,14 +442,12 @@ mod tests { /// Tokens are issued an hour back so that a fixture with a negative /// `expires_in` is still a coherent row: issued in the past, expired since. - fn token(hash: [u8; 32], session_id: Uuid, expires_in: Duration) -> RefreshToken { + fn token(hash: [u8; 32], expires_in: Duration) -> InitialRefreshToken { let now = Utc::now(); - RefreshToken { + InitialRefreshToken { token_hash: hash, - session_id, - issued_at: now - Duration::hours(1), + issued_at: now - Duration::hours(1), expires_at: now + expires_in, - used_at: None, } } @@ -447,7 +459,7 @@ mod tests { ) -> Uuid { let id = Uuid::new_v4(); store - .create_session(&session(id, subject), &token(hash, id, expires_in)) + .create_session(&session(id, subject), &token(hash, expires_in)) .await .unwrap(); id @@ -468,6 +480,16 @@ mod tests { assert_eq!(found.login, "octocat"); assert_eq!(found.avatar_url, "https://example.com/octocat.png"); + let (token_session_id, used_at_ms): (String, Option) = sqlx::query_as( + "SELECT session_id, used_at_ms FROM refresh_tokens WHERE token_hash = ?", + ) + .bind([1_u8; 32].as_slice()) + .fetch_one(&store.pool) + .await + .unwrap(); + assert_eq!(token_session_id, id.to_string()); + assert_eq!(used_at_ms, None); + assert!( store .find_session_by_token_hash(&[9_u8; 32]) @@ -866,7 +888,7 @@ END RotateOutcome::Rotated(_) => rotated += 1, RotateOutcome::ReplayedAndRevoked(_) => replayed_and_revoked += 1, RotateOutcome::NotFound => not_found += 1, - other => panic!("unexpected outcome {other:?}"), + other @ RotateOutcome::Expired => panic!("unexpected outcome {other:?}"), } } // SQLite's write lock serialises the claiming UPDATE, so the losers diff --git a/lib/components/fabro-store/src/lib.rs b/lib/components/fabro-store/src/lib.rs index c77ad0c7f..1800592a3 100644 --- a/lib/components/fabro-store/src/lib.rs +++ b/lib/components/fabro-store/src/lib.rs @@ -21,7 +21,7 @@ pub use artifact_store::{ stage_storage_segment, }; pub use auth_session_store::{ - ActiveCliSession, AuthSessionRecord, AuthSessionStore, RefreshToken, RotateOutcome, + ActiveCliSession, AuthSessionRecord, AuthSessionStore, InitialRefreshToken, RotateOutcome, }; pub use blob_store::{Blob, BlobStore}; pub use error::{Error, Result};