refactor(auth): add withAdmin() wrapper for admin API routes. (#68)
Extract shared 401/403 gate into permission-middleware and sweep the four inline admin checks (import MTG/Pokemon, sync-catalog, card-submissions). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
parent
071a3dca21
commit
b615fac865
8 changed files with 79 additions and 48 deletions
|
|
@ -145,7 +145,7 @@ export default async function handler(req, res) {
|
|||
1. **Method check first.** Reject the wrong verb with 405 before doing any limiter work.
|
||||
2. **Auth check before any user-keyed limiter.** `extractUserIdentifier(userId)` THROWS when `userId` is null/undefined/empty (defensive). For `upload`, `generate`, `import`, and `scan`, the handler MUST call `getUserFromRequest(req)` (or equivalent JWT verification) and confirm a non-null user BEFORE calling the limiter. Wrong order = anonymous user bypasses (the THROW surfaces immediately during dev; do not catch and silently fall back to IP).
|
||||
3. **For IP-keyed limiters (`auth`, `search`), gate placement is flexible** — either at the top of the handler (after the method check) or after a separate auth check that the route happens to also have (e.g. `users/search` JWT-verifies before rate-limiting, both are correct). The limiter only needs `req` for IP extraction.
|
||||
4. **Admin-role check, if applicable, goes between auth and rate-limit.** Used by both `/api/cards/import-*` routes: `if (user.role !== 'admin') return res.status(403).json({ error: 'Admin access required' })` sits between the `if (!user)` 401 and the import rate-limit call.
|
||||
4. **Admin-role check, if applicable, goes between auth and rate-limit.** Import and admin routes use `withAdmin(handler)` from `lib/permission-middleware.js` (401/403 before the inner handler runs); per-user rate-limit calls sit inside the wrapped handler after auth.
|
||||
|
||||
**Identifier extraction:**
|
||||
|
||||
|
|
|
|||
|
|
@ -69,4 +69,4 @@ From there:
|
|||
- **Owner-only** (delete, settings): inside the handler, `if (user.userId !== resource.user_id) return res.status(403)`.
|
||||
- **Editor-or-owner**: use `withCollectionPermission('editor')`.
|
||||
- **Public read**: use `withCollectionPermission('viewer')` — handles `is_public` and explicit-permission case.
|
||||
- **Admin-only**: check `user.role === 'admin'` directly; consider extracting `withAdmin()` if a third call site appears.
|
||||
- **Admin-only**: use `withAdmin(handler)` from `lib/permission-middleware.js`; inner handler receives `(req, res, user)` after 401/403 gates.
|
||||
|
|
|
|||
|
|
@ -105,6 +105,23 @@ function checkRolePermission(userRole, requiredPermission) {
|
|||
return userLevel >= requiredLevel;
|
||||
}
|
||||
|
||||
/**
|
||||
* Wrap an API handler with authenticated admin gate.
|
||||
* Inner handler receives (req, res, user) after 401/403 checks pass.
|
||||
*/
|
||||
export function withAdmin(handler) {
|
||||
return async function adminHandler(req, res) {
|
||||
const user = await getUserFromRequest(req);
|
||||
if (!user) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
if (user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
return handler(req, res, user);
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Middleware to protect collection routes
|
||||
*/
|
||||
|
|
|
|||
|
|
@ -1,17 +1,8 @@
|
|||
import { sql } from '@vercel/postgres';
|
||||
import { getUserFromRequest } from '../../../lib/permission-middleware';
|
||||
import { withAdmin } from '../../../lib/permission-middleware';
|
||||
|
||||
export default async function handler(req, res) {
|
||||
export default withAdmin(async function handler(req, res, user) {
|
||||
try {
|
||||
const user = await getUserFromRequest(req);
|
||||
if (!user) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
|
||||
if (user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
|
||||
if (req.method === 'GET') {
|
||||
const { status = 'pending' } = req.query;
|
||||
const result = await sql`
|
||||
|
|
@ -101,4 +92,4 @@ export default async function handler(req, res) {
|
|||
console.error('[admin/card-submissions]', error);
|
||||
return res.status(500).json({ error: 'Internal server error' });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,20 +1,12 @@
|
|||
import { getUserFromRequest } from '../../../lib/permission-middleware';
|
||||
import { withAdmin } from '../../../lib/permission-middleware';
|
||||
import { checkImportRateLimit } from '../../../lib/rate-limit.js';
|
||||
import { runCatalogSync } from '../../../lib/card-import/sync-catalog.js';
|
||||
|
||||
export default async function handler(req, res) {
|
||||
export default withAdmin(async function handler(req, res, user) {
|
||||
if (req.method !== 'POST') {
|
||||
return res.status(405).json({ error: 'Method not allowed' });
|
||||
}
|
||||
|
||||
const user = await getUserFromRequest(req);
|
||||
if (!user) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
if (user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
|
||||
const { allowed, reset } = await checkImportRateLimit(req, user.userId);
|
||||
if (!allowed) {
|
||||
res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
|
||||
|
|
@ -28,4 +20,4 @@ export default async function handler(req, res) {
|
|||
console.error('[POST /api/admin/sync-catalog]', error);
|
||||
return res.status(500).json({ error: 'Catalog sync failed', details: error.message });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,20 +1,12 @@
|
|||
import { getUserFromRequest } from '../../../lib/permission-middleware';
|
||||
import { withAdmin } from '../../../lib/permission-middleware';
|
||||
import { checkImportRateLimit } from '../../../lib/rate-limit.js';
|
||||
import { importMtgSet } from '../../../lib/card-import/mtg.js';
|
||||
|
||||
export default async function handler(req, res) {
|
||||
export default withAdmin(async function handler(req, res, user) {
|
||||
if (req.method !== 'POST') {
|
||||
return res.status(405).json({ error: 'Method not allowed' });
|
||||
}
|
||||
|
||||
const user = await getUserFromRequest(req);
|
||||
if (!user) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
if (user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
|
||||
const { allowed, reset } = await checkImportRateLimit(req, user.userId);
|
||||
if (!allowed) {
|
||||
res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
|
||||
|
|
@ -42,4 +34,4 @@ export default async function handler(req, res) {
|
|||
details: error.message,
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,20 +1,12 @@
|
|||
import { getUserFromRequest } from '../../../lib/permission-middleware';
|
||||
import { withAdmin } from '../../../lib/permission-middleware';
|
||||
import { checkImportRateLimit } from '../../../lib/rate-limit.js';
|
||||
import { importPokemonSet } from '../../../lib/card-import/pokemon.js';
|
||||
|
||||
export default async function handler(req, res) {
|
||||
export default withAdmin(async function handler(req, res, user) {
|
||||
if (req.method !== 'POST') {
|
||||
return res.status(405).json({ error: 'Method not allowed' });
|
||||
}
|
||||
|
||||
const user = await getUserFromRequest(req);
|
||||
if (!user) {
|
||||
return res.status(401).json({ error: 'Authentication required' });
|
||||
}
|
||||
if (user.role !== 'admin') {
|
||||
return res.status(403).json({ error: 'Admin access required' });
|
||||
}
|
||||
|
||||
const { allowed, reset } = await checkImportRateLimit(req, user.userId);
|
||||
if (!allowed) {
|
||||
res.setHeader('Retry-After', Math.ceil((reset - Date.now()) / 1000));
|
||||
|
|
@ -46,4 +38,4 @@ export default async function handler(req, res) {
|
|||
details: error.message,
|
||||
});
|
||||
}
|
||||
}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -5,7 +5,7 @@ vi.mock('@vercel/postgres', () => ({ sql: vi.fn() }));
|
|||
|
||||
import { sql } from '@vercel/postgres';
|
||||
import { JWT_SECRET } from '../../lib/auth-secret.js';
|
||||
import { getUserFromRequest } from '../../lib/permission-middleware.js';
|
||||
import { getUserFromRequest, withAdmin } from '../../lib/permission-middleware.js';
|
||||
|
||||
function makeToken(payload, opts = {}) {
|
||||
return jwt.sign(payload, JWT_SECRET, { expiresIn: opts.expiresIn ?? '1h' });
|
||||
|
|
@ -97,3 +97,50 @@ describe('getUserFromRequest', () => {
|
|||
expect(user).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('withAdmin', () => {
|
||||
beforeEach(() => {
|
||||
sql.mockReset();
|
||||
sql.mockResolvedValue({ rows: [] });
|
||||
vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
});
|
||||
|
||||
it('returns 401 when unauthenticated', async () => {
|
||||
const inner = vi.fn();
|
||||
const res = { status: vi.fn().mockReturnThis(), json: vi.fn() };
|
||||
await withAdmin(inner)({ headers: {} }, res);
|
||||
expect(res.status).toHaveBeenCalledWith(401);
|
||||
expect(inner).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('returns 403 when user is not admin', async () => {
|
||||
sql.mockResolvedValueOnce({
|
||||
rows: [{ id: 2, email: 'alice@deckhearth.com', role: 'user' }],
|
||||
});
|
||||
const token = makeToken({ userId: 2, email: 'alice@deckhearth.com' });
|
||||
const inner = vi.fn();
|
||||
const res = { status: vi.fn().mockReturnThis(), json: vi.fn() };
|
||||
await withAdmin(inner)(
|
||||
{ headers: { authorization: `Bearer ${token}` } },
|
||||
res
|
||||
);
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(inner).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('calls inner handler with admin user', async () => {
|
||||
sql.mockResolvedValueOnce({
|
||||
rows: [{ id: 1, email: 'admin@deckhearth.com', role: 'admin' }],
|
||||
});
|
||||
const token = makeToken({ userId: 1, email: 'admin@deckhearth.com' });
|
||||
const inner = vi.fn().mockResolvedValue(undefined);
|
||||
const req = { headers: { authorization: `Bearer ${token}` } };
|
||||
const res = {};
|
||||
await withAdmin(inner)(req, res);
|
||||
expect(inner).toHaveBeenCalledWith(req, res, {
|
||||
userId: 1,
|
||||
email: 'admin@deckhearth.com',
|
||||
role: 'admin',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue