Skip to content

Fix redirectUri for roblox oauth#175

Open
raadtotheraad wants to merge 1 commit into
PlanetaryOrbit:mainfrom
raadtotheraad:main
Open

Fix redirectUri for roblox oauth#175
raadtotheraad wants to merge 1 commit into
PlanetaryOrbit:mainfrom
raadtotheraad:main

Conversation

@raadtotheraad

@raadtotheraad raadtotheraad commented Jul 21, 2026

Copy link
Copy Markdown

Not sure if this is right, but orbit is trying to use the discord redirect URL for roblox oauth

This is basically everything that i changed

Or is it like this on purpose?

Summary by CodeRabbit

  • Bug Fixes
    • Corrected the Roblox sign-in flow to redirect users to the appropriate callback endpoint.
    • Improved successful completion of Roblox OAuth authentication.

@coderabbitai

coderabbitai Bot commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

The Roblox OAuth start handler now points authorization redirects to /api/auth/roblox/callback instead of the Discord callback endpoint. No other control-flow behavior changes.

Changes

Roblox OAuth flow

Layer / File(s) Summary
Correct Roblox callback URI
pages/api/auth/roblox/start.ts
The OAuth redirect URI targets the Roblox callback route; the handler’s closing structure is unchanged.

Estimated code review effort: 1 (Trivial) | ~2 minutes

Possibly related PRs

  • PlanetaryOrbit/orbit#171: Updates related Roblox OAuth redirect URI handling in instance configuration and the index page.

Suggested reviewers: breadddevv

🚥 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 matches the main change: fixing the Roblox OAuth redirect URI.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

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

Inline comments:
In `@pages/api/auth/roblox/start.ts`:
- Line 18: Update the redirect URI assignment in the Roblox start handler to use
the same resolver and precedence as the callback handler, including
ROBLOX_REDIRECT_URI and instanceConfig.robloxRedirectUri overrides before base
URL fallbacks. Ensure authorization and token exchange send the identical
callback URI, preferably by reusing the callback’s shared resolver.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 403fa597-ae24-4910-b8e3-2897913e9584

📥 Commits

Reviewing files that changed from the base of the PR and between 1642b97 and 5d69266.

📒 Files selected for processing (1)
  • pages/api/auth/roblox/start.ts

let redirectUri: string | undefined;
clientId = process.env.ROBLOX_CLIENT_ID;
redirectUri = `${process.env.PLANETARY_CLOUD_URL ? `https://${process.env.PLANETARY_CLOUD_URL}` : process.env.NEXTAUTH_URL || process.env.PUBLIC_URL}/api/auth/discord/callback`;
redirectUri = `${process.env.PLANETARY_CLOUD_URL ? `https://${process.env.PLANETARY_CLOUD_URL}` : process.env.NEXTAUTH_URL || process.env.PUBLIC_URL}/api/auth/roblox/callback`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Reuse the callback’s redirect URI resolution.

pages/api/auth/roblox/callback.ts honors ROBLOX_REDIRECT_URI and instanceConfig.robloxRedirectUri, but this line always derives the URI from base URL variables. When an override is configured, Roblox receives one redirect_uri during authorization and a different one during token exchange, causing the OAuth flow to fail. Use a shared resolver or identical precedence here.

🤖 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 `@pages/api/auth/roblox/start.ts` at line 18, Update the redirect URI
assignment in the Roblox start handler to use the same resolver and precedence
as the callback handler, including ROBLOX_REDIRECT_URI and
instanceConfig.robloxRedirectUri overrides before base URL fallbacks. Ensure
authorization and token exchange send the identical callback URI, preferably by
reusing the callback’s shared resolver.

@breadddevv

Copy link
Copy Markdown
Collaborator

I already had that fixed. Just didn't push.

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.

2 participants