fix: secure dashboard role access - #20
Conversation
Restrict the dashboard to elevated roles, move author post management out of /dashboard, and enforce admin-only mutations with database-backed role checks.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR implements role-based access control for the admin dashboard by introducing a centralized role-access module, refactoring authorization checks across admin sections, adding permission-gated UI components, and migrating user-facing blog routes from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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 docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
3 issues found across 40 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="app/(admin)/layout-client.tsx">
<violation number="1" location="app/(admin)/layout-client.tsx:41">
P2: Localize this new dashboard banner instead of hardcoding English text, otherwise the Indonesian locale will regress here.</violation>
</file>
<file name="app/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsx">
<violation number="1" location="app/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsx:151">
P2: This change still allows `read_time_minutes` to become `0` despite `min={1}`. Normalize invalid/empty input to at least `1` before saving to state.</violation>
</file>
<file name="scripts/21_protect_users_privileged_fields.sql">
<violation number="1" location="scripts/21_protect_users_privileged_fields.sql:25">
P1: A user can bypass this role guard by explicitly inserting `role = NULL`. `COALESCE(NULL::integer, 2)` evaluates to `2`, passing the `<> 2` check, but the database stores `NULL` — not `2`. If any application-level role gate uses a comparison that coerces `null` to `0` (e.g., `role <= 1` in JavaScript), the user gains admin/moderator access.
Reject `NULL` by using a default that fails the check instead of one that passes it:</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
| RETURN NEW; | ||
| END IF; | ||
|
|
||
| IF COALESCE((next_row ->> 'role')::integer, 2) <> 2 THEN |
There was a problem hiding this comment.
P1: A user can bypass this role guard by explicitly inserting role = NULL. COALESCE(NULL::integer, 2) evaluates to 2, passing the <> 2 check, but the database stores NULL — not 2. If any application-level role gate uses a comparison that coerces null to 0 (e.g., role <= 1 in JavaScript), the user gains admin/moderator access.
Reject NULL by using a default that fails the check instead of one that passes it:
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At scripts/21_protect_users_privileged_fields.sql, line 25:
<comment>A user can bypass this role guard by explicitly inserting `role = NULL`. `COALESCE(NULL::integer, 2)` evaluates to `2`, passing the `<> 2` check, but the database stores `NULL` — not `2`. If any application-level role gate uses a comparison that coerces `null` to `0` (e.g., `role <= 1` in JavaScript), the user gains admin/moderator access.
Reject `NULL` by using a default that fails the check instead of one that passes it:</comment>
<file context>
@@ -0,0 +1,72 @@
+ RETURN NEW;
+ END IF;
+
+ IF COALESCE((next_row ->> 'role')::integer, 2) <> 2 THEN
+ RAISE EXCEPTION 'Role changes require admin access';
+ END IF;
</file context>
| > | ||
| {isReadOnly && ( | ||
| <div className="border-b bg-muted/40 px-4 py-2 text-sm text-muted-foreground"> | ||
| Moderator access is read-only. Admin approval is required for dashboard changes. |
There was a problem hiding this comment.
P2: Localize this new dashboard banner instead of hardcoding English text, otherwise the Indonesian locale will regress here.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/(admin)/layout-client.tsx, line 41:
<comment>Localize this new dashboard banner instead of hardcoding English text, otherwise the Indonesian locale will regress here.</comment>
<file context>
@@ -32,6 +36,11 @@ export default function DashboardLayoutClient({ children, user }: Props) {
>
+ {isReadOnly && (
+ <div className="border-b bg-muted/40 px-4 py-2 text-sm text-muted-foreground">
+ Moderator access is read-only. Admin approval is required for dashboard changes.
+ </div>
+ )}
</file context>
| min={1} | ||
| value={formData.read_time_minutes} | ||
| onChange={(e) => setFormData({ ...formData, read_time_minutes: parseInt(e.target.value) || 0 })} | ||
| onChange={(e) => setFormData({ ...formData, read_time_minutes: parseInt(e.target.value, 10) || 0 })} |
There was a problem hiding this comment.
P2: This change still allows read_time_minutes to become 0 despite min={1}. Normalize invalid/empty input to at least 1 before saving to state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At app/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsx, line 151:
<comment>This change still allows `read_time_minutes` to become `0` despite `min={1}`. Normalize invalid/empty input to at least `1` before saving to state.</comment>
<file context>
@@ -148,7 +148,7 @@ export function PostEditDialog({ post, open, onOpenChange }: PostEditDialogProps
min={1}
value={formData.read_time_minutes}
- onChange={(e) => setFormData({ ...formData, read_time_minutes: parseInt(e.target.value) || 0 })}
+ onChange={(e) => setFormData({ ...formData, read_time_minutes: parseInt(e.target.value, 10) || 0 })}
/>
</div>
</file context>
| onChange={(e) => setFormData({ ...formData, read_time_minutes: parseInt(e.target.value, 10) || 0 })} | |
| onChange={(e) => setFormData({ ...formData, read_time_minutes: Math.max(1, parseInt(e.target.value, 10) || 1) })} |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e50e4d66c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (error || !user || !access) { | ||
| redirect('/user/auth?redirectTo=/dashboard') |
There was a problem hiding this comment.
Stop sending authenticated failures to /user/auth
This branch redirects to /user/auth?redirectTo=/dashboard even when the user is already signed in but role/profile lookup failed (for example, missing users row or transient DB error). The auth page auto-redirects signed-in sessions back to redirectTo, so affected users can get stuck bouncing between /dashboard and /user/auth instead of recovering. Limit this auth redirect to true unauthenticated cases, and send authenticated-but-invalid access states to a non-looping route (for example /).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
lib/actions/admin/posts.ts (1)
177-220:⚠️ Potential issue | 🟡 MinorFix revalidatePath calls to use the public board URL path.
For
app/(admin)/dashboard/boards/blog, the public pathname is/dashboard/boards/blog. The current revalidatePath calls use/admin/dashboard/boards/blog, which misses the actual board page that admins are viewing, leaving post updates, deletions, and feature toggles to appear stale.Update all three instances in the post mutations:
- Line 219 (updateAdminPost)
- Line 313 (deleteAdminPost)
- Line 349 (toggleAdminPostFeatured)
Change
revalidatePath('/admin/dashboard/boards/blog')torevalidatePath('/dashboard/boards/blog')in each.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/admin/posts.ts` around lines 177 - 220, The three revalidatePath calls in the admin post mutation handlers are using the wrong admin-only path; update revalidatePath('/admin/dashboard/boards/blog') to revalidatePath('/dashboard/boards/blog') in all relevant functions—specifically in updateAdminPost, deleteAdminPost, and toggleAdminPostFeatured—so the public board page is properly revalidated after post updates, deletions, or feature toggles.lib/actions/admin/comments.ts (1)
225-258:⚠️ Potential issue | 🟡 MinorUse
/dashboard/boards/commentsinstead of/admin/dashboard/boards/commentsforrevalidatePath().Next.js route groups like
(admin)are not part of the URL path. Forapp/(admin)/dashboard/boards/comments/page.tsx,revalidatePath()expects/dashboard/boards/comments, not/admin/dashboard/boards/comments. Using the incorrect path will leave stale content after delete/dismiss/action flows.♻️ Proposed fix
- revalidatePath('/admin/dashboard/boards/comments') + revalidatePath('/dashboard/boards/comments')Also applies to: lines 272–290, 307–355
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/admin/comments.ts` around lines 225 - 258, The revalidation paths use the incorrect route group prefix; update all revalidatePath calls in this file (e.g., in the comment delete/dismiss/action handlers such as the function around the supabase delete/select logic and the other blocks flagged at lines 272–290 and 307–355) to remove the "(admin)" segment—replace '/admin/dashboard/boards/comments' with '/dashboard/boards/comments' (and any other occurrences that include '/admin/dashboard/...') so revalidatePath('/dashboard/boards/comments') is used consistently after comment create/delete/dismiss actions.lib/actions/admin/projects.ts (1)
184-234:⚠️ Potential issue | 🟡 MinorFix route path in
revalidatePath()calls for project mutations.The
(admin)route group doesn't appear in the public URL.revalidatePath()expects the pathname users see (/dashboard/boards/projects), not/admin/dashboard/boards/projects. This affects project updates, deletions, and feature toggles.Affected locations
- revalidatePath('/admin/dashboard/boards/projects') + revalidatePath('/dashboard/boards/projects')Applies to lines 232, 273, and 310.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/admin/projects.ts` around lines 184 - 234, Update the revalidation paths used after project mutations: locate the revalidatePath(...) calls in this file (e.g., in the update handler that calls revalidatePath('/project/[slug]') and revalidatePath('/admin/dashboard/boards/projects')) and change the admin-prefixed path to the public path users see — replace '/admin/dashboard/boards/projects' with '/dashboard/boards/projects' (leave '/project/[slug]' as-is); make the same change for the other project mutation handlers (deletion/feature-toggle) that call revalidatePath so all use the public '/dashboard/boards/projects' pathname.lib/actions/admin/users.ts (1)
136-140:⚠️ Potential issue | 🟠 MajorPerformance concern:
listUsers()fetches all users without pagination.
adminSupabase.auth.admin.listUsers()retrieves the entire user list from Supabase Auth on every call togetAllUsers. For large user bases, this causes:
- Excessive memory usage
- Slow response times
- Potential timeouts
Consider paginating or caching the email lookup, or fetching only the emails for the current page's user IDs.
♻️ Suggested approach: Fetch emails only for current page users
- // Get email from auth.users - const { data: authUsers } = await adminSupabase.auth.admin.listUsers() - const emailMap: Record<string, string> = {} - authUsers?.users?.forEach((authUser) => { - emailMap[authUser.id] = authUser.email || '' - }) + // Get emails only for users on this page + const emailMap: Record<string, string> = {} + await Promise.all( + userIds.map(async (id) => { + const { data } = await adminSupabase.auth.admin.getUserById(id) + if (data?.user?.email) { + emailMap[id] = data.user.email + } + }) + )Alternatively, if email lookups are frequent, consider denormalizing email into the
public.userstable during registration/profile sync.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/actions/admin/users.ts` around lines 136 - 140, The current getAllUsers flow calls adminSupabase.auth.admin.listUsers() and builds emailMap by iterating all auth users which paginates poorly; change the implementation to only fetch emails for the requested page's user IDs: in getAllUsers, collect the page's public.users ids, then call adminSupabase.auth.admin.listUsers({ user_ids: [...] }) or use the paginated listUsers API to fetch only those IDs (or batch with pagination) and populate emailMap for those ids instead of calling listUsers() without filters; update uses of emailMap and any helper functions that assume full-list semantics accordingly (refer to adminSupabase.auth.admin.listUsers(), getAllUsers, and emailMap).app/(admin)/dashboard/boards/comments/components/comment-preview.tsx (1)
25-26: 🧹 Nitpick | 🔵 TrivialConsider
router.refresh()instead of full page reload.
window.location.reload()causes a full page reload, losing client state. UsinguseRouter().refresh()fromnext/navigationwould re-fetch server components while preserving client state and providing a smoother UX.♻️ Suggested improvement
+'use client' + +import { useRouter } from 'next/navigation' // ... in component: +const router = useRouter() // ... in handleAction: - window.location.reload() + router.refresh()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(admin)/dashboard/boards/comments/components/comment-preview.tsx around lines 25 - 26, Replace the full page reload with Next's router refresh: import and call useRouter() from next/navigation in the component, get router via const router = useRouter(), keep the existing onOpenChange(false) call, and replace window.location.reload() with router.refresh() (call router.refresh() after closing via onOpenChange in the event handler in comment-preview.tsx).app/(admin)/dashboard/boards/events-approval/components/pending-events-table.tsx (1)
24-45:⚠️ Potential issue | 🟡 MinorRole checks are properly enforced, but add Zod validation for the
eventIdparameter.Both
approveEventandrejectEventcorrectly callrequireAdminAccess()server-side, which validatescanManageAdminDashboardbefore allowing the mutation. However, theeventIdparameter uses only a UUID format check; per guidelines, validate external input with Zod before writes in server actions. Replace theisValidUUID()check with a Zod schema (e.g.,z.string().uuid()).Revalidation is correctly implemented with
revalidatePath()andrevalidateTag()calls.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/`(admin)/dashboard/boards/events-approval/components/pending-events-table.tsx around lines 24 - 45, The server actions approveEvent and rejectEvent still use isValidUUID() for eventId validation; replace that check with a Zod validation (e.g., use z.string().uuid()) at the start of each server action to validate incoming eventId before any writes and before calling requireAdminAccess(), throwing a controlled error on failure; update the functions approveEvent and rejectEvent to parse/validate eventId via a Zod schema (z.string().uuid()) and use the parsed value for subsequent logic (including calls to requireAdminAccess(), revalidatePath(), revalidateTag(), etc.).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/`(admin)/dashboard/boards/projects/page.tsx:
- Around line 14-15: Awaiting getCurrentRoleAccess() serially increases latency;
run independent fetches concurrently. Start getCurrentRoleAccess() and the
project/category queries without awaiting, then await them together (e.g., via
Promise.all) and destructure results; update any subsequent uses of
params/searchParams, getCurrentRoleAccess, and the project/category fetch
functions so they consume the resolved values. Target symbols:
getCurrentRoleAccess, searchParams (params), and the project/category query
functions called around lines 25-28.
In `@app/blog/posts/page.tsx`:
- Around line 20-22: Remove the redundant hardcoded prop isLoggedIn={true} from
the Navbar invocation and let Navbar derive login state from the provided user
prop (or make Navbar accept a boolean computed where it's used); update the call
site that renders Navbar (the JSX with Navbar and prop user) to omit isLoggedIn,
and if necessary adjust Navbar's props/interface (prop types/tsx signature) to
treat user as the single source-of-truth for authentication state (e.g., compute
isLoggedIn = Boolean(user) inside Navbar or its parent).
In `@lib/actions/blog.ts`:
- Around line 94-97: The deletion path isn't revalidated for the posts
list—update deleteBlogPost to call revalidatePath('/blog/posts') after
successful deletion (in addition to revalidatePath('/blog') and
revalidateTag('blog-list-posts','max')) so the removed row disappears; also scan
other delete-related server actions referenced (the functions around the other
ranges) and add the same revalidatePath('/blog/posts') call where appropriate to
mirror createBlogPost()/updateBlogPost() behavior.
In `@lib/server/role-access.ts`:
- Around line 50-69: The getCurrentRoleAccess function is currently wrapped with
React's cache() which can leak memoized role data across server action POST
requests; replace usages to call loadCurrentRoleAccess() directly (remove the
cache wrapper around getCurrentRoleAccess) so that requireElevatedAccess and
requireAdminAccess call loadCurrentRoleAccess() per invocation, or implement a
per-request cache mechanism that explicitly supports server actions instead of
React cache(). Ensure both requireElevatedAccess and requireAdminAccess use the
non-cached loadCurrentRoleAccess() to obtain fresh role checks for each server
action call.
In `@scripts/21_protect_users_privileged_fields.sql`:
- Around line 44-58: Consolidate the three separate suspension checks into one
conditional: check if any of next_row -> 'is_suspended', next_row ->>
'suspension_reason', or next_row ->> 'suspended_at' are DISTINCT FROM their
respective previous_row values and, if so, RAISE EXCEPTION 'Suspension changes
require admin access'; replace the three IF blocks that reference is_suspended,
suspension_reason, and suspended_at with this single check to treat suspension
fields identically.
---
Outside diff comments:
In `@app/`(admin)/dashboard/boards/comments/components/comment-preview.tsx:
- Around line 25-26: Replace the full page reload with Next's router refresh:
import and call useRouter() from next/navigation in the component, get router
via const router = useRouter(), keep the existing onOpenChange(false) call, and
replace window.location.reload() with router.refresh() (call router.refresh()
after closing via onOpenChange in the event handler in comment-preview.tsx).
In
`@app/`(admin)/dashboard/boards/events-approval/components/pending-events-table.tsx:
- Around line 24-45: The server actions approveEvent and rejectEvent still use
isValidUUID() for eventId validation; replace that check with a Zod validation
(e.g., use z.string().uuid()) at the start of each server action to validate
incoming eventId before any writes and before calling requireAdminAccess(),
throwing a controlled error on failure; update the functions approveEvent and
rejectEvent to parse/validate eventId via a Zod schema (z.string().uuid()) and
use the parsed value for subsequent logic (including calls to
requireAdminAccess(), revalidatePath(), revalidateTag(), etc.).
In `@lib/actions/admin/comments.ts`:
- Around line 225-258: The revalidation paths use the incorrect route group
prefix; update all revalidatePath calls in this file (e.g., in the comment
delete/dismiss/action handlers such as the function around the supabase
delete/select logic and the other blocks flagged at lines 272–290 and 307–355)
to remove the "(admin)" segment—replace '/admin/dashboard/boards/comments' with
'/dashboard/boards/comments' (and any other occurrences that include
'/admin/dashboard/...') so revalidatePath('/dashboard/boards/comments') is used
consistently after comment create/delete/dismiss actions.
In `@lib/actions/admin/posts.ts`:
- Around line 177-220: The three revalidatePath calls in the admin post mutation
handlers are using the wrong admin-only path; update
revalidatePath('/admin/dashboard/boards/blog') to
revalidatePath('/dashboard/boards/blog') in all relevant functions—specifically
in updateAdminPost, deleteAdminPost, and toggleAdminPostFeatured—so the public
board page is properly revalidated after post updates, deletions, or feature
toggles.
In `@lib/actions/admin/projects.ts`:
- Around line 184-234: Update the revalidation paths used after project
mutations: locate the revalidatePath(...) calls in this file (e.g., in the
update handler that calls revalidatePath('/project/[slug]') and
revalidatePath('/admin/dashboard/boards/projects')) and change the
admin-prefixed path to the public path users see — replace
'/admin/dashboard/boards/projects' with '/dashboard/boards/projects' (leave
'/project/[slug]' as-is); make the same change for the other project mutation
handlers (deletion/feature-toggle) that call revalidatePath so all use the
public '/dashboard/boards/projects' pathname.
In `@lib/actions/admin/users.ts`:
- Around line 136-140: The current getAllUsers flow calls
adminSupabase.auth.admin.listUsers() and builds emailMap by iterating all auth
users which paginates poorly; change the implementation to only fetch emails for
the requested page's user IDs: in getAllUsers, collect the page's public.users
ids, then call adminSupabase.auth.admin.listUsers({ user_ids: [...] }) or use
the paginated listUsers API to fetch only those IDs (or batch with pagination)
and populate emailMap for those ids instead of calling listUsers() without
filters; update uses of emailMap and any helper functions that assume full-list
semantics accordingly (refer to adminSupabase.auth.admin.listUsers(),
getAllUsers, and emailMap).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e6d07667-d753-4bb1-93d7-cb00bf2e685c
📒 Files selected for processing (40)
app/(admin)/dashboard/boards/blog/components/post-actions.tsxapp/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsxapp/(admin)/dashboard/boards/blog/components/posts-table.tsxapp/(admin)/dashboard/boards/blog/components/tags-manager.tsxapp/(admin)/dashboard/boards/blog/page.tsxapp/(admin)/dashboard/boards/comments/components/comment-preview.tsxapp/(admin)/dashboard/boards/comments/components/report-actions.tsxapp/(admin)/dashboard/boards/comments/components/reports-table.tsxapp/(admin)/dashboard/boards/comments/page.tsxapp/(admin)/dashboard/boards/events-approval/components/pending-events-table.tsxapp/(admin)/dashboard/boards/events-approval/page.tsxapp/(admin)/dashboard/boards/projects/components/project-actions.tsxapp/(admin)/dashboard/boards/projects/components/project-edit-dialog.tsxapp/(admin)/dashboard/boards/projects/components/projects-table.tsxapp/(admin)/dashboard/boards/projects/page.tsxapp/(admin)/dashboard/boards/users/components/user-actions.tsxapp/(admin)/dashboard/boards/users/components/user-role-dialog.tsxapp/(admin)/dashboard/boards/users/components/user-suspend-dialog.tsxapp/(admin)/dashboard/boards/users/components/users-table.tsxapp/(admin)/dashboard/boards/users/page.tsxapp/(admin)/layout-client.tsxapp/(admin)/layout.tsxapp/blog/blog-page-client.tsxapp/blog/editor/[slug]/page.tsxapp/blog/editor/blog-editor-client.tsxapp/blog/posts/page.tsxapp/blog/posts/post-dashboard-client.tsxapp/dashboard/layout.tsxapp/dashboard/posts/page.tsxcomponents/admin-panel/app-sidebar.tsxcomponents/ui/navbar.tsxlib/actions/admin/comments.tslib/actions/admin/posts.tslib/actions/admin/projects.tslib/actions/admin/users.tslib/actions/analytics.tslib/actions/blog.tslib/actions/events.tslib/server/role-access.tsscripts/21_protect_users_privileged_fields.sql
💤 Files with no reviewable changes (2)
- app/dashboard/posts/page.tsx
- app/dashboard/layout.tsx
| const params = await searchParams | ||
| const access = await getCurrentRoleAccess() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Run access and data fetches concurrently to reduce page latency.
getCurrentRoleAccess() is independent of project/category queries; awaiting it first adds an avoidable round trip.
⚡ Suggested refactor
export default async function ProjectsPage({ searchParams }: { searchParams: Promise<SearchParams> }) {
const params = await searchParams
- const access = await getCurrentRoleAccess()
const filters = {
status: params.status as 'all' | 'featured' | 'regular' | undefined,
category: params.category,
search: params.search,
}
const page = params.page ? parseInt(params.page, 10) : 1
- const [{ projects, totalCount, error }, { categories }] = await Promise.all([
+ const [access, { projects, totalCount, error }, { categories }] = await Promise.all([
+ getCurrentRoleAccess(),
getAllProjects(filters, page, 20),
getProjectCategories(),
])Also applies to: 25-28
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/`(admin)/dashboard/boards/projects/page.tsx around lines 14 - 15,
Awaiting getCurrentRoleAccess() serially increases latency; run independent
fetches concurrently. Start getCurrentRoleAccess() and the project/category
queries without awaiting, then await them together (e.g., via Promise.all) and
destructure results; update any subsequent uses of params/searchParams,
getCurrentRoleAccess, and the project/category fetch functions so they consume
the resolved values. Target symbols: getCurrentRoleAccess, searchParams
(params), and the project/category query functions called around lines 25-28.
| <Navbar | ||
| isLoggedIn={true} | ||
| user={user} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Minor: isLoggedIn={true} is redundant after auth check.
Since lines 14-16 guarantee user exists at this point, isLoggedIn={true} is always true. Consider deriving this from the presence of user within Navbar or simply removing it if the component can infer login state from the user prop.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/blog/posts/page.tsx` around lines 20 - 22, Remove the redundant hardcoded
prop isLoggedIn={true} from the Navbar invocation and let Navbar derive login
state from the provided user prop (or make Navbar accept a boolean computed
where it's used); update the call site that renders Navbar (the JSX with Navbar
and prop user) to omit isLoggedIn, and if necessary adjust Navbar's
props/interface (prop types/tsx signature) to treat user as the single
source-of-truth for authentication state (e.g., compute isLoggedIn =
Boolean(user) inside Navbar or its parent).
| revalidatePath('/blog') | ||
| revalidatePath('/dashboard/posts') | ||
| revalidatePath('/blog/posts') | ||
| revalidateTag('blog-list-posts', 'max') | ||
| return { success: true, slug } |
There was a problem hiding this comment.
Also invalidate /blog/posts after deletions.
The route move was handled in createBlogPost() and updateBlogPost(), but deleteBlogPost() still only invalidates /blog and the tag cache. Deleting from /blog/posts will leave the removed row visible until a manual refresh.
♻️ Proposed fix
revalidatePath('/blog')
+ revalidatePath('/blog/posts')
revalidateTag('blog-list-posts', 'max')
return { success: true }Also applies to: 164-168, 272-305
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/actions/blog.ts` around lines 94 - 97, The deletion path isn't
revalidated for the posts list—update deleteBlogPost to call
revalidatePath('/blog/posts') after successful deletion (in addition to
revalidatePath('/blog') and revalidateTag('blog-list-posts','max')) so the
removed row disappears; also scan other delete-related server actions referenced
(the functions around the other ranges) and add the same
revalidatePath('/blog/posts') call where appropriate to mirror
createBlogPost()/updateBlogPost() behavior.
| export const getCurrentRoleAccess = cache(loadCurrentRoleAccess) | ||
|
|
||
| export async function requireElevatedAccess() { | ||
| const access = await getCurrentRoleAccess() | ||
|
|
||
| if (!access?.canAccessAdminDashboard) { | ||
| throw new Error('Elevated access required') | ||
| } | ||
|
|
||
| return access | ||
| } | ||
|
|
||
| export async function requireAdminAccess() { | ||
| const access = await getCurrentRoleAccess() | ||
|
|
||
| if (!access?.canManageAdminDashboard) { | ||
| throw new Error('Admin access required') | ||
| } | ||
|
|
||
| return access |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In Next.js 15 / React 19, is React cache() request-scoped and supported when called from server actions, or is it only guaranteed during Server Component rendering for per-request auth helpers?
💡 Result:
React cache() is request-scoped, but in Next.js 15 / React 19 it’s only documented/guaranteed for Server Component rendering (i.e., to share/memoize work across a React Server Component tree during a single render/request). React’s own docs currently say cache “should only be used in Server Components” and that it’s invalidated across server requests. (react.dev)
In Next.js, the per-request “request memoization” model is likewise described as applying only to the React Component tree during rendering (and not to contexts that aren’t part of that tree). (nextjs.org)
Server Actions are invoked via a separate POST request. (nextjs.org) Because they’re not part of the Server Component render pass, you should not assume cache() will provide shared per-request memoization between RSC rendering and Server Actions (at best, it would only memoize within that action invocation’s request, and that usage isn’t what React/Next.js currently guarantee in docs). (react.dev)
Citations:
- 1: https://react.dev/reference/react/cache
- 2: https://nextjs.org/docs/13/app/building-your-application/caching
- 3: https://nextjs.org/docs/13/app/building-your-application/data-fetching/server-actions-and-mutations?utm_source=openai
- 4: https://react.dev/reference/react/cache
Remove cache() wrapping from getCurrentRoleAccess()—it is not safe for server action auth checks.
React cache() is only guaranteed to provide request-scoped memoization during Server Component rendering. Server actions are invoked via separate POST requests outside the render pass, so cache() does not reliably isolate role lookups per action invocation. This creates a privilege boundary risk: one user's role access could be memoized and incorrectly reused for another user's server action.
Instead, directly call loadCurrentRoleAccess() (without caching) in server action paths, or use Next.js's built-in per-request cache mechanisms that explicitly support server actions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@lib/server/role-access.ts` around lines 50 - 69, The getCurrentRoleAccess
function is currently wrapped with React's cache() which can leak memoized role
data across server action POST requests; replace usages to call
loadCurrentRoleAccess() directly (remove the cache wrapper around
getCurrentRoleAccess) so that requireElevatedAccess and requireAdminAccess call
loadCurrentRoleAccess() per invocation, or implement a per-request cache
mechanism that explicitly supports server actions instead of React cache().
Ensure both requireElevatedAccess and requireAdminAccess use the non-cached
loadCurrentRoleAccess() to obtain fresh role checks for each server action call.
| IF next_row -> 'role' IS DISTINCT FROM previous_row -> 'role' THEN | ||
| RAISE EXCEPTION 'Role changes require admin access'; | ||
| END IF; | ||
|
|
||
| IF next_row -> 'is_suspended' IS DISTINCT FROM previous_row -> 'is_suspended' THEN | ||
| RAISE EXCEPTION 'Suspension changes require admin access'; | ||
| END IF; | ||
|
|
||
| IF next_row ->> 'suspension_reason' IS DISTINCT FROM previous_row ->> 'suspension_reason' THEN | ||
| RAISE EXCEPTION 'Suspension metadata requires admin access'; | ||
| END IF; | ||
|
|
||
| IF next_row ->> 'suspended_at' IS DISTINCT FROM previous_row ->> 'suspended_at' THEN | ||
| RAISE EXCEPTION 'Suspension metadata requires admin access'; | ||
| END IF; |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider consolidating suspension field checks.
Lines 48-58 check is_suspended, suspension_reason, and suspended_at separately, each raising an exception with a slightly different message. While functionally correct, this could be simplified into a single check if all three fields should be treated identically.
♻️ Optional: Consolidate suspension field checks
- IF next_row -> 'is_suspended' IS DISTINCT FROM previous_row -> 'is_suspended' THEN
- RAISE EXCEPTION 'Suspension changes require admin access';
- END IF;
-
- IF next_row ->> 'suspension_reason' IS DISTINCT FROM previous_row ->> 'suspension_reason' THEN
- RAISE EXCEPTION 'Suspension metadata requires admin access';
- END IF;
-
- IF next_row ->> 'suspended_at' IS DISTINCT FROM previous_row ->> 'suspended_at' THEN
- RAISE EXCEPTION 'Suspension metadata requires admin access';
- END IF;
+ IF (next_row -> 'is_suspended' IS DISTINCT FROM previous_row -> 'is_suspended')
+ OR (next_row ->> 'suspension_reason' IS DISTINCT FROM previous_row ->> 'suspension_reason')
+ OR (next_row ->> 'suspended_at' IS DISTINCT FROM previous_row ->> 'suspended_at')
+ THEN
+ RAISE EXCEPTION 'Suspension changes require admin access';
+ END IF;📝 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.
| IF next_row -> 'role' IS DISTINCT FROM previous_row -> 'role' THEN | |
| RAISE EXCEPTION 'Role changes require admin access'; | |
| END IF; | |
| IF next_row -> 'is_suspended' IS DISTINCT FROM previous_row -> 'is_suspended' THEN | |
| RAISE EXCEPTION 'Suspension changes require admin access'; | |
| END IF; | |
| IF next_row ->> 'suspension_reason' IS DISTINCT FROM previous_row ->> 'suspension_reason' THEN | |
| RAISE EXCEPTION 'Suspension metadata requires admin access'; | |
| END IF; | |
| IF next_row ->> 'suspended_at' IS DISTINCT FROM previous_row ->> 'suspended_at' THEN | |
| RAISE EXCEPTION 'Suspension metadata requires admin access'; | |
| END IF; | |
| IF next_row -> 'role' IS DISTINCT FROM previous_row -> 'role' THEN | |
| RAISE EXCEPTION 'Role changes require admin access'; | |
| END IF; | |
| IF (next_row -> 'is_suspended' IS DISTINCT FROM previous_row -> 'is_suspended') | |
| OR (next_row ->> 'suspension_reason' IS DISTINCT FROM previous_row ->> 'suspension_reason') | |
| OR (next_row ->> 'suspended_at' IS DISTINCT FROM previous_row ->> 'suspended_at') | |
| THEN | |
| RAISE EXCEPTION 'Suspension changes require admin access'; | |
| END IF; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/21_protect_users_privileged_fields.sql` around lines 44 - 58,
Consolidate the three separate suspension checks into one conditional: check if
any of next_row -> 'is_suspended', next_row ->> 'suspension_reason', or next_row
->> 'suspended_at' are DISTINCT FROM their respective previous_row values and,
if so, RAISE EXCEPTION 'Suspension changes require admin access'; replace the
three IF blocks that reference is_suspended, suspension_reason, and suspended_at
with this single check to treat suspension fields identically.
Summary
/dashboardso only admins and moderators can access it, with moderators limited to read-only actions via centralized database-backed role checks/dashboard/poststo/blog/postsand update app links so all/dashboard/*routes remain elevated-onlyusersfields while routing admin user mutations through the service-role clientSummary by CodeRabbit
Release Notes
New Features
Improvements
Changes
/blog/posts; improved error handling consistency throughout admin interfaces.