fix: validate "from" redirect target in OAuth/verify flows by paskal · Pull Request #275 · go-pkgz/auth

paskal · 2026-04-18T03:46:37Z

The from query parameter accepted by oauth1, oauth2, apple and verify login flows tells the server where to send the user after a successful auth handshake. Before this PR the value was stored verbatim in the handshake JWT and used as the redirect target with no validation — any external URL fits, so a crafted login link could route a user through legitimate provider OAuth and then bounce them to an attacker-controlled landing page.

Attack

https://comments.example.com/auth/github/login?from=https://evil.example.com/phish

Victim sees the real GitHub consent screen and approves. Provider then calls back to the legitimate AuthHandler, which (pre-fix) executes:

http.Redirect(w, r, oauthClaims.Handshake.From, http.StatusTemporaryRedirect)

The 307 lands the user on evil.example.com/phish, carrying a fresh session cookie and the trust the user had in the legitimate brand. From is JWT-signed so it cannot be tampered mid-flow — but the initial link is attacker-controlled and any URL fits inside the JWT.

The same pattern lives in all four providers: oauth1.go:166, oauth2.go:242, apple.go:396, verify.go:130.

Fix

New token.AllowedHosts interface + token.AllowedHostsFunc adapter — mirrors the existing Audience/AudienceFunc pattern for consistency.
New Opts.AllowedRedirectHosts field threaded through provider.Params (and added directly to VerifyHandler since it doesn't embed Params).
Centralised provider.isAllowedRedirect helper — relative URLs and unparseable URLs are rejected; the host of Opts.URL is always permitted; additional hosts are opt-in via AllowedRedirectHosts.
All four redirect sites gate on the helper and fall back to the existing JSON user-info response on rejection (with a [WARN] log naming the disallowed host).

Default behaviour (nil allowlist) accepts only the same host as Opts.URL — safe for every existing single-host caller without any config change.

For multi-host deployments, callers register additional hosts:

opts := auth.Opts{
    URL: "https://app.example.com",
    AllowedRedirectHosts: token.AllowedHostsFunc(func() ([]string, error) {
        return []string{"admin.example.com", "billing.example.com"}, nil
    }),
}

Reproduction

TestOauth2LoginFromRejectsExternalHost in provider/oauth2_test.go exercises the full handler with from=https://evil.example.com/phish and asserts no external redirect is attempted (response is the JSON user info). The test docblock spells out the attack scenario for future readers. Same pattern repeated in oauth1_test.go, apple_test.go, verify_test.go. TestOauth2LoginFromAllowsAllowlistedHost covers the positive case (allowlisted host gets the 307).

TestIsAllowedRedirect in provider/redirect_test.go covers the validator with 14 cases: empty/relative/unparseable URLs, same-host with various scheme/port combinations, subdomain non-matching, allowlist hits and misses, getter errors.

Verification

$ cd v2 && go test -race ./...
ok  github.com/go-pkgz/auth/v2          1.07s
ok  github.com/go-pkgz/auth/v2/avatar
ok  github.com/go-pkgz/auth/v2/logger
ok  github.com/go-pkgz/auth/v2/middleware
ok  github.com/go-pkgz/auth/v2/provider
ok  github.com/go-pkgz/auth/v2/provider/sender
ok  github.com/go-pkgz/auth/v2/token

go vet clean. New code passes golangci-lint (existing pre-PR findings in telegram.go, providers.go, apple.go are unchanged and unrelated).

Files

v2/token/jwt.go — new AllowedHosts/AllowedHostsFunc
v2/provider/redirect.go (new) — isAllowedRedirect helper
v2/provider/redirect_test.go (new) — 14 unit cases
v2/provider/oauth1.go + oauth1_test.go — gate + reproduction test
v2/provider/oauth2.go + oauth2_test.go — Params.AllowedRedirectHosts field, gate + 2 tests
v2/provider/apple.go + apple_test.go — gate + reproduction test
v2/provider/verify.go + verify_test.go — URL/AllowedRedirectHosts fields on VerifyHandler, gate + 2 tests
v2/auth.go — wire Opts.AllowedRedirectHosts into all 6 provider.Params constructions and into VerifyHandler
README.md — new "Allowed redirect hosts" section + notes on the two from= API entries

Reported via private security audit of remark42 (which vendors this library); confirmed by code inspection here and against the live reference deployment.

coveralls · 2026-04-18T03:49:03Z

Coverage Report for CI Build 24744848855

Coverage increased (+0.4%) to 84.66%

Details

Coverage increased (+0.4%) from the base build.
Patch coverage: 3 uncovered changes across 1 file (116 of 119 lines covered, 97.48%).
No coverage regressions found.

Uncovered Changes

File	Changed	Covered	%
v2/token/jwt.go	4	1	25.0%

Coverage Regressions

No coverage regressions found.

Coverage Stats


Relevant Lines:	3305
Covered Lines:	2798
Line Coverage:	84.66%
Coverage Strength:	7.73 hits per line

💛 - Coveralls

Copilot

Pull request overview

This PR hardens the OAuth1/OAuth2/Apple and verify login flows against open-redirect attacks by validating the from redirect target before issuing the post-auth redirect.

Changes:

Introduces a redirect-host allowlist abstraction (token.AllowedHosts) and threads it through auth.Opts → provider.Params / VerifyHandler.
Adds centralized redirect validation (provider.isAllowedRedirect) and applies it to all affected providers before redirecting.
Adds unit/integration tests and updates README documentation describing the new behavior and configuration.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
v2/token/jwt.go	Adds `AllowedHosts` interface + `AllowedHostsFunc` adapter for allowlisted redirect hosts.
v2/provider/redirect.go	New shared helper to validate `from` redirect targets.
v2/provider/redirect_test.go	Unit tests covering redirect validation cases.
v2/provider/oauth1.go	Gates OAuth1 post-auth redirects via `isAllowedRedirect`.
v2/provider/oauth1_test.go	Adds regression test reproducing the open-redirect scenario.
v2/provider/oauth2.go	Adds `Params.AllowedRedirectHosts` and gates OAuth2 post-auth redirects.
v2/provider/oauth2_test.go	Adds regression + allowlist-positive tests for OAuth2 `from` redirect handling.
v2/provider/apple.go	Gates Apple post-auth redirects via `isAllowedRedirect`.
v2/provider/apple_test.go	Adds regression test reproducing the open-redirect scenario.
v2/provider/verify.go	Adds `URL`/`AllowedRedirectHosts` to `VerifyHandler` and gates redirects.
v2/provider/verify_test.go	Adds regression + allowlist-positive tests for verify flow.
v2/auth.go	Wires `Opts.AllowedRedirectHosts` into provider params and verify handler.
README.md	Documents new “Allowed redirect hosts” behavior and API notes for `from`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+func isAllowedRedirect(from, serviceURL string, allowed token.AllowedHosts) bool {
+	u, err := url.Parse(from)
+	if err != nil || u.Host == "" {
+		return false
+	}
+	if svc, sErr := url.Parse(serviceURL); sErr == nil && svc.Host != "" && svc.Host == u.Host {
+		return true


+			p.Logf("[WARN] rejected redirect to disallowed host: %s", oauthClaims.Handshake.From)
+			rest.RenderJSON(w, &u)


+			h.Logf("[WARN] rejected redirect to disallowed host: %s", oauthClaims.Handshake.From)
+			rest.RenderJSON(w, &u)


 	// redirect to back url if presented in login query params
 	if oauthClaims.Handshake != nil && oauthClaims.Handshake.From != "" {
+		if !isAllowedRedirect(oauthClaims.Handshake.From, ah.URL, ah.AllowedRedirectHosts) {
+			ah.Logf("[WARN] rejected redirect to disallowed host: %s", oauthClaims.Handshake.From)


+			e.Logf("[WARN] rejected redirect to disallowed host: %s", confClaims.Handshake.From)
+			rest.RenderJSON(w, claims.User)


Address Copilot review on PR #275: * isAllowedRedirect compared u.Host (which includes port). A from URL like https://app.example.com:443/x against an Opts.URL of https://app.example.com would be rejected even though they are the same origin. Switch to u.Hostname() so the explicit-default-port form matches, and add three test cases (https:443, http:80, non-default :8080). * The rejection log lines wrote the full from URL, leaking attacker-supplied paths/query strings into operator logs. Add redirectHostForLog helper (parses and returns just the hostname, sentinel on parse failure) and use it from all four sites (oauth1/oauth2/apple/verify). Covered by TestRedirectHostForLog.

Per maintainer review on PR #275: a dependency bump must not silently change behaviour for existing consumers. Flip the default so that a nil Opts.AllowedRedirectHosts means "no host check" (preserves pre-feature behaviour); the validator only runs when the consumer explicitly sets the field. When non-nil: * Opts.URL host is always implicit (single-host deployments enable the policy with `func() ([]string, error) { return nil, nil }`). * Hostname comparison ignores port (https://x and https://x:443 match). * Relative/unparseable URLs are rejected. Test table reorganised to cover the permissive default explicitly and to mark every "policy on" case with a non-nil allowlist. Integration tests in oauth1/oauth2/apple/verify get a paramOpts variadic on their setup helpers so the rejection tests can flip the policy on. README rewritten to make the opt-in nature clear.

paskal · 2026-04-18T04:26:12Z

Pushed two fixup commits addressing review:

8fd76b0 — Copilot inline items:

isAllowedRedirect now compares u.Hostname() instead of u.Host, so https://app.example.com:443 and https://app.example.com match (3 new test cases cover :443, :80, and :8080).
All four [WARN] rejected redirect to disallowed host: ... log lines now use a new redirectHostForLog helper that returns just the hostname; attacker-supplied paths/queries no longer leak into operator logs. Covered by TestRedirectHostForLog.

975bb1f — your call on the default:
You're right, a dep bump shouldn't silently restrict behaviour. Flipped the semantics: nil AllowedRedirectHosts is now permissive (legacy behaviour, no host check); the validator only runs when the consumer explicitly opts in by setting the field. Single-host deployments can enable the policy with token.AllowedHostsFunc(func() ([]string, error) { return nil, nil }) — the Opts.URL host is always implicit when policy is on. Test table reorganised, README rewritten to make the opt-in nature clear.

CI is running.

umputun

nice fix. a few things before merge:

new lint finding — v2/provider/oauth2_test.go:301:13 introduces a G101 gosec issue on the inline oauth2.Endpoint{AuthURL:..., TokenURL:...} string literals. Either //nolint:gosec // G101: test URL, not a credential or build the URLs via fmt.Sprintf like prepOauth2Test does.
case-insensitive hostname compare — v2/provider/redirect.go:44 uses h == fromHost (byte-exact). DNS hostnames are case-insensitive, and checkAuds in this same repo already uses strings.EqualFold (v2/token/jwt.go:384). App.Example.Com in from would be rejected even if the allowlist has app.example.com. Recommend strings.EqualFold for consistency. Worth a test case alongside it.
scheme allowlist — Copilot's second point wasn't addressed. In practice Hostname() == "" filters javascript:/data:, so the attack surface is narrow, but explicit u.Scheme == "http" || u.Scheme == "https" closes it for defense-in-depth. Low priority if you'd rather leave it.
typed-nil interface pitfall — var fn provider.AllowedHostsFunc; opts.AllowedRedirectHosts = fn produces a non-nil interface wrapping a nil func, so allowed == nil is false and allowed.Get() panics. Rare-consumer-error, but a one-line guard in isAllowedRedirect (or just a godoc note on AllowedHostsFunc) would be safer.

question: is there a concrete dynamic-allowlist use case driving the AllowedHosts interface, or is this purely parity with Audience? []string would be simpler if every caller passes a static list — but the parity argument is fine if you want it.

stale-base note: the README CrossOriginProtection section and go.mod downgrades in the diff are stale-base artifacts from #273/#274 landing on master — they'll disappear on rebase, not real changes.

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. Add token.AllowedHosts (interface) and AllowedHostsFunc (adapter), mirroring the existing token.Audience pattern. Expose Opts.AllowedRedirectHosts and thread it through provider.Params and VerifyHandler. The provider's own URL host is always permitted; additional hosts are opt-in via the allowlist. Default (nil allowlist) accepts only the same host as Opts.URL — safe for all existing single-host deployments without any caller code changes. Centralise the policy in provider.isAllowedRedirect(); the four redirect sites all gate on it and fall back to the existing JSON user-info response on rejection (with a [WARN] log).

Add an attack-scenario doc block to TestOauth2LoginFromRejectsExternalHost spelling out the pre-fix flow (attacker-crafted login URL → legitimate OAuth consent → 307 to attacker site). Same pattern applies to oauth1, apple and verify; the comment names them explicitly so a future reader sees what the regression test is guarding against.

Address Copilot review on PR #275: * isAllowedRedirect compared u.Host (which includes port). A from URL like https://app.example.com:443/x against an Opts.URL of https://app.example.com would be rejected even though they are the same origin. Switch to u.Hostname() so the explicit-default-port form matches, and add three test cases (https:443, http:80, non-default :8080). * The rejection log lines wrote the full from URL, leaking attacker-supplied paths/query strings into operator logs. Add redirectHostForLog helper (parses and returns just the hostname, sentinel on parse failure) and use it from all four sites (oauth1/oauth2/apple/verify). Covered by TestRedirectHostForLog.

Per maintainer review on PR #275: a dependency bump must not silently change behaviour for existing consumers. Flip the default so that a nil Opts.AllowedRedirectHosts means "no host check" (preserves pre-feature behaviour); the validator only runs when the consumer explicitly sets the field. When non-nil: * Opts.URL host is always implicit (single-host deployments enable the policy with `func() ([]string, error) { return nil, nil }`). * Hostname comparison ignores port (https://x and https://x:443 match). * Relative/unparseable URLs are rejected. Test table reorganised to cover the permissive default explicitly and to mark every "policy on" case with a non-nil allowlist. Integration tests in oauth1/oauth2/apple/verify get a paramOpts variadic on their setup helpers so the rejection tests can flip the policy on. README rewritten to make the opt-in nature clear.

Address Copilot and maintainer review comments on PR #275: * Hostname compare is case-insensitive (strings.EqualFold), mirroring checkAuds in token/jwt.go. DNS hostnames are case-insensitive, so App.Example.Com in "from" now matches app.example.com in the allowlist or service URL. Covered by three new TestIsAllowedRedirect cases. * Scheme check: only http/https are accepted when the policy is on. Hostname()=="" already filters javascript:/data:/etc. in practice, but an explicit check is defence-in-depth. New test cases cover javascript, ftp and data scheme rejection. * Typed-nil guard: a nil AllowedHostsFunc assigned to the interface field produces a non-nil interface wrapping a nil func, which would panic in Get(). Detect the common adapter case and treat it as "no allowlist configured". Documented on AllowedHostsFunc godoc. New test case exercises the typed-nil path. * TestOauth2LoginFromAllowsAllowlistedHost now reuses prepOauth2Test via its paramOpts hook instead of building a parallel mock server, removing the G101 gosec hit on the inline oauth2.Endpoint literals (flagged by umputun) and cutting ~60 lines of duplication.

Copilot

Pull request overview

Adds redirect-target validation for the from parameter used across OAuth1/OAuth2/Apple and Verify login flows to mitigate open-redirect/phishing risks by introducing an allowlist-based policy.

Changes:

Introduces token.AllowedHosts / AllowedHostsFunc and threads AllowedRedirectHosts through auth.Opts, provider.Params, and VerifyHandler.
Adds centralized redirect validation helpers (isAllowedRedirect, redirectHostForLog) and gates redirects in all affected providers.
Adds unit/integration tests covering rejection/allowlisting behavior and documents the feature in README.md.

Reviewed changes

Copilot reviewed 13 out of 13 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
v2/token/jwt.go	Adds `AllowedHosts` + `AllowedHostsFunc` config-getter pattern.
v2/auth.go	Wires `Opts.AllowedRedirectHosts` into provider params and verify handler.
v2/provider/redirect.go	Implements shared redirect-host validation + safe logging helper.
v2/provider/redirect_test.go	Adds table-driven coverage for redirect validation behavior.
v2/provider/oauth1.go	Gates post-auth redirect via `isAllowedRedirect`.
v2/provider/oauth1_test.go	Adds reproduction test for rejecting external `from` redirects.
v2/provider/oauth2.go	Adds `Params.AllowedRedirectHosts` and gates redirect via helper.
v2/provider/oauth2_test.go	Adds negative/positive redirect-host tests; updates helper setup.
v2/provider/apple.go	Gates post-auth redirect via `isAllowedRedirect`.
v2/provider/apple_test.go	Adds reproduction test for rejecting external `from` redirects.
v2/provider/verify.go	Adds `URL`/`AllowedRedirectHosts` to handler and gates redirect.
v2/provider/verify_test.go	Adds allowlist rejection/acceptance tests for verify flow.
README.md	Documents “Allowed redirect hosts” and updates endpoint docs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+	// parameter of OAuth/verify login flows. The host of URL is always
+	// allowed implicitly. Nil means same-host-only.



+	// AllowedRedirectHosts lists hostnames accepted in the "from" query
+	// parameter of OAuth/verify login flows. The host of URL is always
+	// allowed implicitly. Nil disables additional hosts (same-host only).


+	// a "from" redirect target. Optional but recommended.
+	URL string
+	// AllowedRedirectHosts lists additional hostnames permitted as "from"
+	// redirect targets. Nil means same-host-only.


 For the example above authentication handlers wired as `/auth` and provides:

- `/auth/<provider>/login?site=<site_id>&from=<redirect_url>` - site_id used as `aud` claim for the token and can be processed by `SecretReader` to load/retrieve/define different secrets. redirect_url is the url to redirect after successful login.
+- `/auth/<provider>/login?site=<site_id>&from=<redirect_url>` - site_id used as `aud` claim for the token and can be processed by `SecretReader` to load/retrieve/define different secrets. redirect_url is the url to redirect after successful login. Only redirect targets whose host matches `Opts.URL` or appears in `Opts.AllowedRedirectHosts` are honoured; others fall back to the user-info JSON response. See "Allowed redirect hosts" below.


 The API for this provider:

- - `GET /auth/<name>/login?user=<user>&address=<address>&aud=<site_id>&from=<url>` - send confirmation request to user
+ - `GET /auth/<name>/login?user=<user>&address=<address>&aud=<site_id>&from=<url>` - send confirmation request to user. The `from` URL is subject to the same allowlist check described in "Allowed redirect hosts".


+// path with the default (nil) allowlist, where service URL is "url" — so any
+// real external host is refused.


umputun

all four items from my last pass landed cleanly — strings.EqualFold, explicit http/https scheme check, typed-nil AllowedHostsFunc guard, and the prepOauth2Test/paramOpts refactor that cuts ~60 lines and drops the G101. nice.

on Copilot's new six comments: they're all one issue — when you flipped semantics in 975bb1f to opt-in, redirect.go's docstring got updated but the downstream field godoc and README endpoint docs still describe the old "nil = same-host-only" behaviour. concretely:

v2/auth.go:68 — Opts.AllowedRedirectHosts says "Nil means same-host-only"
v2/provider/oauth2.go:47 — Params.AllowedRedirectHosts says "Nil disables additional hosts (same-host only)"
v2/provider/verify.go:36 — VerifyHandler.AllowedRedirectHosts says "Nil means same-host-only"
README.md:185 — reads as if validation is always on ("Only redirect targets whose host matches Opts.URL or appears in Opts.AllowedRedirectHosts are honoured")
README.md:294 — verify endpoint claims from is "subject to the same allowlist check" with no opt-in caveat
v2/provider/oauth2_test.go:268 — test docblock says "with the default (nil) allowlist" but the test wires non-nil via enablePolicy

all pure doc/comment fixes — one pass aligning the field godoc and README to the opt-in semantics that redirect.go:13-16 already spells out correctly. then good to merge.

Addresses the six doc/comment drift items Copilot flagged and umputun confirmed in his 2026-04-21 review: when 975bb1f flipped the default to "nil = no validation (permissive legacy)", the validator docstring in redirect.go was updated but the downstream field godoc, README endpoint bullets and one test docblock still read as if nil means "same-host-only". * v2/auth.go:66 — Opts.AllowedRedirectHosts * v2/provider/oauth2.go:45 — Params.AllowedRedirectHosts * v2/provider/verify.go:35 — VerifyHandler.AllowedRedirectHosts * README.md:185 — OAuth endpoint bullet * README.md:294 — verify endpoint bullet * v2/provider/oauth2_test.go:268 — TestOauth2LoginFromRejectsExternalHost docblock (test wires non-nil via enablePolicy; calling it "default (nil) allowlist" was misleading) No code changes — redirect.go's policy as implemented stays authoritative.

umputun

LGTM

…2049) * fix(auth): close OAuth open-redirect by wiring AllowedRedirectHosts Bump go-pkgz/auth/v2 to master (v2.1.2-0.20260421203319-686683f19cf7) which carries the `from` redirect validator from go-pkgz/auth#275. The library default with a nil AllowedRedirectHosts is permissive (preserves legacy behavior for existing consumers on a dep bump), so just bumping the dep leaves remark42 vulnerable — a crafted /auth/<provider>/login?from=https://evil.example.com/... still issues the 307 to the attacker host after the user completes legitimate OAuth. Verified end-to-end against a local dev-auth instance before and after this commit. Wire Opts.AllowedRedirectHosts in getAuthenticator to the operator's existing --allowed-hosts config, stripping the CSP "self" sentinel which is not a real hostname. RemarkURL's own host is always implicit per the library contract, so a default single-site deployment gains the protection with no config change. Multi-host embeds work as soon as their embedding hosts are added to AllowedHosts (they already need to be there for CSP frame-ancestors). Refreshed vendor tree to match the new module version. * chore(lint): suppress G703 false positives on image Save CI's newer gosec flags os.MkdirAll/os.WriteFile in FileSystem.Save with G703 because id flows in from the caller. id is validated at the HTTP layer (safePictureSegment in rest_public.go) and dst is derived via f.location — not a real traversal. Targeted //nolint with reason. * fix(auth): normalise AllowedRedirectHosts entries + add unit test Address Copilot review on PR #2049. The previous closure passed raw s.AllowedHosts entries straight to the auth library, but --allowed-hosts holds CSP frame-ancestors source expressions: scheme-prefixed values (https://blog.example.com), entries with ports, and wildcards (*.cdn.example.com) are all valid there but the auth library compares against u.Hostname() and would silently drop them — breaking legitimate redirects on multi-host deployments. Extract getAllowedRedirectHosts that: * trims whitespace, drops empty / 'self' / "self" / wildcard entries * prepends https:// if scheme missing then url.Parse to extract Hostname * logs a warning on parse failure rather than poisoning the allowlist Wire the closure in getAuthenticator to call the helper. Test_getAllowedRedirectHosts covers all the edge cases Copilot flagged (scheme stripping, port handling, self spellings, wildcards, empty, mixed real-world). * fix(auth): preserve explicit port in AllowedRedirectHosts + clarify fs_store nolint Address Copilot follow-up on PR #2049: * getAllowedRedirectHosts stripped explicit ports via u.Hostname(), which broadened the allowlist. The auth validator checks both Hostname() and Host, so an entry like admin.example.com:8443 can and should be kept host:port — allowing only that port, not any. Emit u.Host when u.Port() != "", u.Hostname() otherwise. Updated tests. * fs_store Save nolint rationale said "id validated at HTTP layer", but Save is reached via image.Service.Save and SaveWithID (cache), neither of which is HTTP validation. id is actually a server-generated hash in both paths. Updated the comment.

umputun requested a review from Copilot April 18, 2026 03:48

Copilot started reviewing on behalf of umputun April 18, 2026 03:48 View session

Copilot AI reviewed Apr 18, 2026

View reviewed changes

paskal marked this pull request as ready for review April 20, 2026 22:23

paskal requested a review from umputun as a code owner April 20, 2026 22:23

umputun reviewed Apr 20, 2026

View reviewed changes

paskal added 7 commits April 21, 2026 18:33

docs: document AllowedRedirectHosts and from-redirect validation

986d2a3

chore(lint): use US 'behavior' to satisfy misspell

3159f5e

paskal force-pushed the fix-oauth-from-open-redirect branch from d8c4fc1 to dddfe76 Compare April 21, 2026 18:22

paskal requested review from Copilot and umputun April 21, 2026 18:23

Copilot started reviewing on behalf of paskal April 21, 2026 18:23 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

umputun reviewed Apr 21, 2026

View reviewed changes

paskal and others added 2 commits April 21, 2026 20:38

chore(lint): use US 'honored' to satisfy misspell

1bd56bd

umputun approved these changes Apr 21, 2026

View reviewed changes

umputun merged commit 686683f into master Apr 21, 2026
8 checks passed

umputun deleted the fix-oauth-from-open-redirect branch April 21, 2026 20:33

paskal mentioned this pull request Apr 21, 2026

fix(auth): close OAuth open-redirect by wiring AllowedRedirectHosts umputun/remark42#2049

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix: validate "from" redirect target in OAuth/verify flows#275

fix: validate "from" redirect target in OAuth/verify flows#275
umputun merged 9 commits intomasterfrom
fix-oauth-from-open-redirect

paskal commented Apr 18, 2026

Uh oh!

coveralls commented Apr 18, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

paskal commented Apr 18, 2026

Uh oh!

umputun left a comment

Uh oh!

Copilot AI left a comment

Uh oh!

umputun left a comment

Uh oh!

umputun left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		p.Logf("[WARN] rejected redirect to disallowed host: %s", oauthClaims.Handshake.From)
		rest.RenderJSON(w, &u)

		h.Logf("[WARN] rejected redirect to disallowed host: %s", oauthClaims.Handshake.From)
		rest.RenderJSON(w, &u)

		e.Logf("[WARN] rejected redirect to disallowed host: %s", confClaims.Handshake.From)
		rest.RenderJSON(w, claims.User)

		// parameter of OAuth/verify login flows. The host of URL is always
		// allowed implicitly. Nil means same-host-only.

		// path with the default (nil) allowlist, where service URL is "url" — so any
		// real external host is refused.

Uh oh!

Conversation

paskal commented Apr 18, 2026

Attack

Fix

Reproduction

Verification

Files

Uh oh!

coveralls commented Apr 18, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report for CI Build 24744848855

Coverage increased (+0.4%) to 84.66%

Details

Uncovered Changes

Coverage Regressions

Coverage Stats

💛 - Coveralls

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

paskal commented Apr 18, 2026

Uh oh!

umputun left a comment

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

umputun left a comment

Choose a reason for hiding this comment

Uh oh!

umputun left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

coveralls commented Apr 18, 2026 •

edited

Loading