feat(auth): add RP-initiated logout and OAuth unit tests
OAuth logout now terminates the IdP session via end_session_endpoint with HTTPS-only URL validation. Adds 14 unit tests for PKCE and OAuth discovery.
This commit is contained in:
@@ -117,7 +117,7 @@ This webmail client is designed to work seamlessly with [**Stalwart Mail Server*
|
||||
- SPF/DKIM/DMARC status indicators
|
||||
- No password storage (session-based auth)
|
||||
- TOTP two-factor authentication support
|
||||
- OAuth2/OIDC with PKCE for SSO login (opt-in, Basic Auth remains default)
|
||||
- OAuth2/OIDC with PKCE for SSO login (opt-in, RP-initiated logout, Basic Auth remains default)
|
||||
- External IdP support (Keycloak, Authentik) via configurable issuer URL
|
||||
- Session persistence via httpOnly refresh token cookies
|
||||
- CORS misconfiguration detection with actionable error messages
|
||||
|
||||
+2
-2
@@ -18,7 +18,7 @@ This document tracks the development status and planned features for JMAP Webmai
|
||||
- [x] Authentication error handling
|
||||
- [x] JMAP identities for sender address
|
||||
- [x] TOTP two-factor authentication (Stalwart-compatible)
|
||||
- [x] OAuth2/OIDC with PKCE (opt-in SSO, session persistence via httpOnly refresh tokens)
|
||||
- [x] OAuth2/OIDC with PKCE (opt-in SSO, session persistence, RP-initiated logout)
|
||||
- [x] External IdP support via explicit issuer URL (Keycloak, Authentik, etc.)
|
||||
|
||||
### JMAP Server Connection
|
||||
@@ -220,6 +220,7 @@ This document tracks the development status and planned features for JMAP Webmai
|
||||
- [x] Unit tests for calendar invitation parsing (25 tests)
|
||||
- [x] Unit tests for calendar participants (26 tests)
|
||||
- [x] Unit tests for template utilities (48 tests)
|
||||
- [x] Unit tests for OAuth PKCE and discovery (14 tests)
|
||||
- [x] XSS attack vector testing
|
||||
- [x] Playwright E2E framework setup
|
||||
|
||||
@@ -237,7 +238,6 @@ This document tracks the development status and planned features for JMAP Webmai
|
||||
- [ ] Free/busy queries (Principal/getAvailability)
|
||||
- [ ] Calendar sharing UI (JMAP Sharing RFC 9670)
|
||||
- [ ] Email encryption (PGP/GPG)
|
||||
- [ ] OAuth2 token introspection and userinfo endpoint support
|
||||
|
||||
### Performance Optimizations
|
||||
- [ ] Email content caching
|
||||
|
||||
@@ -37,10 +37,9 @@ async function getTokenEndpoint(): Promise<string> {
|
||||
return metadata.token_endpoint;
|
||||
}
|
||||
|
||||
async function getRevocationEndpoint(): Promise<string | null> {
|
||||
async function getMetadata(): Promise<import('@/lib/oauth/discovery').OAuthMetadata | null> {
|
||||
const { discoveryUrl } = getRequiredConfig();
|
||||
const metadata = await discoverOAuth(discoveryUrl);
|
||||
return metadata?.revocation_endpoint || null;
|
||||
return discoverOAuth(discoveryUrl);
|
||||
}
|
||||
|
||||
function buildOAuthParams(base: Record<string, string>): URLSearchParams {
|
||||
@@ -159,17 +158,22 @@ export async function DELETE() {
|
||||
try {
|
||||
const cookieStore = await cookies();
|
||||
const refreshToken = cookieStore.get(REFRESH_TOKEN_COOKIE)?.value;
|
||||
const metadata = await getMetadata().catch((err) => {
|
||||
logger.warn('Failed to discover OAuth metadata during logout', {
|
||||
error: err instanceof Error ? err.message : 'Unknown error',
|
||||
});
|
||||
return null;
|
||||
});
|
||||
|
||||
if (refreshToken) {
|
||||
const revocationEndpoint = await getRevocationEndpoint();
|
||||
if (revocationEndpoint) {
|
||||
if (metadata?.revocation_endpoint) {
|
||||
const params = buildOAuthParams({
|
||||
token: refreshToken,
|
||||
token_type_hint: 'refresh_token',
|
||||
});
|
||||
|
||||
try {
|
||||
const revocationResponse = await fetch(revocationEndpoint, {
|
||||
const revocationResponse = await fetch(metadata.revocation_endpoint, {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/x-www-form-urlencoded' },
|
||||
body: params.toString(),
|
||||
@@ -185,7 +189,21 @@ export async function DELETE() {
|
||||
cookieStore.delete(REFRESH_TOKEN_COOKIE);
|
||||
}
|
||||
|
||||
return NextResponse.json({ ok: true });
|
||||
let end_session_url: string | undefined;
|
||||
if (metadata?.end_session_endpoint) {
|
||||
try {
|
||||
const parsed = new URL(metadata.end_session_endpoint);
|
||||
if (parsed.protocol === 'https:') {
|
||||
end_session_url = metadata.end_session_endpoint;
|
||||
} else {
|
||||
logger.warn('Ignoring non-HTTPS end_session_endpoint', { url: metadata.end_session_endpoint });
|
||||
}
|
||||
} catch {
|
||||
logger.warn('Invalid end_session_endpoint URL', { url: metadata.end_session_endpoint });
|
||||
}
|
||||
}
|
||||
|
||||
return NextResponse.json({ ok: true, ...(end_session_url && { end_session_url }) });
|
||||
} catch (error) {
|
||||
logger.error('Token revocation error', { error: error instanceof Error ? error.message : 'Unknown error' });
|
||||
return NextResponse.json({ error: 'Internal server error' }, { status: 500 });
|
||||
|
||||
@@ -0,0 +1,108 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest';
|
||||
import type { OAuthMetadata } from '../oauth/discovery';
|
||||
|
||||
const VALID_METADATA: OAuthMetadata = {
|
||||
issuer: 'https://auth.example.com',
|
||||
authorization_endpoint: 'https://auth.example.com/authorize',
|
||||
token_endpoint: 'https://auth.example.com/token',
|
||||
revocation_endpoint: 'https://auth.example.com/revoke',
|
||||
end_session_endpoint: 'https://auth.example.com/logout',
|
||||
};
|
||||
|
||||
describe('oauth/discovery', () => {
|
||||
let discoverOAuth: typeof import('../oauth/discovery').discoverOAuth;
|
||||
|
||||
beforeEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
vi.unstubAllGlobals();
|
||||
vi.resetModules();
|
||||
const mod = await import('../oauth/discovery');
|
||||
discoverOAuth = mod.discoverOAuth;
|
||||
});
|
||||
|
||||
it('discovers metadata from oauth-authorization-server', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(VALID_METADATA),
|
||||
}));
|
||||
|
||||
const result = await discoverOAuth('https://mail.example.com');
|
||||
|
||||
expect(result).toEqual(VALID_METADATA);
|
||||
expect(fetch).toHaveBeenCalledTimes(1);
|
||||
expect(fetch).toHaveBeenCalledWith(
|
||||
'https://mail.example.com/.well-known/oauth-authorization-server'
|
||||
);
|
||||
});
|
||||
|
||||
it('falls back to openid-configuration when first returns 404', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn()
|
||||
.mockResolvedValueOnce({ ok: false, status: 404 })
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(VALID_METADATA),
|
||||
}));
|
||||
|
||||
const result = await discoverOAuth('https://fallback.example.com');
|
||||
|
||||
expect(result).toEqual(VALID_METADATA);
|
||||
expect(fetch).toHaveBeenCalledTimes(2);
|
||||
expect(fetch).toHaveBeenNthCalledWith(
|
||||
2,
|
||||
'https://fallback.example.com/.well-known/openid-configuration'
|
||||
);
|
||||
});
|
||||
|
||||
it('returns null when both endpoints fail', async () => {
|
||||
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
vi.stubGlobal('fetch', vi.fn()
|
||||
.mockResolvedValueOnce({ ok: false, status: 404 })
|
||||
.mockResolvedValueOnce({ ok: false, status: 404 }));
|
||||
|
||||
const result = await discoverOAuth('https://fail.example.com');
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(consoleSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('parses optional fields (revocation_endpoint, end_session_endpoint)', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(VALID_METADATA),
|
||||
}));
|
||||
|
||||
const result = await discoverOAuth('https://optional.example.com');
|
||||
|
||||
expect(result?.revocation_endpoint).toBe('https://auth.example.com/revoke');
|
||||
expect(result?.end_session_endpoint).toBe('https://auth.example.com/logout');
|
||||
});
|
||||
|
||||
it('returns null when required fields (authorization_endpoint, token_endpoint) are missing', async () => {
|
||||
const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
|
||||
vi.stubGlobal('fetch', vi.fn()
|
||||
.mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve({ issuer: 'https://auth.example.com' }),
|
||||
})
|
||||
.mockResolvedValueOnce({ ok: false, status: 404 }));
|
||||
|
||||
const result = await discoverOAuth('https://incomplete.example.com');
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(consoleSpy).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('caches results — second call for same server URL does not re-fetch', async () => {
|
||||
vi.stubGlobal('fetch', vi.fn().mockResolvedValueOnce({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(VALID_METADATA),
|
||||
}));
|
||||
|
||||
const first = await discoverOAuth('https://cached.example.com');
|
||||
const second = await discoverOAuth('https://cached.example.com');
|
||||
|
||||
expect(first).toEqual(VALID_METADATA);
|
||||
expect(second).toEqual(VALID_METADATA);
|
||||
expect(fetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,61 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { generateCodeVerifier, generateCodeChallenge, generateState } from '../oauth/pkce';
|
||||
|
||||
describe('oauth/pkce', () => {
|
||||
describe('generateCodeVerifier', () => {
|
||||
it('returns a 43-character base64url string', () => {
|
||||
const verifier = generateCodeVerifier();
|
||||
expect(verifier).toHaveLength(43);
|
||||
expect(verifier).toMatch(/^[A-Za-z0-9_-]+$/);
|
||||
});
|
||||
|
||||
it('contains no base64 padding or unsafe characters', () => {
|
||||
for (let i = 0; i < 20; i++) {
|
||||
const verifier = generateCodeVerifier();
|
||||
expect(verifier).not.toMatch(/[+/=]/);
|
||||
}
|
||||
});
|
||||
|
||||
it('generates unique values', () => {
|
||||
const a = generateCodeVerifier();
|
||||
const b = generateCodeVerifier();
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generateCodeChallenge', () => {
|
||||
it('returns a base64url string different from the verifier', async () => {
|
||||
const verifier = generateCodeVerifier();
|
||||
const challenge = await generateCodeChallenge(verifier);
|
||||
expect(challenge).toMatch(/^[A-Za-z0-9_-]+$/);
|
||||
expect(challenge).not.toBe(verifier);
|
||||
});
|
||||
|
||||
it('produces the RFC 7636 Appendix B test vector', async () => {
|
||||
const verifier = 'dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk';
|
||||
const challenge = await generateCodeChallenge(verifier);
|
||||
expect(challenge).toBe('E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM');
|
||||
});
|
||||
|
||||
it('is deterministic for the same verifier', async () => {
|
||||
const verifier = generateCodeVerifier();
|
||||
const a = await generateCodeChallenge(verifier);
|
||||
const b = await generateCodeChallenge(verifier);
|
||||
expect(a).toBe(b);
|
||||
});
|
||||
});
|
||||
|
||||
describe('generateState', () => {
|
||||
it('returns a 43-character base64url string', () => {
|
||||
const state = generateState();
|
||||
expect(state).toHaveLength(43);
|
||||
expect(state).toMatch(/^[A-Za-z0-9_-]+$/);
|
||||
});
|
||||
|
||||
it('generates unique values per call', () => {
|
||||
const a = generateState();
|
||||
const b = generateState();
|
||||
expect(a).not.toBe(b);
|
||||
});
|
||||
});
|
||||
});
|
||||
+21
-6
@@ -259,12 +259,7 @@ export const useAuthStore = create<AuthState>()(
|
||||
|
||||
logout: () => {
|
||||
const state = get();
|
||||
|
||||
if (state.authMode === 'oauth') {
|
||||
fetch('/api/auth/token', { method: 'DELETE' }).catch((err) => {
|
||||
debug.error('Token revocation failed:', err);
|
||||
});
|
||||
}
|
||||
const wasOAuth = state.authMode === 'oauth';
|
||||
|
||||
clearRefreshTimer();
|
||||
state.client?.disconnect();
|
||||
@@ -300,6 +295,26 @@ export const useAuthStore = create<AuthState>()(
|
||||
useVacationStore.getState().clearState();
|
||||
useCalendarStore.getState().clearState();
|
||||
useFilterStore.getState().clearState();
|
||||
|
||||
if (wasOAuth) {
|
||||
fetch('/api/auth/token', { method: 'DELETE' })
|
||||
.then((res) => {
|
||||
if (!res.ok) throw new Error(`Revocation failed: ${res.status}`);
|
||||
return res.json();
|
||||
})
|
||||
.then((data) => {
|
||||
if (data.end_session_url) {
|
||||
const locale = window.location.pathname.split('/')[1] || 'en';
|
||||
const redirectUri = `${window.location.origin}/${locale}/login`;
|
||||
const url = new URL(data.end_session_url);
|
||||
url.searchParams.set('post_logout_redirect_uri', redirectUri);
|
||||
window.location.href = url.toString();
|
||||
}
|
||||
})
|
||||
.catch((err) => {
|
||||
debug.error('OAuth logout cleanup failed:', err);
|
||||
});
|
||||
}
|
||||
},
|
||||
|
||||
checkAuth: async () => {
|
||||
|
||||
Reference in New Issue
Block a user