fix: audit the first JWT entry across an IdP row; finish has_any_account; nits

- Gate the JWT `users.login_guest` audit on the transition to `jwt_entry = true`,
  not physical insertion: a guest who signed in through the IdP earlier the same
  day already has a `guest_activity` row with `jwt_entry = false`, and the old
  `xmax = 0` test suppressed the first JWT audit. A CTE reads the prior flag and
  the RETURNING decides it atomically in the upsert.
- Fold the signed-in mint's inline account check into `has_any_account`, so the
  helper has both callers and the two no longer diverge on lowercasing.
- Negative-cache a refused guest JWT for 30s so a replayed bearer past the cap
  does not take the instance-wide allowance advisory lock on every request.
- Update the two stale share-link header comments to the `guest.<jwt>` form,
  drop the "plan gate" rationale on the entry test's cfg, collapse the blank
  lines the SHARE_LINK_SEGMENT removal left, and prettier-format the settings
  card after the isEnterprisePlan wrapper was removed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VF3v6LA9399gNphmZaHYG3
This commit is contained in:
Ruben Fiszel
2026-09-03 23:08:02 +02:00
co-authored by Claude Opus 4.8
parent b8838564ff
commit 4899e51703
10 changed files with 108 additions and 95 deletions
@@ -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"
}
@@ -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"
}
-1
View File
@@ -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";
+2 -1
View File
@@ -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};
+32 -11
View File
@@ -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<String, ()> = Cache::new(2000);
static ref GUEST_JWT_REFUSED_CACHE: Cache<String, std::time::Instant> = 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(),
+3 -12
View File
@@ -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(),
));
-1
View File
@@ -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,
@@ -2240,8 +2240,8 @@ export async function main(
</span>
{:else if guestUsage}
<span class="text-hint text-2xs">
{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(
</span>
{/if}
<div class="mt-4 flex flex-col gap-2 border-t pt-4">
<div class="text-xs font-semibold text-emphasis">
Guest JWT verification key
</div>
<div class="text-2xs text-hint">
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
<code>email</code>, <code>workspace_id</code>, <code>app_path</code> and
<code>exp</code> (lifetime capped at 24h); it opens only the app named by
<code>app_path</code>. 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.
</div>
<ToggleButtonGroup bind:selected={guestJwtKeyType}>
{#snippet children({ item })}
<ToggleButton {item} value="pem" label="PEM public key" />
<ToggleButton {item} value="jwks" label="JWKS URL" />
{/snippet}
</ToggleButtonGroup>
{#if guestJwtKeyType === 'pem'}
<TextInput
underlyingInputEl="textarea"
class="font-mono text-xs"
autosizeParams={{ minHeight: 128 }}
inputProps={{
placeholder: '-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'
}}
bind:value={guestJwtPublicKey}
/>
{:else}
<TextInput
inputProps={{ placeholder: 'https://issuer.example.com/.well-known/jwks.json' }}
bind:value={guestJwtJwksUrl}
/>
{/if}
<div class="text-xs font-semibold text-emphasis">
Guest JWT verification key
</div>
<div class="text-2xs text-hint">
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
<code>email</code>, <code>workspace_id</code>, <code>app_path</code> and
<code>exp</code> (lifetime capped at 24h); it opens only the app named by
<code>app_path</code>. 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.
</div>
<ToggleButtonGroup bind:selected={guestJwtKeyType}>
{#snippet children({ item })}
<ToggleButton {item} value="pem" label="PEM public key" />
<ToggleButton {item} value="jwks" label="JWKS URL" />
{/snippet}
</ToggleButtonGroup>
{#if guestJwtKeyType === 'pem'}
<TextInput
underlyingInputEl="textarea"
class="font-mono text-xs"
autosizeParams={{ minHeight: 128 }}
inputProps={{
placeholder: '-----BEGIN PUBLIC KEY-----\n...\n-----END PUBLIC KEY-----'
}}
bind:value={guestJwtPublicKey}
/>
{:else}
<TextInput
inputProps={{
placeholder: 'https://issuer.example.com/.well-known/jwks.json'
}}
bind:value={guestJwtJwksUrl}
/>
{/if}
</div>
</SettingCard>
<SettingsFooter
@@ -2299,9 +2301,9 @@ export async function main(
onDiscard={discardDefaultAppSettingsChanges}
saveLabel="Save app settings"
disabled={!$enterpriseLicense &&
guestAccessEnabled === initialGuestAccessEnabled &&
effectiveGuestJwt.pem === initialGuestJwtPublicKey &&
effectiveGuestJwt.jwks === initialGuestJwtJwksUrl}
guestAccessEnabled === initialGuestAccessEnabled &&
effectiveGuestJwt.pem === initialGuestJwtPublicKey &&
effectiveGuestJwt.jwks === initialGuestJwtJwksUrl}
/>
{:else if tab == 'native_triggers'}
{#if $workspaceStore}
+3 -3
View File
@@ -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 (`<path>/guest/<jwt>`). The
// marker keeps the two apart; `viewerUrl` uses `path` alone, so neither reaches the
// opaque iframe.
// segment, or a guest JWT in a `guest.<jwt>` last segment (`<path>/guest.<jwt>`). 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
@@ -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 `<secret>/<jwt>`, or a guest JWT as `<secret>/guest/<jwt>`. The `guest`
// marker keeps the two apart with no parsing of the token, which the page cannot
// JWT as `<secret>/<jwt>`, or a guest JWT as `<secret>/guest.<jwt>`. 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): {