mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 10:02:41 +00:00
🪠 fix: Unblock Failed OpenID Publication Flights (#15641)
Some checks are pending
Backend Unit Tests / Build packages (push) Waiting to run
Backend Unit Tests / Codegraph select (push) Waiting to run
Backend Unit Tests / TypeScript type checks (push) Blocked by required conditions
Backend Unit Tests / Circular dependency checks (push) Waiting to run
Backend Unit Tests / Tests: api (shard 1/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 2/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 3/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: data-provider (push) Blocked by required conditions
Backend Unit Tests / Tests: data-schemas (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 1/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 2/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 3/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 4/4) (push) Blocked by required conditions
Codegraph E2E Votes / vote (full suite) (push) Waiting to run
Docker Dev Branch Images Build / publish (push) Waiting to run
Frontend Unit Tests / Codegraph select (push) Waiting to run
Frontend Unit Tests / Build packages (push) Waiting to run
Frontend Unit Tests / TypeScript type checks (client) (push) Blocked by required conditions
Frontend Unit Tests / Tests: @librechat/client (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 1/2) (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 2/2) (push) Blocked by required conditions
Frontend Unit Tests / Vite build verification (push) Blocked by required conditions
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
Some checks are pending
Backend Unit Tests / Build packages (push) Waiting to run
Backend Unit Tests / Codegraph select (push) Waiting to run
Backend Unit Tests / TypeScript type checks (push) Blocked by required conditions
Backend Unit Tests / Circular dependency checks (push) Waiting to run
Backend Unit Tests / Tests: api (shard 1/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 2/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: api (shard 3/3) (push) Blocked by required conditions
Backend Unit Tests / Tests: data-provider (push) Blocked by required conditions
Backend Unit Tests / Tests: data-schemas (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 1/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 2/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 3/4) (push) Blocked by required conditions
Backend Unit Tests / Tests: @librechat/api (shard 4/4) (push) Blocked by required conditions
Codegraph E2E Votes / vote (full suite) (push) Waiting to run
Docker Dev Branch Images Build / publish (push) Waiting to run
Frontend Unit Tests / Codegraph select (push) Waiting to run
Frontend Unit Tests / Build packages (push) Waiting to run
Frontend Unit Tests / TypeScript type checks (client) (push) Blocked by required conditions
Frontend Unit Tests / Tests: @librechat/client (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 1/2) (push) Blocked by required conditions
Frontend Unit Tests / Tests: Ubuntu (shard 2/2) (push) Blocked by required conditions
Frontend Unit Tests / Vite build verification (push) Blocked by required conditions
Sync Locize Translations & Create Translation PR / Sync Translation Keys with Locize (push) Waiting to run
Sync Locize Translations & Create Translation PR / Create Translation PR on Version Published (push) Blocked by required conditions
* fix: settle failed OpenID authentication publications * test: simplify recovery dependency assertion * test: provide a complete typed recovery fixture * fix: distinguish preparation errors from completion dispatch
This commit is contained in:
parent
aeac7a2065
commit
b3b8986752
4 changed files with 187 additions and 1 deletions
53
packages/api/src/auth/openid/flight.spec.ts
Normal file
53
packages/api/src/auth/openid/flight.spec.ts
Normal file
|
|
@ -0,0 +1,53 @@
|
|||
import { createOpenIDRefreshFlightService } from './flight';
|
||||
|
||||
jest.mock('../../utils/identity', () => ({
|
||||
createOpenIDRefreshIdentityTuple: jest.fn(),
|
||||
serializeAuthIdentityTuple: jest.fn(),
|
||||
}));
|
||||
|
||||
describe('OpenID completion write boundary', () => {
|
||||
it.each([true, false])(
|
||||
'marks dispatch only after encryption (encryption fails: %s)',
|
||||
async (fails) => {
|
||||
const onWriteStart = jest.fn();
|
||||
const error = new Error('encryption failed');
|
||||
const complete = jest.fn(async () => {
|
||||
expect(onWriteStart).toHaveBeenCalledTimes(1);
|
||||
return null;
|
||||
});
|
||||
const service = createOpenIDRefreshFlightService({
|
||||
db: {
|
||||
acquireOpenIDRefreshFlight: jest.fn(),
|
||||
completeOpenIDRefreshFlight: complete,
|
||||
renewOpenIDRefreshFlight: jest.fn(),
|
||||
failOpenIDRefreshFlight: jest.fn(),
|
||||
revokeOpenIDRefreshFlight: jest.fn(),
|
||||
findOpenIDRefreshFlight: jest.fn(),
|
||||
claimOpenIDRefreshFlightDelivery: jest.fn(),
|
||||
releaseOpenIDRefreshFlightDelivery: jest.fn(),
|
||||
},
|
||||
logger: { warn: jest.fn() },
|
||||
encrypt: async () => {
|
||||
expect(onWriteStart).not.toHaveBeenCalled();
|
||||
if (fails) throw error;
|
||||
return 'encrypted';
|
||||
},
|
||||
decrypt: jest.fn(),
|
||||
});
|
||||
const result = service.completeOpenIDRefreshFlight({
|
||||
key: 'publication',
|
||||
ownerId: 'owner',
|
||||
tokens: { access_token: 'access' },
|
||||
onWriteStart,
|
||||
});
|
||||
if (fails) {
|
||||
await expect(result).rejects.toBe(error);
|
||||
expect(onWriteStart).not.toHaveBeenCalled();
|
||||
expect(complete).not.toHaveBeenCalled();
|
||||
} else {
|
||||
await expect(result).resolves.toBeNull();
|
||||
expect(complete).toHaveBeenCalledTimes(1);
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
|
@ -88,6 +88,7 @@ export interface OpenIDRefreshFlightService {
|
|||
ownerId?: string;
|
||||
tokens?: TokenResult | null;
|
||||
ttl?: number;
|
||||
onWriteStart?: () => void;
|
||||
}) => Promise<RefreshFlightRecord | null>;
|
||||
createOpenIDRefreshFlightKey: (input: RefreshKeyInput) => string | null;
|
||||
failOpenIDRefreshFlight: (args: {
|
||||
|
|
@ -231,11 +232,13 @@ export function createOpenIDRefreshFlightService({
|
|||
ownerId,
|
||||
tokens,
|
||||
ttl = DEFAULT_FLIGHT_TTL_MS,
|
||||
onWriteStart,
|
||||
}: {
|
||||
key?: string | null;
|
||||
ownerId?: string;
|
||||
tokens?: TokenResult | null;
|
||||
ttl?: number;
|
||||
onWriteStart?: () => void;
|
||||
}): Promise<RefreshFlightRecord | null> {
|
||||
if (!key || !ownerId || !tokens) return null;
|
||||
const serializedTokens: TokenResult = { ...tokens };
|
||||
|
|
@ -255,10 +258,12 @@ export function createOpenIDRefreshFlightService({
|
|||
const usableTokenTtl = Number.isFinite(accessTokenExpiresAt)
|
||||
? Math.max(1, accessTokenExpiresAt - Date.now() - OPENID_EXPIRY_BUFFER_SECONDS * 1000)
|
||||
: ttl;
|
||||
const encryptedResult = await encrypt(JSON.stringify(serializedTokens));
|
||||
onWriteStart?.();
|
||||
return db.completeOpenIDRefreshFlight({
|
||||
key,
|
||||
ownerId,
|
||||
encryptedResult: await encrypt(JSON.stringify(serializedTokens)),
|
||||
encryptedResult,
|
||||
expiresAt: new Date(Date.now() + Math.min(ttl, usableTokenTtl)),
|
||||
});
|
||||
}
|
||||
|
|
|
|||
107
packages/api/src/auth/openid/recovery.spec.ts
Normal file
107
packages/api/src/auth/openid/recovery.spec.ts
Normal file
|
|
@ -0,0 +1,107 @@
|
|||
import type { OpenIDRefreshRecoveryDeps } from './recovery';
|
||||
import { createOpenIDRefreshRecoveryService } from './recovery';
|
||||
|
||||
describe('OpenID authentication publication settlement', () => {
|
||||
function setup() {
|
||||
const deps = {
|
||||
jwt: { decode: jest.fn() },
|
||||
findOpenIDUser: jest.fn(),
|
||||
findUser: jest.fn(),
|
||||
getOpenIdConfig: jest.fn(),
|
||||
getOpenIdEmail: jest.fn(),
|
||||
getOpenIdIssuer: jest.fn(),
|
||||
createAuthIdentityContext: jest.fn(),
|
||||
refreshOpenIDSession: jest.fn(),
|
||||
clearOpenIDAuthTokens: jest.fn(),
|
||||
deleteOpenIDSession: jest.fn(),
|
||||
createOpenIDRefreshFlightKey: jest.fn(),
|
||||
storeRefreshTokenBridge: jest.fn(),
|
||||
deleteRefreshTokenBridges: jest.fn(),
|
||||
waitForOpenIDRefreshFlight: jest.fn(),
|
||||
assertOpenIDRefreshSessionGenerationAvailable: jest.fn(),
|
||||
revokeOpenIDRefreshFlights: jest.fn(),
|
||||
bridgeGraceMs: 1000,
|
||||
logger: { debug: jest.fn(), warn: jest.fn() },
|
||||
createRefreshTokenBridgeFlightKey: jest.fn(() => 'publication'),
|
||||
acquireOpenIDRefreshFlight: jest.fn().mockResolvedValue({
|
||||
acquired: true,
|
||||
ownerId: 'owner',
|
||||
}),
|
||||
withOpenIDRefreshFlightLease: jest.fn(({ operation }) =>
|
||||
operation({
|
||||
assertLeaseOwned: jest.fn().mockResolvedValue(true),
|
||||
markLeaseSettled: jest.fn(),
|
||||
}),
|
||||
),
|
||||
failOpenIDRefreshFlight: jest.fn().mockResolvedValue(null),
|
||||
completeOpenIDRefreshFlight: jest.fn().mockResolvedValue({ status: 'completed' }),
|
||||
getOpenIDAppAuthToken: jest.fn(() => 'app-token'),
|
||||
storeOpenIDSession: jest.fn().mockResolvedValue(undefined),
|
||||
assertOpenIDRefreshFlightAvailable: jest.fn().mockResolvedValue(true),
|
||||
setOpenIDAuthTokens: jest.fn(() => 'app-token'),
|
||||
} satisfies OpenIDRefreshRecoveryDeps;
|
||||
const service = createOpenIDRefreshRecoveryService(deps);
|
||||
const input = {
|
||||
tokenset: { access_token: 'access', id_token: 'id', refresh_token: 'refresh' },
|
||||
user: { _id: 'user' },
|
||||
existingRefreshToken: 'refresh',
|
||||
req: {},
|
||||
res: {},
|
||||
};
|
||||
return { deps, service, input };
|
||||
}
|
||||
|
||||
it('settles a missing-session failure immediately and preserves the original error', async () => {
|
||||
const { deps, service, input } = setup();
|
||||
const error = new Error('failed to load session');
|
||||
const req = { session: { reload: (callback: (error: Error) => void) => callback(error) } };
|
||||
await expect(service.sendOpenIDAuthResponse({ ...input, req })).rejects.toBe(error);
|
||||
expect(deps.failOpenIDRefreshFlight).toHaveBeenCalledWith({
|
||||
key: 'publication',
|
||||
ownerId: 'owner',
|
||||
error,
|
||||
});
|
||||
expect(deps.completeOpenIDRefreshFlight).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('preserves the request error when failure settlement also fails', async () => {
|
||||
const { deps, service, input } = setup();
|
||||
const error = new Error('session unavailable');
|
||||
deps.getOpenIDAppAuthToken.mockImplementation(() => {
|
||||
throw error;
|
||||
});
|
||||
deps.failOpenIDRefreshFlight.mockRejectedValue(new Error('store unavailable'));
|
||||
await expect(service.sendOpenIDAuthResponse(input)).rejects.toBe(error);
|
||||
expect(deps.logger.warn).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('does not fail an indeterminate completion write', async () => {
|
||||
const { deps, service, input } = setup();
|
||||
const error = new Error('completion acknowledgement lost');
|
||||
deps.completeOpenIDRefreshFlight.mockImplementation(({ onWriteStart }) => {
|
||||
onWriteStart?.();
|
||||
return Promise.reject(error);
|
||||
});
|
||||
await expect(service.sendOpenIDAuthResponse(input)).rejects.toBe(error);
|
||||
expect(deps.failOpenIDRefreshFlight).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('settles completion preparation failures before the write starts', async () => {
|
||||
const { deps, service, input } = setup();
|
||||
const error = new Error('encryption failed');
|
||||
deps.completeOpenIDRefreshFlight.mockRejectedValue(error);
|
||||
await expect(service.sendOpenIDAuthResponse(input)).rejects.toBe(error);
|
||||
expect(deps.failOpenIDRefreshFlight).toHaveBeenCalledWith({
|
||||
key: 'publication',
|
||||
ownerId: 'owner',
|
||||
error,
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves successful publication completed', async () => {
|
||||
const { deps, service, input } = setup();
|
||||
await expect(service.sendOpenIDAuthResponse(input)).resolves.toBe('app-token');
|
||||
expect(deps.completeOpenIDRefreshFlight).toHaveBeenCalledTimes(1);
|
||||
expect(deps.failOpenIDRefreshFlight).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
|
@ -191,6 +191,7 @@ export interface OpenIDRefreshRecoveryDeps {
|
|||
key: string;
|
||||
ownerId: string;
|
||||
tokens: SharedOpenIDRefreshResult;
|
||||
onWriteStart?: () => void;
|
||||
}) => Promise<RefreshFlightRecord | null>;
|
||||
failOpenIDRefreshFlight: (args: {
|
||||
key: string;
|
||||
|
|
@ -721,6 +722,7 @@ export function createOpenIDRefreshRecoveryService(
|
|||
preparePublication: false,
|
||||
});
|
||||
}
|
||||
let completionStarted = false;
|
||||
return withOpenIDRefreshFlightLease({
|
||||
key,
|
||||
ownerId: flight.ownerId,
|
||||
|
|
@ -747,6 +749,9 @@ export function createOpenIDRefreshRecoveryService(
|
|||
const completed = await completeOpenIDRefreshFlight({
|
||||
key,
|
||||
ownerId: flight.ownerId,
|
||||
onWriteStart: () => {
|
||||
completionStarted = true;
|
||||
},
|
||||
tokens: {
|
||||
tokenset: publishedTokenset,
|
||||
claims: { sub: openidSubject ?? user.openidId ?? userId },
|
||||
|
|
@ -764,6 +769,22 @@ export function createOpenIDRefreshRecoveryService(
|
|||
markLeaseSettled();
|
||||
},
|
||||
}),
|
||||
}).catch(async (error) => {
|
||||
/** A completion write may have succeeded despite a lost acknowledgement. */
|
||||
if (!completionStarted) {
|
||||
try {
|
||||
await failOpenIDRefreshFlight({
|
||||
key,
|
||||
ownerId: flight.ownerId,
|
||||
error: error instanceof Error ? error : new Error('OpenID publication failed'),
|
||||
});
|
||||
} catch (flightError) {
|
||||
logger.warn('[refreshController] Failed to settle authentication publication', {
|
||||
error: toOpenIDLogArgument(flightError),
|
||||
});
|
||||
}
|
||||
}
|
||||
throw error;
|
||||
});
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue