Skip to content

fix(auth): use first X-Forwarded-Proto/Host token in publicOrigin#180

Open
jony376 wants to merge 2 commits into
MkDev11:mainfrom
jony376:fix/public-origin-forwarded-headers
Open

fix(auth): use first X-Forwarded-Proto/Host token in publicOrigin#180
jony376 wants to merge 2 commits into
MkDev11:mainfrom
jony376:fix/public-origin-forwarded-headers

Conversation

@jony376
Copy link
Copy Markdown
Contributor

@jony376 jony376 commented May 29, 2026

Summary

Use the first comma-separated X-Forwarded-Proto and host token in publicOrigin so OAuth redirect URIs stay valid behind reverse proxies.

Related Issues

Closes #178

Type of Change

  • Bug fix
  • New feature
  • Refactor
  • Documentation
  • Other (describe below)

Testing

  • Tests added/updated
  • Manually tested

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Changes are documented (if applicable)

Summary by CodeRabbit

  • Bug Fixes
    • Improved origin detection for deployments behind reverse proxies and load balancers. The application now correctly extracts the primary protocol and host values from forwarded headers containing multiple comma-separated values, ensuring proper URL generation in proxied environments.

Review Change Stack

Co-authored-by: Cursor <cursoragent@cursor.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 29, 2026

📝 Walkthrough

Walkthrough

The PR adds a helper function to extract the first value from comma-separated forwarded headers and applies it when deriving the protocol and host in the publicOrigin function. This fixes OAuth callback URL construction behind proxies that emit comma-separated header chains.

Changes

Forwarded header parsing fix

Layer / File(s) Summary
Extract first forwarded value and apply to proto/host derivation
src/lib/origin.ts
firstForwardedHeader helper splits comma-separated header values, trims, and returns the first non-empty segment or a fallback. publicOrigin now uses this helper when computing proto from x-forwarded-proto (with fallback to protocol without colon) and host from x-forwarded-host (with fallback to host header or nextUrl.host), ensuring OAuth redirects work correctly when headers contain multiple proxy values.

🎯 2 (Simple) | ⏱️ ~8 minutes

🐰 A hop through the header maze,
Where commas once caused haze,
Now the first value leads the way,
OAuth flows shall save the day! ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: using the first token from X-Forwarded-Proto/Host headers in publicOrigin, which directly matches the changeset.
Linked Issues check ✅ Passed The pull request implements the exact solution required by issue #178: extracting the first comma-separated token from X-Forwarded-Proto and host headers to construct valid OAuth redirect URIs.
Out of Scope Changes check ✅ Passed All changes are in-scope: a helper function to extract the first forwarded header token and updates to publicOrigin to use it, directly addressing the issue without extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

src/lib/origin.ts

ESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox.

Warning

Billing warning: we have not been able to collect payment for this subscription for more than 72 hours. Please update the payment method or pay any pending invoices in Billing to avoid service interruption.


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.

❤️ Share

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

Copy link
Copy Markdown

@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.

🧹 Nitpick comments (1)
src/lib/origin.ts (1)

6-6: 💤 Low value

Optional: Remove redundant optional chaining.

The optional chaining ?. after split(',')[0] is technically unnecessary because split(',') always returns an array with at least one element, so [0] will never be undefined. However, the defensive style is harmless and may improve readability for future maintainers.

♻️ Simplified version (optional)
-  const first = value.split(',')[0]?.trim();
+  const first = value.split(',')[0].trim();
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/lib/origin.ts` at line 6, The expression using redundant optional
chaining on the split result should be simplified: in the assignment to the
variable first (const first = value.split(',')[0]?.trim();), remove the
unnecessary ?. after [0] since split(',') always returns at least one element,
so change it to call trim() directly on the [0] result to avoid the needless
optional check while preserving behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/lib/origin.ts`:
- Line 6: The expression using redundant optional chaining on the split result
should be simplified: in the assignment to the variable first (const first =
value.split(',')[0]?.trim();), remove the unnecessary ?. after [0] since
split(',') always returns at least one element, so change it to call trim()
directly on the [0] result to avoid the needless optional check while preserving
behavior.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 4d50d4d6-2dcd-4089-9919-56b6014b176a

📥 Commits

Reviewing files that changed from the base of the PR and between 22ab15d and b1989d9.

📒 Files selected for processing (1)
  • src/lib/origin.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: publicOrigin breaks OAuth behind comma-separated X-Forwarded-Proto

2 participants