🐛 fix(auth): return 401 for expired OIDC JWT instead of 500 (#14014)

This commit is contained in:
YuTengjing 2026-04-21 16:43:57 +08:00 committed by GitHub
parent 8119789849
commit 61224fe76c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 216 additions and 11 deletions

View file

@ -2,6 +2,7 @@ import { AgentRuntimeError } from '@lobechat/model-runtime';
import { ChatErrorType } from '@lobechat/types';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import { validateOIDCJWT } from '@/libs/oidc-provider/jwt';
import { createErrorResponse } from '@/utils/errorResponse';
import { checkAuth, type RequestHandler } from './index';
@ -14,9 +15,15 @@ vi.mock('@lobechat/model-runtime', () => ({
}));
vi.mock('@lobechat/types', () => ({
ChatErrorType: { Unauthorized: 'Unauthorized', InternalServerError: 'InternalServerError' },
ChatErrorType: {
InternalServerError: 'InternalServerError',
Unauthorized: 'Unauthorized',
},
}));
const consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined);
const consoleInfoSpy = vi.spyOn(console, 'info').mockImplementation(() => undefined);
vi.mock('@/utils/errorResponse', () => ({
createErrorResponse: vi.fn(),
}));
@ -83,6 +90,86 @@ describe('checkAuth', () => {
expect(mockHandler).not.toHaveBeenCalled();
});
it('should return unauthorized when OIDC JWT validation throws UNAUTHORIZED', async () => {
const oidcRequest = new Request('https://example.com', {
headers: { 'Oidc-Auth': 'expired-token' },
});
const oidcError = Object.assign(new Error('JWT token validation failed'), {
code: 'UNAUTHORIZED',
});
vi.mocked(validateOIDCJWT).mockRejectedValueOnce(oidcError);
await checkAuth(mockHandler)(oidcRequest, mockOptions);
expect(createErrorResponse).toHaveBeenCalledWith(ChatErrorType.Unauthorized, {
error: oidcError,
provider: 'mock',
});
expect(consoleInfoSpy).toHaveBeenCalledWith('[auth] OIDC authentication failed', {
clientId: undefined,
code: 'UNAUTHORIZED',
path: '/',
provider: 'mock',
userAgent: null,
xClientType: null,
});
expect(mockHandler).not.toHaveBeenCalled();
});
it('should return 500 when OIDC JWKS infrastructure fails (plain Error, no UNAUTHORIZED code)', async () => {
const oidcRequest = new Request('https://example.com', {
headers: { 'Oidc-Auth': 'any-token' },
});
// Simulates getVerificationKey() throwing due to misconfigured JWKS_KEY —
// a plain Error without `code: 'UNAUTHORIZED'` must bubble up as 500,
// not 401, so ops gets paged instead of the client being asked to re-auth.
const infraError = new Error('JWKS_KEY public key retrieval failed: invalid JWK');
vi.mocked(validateOIDCJWT).mockRejectedValueOnce(infraError);
await checkAuth(mockHandler)(oidcRequest, mockOptions);
expect(createErrorResponse).toHaveBeenCalledWith(ChatErrorType.InternalServerError, {
error: infraError,
provider: 'mock',
});
expect(mockHandler).not.toHaveBeenCalled();
});
it('should log decoded OIDC client info when auth fails with OIDC header', async () => {
const payload = Buffer.from(
JSON.stringify({ client_id: 'lobehub-desktop', sub: 'user-123' }),
'utf8',
).toString('base64url');
const oidcRequest = new Request('https://example.com/webapi/chat/lobehub', {
headers: {
'Oidc-Auth': `header.${payload}.signature`,
'user-agent': 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)',
'x-client-type': 'desktop',
},
});
const oidcError = Object.assign(new Error('JWT token validation failed'), {
code: 'UNAUTHORIZED',
});
vi.mocked(validateOIDCJWT).mockRejectedValueOnce(oidcError);
await checkAuth(mockHandler)(oidcRequest, mockOptions);
expect(consoleInfoSpy).toHaveBeenCalledWith('[auth] OIDC authentication failed', {
clientId: 'lobehub-desktop',
code: 'UNAUTHORIZED',
path: '/webapi/chat/lobehub',
provider: 'mock',
userAgent: 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7)',
xClientType: 'desktop',
});
});
it('should not log OIDC auth info for better-auth session failures', async () => {
await checkAuth(mockHandler)(mockRequest, mockOptions);
expect(consoleInfoSpy).not.toHaveBeenCalled();
});
describe('mock dev user', () => {
it('should use MOCK_DEV_USER_ID when ENABLE_MOCK_DEV_USER is enabled', async () => {
vi.stubEnv('NODE_ENV', 'development');

View file

@ -23,6 +23,40 @@ export type RequestHandler = (
},
) => Promise<Response>;
interface OIDCClientDebugInfo {
clientId?: string;
payload?: Record<string, unknown>;
}
const isUnauthorizedAuthError = (error: unknown) => {
return !!error && typeof error === 'object' && 'code' in error && error.code === 'UNAUTHORIZED';
};
/**
* Decode JWT payload for debugging only.
* The decoded payload must never be trusted for authorization decisions.
*/
const getOIDCClientDebugInfo = (token?: string | null): OIDCClientDebugInfo => {
if (!token) return {};
const [, payload] = token.split('.');
if (!payload) return {};
try {
const normalizedPayload = payload.replaceAll('-', '+').replaceAll('_', '/');
const decodedPayload = JSON.parse(Buffer.from(normalizedPayload, 'base64').toString('utf8')) as
| Record<string, unknown>
| undefined;
const clientId =
typeof decodedPayload?.client_id === 'string' ? decodedPayload.client_id : undefined;
return { clientId, payload: decodedPayload };
} catch {
return {};
}
};
export const checkAuth =
(handler: RequestHandler) => async (req: Request, options: RequestOptions) => {
// Clone the request to avoid "Response body object should not be disturbed or locked" error
@ -68,11 +102,31 @@ export const checkAuth =
}
} catch (e) {
const params = await options.params;
const oidcAuthorization = req.headers.get(LOBE_CHAT_OIDC_AUTH_HEADER);
// Only log OIDC auth failures — better-auth session failures are a common
// baseline (unauthenticated browser hits) and would otherwise flood logs.
if (oidcAuthorization) {
const oidcDebugInfo = getOIDCClientDebugInfo(oidcAuthorization);
console.info('[auth] OIDC authentication failed', {
clientId: oidcDebugInfo.clientId,
code: (e as { code?: string })?.code,
path: new URL(req.url).pathname,
provider: params?.provider,
userAgent: req.headers.get('user-agent'),
xClientType: req.headers.get('x-client-type'),
});
}
// if the error is not a ChatCompletionErrorPayload, it means the application error
if (!(e as ChatCompletionErrorPayload).errorType) {
if ((e as any).code === 'ERR_JWT_EXPIRED')
return createErrorResponse(ChatErrorType.SystemTimeNotMatchError, e);
if (isUnauthorizedAuthError(e)) {
return createErrorResponse(ChatErrorType.Unauthorized, {
error: e,
provider: params?.provider,
});
}
// other issue will be internal server error
console.error(e);

View file

@ -0,0 +1,60 @@
import { TRPCError } from '@trpc/server';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock('@/envs/auth', () => ({
authEnv: {
JWKS_KEY: JSON.stringify({
keys: [
{
alg: 'RS256',
e: 'AQAB',
kid: 'test-key',
kty: 'RSA',
n: 'test-modulus',
use: 'sig',
},
],
}),
},
}));
const importJWKMock = vi.fn();
const jwtVerifyMock = vi.fn();
vi.mock('jose', () => ({
importJWK: (...args: unknown[]) => importJWKMock(...args),
jwtVerify: (...args: unknown[]) => jwtVerifyMock(...args),
}));
describe('validateOIDCJWT', () => {
beforeEach(() => {
vi.clearAllMocks();
importJWKMock.mockResolvedValue('public-key');
});
it('should preserve the original jose error as TRPCError cause', async () => {
const joseError = Object.assign(new Error('"exp" claim timestamp check failed'), {
code: 'ERR_JWT_EXPIRED',
});
jwtVerifyMock.mockRejectedValueOnce(joseError);
const { validateOIDCJWT } = await import('./jwt');
await expect(validateOIDCJWT('header.payload.signature')).rejects.toMatchObject({
cause: joseError,
code: 'UNAUTHORIZED',
});
});
it('should not wrap JWKS/public key retrieval failures as TRPCError', async () => {
importJWKMock.mockRejectedValueOnce(new Error('invalid JWK'));
const { validateOIDCJWT } = await import('./jwt');
const error = await validateOIDCJWT('header.payload.signature').catch((error_) => error_);
expect(error).toBeInstanceOf(Error);
expect(error).not.toBeInstanceOf(TRPCError);
expect((error as Error).message).toBe('JWKS_KEY public key retrieval failed: invalid JWK');
});
});

View file

@ -101,22 +101,23 @@ const getVerificationKey = async () => {
* @returns Parsed token payload and user information
*/
export const validateOIDCJWT = async (token: string) => {
log('Starting OIDC JWT token validation');
// JWKS / signing key retrieval is an infrastructure concern (misconfigured
// env, malformed JWKS, key import failure). Let these errors propagate as
// plain Error so upstream middleware maps them to 500 and triggers ops
// alerts — treating them as 401 would incorrectly ask clients to re-auth
// while the real problem is server-side.
const publicKey = await getVerificationKey();
try {
log('Starting OIDC JWT token validation');
// Get public key
const publicKey = await getVerificationKey();
// Verify JWT
const { jwtVerify } = await import('jose');
const { payload } = await jwtVerify(token, publicKey, {
algorithms: ['RS256'],
// Additional validation options can be added, such as issuer, audience, etc.
});
log('JWT validation successful, payload: %O', payload);
// Extract user information
const userId = payload.sub;
const clientId = payload.client_id;
const aud = payload.aud;
@ -149,7 +150,10 @@ export const validateOIDCJWT = async (token: string) => {
log('JWT validation failed: %O', error);
// Preserve the original jose error via `cause` so upstream middleware
// can still inspect specific codes like `ERR_JWT_EXPIRED`.
throw new TRPCError({
cause: error,
code: 'UNAUTHORIZED',
message: `JWT token validation failed: ${(error as Error).message}`,
});