diff --git a/backend/.sqlx/query-dc37f9ad685a2f2bd781be8678e0548a860815f30e866362344434f23aa7c83c.json b/backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json similarity index 50% rename from backend/.sqlx/query-dc37f9ad685a2f2bd781be8678e0548a860815f30e866362344434f23aa7c83c.json rename to backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json index 02d889c38b..02a2cb9973 100644 --- a/backend/.sqlx/query-dc37f9ad685a2f2bd781be8678e0548a860815f30e866362344434f23aa7c83c.json +++ b/backend/.sqlx/query-4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "WITH active_users as (SELECT distinct username as email FROM audit_partitioned WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh')),\n active_authors as (SELECT distinct email FROM usr WHERE usr.operator IS false AND email IN (SELECT email FROM active_users)),\n active_authors_agg as (SELECT array_agg(email) as authors FROM active_authors),\n active_ops_agg as (SELECT array_agg(email) as operators from active_users WHERE email NOT IN (SELECT email FROM active_authors))\n SELECT active_authors_agg.authors, active_ops_agg.operators, array_length(active_authors_agg.authors, 1) as author_count, array_length(active_ops_agg.operators, 1) as operator_count FROM active_authors_agg, active_ops_agg", + "query": "WITH active_users as (SELECT distinct username as email FROM audit_partitioned WHERE timestamp > NOW() - INTERVAL '1 month' AND (operation = 'users.login' OR operation = 'oauth.login' OR operation = 'users.token.refresh') AND username NOT IN (SELECT email FROM usr WHERE is_service_account)),\n active_authors as (SELECT distinct email FROM usr WHERE usr.operator IS false AND email IN (SELECT email FROM active_users)),\n active_authors_agg as (SELECT array_agg(email) as authors FROM active_authors),\n active_ops_agg as (SELECT array_agg(email) as operators from active_users WHERE email NOT IN (SELECT email FROM active_authors))\n SELECT active_authors_agg.authors, active_ops_agg.operators, array_length(active_authors_agg.authors, 1) as author_count, array_length(active_ops_agg.operators, 1) as operator_count FROM active_authors_agg, active_ops_agg", "describe": { "columns": [ { @@ -34,5 +34,5 @@ null ] }, - "hash": "dc37f9ad685a2f2bd781be8678e0548a860815f30e866362344434f23aa7c83c" + "hash": "4afdc63af51e599b9d6fe6cedcd7980fd761a29df0f2a6820f3c8d7a29c3c20a" } diff --git a/backend/.sqlx/query-be004608fc1de826803f266dfce96e09e634977478443b6372bca5e3dd0ce5ff.json b/backend/.sqlx/query-be004608fc1de826803f266dfce96e09e634977478443b6372bca5e3dd0ce5ff.json new file mode 100644 index 0000000000..dbd7e7c9ec --- /dev/null +++ b/backend/.sqlx/query-be004608fc1de826803f266dfce96e09e634977478443b6372bca5e3dd0ce5ff.json @@ -0,0 +1,22 @@ +{ + "db_name": "PostgreSQL", + "query": "SELECT NOT starts_with(COALESCE(label, ''), 'impersonation:') FROM token\n WHERE token_hash = $1 AND (expiration IS NULL OR expiration > now())", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "?column?", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "be004608fc1de826803f266dfce96e09e634977478443b6372bca5e3dd0ce5ff" +} diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 10041a71c8..4cfbcb8447 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -dba91044a174f8c9275fefea2619dc966b0f9250 +5a2b6b8527250bd12cf856d8a667a9ef3106ec60 diff --git a/backend/windmill-api-integration-tests/tests/users.rs b/backend/windmill-api-integration-tests/tests/users.rs index 65d44509d7..56656e54dd 100644 --- a/backend/windmill-api-integration-tests/tests/users.rs +++ b/backend/windmill-api-integration-tests/tests/users.rs @@ -993,3 +993,56 @@ async fn test_drafts_follow_their_owner_without_a_fkey(db: Pool) -> an Ok(()) } + +#[sqlx::test(migrations = "../migrations", fixtures("base"))] +async fn test_impersonation_session_is_never_refreshed(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let base = format!("http://localhost:{}/api/users", server.addr.port()); + let get = |token: &'static str, path: &str| { + client() + .get(format!("{base}/{path}")) + .header("Authorization", format!("Bearer {token}")) + .send() + }; + + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, email, label, expiration) + VALUES (encode(sha256('IMPERSONATION_TOKEN'::bytea), 'hex'), 'IMPERSONAT', + 'test2@windmill.dev', 'impersonation:test@windmill.dev', now() + interval '1 day')", + ) + .execute(&db) + .await?; + let refreshed = get("IMPERSONATION_TOKEN", "refresh_token") + .await? + .text() + .await?; + assert_eq!(refreshed, "this session cannot be refreshed"); + + // Once the row is gone, as when the monitor sweeps an expired one, the auth cache still + // accepts the token for a while. + sqlx::query("DELETE FROM token WHERE label LIKE 'impersonation:%'") + .execute(&db) + .await?; + assert_eq!(get("IMPERSONATION_TOKEN", "whoami").await?.status(), 200); + let refreshed = get("IMPERSONATION_TOKEN", "refresh_token") + .await? + .text() + .await?; + assert_eq!(refreshed, "this session cannot be refreshed"); + + let sessions: i64 = sqlx::query_scalar( + "SELECT count(*) FROM token WHERE email = 'test2@windmill.dev' AND label = 'session'", + ) + .fetch_one(&db) + .await?; + assert_eq!(sessions, 0); + + let refreshed = get("SECRET_TOKEN_3", "refresh_token").await?.text().await?; + assert_eq!( + refreshed, "token refreshed", + "an ordinary token still refreshes" + ); + + Ok(()) +} diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 8d1242befa..ac9defcd53 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -2866,8 +2866,26 @@ async fn refresh_token( .to_string(), )); } + let t_hash = windmill_common::auth::hash_token(&token); + // Only a live, non-impersonation row may be exchanged for a session: an impersonation must + // end at its 24h expiry, not become a renewable service-account session billed as active. + // Read the row rather than trust `authed`, which the auth cache keeps serving for up to + // 120s after the row expired or was swept. `jwt_` tokens have no row. + if !token.starts_with("jwt_") { + let refreshable = sqlx::query_scalar!( + "SELECT NOT starts_with(COALESCE(label, ''), 'impersonation:') FROM token + WHERE token_hash = $1 AND (expiration IS NULL OR expiration > now())", + &t_hash + ) + .fetch_optional(&db) + .await? + .flatten() + .unwrap_or(false); + if !refreshable { + return Ok("this session cannot be refreshed".to_string()); + } + } if let Some(thresh_s) = query.if_expiring_in_less_than_s { - let t_hash = windmill_common::auth::hash_token(&token); let not_expired = sqlx::query_scalar!("SELECT true FROM token WHERE token_hash = $1 and expiration IS NOT NULL and expiration > now() + $2::int * '1 sec'::interval", &t_hash, thresh_s) .fetch_optional(&db) .await? diff --git a/frontend/src/lib/components/AddUser.svelte b/frontend/src/lib/components/AddUser.svelte index aef1dbd400..1369e33fb2 100644 --- a/frontend/src/lib/components/AddUser.svelte +++ b/frontend/src/lib/components/AddUser.svelte @@ -261,13 +261,13 @@ {/snippet}