diff --git a/src/routes/api/admin/users/[id]/+server.ts b/src/routes/api/admin/users/[id]/+server.ts index d38779c..6de9780 100644 --- a/src/routes/api/admin/users/[id]/+server.ts +++ b/src/routes/api/admin/users/[id]/+server.ts @@ -3,6 +3,7 @@ import type { RequestHandler } from './$types.js'; import { requireAdmin } from '$lib/server/api-utils.js'; import { getUser, deleteUser, updateUserRole, resetPassword, revokeAllSessions } from '$lib/server/auth.js'; import { isEmailConfigured, sendPasswordResetEmail, sendRoleChangedEmail } from '$lib/server/email.js'; +import { canResetPassword, passwordResetBlockedReason } from '$lib/utils/auth-methods.js'; export const PATCH: RequestHandler = async ({ params, request, ...event }) => { const admin = requireAdmin(event); @@ -34,6 +35,20 @@ export const PATCH: RequestHandler = async ({ params, request, ...event }) => { if (body.newPassword.length < 8) { throw error(400, 'Password must be at least 8 characters'); } + // Prevent admin from resetting their own password — changing your own + // password goes through the profile flow, which verifies the current one. + if (userId === admin.id) { + throw error(400, 'Cannot reset your own password — change it from your profile instead'); + } + // Refuse accounts that do not sign in with a password. resetPassword() + // writes only passwordHash, and verifyPassword() matches only rows whose + // authProvider is 'password' — a hash written here could never be used to + // sign in, while the email below would claim the reset worked. The reason + // text comes from the same module as the check, so this matches what the + // UI explains; the ?? only satisfies error()'s string parameter. + if (!canResetPassword(user)) { + throw error(400, passwordResetBlockedReason(user) ?? 'This account does not use password login'); + } await resetPassword(userId, body.newPassword); // Notify user of password reset (fire-and-forget) diff --git a/tests/e2e/admin-users.spec.ts b/tests/e2e/admin-users.spec.ts index 6bf850e..46b44a6 100644 --- a/tests/e2e/admin-users.spec.ts +++ b/tests/e2e/admin-users.spec.ts @@ -1,4 +1,5 @@ -import { test, expect } from './helpers/fixtures.js'; +import { test, expect, TEST_EMAIL } from './helpers/fixtures.js'; +import { randomUUID } from 'crypto'; import { readFileSync } from 'fs'; import { fileURLToPath } from 'url'; import { dirname, join } from 'path'; @@ -23,6 +24,35 @@ async function createVictim( return user.id; } +/** + * Create a victim user with no password via the admin API, returning the user's id. + * Omitting the password is the OAuth-only path: createUser() stores authProvider + * 'none', which verifyPassword() never matches. + */ +async function createOAuthOnlyVictim( + page: import('@playwright/test').Page, + email: string, + displayName = email.split('@')[0] +): Promise { + const res = await page.request.post('/api/admin/users', { + data: { email, displayName, role: 'user' } + }); + expect(res.status()).toBe(201); + const user = await res.json(); + expect(user.authProvider).toBe('none'); + return user.id; +} + +/** The id of the admin account the authenticated page is signed in as. */ +async function currentAdminId(page: import('@playwright/test').Page): Promise { + const res = await page.request.get('/api/admin/users'); + expect(res.status()).toBe(200); + const users = (await res.json()) as Array<{ id: number; email: string }>; + const admin = users.find((u) => u.email === TEST_EMAIL); + if (!admin) throw new Error(`admin ${TEST_EMAIL} is missing from the users list`); + return admin.id; +} + test.describe.serial('Admin — User Deletion', () => { test('Scenario: Admin deletes a user from the users list', async ({ authenticatedPage: page @@ -151,3 +181,106 @@ test.describe.serial('Admin — User Deletion', () => { expect(loginRes.status()).toBe(401); }); }); + +test.describe.serial('Admin — Password Reset', () => { + test('Scenario: An account can sign in with a password the admin set', async ({ + authenticatedPage: page, + browser + }) => { + // Given a password-auth user exists + const email = 'reset-target@test.com'; + const victimId = await createVictim(page, email, 'Reset Target'); + + // When the admin sets a new password for that account + const newPassword = `reset-${randomUUID()}`; + const res = await page.request.patch(`/api/admin/users/${victimId}`, { + data: { newPassword } + }); + + // Then the reset succeeds + expect(res.status()).toBe(200); + + // And the account can sign in with the new password. A separate browser + // context keeps the resulting session cookie out of the admin's jar. + const ctx = await browser.newContext(); + const loginRes = await ctx.request.post('/api/auth/login', { + data: { email, password: newPassword } + }); + expect(loginRes.status()).toBe(200); + await ctx.close(); + + // Clean up + await page.request.delete(`/api/admin/users/${victimId}`); + }); + + test('Scenario: An account that signs in via OAuth cannot be given a password', async ({ + authenticatedPage: page, + browser + }) => { + // Given an OAuth-only user exists + const email = 'oauth-only@test.com'; + const victimId = await createOAuthOnlyVictim(page, email, 'OAuth Only'); + + // When the admin attempts to set a password for that account + const attemptedPassword = `attempt-${randomUUID()}`; + const res = await page.request.patch(`/api/admin/users/${victimId}`, { + data: { newPassword: attemptedPassword } + }); + + // Then the attempt is rejected and the response says why + expect(res.status()).toBe(400); + const body = await res.json(); + expect(body.message).toMatch(/OAuth|password login/i); + + // And the account still cannot sign in with the attempted password. + // This is the positive control for the rejection above: it confirms no + // usable credential was stored, not merely that the request 400'd. + const ctx = await browser.newContext(); + const loginRes = await ctx.request.post('/api/auth/login', { + data: { email, password: attemptedPassword } + }); + expect(loginRes.status()).toBe(401); + await ctx.close(); + + // Clean up + await page.request.delete(`/api/admin/users/${victimId}`); + }); + + test('Scenario: An admin cannot reset their own password', async ({ + authenticatedPage: page + }) => { + // Given the admin's own account + const adminId = await currentAdminId(page); + + // When the admin targets their own account for a password reset + const res = await page.request.patch(`/api/admin/users/${adminId}`, { + data: { newPassword: `self-${randomUUID()}` } + }); + + // Then the attempt is rejected and the response says why + expect(res.status()).toBe(400); + const body = await res.json(); + expect(body.message).toMatch(/your own password/i); + }); + + test('Scenario: A password below the minimum length is rejected for its length, whatever the account', async ({ + authenticatedPage: page + }) => { + // Given an OAuth-only user exists, who is also ineligible for a reset + const victimId = await createOAuthOnlyVictim(page, 'short-pw-oauth@test.com', 'Short Pw'); + + // When the admin attempts to set a password shorter than the minimum + const res = await page.request.patch(`/api/admin/users/${victimId}`, { + data: { newPassword: 'short' } + }); + + // Then the length rule is what the response reports, taking precedence + // over the eligibility rule + expect(res.status()).toBe(400); + const body = await res.json(); + expect(body.message).toMatch(/at least 8 characters/i); + + // Clean up + await page.request.delete(`/api/admin/users/${victimId}`); + }); +});