diff --git a/backend/ee-repo-ref.txt b/backend/ee-repo-ref.txt index 8adbba84ec..06d07b2240 100644 --- a/backend/ee-repo-ref.txt +++ b/backend/ee-repo-ref.txt @@ -1 +1 @@ -163a335a4f4e9ae179ffb96fc92ee1681cb44e60 +9623e36c58c51e085739636efd44dd5cecfd24af \ No newline at end of file diff --git a/frontend/src/lib/components/Login.svelte b/frontend/src/lib/components/Login.svelte index 8629f6496a..177c203c46 100644 --- a/frontend/src/lib/components/Login.svelte +++ b/frontend/src/lib/components/Login.svelte @@ -404,9 +404,7 @@ if (autoRedirect && autoLogin && !error && !shouldSkipAutoRedirect()) { if (autoLogin === 'saml' && saml) { autoRedirecting = true - redirectSaml().then((ok) => { - if (!ok) autoRedirecting = false - }) + if (!redirectSaml()) autoRedirecting = false } else if (logins?.some((l) => l.type === autoLogin)) { autoRedirecting = true if (!storeRedirect(autoLogin)) { @@ -614,46 +612,13 @@ }, 1500) } - /** Have the server write the guest-entry cookie, as the OAuth `login` handler does - * on its own path. SAML goes straight to the IdP and never passes through `login`, - * and a browser-set cookie would be host-only — on a `COOKIE_DOMAIN` deployment - * whose ACS answers on a sibling host it would never arrive. Server-set, it carries - * the same attributes as every other session cookie. Cleared (empty) when this - * sign-in is not a guest entry. `login_externally` consumes it. */ - async function setGuestAppCookie(value: string | undefined): Promise { - try { - const res = await fetch(`${base}/api/oauth/guest_app`, { - method: 'POST', - credentials: 'include', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ guest_app: value ?? '' }) - }) - return res.ok - } catch (e) { - console.error('Could not set the guest app cookie', e) - return false - } - } - - async function redirectSaml(): Promise { + function redirectSaml(): boolean { if (!saml) { sendUserToast('No SAML login available', true) return false } if (previewConfig) return true markLoginMethodPending({ kind: 'saml' }) - // SAML goes straight to the IdP and never passes through - // `/api/oauth/login/`, where the OAuth path has the server write the - // guest-entry cookie; ask for the same write here. With a guest target it must - // have landed before we leave — without it the callback provisions an account — - // so that failure is a stop. Without one this is only a clear, and the callback - // clears on consume anyway: an ordinary SAML sign-in must not depend on it. - const wrote = await setGuestAppCookie(guestApp) - if (guestApp && !wrote) { - clearPendingLoginMethod() - sendUserToast('Could not start sign-in, please try again.', true) - return false - } let target = saml let relayStateSet = false // Carry the SP-initiated deep link through the IdP round-trip via SAML @@ -662,7 +627,17 @@ // full URLs (e.g. the page URL from /a/[...path]) are reduced to their // path component first. The backend re-validates. Cross-origin or // otherwise unsafe values fall through to the localStorage fallback. - const safePath = toSameOriginRelativePath(rd) + // A guest entry rides in the same RelayState as a `guest_app` query parameter + // the ACS lifts out: SAML never passes through `/api/oauth/login/`, + // where the OAuth path hands its target to the server. + let safePath = toSameOriginRelativePath(rd) + if (guestApp && safePath) { + const hashAt = safePath.indexOf('#') + const pathAndQuery = hashAt === -1 ? safePath : safePath.slice(0, hashAt) + const hash = hashAt === -1 ? '' : safePath.slice(hashAt) + const sep = pathAndQuery.includes('?') ? '&' : '?' + safePath = `${pathAndQuery}${sep}guest_app=${encodeURIComponent(guestApp)}${hash}` + } if (safePath) { try { const url = new URL(saml) @@ -673,6 +648,13 @@ console.error('Could not set SAML RelayState', e) } } + if (guestApp && !relayStateSet) { + // Without the target the callback provisions an account, so a guest + // sign-in that cannot carry it does not start. + clearPendingLoginMethod() + sendUserToast('Could not start sign-in, please try again.', true) + return false + } // Only use the localStorage fallback when RelayState is NOT carrying the // deep link. With RelayState the ACS redirects straight to the target and // /user/login never consumes/clears the key, so a persisted value would diff --git a/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte b/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte index d4924be17c..652ae4ce59 100644 --- a/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte +++ b/frontend/src/lib/components/apps/editor/PublicAppFrame.svelte @@ -208,11 +208,12 @@ // ---------------------------- embedder mode ---------------------------- 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) + /** Whether the visitor holds an account session, probed whenever the app denies + * them: `/api/users/email` answers for an account and never for a guest (pinned + * to its workspace) or for nobody. An account this app still refuses is not + * something signing in again can fix — an identity with an account is never given + * a guest session — so the card gives way to an explanation. */ + let accountSession = $state<'unknown' | 'none' | 'held'>('unknown') let deniedStatus: number | undefined = $state(undefined) /** The sign-in card belongs on a 401, and on a 403 unless discovery has settled * that the app is not open to guests — a 403 on a guest app is a session for a @@ -223,7 +224,14 @@ status === 'noPermission' || (status === 'notExists' && deniedStatus === 403 && guestEntry !== 'none') ) - let signInDidNotHelp = $derived(offerSignIn && signedInHere) + let signInDidNotHelp = $derived(offerSignIn && accountSession === 'held') + $effect(() => { + if (offerSignIn && accountSession === 'unknown') { + UserService.getCurrentEmail() + .then(() => (accountSession = 'held')) + .catch(() => (accountSession = 'none')) + } + }) // The stale guest session must be gone before the card mounts: it still // authenticates in this workspace, so the popup's success poll would see it and @@ -577,7 +585,7 @@ onContinue={onSdkConsentContinue} onDecline={onSdkConsentDecline} /> -{:else if offerSignIn && (guestEntry === 'pending' || (deniedStatus === 403 && guestEntry === 'guest' && !staleGuestCleared && !staleGuestLogoutFailed))} +{:else if offerSignIn && (guestEntry === 'pending' || accountSession === 'unknown' || (deniedStatus === 403 && guestEntry === 'guest' && !staleGuestCleared && !staleGuestLogoutFailed))} {:else if offerSignIn && (guestEntry === 'error' || staleGuestLogoutFailed)}
@@ -595,8 +603,9 @@ 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. + It is open to members of its workspace{guestAppPath + ? ', and to guests who have no Windmill account' + : ''}. Ask the person who shared it to give your account access.
{:else} {#if guestAppPath} @@ -612,7 +621,7 @@
{ - signedInHere = true + accountSession = 'unknown' initEmbedder() }} popup