Skip to content

feat: enhance redirect logic in auth controller to include onboarding parameter#2476

Merged
JivusAyrus merged 5 commits intomainfrom
suvij/eng-8829-cosmo-sign-up-migratetrue-flag-always-set
Jan 29, 2026
Merged

feat: enhance redirect logic in auth controller to include onboarding parameter#2476
JivusAyrus merged 5 commits intomainfrom
suvij/eng-8829-cosmo-sign-up-migratetrue-flag-always-set

Conversation

@JivusAyrus
Copy link
Copy Markdown
Member

@JivusAyrus JivusAyrus commented Jan 29, 2026

Summary by CodeRabbit

  • Bug Fixes
    • Improved post-authentication redirect handling to ensure redirects only proceed to approved same-origin URLs and to avoid parse errors.
  • New Features
    • Consolidated redirect logic and automatically appends onboarding=true for users with no organizations to trigger the onboarding flow.

✏️ Tip: You can customize this high-level summary in your review settings.

Checklist

  • I have discussed my proposed changes in an issue and have received approval to proceed.
  • I have followed the coding standards of the project.
  • Tests or benchmarks have been added or updated.
  • Documentation has been updated on https://github.com/wundergraph/cosmo-docs.
  • I have read the Contributors Guide.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Jan 29, 2026

Walkthrough

Consolidates post-authentication redirect logic: computes a single targetUrl starting from webBaseUrl, conditionally replaces it with a validated same-origin redirectURL, appends onboarding=true when orgs === 0, and always redirects to the computed targetUrl.

Changes

Cohort / File(s) Summary
Authentication redirect logic
controlplane/src/core/controllers/auth.ts
Reworked callback redirect flow to compute a single targetUrl from webBaseUrl, parse and accept redirectURL only if same-origin, handle URL parse errors by falling back to webBaseUrl, and append onboarding=true when orgs === 0. Removed prior branching that directly redirected to raw redirectURL or used migrate=true.
Package configuration
package.json
Small metadata/dependency edits (+17/-7 lines).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: enhancing redirect logic to include an onboarding parameter, which aligns with the file modifications in the auth controller.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Fix all issues with AI agents
In `@controlplane/src/core/controllers/auth.ts`:
- Around line 403-407: The current startsWith check is vulnerable; instead parse
redirectURL and opts.webBaseUrl with the URL constructor and verify the
redirect's origin (protocol + hostname + port) exactly matches the web base
origin before assigning targetUrl. Update the logic around
targetUrl/redirectURL/opts.webBaseUrl to: try constructing new URL(redirectURL)
and new URL(opts.webBaseUrl), compare url.origin (or url.protocol, url.hostname
and url.port) for equality, and only set targetUrl = redirectURL when the
origins match; on parse error or mismatch keep targetUrl = opts.webBaseUrl.

@codecov
Copy link
Copy Markdown

codecov bot commented Jan 29, 2026

Codecov Report

❌ Patch coverage is 0% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.20%. Comparing base (177c11d) to head (c4496cc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
controlplane/src/core/controllers/auth.ts 0.00% 13 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            main    #2476       +/-   ##
==========================================
+ Coverage   1.50%   63.20%   +61.70%     
==========================================
  Files        292      296        +4     
  Lines      46748    41480     -5268     
  Branches     431     4312     +3881     
==========================================
+ Hits         703    26219    +25516     
+ Misses     45762    15239    -30523     
+ Partials     283       22      -261     
Files with missing lines Coverage Δ
controlplane/src/core/controllers/auth.ts 9.61% <0.00%> (ø)

... and 587 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JivusAyrus JivusAyrus merged commit fd9302f into main Jan 29, 2026
11 checks passed
@JivusAyrus JivusAyrus deleted the suvij/eng-8829-cosmo-sign-up-migratetrue-flag-always-set branch January 29, 2026 17:51
maxbol pushed a commit to maxbol/cosmo that referenced this pull request Feb 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants