Oauth setup#3538
Conversation
|
Warning Review limit reached
Your plan includes 1 review of capacity. Refill in 22 minutes and 1 second. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more review capacity refills, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than trial, open-source, and free plans. In all cases, review capacity refills continuously over time. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR refactors session authentication architecture by moving token fields from the user object to the top-level session, introduces enhanced OAuth profile fetching with abort/timeout support, and integrates server-backed session verification into the password dialog. Additionally, the subscription status UI now uses a Banner component with severity levels. ChangesSession Architecture and Auth Verification
Subscription Section Banner Update
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
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 |
|
New azure analytics_platform changes available for preview here |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/platform/src/shared/components/auth/SetPasswordPromptDialog.tsx (1)
273-277: ⚡ Quick winRemove redundant
nextAuthMethodsredeclaration.
nextAuthMethodsis already declared at lines 231-235 with identical logic and remains in scope within the catch block. This redeclaration is unnecessary.if (message.toLowerCase().includes('already set')) { - const nextAuthMethods: AuthMethods = { - ...DEFAULT_AUTH_METHODS, - ...authMethods, - password: true, - }; const refreshOutcome = await refreshSessionState(nextAuthMethods);🤖 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/platform/src/shared/components/auth/SetPasswordPromptDialog.tsx` around lines 273 - 277, Remove the redundant redeclaration of nextAuthMethods inside the catch block: reuse the existing nextAuthMethods (declared earlier at lines 231-235) instead of reassigning it again; locate the catch block in SetPasswordPromptDialog where nextAuthMethods, DEFAULT_AUTH_METHODS, and authMethods are referenced and delete the duplicate const nextAuthMethods = { ...DEFAULT_AUTH_METHODS, ...authMethods, password: true } so the code uses the already-in-scope nextAuthMethods variable.src/platform/src/shared/lib/auth.ts (1)
54-89: 💤 Low valueConsider consolidating duplicate
normalizeAuthMethodsandAUTH_METHOD_KEYS.Both
auth.tsandoauth-session.tsdefine identicalAUTH_METHOD_KEYSarrays andnormalizeAuthMethodsfunctions. If these diverge in the future, it could lead to subtle inconsistencies.You could export
normalizeAuthMethodsfromoauth-session.tsand import it here, or extract it into a shared utility.🤖 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/platform/src/shared/lib/auth.ts` around lines 54 - 89, The file defines duplicate AUTH_METHOD_KEYS and normalizeAuthMethods which are identical to ones in oauth-session.ts; extract AUTH_METHOD_KEYS and normalizeAuthMethods into a shared utility module (or export the implementation from oauth-session.ts) and import them here to avoid divergence. Update references in this file to use the shared normalizeAuthMethods and AUTH_METHOD_KEYS symbols (or the exported normalizeAuthMethods) and remove the local duplicate definitions so both auth.ts and oauth-session.ts rely on the single shared implementation.
🤖 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/platform/src/shared/components/auth/SetPasswordPromptDialog.tsx`:
- Around line 273-277: Remove the redundant redeclaration of nextAuthMethods
inside the catch block: reuse the existing nextAuthMethods (declared earlier at
lines 231-235) instead of reassigning it again; locate the catch block in
SetPasswordPromptDialog where nextAuthMethods, DEFAULT_AUTH_METHODS, and
authMethods are referenced and delete the duplicate const nextAuthMethods = {
...DEFAULT_AUTH_METHODS, ...authMethods, password: true } so the code uses the
already-in-scope nextAuthMethods variable.
In `@src/platform/src/shared/lib/auth.ts`:
- Around line 54-89: The file defines duplicate AUTH_METHOD_KEYS and
normalizeAuthMethods which are identical to ones in oauth-session.ts; extract
AUTH_METHOD_KEYS and normalizeAuthMethods into a shared utility module (or
export the implementation from oauth-session.ts) and import them here to avoid
divergence. Update references in this file to use the shared
normalizeAuthMethods and AUTH_METHOD_KEYS symbols (or the exported
normalizeAuthMethods) and remove the local duplicate definitions so both auth.ts
and oauth-session.ts rely on the single shared implementation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef6757b7-8e2c-4153-9571-cbf938c31e1f
📒 Files selected for processing (8)
src/platform/src/modules/billing/components/SubscriptionSection.tsxsrc/platform/src/next-auth.d.tssrc/platform/src/shared/components/auth/SetPasswordPromptDialog.tsxsrc/platform/src/shared/lib/auth.tssrc/platform/src/shared/lib/oauth-session.tssrc/platform/src/shared/services/apiClient.tssrc/platform/src/shared/services/sessionAuthToken.tssrc/platform/src/shared/types/global.d.ts
💤 Files with no reviewable changes (1)
- src/platform/src/shared/types/global.d.ts
…swordPromptDialog
|
New azure analytics_platform changes available for preview here |
|
New azure analytics_platform changes available for preview here |
2 participants
Summary of Changes (What does this PR do?)
Status of maturity (all need to be checked before merging):
How should this be manually tested?
What are the relevant tickets?
Screenshots (optional)
Summary by CodeRabbit
New Features
Bug Fixes
Refactor