Skip to content

fix: secure dashboard role access - #20

Open
julianromli wants to merge 1 commit into
mainfrom
fix/admin-dashboard-role-access
Open

fix: secure dashboard role access#20
julianromli wants to merge 1 commit into
mainfrom
fix/admin-dashboard-role-access

Conversation

@julianromli

@julianromli julianromli commented Apr 6, 2026

Copy link
Copy Markdown
Owner

Summary

  • secure /dashboard so only admins and moderators can access it, with moderators limited to read-only actions via centralized database-backed role checks
  • move the user post management page from /dashboard/posts to /blog/posts and update app links so all /dashboard/* routes remain elevated-only
  • add a Postgres trigger migration to block self-escalation of privileged users fields while routing admin user mutations through the service-role client

Summary by CodeRabbit

Release Notes

  • New Features

    • Role-based access control for admin dashboards with read-only mode for non-managers; admin controls now conditionally appear based on user permissions.
  • Improvements

    • Enhanced authorization system across all admin sections (posts, projects, comments, events, users); read-only notice displays when viewing dashboards without management access.
  • Changes

    • Blog dashboard moved to /blog/posts; improved error handling consistency throughout admin interfaces.

Restrict the dashboard to elevated roles, move author post management out of /dashboard, and enforce admin-only mutations with database-backed role checks.
@vercel

vercel Bot commented Apr 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
vibedevid Ready Ready Preview, Comment, Open in v0 Apr 6, 2026 6:16am

@coderabbitai

coderabbitai Bot commented Apr 6, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This 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 /dashboard/posts to /blog/posts.

Changes

Cohort / File(s) Summary
Role Access Module
lib/server/role-access.ts
New centralized server module that defines ROLES constants, CurrentRoleAccess interface, and exports getCurrentRoleAccess() (cached), requireAdminAccess(), and requireElevatedAccess() for standardized authorization checks.
Admin Dashboard Pages
app/(admin)/dashboard/boards/blog/page.tsx, app/(admin)/dashboard/boards/comments/page.tsx, app/(admin)/dashboard/boards/events-approval/page.tsx, app/(admin)/dashboard/boards/projects/page.tsx, app/(admin)/dashboard/boards/users/page.tsx
Each page now calls getCurrentRoleAccess() to fetch user permissions and passes derived canManage prop to child tables based on access?.canManageAdminDashboard.
Blog Components with Access Gating
app/(admin)/dashboard/boards/blog/components/post-actions.tsx, app/(admin)/dashboard/boards/blog/components/posts-table.tsx, app/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsx, app/(admin)/dashboard/boards/blog/components/tags-manager.tsx
Components now accept canManage prop and conditionally render edit/delete actions and dialogs only when management is permitted; improved error handling with unused error variable renaming.
Comment Moderation Components with Access Gating
app/(admin)/dashboard/boards/comments/components/comment-preview.tsx, app/(admin)/dashboard/boards/comments/components/report-actions.tsx, app/(admin)/dashboard/boards/comments/components/reports-table.tsx
Components now accept and propagate canManage prop; action buttons only render when both canManage is true and report status is pending.
Event Approval Components with Access Gating
app/(admin)/dashboard/boards/events-approval/components/pending-events-table.tsx
Table now conditionally includes an "actions" column based on canManage; displays read-only message when management is disabled.
Project Components with Access Gating
app/(admin)/dashboard/boards/projects/components/project-actions.tsx, app/(admin)/dashboard/boards/projects/components/project-edit-dialog.tsx, app/(admin)/dashboard/boards/projects/components/projects-table.tsx
Components now accept canManage and gate admin-only actions; edit dialogs render only when both conditions are met.
User Management Components with Access Gating
app/(admin)/dashboard/boards/users/components/user-actions.tsx, app/(admin)/dashboard/boards/users/components/user-role-dialog.tsx, app/(admin)/dashboard/boards/users/components/user-suspend-dialog.tsx, app/(admin)/dashboard/boards/users/components/users-table.tsx
Components now accept canManage and gate role/suspension management UI; added explicit button type="button" attribute; improved parseInt with explicit radix.
Admin Layout and Sidebar
app/(admin)/layout.tsx, app/(admin)/layout-client.tsx, components/admin-panel/app-sidebar.tsx
Layout now fetches role access, passes isReadOnly to client components, and sidebar conditionally displays "Moderator Read-only" or "Admin Dashboard" based on management capability; read-only banner displayed when needed.
Navigation Updates
components/ui/navbar.tsx
Updated "My Posts" links from /dashboard/posts to /blog/posts across desktop and mobile navigation.
Blog Route Migration
app/blog/posts/page.tsx, app/blog/posts/post-dashboard-client.tsx, app/blog/editor/[slug]/page.tsx, app/blog/editor/blog-editor-client.tsx, app/blog/blog-page-client.tsx
New authenticated blog posts dashboard at /blog/posts; moved user-facing blog management out of admin dashboard; removed scroll-to-section helper; updated redirect paths and tab filtering with proper types.
Removed Dashboard Routes
app/dashboard/layout.tsx, app/dashboard/posts/page.tsx
Removed old dashboard layout and posts page; functionality migrated to blog routes.
Admin Action Modules Refactored
lib/actions/admin/comments.ts, lib/actions/admin/posts.ts, lib/actions/admin/projects.ts, lib/actions/admin/users.ts
Replaced local checkAdminAccess() implementations with calls to centralized requireAdminAccess() and requireElevatedAccess(); removed duplicate ROLES constants; maintained function signatures.
Analytics and Events Modules Refactored
lib/actions/analytics.ts, lib/actions/events.ts
Removed local authorization helpers and replaced with centralized requireElevatedAccess() and requireAdminAccess() calls.
Blog Actions Updated
lib/actions/blog.ts
Changed cache invalidation paths from /dashboard/posts to /blog/posts; updated deleteBlogPost authorization to use getCurrentRoleAccess() instead of auth metadata.
Database Trigger
scripts/21_protect_users_privileged_fields.sql
Added PostgreSQL trigger to prevent authenticated users from modifying privileged fields (role, suspension status) except through admin operations; enforces field immutability at database level.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • PR #8: Modifies authorization flow and lib/actions/events.ts for event moderation; related through shared refactoring of admin access control patterns.
  • PR #9: Updates blog post caching with revalidateTag in lib/actions/blog.ts; related through blog post management cache invalidation changes.

Poem

🐰 Roles and access, now neatly arranged,
Admins and mods, permissions exchanged,
From dashboard to blog, the paths realigned,
With canManage flags and gating refined,
The rabbit hops through this auth redesign! 🌟

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 8.82% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'fix: secure dashboard role access' directly addresses the main objective of securing dashboard access with role-based controls and is clearly specific to the primary change of implementing centralized role access management.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/admin-dashboard-role-access

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@cubic-dev-ai cubic-dev-ai Bot Apr 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with Cubic

>
{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.

@cubic-dev-ai cubic-dev-ai Bot Apr 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Fix with Cubic

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 })}

@cubic-dev-ai cubic-dev-ai Bot Apr 6, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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>
Suggested change
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) })}
Fix with Cubic

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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".

Comment thread app/(admin)/layout.tsx
Comment on lines +15 to +16
if (error || !user || !access) {
redirect('/user/auth?redirectTo=/dashboard')

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge 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 👍 / 👎.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 | 🟡 Minor

Fix 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') to revalidatePath('/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 | 🟡 Minor

Use /dashboard/boards/comments instead of /admin/dashboard/boards/comments for revalidatePath().

Next.js route groups like (admin) are not part of the URL path. For app/(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 | 🟡 Minor

Fix 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 | 🟠 Major

Performance concern: listUsers() fetches all users without pagination.

adminSupabase.auth.admin.listUsers() retrieves the entire user list from Supabase Auth on every call to getAllUsers. 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.users table 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 | 🔵 Trivial

Consider router.refresh() instead of full page reload.

window.location.reload() causes a full page reload, losing client state. Using useRouter().refresh() from next/navigation would 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 | 🟡 Minor

Role checks are properly enforced, but add Zod validation for the eventId parameter.

Both approveEvent and rejectEvent correctly call requireAdminAccess() server-side, which validates canManageAdminDashboard before allowing the mutation. However, the eventId parameter uses only a UUID format check; per guidelines, validate external input with Zod before writes in server actions. Replace the isValidUUID() check with a Zod schema (e.g., z.string().uuid()).

Revalidation is correctly implemented with revalidatePath() and revalidateTag() 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

📥 Commits

Reviewing files that changed from the base of the PR and between 896ed68 and e50e4d6.

📒 Files selected for processing (40)
  • app/(admin)/dashboard/boards/blog/components/post-actions.tsx
  • app/(admin)/dashboard/boards/blog/components/post-edit-dialog.tsx
  • app/(admin)/dashboard/boards/blog/components/posts-table.tsx
  • app/(admin)/dashboard/boards/blog/components/tags-manager.tsx
  • app/(admin)/dashboard/boards/blog/page.tsx
  • app/(admin)/dashboard/boards/comments/components/comment-preview.tsx
  • app/(admin)/dashboard/boards/comments/components/report-actions.tsx
  • app/(admin)/dashboard/boards/comments/components/reports-table.tsx
  • app/(admin)/dashboard/boards/comments/page.tsx
  • app/(admin)/dashboard/boards/events-approval/components/pending-events-table.tsx
  • app/(admin)/dashboard/boards/events-approval/page.tsx
  • app/(admin)/dashboard/boards/projects/components/project-actions.tsx
  • app/(admin)/dashboard/boards/projects/components/project-edit-dialog.tsx
  • app/(admin)/dashboard/boards/projects/components/projects-table.tsx
  • app/(admin)/dashboard/boards/projects/page.tsx
  • app/(admin)/dashboard/boards/users/components/user-actions.tsx
  • app/(admin)/dashboard/boards/users/components/user-role-dialog.tsx
  • app/(admin)/dashboard/boards/users/components/user-suspend-dialog.tsx
  • app/(admin)/dashboard/boards/users/components/users-table.tsx
  • app/(admin)/dashboard/boards/users/page.tsx
  • app/(admin)/layout-client.tsx
  • app/(admin)/layout.tsx
  • app/blog/blog-page-client.tsx
  • app/blog/editor/[slug]/page.tsx
  • app/blog/editor/blog-editor-client.tsx
  • app/blog/posts/page.tsx
  • app/blog/posts/post-dashboard-client.tsx
  • app/dashboard/layout.tsx
  • app/dashboard/posts/page.tsx
  • components/admin-panel/app-sidebar.tsx
  • components/ui/navbar.tsx
  • lib/actions/admin/comments.ts
  • lib/actions/admin/posts.ts
  • lib/actions/admin/projects.ts
  • lib/actions/admin/users.ts
  • lib/actions/analytics.ts
  • lib/actions/blog.ts
  • lib/actions/events.ts
  • lib/server/role-access.ts
  • scripts/21_protect_users_privileged_fields.sql
💤 Files with no reviewable changes (2)
  • app/dashboard/posts/page.tsx
  • app/dashboard/layout.tsx

Comment on lines 14 to +15
const params = await searchParams
const access = await getCurrentRoleAccess()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Comment thread app/blog/posts/page.tsx
Comment on lines +20 to +22
<Navbar
isLoggedIn={true}
user={user}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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).

Comment thread lib/actions/blog.ts
Comment on lines 94 to 97
revalidatePath('/blog')
revalidatePath('/dashboard/posts')
revalidatePath('/blog/posts')
revalidateTag('blog-list-posts', 'max')
return { success: true, slug }

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

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 }
As per coding guidelines: Revalidate affected routes or tags after successful mutations in server actions.

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.

Comment thread lib/server/role-access.ts
Comment on lines +50 to +69
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 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:


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.

Comment on lines +44 to +58
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;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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.

Suggested change
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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant