go-pkgz / auth / 25571721674

85%

master: 85%

Build Type

Pull #282

github

Committed by paskal

Commit Message

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

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 to v1 with the same opt-in policy:

* 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 (oauth1.go:165, oauth2.go:241, apple.go:395, verify.go:141)
  gate on it and fall back to the existing JSON user-info response on
  rejection (with a [WARN] log via redirectHostForLog so attacker-
  supplied paths/queries do not leak into logs).

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
is case-insensitive and ignores the default port; non-http(s) schemes
(javascript:, data:, ftp:) are rejected.

Tests:
* TestIsAllowedRedirect — 24 table cases covering permissive default,
  typed-nil guard, port equivalence, case-insensitivity, scheme
  rejection, allowlist matching.
* TestRedirectHostForLog — 5 cases.
* TestOauth2LoginFromRejectsExternalHost / TestOauth2LoginFromAllowsAllowlistedHost
  — integration coverage of the oauth2 path (negative + positive).
* TestVerifyHand... (continued)

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

Coverage Stats

105 of 118 new or added lines in 7 files covered. (88.98%)

8 existing lines in 1 file now uncovered.

2763 of 3274 relevant lines covered (84.39%)

7.46 hits per line