Repository navigation
fix(sso): return users to the page they asked for after logging in - #213
Conversation
Port the reference platform's verified fix (devantler-tech/platform#3169): register oauth2-proxy's callback on every fronted host with Dex, drop the fixed redirect_url, and drop the per-route X-Auth-Request-Redirect headers. The Coroot profile retires the callbacks of the UIs it removes. The guard test renders both observability profiles on both providers. Fixes #200 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@coderabbitai review |
✅ Action performedReview finished.
|
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Advanced Run ID: 📒 Files selected for processing (13)
💤 Files with no reviewable changes (6)
Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review. 📜 Recent review details🔇 Additional comments (7)
📝 WalkthroughWalkthroughDex callback registrations and oauth2-proxy settings now support callbacks on protected hosts. HTTPRoutes no longer set a fixed Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: ⚪ Minimal · up to This change registers per-host SSO callbacks so users return to the page they opened after signing in. No concrete merge-blocking issue was found. CI should run the new redirect validation script before merge. Security Architecture ReviewSecurity architecture risk: 🔵 Low · up to The callback expansion is limited to configured protected sites, which remain behind the same authentication proxy and group restriction. No authentication bypass or credential disclosure was established. Request-header handling, login-state protection, and behavior during partially completed upgrades remain unverified. Retained concerns Security review detailsSecurity Blast Radius
Trust Boundaries and Controls
Resilience and Maintainability Implications
Hardening Proposals
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 2 files. (5 skipped: 5 unsupported.)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Readiness at Tried as a user: nothing can run here without a cluster, so I inspected the rendered overlays. With the default observability setup, both providers put six hosts and oauth2-proxy's own host behind sign-in. Dex now accepts each host's own login callback, no route forces a fixed return URL, and oauth2-proxy no longer pins one callback host. With the Coroot setup, Dex keeps exactly the callbacks of the hosts that setup still serves (home, Hubble and Coroot). The OpenCost, Prometheus and Alertmanager callbacks are retired along with their UIs. The new guard passes on all four renders, and it fails on the unchanged template. |
Why
On a platform created from this template, someone who opens a link into a protected dashboard while signed out (an alert link, a bookmarked report) lands on that service's front page after signing in, not on the page they asked for. The reference platform fixed and verified the same problem live last week.
What
Signing in now returns people to the exact page they opened, on every protected site, in both the default and the Coroot observability setup. A new check in the template's validation fails if a protected site is added without the matching sign-in setting, or if a leftover one remains.
Fixes #200