fix(core): fix trusted origin matching logic#91
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughAdds origin-trust validation across sign-in and callback flows: new Changes
Sequence DiagramsequenceDiagram
participant Request as Request Handler
participant AuthFlow as Auth Flow
participant OriginResolver as Origin Resolver (getOriginURL/getTrustedOrigins)
participant Validator as Validator (isTrustedOrigin/isSameOrigin)
participant Logger as Logger
Request->>AuthFlow: incoming request + redirectTo
AuthFlow->>OriginResolver: resolve origin (request + context)
OriginResolver->>OriginResolver: consider proxy headers / derive origin
OriginResolver->>Validator: check origin against trustedOrigins/patterns
alt origin trusted
Validator-->>AuthFlow: valid origin
AuthFlow->>Logger: log success / store redirect cookie
AuthFlow-->>Request: proceed with redirect
else origin untrusted
Validator-->>Logger: log UNTRUSTED_ORIGIN / OPEN_REDIRECT_ATTACK
Logger-->>AuthFlow: logged
AuthFlow-->>Request: reject or fallback to "/"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/core/src/actions/callback/callback.ts`:
- Around line 90-106: In the callback handling code in
packages/core/src/actions/callback/callback.ts (inside the redirect validation
block), there's a ReferenceError: the conditional uses undefined variable
`origin.length`; change that to check `origins.length` (the array returned by
getTrustedOrigins) so the isValid calculation uses origins correctly in the
isTrustedOrigin call and subsequent logging/throw path; update the conditional
expression where `origin.length > 0 ? isTrustedOrigin(cookieRedirectTo, origins)
: isSameOrigin(cookieRedirectTo, requestOrigin)` to use `origins.length`
instead.
In `@packages/core/src/actions/signIn/authorization.ts`:
- Around line 56-75: The host header lookup in getOriginURL is inconsistent with
the protocol extraction when context?.trustedProxyHeaders is true: change the
host resolution order to match the protocol's RFC-7239-first approach by
preferring headers.get("Forwarded")?.match(/host=([^;]+)/i)?.[1] then
headers.get("X-Forwarded-Host") then fall back to headers.get("Host"); update
the host assignment used to build origin and keep the rest of the logic
(trustedOrigins via getTrustedOrigins and origin validation via isTrustedOrigin)
unchanged so origin construction and logging still occur when untrusted.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/core/src/actions/callback/callback.ts (1)
88-108: Redirect validation logic is sound; consider usingconst.The fix correctly uses
origins.lengthand implements appropriate validation: trusted-origin check when configured, same-origin fallback otherwise. The enhanced logging withhas_trusted_originsandrequest_originprovides good diagnostic context.Minor style nit:
isValidis never reassigned, so it could be declared withconstinstead oflet.♻️ Optional: use const for isValid
if (!isRelativeURL(cookieRedirectTo)) { - let isValid = + const isValid = origins.length > 0 ? isTrustedOrigin(cookieRedirectTo, origins) : isSameOrigin(cookieRedirectTo, requestOrigin)
Description
This pull request fixes and clarifies the origin matching logic used during
signInandcallbackflows when requests originate from trusted or untrusted environments. The update improves validation behavior when using thetrustedProxyHeadersandtrustedOriginsconfiguration options.The origin verification process now behaves consistently across the following scenarios:
Origin Matching Behavior
When
trustedProxyHeadersis NOT configuredThe request origin is derived directly from
request.url. Redirects are restricted to the origin where the server is mounted.When
trustedOriginsis NOT configuredRedirects outside the server’s origin are rejected. Only relative paths or absolute URLs matching the request origin are allowed.
When BOTH
trustedProxyHeadersandtrustedOriginsare configuredThis mode supports deployments behind reverse proxies, gateways, or load balancers.
The incoming request URL is reconstructed using trusted proxy headers and then validated against the
trustedOriginsallowlist.If the reconstructed origin:
/) is returnedNote
A reconstructed URL must match a value defined in
trustedOrigins. Otherwise, the request is rejected or redirected to the default path.Additional Changes
Origin,Referer, andredirectTovalidation paths.Resources