From badbd7f874b82af35049541fcf19e443fba9c331 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 21:48:22 +0000 Subject: [PATCH 01/10] feat(terminal): add a join-grants table and backfill existing invite tokens --- migrations/037_terminal_session_grants.sql | 26 ++++++++++++++ src/main.rs | 1 + src/session_grants.rs | 41 ++++++++++++++++++++++ 3 files changed, 68 insertions(+) create mode 100644 migrations/037_terminal_session_grants.sql create mode 100644 src/session_grants.rs diff --git a/migrations/037_terminal_session_grants.sql b/migrations/037_terminal_session_grants.sql new file mode 100644 index 0000000..e9cb54e --- /dev/null +++ b/migrations/037_terminal_session_grants.sql @@ -0,0 +1,26 @@ +CREATE TABLE terminal_session_grants ( + id UUID PRIMARY KEY DEFAULT gen_random_uuid(), + session_id UUID NOT NULL REFERENCES terminal_sessions(id) ON DELETE CASCADE, + kind TEXT NOT NULL CHECK (kind IN ('legacy_token','short_code','guest')), + secret_hash BYTEA NOT NULL, + expires_at TIMESTAMPTZ, + revoked_at TIMESTAMPTZ, + created_by UUID NOT NULL REFERENCES users(id) ON DELETE CASCADE, + redeemed_by UUID REFERENCES users(id) ON DELETE CASCADE, + created_at TIMESTAMPTZ NOT NULL DEFAULT now() +); + +CREATE UNIQUE INDEX idx_tsg_secret ON terminal_session_grants(secret_hash); + +-- One live short code per session. Regeneration revokes the old row inside the +-- same transaction, so this index is what makes the swap race-safe. +CREATE UNIQUE INDEX idx_tsg_one_live_code + ON terminal_session_grants(session_id) + WHERE kind = 'short_code' AND revoked_at IS NULL; + +CREATE INDEX idx_tsg_session ON terminal_session_grants(session_id); + +INSERT INTO terminal_session_grants (session_id, kind, secret_hash, created_by) +SELECT id, 'legacy_token', sha256(convert_to(invite_token, 'UTF8')), host_user_id +FROM terminal_sessions +WHERE invite_token IS NOT NULL AND ended_at IS NULL; diff --git a/src/main.rs b/src/main.rs index 4924a81..9317c9e 100644 --- a/src/main.rs +++ b/src/main.rs @@ -10,6 +10,7 @@ mod permissions; mod rate_limit; mod routes; mod self_host; +mod session_grants; mod sync_notifier; mod terminal_manager; #[cfg(test)] diff --git a/src/session_grants.rs b/src/session_grants.rs new file mode 100644 index 0000000..d592203 --- /dev/null +++ b/src/session_grants.rs @@ -0,0 +1,41 @@ +#[cfg(test)] +mod tests { + use crate::test_pool_or_skip; + use crate::test_support::{seed_team, seed_user}; + use uuid::Uuid; + + #[tokio::test] + async fn backfill_creates_a_legacy_grant_for_a_live_invite_link_session() { + let pool = test_pool_or_skip!(); + + let host = seed_user(&pool).await; + let team = seed_team(&pool, host).await; + // invite_token is UNIQUE; the throwaway DB persists across test runs. + let token = format!("fake-legacy-token-{}", Uuid::new_v4()); + + let session: Uuid = sqlx::query_scalar( + "INSERT INTO terminal_sessions (team_id, host_user_id, connection_name, visibility, invite_token) \ + VALUES ($1, $2, 'box', 'invite_link', $3) RETURNING id", + ) + .bind(team) + .bind(host) + .bind(&token) + .fetch_one(&pool) + .await + .unwrap(); + + // The migration already ran for pre-existing rows; this row is newer, so + // apply the same expression the migration uses to prove it matches. + let matches: bool = sqlx::query_scalar( + "SELECT sha256(convert_to($1, 'UTF8')) = sha256(convert_to(invite_token, 'UTF8')) \ + FROM terminal_sessions WHERE id = $2", + ) + .bind(&token) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + + assert!(matches, "migration hash expression must match the stored token"); + } +} From fc2dcc960f12188aea84e6adc6fb6cb2c13d8a35 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 22:04:55 +0000 Subject: [PATCH 02/10] feat(terminal): generate and normalize Crockford short codes --- src/session_grants.rs | 116 +++++++++++++++++++++++++++++++++++++++++- 1 file changed, 115 insertions(+), 1 deletion(-) diff --git a/src/session_grants.rs b/src/session_grants.rs index d592203..1a24313 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -1,5 +1,58 @@ +use rand::Rng; +use sha2::{Digest, Sha256}; + +/// Crockford base32: digits plus letters with I, L, O and U removed, so a code +/// survives being read aloud and retyped. +const ALPHABET: &[u8; 32] = b"0123456789ABCDEFGHJKMNPQRSTVWXYZ"; + +const CODE_LEN: usize = 10; + +pub fn generate_short_code() -> String { + let mut rng = rand::thread_rng(); + let symbols: Vec = (0..CODE_LEN) + .map(|_| ALPHABET[rng.gen_range(0..ALPHABET.len())] as char) + .collect(); + format!( + "{}-{}-{}", + symbols[0..4].iter().collect::(), + symbols[4..8].iter().collect::(), + symbols[8..10].iter().collect::(), + ) +} + +pub fn normalize_short_code(input: &str) -> Option { + let normalized: String = input + .chars() + .filter(|c| !c.is_whitespace() && *c != '-') + .map(|c| match c.to_ascii_uppercase() { + 'I' | 'L' => '1', + 'O' => '0', + other => other, + }) + .collect(); + + if normalized.len() != CODE_LEN { + return None; + } + if !normalized.bytes().all(|b| ALPHABET.contains(&b)) { + return None; + } + Some(normalized) +} + +pub fn hash_secret(secret: &str) -> Vec { + Sha256::digest(secret.as_bytes()).to_vec() +} + +/// The existing `invite_token` shape, kept identical so redeemed guests can use +/// the unchanged `my-key` and WebSocket query parameters. +pub fn new_token_secret() -> String { + uuid::Uuid::new_v4().to_string().replace('-', "") +} + #[cfg(test)] mod tests { + use super::*; use crate::test_pool_or_skip; use crate::test_support::{seed_team, seed_user}; use uuid::Uuid; @@ -36,6 +89,67 @@ mod tests { .await .unwrap(); - assert!(matches, "migration hash expression must match the stored token"); + assert!( + matches, + "migration hash expression must match the stored token" + ); + } + + #[test] + fn generated_codes_are_dashed_and_ten_symbols() { + let code = generate_short_code(); + assert_eq!(code.len(), 12, "4-4-2 grouping adds two dashes"); + assert_eq!(code.chars().filter(|c| *c == '-').count(), 2); + assert_eq!(normalize_short_code(&code).unwrap().len(), 10); + } + + #[test] + fn generated_codes_avoid_the_ambiguous_letters() { + for _ in 0..500 { + let code = generate_short_code(); + assert!( + !code.contains(['I', 'L', 'O', 'U']), + "Crockford excludes I, L, O and U: {code}" + ); + } + } + + #[test] + fn normalization_folds_spelling_variants_to_one_value() { + let canonical = normalize_short_code("K7M2-P9QX-3B").unwrap(); + for variant in ["k7m2p9qx3b", "K7M2 P9QX 3B", " k7m2-p9qx-3b "] { + assert_eq!(normalize_short_code(variant).unwrap(), canonical); + } + } + + #[test] + fn normalization_maps_the_confusable_letters_onto_digits() { + // A guest who hears "oh" types O; Crockford says that is a zero. + assert_eq!(normalize_short_code("O1IL-2345-67").unwrap(), "0111234567"); + } + + #[test] + fn normalization_rejects_wrong_length_and_foreign_symbols() { + assert!(normalize_short_code("K7M2-P9QX").is_none()); + assert!(normalize_short_code("K7M2-P9QX-3B4").is_none()); + assert!(normalize_short_code("K7M2-P9QX-3$").is_none()); + assert!(normalize_short_code("").is_none()); + } + + #[test] + fn equal_normalized_codes_hash_equal_and_different_ones_do_not() { + let a = hash_secret(&normalize_short_code("k7m2p9qx3b").unwrap()); + let b = hash_secret(&normalize_short_code("K7M2-P9QX-3B").unwrap()); + let c = hash_secret(&normalize_short_code("K7M2-P9QX-3C").unwrap()); + assert_eq!(a, b); + assert_ne!(a, c); + assert_eq!(a.len(), 32); + } + + #[test] + fn token_secrets_are_thirty_two_hex_characters() { + let secret = new_token_secret(); + assert_eq!(secret.len(), 32); + assert!(secret.chars().all(|c| c.is_ascii_hexdigit())); } } From 5d14fe2a1b3425855b83452aab5364505578b534 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 22:13:29 +0000 Subject: [PATCH 03/10] test: add coverage for U in short code normalization rejection --- src/session_grants.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/session_grants.rs b/src/session_grants.rs index 1a24313..7c8dc92 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -133,6 +133,7 @@ mod tests { assert!(normalize_short_code("K7M2-P9QX").is_none()); assert!(normalize_short_code("K7M2-P9QX-3B4").is_none()); assert!(normalize_short_code("K7M2-P9QX-3$").is_none()); + assert!(normalize_short_code("K7M2-P9QU-3B").is_none()); assert!(normalize_short_code("").is_none()); } From c308b9fc3449e3e8d1e73022136c9b75225ca897 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 22:33:44 +0000 Subject: [PATCH 04/10] feat(terminal): resolve, mint and rotate session join grants --- src/routes/terminal.rs | 16 +-- src/session_grants.rs | 264 ++++++++++++++++++++++++++++++++++++++++- src/test_support.rs | 13 ++ 3 files changed, 279 insertions(+), 14 deletions(-) diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index e19df95..2a438fc 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -1918,21 +1918,11 @@ mod tests { use super::*; use crate::rate_limit::RateLimiter; use crate::test_pool_or_skip; - use crate::test_support::{add_member, default_knock_limiter as knocks, seed_team, seed_user}; + use crate::test_support::{ + add_member, default_knock_limiter as knocks, seed_session, seed_team, seed_user, + }; use std::time::Duration; - async fn seed_session(pool: &PgPool, host: Uuid, visibility: &str) -> Uuid { - sqlx::query_scalar::<_, Uuid>( - "INSERT INTO terminal_sessions (host_user_id, connection_name, visibility) \ - VALUES ($1, 'web-prod', $2) RETURNING id", - ) - .bind(host) - .bind(visibility) - .fetch_one(pool) - .await - .expect("insert session") - } - fn harness() -> (crate::sync_notifier::SyncNotifier, TerminalManager) { ( crate::sync_notifier::SyncNotifier::new(), diff --git a/src/session_grants.rs b/src/session_grants.rs index 7c8dc92..1873616 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -1,5 +1,8 @@ +use chrono::{DateTime, Duration, Utc}; use rand::Rng; use sha2::{Digest, Sha256}; +use sqlx::PgPool; +use uuid::Uuid; /// Crockford base32: digits plus letters with I, L, O and U removed, so a code /// survives being read aloud and retyped. @@ -50,13 +53,135 @@ pub fn new_token_secret() -> String { uuid::Uuid::new_v4().to_string().replace('-', "") } +pub const SHORT_CODE_TTL_MINUTES: i64 = 10; + +pub struct Grant { + pub id: Uuid, + pub session_id: Uuid, + pub kind: String, +} + +pub async fn insert_grant( + pool: &PgPool, + session_id: Uuid, + kind: &str, + secret: &str, + expires_at: Option>, + created_by: Uuid, + redeemed_by: Option, +) -> Result<(), sqlx::Error> { + sqlx::query( + "INSERT INTO terminal_session_grants \ + (session_id, kind, secret_hash, expires_at, created_by, redeemed_by) \ + VALUES ($1, $2, $3, $4, $5, $6)", + ) + .bind(session_id) + .bind(kind) + .bind(hash_secret(secret)) + .bind(expires_at) + .bind(created_by) + .bind(redeemed_by) + .execute(pool) + .await + .map(|_| ()) +} + +/// Kind-agnostic: a live grant is a live grant. Short codes never travel this +/// path — they are redeemed at their own endpoint — so the secret is hashed raw. +pub async fn resolve_join_grant(pool: &PgPool, session_id: Uuid, presented: &str) -> Option { + sqlx::query_as::<_, (Uuid, Uuid, String)>( + "SELECT g.id, g.session_id, g.kind \ + FROM terminal_session_grants g \ + JOIN terminal_sessions ts ON ts.id = g.session_id \ + WHERE g.session_id = $1 AND g.secret_hash = $2 \ + AND g.revoked_at IS NULL \ + AND (g.expires_at IS NULL OR g.expires_at > now()) \ + AND ts.ended_at IS NULL", + ) + .bind(session_id) + .bind(hash_secret(presented)) + .fetch_optional(pool) + .await + .ok() + .flatten() + .map(|(id, session_id, kind)| Grant { + id, + session_id, + kind, + }) +} + +pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { + let normalized = normalize_short_code(code)?; + sqlx::query_as::<_, (Uuid, Uuid, String)>( + "SELECT g.id, g.session_id, g.kind \ + FROM terminal_session_grants g \ + JOIN terminal_sessions ts ON ts.id = g.session_id \ + WHERE g.secret_hash = $1 AND g.kind = 'short_code' \ + AND g.revoked_at IS NULL AND g.expires_at > now() \ + AND ts.ended_at IS NULL", + ) + .bind(hash_secret(&normalized)) + .fetch_optional(pool) + .await + .ok() + .flatten() + .map(|(id, session_id, kind)| Grant { + id, + session_id, + kind, + }) +} + +pub async fn rotate_short_code( + pool: &PgPool, + session_id: Uuid, + created_by: Uuid, +) -> Result<(String, DateTime), sqlx::Error> { + let code = generate_short_code(); + let normalized = normalize_short_code(&code).expect("generated codes normalize"); + let expires_at = Utc::now() + Duration::minutes(SHORT_CODE_TTL_MINUTES); + + let mut tx = pool.begin().await?; + // No expiry condition: an expired-but-unrevoked row still occupies the + // partial unique index, so it has to be swept too. + sqlx::query( + "UPDATE terminal_session_grants SET revoked_at = now() \ + WHERE session_id = $1 AND kind = 'short_code' AND revoked_at IS NULL", + ) + .bind(session_id) + .execute(&mut *tx) + .await?; + + sqlx::query( + "INSERT INTO terminal_session_grants \ + (session_id, kind, secret_hash, expires_at, created_by) \ + VALUES ($1, 'short_code', $2, $3, $4)", + ) + .bind(session_id) + .bind(hash_secret(&normalized)) + .bind(expires_at) + .bind(created_by) + .execute(&mut *tx) + .await?; + + tx.commit().await?; + Ok((code, expires_at)) +} + #[cfg(test)] mod tests { use super::*; use crate::test_pool_or_skip; - use crate::test_support::{seed_team, seed_user}; + use crate::test_support::{seed_session, seed_team, seed_user}; use uuid::Uuid; + async fn seed_host_and_session(pool: &sqlx::PgPool) -> (Uuid, Uuid) { + let host = seed_user(pool).await; + let session = seed_session(pool, host, "invite_link").await; + (host, session) + } + #[tokio::test] async fn backfill_creates_a_legacy_grant_for_a_live_invite_link_session() { let pool = test_pool_or_skip!(); @@ -153,4 +278,141 @@ mod tests { assert_eq!(secret.len(), 32); assert!(secret.chars().all(|c| c.is_ascii_hexdigit())); } + + #[tokio::test] + async fn a_live_grant_resolves_and_a_wrong_secret_does_not() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let secret = format!("fake-grant-secret-{}", Uuid::new_v4()); + + insert_grant(&pool, session, "legacy_token", &secret, None, host, None) + .await + .unwrap(); + + assert!(resolve_join_grant(&pool, session, &secret).await.is_some()); + assert!( + resolve_join_grant(&pool, session, "fake-grant-secret-wrong") + .await + .is_none() + ); + } + + #[tokio::test] + async fn expired_revoked_wrong_session_and_ended_grants_all_fail_to_resolve() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let (_, other_session) = seed_host_and_session(&pool).await; + + let expired = format!("fake-grant-secret-{}", Uuid::new_v4()); + let revoked = format!("fake-grant-secret-{}", Uuid::new_v4()); + let live = format!("fake-grant-secret-{}", Uuid::new_v4()); + + insert_grant( + &pool, + session, + "guest", + &expired, + Some(Utc::now() - Duration::minutes(1)), + host, + None, + ) + .await + .unwrap(); + insert_grant(&pool, session, "guest", &revoked, None, host, None) + .await + .unwrap(); + sqlx::query("UPDATE terminal_session_grants SET revoked_at = now() WHERE secret_hash = $1") + .bind(hash_secret(&revoked)) + .execute(&pool) + .await + .unwrap(); + insert_grant(&pool, session, "guest", &live, None, host, None) + .await + .unwrap(); + + assert!(resolve_join_grant(&pool, session, &expired).await.is_none()); + assert!(resolve_join_grant(&pool, session, &revoked).await.is_none()); + assert!( + resolve_join_grant(&pool, other_session, &live) + .await + .is_none(), + "a grant must not resolve against a different session" + ); + + sqlx::query("UPDATE terminal_sessions SET ended_at = now() WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + assert!( + resolve_join_grant(&pool, session, &live).await.is_none(), + "an ended session must admit nobody" + ); + } + + #[tokio::test] + async fn rotating_the_code_revokes_the_previous_one() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + + let (first, _) = rotate_short_code(&pool, session, host).await.unwrap(); + let (second, expires_at) = rotate_short_code(&pool, session, host).await.unwrap(); + + assert!( + resolve_short_code(&pool, &first).await.is_none(), + "regenerating kills the old code" + ); + assert!(resolve_short_code(&pool, &second).await.is_some()); + assert!(expires_at > Utc::now()); + } + + #[tokio::test] + async fn rotation_sweeps_an_expired_but_unrevoked_code() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + + rotate_short_code(&pool, session, host).await.unwrap(); + // Age the live row without revoking it: the partial unique index still + // counts it, so a sweep conditioned on expiry would deadlock the host + // out of ever minting again. + sqlx::query( + "UPDATE terminal_session_grants SET expires_at = now() - interval '1 minute' \ + WHERE session_id = $1 AND kind = 'short_code'", + ) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + let (fresh, _) = rotate_short_code(&pool, session, host).await.unwrap(); + assert!(resolve_short_code(&pool, &fresh).await.is_some()); + + let live: i64 = sqlx::query_scalar( + "SELECT count(*) FROM terminal_session_grants \ + WHERE session_id = $1 AND kind = 'short_code' AND revoked_at IS NULL", + ) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(live, 1, "exactly one live short code per session"); + } + + #[tokio::test] + async fn an_expired_code_does_not_resolve() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + + let (code, _) = rotate_short_code(&pool, session, host).await.unwrap(); + sqlx::query( + "UPDATE terminal_session_grants SET expires_at = now() - interval '1 second' \ + WHERE session_id = $1 AND kind = 'short_code'", + ) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + assert!(resolve_short_code(&pool, &code).await.is_none()); + } } diff --git a/src/test_support.rs b/src/test_support.rs index 684fc1d..0a33b08 100644 --- a/src/test_support.rs +++ b/src/test_support.rs @@ -120,6 +120,19 @@ pub async fn seed_user_with_credentials(pool: &PgPool, account_id: Uuid, auth_ke id } +/// Insert a terminal session hosted by `host` with the given `visibility`. +pub async fn seed_session(pool: &PgPool, host: Uuid, visibility: &str) -> Uuid { + sqlx::query_scalar::<_, Uuid>( + "INSERT INTO terminal_sessions (host_user_id, connection_name, visibility) \ + VALUES ($1, 'web-prod', $2) RETURNING id", + ) + .bind(host) + .bind(visibility) + .fetch_one(pool) + .await + .expect("insert session") +} + /// Insert a team owned by `owner` and return its id. pub async fn seed_team(pool: &PgPool, owner: Uuid) -> Uuid { let id = Uuid::new_v4(); From 76379b2f696adb5ba9a9aa31f335abbf2cc058fb Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 23:06:37 +0000 Subject: [PATCH 05/10] refactor(terminal): resolve invite credentials through join grants --- src/routes/teams.rs | 2 +- src/routes/terminal.rs | 159 ++++++++++++++++++++++++++++++++++++----- 2 files changed, 142 insertions(+), 19 deletions(-) diff --git a/src/routes/teams.rs b/src/routes/teams.rs index 6f144f4..395bdab 100644 --- a/src/routes/teams.rs +++ b/src/routes/teams.rs @@ -1502,7 +1502,7 @@ mod authz_tests { .invitees .clone(); crate::routes::terminal::is_authorized_participant( - pool, user, host, "direct", &[], &[], None, None, &invitees, + pool, session_id, user, host, "direct", &[], &[], None, &invitees, ) .await } diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index 2a438fc..7f32a36 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -595,6 +595,17 @@ pub async fn create_session( StatusCode::INTERNAL_SERVER_ERROR })?; + if let Some(token) = &invite_token { + crate::session_grants::insert_grant( + &pool, session_id, "legacy_token", token, None, auth.0, None, + ) + .await + .map_err(|e| { + error!(error = %e, "Failed to insert legacy join grant"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + } + // Insert vault associations for vault_id in &body.vault_ids { sqlx::query( @@ -929,9 +940,16 @@ pub async fn get_my_session_key( // Invite link session: validate token, return raw key if let Some(token) = &query.invite_token { - let row = sqlx::query_as::<_, (Option, String, Option)>( + if crate::session_grants::resolve_join_grant(&pool, session_id, token) + .await + .is_none() + { + return Err(StatusCode::FORBIDDEN); + } + + let row = sqlx::query_as::<_, (Option, String)>( r#" - SELECT ts.session_key_bytes, u.public_key, ts.invite_token + SELECT ts.session_key_bytes, u.public_key FROM terminal_sessions ts JOIN users u ON u.id = ts.host_user_id WHERE ts.id = $1 AND ts.visibility = 'invite_link' AND ts.ended_at IS NULL @@ -946,12 +964,7 @@ pub async fn get_my_session_key( })? .ok_or(StatusCode::NOT_FOUND)?; - let (session_key_bytes, host_public_key, stored_token) = row; - - if stored_token.as_deref() != Some(token.as_str()) { - return Err(StatusCode::FORBIDDEN); - } - + let (session_key_bytes, host_public_key) = row; let raw_key = session_key_bytes.ok_or(StatusCode::INTERNAL_SERVER_ERROR)?; return Ok(Json(SessionKeyResponse { wrapped_key: None, @@ -1232,12 +1245,12 @@ pub async fn ws_handler( #[allow(clippy::too_many_arguments)] pub(crate) async fn is_authorized_participant( pool: &PgPool, + session_id: Uuid, user_id: Uuid, host_user_id: Uuid, visibility: &str, vault_ids: &[Uuid], allowed_roles: &[String], - stored_token: Option<&str>, presented_token: Option<&str>, invitees: &std::collections::HashSet, ) -> bool { @@ -1248,8 +1261,12 @@ pub(crate) async fn is_authorized_participant( return true; } if visibility == "invite_link" { - // Invite link: validate token - return presented_token.is_some() && presented_token == stored_token; + let Some(presented) = presented_token else { + return false; + }; + return crate::session_grants::resolve_join_grant(pool, session_id, presented) + .await + .is_some(); } // Vault session: user must be a member of one of the session's vaults, // satisfy the role filter (if any), and have JOIN_TERMINAL_SESSION permission. @@ -1328,7 +1345,6 @@ async fn handle_socket( s.vault_ids.clone(), s.visibility.clone(), s.allowed_roles.clone(), - s.invite_token.clone(), s.host_user_id, s.vault_owner_id, s.invitees.clone(), @@ -1340,7 +1356,6 @@ async fn handle_socket( vault_ids, visibility, allowed_roles, - stored_token, host_user_id, vault_owner_id, invitees, @@ -1354,12 +1369,12 @@ async fn handle_socket( let authorized = is_authorized_participant( &pool, + session_id, user_id, host_user_id, &visibility, &vault_ids, &allowed_roles, - stored_token.as_deref(), invite_token.as_deref(), &invitees, ) @@ -1911,6 +1926,43 @@ mod authz_tests { assert_eq!(res.unwrap_err(), axum::http::StatusCode::FORBIDDEN); } + + #[tokio::test] + async fn creating_an_invite_link_session_also_mints_a_legacy_grant() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + + let res = create_session( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(claims_for(host)), + Extension(TerminalManager::new()), + Extension(SyncNotifier::new()), + Extension(knocks()), + Json(session_request(Vec::new(), "invite_link", Vec::new())), + ) + .await + .expect("invite_link session creates"); + + let (_, Json(body)) = res; + let token = body.invite_token.expect("invite_link sessions carry a token"); + + let grant_kind: String = sqlx::query_scalar( + "SELECT kind FROM terminal_session_grants WHERE session_id = $1", + ) + .bind(body.session_id) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(grant_kind, "legacy_token"); + + assert!( + crate::session_grants::resolve_join_grant(&pool, body.session_id, &token) + .await + .is_some(), + "the token an old client already holds must keep resolving" + ); + } } #[cfg(test)] @@ -2131,9 +2183,9 @@ mod tests { .unwrap(); let invitees = manager.sessions.lock().await.get(&session_id).unwrap().invitees.clone(); - assert!(is_authorized_participant(&pool, mate, host, "direct", &[], &[], None, None, &invitees).await); + assert!(is_authorized_participant(&pool, session_id, mate, host, "direct", &[], &[], None, &invitees).await); let stranger = seed_user(&pool).await; - assert!(!is_authorized_participant(&pool, stranger, host, "direct", &[], &[], None, None, &invitees).await); + assert!(!is_authorized_participant(&pool, session_id, stranger, host, "direct", &[], &[], None, &invitees).await); } #[tokio::test] @@ -2591,7 +2643,7 @@ mod tests { let invitees = manager.sessions.lock().await.get(&session_id).unwrap().invitees.clone(); assert!( - !is_authorized_participant(&pool, stranger, host, "direct", &[], &[], None, None, &invitees).await, + !is_authorized_participant(&pool, session_id, stranger, host, "direct", &[], &[], None, &invitees).await, "the suppressed row must not admit the WebSocket" ); } @@ -2691,7 +2743,7 @@ mod tests { let invitees = manager.sessions.lock().await.get(&session_id).map(|s| s.invitees.clone()).unwrap_or_default(); // Asserted through the admission function, not a row count: a DB-only // revoke left live WebSocket access open — the Critical from #66. - assert!(!is_authorized_participant(&pool, stranger, host, "direct", &[], &[], None, None, &invitees).await); + assert!(!is_authorized_participant(&pool, session_id, stranger, host, "direct", &[], &[], None, &invitees).await); let keys: i64 = sqlx::query_scalar( "SELECT count(*) FROM terminal_session_keys WHERE session_id = $1 AND user_id = $2") @@ -2956,4 +3008,75 @@ mod tests { // The alias pre-0.26 clients read. Deleted in 0.27. assert_eq!(json["display_name"], "merry-quartz-2597"); } + + #[tokio::test] + async fn a_guest_grant_authorizes_the_key_endpoint_and_the_ws_upgrade() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let guest = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + let secret = format!("fake-grant-secret-{}", Uuid::new_v4()); + + sqlx::query("UPDATE terminal_sessions SET session_key_bytes = 'fake-key-bytes' WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + crate::session_grants::insert_grant( + &pool, session, "guest", &secret, None, host, Some(guest), + ) + .await + .unwrap(); + + let key = get_my_session_key( + State(pool.clone()), + Extension(AuthUser(guest)), + axum::extract::Path(session), + axum::extract::Query(GetKeyQuery { + invite_token: Some(secret.clone()), + }), + ) + .await + .expect("a guest grant unlocks the raw key"); + assert!(key.0.raw_key.is_some()); + + assert!( + is_authorized_participant( + &pool, session, guest, host, "invite_link", &[], &[], + Some(secret.as_str()), + &std::collections::HashSet::new(), + ) + .await + ); + } + + #[tokio::test] + async fn a_revoked_guest_grant_stops_authorizing() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let guest = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + let secret = format!("fake-grant-secret-{}", Uuid::new_v4()); + + crate::session_grants::insert_grant( + &pool, session, "guest", &secret, None, host, Some(guest), + ) + .await + .unwrap(); + sqlx::query("UPDATE terminal_session_grants SET revoked_at = now() WHERE session_id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + + assert!( + !is_authorized_participant( + &pool, session, guest, host, "invite_link", &[], &[], + Some(secret.as_str()), + &std::collections::HashSet::new(), + ) + .await, + "revoking one guest's grant must lock that guest out" + ); + } } From febf86647bbbe3f9d1880e9d644ad7c931197a17 Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 23:16:06 +0000 Subject: [PATCH 06/10] fix(terminal): atomic legacy grant insert, correct 404/403 order, per-guest revoke proof --- src/routes/terminal.rs | 67 +++++++++++++++++++++++++++++++----------- src/session_grants.rs | 48 +++++++++++++++++++++++++++--- 2 files changed, 94 insertions(+), 21 deletions(-) diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index 7f32a36..f59cb46 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -570,12 +570,19 @@ pub async fn create_session( // Generate invite token for invite_link sessions let invite_token: Option = if visibility == "invite_link" { - Some(Uuid::new_v4().to_string().replace('-', "")) + Some(crate::session_grants::new_token_secret()) } else { None }; - // Insert session record + // Insert session record and its legacy join grant together: if the grant + // insert fails, the session row must not survive carrying a token that + // can never resolve. + let mut tx = pool.begin().await.map_err(|e| { + error!(error = %e, "Failed to start session creation transaction"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + let session_id = sqlx::query_scalar::<_, Uuid>( r#"INSERT INTO terminal_sessions (host_user_id, connection_name, visibility, session_key_bytes, allowed_roles, invite_token) @@ -588,7 +595,7 @@ pub async fn create_session( .bind(&body.session_key_bytes) .bind(&body.allowed_roles) .bind(&invite_token) - .fetch_one(&pool) + .fetch_one(&mut *tx) .await .map_err(|e| { error!(error = %e, "Failed to insert terminal session"); @@ -597,7 +604,7 @@ pub async fn create_session( if let Some(token) = &invite_token { crate::session_grants::insert_grant( - &pool, session_id, "legacy_token", token, None, auth.0, None, + &mut *tx, session_id, "legacy_token", token, None, auth.0, None, ) .await .map_err(|e| { @@ -606,6 +613,11 @@ pub async fn create_session( })?; } + tx.commit().await.map_err(|e| { + error!(error = %e, "Failed to commit session creation transaction"); + StatusCode::INTERNAL_SERVER_ERROR + })?; + // Insert vault associations for vault_id in &body.vault_ids { sqlx::query( @@ -938,15 +950,10 @@ pub async fn get_my_session_key( })); } - // Invite link session: validate token, return raw key + // Invite link session: session must exist first (404), then the token + // must resolve to a live grant (403) — preserves the pre-grant status + // code contract for unknown/ended sessions vs. a bad credential. if let Some(token) = &query.invite_token { - if crate::session_grants::resolve_join_grant(&pool, session_id, token) - .await - .is_none() - { - return Err(StatusCode::FORBIDDEN); - } - let row = sqlx::query_as::<_, (Option, String)>( r#" SELECT ts.session_key_bytes, u.public_key @@ -964,6 +971,13 @@ pub async fn get_my_session_key( })? .ok_or(StatusCode::NOT_FOUND)?; + if crate::session_grants::resolve_join_grant(&pool, session_id, token) + .await + .is_none() + { + return Err(StatusCode::FORBIDDEN); + } + let (session_key_bytes, host_public_key) = row; let raw_key = session_key_bytes.ok_or(StatusCode::INTERNAL_SERVER_ERROR)?; return Ok(Json(SessionKeyResponse { @@ -3055,16 +3069,26 @@ mod tests { let pool = test_pool_or_skip!(); let host = seed_user(&pool).await; let guest = seed_user(&pool).await; + let other_guest = seed_user(&pool).await; let session = seed_session(&pool, host, "invite_link").await; - let secret = format!("fake-grant-secret-{}", Uuid::new_v4()); + let revoked_secret = format!("fake-grant-secret-{}", Uuid::new_v4()); + let live_secret = format!("fake-grant-secret-{}", Uuid::new_v4()); crate::session_grants::insert_grant( - &pool, session, "guest", &secret, None, host, Some(guest), + &pool, session, "guest", &revoked_secret, None, host, Some(guest), ) .await .unwrap(); - sqlx::query("UPDATE terminal_session_grants SET revoked_at = now() WHERE session_id = $1") - .bind(session) + crate::session_grants::insert_grant( + &pool, session, "guest", &live_secret, None, host, Some(other_guest), + ) + .await + .unwrap(); + + // Revoke by secret_hash, not by session: proves the per-guest grant + // this design exists for, not "revoking the session locks everyone out". + sqlx::query("UPDATE terminal_session_grants SET revoked_at = now() WHERE secret_hash = $1") + .bind(crate::session_grants::hash_secret(&revoked_secret)) .execute(&pool) .await .unwrap(); @@ -3072,11 +3096,20 @@ mod tests { assert!( !is_authorized_participant( &pool, session, guest, host, "invite_link", &[], &[], - Some(secret.as_str()), + Some(revoked_secret.as_str()), &std::collections::HashSet::new(), ) .await, "revoking one guest's grant must lock that guest out" ); + assert!( + is_authorized_participant( + &pool, session, other_guest, host, "invite_link", &[], &[], + Some(live_secret.as_str()), + &std::collections::HashSet::new(), + ) + .await, + "a different guest's grant on the same session must keep working" + ); } } diff --git a/src/session_grants.rs b/src/session_grants.rs index 1873616..112e266 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -61,15 +61,21 @@ pub struct Grant { pub kind: String, } -pub async fn insert_grant( - pool: &PgPool, +/// Generic over the executor so a caller can run this inside its own +/// transaction (e.g. alongside the session INSERT it must not outlive) or +/// just pass a `&PgPool` for a standalone grant. +pub async fn insert_grant<'c, E>( + executor: E, session_id: Uuid, kind: &str, secret: &str, expires_at: Option>, created_by: Uuid, redeemed_by: Option, -) -> Result<(), sqlx::Error> { +) -> Result<(), sqlx::Error> +where + E: sqlx::Executor<'c, Database = sqlx::Postgres>, +{ sqlx::query( "INSERT INTO terminal_session_grants \ (session_id, kind, secret_hash, expires_at, created_by, redeemed_by) \ @@ -81,7 +87,7 @@ pub async fn insert_grant( .bind(expires_at) .bind(created_by) .bind(redeemed_by) - .execute(pool) + .execute(executor) .await .map(|_| ()) } @@ -220,6 +226,40 @@ mod tests { ); } + /// The deploy-day case: an already-live session whose grant only exists + /// because the migration backfilled it, hashing in SQL — not through + /// `insert_grant`/`hash_secret`. Proves the two hashing paths agree; if + /// they didn't, every mid-session guest would be locked out at deploy. + #[tokio::test] + async fn a_migration_backfilled_grant_resolves_the_pre_existing_token() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + let token = format!("fake-legacy-token-{}", Uuid::new_v4()); + + sqlx::query("UPDATE terminal_sessions SET invite_token = $1 WHERE id = $2") + .bind(&token) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + sqlx::query( + "INSERT INTO terminal_session_grants (session_id, kind, secret_hash, created_by) \ + SELECT id, 'legacy_token', sha256(convert_to(invite_token, 'UTF8')), host_user_id \ + FROM terminal_sessions WHERE id = $1", + ) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + assert!( + resolve_join_grant(&pool, session, &token).await.is_some(), + "SQL-side and Rust-side hashing must agree on deploy day" + ); + } + #[test] fn generated_codes_are_dashed_and_ten_symbols() { let code = generate_short_code(); From cab1b083fa90a8b8f5c2093a66f35c1d95ee2fea Mon Sep 17 00:00:00 2001 From: kipavy Date: Sun, 16 Aug 2026 23:42:14 +0000 Subject: [PATCH 07/10] feat(terminal): mint and redeem short-lived session join codes --- src/main.rs | 20 ++- src/rate_limit.rs | 45 +++--- src/routes/mod.rs | 1 + src/routes/session_codes.rs | 297 ++++++++++++++++++++++++++++++++++++ src/routes/terminal.rs | 2 +- 5 files changed, 345 insertions(+), 20 deletions(-) create mode 100644 src/routes/session_codes.rs diff --git a/src/main.rs b/src/main.rs index 9317c9e..b618be5 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,8 +23,8 @@ use axum::{ }; use dashmap::{DashMap, DashSet}; use rate_limit::{ - InviteRateLimiter, KnockRateLimiter, RateLimiter, RegisterRateLimiter, SearchRateLimiter, - SyncRateLimiter, WaitlistRateLimiter, + InviteRateLimiter, KnockRateLimiter, RateLimiter, RedeemRateLimiter, RegisterRateLimiter, + SearchRateLimiter, SessionCodeRateLimiter, SyncRateLimiter, WaitlistRateLimiter, }; use routes::audit::AuditClientRateLimiter; use std::net::SocketAddr; @@ -180,6 +180,10 @@ async fn main() { SearchRateLimiter(RateLimiter::::new(search_rate, Duration::from_secs(60))); let knock_limiter = KnockRateLimiter(RateLimiter::::new(knock_per_hour, Duration::from_secs(3600))); + let session_code_limiter = + SessionCodeRateLimiter(RateLimiter::::new(30, Duration::from_secs(3600))); + let redeem_limiter = + RedeemRateLimiter(RateLimiter::::new(20, Duration::from_secs(3600))); // Lemon Squeezy live metrics cache (background refresh every 5 min). let ls_cache = lemonsqueezy::LsCache::default(); @@ -441,6 +445,16 @@ async fn main() { "/v1/terminal-sessions", post(routes::terminal::create_session), ) + // `redeem` is a literal segment registered before the `:id` routes, for + // the same reason the literal `me` invitee route is. + .route( + "/v1/terminal-sessions/redeem", + post(routes::session_codes::redeem_code), + ) + .route( + "/v1/terminal-sessions/:id/code", + post(routes::session_codes::create_code), + ) .route( "/v1/terminal-sessions/:id/my-key", get(routes::terminal::get_my_session_key), @@ -494,6 +508,8 @@ async fn main() { .layer(Extension(sync_limiter)) .layer(Extension(search_limiter)) .layer(Extension(knock_limiter)) + .layer(Extension(session_code_limiter)) + .layer(Extension(redeem_limiter)) .layer(middleware::from_fn(auth::auth_middleware)) .layer(Extension(notifier.clone())) .layer(Extension(terminal_manager.clone())) diff --git a/src/rate_limit.rs b/src/rate_limit.rs index bf82007..e7c15ab 100644 --- a/src/rate_limit.rs +++ b/src/rate_limit.rs @@ -1,9 +1,4 @@ -use axum::{ - extract::Request, - http::StatusCode, - middleware::Next, - response::Response, -}; +use axum::{extract::Request, http::StatusCode, middleware::Next, response::Response}; use std::collections::HashMap; use std::hash::Hash; use std::net::IpAddr; @@ -104,6 +99,15 @@ pub struct SearchRateLimiter(pub RateLimiter); #[derive(Clone)] pub struct KnockRateLimiter(pub RateLimiter); +/// Short-code mints per host. Regenerate-spam must not become a mint oracle. +#[derive(Clone)] +pub struct SessionCodeRateLimiter(pub RateLimiter); + +/// Code redemptions per user. Keyed by user because the endpoint is +/// authenticated, so brute force costs accounts, not just addresses. +#[derive(Clone)] +pub struct RedeemRateLimiter(pub RateLimiter); + /// Register endpoint: N registrations/day per IP. pub async fn register_rate_limit( axum::Extension(RegisterRateLimiter(limiter)): axum::Extension, @@ -118,6 +122,22 @@ pub async fn register_rate_limit( Ok(next.run(req).await) } +/// Shared body for the per-user middlewares below: check the limiter keyed on +/// the authenticated caller, or reject with 429. +async fn user_keyed_limit( + limiter: &RateLimiter, + user: Uuid, + label: &str, + req: Request, + next: Next, +) -> Result { + if !limiter.check(user).await { + warn!(user_id = %user, limiter = label, "Rate limit exceeded"); + return Err(StatusCode::TOO_MANY_REQUESTS); + } + Ok(next.run(req).await) +} + /// Invite endpoint: N invitations/hour per user (auth_middleware must run first). pub async fn invite_rate_limit( axum::Extension(InviteRateLimiter(limiter)): axum::Extension, @@ -125,11 +145,7 @@ pub async fn invite_rate_limit( req: Request, next: Next, ) -> Result { - if !limiter.check(auth.0).await { - warn!(user_id = %auth.0, "Invite rate limit exceeded"); - return Err(StatusCode::TOO_MANY_REQUESTS); - } - Ok(next.run(req).await) + user_keyed_limit(&limiter, auth.0, "invite", req, next).await } /// Sync endpoints: N requests/hour per user (auth_middleware must run first). @@ -139,11 +155,7 @@ pub async fn sync_rate_limit( req: Request, next: Next, ) -> Result { - if !limiter.check(auth.0).await { - warn!(user_id = %auth.0, "Sync rate limit exceeded"); - return Err(StatusCode::TOO_MANY_REQUESTS); - } - Ok(next.run(req).await) + user_keyed_limit(&limiter, auth.0, "sync", req, next).await } /// Auth endpoints: 10 requests/minute per IP. @@ -175,4 +187,3 @@ pub async fn waitlist_rate_limit( } Ok(next.run(req).await) } - diff --git a/src/routes/mod.rs b/src/routes/mod.rs index e155632..f828ee8 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -5,6 +5,7 @@ pub mod billing; pub mod invitations; pub mod meta; pub mod presence; +pub mod session_codes; pub mod sync; pub mod team_sync; pub mod team_objects; diff --git a/src/routes/session_codes.rs b/src/routes/session_codes.rs new file mode 100644 index 0000000..31dce59 --- /dev/null +++ b/src/routes/session_codes.rs @@ -0,0 +1,297 @@ +use axum::extract::{Path, State}; +use axum::http::StatusCode; +use axum::{Extension, Json}; +use chrono::{DateTime, Utc}; +use serde::{Deserialize, Serialize}; +use sqlx::PgPool; +use tracing::warn; +use uuid::Uuid; + +use crate::auth::AuthUser; +use crate::rate_limit::{RedeemRateLimiter, SessionCodeRateLimiter}; +use crate::session_grants; + +#[derive(Debug, Serialize)] +pub struct CreateCodeResponse { + pub code: String, + pub expires_at: DateTime, +} + +#[derive(Deserialize)] +pub struct RedeemRequest { + pub code: String, +} + +#[derive(Debug, Serialize)] +pub struct RedeemResponse { + pub session_id: Uuid, + pub invite_token: String, +} + +pub async fn create_code( + State(pool): State, + Extension(auth): Extension, + Extension(SessionCodeRateLimiter(limiter)): Extension, + Path(session_id): Path, +) -> Result<(StatusCode, Json), StatusCode> { + if !limiter.check(auth.0).await { + return Err(StatusCode::TOO_MANY_REQUESTS); + } + crate::routes::terminal::require_active_session_host(&pool, session_id, auth.0).await?; + + // Only invite_link sessions serve raw keys through the short-code/grant + // path (get_my_session_key, is_authorized_participant); minting for a + // vault or direct session would hand out a grant nothing can redeem it into. + let visibility: String = + sqlx::query_scalar("SELECT visibility FROM terminal_sessions WHERE id = $1") + .bind(session_id) + .fetch_one(&pool) + .await + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + if visibility != "invite_link" { + return Err(StatusCode::FORBIDDEN); + } + + let (code, expires_at) = session_grants::rotate_short_code(&pool, session_id, auth.0) + .await + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + Ok(( + StatusCode::CREATED, + Json(CreateCodeResponse { code, expires_at }), + )) +} + +pub async fn redeem_code( + State(pool): State, + Extension(auth): Extension, + Extension(RedeemRateLimiter(limiter)): Extension, + Json(body): Json, +) -> Result, StatusCode> { + if !limiter.check(auth.0).await { + return Err(StatusCode::TOO_MANY_REQUESTS); + } + + // Unknown, malformed, expired and revoked all answer 404: no response + // distinguishes a real code from a wrong one. + let Some(grant) = session_grants::resolve_short_code(&pool, &body.code).await else { + warn!(user_id = %auth.0, "Short code redemption failed"); + return Err(StatusCode::NOT_FOUND); + }; + + let secret = session_grants::new_token_secret(); + session_grants::insert_grant( + &pool, + grant.session_id, + "guest", + &secret, + None, + auth.0, + Some(auth.0), + ) + .await + .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; + + Ok(Json(RedeemResponse { + session_id: grant.session_id, + invite_token: secret, + })) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::rate_limit::{RateLimiter, RedeemRateLimiter, SessionCodeRateLimiter}; + use crate::test_pool_or_skip; + use crate::test_support::{seed_session, seed_user}; + use axum::extract::{Path, State}; + use axum::{Extension, Json}; + use std::time::Duration; + use uuid::Uuid; + + fn code_budget() -> SessionCodeRateLimiter { + SessionCodeRateLimiter(RateLimiter::new(30, Duration::from_secs(3600))) + } + + fn redeem_budget() -> RedeemRateLimiter { + RedeemRateLimiter(RateLimiter::new(20, Duration::from_secs(3600))) + } + + async fn seed_host_and_session(pool: &sqlx::PgPool) -> (Uuid, Uuid) { + let host = seed_user(pool).await; + let session = seed_session(pool, host, "invite_link").await; + (host, session) + } + + #[tokio::test] + async fn only_the_host_can_mint_a_code() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let (stranger, _) = seed_host_and_session(&pool).await; + + assert!(create_code( + State(pool.clone()), + Extension(AuthUser(stranger)), + Extension(code_budget()), + Path(session), + ) + .await + .is_err()); + + assert!(create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .is_ok()); + } + + #[tokio::test] + async fn only_invite_link_sessions_can_mint_a_code() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let vault_session = seed_session(&pool, host, "vault").await; + let direct_session = seed_session(&pool, host, "direct").await; + + for session in [vault_session, direct_session] { + let err = create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap_err(); + assert_eq!(err, StatusCode::FORBIDDEN); + } + } + + #[tokio::test] + async fn redeeming_returns_a_working_guest_secret() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let (guest, _) = seed_host_and_session(&pool).await; + + let (_, Json(minted)) = create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap(); + + let Json(redeemed) = redeem_code( + State(pool.clone()), + Extension(AuthUser(guest)), + Extension(redeem_budget()), + Json(RedeemRequest { + code: minted.code.clone(), + }), + ) + .await + .unwrap(); + + assert_eq!(redeemed.session_id, session); + assert!( + crate::session_grants::resolve_join_grant(&pool, session, &redeemed.invite_token) + .await + .is_some() + ); + } + + #[tokio::test] + async fn one_code_admits_several_guests_until_it_expires() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let (first, _) = seed_host_and_session(&pool).await; + let (second, _) = seed_host_and_session(&pool).await; + + let (_, Json(minted)) = create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap(); + + for guest in [first, second] { + assert!(redeem_code( + State(pool.clone()), + Extension(AuthUser(guest)), + Extension(redeem_budget()), + Json(RedeemRequest { + code: minted.code.clone() + }), + ) + .await + .is_ok()); + } + + let guests: i64 = sqlx::query_scalar( + "SELECT count(*) FROM terminal_session_grants WHERE session_id = $1 AND kind = 'guest'", + ) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(guests, 2, "each redemption gets its own revocable grant"); + } + + #[tokio::test] + async fn unknown_and_malformed_codes_are_indistinguishable() { + let pool = test_pool_or_skip!(); + let (guest, _) = seed_host_and_session(&pool).await; + + for candidate in ["K7M2-P9QX-3B", "nonsense"] { + let err = redeem_code( + State(pool.clone()), + Extension(AuthUser(guest)), + Extension(redeem_budget()), + Json(RedeemRequest { + code: candidate.to_string(), + }), + ) + .await + .unwrap_err(); + assert_eq!(err, axum::http::StatusCode::NOT_FOUND); + } + } + + #[tokio::test] + async fn exhausted_budgets_return_too_many_requests() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + + let exhausted_mint = SessionCodeRateLimiter(RateLimiter::new(0, Duration::from_secs(3600))); + assert_eq!( + create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(exhausted_mint), + Path(session) + ) + .await + .unwrap_err(), + axum::http::StatusCode::TOO_MANY_REQUESTS + ); + + let exhausted_redeem = RedeemRateLimiter(RateLimiter::new(0, Duration::from_secs(3600))); + assert_eq!( + redeem_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(exhausted_redeem), + Json(RedeemRequest { + code: "K7M2-P9QX-3B".to_string() + }), + ) + .await + .unwrap_err(), + axum::http::StatusCode::TOO_MANY_REQUESTS + ); + } +} diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index f59cb46..5a64fc7 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -994,7 +994,7 @@ pub async fn get_my_session_key( /// status the caller should return: `NOT_FOUND` if the session doesn't exist /// or has already ended, `FORBIDDEN` if `caller` isn't its host. Shared by /// `end_session` and `invite_to_session` — both gate on exactly this check. -async fn require_active_session_host( +pub(crate) async fn require_active_session_host( pool: &PgPool, session_id: Uuid, caller: Uuid, From df09f5a6c98a1066f20e38d5f9d710f6c5ab326d Mon Sep 17 00:00:00 2001 From: kipavy Date: Mon, 17 Aug 2026 00:00:43 +0000 Subject: [PATCH 08/10] fix(terminal): drop dead grant/session-state fields, harden code mint edge cases - Grant no longer carries id/kind: nothing read them, and both were only ever written, tripping clippy dead_code. - SessionState.invite_token: its last reader was removed with legacy-token grants; dead in-memory state must not linger next to the real source of truth in terminal_session_grants. - create_code: fetch_optional the visibility lookup so a session ending between the host check and this query 404s instead of 500ing. - comment fixes: correct the redeem-route-ordering rationale (matchit prefers static segments regardless of order), note why the guest grant has no expiry. --- src/main.rs | 5 +++-- src/routes/session_codes.rs | 33 +++++++++++++++++++++++++++++---- src/routes/terminal.rs | 1 - src/session_grants.rs | 22 ++++++---------------- src/terminal_manager.rs | 3 --- 5 files changed, 38 insertions(+), 26 deletions(-) diff --git a/src/main.rs b/src/main.rs index b618be5..c6e1a70 100644 --- a/src/main.rs +++ b/src/main.rs @@ -445,8 +445,9 @@ async fn main() { "/v1/terminal-sessions", post(routes::terminal::create_session), ) - // `redeem` is a literal segment registered before the `:id` routes, for - // the same reason the literal `me` invitee route is. + // `redeem` is listed before the `:id` routes by convention, matching + // the literal `me` invitee route; axum's matchit prefers a static + // segment over a param regardless of registration order. .route( "/v1/terminal-sessions/redeem", post(routes::session_codes::redeem_code), diff --git a/src/routes/session_codes.rs b/src/routes/session_codes.rs index 31dce59..9457daf 100644 --- a/src/routes/session_codes.rs +++ b/src/routes/session_codes.rs @@ -42,14 +42,16 @@ pub async fn create_code( // Only invite_link sessions serve raw keys through the short-code/grant // path (get_my_session_key, is_authorized_participant); minting for a // vault or direct session would hand out a grant nothing can redeem it into. - let visibility: String = + let visibility: Option = sqlx::query_scalar("SELECT visibility FROM terminal_sessions WHERE id = $1") .bind(session_id) - .fetch_one(&pool) + .fetch_optional(&pool) .await .map_err(|_| StatusCode::INTERNAL_SERVER_ERROR)?; - if visibility != "invite_link" { - return Err(StatusCode::FORBIDDEN); + match visibility { + Some(v) if v == "invite_link" => {} + Some(_) => return Err(StatusCode::FORBIDDEN), + None => return Err(StatusCode::NOT_FOUND), } let (code, expires_at) = session_grants::rotate_short_code(&pool, session_id, auth.0) @@ -80,6 +82,8 @@ pub async fn redeem_code( }; let secret = session_grants::new_token_secret(); + // expires_at: None — the guest grant outlives the 10-minute code; it ends + // only by revoke or session end, same as any other invited participant. session_grants::insert_grant( &pool, grant.session_id, @@ -148,6 +152,27 @@ mod tests { .is_ok()); } + #[tokio::test] + async fn minting_for_an_ended_session_is_not_found_not_a_crash() { + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + sqlx::query("UPDATE terminal_sessions SET ended_at = now() WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + + let err = create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap_err(); + assert_eq!(err, StatusCode::NOT_FOUND); + } + #[tokio::test] async fn only_invite_link_sessions_can_mint_a_code() { let pool = test_pool_or_skip!(); diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index 5a64fc7..af2bea5 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -677,7 +677,6 @@ pub async fn create_session( crate::terminal_manager::SessionState { vault_ids: body.vault_ids.clone(), allowed_roles: body.allowed_roles.clone(), - invite_token: invite_token.clone(), invitees: std::collections::HashSet::new(), host_user_id: auth.0, host_public_key, diff --git a/src/session_grants.rs b/src/session_grants.rs index 112e266..55a2686 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -56,9 +56,7 @@ pub fn new_token_secret() -> String { pub const SHORT_CODE_TTL_MINUTES: i64 = 10; pub struct Grant { - pub id: Uuid, pub session_id: Uuid, - pub kind: String, } /// Generic over the executor so a caller can run this inside its own @@ -95,8 +93,8 @@ where /// Kind-agnostic: a live grant is a live grant. Short codes never travel this /// path — they are redeemed at their own endpoint — so the secret is hashed raw. pub async fn resolve_join_grant(pool: &PgPool, session_id: Uuid, presented: &str) -> Option { - sqlx::query_as::<_, (Uuid, Uuid, String)>( - "SELECT g.id, g.session_id, g.kind \ + sqlx::query_as::<_, (Uuid,)>( + "SELECT g.session_id \ FROM terminal_session_grants g \ JOIN terminal_sessions ts ON ts.id = g.session_id \ WHERE g.session_id = $1 AND g.secret_hash = $2 \ @@ -110,17 +108,13 @@ pub async fn resolve_join_grant(pool: &PgPool, session_id: Uuid, presented: &str .await .ok() .flatten() - .map(|(id, session_id, kind)| Grant { - id, - session_id, - kind, - }) + .map(|(session_id,)| Grant { session_id }) } pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { let normalized = normalize_short_code(code)?; - sqlx::query_as::<_, (Uuid, Uuid, String)>( - "SELECT g.id, g.session_id, g.kind \ + sqlx::query_as::<_, (Uuid,)>( + "SELECT g.session_id \ FROM terminal_session_grants g \ JOIN terminal_sessions ts ON ts.id = g.session_id \ WHERE g.secret_hash = $1 AND g.kind = 'short_code' \ @@ -132,11 +126,7 @@ pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { .await .ok() .flatten() - .map(|(id, session_id, kind)| Grant { - id, - session_id, - kind, - }) + .map(|(session_id,)| Grant { session_id }) } pub async fn rotate_short_code( diff --git a/src/terminal_manager.rs b/src/terminal_manager.rs index eaeb26d..1c5563c 100644 --- a/src/terminal_manager.rs +++ b/src/terminal_manager.rs @@ -32,8 +32,6 @@ pub struct SessionState { pub vault_ids: Vec, /// Role filter — empty means all roles; non-empty means only these roles can join pub allowed_roles: Vec, - /// Invite token — set for invite_link sessions; required to join/get key - pub invite_token: Option, /// Users granted access individually (issue #66). Authoritative for WS /// authorization; lost on restart along with the session itself. pub invitees: std::collections::HashSet, @@ -76,7 +74,6 @@ impl TerminalManager { SessionState { vault_ids: vec![], allowed_roles: vec![], - invite_token: None, invitees: std::collections::HashSet::new(), host_user_id: host, host_public_key: String::new(), From 4a3988e8b8024bc2a32ab85776de056dc9fb98fb Mon Sep 17 00:00:00 2001 From: kipavy Date: Mon, 17 Aug 2026 00:05:53 +0000 Subject: [PATCH 09/10] test(terminal): rename ended-session mint test to what it proves require_active_session_host catches ended sessions before create_code's fetch_optional visibility lookup runs; only the row-disappears-mid-request race reaches that branch, and that isn't worth simulating. --- src/routes/session_codes.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/routes/session_codes.rs b/src/routes/session_codes.rs index 9457daf..18f7d08 100644 --- a/src/routes/session_codes.rs +++ b/src/routes/session_codes.rs @@ -153,7 +153,10 @@ mod tests { } #[tokio::test] - async fn minting_for_an_ended_session_is_not_found_not_a_crash() { + async fn minting_for_an_ended_session_is_not_found() { + // Caught by require_active_session_host, not the fetch_optional visibility + // lookup — that branch only fires on the row-disappears-mid-request race, + // which isn't worth simulating. let pool = test_pool_or_skip!(); let (host, session) = seed_host_and_session(&pool).await; sqlx::query("UPDATE terminal_sessions SET ended_at = now() WHERE id = $1") From e940d6bcf8d8e556c19dca99fcb6adc3eb922a65 Mon Sep 17 00:00:00 2001 From: kipavy Date: Mon, 17 Aug 2026 01:30:04 +0000 Subject: [PATCH 10/10] fix(terminal): close short-code join bypass, serialize code rotation, reconcile legacy grants - resolve_join_grant excludes kind=short_code so a spoken code can't skip /redeem, bypassing the guest-grant audit trail and the redeem limiter - rotate_short_code takes a FOR UPDATE lock on the session row first, serializing concurrent regenerate calls against idx_tsg_one_live_code - server boot reconciles orphaned invite_token rows left by a rollback/roll-forward cycle past migration 037's one-shot backfill; ON CONFLICT DO NOTHING makes it safe under a rolling deploy - rate_limit::check_user_budget is the one implementation of the warn-and-429 shape, used by both session_codes handlers and the existing per-user middleware - redeemed guest grants are attributed to the session host, not the redeeming guest, so host-facing tooling can find them - delete a vacuous migration test, tighten a FORBIDDEN assertion, add get_my_session_key status-code regression coverage --- src/db.rs | 92 +++++++++++++++++++++++ src/rate_limit.rs | 19 ++++- src/routes/session_codes.rs | 76 ++++++++++++++----- src/routes/terminal.rs | 139 ++++++++++++++++++++++++++++++++--- src/session_grants.rs | 141 +++++++++++++++++------------------- 5 files changed, 359 insertions(+), 108 deletions(-) diff --git a/src/db.rs b/src/db.rs index 57b8f98..68755f8 100644 --- a/src/db.rs +++ b/src/db.rs @@ -27,5 +27,97 @@ pub async fn create_pool() -> PgPool { }); info!("Migrations applied successfully"); + reconcile_legacy_grants(&pool).await; + pool } + +/// Migration 037's backfill is one-shot: a rollback-then-forward-again cycle +/// can leave live invite_link sessions with an `invite_token` but no grant, +/// since nothing reads that column outside the grants path any more. Runs +/// every boot and is idempotent — the anti-join only ever inserts missing rows. +/// ON CONFLICT DO NOTHING on idx_tsg_secret: a rolling deploy can start two +/// instances close enough together that their anti-joins both see the same +/// orphan under READ COMMITTED; the loser must not crash the boot. +async fn reconcile_legacy_grants(pool: &PgPool) { + let result = sqlx::query( + "INSERT INTO terminal_session_grants (session_id, kind, secret_hash, created_by) \ + SELECT ts.id, 'legacy_token', sha256(convert_to(ts.invite_token, 'UTF8')), ts.host_user_id \ + FROM terminal_sessions ts \ + LEFT JOIN terminal_session_grants g \ + ON g.session_id = ts.id AND g.kind = 'legacy_token' \ + WHERE ts.invite_token IS NOT NULL AND ts.ended_at IS NULL AND g.id IS NULL \ + ON CONFLICT (secret_hash) DO NOTHING", + ) + .execute(pool) + .await; + + match result { + Ok(res) => info!( + reconciled = res.rows_affected(), + "Reconciled legacy invite-token grants" + ), + Err(err) => { + error!(error = %err, "Failed to reconcile legacy invite-token grants"); + panic!("Failed to reconcile legacy invite-token grants"); + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::test_pool_or_skip; + use crate::test_support::{seed_session, seed_user}; + use uuid::Uuid; + + #[tokio::test] + async fn reconciliation_backfills_a_grant_for_a_token_with_none() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + let token = format!("fake-rollback-orphan-{}", Uuid::new_v4()); + + sqlx::query("UPDATE terminal_sessions SET invite_token = $1 WHERE id = $2") + .bind(&token) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + reconcile_legacy_grants(&pool).await; + + assert!( + crate::session_grants::resolve_join_grant(&pool, session, &token).await, + "reconciliation must mint a resolvable grant" + ); + } + + #[tokio::test] + async fn reconciliation_is_idempotent() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + let token = format!("fake-rollback-orphan-{}", Uuid::new_v4()); + + sqlx::query("UPDATE terminal_sessions SET invite_token = $1 WHERE id = $2") + .bind(&token) + .bind(session) + .execute(&pool) + .await + .unwrap(); + + reconcile_legacy_grants(&pool).await; + reconcile_legacy_grants(&pool).await; + + let count: i64 = sqlx::query_scalar( + "SELECT count(*) FROM terminal_session_grants \ + WHERE session_id = $1 AND kind = 'legacy_token'", + ) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(count, 1, "second run must insert nothing new"); + } +} diff --git a/src/rate_limit.rs b/src/rate_limit.rs index e7c15ab..1a92a7c 100644 --- a/src/rate_limit.rs +++ b/src/rate_limit.rs @@ -122,6 +122,20 @@ pub async fn register_rate_limit( Ok(next.run(req).await) } +/// Checks a per-user limiter, warning and returning 429 on exhaustion. The +/// one place this shape is expressed; middlewares and handlers alike call it. +pub async fn check_user_budget( + limiter: &RateLimiter, + user: Uuid, + label: &str, +) -> Result<(), StatusCode> { + if !limiter.check(user).await { + warn!(user_id = %user, limiter = label, "Rate limit exceeded"); + return Err(StatusCode::TOO_MANY_REQUESTS); + } + Ok(()) +} + /// Shared body for the per-user middlewares below: check the limiter keyed on /// the authenticated caller, or reject with 429. async fn user_keyed_limit( @@ -131,10 +145,7 @@ async fn user_keyed_limit( req: Request, next: Next, ) -> Result { - if !limiter.check(user).await { - warn!(user_id = %user, limiter = label, "Rate limit exceeded"); - return Err(StatusCode::TOO_MANY_REQUESTS); - } + check_user_budget(limiter, user, label).await?; Ok(next.run(req).await) } diff --git a/src/routes/session_codes.rs b/src/routes/session_codes.rs index 18f7d08..5d5bd00 100644 --- a/src/routes/session_codes.rs +++ b/src/routes/session_codes.rs @@ -8,7 +8,7 @@ use tracing::warn; use uuid::Uuid; use crate::auth::AuthUser; -use crate::rate_limit::{RedeemRateLimiter, SessionCodeRateLimiter}; +use crate::rate_limit::{check_user_budget, RedeemRateLimiter, SessionCodeRateLimiter}; use crate::session_grants; #[derive(Debug, Serialize)] @@ -34,9 +34,7 @@ pub async fn create_code( Extension(SessionCodeRateLimiter(limiter)): Extension, Path(session_id): Path, ) -> Result<(StatusCode, Json), StatusCode> { - if !limiter.check(auth.0).await { - return Err(StatusCode::TOO_MANY_REQUESTS); - } + check_user_budget(&limiter, auth.0, "session_code_mint").await?; crate::routes::terminal::require_active_session_host(&pool, session_id, auth.0).await?; // Only invite_link sessions serve raw keys through the short-code/grant @@ -70,9 +68,7 @@ pub async fn redeem_code( Extension(RedeemRateLimiter(limiter)): Extension, Json(body): Json, ) -> Result, StatusCode> { - if !limiter.check(auth.0).await { - return Err(StatusCode::TOO_MANY_REQUESTS); - } + check_user_budget(&limiter, auth.0, "session_code_redeem").await?; // Unknown, malformed, expired and revoked all answer 404: no response // distinguishes a real code from a wrong one. @@ -84,13 +80,15 @@ pub async fn redeem_code( let secret = session_grants::new_token_secret(); // expires_at: None — the guest grant outlives the 10-minute code; it ends // only by revoke or session end, same as any other invited participant. + // created_by is the session's host, not the redeeming guest, so host-facing + // tooling filtering by created_by still finds these grants. session_grants::insert_grant( &pool, grant.session_id, "guest", &secret, None, - auth.0, + grant.host_user_id, Some(auth.0), ) .await @@ -133,14 +131,17 @@ mod tests { let (host, session) = seed_host_and_session(&pool).await; let (stranger, _) = seed_host_and_session(&pool).await; - assert!(create_code( - State(pool.clone()), - Extension(AuthUser(stranger)), - Extension(code_budget()), - Path(session), - ) - .await - .is_err()); + assert_eq!( + create_code( + State(pool.clone()), + Extension(AuthUser(stranger)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap_err(), + StatusCode::FORBIDDEN + ); assert!(create_code( State(pool.clone()), @@ -224,12 +225,49 @@ mod tests { assert_eq!(redeemed.session_id, session); assert!( - crate::session_grants::resolve_join_grant(&pool, session, &redeemed.invite_token) - .await - .is_some() + crate::session_grants::resolve_join_grant(&pool, session, &redeemed.invite_token).await ); } + #[tokio::test] + async fn a_redeemed_guest_grant_is_attributed_to_the_host_not_the_guest() { + // Host-facing tooling filters grants by created_by; attributing the row + // to the redeeming guest would make it invisible there. + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + let (guest, _) = seed_host_and_session(&pool).await; + + let (_, Json(minted)) = create_code( + State(pool.clone()), + Extension(AuthUser(host)), + Extension(code_budget()), + Path(session), + ) + .await + .unwrap(); + + let _: Json = redeem_code( + State(pool.clone()), + Extension(AuthUser(guest)), + Extension(redeem_budget()), + Json(RedeemRequest { + code: minted.code.clone(), + }), + ) + .await + .unwrap(); + + let created_by: Uuid = sqlx::query_scalar( + "SELECT created_by FROM terminal_session_grants \ + WHERE session_id = $1 AND kind = 'guest'", + ) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(created_by, host); + } + #[tokio::test] async fn one_code_admits_several_guests_until_it_expires() { let pool = test_pool_or_skip!(); diff --git a/src/routes/terminal.rs b/src/routes/terminal.rs index af2bea5..676196b 100644 --- a/src/routes/terminal.rs +++ b/src/routes/terminal.rs @@ -970,10 +970,7 @@ pub async fn get_my_session_key( })? .ok_or(StatusCode::NOT_FOUND)?; - if crate::session_grants::resolve_join_grant(&pool, session_id, token) - .await - .is_none() - { + if !crate::session_grants::resolve_join_grant(&pool, session_id, token).await { return Err(StatusCode::FORBIDDEN); } @@ -1277,9 +1274,7 @@ pub(crate) async fn is_authorized_participant( let Some(presented) = presented_token else { return false; }; - return crate::session_grants::resolve_join_grant(pool, session_id, presented) - .await - .is_some(); + return crate::session_grants::resolve_join_grant(pool, session_id, presented).await; } // Vault session: user must be a member of one of the session's vaults, // satisfy the role filter (if any), and have JOIN_TERMINAL_SESSION permission. @@ -1970,9 +1965,7 @@ mod authz_tests { assert_eq!(grant_kind, "legacy_token"); assert!( - crate::session_grants::resolve_join_grant(&pool, body.session_id, &token) - .await - .is_some(), + crate::session_grants::resolve_join_grant(&pool, body.session_id, &token).await, "the token an old client already holds must keep resolving" ); } @@ -3111,4 +3104,130 @@ mod tests { "a different guest's grant on the same session must keep working" ); } + + #[tokio::test] + async fn a_short_code_does_not_work_as_an_invite_token() { + // A guest holding the spoken code must go through /redeem — resolving + // it directly would skip minting a guest row and bypass that + // endpoint's own rate limiter. + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let guest = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + + sqlx::query("UPDATE terminal_sessions SET session_key_bytes = 'fake-key-bytes' WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + + let (code, _) = crate::session_grants::rotate_short_code(&pool, session, host) + .await + .unwrap(); + let normalized = crate::session_grants::normalize_short_code(&code).unwrap(); + + // SessionKeyResponse derives only Serialize, so unwrap_err() (which + // requires Ok: Debug) doesn't compile here; match on Err directly. + let res = get_my_session_key( + State(pool.clone()), + Extension(AuthUser(guest)), + axum::extract::Path(session), + axum::extract::Query(GetKeyQuery { + invite_token: Some(normalized.clone()), + }), + ) + .await; + assert!(matches!(res, Err(StatusCode::FORBIDDEN))); + + assert!( + !is_authorized_participant( + &pool, session, guest, host, "invite_link", &[], &[], + Some(normalized.as_str()), + &std::collections::HashSet::new(), + ) + .await + ); + + // The secret minted by an actual redemption of that same code succeeds + // at both. + let Json(redeemed) = crate::routes::session_codes::redeem_code( + State(pool.clone()), + Extension(AuthUser(guest)), + Extension(crate::rate_limit::RedeemRateLimiter( + crate::rate_limit::RateLimiter::new(20, std::time::Duration::from_secs(3600)), + )), + Json(crate::routes::session_codes::RedeemRequest { code }), + ) + .await + .unwrap(); + + let key = get_my_session_key( + State(pool.clone()), + Extension(AuthUser(guest)), + axum::extract::Path(session), + axum::extract::Query(GetKeyQuery { + invite_token: Some(redeemed.invite_token.clone()), + }), + ) + .await + .expect("the redeemed secret must unlock the key endpoint"); + assert!(key.0.raw_key.is_some()); + + assert!( + is_authorized_participant( + &pool, session, guest, host, "invite_link", &[], &[], + Some(redeemed.invite_token.as_str()), + &std::collections::HashSet::new(), + ) + .await + ); + } + + #[tokio::test] + async fn an_ended_session_is_not_found_regardless_of_the_token() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let guest = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + sqlx::query("UPDATE terminal_sessions SET ended_at = now() WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + + let res = get_my_session_key( + State(pool.clone()), + Extension(AuthUser(guest)), + axum::extract::Path(session), + axum::extract::Query(GetKeyQuery { + invite_token: Some(format!("fake-token-{}", Uuid::new_v4())), + }), + ) + .await; + assert!(matches!(res, Err(StatusCode::NOT_FOUND))); + } + + #[tokio::test] + async fn a_live_session_with_a_wrong_token_is_forbidden_not_not_found() { + let pool = test_pool_or_skip!(); + let host = seed_user(&pool).await; + let guest = seed_user(&pool).await; + let session = seed_session(&pool, host, "invite_link").await; + sqlx::query("UPDATE terminal_sessions SET session_key_bytes = 'fake-key-bytes' WHERE id = $1") + .bind(session) + .execute(&pool) + .await + .unwrap(); + + let res = get_my_session_key( + State(pool.clone()), + Extension(AuthUser(guest)), + axum::extract::Path(session), + axum::extract::Query(GetKeyQuery { + invite_token: Some(format!("fake-wrong-token-{}", Uuid::new_v4())), + }), + ) + .await; + assert!(matches!(res, Err(StatusCode::FORBIDDEN))); + } } diff --git a/src/session_grants.rs b/src/session_grants.rs index 55a2686..1fdaebb 100644 --- a/src/session_grants.rs +++ b/src/session_grants.rs @@ -55,10 +55,6 @@ pub fn new_token_secret() -> String { pub const SHORT_CODE_TTL_MINUTES: i64 = 10; -pub struct Grant { - pub session_id: Uuid, -} - /// Generic over the executor so a caller can run this inside its own /// transaction (e.g. alongside the session INSERT it must not outlive) or /// just pass a `&PgPool` for a standalone grant. @@ -90,31 +86,38 @@ where .map(|_| ()) } -/// Kind-agnostic: a live grant is a live grant. Short codes never travel this -/// path — they are redeemed at their own endpoint — so the secret is hashed raw. -pub async fn resolve_join_grant(pool: &PgPool, session_id: Uuid, presented: &str) -> Option { - sqlx::query_as::<_, (Uuid,)>( - "SELECT g.session_id \ - FROM terminal_session_grants g \ - JOIN terminal_sessions ts ON ts.id = g.session_id \ - WHERE g.session_id = $1 AND g.secret_hash = $2 \ - AND g.revoked_at IS NULL \ - AND (g.expires_at IS NULL OR g.expires_at > now()) \ - AND ts.ended_at IS NULL", +/// Excludes `short_code`: that kind is only ever redeemed at its own endpoint +/// (POST .../redeem), which mints a distinct, revocable `guest` grant and +/// counts against its own limiter. Letting a spoken code resolve here would +/// let a guest skip both. +pub async fn resolve_join_grant(pool: &PgPool, session_id: Uuid, presented: &str) -> bool { + sqlx::query_scalar::<_, bool>( + "SELECT EXISTS( \ + SELECT 1 FROM terminal_session_grants g \ + JOIN terminal_sessions ts ON ts.id = g.session_id \ + WHERE g.session_id = $1 AND g.secret_hash = $2 \ + AND g.kind <> 'short_code' \ + AND g.revoked_at IS NULL \ + AND (g.expires_at IS NULL OR g.expires_at > now()) \ + AND ts.ended_at IS NULL \ + )", ) .bind(session_id) .bind(hash_secret(presented)) - .fetch_optional(pool) + .fetch_one(pool) .await - .ok() - .flatten() - .map(|(session_id,)| Grant { session_id }) + .unwrap_or(false) +} + +pub struct ShortCodeGrant { + pub session_id: Uuid, + pub host_user_id: Uuid, } -pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { +pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { let normalized = normalize_short_code(code)?; - sqlx::query_as::<_, (Uuid,)>( - "SELECT g.session_id \ + sqlx::query_as::<_, (Uuid, Uuid)>( + "SELECT g.session_id, ts.host_user_id \ FROM terminal_session_grants g \ JOIN terminal_sessions ts ON ts.id = g.session_id \ WHERE g.secret_hash = $1 AND g.kind = 'short_code' \ @@ -126,7 +129,10 @@ pub async fn resolve_short_code(pool: &PgPool, code: &str) -> Option { .await .ok() .flatten() - .map(|(session_id,)| Grant { session_id }) + .map(|(session_id, host_user_id)| ShortCodeGrant { + session_id, + host_user_id, + }) } pub async fn rotate_short_code( @@ -139,6 +145,13 @@ pub async fn rotate_short_code( let expires_at = Utc::now() + Duration::minutes(SHORT_CODE_TTL_MINUTES); let mut tx = pool.begin().await?; + // Serializes overlapping regenerate calls for the same session: under READ + // COMMITTED two concurrent revoke+insert pairs can both pass the revoking + // UPDATE, and the loser's INSERT then trips idx_tsg_one_live_code. + sqlx::query("SELECT id FROM terminal_sessions WHERE id = $1 FOR UPDATE") + .bind(session_id) + .execute(&mut *tx) + .await?; // No expiry condition: an expired-but-unrevoked row still occupies the // partial unique index, so it has to be swept too. sqlx::query( @@ -169,7 +182,7 @@ pub async fn rotate_short_code( mod tests { use super::*; use crate::test_pool_or_skip; - use crate::test_support::{seed_session, seed_team, seed_user}; + use crate::test_support::{seed_session, seed_user}; use uuid::Uuid; async fn seed_host_and_session(pool: &sqlx::PgPool) -> (Uuid, Uuid) { @@ -178,44 +191,6 @@ mod tests { (host, session) } - #[tokio::test] - async fn backfill_creates_a_legacy_grant_for_a_live_invite_link_session() { - let pool = test_pool_or_skip!(); - - let host = seed_user(&pool).await; - let team = seed_team(&pool, host).await; - // invite_token is UNIQUE; the throwaway DB persists across test runs. - let token = format!("fake-legacy-token-{}", Uuid::new_v4()); - - let session: Uuid = sqlx::query_scalar( - "INSERT INTO terminal_sessions (team_id, host_user_id, connection_name, visibility, invite_token) \ - VALUES ($1, $2, 'box', 'invite_link', $3) RETURNING id", - ) - .bind(team) - .bind(host) - .bind(&token) - .fetch_one(&pool) - .await - .unwrap(); - - // The migration already ran for pre-existing rows; this row is newer, so - // apply the same expression the migration uses to prove it matches. - let matches: bool = sqlx::query_scalar( - "SELECT sha256(convert_to($1, 'UTF8')) = sha256(convert_to(invite_token, 'UTF8')) \ - FROM terminal_sessions WHERE id = $2", - ) - .bind(&token) - .bind(session) - .fetch_one(&pool) - .await - .unwrap(); - - assert!( - matches, - "migration hash expression must match the stored token" - ); - } - /// The deploy-day case: an already-live session whose grant only exists /// because the migration backfilled it, hashing in SQL — not through /// `insert_grant`/`hash_secret`. Proves the two hashing paths agree; if @@ -245,7 +220,7 @@ mod tests { .unwrap(); assert!( - resolve_join_grant(&pool, session, &token).await.is_some(), + resolve_join_grant(&pool, session, &token).await, "SQL-side and Rust-side hashing must agree on deploy day" ); } @@ -319,12 +294,8 @@ mod tests { .await .unwrap(); - assert!(resolve_join_grant(&pool, session, &secret).await.is_some()); - assert!( - resolve_join_grant(&pool, session, "fake-grant-secret-wrong") - .await - .is_none() - ); + assert!(resolve_join_grant(&pool, session, &secret).await); + assert!(!resolve_join_grant(&pool, session, "fake-grant-secret-wrong").await); } #[tokio::test] @@ -360,12 +331,10 @@ mod tests { .await .unwrap(); - assert!(resolve_join_grant(&pool, session, &expired).await.is_none()); - assert!(resolve_join_grant(&pool, session, &revoked).await.is_none()); + assert!(!resolve_join_grant(&pool, session, &expired).await); + assert!(!resolve_join_grant(&pool, session, &revoked).await); assert!( - resolve_join_grant(&pool, other_session, &live) - .await - .is_none(), + !resolve_join_grant(&pool, other_session, &live).await, "a grant must not resolve against a different session" ); @@ -375,7 +344,7 @@ mod tests { .await .unwrap(); assert!( - resolve_join_grant(&pool, session, &live).await.is_none(), + !resolve_join_grant(&pool, session, &live).await, "an ended session must admit nobody" ); } @@ -428,6 +397,28 @@ mod tests { assert_eq!(live, 1, "exactly one live short code per session"); } + #[tokio::test] + async fn sequential_rotations_each_succeed_and_leave_one_live_row() { + // The concurrent case (two overlapping transactions racing the FOR + // UPDATE lock) is the live gate's job; this only proves the happy path + // still works and still converges to one live row. + let pool = test_pool_or_skip!(); + let (host, session) = seed_host_and_session(&pool).await; + + assert!(rotate_short_code(&pool, session, host).await.is_ok()); + assert!(rotate_short_code(&pool, session, host).await.is_ok()); + + let live: i64 = sqlx::query_scalar( + "SELECT count(*) FROM terminal_session_grants \ + WHERE session_id = $1 AND kind = 'short_code' AND revoked_at IS NULL", + ) + .bind(session) + .fetch_one(&pool) + .await + .unwrap(); + assert_eq!(live, 1); + } + #[tokio::test] async fn an_expired_code_does_not_resolve() { let pool = test_pool_or_skip!();