feat: create RoleGuard component for role-based route protection #178
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a client-side role system (RoleGuard component, useUserRole hook, and role-related types). Other edits are whitespace/formatting-only changes across backend configs, services, controllers, tests, web tests, and tsconfig files. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Page as Next.js Page
participant RG as RoleGuard
participant Hook as useUserRole
participant Auth as useAuth
participant API as profileAPI
participant LS as localStorage
participant Router as next/router
Page->>RG: mount (requiredRole)
RG->>Hook: call useUserRole()
Hook->>Auth: read user, isAuthenticated
alt not authenticated
Hook-->>RG: roleInfo { role: 'guest', canAccessHostDashboard: false, hasProperties: false } (isLoading=false)
else authenticated
Hook->>API: getUserProfile(user.id)
alt API success
API-->>Hook: profile { hostStatus, hasProperties }
Hook-->>LS: save hostStatus/hasProperties
Hook-->>RG: roleInfo { role, hostStatus, canAccessHostDashboard, hasProperties } (isLoading=false)
else API failure
Hook->>LS: read hostStatus/hasProperties
LS-->>Hook: stored values
Hook-->>RG: derived roleInfo from localStorage (isLoading=false)
end
end
alt requiredRole == 'host' AND !canAccessHostDashboard
RG->>Router: push(fallbackPath) — redirect in useEffect
Router-->>Page: navigation to fallback
else
RG-->>Page: render children
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (5)
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: 4
🧹 Nitpick comments (3)
apps/web/src/types/roles.ts (1)
1-11: LGTM! Consider adding JSDoc for clarity (optional).The role types are well-defined and support the new role-based guard system. The interface is clear and minimal.
Optionally, add JSDoc comments to improve discoverability and IDE support:
// Role types for guest/host system +/** + * Represents the user's role in the platform. + * - 'guest': User who books properties + * - 'host': User who lists properties + * - 'dual': User who can both book and list properties + */ export type UserRole = 'guest' | 'host' | 'dual'; +/** + * Host verification status. + * - 'pending': Awaiting verification + * - 'verified': Approved to list properties + * - 'rejected': Verification denied + * - 'suspended': Temporarily restricted + */ export type HostStatus = 'pending' | 'verified' | 'rejected' | 'suspended'; +/** + * Complete role information for the current user. + */ export interface RoleInfo { + /** User's current role */ role: UserRole; + /** Host verification status (only applicable for hosts) */ hostStatus?: HostStatus; + /** Whether user can access the host dashboard */ canAccessHostDashboard: boolean; + /** Whether user has any listed properties */ hasProperties: boolean; }apps/web/src/components/guards/RoleGuard.tsx (1)
14-46: Consider expanding RoleGuard to handle all UserRole types.Currently, RoleGuard only checks
requiredRole === 'host'and ignores 'guest' and 'dual' roles. This limits the component's utility for protecting other role-based routes.Consider adding logic to handle all role types:
export function RoleGuard({ children, requiredRole, fallbackPath = '/become-host', }: RoleGuardProps) { - const { canAccessHostDashboard } = useUserRole(); + const { role, canAccessHostDashboard } = useUserRole(); const router = useRouter(); - useEffect(() => { - if (requiredRole === 'host' && !canAccessHostDashboard) { - router.push(fallbackPath); - } - }, [requiredRole, canAccessHostDashboard, router, fallbackPath]); - + // Check access based on required role + const hasAccess = + requiredRole === 'guest' ? true : + requiredRole === 'host' ? canAccessHostDashboard : + requiredRole === 'dual' ? role === 'dual' : + false; + - if (requiredRole === 'host' && !canAccessHostDashboard) { + if (!hasAccess) { return ( <div className="flex flex-col items-center justify-center min-h-screen p-4"> <div className="text-center max-w-md"> - <h2 className="text-2xl font-bold mb-4">Host Access Required</h2> - <p className="text-gray-600 mb-6">You need to become a host to access this page.</p> + <h2 className="text-2xl font-bold mb-4">Access Required</h2> + <p className="text-gray-600 mb-6"> + You need {requiredRole} access to view this page. + </p> <button type="button" onClick={() => router.push(fallbackPath)} className="bg-blue-600 text-white px-6 py-3 rounded-lg hover:bg-blue-700" > - Become a Host + {requiredRole === 'host' ? 'Become a Host' : 'Get Access'} </button> </div> </div> ); } return <>{children}</>; }apps/web/src/hooks/useUserRole.tsx (1)
15-48: Consider SSR hydration mismatch with localStorage.This hook reads from localStorage during render, which is not available during server-side rendering in Next.js. This could cause hydration mismatches where the server renders a "guest" state but the client immediately updates to a different role, causing React hydration warnings.
Add a loading/mounted state to prevent hydration mismatches:
export function useUserRole(): RoleInfo { const { user, isAuthenticated } = useAuth(); + const [isMounted, setIsMounted] = useState(false); const [roleInfo, setRoleInfo] = useState<RoleInfo>({ role: 'guest', canAccessHostDashboard: false, hasProperties: false, }); + // Handle client-side mounting + useEffect(() => { + setIsMounted(true); + }, []); + useEffect(() => { + if (!isMounted) return; + if (!isAuthenticated || !user) { setRoleInfo({ role: 'guest', canAccessHostDashboard: false, hasProperties: false, }); return; } // ... rest of the logic - }, [user, isAuthenticated]); + }, [user, isAuthenticated, isMounted]); return roleInfo; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/backend/bun.config.ts(0 hunks)apps/backend/src/config/supabase.ts(7 hunks)apps/backend/src/controllers/booking.controller.ts(4 hunks)apps/backend/src/services/sync.service.ts(1 hunks)apps/backend/tests/integration/booking-integration.test.ts(3 hunks)apps/backend/tests/utils/booking-test.utils.ts(1 hunks)apps/web/src/components/guards/RoleGuard.tsx(1 hunks)apps/web/src/hooks/useUserRole.tsx(1 hunks)apps/web/src/types/roles.ts(1 hunks)apps/web/test-results/.last-run.json(1 hunks)apps/web/tests/e2e/auth-guards.spec.ts(4 hunks)apps/web/tests/e2e/auth.spec.ts(1 hunks)apps/web/tests/e2e/dashboards.spec.ts(2 hunks)apps/web/tests/e2e/role-separation.spec.ts(1 hunks)apps/web/tsconfig.json(3 hunks)
💤 Files with no reviewable changes (1)
- apps/backend/bun.config.ts
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/components/guards/RoleGuard.tsx (2)
apps/web/src/types/roles.ts (1)
UserRole(2-2)apps/web/src/hooks/useUserRole.tsx (1)
useUserRole(7-51)
apps/web/src/hooks/useUserRole.tsx (3)
apps/web/src/types/roles.ts (2)
RoleInfo(6-11)UserRole(2-2)apps/web/src/hooks/auth/use-auth.tsx (1)
useAuth(251-251)apps/web/src/services/authService.ts (1)
isAuthenticated(86-105)
🪛 ast-grep (0.39.5)
apps/web/tests/e2e/auth-guards.spec.ts
[warning] 28-28: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(PUBLIC_ROUTE)
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
- https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
- https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🔇 Additional comments (2)
apps/backend/tests/integration/booking-integration.test.ts (1)
1-1: LGTM!The explicit
bun:testimport clarifies the test framework in use. The formatting adjustments across the mock setup improve consistency without altering behavior.apps/web/tests/e2e/auth-guards.spec.ts (1)
28-28: Static analysis false positive can be ignored.The static analysis tool flagged
new RegExp(PUBLIC_ROUTE)as a potential ReDoS vulnerability. However,PUBLIC_ROUTEis a constant string'/login'defined at line 5, not user-controlled input, so this warning is a false positive and can be safely ignored.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/web/src/components/guards/RoleGuard.tsx (1)
7-11: Type-implementation mismatch: only 'host' role is guarded.The
requiredRoleprop acceptsUserRole('guest' | 'host' | 'dual'), but line 21 only implements protection for 'host'. Passing 'guest' or 'dual' results in no access control.Consider either:
Option 1: Narrow the type to reflect current implementation:
interface RoleGuardProps { children: React.ReactNode; - requiredRole: UserRole; + requiredRole: 'host'; fallbackPath?: string; }Option 2: Implement guards for all roles:
export function RoleGuard({ children, requiredRole, fallbackPath = '/become-host', }: RoleGuardProps) { - const { canAccessHostDashboard } = useUserRole(); + const { canAccessHostDashboard, role } = useUserRole(); const router = useRouter(); if (requiredRole === 'host' && !canAccessHostDashboard) { return ( // ... existing fallback UI ); } + + if (requiredRole === 'guest' && role !== 'guest') { + // Optionally guard guest-only routes + return <div>/* guest-only fallback */</div>; + } return <>{children}</>; }Also applies to: 21-21
apps/web/src/hooks/useUserRole.tsx (1)
29-34: Type assertion now safe; consider importing HostStatus for consistency.The validation logic correctly guards against invalid localStorage values before the type assertion, resolving the previous unsafe cast issue.
For better maintainability, import
HostStatusfrom the types module instead of duplicating the union inline:-import type { RoleInfo, UserRole } from '~/types/roles'; +import type { HostStatus, RoleInfo, UserRole } from '~/types/roles'; import { useAuth } from './auth/use-auth'; export function useUserRole(): RoleInfo { // ... // Validate hostStatus - const validHostStatuses = ['pending', 'verified', 'rejected', 'suspended']; + const validHostStatuses: HostStatus[] = ['pending', 'verified', 'rejected', 'suspended']; const hostStatus = storedHostStatus && validHostStatuses.includes(storedHostStatus) - ? (storedHostStatus as 'pending' | 'verified' | 'rejected' | 'suspended') + ? (storedHostStatus as HostStatus) : undefined;This ensures type safety: if
HostStatuschanges, TypeScript will flag the mismatch in the validation array.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/src/components/guards/RoleGuard.tsx(1 hunks)apps/web/src/hooks/useUserRole.tsx(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
apps/web/src/components/guards/RoleGuard.tsx (2)
apps/web/src/types/roles.ts (1)
UserRole(2-2)apps/web/src/hooks/useUserRole.tsx (1)
useUserRole(7-58)
apps/web/src/hooks/useUserRole.tsx (3)
apps/web/src/types/roles.ts (2)
RoleInfo(6-11)UserRole(2-2)apps/web/src/hooks/auth/use-auth.tsx (1)
useAuth(251-251)apps/web/src/services/authService.ts (1)
isAuthenticated(86-105)
🔇 Additional comments (2)
apps/web/src/components/guards/RoleGuard.tsx (2)
21-37: Previous race condition issue resolved.The removal of the useEffect redirect eliminates the flash-of-content issue. The guard now properly renders only the fallback UI when access is denied, without any navigation side effects.
29-29: Previous hardcoded path issue resolved.The button now correctly uses the
fallbackPathprop instead of hardcoding '/become-host', making the component properly configurable.
- Add useEffect hook for automatic redirection when access is denied - Implement loading state to prevent flash of unauthorized content - Show spinner during authentication verification - Return null during redirect to prevent content flash - Improve UX with clear loading feedback
- Add isLoading state to track role verification progress - Fetch user role from API instead of relying solely on localStorage - Implement fallback to localStorage when API fails for offline support - Cache API response in localStorage for faster subsequent loads - Export UseUserRoleReturn interface for type safety - Improve error handling with warning logs
- Add hostStatus field to UserProfile (pending, verified, rejected, suspended) - Add hasProperties boolean flag to UserProfile - Add isLoading optional field to RoleInfo interface - Update both index.ts and shared.ts type definitions for consistency - Enables proper type checking for host verification flow
2 participants
Pull Request | StellarRent
📝 Summary
Acceptance Criteria Met:
🔗 Related Issues
Closes **#165
🔄 Changes Made
Provide a general description of the changes. Include any relevant background information or context to help reviewers understand the purpose of this PR.
🖼️ Current Output
Provide visual evidence of the changes:
🧪 Testing
If applicable, describe the tests performed. Include screenshots, test outputs, or any resources that help reviewers understand how the changes were tested.
✅ Testing Checklist
List any possible issues that might arise with this change.
🚀 Next Steps & Improvements
This change lays a solid foundation for further optimizations. Some areas that could benefit from future improvements include:
💬 Comments
Any additional context, questions, or considerations for reviewers.
Summary by CodeRabbit
New Features
Types
Tests
Chores