feat: secure admin authentication with obfuscated paths #1
Conversation
- Replace hardcoded /admin routes with dynamic /p/[id] paths - Admin URL now controlled by ADMIN_PATH environment variable - Implement signed HMAC-SHA256 authentication tokens - Add timing-safe password comparison to prevent timing attacks - Protect /api/books POST, PUT, DELETE endpoints with auth - Add defense-in-depth auth checks in getServerSideProps - Remove old admin files, no trace of "admin" in codebase - Update README and AGENTS.md with security documentation Security improvements: - Tokens cannot be forged without ADMIN_PASSWORD - Tokens expire after 24 hours - Multiple validation layers (middleware + SSR + API) - No stack traces exposed for missing env vars Co-Authored-By: Claude Opus 4.5 <[email protected]>
Add responsive home buttons that display icons on mobile and text on desktop. Dashboard has Home and Logout buttons side-by-side; login page has a "Back to Home" link below the sign-in form. Co-Authored-By: Claude Opus 4.5 <[email protected]>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces the old client-side admin page with a secret-path, server-protected admin dashboard and login. Adds HMAC-signed token auth, cookie management, middleware enforcement, server-side props validation, auth/login/logout API routes, and makes mutation APIs require authentication. Requires ADMIN_PATH, ADMIN_PASSWORD, AUTH_SECRET environment vars. Changes
Sequence DiagramssequenceDiagram
participant Client
participant LoginPage as /p/{id}/login
participant AuthAPI as /api/p/{id}/auth
participant Middleware
participant Dashboard as /p/{id}/index
participant Cookies
rect rgba(200,220,255,0.5)
note over Client,LoginPage: Login flow
Client->>LoginPage: GET /p/{ADMIN_PATH}/login
LoginPage->>Middleware: request (no valid cookie)
Middleware-->>LoginPage: allow
LoginPage-->>Client: render form
Client->>AuthAPI: POST { password }
AuthAPI->>AuthAPI: validate ADMIN_PASSWORD (timing-safe)
alt valid
AuthAPI->>AuthAPI: generate signed token
AuthAPI->>Cookies: set HttpOnly auth cookie
AuthAPI-->>Client: 200 { redirectTo: /p/{ADMIN_PATH} }
else invalid
AuthAPI-->>Client: 401
end
end
rect rgba(220,255,220,0.5)
note over Client,Dashboard: Dashboard access
Client->>Dashboard: GET /p/{ADMIN_PATH}
Dashboard->>Middleware: request with auth cookie
Middleware->>Middleware: validate token (expiry, signature)
alt valid
Middleware-->>Dashboard: allow
Dashboard-->>Client: render dashboard
else invalid/missing
Middleware-->>Client: redirect to /p/{ADMIN_PATH}/login
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 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 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 |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/p/[id]/index.tsx (1)
44-70: Guard againstidbeing undefined or an array before use.
router.query.idis typed asstring | string[] | undefined. When callinghandleLogoutbefore the router is ready,idcould be undefined, resulting in a request to/api/p/undefined/logout.🛠️ Proposed fix
export default function AdminDashboard() { const router = useRouter(); const { id } = router.query; + const adminPath = typeof id === 'string' ? id : ''; // Form state ... async function handleLogout() { + if (!adminPath) return; try { - const response = await fetch(`/api/p/${id}/logout`, { method: 'POST' }); + const response = await fetch(`/api/p/${adminPath}/logout`, { method: 'POST' }); const data = await response.json(); if (data.redirectTo) { router.push(data.redirectTo); } } catch (error) { setMessage({ type: 'error', text: 'Failed to logout' }); } }
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Around line 112-131: Update the Admin Security section to include the missing
implementation details: document the token-based authentication using
HMAC-SHA256 signed tokens with 24-hour expiry, mention the middleware
enforcement via middleware.ts that protects /p/* and API routes, describe use of
crypto.timingSafeEqual for timing-safe comparisons during password/token
validation, and state the defense-in-depth approach (middleware +
getServerSideProps validation + API route checks) so readers can understand the
full security model and where to find the relevant implementations
(middleware.ts, getServerSideProps in pages/p/[id]/, and API handlers in
pages/api/p/[id]/).
- Around line 90-92: Add documentation for the required AUTH_SECRET environment
variable alongside the existing ADMIN_PATH and ADMIN_PASSWORD entries: state
that AUTH_SECRET is required (no default) and used as the HMAC-SHA256 signing
secret for tokens, include a short usage note (e.g., format/length guidance or
how to generate/store securely), and mirror the style of the existing lines
(bullet entry like `- AUTH_SECRET: ...`) so it appears with ADMIN_PATH and
ADMIN_PASSWORD in AGENTS.md.
In `@lib/auth.ts`:
- Around line 46-50: The token age check in lib/auth.ts only rejects overly old
tokens but allows future timestamps (negative tokenAge); update the validation
around tokenAge (computed from timestamp via parseInt) to also reject tokenAge <
0 (return false) and ensure isNaN(tokenAge) is still handled—i.e., after
computing tokenAge, add a check like if (isNaN(tokenAge) || tokenAge < 0 ||
tokenAge > COOKIE_MAX_AGE * 1000) return false so tokens with future timestamps
are rejected.
In `@middleware.ts`:
- Around line 61-65: The middleware currently logs and calls NextResponse.next()
when adminPath or adminPassword are missing, which allows requests to proceed;
change this to fail closed by returning an explicit 500 response (or redirect to
a safe page) instead of NextResponse.next(). Locate the check using adminPath
and adminPassword and replace the NextResponse.next() branch with a
NextResponse.error() or a NextResponse.redirect(...) that provides a clear error
response so admin routes cannot be accessed when those env vars are unset.
In `@package.json`:
- Line 10: The dependency bump to "@prisma/client": "^6.19.2" requires
performing Prisma v4→v6 migration steps: update your Prisma schema to handle
implicit many-to-many changes and run a migration (e.g., run the migrate dev
flow), remove previewFeatures entries "fullTextIndex" and "fullTextSearch" and
if you use Postgres full-text search rename the flag to "fullTextSearchPostgres"
in the schema, search for and replace any code importing or catching
NotFoundError (it no longer exists), audit code that converts between
Buffer/Uint8Array for any API changes, ensure no Prisma model names use reserved
words like async/await/using, regenerate the client (prisma generate) and run
the full test suite to surface TypeScript/runtime issues.
In `@pages/api/p/`[id]/auth.ts:
- Around line 13-29: The handler currently reads id from req.query and compares
it to adminPath but req.query.id may be a string array; update the code that
extracts id (from req.query) to normalize it to a single string (e.g., if
Array.isArray(req.query.id) use the first element or join appropriately) and
handle undefined before comparing to adminPath so the comparison in the existing
conditional (id !== adminPath) behaves correctly; adjust references to id,
req.query, and adminPath accordingly to ensure proper 404 handling.
In `@pages/p/`[id]/login.tsx:
- Around line 41-42: router.query.id can be undefined on initial render causing
requests to go to /api/p/undefined/auth; update the page to ensure a stable id
before submitting by either (A) guard using router.isReady and only read
router.query.id once router.isReady is true (and disable the form/submit until
ready), or (B) extract the id server-side in getServerSideProps and pass it into
the component as a prop (use that prop instead of router.query.id in the submit
handler); locate uses of useRouter, router.query, and the form submit handler in
pages/p/[id]/login.tsx and change the id source or add the isReady check
accordingly.
🧹 Nitpick comments (5)
README.md (1)
27-39: Consider documentinglib/auth.tsin project structure.The project structure section (lines 78-96) lists
lib/prisma.tsbut omitslib/auth.ts, which is a key file for the new authentication system. Consider updating the structure to include it for completeness.📝 Suggested documentation update
lib/ -└── prisma.ts # Prisma client +├── prisma.ts # Prisma client +└── auth.ts # Authentication utilitiespages/p/[id]/index.tsx (1)
211-230: Use Next.jsLinkcomponent for client-side navigation.Using a plain
<a>tag for internal navigation bypasses Next.js client-side routing, causing a full page reload. Use theLinkcomponent fromnext/linkfor better performance.♻️ Proposed refactor
+import Link from 'next/link'; ... - <a + <Link href="/" className="px-3 py-2 text-sm bg-blue-600 text-white rounded-md hover:bg-blue-700 transition-colors flex items-center gap-2" aria-label="Home" > ... <span className="hidden sm:inline">Home</span> - </a> + </Link>lib/auth.ts (1)
10-23: Consider using a dedicated signing secret separate from the password.Using
ADMIN_PASSWORDas the HMAC signing key means that changing the password will invalidate all existing sessions, which may be acceptable. However, a dedicatedAUTH_SECRETenvironment variable would provide better separation of concerns and allow password changes without forcing all sessions to expire.pages/api/p/[id]/logout.ts (1)
4-26: Consider adding authentication check for logout.The logout endpoint clears the auth cookie without verifying the user is authenticated first. While this is generally harmless (clearing a non-existent cookie has no effect), adding an auth check would:
- Maintain consistency with other protected endpoints
- Prevent potential logout CSRF attacks where an attacker could force-logout authenticated users
That said, this is a minor concern since the impact is limited to session termination.
♻️ Optional: Add authentication check
import type { NextApiRequest, NextApiResponse } from 'next'; -import { generateClearAuthCookie } from '@/lib/auth'; +import { generateClearAuthCookie, isAuthenticated } from '@/lib/auth'; export default function handler(req: NextApiRequest, res: NextApiResponse) { if (req.method !== 'POST') { return res.status(405).json({ error: 'Method not allowed' }); } const { id } = req.query; const adminPath = process.env.ADMIN_PATH; if (!adminPath) { console.error('ADMIN_PATH environment variable is not set'); return res.status(500).json({ error: 'Server configuration error' }); } // Validate the path matches if (id !== adminPath) { return res.status(404).json({ error: 'Not found' }); } + // Only allow logout if authenticated (prevents logout CSRF) + if (!isAuthenticated(req.cookies)) { + return res.status(401).json({ error: 'Unauthorized' }); + } + // Clear the auth cookie res.setHeader('Set-Cookie', generateClearAuthCookie()); return res.status(200).json({ success: true, redirectTo: `/p/${adminPath}/login` }); }middleware.ts (1)
4-5: Duplicate constants fromlib/auth.ts.
AUTH_COOKIE_NAMEandCOOKIE_MAX_AGEare defined in bothlib/auth.tsand here. While Edge middleware can't import Node.js modules, these constants could be extracted to a shared module without Node.js dependencies.♻️ Extract shared constants
Create a new file
lib/auth-constants.ts:export const AUTH_COOKIE_NAME = 'admin_auth'; export const COOKIE_MAX_AGE = 60 * 60 * 24; // 24 hours in secondsThen import from both
lib/auth.tsandmiddleware.ts:-const AUTH_COOKIE_NAME = 'admin_auth'; -const COOKIE_MAX_AGE = 60 * 60 * 24; // 24 hours in seconds +import { AUTH_COOKIE_NAME, COOKIE_MAX_AGE } from '@/lib/auth-constants';
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (12)
AGENTS.mdREADME.mdlib/auth.tsmiddleware.tspackage.jsonpages/api/books/[id].tspages/api/books/index.tspages/api/p/[id]/auth.tspages/api/p/[id]/logout.tspages/p/[id]/index.tsxpages/p/[id]/login.tsxprisma/schema.prisma
🧰 Additional context used
🧬 Code graph analysis (6)
pages/api/books/[id].ts (1)
lib/auth.ts (1)
isAuthenticated(111-114)
pages/api/p/[id]/logout.ts (1)
lib/auth.ts (1)
generateClearAuthCookie(102-105)
middleware.ts (1)
lib/auth.ts (3)
AUTH_COOKIE_NAME(3-3)COOKIE_MAX_AGE(4-4)validateAuthToken(29-59)
pages/p/[id]/index.tsx (2)
pages/p/[id]/login.tsx (1)
getServerSideProps(7-35)lib/auth.ts (2)
AUTH_COOKIE_NAME(3-3)validateAuthToken(29-59)
pages/p/[id]/login.tsx (2)
pages/p/[id]/index.tsx (1)
getServerSideProps(14-42)lib/auth.ts (2)
AUTH_COOKIE_NAME(3-3)validateAuthToken(29-59)
pages/api/books/index.ts (4)
pages/api/books/[id].ts (1)
handler(6-40)pages/api/p/[id]/auth.ts (1)
handler(8-44)types/book.ts (2)
ApiResponse(21-25)BookWithAuthors(17-19)lib/auth.ts (1)
isAuthenticated(111-114)
🔇 Additional comments (15)
prisma/schema.prisma (2)
10-16: LGTM!The explicit
@relation("AuthorToBook")correctly names this side of the many-to-many relationship. This improves schema clarity and ensures Prisma unambiguously resolves the relation.
18-22: LGTM!The relation name
"AuthorToBook"correctly matches the corresponding relation on theBookmodel, ensuring Prisma properly resolves the bidirectional many-to-many relationship.lib/auth.ts (2)
64-77: Good implementation of timing-safe comparison with constant-time length handling.The dummy comparison when lengths differ maintains constant execution time, which is the correct approach to prevent timing attacks. The implementation correctly uses
crypto.timingSafeEqualfor the actual comparison.
94-97: LGTM! Cookie security attributes are properly configured.The cookie implementation correctly sets
HttpOnly,SameSite=Strict, and conditionally addsSecurein production. This provides good protection against XSS and CSRF attacks.pages/api/p/[id]/auth.ts (1)
31-43: LGTM! Authentication flow is well-structured.The authentication logic correctly:
- Uses timing-safe password comparison via
validatePassword- Generates signed tokens with
generateAuthToken- Sets HTTP-only cookies with proper security attributes
- Returns appropriate status codes (401 for invalid password, 404 for path mismatch)
package.json (1)
27-27: TypeScript 5.9.3 is available and stable on npm registry, currently marked as the latest release. The version specification is valid.pages/api/books/index.ts (2)
3-3: LGTM!Import of
isAuthenticatedis correctly added to enable authentication checks for write operations.
13-18: LGTM!The authentication guard for POST requests is correctly implemented, consistent with the pattern used in
pages/api/books/[id].tsfor PUT and DELETE operations. The 401 response format matches the API response type.pages/api/books/[id].ts (2)
3-3: LGTM!Import added correctly for authentication utility.
25-36: LGTM!Authentication guards for PUT and DELETE operations are correctly implemented. The pattern is consistent with the POST protection in
pages/api/books/index.ts, providing uniform protection for all write operations.middleware.ts (2)
44-54: LGTM on timing-safe comparison.The constant-time comparison implementation correctly prevents timing attacks by:
- First checking length equality (constant-time short-circuit)
- XOR-ing all characters and OR-ing results to avoid early exit
70-89: API routes are adequately protected at the route level.API routes
/api/p/[id]/authand/api/p/[id]/logouthave proper path validation. Both routes validate that theidparameter matchesadminPathearly in the handler and return a 404 status if validation fails, making middleware-level protection unnecessary.pages/p/[id]/login.tsx (2)
7-35: LGTM!Server-side props correctly implement defense-in-depth by:
- Validating
ADMIN_PATHenvironment variable- Checking path parameter matches admin path
- Redirecting already-authenticated users to dashboard
This is consistent with the pattern in
pages/p/[id]/index.tsx.
72-136: LGTM on the UI implementation.The login form includes:
- Proper accessibility with
htmlForandidattributes- Loading state to prevent double submissions
- Error display for failed attempts
- Disabled button styling during loading
- Home navigation link
AGENTS.md (1)
46-54: Documentation accurately describes the admin page structure.The documentation clearly explains the protected admin dashboard and login page with their key characteristics.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
- Add AUTH_SECRET as dedicated signing secret (separate from password) - Reject future timestamps in token validation (tokenAge < 0) - Normalize req.query.id to handle string[] type in API routes - Pass adminPath from getServerSideProps to fix undefined router.query - Update AGENTS.md with comprehensive security documentation Co-Authored-By: Claude Opus 4.5 <[email protected]>
Show a spinner while books are being fetched, display an error message with a styled retry button if the fetch fails, and only show the empty state after confirming there are no books. Co-Authored-By: Claude Opus 4.5 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/p/[id]/index.tsx (1)
45-71: Guard logout against undefinedidand validate response status.
router.query.idis undefined during initial hydration and can bestring[]from dynamic routes. This allows requests like/api/p/undefined/logout. Additionally, the missingresponse.okcheck means error responses (404, 500) are processed without indication to the user. Normalize the id on the client and validate the response before proceeding.🛠️ Proposed fix
- const { id } = router.query; + const rawId = router.query.id; + const id = Array.isArray(rawId) ? rawId[0] : rawId; async function handleLogout() { try { + if (!id) { + setMessage({ type: 'error', text: 'Missing admin path' }); + return; + } const response = await fetch(`/api/p/${id}/logout`, { method: 'POST' }); const data = await response.json(); + if (!response.ok) { + setMessage({ type: 'error', text: data.error || 'Failed to logout' }); + return; + } if (data.redirectTo) { router.push(data.redirectTo); }
🤖 Fix all issues with AI agents
In `@pages/api/p/`[id]/auth.ts:
- Around line 38-41: Check that req.body.password is a string before calling
validatePassword; if typeof password !== 'string' (or it's missing), return a
400 response with an explanatory error instead of passing non-strings into
validatePassword. Update the handler around the existing password, req.body and
validatePassword usage to perform this type check and short-circuit with
res.status(400).json(...) when invalid, then only call
validatePassword(password) for string inputs to preserve the timing-safe
comparison.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
AGENTS.mdlib/auth.tsmiddleware.tspages/api/p/[id]/auth.tspages/api/p/[id]/logout.tspages/p/[id]/index.tsxpages/p/[id]/login.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- lib/auth.ts
- middleware.ts
- pages/p/[id]/login.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
pages/p/[id]/index.tsx (2)
pages/p/[id]/login.tsx (1)
getServerSideProps(7-36)lib/auth.ts (2)
AUTH_COOKIE_NAME(3-3)validateAuthToken(29-60)
🔇 Additional comments (5)
pages/api/p/[id]/logout.ts (1)
4-27: Logout handler aligns with the auth flow.
Path normalization, env guards, and cookie clearing are consistent and safe.AGENTS.md (1)
46-155: Security documentation updates are clear and complete.
The env var requirements and defense‑in‑depth description read well.pages/p/[id]/index.tsx (3)
14-42: Server‑side access checks look solid.
The path validation + auth‑cookie gate provide a strong defense‑in‑depth layer.Please sanity‑check unauthenticated redirects in a deployed environment.
59-102: Loading/error handling is consistent end‑to‑end.
State updates infetchBooksmap cleanly to the list’s loading and retry UI.Please sanity‑check the loading → error → retry flow in the UI.
Also applies to: 370-409
213-263: Header actions and labeling look good.
Clear affordances for Home and Logout with accessible labels.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Summary
ADMIN_PATHenvironment variable (e.g.,/p/x7k9m2p4instead of/admin)Changes
Security Features
Environment Variables Required
Test plan
/adminand/admin-loginreturn 404/p/wrong-guessreturns 404/p/${ADMIN_PATH}redirects to login when unauthenticated🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.