Repository navigation
Conversation
The login callback built its redirects from request.url. The image's standalone server sees its listening address as the origin of requests (http://0.0.0.0:3000), so after a login it sent the browser to http://0.0.0.0:3000/dashboard, where the session cookies (set for the real origin) don't apply. The redirects now go to APP_ORIGIN, and to the origin of the request only when it isn't set. Found by running the end-to-end tests against the image. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
An APP_ORIGIN without a scheme made the callback throw (a 500, or a misleading log line in the token error path). It now logs the problem and redirects to the origin of the request, as without the setting. The tests cover every redirect of the callback with a request at the listening address. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Reviewer's GuideAPIBuild every callback redirect from
DocumentationExplain that
TestsExercise redirects for each callback outcome and verify fallback behavior when
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #84 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 12 12
Lines 273 279 +6
Branches 77 78 +1
=========================================
+ Hits 273 279 +6 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@AnonymousXC could you review this one? It changes app code (the login callback), so it's yours to decide and merge. The new end-to-end tests against the Docker image (#79, PR #82) found that after a login the browser was sent to the server's listening address ( |
Linked issuesChecked at commit 7899f7e #83 fix: login redirects to the server's listening address · 3 of 5 met · 1 to check by hand
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 3 issues
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="src/app/auth/api/callback/route.ts" line_range="89" />
<code_context>
const responseRedirect = NextResponse.redirect(
- new URL("/dashboard", request.url),
+ appUrl("/dashboard", request),
);
</code_context>
<issue_to_address>
**Cross-origin redirect drops session**
When the browser-facing origin in `OIDC_REDIRECT_URI` differs from `APP_ORIGIN`, `appUrl` sends the browser to `APP_ORIGIN`, but the callback response sets host-only session cookies for the origin at which the browser reached the callback. When those origins differ, the browser does not send the cookies to the dashboard, so the user is not logged in there.
Validate that the public callback and application origins match, or configure cookie scope so the session is available on both origins.
</issue_to_address>
### Comment 2
<location path="src/app/auth/api/callback/route.ts" line_range="19" />
<code_context>
+ const appOrigin = process.env.APP_ORIGIN;
+ if (appOrigin) {
+ try {
+ return new URL(path, new URL(appOrigin).origin);
+ } catch {
+ console.error(
</code_context>
<issue_to_address>
**Dashboard redirects leave the web app**
When APP_ORIGIN is syntactically valid but uses a non-HTTP(S) scheme, `appUrl` accepts the URL and returns a redirect using that scheme, so the browser cannot reach the application dashboard and login fails.
Require `APP_ORIGIN` to use HTTP or HTTPS before returning the redirect, and otherwise use the fallback.
</issue_to_address>
### Comment 3
<location path="src/app/auth/api/callback/route.ts" line_range="26" />
<code_context>
+ );
+ }
+ }
+ return new URL(path, request.url);
+}
+
</code_context>
<issue_to_address>
**Unset origin fallback goes unreported**
When APP_ORIGIN is unset or empty, `appUrl` returns a URL based on `request.url` without logging that it used the fallback, so operators cannot diagnose the configuration behind failed logins.
Log the fallback in the unset-or-empty branch, as the invalid-URL branch already does.
Also at `src/app/auth/api/callback/route.ts:17`.
</issue_to_address>Sourcery assessment
Approval pending. 2 findings to address first.
Blocking findings: src/app/auth/api/callback/route.ts:89, src/app/auth/api/callback/route.ts:19
|
|
||
| const responseRedirect = NextResponse.redirect( | ||
| new URL("/dashboard", request.url), | ||
| appUrl("/dashboard", request), |
There was a problem hiding this comment.
🟡 Medium · Cross-origin redirect drops session
When the browser-facing origin in OIDC_REDIRECT_URI differs from APP_ORIGIN, appUrl sends the browser to APP_ORIGIN, but the callback response sets host-only session cookies for the origin at which the browser reached the callback. When those origins differ, the browser does not send the cookies to the dashboard, so the user is not logged in there.
Validate that the public callback and application origins match, or configure cookie scope so the session is available on both origins.
Prompt for AI agents
In `src/app/auth/api/callback/route.ts` at line 89:
**Cross-origin redirect drops session**
When the browser-facing origin in `OIDC_REDIRECT_URI` differs from `APP_ORIGIN`, `appUrl` sends the browser to `APP_ORIGIN`, but the callback response sets host-only session cookies for the origin at which the browser reached the callback. When those origins differ, the browser does not send the cookies to the dashboard, so the user is not logged in there.
Validate that the public callback and application origins match, or configure cookie scope so the session is available on both origins.| const appOrigin = process.env.APP_ORIGIN; | ||
| if (appOrigin) { | ||
| try { | ||
| return new URL(path, new URL(appOrigin).origin); |
There was a problem hiding this comment.
🟡 Medium · Dashboard redirects leave the web app
When APP_ORIGIN is syntactically valid but uses a non-HTTP(S) scheme, appUrl accepts the URL and returns a redirect using that scheme, so the browser cannot reach the application dashboard and login fails.
Require APP_ORIGIN to use HTTP or HTTPS before returning the redirect, and otherwise use the fallback.
Prompt for AI agents
In `src/app/auth/api/callback/route.ts` at line 19:
**Dashboard redirects leave the web app**
When APP_ORIGIN is syntactically valid but uses a non-HTTP(S) scheme, `appUrl` accepts the URL and returns a redirect using that scheme, so the browser cannot reach the application dashboard and login fails.
Require `APP_ORIGIN` to use HTTP or HTTPS before returning the redirect, and otherwise use the fallback.| ); | ||
| } | ||
| } | ||
| return new URL(path, request.url); |
There was a problem hiding this comment.
⚪ Low · Unset origin fallback goes unreported
When APP_ORIGIN is unset or empty, appUrl returns a URL based on request.url without logging that it used the fallback, so operators cannot diagnose the configuration behind failed logins.
Log the fallback in the unset-or-empty branch, as the invalid-URL branch already does.
Also at src/app/auth/api/callback/route.ts:17.
Prompt for AI agents
In `src/app/auth/api/callback/route.ts` at line 26:
**Unset origin fallback goes unreported**
When APP_ORIGIN is unset or empty, `appUrl` returns a URL based on `request.url` without logging that it used the fallback, so operators cannot diagnose the configuration behind failed logins.
Log the fallback in the unset-or-empty branch, as the invalid-URL branch already does.
Also at `src/app/auth/api/callback/route.ts:17`.From #83:
Fall back to the request origin when APP_ORIGIN is unset or isn't a URL, and log the fallback.
Closes #83
Summary
The login callback built its redirects from
request.url. In the image, the standalone server sees its listening address as that origin, so after a login the browser went tohttp://0.0.0.0:3000/dashboard, without the session.APP_ORIGIN. It falls back to the origin of the request, with a log line, ifAPP_ORIGINis unset or isn't a URL.APP_ORIGINis used for.Found by the end-to-end tests against the image (#79, PR #82, which carries these two commits until this PR is merged).
Verification
npm run test:coverage→ 124 passed, 100% coverage.bun run lintandbunx tsc --noEmit→ clean.dev→ 5 of 11 Chromium tests failed at the redirect after the login.End-to-end tests (image).adversarial-reviewerandquality-reviewer, two rounds; all findings are fixed.Declined review findings
None.
Open questions
None.
🤖 Generated with Claude Code
Summary by Sourcery
Ensure OIDC callback redirects use the configured application origin while retaining a safe request-origin fallback.
Bug Fixes:
Documentation:
Tests: