Reload refresh token if required #286
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces enhancements to the authentication and permissions flow in the web application. Key changes include the addition of a refresh token check in the Changes
Possibly Related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
apps/web/app/api/auth/[...nextauth]/auth.ts (2)
7-7: Consider renaming for clarity.Since
authOptionsis later overridden by a dynamic return withinNextAuth, consider renaming this constant to something likedefaultAuthOptionsto clarify it’s the baseline.-const authOptions = getAuthOptions(); +const defaultAuthOptions = getAuthOptions();
12-23: Dynamic auth options look good; consider edge cases.Switching authentication options based on the
consentparameter is useful. However, consider edge cases like invalid query parameters. Make sure any future query parameters you add inreq.urldon’t conflict with theconsentlogic.apps/web/app/(app)/permissions/consent/page.tsx (1)
8-38: New consent page is succinct and consistent with the design.The layout is clean and the message is clear. Minor mismatch: The component is named
PermissionsErrorPagebut the text references consent. Consider harmonizing to reflect “Consent” in the component name for clarity.-export default function PermissionsErrorPage() { +export default function PermissionsConsentPage() {apps/web/utils/auth.ts (1)
263-274: Ensure robust logging for missing refresh tokens.
Logging an error and capturing an exception when therefreshTokenis null is a good approach to diagnosing token mishandling. Consider whether to escalate this via user-facing messaging or solely rely on logs.apps/web/utils/actions/permissions.ts (1)
27-39: Distinct separation ofhasRefreshTokenstatus.
The new logic clarifies whether a user is missing a refresh token or is fully permissioned, improving control flows in higher-level components. You might want to unify these checks with the existing error returns for easier extension in the future.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/web/app/(app)/PermissionsCheck.tsx(1 hunks)apps/web/app/(app)/permissions/consent/page.tsx(1 hunks)apps/web/app/(app)/permissions/error/page.tsx(1 hunks)apps/web/app/(landing)/login/LoginForm.tsx(2 hunks)apps/web/app/(landing)/login/page.tsx(1 hunks)apps/web/app/api/auth/[...nextauth]/auth.ts(1 hunks)apps/web/components/TokenCheck.tsx(1 hunks)apps/web/utils/actions/permissions.ts(1 hunks)apps/web/utils/auth.ts(3 hunks)
🔇 Additional comments (10)
apps/web/app/api/auth/[...nextauth]/auth.ts (2)
2-3: Good import refactoring for modularization.Importing
getAuthOptionsandcreateScopedLoggerneatly separates concerns. This makes the authentication logic more readable and easier to maintain.
5-5: Logger usage is a solid addition.Instantiating a scoped logger will improve observability and help trace execution flow for the auth logic.
apps/web/components/TokenCheck.tsx (1)
14-18: Clear error handling and redirection logic.Handling each error case separately is straightforward and improves maintainability. Good work returning immediately to prevent unintended fall-through.
apps/web/app/(app)/PermissionsCheck.tsx (1)
18-18: Refresh token check aligns with broader consent flow.It’s consistent to redirect to
/permissions/consentwhen the refresh token is missing. This ensures a clear path for re-consent if needed.apps/web/app/(app)/permissions/error/page.tsx (1)
12-12: Update to display and logout behavior is user-friendly.Changing the heading to “We are missing permissions” shifts perspective in a welcoming way. Passing an error parameter on logout effectively channels the user to re-consent. This helps unify the consent handling flow.
Also applies to: 20-23
apps/web/app/(landing)/login/LoginForm.tsx (2)
3-3: Good practice for import ordering.
The import foruseStateat the top is a nice tidy approach, keeping dependencies organized for better clarity.
56-66: Conditional parameter in signIn call.
The logic to spread the callbackUrl conditionally is clear. Includingconsentonly whenerror === "RequiresReconsent"is a good approach to handle these specific flows.apps/web/app/(landing)/login/page.tsx (1)
44-44: Refine user experience for custom error states.
Hiding the generic error UI forRequiresReconsentensures the user is not shown a confusing failure message when prompting re-consent. This is a clean, user-friendly approach to error handling and re-consent flows.apps/web/utils/auth.ts (1)
26-28: Clarity forgetAuthOptionssignature.
ExportinggetAuthOptionswith a clearconsentparameter helps keep the authentication flow explicit. The typed approach ensures usage clarity across the codebase.apps/web/utils/actions/permissions.ts (1)
25-26: Early return for insufficient permissions.
This short-circuit return keeps the flow streamlined by making it clear when the user lacks certain permissions.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores