diff --git a/backend/.sqlx/query-5a52b42ddae68a7d280cfe1c9e5e5983be237f8a3013c680d2b1967e703384e6.json b/backend/.sqlx/query-5a52b42ddae68a7d280cfe1c9e5e5983be237f8a3013c680d2b1967e703384e6.json deleted file mode 100644 index d7418e3ed1..0000000000 --- a/backend/.sqlx/query-5a52b42ddae68a7d280cfe1c9e5e5983be237f8a3013c680d2b1967e703384e6.json +++ /dev/null @@ -1,23 +0,0 @@ -{ - "db_name": "PostgreSQL", - "query": "INSERT INTO guest_activity (email, workspace_id, day, jwt_entry)\n VALUES ($1, $2, CURRENT_DATE, true)\n ON CONFLICT (email, workspace_id, day)\n DO UPDATE SET jwt_entry = true, last_seen_at = now()\n RETURNING (xmax = 0) AS \"inserted!\"", - "describe": { - "columns": [ - { - "ordinal": 0, - "name": "inserted!", - "type_info": "Bool" - } - ], - "parameters": { - "Left": [ - "Varchar", - "Varchar" - ] - }, - "nullable": [ - null - ] - }, - "hash": "5a52b42ddae68a7d280cfe1c9e5e5983be237f8a3013c680d2b1967e703384e6" -} diff --git a/backend/.sqlx/query-7c7298a76978f4e4d0f7b644ea6f84e01dda54b9342a02d6645a4462e5e6bb19.json b/backend/.sqlx/query-7c7298a76978f4e4d0f7b644ea6f84e01dda54b9342a02d6645a4462e5e6bb19.json new file mode 100644 index 0000000000..272c3f4ab6 --- /dev/null +++ b/backend/.sqlx/query-7c7298a76978f4e4d0f7b644ea6f84e01dda54b9342a02d6645a4462e5e6bb19.json @@ -0,0 +1,23 @@ +{ + "db_name": "PostgreSQL", + "query": "WITH prior AS (\n SELECT jwt_entry FROM guest_activity\n WHERE email = $1 AND workspace_id = $2 AND day = CURRENT_DATE\n )\n INSERT INTO guest_activity (email, workspace_id, day, jwt_entry)\n VALUES ($1, $2, CURRENT_DATE, true)\n ON CONFLICT (email, workspace_id, day)\n DO UPDATE SET jwt_entry = true, last_seen_at = now()\n RETURNING (NOT COALESCE((SELECT jwt_entry FROM prior), false)) AS \"first_jwt!\"", + "describe": { + "columns": [ + { + "ordinal": 0, + "name": "first_jwt!", + "type_info": "Bool" + } + ], + "parameters": { + "Left": [ + "Text", + "Text" + ] + }, + "nullable": [ + null + ] + }, + "hash": "7c7298a76978f4e4d0f7b644ea6f84e01dda54b9342a02d6645a4462e5e6bb19" +} diff --git a/backend/tests/app_guest_jwt_allowance.rs b/backend/tests/app_guest_jwt_allowance.rs index 9bdac83df8..85cb51d39e 100644 --- a/backend/tests/app_guest_jwt_allowance.rs +++ b/backend/tests/app_guest_jwt_allowance.rs @@ -31,7 +31,6 @@ fn set_plan(pro: bool) { let _ = pro; } - const JWT_PUB: &str = "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQYIKoZIzj0DAQcDQgAEzAfqyCh34iYOCW0vg4ejq/zzJlzL\nSZScjnVyPjLGTapEwo4gc6/y1Yudd/v54wKh0OdfTfzAKMPWx/2NWx/ugg==\n-----END PUBLIC KEY-----\n"; const JWT_PRIV: &str = "-----BEGIN PRIVATE KEY-----\nMIGHAgEAMBMGByqGSM49AgEGCCqGSM49AwEHBG0wawIBAQQgu27S2DbSwUh8BmQb\n/i4/VhNdoXV7PJekhnoceMULYLihRANCAATMB+rIKHfiJg4JbS+Dh6Or/PMmXMtJ\nlJyOdXI+MsZNqkTCjiBzr/LVi513+/njAqHQ519N/MAow9bH/Y1bH+6C\n-----END PRIVATE KEY-----\n"; diff --git a/backend/tests/app_guest_jwt_entry.rs b/backend/tests/app_guest_jwt_entry.rs index 6f6d4f8d28..e2b58b2ffd 100644 --- a/backend/tests/app_guest_jwt_entry.rs +++ b/backend/tests/app_guest_jwt_entry.rs @@ -12,7 +12,8 @@ //! The keys are fixed test vectors (EC P-256, PKCS8), so signing is deterministic and //! needs no key generation at runtime. -// The plan gate refuses every guest on a build without these; CI builds with them. +// Built with these like the sibling guest-execution suite: the guest run executes as +// the publisher through EE on-behalf-of code. CI builds with them. #![cfg(all(feature = "enterprise", feature = "private"))] use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index 7739d01b7b..f2af12e737 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -719,25 +719,37 @@ impl AuthCache { /// day-keyed activity dedupe below reachable across a midnight. const GUEST_JWT_CACHE_TTL: chrono::Duration = chrono::Duration::minutes(5); +/// A refused JWT (a stranger past the allowance) is remembered this long so a replayed +/// bearer does not take the instance-wide allowance advisory lock on every request. +/// Short, so a stranger admitted once the window frees is re-checked soon. +const GUEST_JWT_REFUSED_TTL: std::time::Duration = std::time::Duration::from_secs(30); + lazy_static::lazy_static! { // One `guest_activity` upsert and one `users.login_guest` audit per email, // workspace and day: the arm re-runs every GUEST_JWT_CACHE_TTL, and neither the // seat scan nor the audit trail wants a write each time. LRU-bounded; the day is in // the key, so a new day writes again. static ref GUEST_JWT_ACTIVITY_CACHE: Cache = Cache::new(2000); + static ref GUEST_JWT_REFUSED_CACHE: Cache = Cache::new(2000); } -/// Record that a JWT guest was seen today (the only durable trace of a guest, since it -/// leaves no `usr` row), and audit the login the first time. `jwt_entry` marks the row -/// so the seat telemetry can tell a JWT guest from a signed-in one. The audit is gated -/// on the upsert freshly inserting the row (`xmax = 0`), decided atomically by the DB, -/// so concurrent first requests and separate API nodes emit `users.login_guest` at -/// most once a day. `email` is already lowercased by the caller. +/// Admit a JWT guest against the instance allowance and record today's activity, in one +/// transaction so the advisory lock in `guest_admission` spans the count check and the +/// row that changes it. Returns false when the allowance refuses the email or on a DB +/// error, both of which deny the guest. Cached per email, workspace and day: a bearer +/// replayed every request runs this at most once a day, and a refused one is remembered +/// briefly so it does not re-take the allowance lock. `email` is already lowercased. async fn admit_and_record_guest_jwt(db: &DB, w_id: &str, email: &str, app_path: &str) -> bool { let cache_key = format!("{email}|{w_id}|{}", chrono::Utc::now().date_naive()); if GUEST_JWT_ACTIVITY_CACHE.get(&cache_key).is_some() { return true; } + if GUEST_JWT_REFUSED_CACHE + .get(&cache_key) + .is_some_and(|at| at.elapsed() < GUEST_JWT_REFUSED_TTL) + { + return false; + } let mut tx = match db.begin().await { Ok(tx) => tx, Err(e) => { @@ -751,20 +763,29 @@ async fn admit_and_record_guest_jwt(db: &DB, w_id: &str, email: &str, app_path: // instance) or a DB error rolls the tx back and denies the guest. if let Err(e) = windmill_common::workspaces::guest_admission(&mut *tx, email).await { tracing::info!("guest JWT not admitted for {w_id}: {e:#}"); + GUEST_JWT_REFUSED_CACHE.insert(cache_key, std::time::Instant::now()); return false; } - let inserted = sqlx::query_scalar!( - r#"INSERT INTO guest_activity (email, workspace_id, day, jwt_entry) + // `first_jwt` is true when this is the first JWT entry for the email today: a fresh + // row, or one an identity-provider sign-in created earlier with `jwt_entry = false`. + // The audit is gated on it, decided atomically in the upsert, so `users.login_guest` + // (entry=jwt) fires once a day even when today's row already existed. + let first_jwt = sqlx::query_scalar!( + r#"WITH prior AS ( + SELECT jwt_entry FROM guest_activity + WHERE email = $1 AND workspace_id = $2 AND day = CURRENT_DATE + ) + INSERT INTO guest_activity (email, workspace_id, day, jwt_entry) VALUES ($1, $2, CURRENT_DATE, true) ON CONFLICT (email, workspace_id, day) DO UPDATE SET jwt_entry = true, last_seen_at = now() - RETURNING (xmax = 0) AS "inserted!""#, + RETURNING (NOT COALESCE((SELECT jwt_entry FROM prior), false)) AS "first_jwt!""#, email, w_id, ) .fetch_one(&mut *tx) .await; - let inserted = match inserted { + let first_jwt = match first_jwt { Ok(v) => v, Err(e) => { tracing::error!("recording guest JWT activity for {w_id}: {e:#}"); @@ -780,7 +801,7 @@ async fn admit_and_record_guest_jwt(db: &DB, w_id: &str, email: &str, app_path: // `audit_partitioned` failure but that failing statement still aborts the // transaction it runs in, so auditing before the commit would let the whole // activity row roll back while this returned success, admitting an uncounted guest. - if inserted { + if first_jwt { let author = windmill_common::audit::AuditAuthor { email: email.to_string(), username: email.to_string(), diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index a347d6b5bc..53a248ce6a 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -2971,18 +2971,9 @@ pub async fn create_guest_session_token<'c>( }; let scopes = windmill_api_auth::scopes::guest_session_scopes(app_path); - // No account at all (see `ExecutionMode::Guest`): a deactivated `password` row - // counts, since the sign-in path's own lookup filters on `disabled = false` and a - // SCIM-offboarded account would otherwise read as absent; so does a `usr` row in - // any workspace, which is what a service account has instead of a password. - let has_account: bool = sqlx::query_scalar( - "SELECT EXISTS(SELECT 1 FROM password WHERE email = $1) - OR EXISTS(SELECT 1 FROM usr WHERE email = $1)", - ) - .bind(email) - .fetch_one(&mut **tx) - .await?; - if has_account { + // No account at all (see `has_any_account`): an account holder is refused a guest + // session, never handed a second, cheaper identity. The same helper the JWT arm uses. + if windmill_common::users::has_any_account(&mut **tx, email).await? { return Err(Error::NotAuthorized( "an existing account cannot hold a guest session".to_string(), )); diff --git a/backend/windmill-common/src/guest_jwt.rs b/backend/windmill-common/src/guest_jwt.rs index b4a671d687..a6c6231716 100644 --- a/backend/windmill-common/src/guest_jwt.rs +++ b/backend/windmill-common/src/guest_jwt.rs @@ -28,7 +28,6 @@ pub const MAX_LIFETIME_SECS: u64 = 24 * 60 * 60; /// so a rotated key revokes within minutes. See the arm in `windmill-api-auth`. pub const BEARER_PREFIX: &str = "jwt_guest_"; - const RSA_ALGORITHMS: [Algorithm; 6] = [ Algorithm::RS256, Algorithm::RS384, diff --git a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte index b241d5cfce..a90fe70e5d 100644 --- a/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte +++ b/frontend/src/routes/(root)/(logged)/workspace_settings/+page.svelte @@ -2240,8 +2240,8 @@ export async function main( {:else if guestUsage} - {guestUsage.guest_count} of {guestUsage.free_allowance} free guests used across - this instance in the last {guestUsage.window_days} days. + {guestUsage.guest_count} of {guestUsage.free_allowance} free guests used across this + instance in the last {guestUsage.window_days} days. {#if guestUsage.metered} Beyond that, every four guests count as one seat{guestUsage.guest_seats > 0 ? ` (${guestUsage.guest_seats} now)` @@ -2253,43 +2253,45 @@ export async function main( {/if}
-
- Guest JWT verification key -
-
- A guest can also enter through a JWT your own backend mints and signs, with - no identity-provider round-trip, for iframe embedding. The token must carry - email, workspace_id, app_path and - exp (lifetime capped at 24h); it opens only the app named by - app_path. Accepted algorithms: RS256/384/512, PS256/384/512, - ES256/384. Symmetric algorithms (HS*) are refused. Configure one key, a PEM - public key or a JWKS URL. Point it at an issuer you control: any token that - key signs carrying these claims is accepted, so a shared multi-tenant issuer - is not a good fit. -
- - {#snippet children({ item })} - - - {/snippet} - - {#if guestJwtKeyType === 'pem'} - - {:else} - - {/if} +
+ Guest JWT verification key
+
+ A guest can also enter through a JWT your own backend mints and signs, with no + identity-provider round-trip, for iframe embedding. The token must carry + email, workspace_id, app_path and + exp (lifetime capped at 24h); it opens only the app named by + app_path. Accepted algorithms: RS256/384/512, PS256/384/512, + ES256/384. Symmetric algorithms (HS*) are refused. Configure one key, a PEM + public key or a JWKS URL. Point it at an issuer you control: any token that key + signs carrying these claims is accepted, so a shared multi-tenant issuer is not + a good fit. +
+ + {#snippet children({ item })} + + + {/snippet} + + {#if guestJwtKeyType === 'pem'} + + {:else} + + {/if} +
{:else if tab == 'native_triggers'} {#if $workspaceStore} diff --git a/frontend/src/routes/a/[...path]/+page.svelte b/frontend/src/routes/a/[...path]/+page.svelte index 673ad196dd..e9fd20877c 100644 --- a/frontend/src/routes/a/[...path]/+page.svelte +++ b/frontend/src/routes/a/[...path]/+page.svelte @@ -35,9 +35,9 @@ } // The custom path may carry a trailing credential: an external JWT as its last - // segment, or a guest JWT preceded by a `guest` marker (`/guest/`). The - // marker keeps the two apart; `viewerUrl` uses `path` alone, so neither reaches the - // opaque iframe. + // segment, or a guest JWT in a `guest.` last segment (`/guest.`). The + // `guest.` prefix keeps the two apart; `viewerUrl` uses `path` alone, so neither + // reaches the opaque iframe. function parseCustomPath(customPath: string): { path: string jwt: string | undefined diff --git a/frontend/src/routes/public/[workspace]/[...secret]/+page.svelte b/frontend/src/routes/public/[workspace]/[...secret]/+page.svelte index 2e1c339ec2..b2a29b6316 100644 --- a/frontend/src/routes/public/[workspace]/[...secret]/+page.svelte +++ b/frontend/src/routes/public/[workspace]/[...secret]/+page.svelte @@ -28,8 +28,8 @@ let guestEntry: 'pending' | 'none' | 'guest' | 'error' = $state('pending') // The share link carries a trailing credential the embedder consumes: an external - // JWT as `/`, or a guest JWT as `/guest/`. The `guest` - // marker keeps the two apart with no parsing of the token, which the page cannot + // JWT as `/`, or a guest JWT as `/guest.`. The `guest.` + // prefix keeps the two apart with no parsing of the token, which the page cannot // verify anyway. Either way `viewerUrl` below uses `secret` alone, so no JWT // reaches the opaque iframe. function parseSecret(secret: string): {