mirror of
https://github.com/bulwarkmail/webmail.git
synced 2026-10-30 16:16:19 +00:00
fix: keep a pairing re-auth from signing the browser in as another account
The step-up for linking a device sends the user to the identity provider with `prompt=login`. The login route accepted that pending state too, so a code from the re-auth could be spent there and sign this browser in as whichever account answered the prompt. It now refuses a re-auth state; only the pairing route completes one. The callback page also handles its code once. React's development double effect ran it twice: the second pass no longer saw the re-auth flag and raced the pairing route for the single-use code.
This commit is contained in:
@@ -1,6 +1,6 @@
|
||||
"use client";
|
||||
|
||||
import { Suspense, useEffect, useState } from "react";
|
||||
import { Suspense, useEffect, useRef, useState } from "react";
|
||||
import { useRouter, useSearchParams } from "next/navigation";
|
||||
import { useTranslations } from "next-intl";
|
||||
import { useAuthStore } from "@/stores/auth-store";
|
||||
@@ -18,8 +18,14 @@ function OAuthCallbackInner() {
|
||||
const t = useTranslations("login");
|
||||
const { loginWithOAuth, loginWithServerSso } = useAuthStore();
|
||||
const [error, setError] = useState<string | null>(null);
|
||||
// The authorization code is single-use. React's development double effect
|
||||
// ran this twice: the second pass no longer saw the pairing re-auth flag
|
||||
// (the first had removed it) and spent the code on the login flow instead.
|
||||
const handledRef = useRef(false);
|
||||
|
||||
useEffect(() => {
|
||||
if (handledRef.current) return;
|
||||
handledRef.current = true;
|
||||
const code = searchParams.get("code");
|
||||
const state = searchParams.get("state");
|
||||
const errorParam = searchParams.get("error");
|
||||
|
||||
@@ -63,6 +63,14 @@ export async function POST(request: NextRequest) {
|
||||
return NextResponse.json({ error: 'SSO session expired. Please try again.' }, { status: 400 });
|
||||
}
|
||||
|
||||
// A step-up for device pairing completes at /api/auth/reauth/sso/complete
|
||||
// only; here it would sign this browser in as whoever answered the prompt.
|
||||
if (pending.purpose === 'reauth') {
|
||||
logger.warn('SSO complete: refused a pairing re-auth session');
|
||||
cookieStore.delete(SSO_PENDING_COOKIE);
|
||||
return NextResponse.json({ error: 'Not a login session' }, { status: 400 });
|
||||
}
|
||||
|
||||
const codeVerifier = pending.code_verifier as string;
|
||||
const redirectUri = pending.redirect_uri as string;
|
||||
const pendingServerId = typeof pending.server_id === 'string' ? pending.server_id : null;
|
||||
|
||||
@@ -0,0 +1,77 @@
|
||||
// @vitest-environment node
|
||||
import { beforeEach, describe, expect, it, vi } from 'vitest';
|
||||
import { NextRequest } from 'next/server';
|
||||
|
||||
// A pairing step-up (purpose `reauth`) completes at
|
||||
// /api/auth/reauth/sso/complete only. Handed to the login route, its code
|
||||
// would sign this browser in as whoever answered the provider's prompt.
|
||||
|
||||
vi.mock('@/lib/auth/session-secret', () => ({
|
||||
getSessionSecret: () => 's'.repeat(64),
|
||||
hasSessionSecret: () => true,
|
||||
}));
|
||||
vi.mock('@/lib/logger', () => ({
|
||||
logger: { warn: () => {}, error: () => {}, info: () => {}, debug: () => {} },
|
||||
}));
|
||||
|
||||
const jar = new Map<string, string>();
|
||||
const cookieStore = {
|
||||
get: (name: string) => (jar.has(name) ? { name, value: jar.get(name)! } : undefined),
|
||||
set: (name: string, value: string) => { jar.set(name, value); },
|
||||
delete: (arg: string | { name: string }) => { jar.delete(typeof arg === 'string' ? arg : arg.name); },
|
||||
getAll: () => [...jar].map(([name, value]) => ({ name, value })),
|
||||
};
|
||||
vi.mock('next/headers', () => ({ cookies: async () => cookieStore }));
|
||||
|
||||
const exchangeCodeForTokens = vi.fn();
|
||||
vi.mock('@/lib/oauth/token-exchange', () => ({
|
||||
exchangeCodeForTokens: (...args: unknown[]) => exchangeCodeForTokens(...args),
|
||||
getTokenEndpoint: async () => 'https://idp.example.net/token',
|
||||
getRequiredConfig: () => ({ clientId: 'webmail', serverUrl: 'https://mail.example.org', discoveryUrl: 'https://idp.example.net' }),
|
||||
}));
|
||||
|
||||
import { encryptPayload } from '@/lib/auth/crypto';
|
||||
import { POST } from '@/app/api/auth/sso/complete/route';
|
||||
|
||||
function pending(extra: Record<string, unknown> = {}) {
|
||||
jar.set('sso_pending', encryptPayload({
|
||||
state: 'state-1',
|
||||
code_verifier: 'verifier-1',
|
||||
redirect_uri: 'https://webmail.example/en/auth/callback',
|
||||
created_at: Date.now(),
|
||||
...extra,
|
||||
}, 'sso-pending'));
|
||||
}
|
||||
|
||||
async function complete() {
|
||||
const res = await POST(new NextRequest('https://webmail.example/api/auth/sso/complete', {
|
||||
method: 'POST',
|
||||
headers: { 'content-type': 'application/json', 'sec-fetch-site': 'same-origin' },
|
||||
body: JSON.stringify({ code: 'code-1', state: 'state-1' }),
|
||||
}));
|
||||
return { status: res.status, body: await res.json() };
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
jar.clear();
|
||||
exchangeCodeForTokens.mockReset().mockResolvedValue({ access_token: 'at', refresh_token: 'rt', expires_in: 3600 });
|
||||
});
|
||||
|
||||
describe('/api/auth/sso/complete', () => {
|
||||
it('refuses a pairing re-auth and spends nothing', async () => {
|
||||
pending({ purpose: 'reauth', slot: 0 });
|
||||
const res = await complete();
|
||||
expect(res.status).toBe(400);
|
||||
expect(exchangeCodeForTokens).not.toHaveBeenCalled();
|
||||
expect(jar.has('jmap_rt')).toBe(false);
|
||||
expect(jar.has('sso_pending')).toBe(false);
|
||||
});
|
||||
|
||||
it('still completes an ordinary login', async () => {
|
||||
pending();
|
||||
const res = await complete();
|
||||
expect(res.status).toBe(200);
|
||||
expect(exchangeCodeForTokens).toHaveBeenCalledOnce();
|
||||
expect(jar.get('jmap_rt')).toBe('rt');
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user