Claude/admin dashboard security 011 cu qb nm yr6p5odtdc4 xm5 l #4
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit implements a multi-layered security system for the admin dashboard and API endpoints, addressing critical vulnerabilities where admin functionality was publicly accessible. ## Security Implementation ### 1. Database Layer - Added `role` column to users table (user, admin, super_admin) - Created `admin_audit_logs` table for tracking admin actions - Implemented Row Level Security (RLS) policies for admin data - Added PostgreSQL helper functions (is_admin, is_super_admin, get_user_role) - Created comprehensive migration file ### 2. Backend Authentication & Authorization - Created lib/auth/admin-auth.ts with enterprise-grade admin utilities - Implemented `requireAdmin()` wrapper for route protection - Added `logAdminAction()` for audit trail compliance - Developed role-based access control (RBAC) functions - Type-safe admin user interfaces with proper TypeScript strict mode ### 3. Frontend Security - Created `AdminGuard` component for page-level protection - Implemented `useAdminAuth` hook for client-side role checking - Protected both admin dashboard pages: - /app/admin/dashboard/page.tsx - /app/admin/api-keys/page.tsx - Automatic redirect for unauthorized users - Professional loading and access denied states ### 4. API Route Security (4 Critical Routes Secured) - /api/admin/users - Now requires admin auth + audit logging - /api/admin/activity - Now requires admin auth + audit logging - /api/admin/stats - Now requires admin auth + audit logging - /api/admin/analytics - Replaced weak API key auth with role-based auth ### 5. Middleware Enhancement - Removed admin route exclusion from middleware config - Admin routes now require authentication at middleware level - Defense-in-depth: auth check (middleware) + role check (requireAdmin) ## Security Features Implemented - Multi-layer defense: Database RLS + Middleware + Route handlers - Comprehensive audit logging with IP address and user agent tracking - Session-based authentication (not just API keys) - Professional error messages (user-friendly, no technical jargon) - Type-safe implementation with TypeScript strict mode - Zero tolerance: No console.log, no emojis, full JSDoc comments ## What's Next (Phase 2) Remaining work to complete admin security: 1. Secure 14 additional admin API routes with requireAdmin() 2. Apply database migration to add roles 3. Manually set first super_admin user in database 4. Run comprehensive pre-commit checks 5. Test all admin functionality with proper credentials ## Setup Instructions After deploying this code: 1. Apply the database migration: ```bash supabase db push ``` 2. Set your first super admin user (replace with your user ID): ```sql UPDATE users SET role = 'super_admin' WHERE email = 'your-email@example.com'; ``` 3. Test admin dashboard access at /admin/dashboard Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Secured 5 additional critical admin routes with role-based authentication and comprehensive audit logging. ## Routes Secured in This Commit ### 1. /api/admin/api-keys (GET, POST, DELETE) - Replaced weak/missing auth with requireAdmin() wrapper - Added audit logging for view, add, and delete operations - Professional JSDoc documentation - Proper TypeScript error handling ### 2. /api/admin/api-keys/control (POST) - Critical service control endpoint (start/stop scraper & validator) - Replaced API key auth with role-based auth - Audit logging for all service control actions - Comprehensive documentation of action types ### 3. /api/admin/scraper/start (POST, GET, DELETE) - Background scraper control endpoints - All three HTTP methods secured with requireAdmin() - Audit logging for start, stop, and status check operations - Proper error handling with TypeScript strict mode ### 4. /api/admin/validator/start (POST, GET, DELETE) - Continuous validator control endpoints - Secured all HTTP methods with role-based auth - Complete audit trail for validator operations - Professional documentation ### 5. /api/admin/bulk-scrape (POST, GET) - Long-running bulk scrape operation endpoint - Critical endpoint secured (5-minute max duration) - Audit logging with detailed operation statistics - Proper error handling for long-running operations ## Security Improvements - Removed weak API key authentication from all routes - Implemented requireAdmin() wrapper for role verification - Added comprehensive audit logging with IP and user agent tracking - Professional error messages (user-friendly, no technical jargon) - TypeScript strict mode compliance ## Current Progress - Phase 1: 4 routes secured (users, activity, stats, analytics) - Phase 2a: 5 routes secured (this commit) - **Total: 9/19 admin routes secured** - Remaining: 10 routes to secure Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
Created detailed setup guide covering: - Step-by-step migration instructions - Admin user management SQL commands - Audit logging queries - API integration examples - Security best practices - Troubleshooting common issues This guide provides everything needed to configure and manage the new role-based admin security system. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Move Supabase client creation inside useEffect to prevent exhaustive-deps violation. This fixes the ESLint error where 'supabase' was used in the effect but not included in the dependency array. The Supabase client is now created inside the effect, ensuring it's properly scoped and doesn't cause dependency issues. Fixes GitHub Actions CI/CD pipeline failure. Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
10 issues found across 17 files
Prompt for AI agents (all 10 issues)
Understand the root cause of the following 10 issues and fix them.
<file name="hooks/useAdminAuth.ts">
<violation number="1" location="hooks/useAdminAuth.ts:46">
Core authentication state management logic duplicates hooks/useAuth.ts:useAuth() function's useEffect block. This includes initial user fetching and subscription to auth state changes.</violation>
<violation number="2" location="hooks/useAdminAuth.ts:86">
Clear the error state when the profile fetch succeeds so the hook doesn't leave a stale error visible after recovery.</violation>
</file>
<file name="lib/auth/admin-auth.ts">
<violation number="1" location="lib/auth/admin-auth.ts:49">
User data fetching logic duplicates `lib/auth/helpers.ts:getAuthUser()` function. `getAdminUser` is nearly identical to `getAuthUser` but includes the user's role.</violation>
</file>
<file name="app/api/admin/validator/start/route.ts">
<violation number="1" location="app/api/admin/validator/start/route.ts:35">
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</violation>
<violation number="2" location="app/api/admin/validator/start/route.ts:50">
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</violation>
<violation number="3" location="app/api/admin/validator/start/route.ts:88">
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</violation>
<violation number="4" location="app/api/admin/validator/start/route.ts:131">
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</violation>
</file>
<file name="app/api/admin/activity/route.ts">
<violation number="1" location="app/api/admin/activity/route.ts:114">
Passing `{ error }` as the second argument means the logger no longer receives the thrown error object, so it won’t log the stack trace. Please pass the actual error (and metadata, if needed, as the third argument) to preserve diagnostic details.</violation>
</file>
<file name="app/api/admin/api-keys/control/route.ts">
<violation number="1" location="app/api/admin/api-keys/control/route.ts:140">
Pass the actual error object to logger.error so its name, message, and stack are logged.</violation>
</file>
<file name="app/api/admin/users/route.ts">
<violation number="1" location="app/api/admin/users/route.ts:31">
If the limit query param is non-numeric, parseInt returns NaN, so Math.min propagates NaN and the Supabase query is executed with limit=NaN, causing a PostgREST error and a 500 response. Default the limit when parsing fails.</violation>
</file>
React with 👍 or 👎 to teach cubic. Mention @cubic-dev-ai to give feedback, ask questions, or re-run the review.
| const [error, setError] = useState<string | null>(null) | ||
| const supabase = createClient() | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
Core authentication state management logic duplicates hooks/useAuth.ts:useAuth() function's useEffect block. This includes initial user fetching and subscription to auth state changes.
Prompt for AI agents
Address the following comment on hooks/useAdminAuth.ts at line 46:
<comment>Core authentication state management logic duplicates hooks/useAuth.ts:useAuth() function's useEffect block. This includes initial user fetching and subscription to auth state changes.</comment>
<file context>
@@ -0,0 +1,146 @@
+ const [error, setError] = useState<string | null>(null)
+ const supabase = createClient()
+
+ useEffect(() => {
+ /**
+ * Initialize admin auth state
</file context>
| return | ||
| } | ||
|
|
||
| setUser({ |
There was a problem hiding this comment.
Clear the error state when the profile fetch succeeds so the hook doesn't leave a stale error visible after recovery.
Prompt for AI agents
Address the following comment on hooks/useAdminAuth.ts at line 86:
<comment>Clear the error state when the profile fetch succeeds so the hook doesn't leave a stale error visible after recovery.</comment>
<file context>
@@ -0,0 +1,146 @@
+ return
+ }
+
+ setUser({
+ id: profileData.id,
+ email: profileData.email,
</file context>
| * } | ||
| * ``` | ||
| */ | ||
| export async function getAdminUser( |
There was a problem hiding this comment.
User data fetching logic duplicates lib/auth/helpers.ts:getAuthUser() function. getAdminUser is nearly identical to getAuthUser but includes the user's role.
Prompt for AI agents
Address the following comment on lib/auth/admin-auth.ts at line 49:
<comment>User data fetching logic duplicates `lib/auth/helpers.ts:getAuthUser()` function. `getAdminUser` is nearly identical to `getAuthUser` but includes the user's role.</comment>
<file context>
@@ -0,0 +1,369 @@
+ * }
+ * ```
+ */
+export async function getAdminUser(
+ request: NextRequest
+): Promise<AdminUser | null> {
</file context>
| } catch (error: unknown) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error' | ||
| logger.error('[API] Error getting validator status', { error }) |
There was a problem hiding this comment.
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.
Prompt for AI agents
Address the following comment on app/api/admin/validator/start/route.ts at line 131:
<comment>Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</comment>
<file context>
@@ -27,83 +31,107 @@ export async function POST(request: NextRequest) {
+ } catch (error: unknown) {
+ const errorMessage =
+ error instanceof Error ? error.message : 'Unknown error'
+ logger.error('[API] Error getting validator status', { error })
return NextResponse.json(
- { success: false, error: error.message },
</file context>
Suggested change
| logger.error('[API] Error getting validator status', { error }) | |
| logger.error('[API] Error getting validator status', error) |
| } catch (error: unknown) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error' | ||
| logger.error('[API] Error stopping validator', { error }) |
There was a problem hiding this comment.
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.
Prompt for AI agents
Address the following comment on app/api/admin/validator/start/route.ts at line 88:
<comment>Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</comment>
<file context>
@@ -27,83 +31,107 @@ export async function POST(request: NextRequest) {
+ } catch (error: unknown) {
+ const errorMessage =
+ error instanceof Error ? error.message : 'Unknown error'
+ logger.error('[API] Error stopping validator', { error })
return NextResponse.json(
- { success: false, error: error.message },
</file context>
Suggested change
| logger.error('[API] Error stopping validator', { error }) | |
| logger.error('[API] Error stopping validator', error) |
| } catch (error: unknown) { | ||
| const errorMessage = | ||
| error instanceof Error ? error.message : 'Unknown error' | ||
| logger.error('[API] Error starting validator', { error }) |
There was a problem hiding this comment.
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.
Prompt for AI agents
Address the following comment on app/api/admin/validator/start/route.ts at line 50:
<comment>Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</comment>
<file context>
@@ -27,83 +31,107 @@ export async function POST(request: NextRequest) {
+ } catch (error: unknown) {
+ const errorMessage =
+ error instanceof Error ? error.message : 'Unknown error'
+ logger.error('[API] Error starting validator', { error })
return NextResponse.json(
- { success: false, error: error.message },
</file context>
Suggested change
| logger.error('[API] Error starting validator', { error }) | |
| logger.error('[API] Error starting validator', error) |
| .catch((error: any) => { | ||
| logger.error('[API] Validator error', error) | ||
| .catch((error: unknown) => { | ||
| logger.error('[API] Validator error', { error }) |
There was a problem hiding this comment.
Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.
Prompt for AI agents
Address the following comment on app/api/admin/validator/start/route.ts at line 35:
<comment>Pass the caught error directly to logger.error so the stack trace is captured; wrapping it in an object causes the logger to drop the useful error fields.</comment>
<file context>
@@ -27,83 +31,107 @@ export async function POST(request: NextRequest) {
- .catch((error: any) => {
- logger.error('[API] Validator error', error)
+ .catch((error: unknown) => {
+ logger.error('[API] Validator error', { error })
})
</file context>
Suggested change
| logger.error('[API] Validator error', { error }) | |
| logger.error('[API] Validator error', error) |
| }) | ||
| } catch (error) { | ||
| logger.error('Error fetching activity', error) | ||
| logger.error('Error fetching activity', { error }) |
There was a problem hiding this comment.
Passing { error } as the second argument means the logger no longer receives the thrown error object, so it won’t log the stack trace. Please pass the actual error (and metadata, if needed, as the third argument) to preserve diagnostic details.
Prompt for AI agents
Address the following comment on app/api/admin/activity/route.ts at line 114:
<comment>Passing `{ error }` as the second argument means the logger no longer receives the thrown error object, so it won’t log the stack trace. Please pass the actual error (and metadata, if needed, as the third argument) to preserve diagnostic details.</comment>
<file context>
@@ -71,12 +94,24 @@ export async function GET(_request: NextRequest) {
})
} catch (error) {
- logger.error('Error fetching activity', error)
+ logger.error('Error fetching activity', { error })
return NextResponse.json(
{
</file context>
Suggested change
| logger.error('Error fetching activity', { error }) | |
| logger.error('Error fetching activity', error) |
| return NextResponse.json(result) | ||
| } catch (error) { | ||
| logger.error('Error controlling services', error) | ||
| logger.error('Error controlling services', { error }) |
There was a problem hiding this comment.
Pass the actual error object to logger.error so its name, message, and stack are logged.
Prompt for AI agents
Address the following comment on app/api/admin/api-keys/control/route.ts at line 140:
<comment>Pass the actual error object to logger.error so its name, message, and stack are logged.</comment>
<file context>
@@ -124,9 +127,17 @@ export async function POST(request: NextRequest) {
return NextResponse.json(result)
} catch (error) {
- logger.error('Error controlling services', error)
+ logger.error('Error controlling services', { error })
return NextResponse.json(
{
</file context>
Suggested change
| logger.error('Error controlling services', { error }) | |
| logger.error('Error controlling services', error) |
| const supabase = await createClient() | ||
| const searchParams = request.nextUrl.searchParams | ||
| const limit = parseInt(searchParams.get('limit') || '50') | ||
| const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 200) |
There was a problem hiding this comment.
If the limit query param is non-numeric, parseInt returns NaN, so Math.min propagates NaN and the Supabase query is executed with limit=NaN, causing a PostgREST error and a 500 response. Default the limit when parsing fails.
Prompt for AI agents
Address the following comment on app/api/admin/users/route.ts at line 31:
<comment>If the limit query param is non-numeric, parseInt returns NaN, so Math.min propagates NaN and the Supabase query is executed with limit=NaN, causing a PostgREST error and a 500 response. Default the limit when parsing fails.</comment>
<file context>
@@ -2,30 +2,63 @@ import { createClient } from '@/services/supabase/server'
const supabase = await createClient()
const searchParams = request.nextUrl.searchParams
- const limit = parseInt(searchParams.get('limit') || '50')
+ const limit = Math.min(parseInt(searchParams.get('limit') || '50'), 200)
// Get recent users
</file context>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary by cubic
Implemented role-based admin security with defense-in-depth and full audit logging for the admin dashboard and APIs. Replaces legacy API-key checks so only authenticated admins can access and all actions are tracked.
New Features
Migration