fix: validate OIDC redirect URL protocol before navigation

[ital0]

Summary

• Adds validateOidcRedirectUrl to reject javascript:, data:, and non-HTTPS (except localhost) redirect URLs before navigating, preventing open-redirect and XSS via a malicious OIDC provider response
• Validation errors propagate to the existing catch block and surface in the console
• Adds 8 pure-function unit tests covering HTTPS, localhost, protocol injection, and malformed URLs

Closes THU-375 #25

Test plan

• bun run type-check passes
• bun test src/components/oidc-redirect passes (8/8, stable across 5 reruns)
• Manual: verify OIDC login flow still redirects correctly in staging

🤖 Generated with Claude Code

---

Note

Medium Risk
Touches the OIDC sign-in redirect flow and adds a new backend endpoint used during login, so a mistake could block authentication or mis-validate redirects. The changes are small and covered by unit tests, reducing risk.

Overview
Hardens the OIDC login redirect by validating the provider-supplied redirect URL before navigation, rejecting non-https: schemes (except http for localhost/127.0.0.1) and optionally enforcing an expected issuer origin.

Adds a lightweight backend GET /v1/auth/oidc/config endpoint (only active in authMode: 'oidc') that returns the configured issuer origin for the frontend to validate against, plus unit tests for both the frontend validator and the new backend route.

^{Reviewed by Cursor Bugbot for commit 3be7c6d. Bugbot is set up for automated code reviews on this repo. Configure here.}

[github-actions]

Semgrep Security Scan

No security issues found.

[github-actions]

PR Metrics

Metric	Value
Lines changed (prod code)	+58 / -2
JS bundle size (gzipped)	🟢 1.00 MB → 1023.6 KB (-4.5 KB, -0.4%)
Test coverage	🟢 70.40% → 70.38% (+-0.0%)
Load time (preview)	🔴 46/100 · First Contentful Paint 5.0 s · Largest Contentful Paint 5.8 s · Total Blocking Time 710 ms

Updated Wed, 15 Apr 2026 14:03:16 GMT · run #867

[greptile-apps]

Greptile Summary

This PR adds client-side validation of the OIDC redirect URL before navigating, closing an open-redirect / XSS vector where a malicious OIDC provider response could redirect to a javascript: or data: URI. It also introduces a new unauthenticated /v1/auth/oidc/config backend endpoint that surfaces the configured issuer origin, which the frontend uses for optional origin-pinning on top of the protocol check.

Key changes:

• validateOidcRedirectUrl rejects any non-HTTPS URL except http://localhost and http://127.0.0.1 (dev), and throws if the URL origin doesn't match the backend-supplied issuer origin
• Both concerns raised in the previous review (overly-broad localhost check, missing javascript://localhost/ edge-case test) are now fully addressed
• A user-facing error UI replaces the silent failure when the OIDC redirect fails
• The backend /auth/oidc/config route correctly returns 404 in non-OIDC deployments, keeping the feature isolated
• The "best-effort" fallback (continuing with protocol-only validation when the config fetch fails) is intentional and documented — the sign-in request itself would also fail in that scenario

Confidence Score: 5/5

Safe to merge — the security fix is correct, both prior review concerns are addressed, and no logic bugs were found.

The validation logic is sound: the http: + localhost/127.0.0.1 explicit check eliminates the overly-broad localhost exemption, and the javascript://localhost/ edge case is now tested. The backend route is minimal, correctly scoped to OIDC mode, and cleanly wired into the existing prefix. Remaining comments are P2 style (unused import, test duplication) that don't affect runtime behaviour.

No files require special attention; backend/src/auth/oidc.test.ts has minor style issues worth cleaning up but nothing blocking.

Important Files Changed

Filename	Overview
src/components/oidc-redirect.tsx	Adds validateOidcRedirectUrl (HTTPS + localhost guard, optional origin-pinning) and a user-facing error state; previous review concerns about the localhost exemption breadth and the javascript://localhost/ edge case are both addressed.
src/components/oidc-redirect.test.ts	13 unit tests covering HTTPS, localhost, 127.0.0.1, javascript://, data:, origin-pinning, and malformed URLs — comprehensive and all edge-case suggestions from prior review are now present.
backend/src/auth/oidc.ts	New unauthenticated /auth/oidc/config route that exposes the configured OIDC issuer origin; correctly returns 404 in non-OIDC mode and mounts cleanly under the app's existing /v1 prefix.
backend/src/auth/oidc.test.ts	Tests for the /auth/oidc/config route cover OIDC mode, consumer mode, and path-stripping; minor issues: unused beforeAll import and significant mock duplication across all three test cases.
backend/src/index.ts	Single-line addition wiring createOidcConfigRoutes() into the existing app builder — no issues.

Sequence Diagram

sequenceDiagram
    participant App as OidcRedirect (frontend)
    participant Backend
    participant Provider as OIDC Provider

    App->>Backend: GET /v1/auth/oidc/config
    alt OIDC mode enabled
        Backend-->>App: { issuerOrigin: "https://auth.example.com" }
    else Non-OIDC / unavailable
        Backend-->>App: 404 / error → expectedOrigin = undefined
    end

    App->>Backend: POST /v1/api/auth/sign-in/oauth2
    Backend-->>App: { url: "https://auth.example.com/authorize?..." }

    App->>App: validateOidcRedirectUrl(url, expectedOrigin)
    alt Protocol is javascript: or data:
        App-->>App: throw → show error UI
    else HTTP on non-localhost
        App-->>App: throw → show error UI
    else Origin mismatch (when expectedOrigin set)
        App-->>App: throw → show error UI
    else Valid URL
        App->>Provider: window.location.href = validatedUrl.href
    end

Loading

_{Reviews (6): Last reviewed commit: "feat: add OIDC issuer origin validation ..." | Re-trigger Greptile}

[claude]

LGTM. The OIDC redirect validation is correct and covers the relevant attack vectors. Tests are comprehensive.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit d2d065a. Configure here.}

