fix(provider): backport "from" redirect validator to v1 (sibling of #275)

[paskal]

Summary

The "from" query parameter accepted by oauth1 / oauth2 / apple / verify login handlers was stored verbatim in the handshake JWT and used as the redirect target after a successful auth handshake with no validation. Any external URL passed as from became a 307 redirect after the user completed the real OAuth flow with the legitimate provider — usable for phishing and post-auth landing-page substitution.

This is the same vulnerability fixed in v2 by #275. v1 was untouched. This PR ports the validator with the same opt-in policy.

Changes

• token.AllowedHosts (interface) + AllowedHostsFunc (adapter), mirroring the existing token.Audience pattern.
• Opts.AllowedRedirectHosts threaded through provider.Params, AppleHandler (via embedded Params) and VerifyHandler (own URL + AllowedRedirectHosts fields).
• provider.isAllowedRedirect centralises the check; all four redirect call sites gate on it and fall back to the existing JSON user-info response on rejection, logging via redirectHostForLog so attacker-supplied paths/queries do not leak into logs:
  • oauth1.go:165 → AuthHandler
  • oauth2.go:241 → AuthHandler
  • apple.go:395 → LoginHandler
  • verify.go:141 → LoginHandler

Policy

Default (nil allowlist) is permissive — preserves pre-feature behaviour so existing consumers see no change. Hardening is enabled by setting Opts.AllowedRedirectHosts. Passing an AllowedHostsFunc that returns nil restricts redirects to the service URL host only.

• Hostname comparison: case-insensitive, port-insensitive (https://x and https://x:443 match)
• Non-http(s) schemes rejected (javascript:, data:, ftp:)
• Relative / unparseable URLs rejected when policy is on
• Typed-nil AllowedHostsFunc guarded (treated as no allowlist configured)

Tests

• TestIsAllowedRedirect — 24 table cases covering permissive default, typed-nil guard, port equivalence, case-insensitivity, scheme rejection, allowlist matching, getter error handling.
• TestRedirectHostForLog — 5 cases.
• TestOauth2LoginFromRejectsExternalHost / TestOauth2LoginFromAllowsAllowlistedHost — integration coverage of the oauth2 path (negative + positive).
• TestVerifyHandler_LoginAcceptConfirmFromRejectsExternalHost / TestVerifyHandler_LoginAcceptConfirmFromAllowsAllowlistedHost — integration coverage of the verify path (negative + positive).

oauth1 and apple share the same code path through the embedded Params; the unit tests and shared isAllowedRedirect cover them.

Full go test -race ./... green; golangci-lint run --new-from-rev=master 0 issues.

Credit

Thanks to Admir Bajric (@AdmirBajric) for flagging on 2026-05-08 that v1 was still vulnerable after #275 fixed v2 — that report prompted this backport.

[coveralls]

Coverage Report for CI Build 25587333503

Coverage increased (+0.03%) to 84.904%

Details

• Coverage increased (+0.03%) from the base build.
• Patch coverage: 7 of 7 lines across 1 file are fully covered (100%).
• No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.

---

Coverage Stats

Relevant Lines:	3332
Covered Lines:	2829
Line Coverage:	84.9%
Coverage Strength:	7.86 hits per line

---

💛 - Coveralls

[umputun]

backport itself is byte-faithful to v2. provider/redirect.go, provider/redirect_test.go, and the token.AllowedHosts/AllowedHostsFunc addition all match v2 modulo the import path. Wiring in auth.go mirrors v2 line-for-line. Tests + race + lint clean in both modules.

three findings worth addressing, the first one is a real bug that lives in both v1 (this PR) and v2 (#275 as merged):

1. verify flow: the redirect validator can't fire in production. sendConfirmation at provider/verify.go:170-187 builds Handshake{State: "", ID: user + "::" + address} and never copies r.URL.Query().Get("from"). The validator block at provider/verify.go:151 only sees confClaims.Handshake.From, which is empty for every real login URL. The integration test passes because it directly mints a confirmation token with Handshake.From set, but production code can't produce such a token (the JWT is signed). Verified the same gap exists in v2/provider/verify.go:170-187, so it's not v1 drift, it's a bug in #275 that this PR faithfully reproduces. The README claims ?from=<url> on /auth/<name>/login is subject to the allowlist, but production never honors from for verify in the first place. Two-part fix:

   • in both v1 and v2, add From: r.URL.Query().Get("from") to the Handshake literal in sendConfirmation.
   • replace the synthetic-token verify integration test with one that drives the public /auth/<name>/login?from=... path end-to-end, so this kind of regression can't recur.

   Since the underlying bug is in v2 too, this might warrant a separate issue/PR rather than blocking #282 on it. Up to you whether to land this PR as-is and file a follow-up, or fold the fix into both modules here.

2. missing oauth1 + apple rejection integration tests. The v2 module ships TestOauth1LoginFromRejectsExternalHost and TestAppleHandler_LoginHandlerFromRejectsExternalHost as direct regression tests for the new gates at provider/oauth1.go:166 and provider/apple.go:396. The v1 PR omits both. The "shared code path through embedded Params" rationale in the description is true at the unit level (TestIsAllowedRedirect covers the validator), but the rejection-branch wiring at the handler boundary is uncovered for these two providers, which is exactly what coveralls is flagging. About 30 lines each, copy-paste-ready from v2.

3. untested bypass categories (also missing from v2, pre-existing gap, low priority): scheme-relative URLs (//evil.com), userinfo bypass (https://allowed@evil.com), IPv6 ([::1]), IDN/punycode, percent-encoded hostnames, backslash variants. Verified manually that each is correctly rejected by url.Parse+Hostname() semantics, so the validator is right, just not pinned. Worth a tracking issue for both modules.

note: coveralls' uncovered line at token/jwt.go:443 (AllowedHostsFunc.Get) is a false positive. The body is exercised through every test that constructs an AllowedHostsFunc, but coveralls' per-package report doesn't see cross-package execution. The v2 module has the same false positive and was merged. Not blocking.

[paskal]

addressed all three findings in the last push:

1. real bug from fix: validate "from" redirect target in OAuth/verify flows #275: sendConfirmation now copies From: r.URL.Query().Get("from") into the Handshake claim in both v1 (this PR) and v2 (so the v2 module's redirect validator now actually fires in production too -- previously the JWT carried an empty Handshake.From and the validator block in LoginHandler never ran). new integration test TestVerifyHandler_PublicFromFlow_RejectsExternalHost drives the public flow end-to-end (/login?from=... -> sendConfirmation -> email-link click -> /login?token=...) with an explicit assertion on parsed.Handshake.From == "https://evil.example.com/phish" to catch any future regression of this exact kind.

2. v1 oauth1+apple rejection integration tests added: TestOauth1LoginFromRejectsExternalHost in provider/oauth1_test.go and TestAppleHandler_LoginHandlerFromRejectsExternalHost in provider/apple_test.go. mirror the v2 versions, plumbed through paramOpts ...func(*Params) on prepOauth1Test and prepareAppleOauthTest. closes the parity gap coveralls was flagging.

3. untested bypass categories (scheme-relative, userinfo, IPv6, IDN/punycode, percent-encoded, backslash) -- agree with the low-priority assessment, leaving as a separate follow-up issue rather than expanding this PR.

go fix + race + lint clean in both modules.

[paskal]

folded the v1 + v2 URL bypass-category characterization tests into TestIsAllowedRedirect:

• scheme-relative //evil.com/x
• userinfo bypass https://admin.example.com@evil.com/x (rejected) + https://evil@admin.example.com/x (allowed -- userinfo doesn't change the host check)
• IPv6 https://[::1]/x (rejected unless explicitly listed)
• IDN homoglyph (cyrillic а) and its punycode form not equal to ASCII allowlist entry
• percent-encoded host parse-fails
• backslash userinfo trick parse-fails
• opaque scheme:host forms (https:evil.com, https:///evil.com)

pure characterisation -- no behaviour change. v2 tests added too even though v2 redirect.go is already merged in #275, so the bypass coverage stays in lockstep across both modules.

[umputun]

all 3 round-1 findings addressed cleanly:

1. sendConfirmation Handshake.From bug fixed in both v1 and v2; new TestVerifyHandler_PublicFromFlow_RejectsExternalHost drives the public flow end-to-end with the regression-pin assertion on parsed.Handshake.From.
2. v1 oauth1 + apple rejection integration tests added, mirroring v2.
3. bypass-category characterization tests folded in even though I marked them low-priority deferrable. Nice. Coveralls now SUCCESS at 84.9% (+0.03%).

LGTM, thx.