-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Support validating users based off their user roles #623
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: Support validating users based off their user roles #623
Conversation
🦋 Changeset detectedLatest commit: f2bacb4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis change set implements a comprehensive overhaul of authentication and authorization handling across the codebase. It introduces role-based user validation, propagates user roles throughout session types and GraphQL schemas, and updates generator configurations to reflect these enhancements. Numerous generator names and import paths are refactored for clarity and specificity, particularly in the local-auth domain. Several obsolete files and dependencies, such as Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Frontend (React)
participant GraphQL API
participant DB
User->>Frontend (React): Login/Register
Frontend (React)->>GraphQL API: Mutation (Login/Register)
GraphQL API->>DB: Validate credentials, fetch roles
DB-->>GraphQL API: User data + roles
GraphQL API-->>Frontend (React): Session { userId, roles }
Frontend (React)->>Frontend (React): Store session (roles included)
User->>Frontend (React): Navigate to protected route
Frontend (React)->>Frontend (React): Check session.userId and session.roles
alt Not authenticated
Frontend (React)-->>User: Redirect to login
else Lacks required role
Frontend (React)-->>User: Show InvalidRoleError
else Authorized
Frontend (React)-->>User: Render route
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
Documentation and Community
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (6)
.changeset/fluffy-toys-ring.md (1)
1-6: Polish the summary for cleaner release notesThe free-form line is what ends up in the changelog. Turning it into a full-sentence description makes the entry easier to scan and keeps the tense consistent with other notes.
-Remove @headlessui/react package +Remove unused @headlessui/react dependency from react-generatorspackages/react-generators/src/generators/auth/auth-errors/templates/src/utils/auth-errors.ts (1)
3-8: LGTM! Consider enhancing error context for better debugging.The
InvalidRoleErrorclass correctly extendsErrorand follows standard patterns. Consider adding additional context properties for better error handling:export class InvalidRoleError extends Error { - constructor(message: string) { + constructor(message: string, public requiredRoles?: string[], public userRoles?: string[]) { super(message); this.name = 'InvalidRoleError'; } }This would provide more context for debugging role validation failures.
plugins/plugin-auth/src/local-auth/core/generators/auth-hooks/templates/src/hooks/use-log-out.ts (1)
5-6: Fix import order to follow alphabetical sorting.The imports are not properly sorted -
%reactSessionImportsshould come after@apollo/clientalphabetically.Apply this diff to fix the import order:
-import { userSessionClient } from '%reactSessionImports'; import { useApolloClient, useMutation } from '@apollo/client'; +import { userSessionClient } from '%reactSessionImports';plugins/plugin-auth/src/local-auth/core/generators/auth-hooks/templates/src/hooks/use-session.ts (1)
1-1: Consider removing @ts-nocheck directiveThe
@ts-nocheckdirective disables all TypeScript checking for this file. If this is a template file that gets processed, consider if TypeScript checking can be enabled to catch potential type errors early.plugins/plugin-auth/src/local-auth/core/generators/react-session/templates/src/app/user-session-provider.tsx (1)
65-67: Consider handling loading state explicitly.The
ErrorableLoadercomponent is rendered whensessionis undefined (loading state), but it's only passed the error prop. This might not show a proper loading indicator.Consider passing a loading prop or using a dedicated loading component:
- if (!session) { - return <ErrorableLoader error={sessionError} />; - } + if (!session) { + return <ErrorableLoader error={sessionError} loading={!sessionQueryData && !!cachedUserId} />; + }plugins/plugin-auth/src/local-auth/core/generators/react-session/templates/src/services/user-session-client.ts (1)
66-70: Consider consistency in null vs undefined handling.The method returns
string | undefinedbut the internal logic mixes null and undefined. The nullish coalescing with?? undefinedconverts null to undefined, which is good for API consistency, but the privatesetUserIdmethod still uses null internally.Consider updating the private method to use undefined consistently:
- private setUserId(userId: string | null): void { + private setUserId(userId: string | undefined): void { const currentUserId = this.getUserId(); const userIdChanged = userId !== currentUserId; if (!userIdChanged) { return; } if (userId) { this.storage.setItem(UserSessionClient.USER_ID_STORAGE_KEY, userId); } else { this.storage.removeItem(UserSessionClient.USER_ID_STORAGE_KEY); } }And update the
signOutmethod:- this.setUserId(null); + this.setUserId(undefined);
| import { userSessionClient } from '$userSessionClient'; | ||
| import { GetCurrentUserSessionDocument } from '%generatedGraphqlImports'; | ||
| import { AuthSessionContext } from '%localAuthHooksImports'; | ||
| import { ErrorableLoader } from '%reactComponentsImports'; | ||
| import { logAndFormatError } from '%reactErrorImports'; | ||
| import { useQuery } from '@apollo/client'; | ||
| import { useEffect, useMemo, useState } from 'react'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix import ordering to comply with ESLint rules.
External imports should come before internal/local imports according to the coding guidelines.
Apply this diff to fix the import ordering:
-import { userSessionClient } from '$userSessionClient';
-import { GetCurrentUserSessionDocument } from '%generatedGraphqlImports';
-import { AuthSessionContext } from '%localAuthHooksImports';
-import { ErrorableLoader } from '%reactComponentsImports';
-import { logAndFormatError } from '%reactErrorImports';
-import { useQuery } from '@apollo/client';
+import { useQuery } from '@apollo/client';
import { useEffect, useMemo, useState } from 'react';
+
+import { GetCurrentUserSessionDocument } from '%generatedGraphqlImports';
+import { AuthSessionContext } from '%localAuthHooksImports';
+import { ErrorableLoader } from '%reactComponentsImports';
+import { logAndFormatError } from '%reactErrorImports';
+import { userSessionClient } from '$userSessionClient';📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { userSessionClient } from '$userSessionClient'; | |
| import { GetCurrentUserSessionDocument } from '%generatedGraphqlImports'; | |
| import { AuthSessionContext } from '%localAuthHooksImports'; | |
| import { ErrorableLoader } from '%reactComponentsImports'; | |
| import { logAndFormatError } from '%reactErrorImports'; | |
| import { useQuery } from '@apollo/client'; | |
| import { useEffect, useMemo, useState } from 'react'; | |
| import { useQuery } from '@apollo/client'; | |
| import { useEffect, useMemo, useState } from 'react'; | |
| import { GetCurrentUserSessionDocument } from '%generatedGraphqlImports'; | |
| import { AuthSessionContext } from '%localAuthHooksImports'; | |
| import { ErrorableLoader } from '%reactComponentsImports'; | |
| import { logAndFormatError } from '%reactErrorImports'; | |
| import { userSessionClient } from '$userSessionClient'; |
🧰 Tools
🪛 ESLint
[error] 7-7: Expected "%generatedGraphqlImports" to come before "$userSessionClient".
(perfectionist/sort-imports)
[error] 11-11: Expected "@apollo/client" to come before "%reactErrorImports".
(perfectionist/sort-imports)
🤖 Prompt for AI Agents
In
plugins/plugin-auth/src/local-auth/core/generators/react-session/templates/src/app/user-session-provider.tsx
around lines 6 to 12, the import statements are not ordered correctly according
to ESLint rules. Fix this by placing all external imports (like '@apollo/client'
and 'react' hooks) before any internal or local imports (those starting with '$'
or '%'). Reorder the imports so that external libraries come first, followed by
local project imports.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please ignore these rules for template folders
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kingston, understood! I'll ignore ESLint import ordering rules for files in template folders since they're used for code generation rather than direct execution.
(\ /)
( .. )
o(")(") thoughtful rabbit
✏️ Learnings added
Learnt from: kingston
PR: halfdomelabs/baseplate#623
File: plugins/plugin-auth/src/local-auth/core/generators/react-session/templates/src/app/user-session-provider.tsx:6-12
Timestamp: 2025-07-30T13:27:19.975Z
Learning: Ignore ESLint import ordering rules for files in templates/** directories as these are code generation templates, not direct source code.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T09:09:23.016Z
Learning: Applies to **/*.{ts,tsx} : Use both standalone and React Hook Form controller variants for form components from `@baseplate-dev/ui-components` as appropriate.
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T09:09:23.016Z
Learning: Applies to **/*.tsx : When using form components, integrate them with React Hook Form by passing the `control` prop from `useForm()`.
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{ts,tsx,js} : Sort imports by group: external libs first, then local imports
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{ts,tsx,js} : Order functions such that functions are placed below the variables/functions they use
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: CLAUDE.md:0-0
Timestamp: 2025-07-22T09:09:23.016Z
Learning: Applies to **/*.{ts,tsx} : If a particular interface or type is not exported, change the file so it is exported.
Learnt from: kingston
PR: halfdomelabs/baseplate#613
File: packages/react-generators/src/generators/core/react-components/templates/components/ui/circular-progress/circular-progress.tsx:0-0
Timestamp: 2025-07-16T17:15:56.714Z
Learning: For imports starting with `$` (template aliases), do not require `.js` extensions as these are resolved differently during the generation process, unlike regular file imports which require explicit `.js` extensions under Node 16 module resolution.
Learnt from: kingston
PR: halfdomelabs/baseplate#428
File: packages/project-builder-server/src/sync/index.ts:5-5
Timestamp: 2025-01-23T09:12:46.178Z
Learning: Avoid importing directly from 'dist' directories. Instead, expose functionality through the package's public API and import from the main package entry point.
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{ts,tsx} : If a particular interface or type is not exported, change the file so it is exported
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/index.{ts,tsx,js} : Prefer barrel exports e.g. export * from './foo.js' instead of individual named exports
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{ts,tsx} : Always include return types on top-level functions including React components (`React.ReactElement`)
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{js,ts,tsx} : Node 16 module resolution - include file extensions in imports (`.js`)
Learnt from: CR
PR: halfdomelabs/baseplate#0
File: .cursor/rules/code-style.mdc:0-0
Timestamp: 2025-07-22T09:10:31.413Z
Learning: Applies to **/*.{ts,tsx} : TypeScript with strict type checking
Learnt from: kingston
PR: halfdomelabs/baseplate#592
File: plugins/plugin-auth/src/auth0/generators/react/auth0-hooks/templates/src/hooks/use-required-user-id.ts:1-2
Timestamp: 2025-07-07T18:24:17.522Z
Learning: Files under templates/** directories can use `// @ts-nocheck` because they are templates meant for code generation, not direct type checking.
Summary by CodeRabbit
New Features
Improvements
Refactor
@headlessui/react.Bug Fixes
Chores
Documentation