fix: say when an admin sign-out or password change couldn't be stored instead of answering ok or wrong password #1066

This commit is contained in:
Linus Rath
2026-11-01 00:05:15 +01:00
parent 740a892590
commit ff08fe50fe
5 changed files with 67 additions and 7 deletions
+12 -1
View File
@@ -283,7 +283,18 @@ export async function DELETE(request: NextRequest) {
return NextResponse.json({ error: 'Cross-origin request rejected' }, { status: 403 });
}
const ip = getClientIP(request);
await clearAdminSessionCookie();
try {
await clearAdminSessionCookie();
} catch (error) {
if (!isStoreUnavailable(error)) throw error;
// The cookie is gone from this browser, but the sign-out couldn't be
// stored, so a copy of the cookie isn't refused yet.
await auditLog('admin.logout', { stored: false }, ip);
return NextResponse.json(
{ error: 'Signed out in this browser, but the sign-out could not be saved, so a copy of the session still works until it expires. Try signing out again in a moment.' },
{ status: 503, headers: { 'Retry-After': '10' } },
);
}
await auditLog('admin.logout', {}, ip);
return NextResponse.json({ ok: true });
} catch (error) {
+2 -1
View File
@@ -1,5 +1,5 @@
import { NextRequest, NextResponse } from 'next/server';
import { changeAdminPassword } from '@/lib/admin/password';
import { changeAdminPassword, isAdminPasswordConflict } from '@/lib/admin/password';
import { requireAdminAuth, getClientIP, setAdminSessionCookie } from '@/lib/admin/session';
import { auditLog } from '@/lib/admin/audit';
import { logger } from '@/lib/logger';
@@ -35,6 +35,7 @@ export async function POST(request: NextRequest) {
await auditLog('admin.change-password', {}, ip);
return NextResponse.json({ ok: true });
} catch (error) {
if (isAdminPasswordConflict(error)) return NextResponse.json({ error: error.message }, { status: 409 });
const readOnly = configReadOnlyResponse(error);
if (readOnly) return readOnly;
logger.error('Admin change password error', { error: error instanceof Error ? error.message : 'Unknown error' });
@@ -5,6 +5,7 @@ import { mkdtempSync, rmSync } from 'node:fs';
import { tmpdir } from 'node:os';
import path from 'node:path';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { NextRequest } from 'next/server';
// With a shared store (#1066) an admin signed out on one replica has to be
// refused on every other one at once, a new admin password has to sign out
@@ -201,4 +202,32 @@ describe('admin sessions with a shared store', () => {
store.down = false;
expect(await statusFor(token)).toBe(200);
});
it('says so when a sign-out could not be stored', async () => {
await setInitialAdminPassword('first-password-1');
const token = await signIn();
store.down = true;
const { DELETE } = await import('@/app/api/admin/auth/route');
const res = await DELETE(new NextRequest('http://localhost/api/admin/auth', { method: 'DELETE', headers: { 'sec-fetch-site': 'same-origin' } }));
expect(res.status).toBe(503);
expect((await res.json()).error).toMatch(/could not be saved/);
expect(jar.has(COOKIE)).toBe(false);
store.down = false;
// As said: the copy still works.
expect(await statusFor(token)).toBe(200);
});
it('refuses the second of two password changes at once with 409, not "incorrect"', async () => {
await setInitialAdminPassword('first-password-1');
await signIn();
const { POST } = await import('@/app/api/admin/change-password/route');
const change = (newPassword: string) => POST(new NextRequest('http://localhost/api/admin/change-password', {
method: 'POST',
headers: { 'sec-fetch-site': 'same-origin', 'content-type': 'application/json' },
body: JSON.stringify({ currentPassword: 'first-password-1', newPassword }),
}));
const statuses = (await Promise.all([change('second-password-2'), change('third-password-3')])).map((r) => r.status);
expect(statuses.sort()).toEqual([200, 409]);
expect([await verifyAdminPassword('second-password-2'), await verifyAdminPassword('third-password-3')].filter(Boolean)).toHaveLength(1);
});
});
+20 -2
View File
@@ -330,7 +330,25 @@ export async function verifyAdminPassword(password: string): Promise<boolean> {
}
/**
* Change the admin password. Returns true on success.
* Two password changes at once (on two replicas): the one that came second
* was checked against a password that no longer applies.
*/
export class AdminPasswordConflictError extends Error {
constructor() {
super('The admin password was changed somewhere else a moment ago. Sign in with the new password and try again.');
this.name = 'AdminPasswordConflictError';
}
}
/** By name, as the class may come from another bundle's copy of this module. */
export function isAdminPasswordConflict(error: unknown): error is AdminPasswordConflictError {
return error instanceof Error && error.name === 'AdminPasswordConflictError';
}
/**
* Change the admin password. Returns true on success, false when the
* current password is wrong. Throws AdminPasswordConflictError when another
* change landed between the check and the write (shared store).
*/
export async function changeAdminPassword(currentPassword: string, newPassword: string): Promise<boolean> {
const shared = isSharedMode();
@@ -347,7 +365,7 @@ export async function changeAdminPassword(currentPassword: string, newPassword:
const hash = await hashPassword(newPassword);
const next: AdminConfigData = { passwordHash: hash, passwordHashFile: undefined };
if (shared) {
if (!(await writeConfigData(next, { ifVersion: stored!.version })).ok) return false;
if (!(await writeConfigData(next, { ifVersion: stored!.version })).ok) throw new AdminPasswordConflictError();
} else {
await writeConfigData(next);
}
+4 -3
View File
@@ -266,12 +266,13 @@ export async function clearAdminSessionCookie(): Promise<void> {
try {
await revokeAdminSession(payload.sid, payload.exp);
} catch (error) {
// Local mode keeps it in memory regardless; only a restart forgets it.
// A shared store that is down keeps every admin request out until it
// is back, but then the token works again until it expires.
logger.warn('Failed to save the admin session sign-out', {
error: error instanceof Error ? error.message : 'Unknown error',
});
// Local mode keeps it in memory regardless; only a restart forgets it.
// With a shared store nothing kept it: once the store is back a copy of
// the cookie would work again until it expires, so the caller is told.
if (isStoreUnavailable(error)) throw error;
}
}