fix: guest target survives http (Lax cookie), rides SAML RelayState; tell account holders on arrival

backend/ee-repo-ref.txt

@@ -1 +1 @@
-163a335a4f4e9ae179ffb96fc92ee1681cb44e60
+9623e36c58c51e085739636efd44dd5cecfd24af

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<boolean> {
-		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<boolean> {
+	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/<client>`, 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/<client>`,
+		// 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

frontend/src/lib/components/apps/editor/PublicAppFrame.svelte

@@ -208,11 +208,12 @@
 	// ---------------------------- embedder mode ----------------------------
 	type FrameStatus = 'loading' | 'ready' | 'noPermission' | 'notExists' | 'sdkPrompt'
 	let status = $state<FrameStatus>('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))}
 	<Skeleton layout={[[4], 0.5, [50]]} />
 {:else if offerSignIn && (guestEntry === 'error' || staleGuestLogoutFailed)}
 	<div class="px-4 mt-20">
@@ -595,8 +603,9 @@
 			You are signed in, but this app is not open to you
 		</div>
 		<div class="text-center mt-8 text-sm text-primary">
-			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.
 		</div>
 	{:else}
 		{#if guestAppPath}
@@ -612,7 +621,7 @@
 		<div class="px-2 mx-auto mt-20 max-w-xl w-full">
 			<Login
 				onLoginSuccess={() => {
-					signedInHere = true
+					accountSession = 'unknown'
 					initEmbedder()
 				}}
 				popup