diff --git a/CHANGELOG.md b/CHANGELOG.md index bfb13c19..12b13b8a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Changes + +- **Shared auth-guard helpers (`requireSession` / `requireUser` / `requireAdmin`) replace 5+ inlined session+ownership patterns across `src/db/queries/` and `src/features/`.** The auth-guard pattern (`if (!auth) → session lookup → optional ownership/role check → return uniform error envelope`) was duplicated 5 times in `api.mjs` (`getApiKeysByUserId`, `generateApiKey`, `deleteApiKey`), 2 times in `account.mjs` (`exportUserData`, `deleteUserAccount`), and reimplemented inline in `features/archives/reseedSeason.mjs` and `features/admin/actions.mjs` (`sendTestNotification`) — each with a slightly different error string for the same auth-failure condition (`'No session found'`, `'User does not match'`, `"You must be signed in to generate an API key"`, `"You don't have permission to delete this API key"`, `'Unauthorized'`, `'Forbidden'`). Extracted to a new `src/db/queries/_authGuards.mjs` module returning a `{ user, error }` discriminated-union with both keys always present (one nullable) — matching how callers were already destructuring the previous `requireAdmin` and fixing a long-standing JSDoc lie. Error strings standardize to `'Auth not configured'` / `'Not authenticated'` / `'Not authorized'` / `'Forbidden'`, which also distinguishes "no session at all" from "wrong user" in places (e.g. `deleteApiKey`) that previously collapsed both into one generic "permission" message. Closes the auth_consistency 87.5 → 80.0 strict-score regression flagged by `/desloppify`. Affected tests updated to assert the new uniform strings. + ## 0.51.0 ### Features diff --git a/package-lock.json b/package-lock.json index 277a9e8b..72681787 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "helldivers.bot", - "version": "0.47.4", + "version": "0.51.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "helldivers.bot", - "version": "0.47.4", + "version": "0.51.0", "dependencies": { "@asteasolutions/zod-to-openapi": "^8.5.0", "@mdx-js/loader": "^3.1.1", diff --git a/src/__tests__/unit/features/admin/actions.test.mjs b/src/__tests__/unit/features/admin/actions.test.mjs index a47c4bc0..16a12c5a 100644 --- a/src/__tests__/unit/features/admin/actions.test.mjs +++ b/src/__tests__/unit/features/admin/actions.test.mjs @@ -56,14 +56,14 @@ describe('sendTestNotification', () => { it('returns errors when session is null', async () => { auth.api.getSession.mockResolvedValue(null); const result = await sendTestNotification(); - expect(result.errors).toEqual({ auth: 'Unauthorized' }); + expect(result.errors).toEqual({ auth: 'Not authenticated' }); expect(result.time).toEqual(expect.any(Number)); }); it('returns errors when user is not admin', async () => { auth.api.getSession.mockResolvedValue({ user: { role: 'user' } }); const result = await sendTestNotification(); - expect(result.errors).toEqual({ auth: 'Unauthorized' }); + expect(result.errors).toEqual({ auth: 'Forbidden' }); expect(result.time).toEqual(expect.any(Number)); }); diff --git a/src/__tests__/unit/features/archives/reseedSeason.test.mjs b/src/__tests__/unit/features/archives/reseedSeason.test.mjs index 352c75ab..9aafa8b1 100644 --- a/src/__tests__/unit/features/archives/reseedSeason.test.mjs +++ b/src/__tests__/unit/features/archives/reseedSeason.test.mjs @@ -44,7 +44,7 @@ describe('reseedSeason', () => { it('returns errors when session is null', async () => { auth.api.getSession.mockResolvedValue(null); const result = await reseedSeason(153); - expect(result.errors).toEqual({ auth: 'Forbidden' }); + expect(result.errors).toEqual({ auth: 'Not authenticated' }); expect(result.time).toEqual(expect.any(Number)); expect(updateSeason).not.toHaveBeenCalled(); }); diff --git a/src/__tests__/unit/queries/api.test.mjs b/src/__tests__/unit/queries/api.test.mjs index 03ad851c..200a6f1c 100644 --- a/src/__tests__/unit/queries/api.test.mjs +++ b/src/__tests__/unit/queries/api.test.mjs @@ -20,7 +20,7 @@ describe('getApiKeysByUserId', () => { test('returns auth error when no session', async () => { const result = await getApiKeysByUserId(userId); - expect(result.errors.auth).toBe('No session found'); + expect(result.errors.auth).toBe('Not authenticated'); expect(result.data).toBeUndefined(); }); @@ -29,7 +29,7 @@ describe('getApiKeysByUserId', () => { const result = await getApiKeysByUserId(otherUserId); - expect(result.errors.auth).toBe('User does not match'); + expect(result.errors.auth).toBe('Not authorized'); expect(result.data).toBeUndefined(); }); @@ -85,7 +85,7 @@ describe('generateApiKey', () => { const result = await generateApiKey(null, validFormData); - expect(result.errors.auth).toMatch(/signed in/i); + expect(result.errors.auth).toBe('Not authenticated'); }); test('returns validation errors for invalid formData', async () => { @@ -107,7 +107,7 @@ describe('generateApiKey', () => { const result = await generateApiKey(null, mismatchFormData); - expect(result.errors.auth).toMatch(/permission/i); + expect(result.errors.auth).toBe('Not authorized'); }); test('returns max limit error when user has 5 keys', async () => { @@ -171,7 +171,7 @@ describe('deleteApiKey', () => { const result = await deleteApiKey(null, validFormData); - expect(result.errors.auth).toMatch(/permission/i); + expect(result.errors.auth).toBe('Not authenticated'); }); test('returns validation errors for invalid formData', async () => { @@ -190,7 +190,7 @@ describe('deleteApiKey', () => { const result = await deleteApiKey(null, mismatchFormData); - expect(result.errors.auth).toMatch(/permission/i); + expect(result.errors.auth).toBe('Not authorized'); }); test('deletes api key and revalidates on success', async () => { diff --git a/src/db/queries/_authGuards.mjs b/src/db/queries/_authGuards.mjs new file mode 100644 index 00000000..98513670 --- /dev/null +++ b/src/db/queries/_authGuards.mjs @@ -0,0 +1,44 @@ +'use server'; +import { auth } from '@/auth'; +import { headers } from 'next/headers'; +import { ROLE } from '@/shared/enums/roles.mjs'; + +/** + * @typedef {{ user: object, error: null } | { user: null, error: string }} AuthGuardResult + * Both `user` and `error` are always present — one is non-null, the other is null. + * This lets callers destructure either field without conditional guards. + */ + +/** + * Verify the request has an authenticated session. + * @returns {Promise} + */ +export async function requireSession() { + if (!auth) return { user: null, error: 'Auth not configured' }; + const session = await auth.api.getSession({ headers: await headers() }); + if (!session?.user) return { user: null, error: 'Not authenticated' }; + return { user: session.user, error: null }; +} + +/** + * Verify the request is from an authenticated user whose id matches `userId`. + * @param {string} userId - User id the action targets. + * @returns {Promise} + */ +export async function requireUser(userId) { + const { user, error } = await requireSession(); + if (error) return { user: null, error }; + if (user.id !== userId) return { user: null, error: 'Not authorized' }; + return { user, error: null }; +} + +/** + * Verify the request is from an authenticated admin. + * @returns {Promise} + */ +export async function requireAdmin() { + const { user, error } = await requireSession(); + if (error) return { user: null, error }; + if (user.role !== ROLE.ADMIN) return { user: null, error: 'Forbidden' }; + return { user, error: null }; +} diff --git a/src/db/queries/account.mjs b/src/db/queries/account.mjs index b228d766..50e470cf 100644 --- a/src/db/queries/account.mjs +++ b/src/db/queries/account.mjs @@ -4,6 +4,7 @@ import { auth } from '@/auth'; import { headers } from 'next/headers'; import { tryCatch } from '@/shared/utils/tryCatch.mjs'; import { performanceTime } from '@/shared/utils/time.mjs'; +import { requireSession, requireUser } from '@/db/queries/_authGuards.mjs'; /** * Export all data for the authenticated user (profile, accounts, settings, API keys). @@ -12,16 +13,8 @@ import { performanceTime } from '@/shared/utils/time.mjs'; */ export async function exportUserData(userId) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - const session = await auth.api.getSession({ headers: await headers() }); - - if (!session || !session.user) { - return { errors: { auth: 'Not authenticated' }, time: performanceTime(start) }; - } - if (session.user.id !== userId) { - return { errors: { auth: 'Not authorized' }, time: performanceTime(start) }; - } + const { error: authError } = await requireUser(userId); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const { data: userData, error } = await tryCatch( db.user.findUnique({ @@ -56,18 +49,16 @@ export async function exportUserData(userId) { */ export async function deleteUserAccount(_, formData) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - const session = await auth.api.getSession({ headers: await headers() }); - - if (!session || !session.user) { - return { errors: { auth: 'Not authenticated' }, time: performanceTime(start) }; - } + const { user, error: authError } = await requireSession(); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const userId = formData.get('userId'); - if (session.user.id !== userId) { - return { errors: { auth: 'Not authorized' }, time: performanceTime(start) }; + if (user.id !== userId) { + return { + errors: { auth: 'Not authorized' }, + time: performanceTime(start), + }; } // Revoke all sessions before deleting user (cascade will delete sessions from DB, diff --git a/src/db/queries/admin.mjs b/src/db/queries/admin.mjs index 6f4b2f76..92d04768 100644 --- a/src/db/queries/admin.mjs +++ b/src/db/queries/admin.mjs @@ -1,25 +1,12 @@ 'use server'; import { z } from 'zod'; import db from '@/db/db'; -import { auth } from '@/auth'; -import { headers } from 'next/headers'; import { tryCatch } from '@/shared/utils/tryCatch.mjs'; import { performanceTime } from '@/shared/utils/time.mjs'; import { revalidatePath } from 'next/cache'; import { computeWorkerHealth } from '@/shared/utils/admin/computeWorkerHealth.mjs'; import { ROLE } from '@/shared/enums/roles.mjs'; - -/** - * Verify the current request is from an authenticated admin user. - * @returns {Promise<{ user: object } | { error: string }>} User object or error message - */ -async function requireAdmin() { - if (!auth) return { error: 'Auth not configured' }; - const session = await auth.api.getSession({ headers: await headers() }); - if (!session || !session.user) return { error: 'Not authenticated' }; - if (session.user.role !== ROLE.ADMIN) return { error: 'Forbidden' }; - return { user: session.user }; -} +import { requireAdmin } from '@/db/queries/_authGuards.mjs'; export async function getAllUsers() { const start = performance.now(); diff --git a/src/db/queries/api.mjs b/src/db/queries/api.mjs index 3d041083..efb8ce74 100644 --- a/src/db/queries/api.mjs +++ b/src/db/queries/api.mjs @@ -1,13 +1,12 @@ 'use server'; import { z } from 'zod'; import db from '@/db/db'; -import { auth } from '@/auth'; -import { headers } from 'next/headers'; import { tryCatch } from '@/shared/utils/tryCatch.mjs'; import { performance } from 'perf_hooks'; import { performanceTime } from '@/shared/utils/time.mjs'; import { randomUUID, createHash } from 'crypto'; import { revalidatePath } from 'next/cache'; +import { requireSession, requireUser } from '@/db/queries/_authGuards.mjs'; /** * Retrieve all API keys for the authenticated user. @@ -16,22 +15,8 @@ import { revalidatePath } from 'next/cache'; */ export async function getApiKeysByUserId(userId) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - const session = await auth.api.getSession({ headers: await headers() }); - - if (!session || !session.user) { - return { - errors: { auth: 'No session found' }, - time: performanceTime(start), - }; - } - if (session.user.id !== userId) { - return { - errors: { auth: 'User does not match' }, - time: performanceTime(start), - }; - } + const { error: authError } = await requireUser(userId); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const { data: result, error } = await tryCatch( db.ApiKey.findMany({ @@ -58,16 +43,8 @@ export async function getApiKeysByUserId(userId) { */ export async function generateApiKey(_, formData) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - - const session = await auth.api.getSession({ headers: await headers() }); - if (!session || !session?.user) { - return { - errors: { auth: 'You must be signed in to generate an API key' }, - time: performanceTime(start), - }; - } + const { user, error: authError } = await requireSession(); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const formValues = { userId: formData.get('userId'), @@ -87,9 +64,9 @@ export async function generateApiKey(_, formData) { }; } - if (session.user.id !== formValues.userId) { + if (user.id !== formValues.userId) { return { - errors: { auth: "You don't have permission to create this API key" }, + errors: { auth: 'Not authorized' }, time: performanceTime(start), }; } @@ -137,16 +114,8 @@ export async function generateApiKey(_, formData) { */ export async function deleteApiKey(_, formData) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - - const session = await auth.api.getSession({ headers: await headers() }); - if (!session || !session?.user) { - return { - errors: { auth: "You don't have permission to delete this API key" }, - time: performanceTime(start), - }; - } + const { user, error: authError } = await requireSession(); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const formValues = { userId: formData.get('userId'), @@ -166,9 +135,9 @@ export async function deleteApiKey(_, formData) { }; } - if (session.user.id !== formValues.userId) { + if (user.id !== formValues.userId) { return { - errors: { auth: "You don't have permission to delete this API key" }, + errors: { auth: 'Not authorized' }, time: performanceTime(start), }; } diff --git a/src/features/admin/actions.mjs b/src/features/admin/actions.mjs index b5744df8..dcedc4b8 100644 --- a/src/features/admin/actions.mjs +++ b/src/features/admin/actions.mjs @@ -1,6 +1,4 @@ 'use server'; -import { auth } from '@/auth'; -import { headers } from 'next/headers'; import { tryCatch } from '@/shared/utils/tryCatch.mjs'; import { performance } from 'perf_hooks'; import { performanceTime } from '@/shared/utils/time.mjs'; @@ -10,7 +8,7 @@ import { buildPayload, } from '@/update/pushNotifier.mjs'; import db from '@/db/db'; -import { ROLE } from '@/shared/enums/roles.mjs'; +import { requireAdmin } from '@/db/queries/_authGuards.mjs'; /** * Send a test push notification using the same payload format as real events. @@ -30,12 +28,8 @@ export async function sendTestNotification({ event_id, } = {}) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - const session = await auth.api.getSession({ headers: await headers() }); - if (!session?.user || session.user.role !== ROLE.ADMIN) { - return { errors: { auth: 'Unauthorized' }, time: performanceTime(start) }; - } + const { error: authError } = await requireAdmin(); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; if (!ensureVapid()) { return { diff --git a/src/features/archives/reseedSeason.mjs b/src/features/archives/reseedSeason.mjs index aeb6eff3..b1a9ad69 100644 --- a/src/features/archives/reseedSeason.mjs +++ b/src/features/archives/reseedSeason.mjs @@ -1,15 +1,13 @@ 'use server'; import { z } from 'zod'; import { revalidatePath } from 'next/cache'; -import { auth } from '@/auth'; -import { headers } from 'next/headers'; import { tryCatch } from '@/shared/utils/tryCatch.mjs'; import { performance } from 'perf_hooks'; import { performanceTime } from '@/shared/utils/time.mjs'; import { updateSeason } from '@/update/season.mjs'; import { computeBucket } from '@/shared/utils/bucketing.mjs'; import db from '@/db/db'; -import { ROLE } from '@/shared/enums/roles.mjs'; +import { requireAdmin } from '@/db/queries/_authGuards.mjs'; const seasonSchema = z.number().int().positive(); @@ -33,13 +31,8 @@ const seasonSchema = z.number().int().positive(); */ export async function reseedSeason(season) { const start = performance.now(); - if (!auth) - return { errors: { auth: 'Auth not configured' }, time: performanceTime(start) }; - - const session = await auth.api.getSession({ headers: await headers() }); - if (!session?.user || session.user.role !== ROLE.ADMIN) { - return { errors: { auth: 'Forbidden' }, time: performanceTime(start) }; - } + const { error: authError } = await requireAdmin(); + if (authError) return { errors: { auth: authError }, time: performanceTime(start) }; const parsed = seasonSchema.safeParse(season); if (!parsed.success)