[claude]

Review

Security

• Protocol-only check doesn't prevent open redirect (src/components/oidc-redirect.tsx:9): The validator blocks javascript: and plain http: but passes any HTTPS URL — including https://attacker.com. If the backend returns a compromised or unexpected URL, users get silently redirected to an arbitrary HTTPS domain. The stated goal of the check seems to be protecting against redirect hijacking, but protocol alone is insufficient. Consider comparing the redirect URL's origin against the configured cloudUrl value already available in the component.

Bugs

• Silent failure on validation error (src/components/oidc-redirect.tsx:50): If validateOidcRedirectUrl throws (malformed URL from backend, bad protocol), the error falls through the AbortError guard and is swallowed with console.error, leaving the user stuck on <Loading /> indefinitely. A validation failure is not a transient network error — it should surface as visible error state.

[claude]

Code Review

One convention violation found.

---

Project Convention Compliance

src/components/oidc-redirect.tsx:64 — text-sm used instead of text-[length:var(--font-size-sm)]. CLAUDE.md explicitly lists text sizes as custom CSS variables with no Tailwind equivalent: "Text: text-[length:var(--font-size-body)], text-[length:var(--font-size-sm)], text-[length:var(--font-size-xs)]".

---

Everything else looks good. The validateOidcRedirectUrl allowlist correctly blocks javascript:, data:, and HTTP non-localhost redirects before they reach window.location.href.

[claude]

Code Review

Bugs & Data Loss

Error state is unrecoverable — src/components/oidc-redirect.tsx:52

Once setError(...) is called, the component renders the error view and the useEffect never re-runs (dependencies cloudUrl.isLoading / cloudUrl.value don't change after load). The message tells the user to "try again" but there's no mechanism to do so — no retry button, no reset of the error state. The user is stuck until they hard-reload.

Security / Breaking Changes: No issues found.

Convention Compliance: No issues found.

[claude]

Code Review — OIDC Redirect URL Validation

No bugs, security issues, or convention violations found above the reporting threshold.

The validateOidcRedirectUrl function correctly blocks javascript:, data:, and other dangerous protocols using strict equality on url.protocol. The javascript://localhost/ edge case (where the hostname would otherwise pass the localhost check) is handled correctly and covered by a test. Using validatedUrl.href (the URL object's normalized form) rather than the raw string is the right call.

One thing to be aware of: the error state is never reset, so if the OIDC fetch fails, the error UI persists until the user reloads the page. Given that the effect only fires once (when cloudUrl loads) and OIDC failure is typically a configuration problem rather than a transient one, this is acceptable — just note that "Please try again" in the error copy implies a retry mechanism that doesn't exist yet.

Looks good to merge. ✅

[claude]

Code Review

Security

src/components/oidc-redirect.tsx — Origin check silently disabled on config fetch failure

The inner catch {} discards all errors from the /v1/auth/oidc/config fetch, leaving expectedOrigin as undefined. When that happens, validateOidcRedirectUrl accepts any HTTPS URL — the whole point of fetching the issuer origin is to constrain which host is accepted. A transient network error, timeout, or misconfiguration between the two requests is enough to silently drop the origin check. If this is intentionally best-effort, the fallback behavior should be documented and the failure should at minimum be logged. If the origin check is load-bearing for security, the outer try/catch should treat a missing expectedOrigin as a hard failure rather than a fallback.

---

Convention

backend/src/auth/oidc.test.ts — .then() chains instead of async/await

All three test cases use return app.handle(...).then(async (response) => { ... }). CLAUDE.md: "Prefer async/await over .then/.catch". Each it callback should be declared async and the handle call awaited directly.