mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-11-03 16:16:59 +00:00
fix: key synced settings on the account a bearer token belongs to
A Bearer login may claim a bare local part, which is accepted when the server's `Session.username` has the same local part (TOTP users who type `john` log in this way). Settings were keyed on that claim, so on a multi-domain server `john@b.example` could claim `john` and read and overwrite the synced settings of the `john@a.example` who signs in as `john` with a password. `resolveJmapIdentity` now returns the canonical account name for a bare Bearer claim, the context cookie carries it as `accountName`, and the settings route keys on it. Basic logins and fully-qualified claims keep their existing key.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
import { NextRequest, NextResponse } from 'next/server';
|
||||
import { logger } from '@/lib/logger';
|
||||
import { JmapAuthVerificationError, verifyJmapIdentity } from '@/lib/auth/verify-jmap-auth';
|
||||
import { JmapAuthVerificationError, resolveJmapIdentity } from '@/lib/auth/verify-jmap-auth';
|
||||
import { setStalwartAuthContext } from '@/lib/stalwart/auth-context';
|
||||
import { configManager } from '@/lib/admin/config-manager';
|
||||
import { isPublicHttpUrl } from '@/lib/security/url-guard';
|
||||
@@ -71,7 +71,7 @@ export async function POST(request: NextRequest) {
|
||||
// any user's synced settings. Admin-configured servers are `trusted`,
|
||||
// which only relaxes the public-address requirement (they may live on a
|
||||
// private network) - never the credential check.
|
||||
const normalizedServerUrl = await verifyJmapIdentity(upstreamUrl, authHeader, username, {
|
||||
const { serverUrl: normalizedServerUrl, accountName } = await resolveJmapIdentity(upstreamUrl, authHeader, username, {
|
||||
trusted: upstreamTrusted,
|
||||
});
|
||||
|
||||
@@ -79,6 +79,7 @@ export async function POST(request: NextRequest) {
|
||||
serverUrl: normalizedServerUrl,
|
||||
username,
|
||||
authHeader,
|
||||
...(accountName !== username ? { accountName } : {}),
|
||||
});
|
||||
|
||||
void recordLogin(username, normalizedServerUrl);
|
||||
|
||||
+17
-11
@@ -67,9 +67,12 @@ function normalizeUrl(url: string): string {
|
||||
* With multi-account, the requesting account may be on any slot.
|
||||
* Checks both basic-auth session cookies and stalwart auth context cookies
|
||||
* (used by OAuth/SSO and TOTP-upgraded sessions).
|
||||
* Returns true only if a matching cookie is found.
|
||||
* Returns the account name to key the settings on, or null when no cookie
|
||||
* matches. That is the cookie's `accountName` when it has one: a Bearer
|
||||
* login claimed with a bare local part must not share `john`'s settings
|
||||
* with every other `john` on a multi-domain server.
|
||||
*/
|
||||
async function verifyIdentity(username: string, serverUrl: string): Promise<boolean> {
|
||||
async function verifyIdentity(username: string, serverUrl: string): Promise<string | null> {
|
||||
const cookieStore = await cookies();
|
||||
const normalizedServerUrl = normalizeUrl(serverUrl);
|
||||
|
||||
@@ -79,19 +82,19 @@ async function verifyIdentity(username: string, serverUrl: string): Promise<bool
|
||||
if (token) {
|
||||
const session = decryptSession(token);
|
||||
if (session && session.username === username && normalizeUrl(session.serverUrl) === normalizedServerUrl) {
|
||||
return true;
|
||||
return username;
|
||||
}
|
||||
}
|
||||
|
||||
// Check stalwart auth context cookie (set for all auth modes)
|
||||
const ctx = readStalwartAuthContextFromStore(cookieStore, slot);
|
||||
if (ctx && ctx.username === username && normalizeUrl(ctx.serverUrl) === normalizedServerUrl) {
|
||||
return true;
|
||||
return ctx.accountName ?? username;
|
||||
}
|
||||
}
|
||||
|
||||
// No matching session found (or no cookies at all) → reject
|
||||
return false;
|
||||
return null;
|
||||
}
|
||||
|
||||
export async function GET(request: NextRequest) {
|
||||
@@ -105,12 +108,13 @@ export async function GET(request: NextRequest) {
|
||||
return NextResponse.json({ error: 'Missing identity headers' }, { status: 400 });
|
||||
}
|
||||
|
||||
if (!(await verifyIdentity(username, serverUrl))) {
|
||||
const accountName = await verifyIdentity(username, serverUrl);
|
||||
if (!accountName) {
|
||||
return NextResponse.json({ error: 'Identity mismatch' }, { status: 403 });
|
||||
}
|
||||
|
||||
try {
|
||||
const settings = await loadUserSettings(username, serverUrl);
|
||||
const settings = await loadUserSettings(accountName, serverUrl);
|
||||
return NextResponse.json({ settings: settings || null });
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Unknown error';
|
||||
@@ -140,7 +144,8 @@ export async function POST(request: NextRequest) {
|
||||
return NextResponse.json({ error: 'Settings must be an object' }, { status: 400 });
|
||||
}
|
||||
|
||||
if (!(await verifyIdentity(username, serverUrl))) {
|
||||
const accountName = await verifyIdentity(username, serverUrl);
|
||||
if (!accountName) {
|
||||
return NextResponse.json({ error: 'Identity mismatch' }, { status: 403 });
|
||||
}
|
||||
|
||||
@@ -171,7 +176,7 @@ export async function POST(request: NextRequest) {
|
||||
}
|
||||
}
|
||||
|
||||
await saveUserSettings(username, serverUrl, filteredSettings);
|
||||
await saveUserSettings(accountName, serverUrl, filteredSettings);
|
||||
return NextResponse.json({ ok: true });
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Unknown error';
|
||||
@@ -197,11 +202,12 @@ export async function DELETE(request: NextRequest) {
|
||||
return NextResponse.json({ error: 'Missing required fields' }, { status: 400 });
|
||||
}
|
||||
|
||||
if (!(await verifyIdentity(username, serverUrl))) {
|
||||
const accountName = await verifyIdentity(username, serverUrl);
|
||||
if (!accountName) {
|
||||
return NextResponse.json({ error: 'Identity mismatch' }, { status: 403 });
|
||||
}
|
||||
|
||||
await deleteUserSettings(username, serverUrl);
|
||||
await deleteUserSettings(accountName, serverUrl);
|
||||
return NextResponse.json({ ok: true });
|
||||
} catch (error) {
|
||||
const message = error instanceof Error ? error.message : 'Unknown error';
|
||||
|
||||
@@ -0,0 +1,97 @@
|
||||
// @vitest-environment node
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { NextRequest } from 'next/server';
|
||||
|
||||
// A Bearer login may claim a bare local part ("john"). The settings route
|
||||
// keyed synced settings on the claim, so on a multi-domain server
|
||||
// john@b.example's token could read and overwrite the settings of the
|
||||
// john@a.example who signs in as "john". The context cookie now carries the
|
||||
// canonical account name and the route keys on it.
|
||||
|
||||
vi.mock('@/lib/auth/session-secret', () => ({
|
||||
getSessionSecret: () => 's'.repeat(64),
|
||||
hasSessionSecret: () => true,
|
||||
}));
|
||||
vi.mock('@/lib/logger', () => ({
|
||||
logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} },
|
||||
}));
|
||||
vi.mock('@/lib/admin/config-manager', () => ({
|
||||
configManager: {
|
||||
ensureLoaded: async () => {},
|
||||
get: (key: string, fallback: unknown) => (key === 'settingsSyncEnabled' ? true : fallback),
|
||||
getPolicy: () => ({ restrictions: {} }),
|
||||
},
|
||||
}));
|
||||
|
||||
const jar = new Map<string, string>();
|
||||
vi.mock('next/headers', () => ({
|
||||
cookies: async () => ({
|
||||
get: (name: string) => (jar.has(name) ? { name, value: jar.get(name)! } : undefined),
|
||||
set: (name: string, value: string) => { jar.set(name, value); },
|
||||
delete: (name: string) => { jar.delete(name); },
|
||||
getAll: () => [...jar].map(([name, value]) => ({ name, value })),
|
||||
}),
|
||||
}));
|
||||
|
||||
const store = new Map<string, Record<string, unknown>>();
|
||||
vi.mock('@/lib/settings-sync', () => ({
|
||||
loadUserSettings: async (username: string, serverUrl: string) => store.get(`${username}|${serverUrl}`) ?? null,
|
||||
saveUserSettings: async (username: string, serverUrl: string, settings: Record<string, unknown>) => {
|
||||
store.set(`${username}|${serverUrl}`, settings);
|
||||
},
|
||||
deleteUserSettings: async (username: string, serverUrl: string) => { store.delete(`${username}|${serverUrl}`); },
|
||||
}));
|
||||
|
||||
const SERVER = 'https://mail.example.org';
|
||||
const SAME_ORIGIN = { 'sec-fetch-site': 'same-origin', 'content-type': 'application/json' };
|
||||
|
||||
async function signIn(context: { username: string; accountName?: string }) {
|
||||
jar.clear();
|
||||
const { setStalwartAuthContextInStore } = await import('@/lib/stalwart/auth-context');
|
||||
const cookieStore = {
|
||||
get: (n: string) => (jar.has(n) ? { name: n, value: jar.get(n)! } : undefined),
|
||||
set: (n: string, v: string) => { jar.set(n, v); },
|
||||
delete: (n: string) => { jar.delete(n); },
|
||||
};
|
||||
setStalwartAuthContextInStore(cookieStore as never, 0, { serverUrl: SERVER, authHeader: 'Bearer t', ...context });
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
store.clear();
|
||||
process.env.SETTINGS_SYNC_ENABLED = 'true';
|
||||
});
|
||||
|
||||
describe('settings route keys on the account the credential belongs to', () => {
|
||||
it('saves a bare Bearer claim under the canonical account name', async () => {
|
||||
await signIn({ username: 'john', accountName: 'john@b.example' });
|
||||
const { POST } = await import('@/app/api/settings/route');
|
||||
const res = await POST(new NextRequest('https://webmail.example/api/settings', {
|
||||
method: 'POST',
|
||||
headers: SAME_ORIGIN,
|
||||
body: JSON.stringify({ username: 'john', serverUrl: SERVER, settings: { theme: 'dark' } }),
|
||||
}));
|
||||
expect(res.status).toBe(200);
|
||||
expect([...store.keys()]).toEqual([`john@b.example|${SERVER}`]);
|
||||
});
|
||||
|
||||
it("does not read the settings of the john who signs in with a password", async () => {
|
||||
store.set(`john|${SERVER}`, { signature: 'john@a.example private' });
|
||||
await signIn({ username: 'john', accountName: 'john@b.example' });
|
||||
const { GET } = await import('@/app/api/settings/route');
|
||||
const res = await GET(new NextRequest('https://webmail.example/api/settings', {
|
||||
headers: { 'x-settings-username': 'john', 'x-settings-server': SERVER },
|
||||
}));
|
||||
expect(await res.json()).toEqual({ settings: null });
|
||||
});
|
||||
|
||||
it('keeps keying on the login name when the context names no other account', async () => {
|
||||
await signIn({ username: 'alice@example.org' });
|
||||
const { POST } = await import('@/app/api/settings/route');
|
||||
await POST(new NextRequest('https://webmail.example/api/settings', {
|
||||
method: 'POST',
|
||||
headers: SAME_ORIGIN,
|
||||
body: JSON.stringify({ username: 'alice@example.org', serverUrl: SERVER, settings: { theme: 'light' } }),
|
||||
}));
|
||||
expect([...store.keys()]).toEqual([`alice@example.org|${SERVER}`]);
|
||||
});
|
||||
});
|
||||
@@ -179,3 +179,37 @@ describe('verifyJmapIdentity', () => {
|
||||
).rejects.toMatchObject({ status: 502 });
|
||||
});
|
||||
});
|
||||
|
||||
describe('resolveJmapIdentity account name', () => {
|
||||
let fetchSpy: ReturnType<typeof vi.spyOn>;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.resetModules();
|
||||
fetchSpy = vi.spyOn(globalThis, 'fetch');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
fetchSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('keys a bare Bearer claim on the canonical login, so john@b cannot become john', async () => {
|
||||
fetchSpy.mockResolvedValueOnce(json({ apiUrl: 'https://mail.internal/jmap/', username: 'john@b.example', accounts: {} }));
|
||||
const { resolveJmapIdentity } = await load();
|
||||
await expect(resolveJmapIdentity('https://mail.internal', 'Bearer tok', 'john', { trusted: true }))
|
||||
.resolves.toEqual({ serverUrl: 'https://mail.internal', accountName: 'john@b.example' });
|
||||
});
|
||||
|
||||
it('keeps a fully-qualified Bearer claim as it is', async () => {
|
||||
fetchSpy.mockResolvedValueOnce(json({ apiUrl: 'https://mail.internal/jmap/', username: 'john@b.example', accounts: {} }));
|
||||
const { resolveJmapIdentity } = await load();
|
||||
await expect(resolveJmapIdentity('https://mail.internal', 'Bearer tok', 'John@B.example', { trusted: true }))
|
||||
.resolves.toEqual({ serverUrl: 'https://mail.internal', accountName: 'John@B.example' });
|
||||
});
|
||||
|
||||
it('keeps a Basic claim as it is: the server accepted exactly that login', async () => {
|
||||
fetchSpy.mockResolvedValueOnce(json({ apiUrl: 'https://mail.internal/jmap/', username: 'john@a.example', accounts: {} }));
|
||||
const { resolveJmapIdentity } = await load();
|
||||
await expect(resolveJmapIdentity('https://mail.internal', basic('john', 'pw'), 'john', { trusted: true }))
|
||||
.resolves.toEqual({ serverUrl: 'https://mail.internal', accountName: 'john' });
|
||||
});
|
||||
});
|
||||
|
||||
@@ -220,6 +220,29 @@ export async function verifyJmapIdentity(
|
||||
claimedUsername: string,
|
||||
options: VerifyJmapAuthOptions = {},
|
||||
): Promise<string> {
|
||||
return (await resolveJmapIdentity(serverUrl, authHeader, claimedUsername, options)).serverUrl;
|
||||
}
|
||||
|
||||
export interface ResolvedJmapIdentity {
|
||||
/** Normalized server URL. */
|
||||
serverUrl: string;
|
||||
/**
|
||||
* The account the credential belongs to. Equal to the claim, except for a
|
||||
* Bearer credential claimed with a bare local part: that claim names no
|
||||
* domain, so two users (`john@a.example`, `john@b.example`) would share it,
|
||||
* and the server's canonical `Session.username` is used instead. Anything
|
||||
* keyed per user (synced settings) must key on this, not on the claim.
|
||||
*/
|
||||
accountName: string;
|
||||
}
|
||||
|
||||
/** {@link verifyJmapIdentity}, also returning the account name to key per-user data on. */
|
||||
export async function resolveJmapIdentity(
|
||||
serverUrl: string,
|
||||
authHeader: string,
|
||||
claimedUsername: string,
|
||||
options: VerifyJmapAuthOptions = {},
|
||||
): Promise<ResolvedJmapIdentity> {
|
||||
if (!claimedUsername) {
|
||||
throw new JmapAuthVerificationError('Missing username', 400);
|
||||
}
|
||||
@@ -231,16 +254,19 @@ export async function verifyJmapIdentity(
|
||||
|
||||
const { serverUrl: normalizedServerUrl, session } = await fetchVerifiedSession(serverUrl, authHeader, options);
|
||||
if (isBasic) {
|
||||
return normalizedServerUrl;
|
||||
// The server just accepted exactly this user:password, and a bare name
|
||||
// resolves to one account on it.
|
||||
return { serverUrl: normalizedServerUrl, accountName: claimedUsername };
|
||||
}
|
||||
|
||||
if (usernameMatchesSession(claimedUsername, session.username)) {
|
||||
return normalizedServerUrl;
|
||||
const bareClaim = !claimedUsername.includes('@') && session.username!.includes('@');
|
||||
return { serverUrl: normalizedServerUrl, accountName: bareClaim ? session.username! : claimedUsername };
|
||||
}
|
||||
|
||||
const identityEmails = await fetchIdentityEmails(normalizedServerUrl, authHeader, session, options);
|
||||
if (identityEmails.some((email) => usernamesEqual(email, claimedUsername))) {
|
||||
return normalizedServerUrl;
|
||||
return { serverUrl: normalizedServerUrl, accountName: claimedUsername };
|
||||
}
|
||||
|
||||
throw new JmapAuthVerificationError('Username does not match credentials', 403);
|
||||
|
||||
@@ -8,6 +8,12 @@ export interface StalwartAuthContext {
|
||||
serverUrl: string;
|
||||
username: string;
|
||||
authHeader: string;
|
||||
/**
|
||||
* The account the credential belongs to, when it differs from the login
|
||||
* name the client claimed (see ResolvedJmapIdentity.accountName). Per-user
|
||||
* data such as synced settings is keyed on this.
|
||||
*/
|
||||
accountName?: string;
|
||||
}
|
||||
|
||||
type CookieStore = Awaited<ReturnType<typeof cookies>>;
|
||||
@@ -32,7 +38,8 @@ function isValidContext(payload: unknown): payload is StalwartAuthContext {
|
||||
}
|
||||
return typeof candidate.serverUrl === 'string'
|
||||
&& typeof candidate.username === 'string'
|
||||
&& typeof candidate.authHeader === 'string';
|
||||
&& typeof candidate.authHeader === 'string'
|
||||
&& (candidate.accountName === undefined || typeof candidate.accountName === 'string');
|
||||
}
|
||||
|
||||
function getSessionCookieOptions() {
|
||||
@@ -52,8 +59,8 @@ export function readStalwartAuthContextFromStore(
|
||||
|
||||
// Hand back exactly the declared shape: the envelope's own bookkeeping
|
||||
// stays in the envelope rather than riding along into the credentials.
|
||||
const { serverUrl, username, authHeader } = payload;
|
||||
return { serverUrl, username, authHeader };
|
||||
const { serverUrl, username, authHeader, accountName } = payload;
|
||||
return accountName ? { serverUrl, username, authHeader, accountName } : { serverUrl, username, authHeader };
|
||||
}
|
||||
|
||||
export async function readStalwartAuthContext(slot: number): Promise<StalwartAuthContext | null> {
|
||||
|
||||
Reference in New Issue
Block a user