Files
windmill/frontend/src/lib/logoutRedirect.ts
T
Ruben Fiszelandwindmill-internal-app[bot] 78cf6c7f81 fix(saml): preserve deep links from /a/[...path] across SAML round-trip (#9259)
* [ee] fix(saml): preserve deep links from /a/[...path] across SAML round-trip

Fixes WIN-1962.

PR #9225 only covered users who pass through /user/login on their way to
the IdP — that's where `redirectSaml()` runs and where the deep link gets
stuffed into `RelayState`. The reported flow doesn't go through that
page: it hits `/a/[...path]` (the public-app custom-path route, outside
the `(logged)` layout) where `PublicApp.svelte` renders its own `<Login>`
and was passing `page.url.toString()` as `rd` — the full URL.

Three problems compounded:

1. `redirectSaml()` only set `RelayState` when `rd.startsWith('/')`,
   so a full URL silently fell through and the deep link was lost.
   The IdP echoed back the SP-library default (BASE_URL), which the
   ACS validator correctly rejected as a potential open-redirect.
2. `persistRd()` stored the full URL in `localStorage.rd`. On the
   fallback landing at `/user/login`, the post-login redirect saw
   an `http://...` value, hit the cross-origin branch, and bounced
   to `/` — which from a logged-in but workspace-less state shows
   the "Loading user…" modal forever (bug 2).
3. The EE `safe_relay_state_redirect` validator rejected any full
   URL, including same-origin ones, so even IdPs that prepend the
   origin or that pass a configured absolute deep link via
   IdP-initiated SSO got dropped on the floor.

The fix is a single concept applied at every layer: reduce a redirect
target to a safe same-origin relative path, or refuse it.

Frontend:
- `logoutRedirect.ts`: new `toSameOriginRelativePath(rd)` helper that
  accepts both `/foo` and `https://current-origin/foo`, with the same
  open-redirect guards as the backend (length cap, control chars, no
  protocol-relative or back-slash tricks). Returns `null` for
  cross-origin or malformed input.
- `PublicApp.svelte`: pass `pathname + search + hash` to `<Login>`
  instead of the full URL — this alone fixes the happy path.
- `Login.svelte`: `redirectSaml()`, `persistRd()`, and `redirectUser()`
  all route through the helper, so full URLs from `/a/[...path]` are
  reduced before being put in `RelayState`/`localStorage`/`goto()`.
- `/user/login/+page.svelte`: the same reduction is applied to the
  resolved `rd` so any stale full-URL value in `localStorage.rd` still
  navigates to the intended page instead of falling into the
  cross-origin branch.

Backend (EE companion: windmill-ee-private#TBD):
- `safe_relay_state_redirect` now reduces a `RelayState` whose origin
  matches `BASE_URL` to its path before applying the same-origin path
  safety rules. Bare BASE_URL with no path still falls back to
  `/user/login` (no useful deep link to honor).
- New `same_origin_relative_path` helper + expanded unit tests.

Test plan:
- [x] Frontend: `vitest run src/lib/logoutRedirect.test.ts` — 9 passed
- [x] Backend: `cargo test -p windmill-api ... saml_ee::tests` — 3 passed
  (`honors_same_origin_relative_path`, `reduces_same_origin_full_url_to_path`,
  `falls_back_on_open_redirect_attempts`)
- [ ] Manual e2e (needs configured SAML IdP — not on local CE):
  - Unauthenticated visit to `/a/<path>` → click SSO → SAML → land on
    `/a/<path>` (RelayState now carries the relative path).
  - IdP that echoes BASE_URL as default → ACS still falls back to
    `/user/login` (no useful path to honor), but the page no longer
    hangs: the stale full-URL `localStorage.rd` is reduced to its path
    and the post-login redirect navigates to it.
  - Tampered `RelayState` (`//evil.com`, `https://evil.com/x`) → ACS
    rejects, lands on `/user/login`.

* chore: update ee-repo-ref to 3489c243b0e5a8eb0dbc86e90917fbe72843573b

This commit updates the EE repository reference after PR #584 was merged in windmill-ee-private.

Previous ee-repo-ref: 635ff3eeb8e47bb84d5686942605f67f8f6224b4

New ee-repo-ref: 3489c243b0e5a8eb0dbc86e90917fbe72843573b

Automated by sync-ee-ref workflow.

---------

Co-authored-by: windmill-internal-app[bot] <windmill-internal-app[bot]@users.noreply.github.com>
2026-05-20 12:46:58 +00:00

64 lines
1.9 KiB
TypeScript

import { get } from 'svelte/store'
import { hubBaseUrlStore } from './stores'
export function isValidLogoutRedirect(url: string): boolean {
if (url.startsWith('/') && !url.startsWith('//')) {
return true
}
try {
const parsed = new URL(url)
const host = parsed.hostname
if (host === 'windmill.dev' || host.endsWith('.windmill.dev')) {
return true
}
const hubBaseUrl = get(hubBaseUrlStore)
try {
const hubHost = new URL(hubBaseUrl).hostname
if (host === hubHost) {
return true
}
} catch {}
} catch {}
return false
}
/**
* Reduces a redirect target to a safe same-origin relative path.
*
* Returns the path (`/foo?bar#baz`) when the input is either:
* - already an absolute-but-relative path (`/foo`) that isn't protocol-relative
* (`//evil.com`) or a back-slash trick (`/\\evil.com`), or
* - a full URL whose origin matches the current page's origin.
*
* Returns `null` for anything else (cross-origin URLs, protocol-relative paths,
* malformed input). Used to sanitize values flowing into `RelayState`,
* `localStorage.rd`, and post-login redirects so we don't either lose a valid
* same-origin deep link or open-redirect into a foreign origin.
*/
export function toSameOriginRelativePath(rd: string | null | undefined): string | null {
if (!rd) return null
if (rd.length > 2048) return null
if (hasControlChar(rd)) return null
if (rd.startsWith('/')) {
if (rd.startsWith('//') || rd.startsWith('/\\')) return null
return rd
}
if (typeof window === 'undefined') return null
try {
const url = new URL(rd, window.location.origin)
if (url.origin !== window.location.origin) return null
const path = url.pathname + url.search + url.hash
return path || '/'
} catch {
return null
}
}
function hasControlChar(s: string): boolean {
for (let i = 0; i < s.length; i++) {
const c = s.charCodeAt(i)
if (c < 0x20 || c === 0x7f) return true
}
return false
}