fix: redirect OAuth login in current page - #3329
Conversation
WalkthroughA new shared Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
web/src/helpers/api.js (3)
266-268: Consider addingopenInNewTabparameter for API consistency.The Discord OAuth flow now redirects in the current window by default, which is a breaking behavior change from the previous
window.open()implementation. While this aligns with the PR objective, consider exposing anopenInNewTaboption (like OIDC does) to maintain API consistency and provide flexibility for different use cases.♻️ Proposed change for consistency
-export async function onDiscordOAuthClicked(client_id, options = {}) { +export async function onDiscordOAuthClicked(client_id, openInNewTab = false, options = {}) { const state = await prepareOAuthState(options); if (!state) return; const redirect_uri = `${window.location.origin}/oauth/discord`; const response_type = 'code'; const scope = 'identify+openid'; - redirectToOAuthUrl( + redirectToOAuthUrl( `https://discord.com/oauth2/authorize?client_id=${client_id}&redirect_uri=${redirect_uri}&response_type=${response_type}&scope=${scope}&state=${state}`, + { openInNewTab } ); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/api.js` around lines 266 - 268, Add an optional openInNewTab boolean parameter to the Discord OAuth helper (the redirectToOAuthUrl call/site that constructs the URL) so callers can preserve the old window.open behavior; when openInNewTab is true use window.open(oauthUrl, "_blank", "noopener") and when false use window.location.assign(oauthUrl) (or the existing redirectToOAuthUrl logic) and default the parameter to false for current behavior, updating function signature(s) and any callers that should opt into the new option.
40-50: Add JSDoc documentation for the new helper function.The
redirectToOAuthUrlfunction centralizes important OAuth navigation logic but lacks documentation. Adding JSDoc will improve maintainability and help future developers understand the purpose and parameters.📝 Suggested documentation
+/** + * Redirect to OAuth URL either in current window or new tab + * `@param` {string|URL} url - The OAuth URL to navigate to + * `@param` {Object} options - Navigation options + * `@param` {boolean} options.openInNewTab - Whether to open URL in new tab (default: false) + */ function redirectToOAuthUrl(url, options = {}) { const { openInNewTab = false } = options; const targetUrl = typeof url === 'string' ? url : url.toString();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/api.js` around lines 40 - 50, Add JSDoc for the new helper function redirectToOAuthUrl: document the function purpose (redirects to an OAuth URL, either in same tab or new tab), annotate the url parameter as (string|URL) and explain it will be coerced via toString, document the optional options param with property openInNewTab {boolean} default false, indicate the function returns {void}, and include a short example of calling redirectToOAuthUrl('https://...', { openInNewTab: true }) to show intended usage; attach this JSDoc immediately above the redirectToOAuthUrl function declaration.
291-293: Consider addingopenInNewTabparameter for API consistency.Both GitHub and LinuxDO OAuth flows now redirect in the current window by default, changing from the previous
window.open()behavior. For consistency with the OIDC implementation, consider adding anopenInNewTabparameter to these functions as well.Also applies to: 302-304
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/src/helpers/api.js` around lines 291 - 293, The GitHub and LinuxDO OAuth helpers currently always redirect in the same window; add an optional boolean parameter openInNewTab to the functions that build/trigger the OAuth redirect (e.g., the GitHub helper calling redirectToOAuthUrl and the LinuxDO helper) defaulting to false, and when true invoke window.open(url, "_blank", "noopener,noreferrer") (or pass a flag through to redirectToOAuthUrl) instead of navigating the current window; update all call sites to pass openInNewTab where the OIDC flow expects it to maintain consistent behaviour across auth flows.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@web/src/helpers/api.js`:
- Line 352: Call redirectToOAuthUrl with the open-in-new-tab option to restore
original behavior and match the OIDC call pattern: replace the bare
redirectToOAuthUrl(authUrl) invocation with redirectToOAuthUrl(authUrl, {
openInNewTab }) (or redirectToOAuthUrl(authUrl, { openInNewTab: true }) if you
want to always open a new window). Update callers that rely on loading states
(LoginForm.jsx, RegisterForm.jsx and AccountManagement's
handleBindCustomOAuth/onCustomOAuthClicked) to pass the appropriate openInNewTab
value so the API contract remains consistent and UI feedback remains correct.
---
Nitpick comments:
In `@web/src/helpers/api.js`:
- Around line 266-268: Add an optional openInNewTab boolean parameter to the
Discord OAuth helper (the redirectToOAuthUrl call/site that constructs the URL)
so callers can preserve the old window.open behavior; when openInNewTab is true
use window.open(oauthUrl, "_blank", "noopener") and when false use
window.location.assign(oauthUrl) (or the existing redirectToOAuthUrl logic) and
default the parameter to false for current behavior, updating function
signature(s) and any callers that should opt into the new option.
- Around line 40-50: Add JSDoc for the new helper function redirectToOAuthUrl:
document the function purpose (redirects to an OAuth URL, either in same tab or
new tab), annotate the url parameter as (string|URL) and explain it will be
coerced via toString, document the optional options param with property
openInNewTab {boolean} default false, indicate the function returns {void}, and
include a short example of calling redirectToOAuthUrl('https://...', {
openInNewTab: true }) to show intended usage; attach this JSDoc immediately
above the redirectToOAuthUrl function declaration.
- Around line 291-293: The GitHub and LinuxDO OAuth helpers currently always
redirect in the same window; add an optional boolean parameter openInNewTab to
the functions that build/trigger the OAuth redirect (e.g., the GitHub helper
calling redirectToOAuthUrl and the LinuxDO helper) defaulting to false, and when
true invoke window.open(url, "_blank", "noopener,noreferrer") (or pass a flag
through to redirectToOAuthUrl) instead of navigating the current window; update
all call sites to pass openInNewTab where the OIDC flow expects it to maintain
consistent behaviour across auth flows.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 275ad87d-b7ee-404c-a263-0c78d6cdb476
📒 Files selected for processing (1)
web/src/helpers/api.js
| window.open(authUrl.toString()); | ||
|
|
||
| redirectToOAuthUrl(authUrl); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify loading state handling in OAuth calling components
# Check LoginForm loading state handling
echo "=== LoginForm custom OAuth loading state ==="
rg -n -A10 -B2 'handleCustomOAuthClick' web/src/components/auth/LoginForm.jsx
echo -e "\n=== RegisterForm custom OAuth loading state ==="
rg -n -A10 -B2 'handleCustomOAuthClick' web/src/components/auth/RegisterForm.jsx
echo -e "\n=== AccountManagement custom OAuth handling ==="
rg -n -A5 -B2 'handleBindCustomOAuth|onCustomOAuthClicked' web/src/components/settings/personal/cards/AccountManagement.jsxRepository: QuantumNous/new-api
Length of output: 3248
🏁 Script executed:
# Find the redirectToOAuthUrl function implementation
rg -n 'function redirectToOAuthUrl|const redirectToOAuthUrl|redirectToOAuthUrl\s*=' web/src/helpers/ -A15Repository: QuantumNous/new-api
Length of output: 888
🏁 Script executed:
# Also check the api.js file around line 352 to understand context
cat -n web/src/helpers/api.js | sed -n '340,360p'Repository: QuantumNous/new-api
Length of output: 811
🏁 Script executed:
# Check if there are any other references to this function to understand its usage
rg -n 'redirectToOAuthUrl' web/src/ -B2 -A2Repository: QuantumNous/new-api
Length of output: 2025
Pass openInNewTab parameter for API consistency and preserve original new-window behavior.
The call at line 352 should pass { openInNewTab } to match the OIDC pattern (line 285). The original implementation used window.open(), which opens in a new window. The current redirectToOAuthUrl(authUrl) call defaults to same-window redirect via window.location.assign(), which changes the user experience:
-
Loading state visibility: The
finallyblocks inLoginForm.jsx(lines 399-403) andRegisterForm.jsx(lines 340-344) won't execute during same-window redirects. ThesetTimeoutin RegisterForm (3000ms) will be abandoned when the page unloads, causing the loading spinner to disappear before OAuth completes. LoginForm already documents this limitation (line 400). -
API consistency: Passing
{ openInNewTab }aligns with how OIDC calls this function (line 285) and maintains a consistent API contract. -
Missing feedback in AccountManagement: The
handleBindCustomOAuthfunction (lines 146-148) callsonCustomOAuthClickedwithout any loading state, leaving users without visual feedback when binding OAuth from account settings.
Recommendation: Pass { openInNewTab: true } to restore the original new-window behavior, or if switching to same-window, update the calling components to handle loading state appropriately.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@web/src/helpers/api.js` at line 352, Call redirectToOAuthUrl with the
open-in-new-tab option to restore original behavior and match the OIDC call
pattern: replace the bare redirectToOAuthUrl(authUrl) invocation with
redirectToOAuthUrl(authUrl, { openInNewTab }) (or redirectToOAuthUrl(authUrl, {
openInNewTab: true }) if you want to always open a new window). Update callers
that rely on loading states (LoginForm.jsx, RegisterForm.jsx and
AccountManagement's handleBindCustomOAuth/onCustomOAuthClicked) to pass the
appropriate openInNewTab value so the API contract remains consistent and UI
feedback remains correct.
2 participants
将OAuth登录的打开方式由新页面打开改成当前页面打开以避免safari等浏览器的拦截
Summary by CodeRabbit