From 75bde58bb8e9b3c5f7be564bf836f1b85aaf9e36 Mon Sep 17 00:00:00 2001 From: Ruben Fiszel Date: Tue, 1 Sep 2026 21:41:08 +0000 Subject: [PATCH] fix: make the guest grant a server-minted label, not a declarable scope Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01BayTppRCstWX6qTf3LMco5 --- ...02200df0007291133033f2b9013c2e508fe1.json} | 5 +- backend/tests/app_guest_execution_mode.rs | 97 ++++++++++++++----- backend/windmill-api-auth/src/auth.rs | 14 ++- backend/windmill-api-auth/src/scopes.rs | 24 ++--- backend/windmill-api-users/src/users.rs | 25 ++++- backend/windmill-api/openapi.yaml | 5 +- backend/windmill-api/src/apps.rs | 24 ++++- backend/windmill-common/src/auth.rs | 17 ++++ backend/windmill-common/src/error.rs | 28 +----- frontend/src/lib/components/Login.svelte | 17 ++++ .../apps/editor/AppEditorHeaderDeploy.svelte | 11 ++- .../apps/editor/PublicAppFrame.svelte | 58 +++++++++-- .../(logged)/workspace_settings/+page.svelte | 6 ++ .../[workspace]/[...secret]/+page.svelte | 19 +++- 14 files changed, 255 insertions(+), 95 deletions(-) rename backend/.sqlx/{query-02bf6098743be1cc61b72747951fded5dad1fb16b10f1ba27a580261bb9d050d.json => query-1553608e0d5a9a9b22c1a2c200bf02200df0007291133033f2b9013c2e508fe1.json} (66%) diff --git a/backend/.sqlx/query-02bf6098743be1cc61b72747951fded5dad1fb16b10f1ba27a580261bb9d050d.json b/backend/.sqlx/query-1553608e0d5a9a9b22c1a2c200bf02200df0007291133033f2b9013c2e508fe1.json similarity index 66% rename from backend/.sqlx/query-02bf6098743be1cc61b72747951fded5dad1fb16b10f1ba27a580261bb9d050d.json rename to backend/.sqlx/query-1553608e0d5a9a9b22c1a2c200bf02200df0007291133033f2b9013c2e508fe1.json index 9dad2ec26c..3e4b85a916 100644 --- a/backend/.sqlx/query-02bf6098743be1cc61b72747951fded5dad1fb16b10f1ba27a580261bb9d050d.json +++ b/backend/.sqlx/query-1553608e0d5a9a9b22c1a2c200bf02200df0007291133033f2b9013c2e508fe1.json @@ -1,6 +1,6 @@ { "db_name": "PostgreSQL", - "query": "INSERT INTO token\n (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id)\n VALUES ($1, $2, $3, $4, 'session', now() + ($5 || ' seconds')::interval, false, $6, $7)", + "query": "INSERT INTO token\n (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id)\n VALUES ($1, $2, $3, $4, $5, now() + ($6 || ' seconds')::interval, false, $7, $8)", "describe": { "columns": [], "parameters": { @@ -9,6 +9,7 @@ "Varchar", "Varchar", "Varchar", + "Varchar", "Text", "TextArray", "Varchar" @@ -16,5 +17,5 @@ }, "nullable": [] }, - "hash": "02bf6098743be1cc61b72747951fded5dad1fb16b10f1ba27a580261bb9d050d" + "hash": "1553608e0d5a9a9b22c1a2c200bf02200df0007291133033f2b9013c2e508fe1" } diff --git a/backend/tests/app_guest_execution_mode.rs b/backend/tests/app_guest_execution_mode.rs index ba356ea0f0..cee3bb49f0 100644 --- a/backend/tests/app_guest_execution_mode.rs +++ b/backend/tests/app_guest_execution_mode.rs @@ -3,11 +3,13 @@ //! A guest is someone the identity provider authenticated who is a member of no //! workspace: no `usr` row, no `password` row, and so no seat on any counter. That //! absence is the whole point, and it means a guest session has no ACL of its own — -//! its token's scopes are its entire grant. These tests pin the two things that +//! its token's scopes are its entire grant. These tests pin the three things that //! would silently undo it: //! +//! * what makes a token a guest — the server-minted label, never a scope anyone +//! could type into `users/tokens/create`; //! * the confinement — a guest reaches the one app it was let in for and nothing -//! else, and is told its denial is fixable by signing up properly; +//! else; //! * the two gates — an app's own `execution_mode: guest` is inert unless the //! workspace switch is on, checked at the door rather than only where a policy //! is written (git-sync and the CLI push policies past every UI). @@ -34,18 +36,8 @@ fn authed(builder: reqwest::RequestBuilder, token: &str) -> reqwest::RequestBuil builder.header("Authorization", format!("Bearer {}", token)) } -/// Insert a guest session for `test-workspace`, scoped to `APP_PATH`. Mirrors -/// `create_guest_session_token`: the sentinel, the narrow reads, the two path-scoped -/// app grants, and the workspace pin. -async fn insert_guest_token(db: &Pool, workspace: &str) -> anyhow::Result<()> { - sqlx::query( - "INSERT INTO token (token_hash, token_prefix, token, email, label, scopes, workspace_id) - VALUES (encode(sha256($1::bytea), 'hex'), 'GUEST_SECR', $2, 'guest@example.com', - 'session', $3, $4)", - ) - .bind(GUEST_TOKEN.as_bytes()) - .bind(GUEST_TOKEN) - .bind(vec![ +fn guest_scopes() -> Vec { + vec![ "guest".to_string(), "jobs:read".to_string(), "resources:run".to_string(), @@ -53,7 +45,21 @@ async fn insert_guest_token(db: &Pool, workspace: &str) -> anyhow::Res "folders:read".to_string(), format!("apps:read:{APP_PATH}"), format!("apps:run:{APP_PATH}"), - ]) + ] +} + +/// Insert a guest session for `test-workspace`, scoped to `APP_PATH`. Mirrors +/// `create_guest_session_token`: the server-minted label, the narrow reads, the two +/// path-scoped app grants, and the workspace pin. +async fn insert_guest_token(db: &Pool, workspace: &str) -> anyhow::Result<()> { + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, scopes, workspace_id) + VALUES (encode(sha256($1::bytea), 'hex'), 'GUEST_SECR', $2, 'guest@example.com', + 'guest_session', $3, $4)", + ) + .bind(GUEST_TOKEN.as_bytes()) + .bind(GUEST_TOKEN) + .bind(guest_scopes()) .bind(workspace) .execute(db) .await?; @@ -84,8 +90,6 @@ async fn guest_session_is_confined_to_its_app(db: Pool) -> anyhow::Res assert_eq!(me["operator"], json!(true)); assert_eq!(me["is_admin"], json!(false)); - // Everything outside the app surface is denied, and denied in a way the frontend - // can act on: `x-windmill-promote` is what turns a dead end into a sign-up. // `resources/list_names` and the type schemas stay open — a guest drives an app, // and app pickers need them — so the line to pin is the value-returning route. for route in [ @@ -105,13 +109,6 @@ async fn guest_session_is_confined_to_its_app(db: Pool) -> anyhow::Res "guest must be denied {route}, got {}", resp.status() ); - assert_eq!( - resp.headers() - .get("x-windmill-promote") - .map(|v| v.to_str().unwrap()), - Some("1"), - "denial of {route} must be marked promotable" - ); } Ok(()) @@ -232,3 +229,55 @@ async fn guest_entry_needs_both_the_app_mode_and_the_workspace_switch( Ok(()) } + +/// The guest grant is the server-minted label, never the `guest` scope. Scopes on a +/// user-created token are whatever the caller typed, so if the scope granted anything +/// then any member of any workspace could mint themselves non-member access to every +/// guest-mode app on the instance. +#[sqlx::test(fixtures("base"))] +async fn a_self_declared_guest_scope_grants_nothing(db: Pool) -> anyhow::Result<()> { + initialize_tracing().await; + let server = ApiServer::start(db.clone()).await?; + let port = server.addr.port(); + let ws = format!("http://localhost:{port}/api/w/test-workspace"); + + // `users/tokens/create` must refuse the label outright... + let resp = authed( + client().post(format!("http://localhost:{port}/api/users/tokens/create")), + ADMIN_TOKEN, + ) + .json(&json!({ "label": "guest_session", "scopes": guest_scopes() })) + .send() + .await?; + assert_eq!( + resp.status(), + 400, + "the guest session label must be server-minted only" + ); + + // ...and a token that carries the scopes under any other label authenticates as + // nothing in a workspace its owner is not a member of. + // An email with no `usr` row anywhere: exactly the identity the guest arm exists + // to admit, and the one a forged scope must not admit. + sqlx::query( + "INSERT INTO token (token_hash, token_prefix, token, email, label, scopes) + VALUES (encode(sha256($1::bytea), 'hex'), 'FORGED_SCO', $2, 'outsider@example.com', + 'forged', $3)", + ) + .bind(b"FORGED_SCOPES".as_slice()) + .bind("FORGED_SCOPES") + .bind(guest_scopes()) + .execute(&db) + .await?; + + let resp = authed(client().get(format!("{ws}/users/whoami")), "FORGED_SCOPES") + .send() + .await?; + assert_eq!( + resp.status(), + 401, + "declaring the guest scope must not turn a non-member into an identity" + ); + + Ok(()) +} diff --git a/backend/windmill-api-auth/src/auth.rs b/backend/windmill-api-auth/src/auth.rs index c6a824c07d..978a22ac59 100644 --- a/backend/windmill-api-auth/src/auth.rs +++ b/backend/windmill-api-auth/src/auth.rs @@ -427,6 +427,8 @@ impl AuthCache { } (_, Some(email), super_admin, scopes, label, read_only) => { let is_session_token = is_session_label(label.as_deref()); + let is_guest_session = + windmill_common::auth::is_guest_session_label(label.as_deref()); let (username_override, username_override_is_token_label) = username_override_from_label(label); if w_id.is_some() { @@ -517,10 +519,14 @@ impl AuthCache { // (`guest_route_denied`). Placed after the // superadmin arm so a superadmin token can // never be demoted into this one. - None if crate::scopes::has_guest_sentinel( - scopes.as_deref(), - ) => - { + // + // Keyed on the server-minted label, never on + // the `guest` scope: this arm is the only + // thing in the codebase that turns "no `usr` + // row" from a rejection into an identity, and + // scopes on a user-minted token are whatever + // the caller typed. + None if is_guest_session => { Some(ApiAuthed { username: email.clone(), email, diff --git a/backend/windmill-api-auth/src/scopes.rs b/backend/windmill-api-auth/src/scopes.rs index a74f1653f5..a3e9b51ab5 100644 --- a/backend/windmill-api-auth/src/scopes.rs +++ b/backend/windmill-api-auth/src/scopes.rs @@ -501,14 +501,11 @@ pub fn check_route_access( } // A guest session carries the same broad read scopes as an embed token and for - // the same handful of routes, so it gets the same default-deny. The denial is - // `GuestPromotionRequired` rather than `PermissionDenied`: a guest is not short - // one grant, they are short an account, and that is fixable from the browser. - let is_guest = has_guest_sentinel(Some(token_scopes)); - if is_guest { + // the same handful of routes, so it gets the same default-deny. + if has_guest_sentinel(Some(token_scopes)) { if let Some(suffix) = route_suffix.as_deref() { if guest_route_denied(required_domain, suffix) { - return Err(Error::GuestPromotionRequired(format!( + return Err(Error::PermissionDenied(format!( "a guest session cannot access {route_path}" ))); } @@ -598,12 +595,6 @@ pub fn check_route_access( format!("{}:{}", required_domain.as_str(), required_action.as_str()) }; - if is_guest { - return Err(Error::GuestPromotionRequired(format!( - "a guest session cannot access {route_path} (would need {scope_display})" - ))); - } - Err(Error::PermissionDenied(format!( "Access denied. Required scope: {}", scope_display @@ -781,10 +772,11 @@ pub fn has_app_embed_sentinel(scopes: Option<&[String]>) -> bool { } /// Sentinel in a guest session token: someone the identity provider authenticated -/// who is a member of no workspace. Grants nothing itself. It confines the session -/// to the app surface the same way `app_embed` does, and it turns a denial into -/// [`Error::GuestPromotionRequired`] so the frontend offers a real account instead -/// of a dead end. +/// who is a member of no workspace. Grants nothing itself — it only confines the +/// session to the app surface, the same way `app_embed` does. What makes a session a +/// guest at all is the server-minted label +/// [`windmill_common::auth::GUEST_SESSION_LABEL`]; a forged sentinel here can only +/// narrow its own token. pub const GUEST_SENTINEL: &str = "guest"; /// True if a token is a guest session. Such a session has no `usr` row, so its diff --git a/backend/windmill-api-users/src/users.rs b/backend/windmill-api-users/src/users.rs index 4c8bf1098d..9446fa8320 100644 --- a/backend/windmill-api-users/src/users.rs +++ b/backend/windmill-api-users/src/users.rs @@ -2941,6 +2941,9 @@ lazy_static::lazy_static! { /// reads narrowed to a route allowlist by the sentinel (`guest_route_denied`) — plus the /// two path-scoped app grants minted per app. A guest has no `usr` row, so this list is /// the whole of what it can do. +/// +/// The `guest` sentinel here only narrows. What makes the session a guest at all is the +/// server-minted label ([`windmill_common::auth::GUEST_SESSION_LABEL`]). fn guest_session_scopes(app_path: &str) -> Vec { vec![ windmill_api_auth::scopes::GUEST_SENTINEL.to_string(), @@ -2967,7 +2970,11 @@ fn guest_session_scopes(app_path: &str) -> Vec { /// chrome-less public app page calls none of them; a page that needs one for a guest /// has to become workspace-scoped rather than the pin being loosened. /// -/// The caller is responsible for having checked [`is_guest_access_enabled`]. +/// Refuses unless `app_path` is currently in `guest` execution mode, so no caller can +/// mint a guest session for an app that does not admit one. The caller still owes the +/// workspace switch ([`windmill_common::workspaces::is_guest_access_enabled`]) and the +/// authentication of `email` — this function trusts neither the path nor the workspace +/// on its own, only that the identity provider vouched for who is asking. pub async fn create_guest_session_token<'c>( email: &str, w_id: &str, @@ -2987,14 +2994,28 @@ pub async fn create_guest_session_token<'c>( }; let scopes = guest_session_scopes(app_path); + let mode: Option> = sqlx::query_scalar( + "SELECT policy->>'execution_mode' FROM app WHERE workspace_id = $1 AND path = $2", + ) + .bind(w_id) + .bind(app_path) + .fetch_optional(&mut **tx) + .await?; + if mode.flatten().as_deref() != Some("guest") { + return Err(Error::NotAuthorized(format!( + "app {app_path} is not open to guests" + ))); + } + sqlx::query!( "INSERT INTO token (token_hash, token_prefix, token, email, label, expiration, super_admin, scopes, workspace_id) - VALUES ($1, $2, $3, $4, 'session', now() + ($5 || ' seconds')::interval, false, $6, $7)", + VALUES ($1, $2, $3, $4, $5, now() + ($6 || ' seconds')::interval, false, $7, $8)", t_hash, t_prefix, plaintext as Option<&str>, email, + windmill_common::auth::GUEST_SESSION_LABEL, &GUEST_SESSION_VALIDITY_SECONDS.to_string(), &scopes, w_id, diff --git a/backend/windmill-api/openapi.yaml b/backend/windmill-api/openapi.yaml index fea84d9322..cf4f5d1d19 100644 --- a/backend/windmill-api/openapi.yaml +++ b/backend/windmill-api/openapi.yaml @@ -33204,8 +33204,9 @@ components: requires an authenticated viewer), while updating one keeps the mode the app is already deployed under. Neither `anonymous`, which makes the app publicly executable, nor `guest`, which opens it to anyone the - identity provider authenticates, is ever assumed. `guest` is - additionally inert unless the workspace has `guest_access_enabled` + identity provider authenticates, is ever assumed. A guest is only + admitted where the workspace also has `guest_access_enabled`, which + gates both minting a guest session and running an app's components on_behalf_of: type: string on_behalf_of_email: diff --git a/backend/windmill-api/src/apps.rs b/backend/windmill-api/src/apps.rs index 178dad95f3..bf52e28887 100644 --- a/backend/windmill-api/src/apps.rs +++ b/backend/windmill-api/src/apps.rs @@ -357,7 +357,7 @@ pub fn authorize_non_member_viewer( return Ok(true); } if is_guest { - return Err(Error::GuestPromotionRequired(format!( + return Err(Error::PermissionDenied(format!( "app {app_path} is not open to guests" ))); } @@ -1645,7 +1645,11 @@ pub async fn mint_app_embed_token( } let expiration = chrono::Utc::now() + chrono::Duration::hours(APP_EMBED_TOKEN_VALIDITY_HOURS); - let mut scopes: Vec = APP_EMBED_SCOPES.iter().map(|s| s.to_string()).collect(); + let mut scopes: Vec = APP_EMBED_SCOPES + .iter() + .filter(|s| **s != windmill_api_auth::scopes::APP_EMBED_SENTINEL) + .map(|s| s.to_string()) + .collect(); // Path-scoped read so the app can fetch its OWN definition (apps/get/p, // which the in-workspace sandboxed viewer uses) — but no other app's. The // public viewer fetches via apps_u/public_app and doesn't rely on this. @@ -1656,8 +1660,12 @@ pub async fn mint_app_embed_token( scopes.push(format!("apps:run:{app_path}")); // A scope-restricted caller token must not bootstrap a broader-scoped // embed token (`create_token_internal` deliberately does not check this - // itself). No-op for unscoped sessions — the normal embed flow. + // itself). Checked on the real scopes only: a sentinel is a one-part string + // that `ScopeDefinition::from_scope_string` rejects, so leaving it in the + // requested set makes this fail outright for any scoped caller — which a + // guest session is. `mint_raw_app_sdk_token` has the same shape. ensure_scopes_within_caller(authed, Some(&scopes))?; + scopes.push(windmill_api_auth::scopes::APP_EMBED_SENTINEL.to_string()); let token_config = NewToken::new( Some(format!("embed_app:{app_path}")), Some(expiration), @@ -4000,12 +4008,18 @@ async fn execute_component( // A guest session holds no ACL of its own, so the read-permit probe below would // deny every guest. What confines it is the scope the session was minted with, // naming the one app it may run — and the app has to be open to guests at all. + // + // The workspace switch is re-read here rather than trusted from mint time, so + // turning guests off stops them running code within the request, not within the + // session's remaining lifetime. One indexed lookup, and only on the guest path. if let Some(authed) = opt_authed .as_ref() .filter(|a| windmill_api_auth::scopes::has_guest_sentinel(a.scopes.as_deref())) { - if !matches!(policy.execution_mode(), ExecutionMode::Guest) { - return Err(Error::GuestPromotionRequired(format!( + if !matches!(policy.execution_mode(), ExecutionMode::Guest) + || !windmill_common::workspaces::is_guest_access_enabled(&db, &w_id).await? + { + return Err(Error::PermissionDenied(format!( "app {path} is not open to guests" ))); } diff --git a/backend/windmill-common/src/auth.rs b/backend/windmill-common/src/auth.rs index fa1bc96334..ad0c7b4761 100644 --- a/backend/windmill-common/src/auth.rs +++ b/backend/windmill-common/src/auth.rs @@ -56,9 +56,26 @@ pub fn is_server_minted_label(label: &str) -> bool { || label.starts_with("ephemeral-script-end-user-") || label == "ephemeral-script" || label == "session" + || label == GUEST_SESSION_LABEL || label.starts_with("mcp-oauth-") } +/// Label on a guest session — someone the identity provider authenticated who is a +/// member of no workspace. This is the *grant*: `AuthCache` will resolve a token +/// carrying it into an identity with no `usr` row behind it, which nothing else can +/// do. It must therefore stay unforgeable, which is what listing it in +/// [`is_server_minted_label`] buys — `/users/tokens/create` refuses it. +/// +/// Do not move this test onto the token's scopes. Scopes on a user-minted token are +/// caller-supplied and only ever *narrow* (`app_embed`, `raw_app_sdk`), so a scope +/// that granted non-member access would be free for anyone to declare. +pub const GUEST_SESSION_LABEL: &str = "guest_session"; + +/// Whether `label` marks a guest session. See [`GUEST_SESSION_LABEL`]. +pub fn is_guest_session_label(label: Option<&str>) -> bool { + label == Some(GUEST_SESSION_LABEL) +} + /// Whether `label` is the one minted for a browser session at login. [`is_server_minted_label`] /// stops a member minting it directly, but `/users/refresh_token` hands one to any authenticated /// caller, so this attributes a request to the UI without proving it: never gate authority on it. diff --git a/backend/windmill-common/src/error.rs b/backend/windmill-common/src/error.rs index a887db80ba..0d5ebdc2c3 100644 --- a/backend/windmill-common/src/error.rs +++ b/backend/windmill-common/src/error.rs @@ -36,14 +36,6 @@ pub enum Error { MetricNotFound(String), #[error("Permission denied: {0}")] PermissionDenied(String), - /// A guest session asked for something outside the app it was let in for. The - /// response carries `x-windmill-promote`, which the frontend answers by trading - /// the guest session for a real account (`users/promote_guest`). Return it only - /// where promotion could actually resolve the denial, never as a synonym for - /// [`Self::PermissionDenied`]: a member who is merely lacking a grant would be - /// sent through a signup that cannot help them. - #[error("Guest session cannot access this: {0}")] - GuestPromotionRequired(String), #[error("Require Admin privileges for {0}")] RequireAdmin(String), #[error("{0}")] @@ -138,7 +130,6 @@ impl Error { Self::NotAuthorized(_) => "NotAuthorized", Self::MetricNotFound(_) => "MetricNotFound", Self::PermissionDenied(_) => "PermissionDenied", - Self::GuestPromotionRequired(_) => "GuestPromotionRequired", _ => "InternalErr", } } @@ -296,9 +287,7 @@ impl IntoResponse for Error { let status = match self { Self::NotFound(_) => axum::http::StatusCode::NOT_FOUND, Self::NotAuthorized(_) => axum::http::StatusCode::UNAUTHORIZED, - Self::RequireAdmin(_) | Self::PermissionDenied(_) | Self::GuestPromotionRequired(_) => { - axum::http::StatusCode::FORBIDDEN - } + Self::RequireAdmin(_) | Self::PermissionDenied(_) => axum::http::StatusCode::FORBIDDEN, Self::SqlErr { .. } | Self::BadRequest(_) | Self::AIError(_) @@ -319,21 +308,14 @@ impl IntoResponse for Error { let body = Body::from(e.to_string()); - let mut builder = axum::response::Response::builder() + axum::response::Response::builder() .header("Content-Type", "text/plain") - .status(status); - if matches!(e, Self::GuestPromotionRequired(_)) { - builder = builder.header(GUEST_PROMOTE_HEADER, "1"); - } - builder.body(body).unwrap() + .status(status) + .body(body) + .unwrap() } } -/// Marks a denial a guest can resolve by trading their session for a real account. -/// The frontend keys its promotion prompt off this rather than off the message, -/// which is `text/plain` prose. -pub const GUEST_PROMOTE_HEADER: &str = "x-windmill-promote"; - /// Render a `JsonErr` payload as a readable message suitable for direct /// display in a toast: surface the `error` field as the headline, append a /// short summary of `details` (e.g. duplicate paths) when present, and fall diff --git a/frontend/src/lib/components/Login.svelte b/frontend/src/lib/components/Login.svelte index 1534fbeab0..2a5eb23c7b 100644 --- a/frontend/src/lib/components/Login.svelte +++ b/frontend/src/lib/components/Login.svelte @@ -606,6 +606,16 @@ }, 1500) } + /** Mirrors the server-side write in the OAuth `login` handler, including clearing + * it when this sign-in is not a guest entry. `login_externally` consumes it. */ + function setGuestAppCookie(value: string | undefined) { + try { + document.cookie = `guest_app=${encodeURIComponent(value ?? '')}; path=/; SameSite=None; Secure` + } catch (e) { + console.error('Could not set the guest app cookie', e) + } + } + function redirectSaml(): boolean { if (!saml) { sendUserToast('No SAML login available', true) @@ -613,6 +623,13 @@ } if (previewConfig) return true markLoginMethodPending({ kind: 'saml' }) + // SAML goes straight to the IdP and never passes through + // `/api/oauth/login/`, which is where the OAuth path has the server + // write this cookie. Write it here so a SAML-only instance can admit guests + // too. Client-set is safe: the callback still checks that the named app is in + // guest mode and that the workspace allows guests, so the worst a forged value + // can do is give its own author a narrower session than they'd otherwise get. + setGuestAppCookie(guestApp) let target = saml let relayStateSet = false // Carry the SP-initiated deep link through the IdP round-trip via SAML diff --git a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte index 27e8b8a1bf..770b2130fa 100644 --- a/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte +++ b/frontend/src/lib/components/apps/editor/AppEditorHeaderDeploy.svelte @@ -465,8 +465,10 @@ /` when this app is open to guests. The embedder's + * login gate fires before the page's own load, so the page must resolve this + * up front and pass it down — otherwise a signed-out visitor is offered an + * ordinary sign-in that creates an account and still cannot open the app. */ + guestAppPath?: string | undefined } = $props() const EMBED_PARAM = 'wm_embed' @@ -194,7 +200,14 @@ } // ---------------------------- embedder mode ---------------------------- - let status: 'loading' | 'ready' | 'noPermission' | 'notExists' | 'sdkPrompt' = $state('loading') + type FrameStatus = 'loading' | 'ready' | 'noPermission' | 'notExists' | 'sdkPrompt' + let status = $state('loading') + /** Set once a sign-in completed on this page. A second `noPermission` after that + * is not something signing in again can fix — an identity that already has an + * account is never given a guest session, so an account holder who is not a + * member of this workspace lands here. */ + let signedInHere = $state(false) + let signInDidNotHelp = $derived(status === 'noPermission' && signedInHere) let embedToken: string | null = $state(null) let iframeEl: HTMLIFrameElement | undefined = $state(undefined) @@ -532,14 +545,39 @@ {:else if status === 'noPermission'} -
This app requires read access
-
- initEmbedder()} - popup - rd={page.url.pathname + page.url.search + page.url.hash} - /> -
+ {#if signInDidNotHelp} + +
+ You are signed in, but this app is not open to you +
+
+ It is open to members of its workspace, and to guests who have no Windmill account. Ask the + person who shared it to give your account access. +
+ {:else} + {#if guestAppPath} +
Sign in to open this app
+
+ You do not need a Windmill account. Signing in lets you open this app and nothing else. +
+ {:else} +
+ This app requires read access +
+ {/if} +
+ { + signedInHere = true + initEmbedder() + }} + popup + guestApp={guestAppPath} + rd={page.url.pathname + page.url.search + page.url.hash} + /> +
+ {/if} {:else if unsandboxed}