diff --git a/api/server/experimental.js b/api/server/experimental.js index 2ad6e559fa0..00d9c6c83e3 100644 --- a/api/server/experimental.js +++ b/api/server/experimental.js @@ -641,6 +641,7 @@ if (cluster.isMaster) { app.use('/api/search', routes.search); app.use('/api/messages', routes.messages); app.use('/api/convos', routes.convos); + app.use('/api/pull-requests', preAuthTenantMiddleware, routes.pullRequests); app.use('/api/presets', routes.presets); app.use('/api/projects', routes.projects); app.use('/api/prompts', routes.prompts); diff --git a/api/server/index.js b/api/server/index.js index 9d28fc7a17a..1a97a81d010 100644 --- a/api/server/index.js +++ b/api/server/index.js @@ -421,6 +421,7 @@ const startServer = async () => { app.use('/api/search', routes.search); app.use('/api/messages', routes.messages); app.use('/api/convos', routes.convos); + app.use('/api/pull-requests', preAuthTenantMiddleware, routes.pullRequests); app.use('/api/traces', routes.traces); app.use('/api/presets', routes.presets); app.use('/api/projects', routes.projects); diff --git a/api/server/routes/__test-utils__/convos-route-mocks.js b/api/server/routes/__test-utils__/convos-route-mocks.js index 06eae310d8c..231e6db8a79 100644 --- a/api/server/routes/__test-utils__/convos-route-mocks.js +++ b/api/server/routes/__test-utils__/convos-route-mocks.js @@ -151,6 +151,7 @@ module.exports = { createProxyAwareFetch: jest.fn(() => jest.fn()), createPullRequestLookup: jest.fn(() => jest.fn()), createPullRequestLookupStore: jest.fn(() => undefined), + createPullRequestUserTokenFromEnv: jest.fn(() => undefined), createConversationPullRequestsHandler: jest.fn(() => jest.fn()), createConversationPullRequestHandler: jest.fn( () => (_req, res) => res.status(200).json({ pullRequest: null }), diff --git a/api/server/routes/admin/users.js b/api/server/routes/admin/users.js index bbad3a35825..2823abd9f5a 100644 --- a/api/server/routes/admin/users.js +++ b/api/server/routes/admin/users.js @@ -40,6 +40,7 @@ const handlers = createAdminUsersHandlers({ deleteConfig: db.deleteConfig, deleteAclEntries: db.deleteAclEntries, deletePasskeysByUser: db.deletePasskeysByUser, + deleteTokens: db.deleteTokens, }); router.use(requireJwtAuth, requireAdminAccess); diff --git a/api/server/routes/convos.js b/api/server/routes/convos.js index 4ec0007c4e1..25d71a37576 100644 --- a/api/server/routes/convos.js +++ b/api/server/routes/convos.js @@ -26,6 +26,7 @@ const { createPullRequestLookupStore, keyvRedisClient, cacheConfig, + createPullRequestUserTokenFromEnv, createConversationPullRequestsHandler, backgroundTaskRegistry, createSubagentThreadViewHandler, @@ -180,17 +181,24 @@ const pullRequestLookup = createPullRequestLookup({ source: createPullRequestSource({ fetchFn: createProxyAwareFetch() }), store: createPullRequestLookupStore({ client: keyvRedisClient, config: cacheConfig }), }); +const pullRequestUserToken = createPullRequestUserTokenFromEnv({ + store: db, + fetchFn: createProxyAwareFetch(), + env: process.env, +}); const conversationPullRequestHandler = createConversationPullRequestHandler({ getConvoLaneGit: db.getConvoLaneGit, getAppConfig, lookup: pullRequestLookup, env: process.env, + userToken: pullRequestUserToken, }); const conversationPullRequestsHandler = createConversationPullRequestsHandler({ getConvosLaneGit: db.getConvosLaneGit, getAppConfig, lookup: pullRequestLookup, env: process.env, + userToken: pullRequestUserToken, }); router.use(requireJwtAuth); diff --git a/api/server/routes/index.js b/api/server/routes/index.js index 14afa6f2691..d0d678ab2d8 100644 --- a/api/server/routes/index.js +++ b/api/server/routes/index.js @@ -29,6 +29,7 @@ const openapi = require('./openapi'); const search = require('./search'); const models = require('./models'); const convos = require('./convos'); +const pullRequests = require('./pullRequests'); const traces = require('./traces'); const config = require('./config'); const agents = require('./agents'); @@ -45,6 +46,7 @@ const rum = require('./rum'); const insights = require('./insights'); module.exports = { + pullRequests, insights, rum, mcp, diff --git a/api/server/routes/pullRequests.js b/api/server/routes/pullRequests.js new file mode 100644 index 00000000000..efdac11565c --- /dev/null +++ b/api/server/routes/pullRequests.js @@ -0,0 +1,17 @@ +const { createProxyAwareFetch, createPullRequestConnectionRouter } = require('@librechat/api'); +const requireJwtAuth = require('~/server/middleware/requireJwtAuth'); +const { markOAuthNavigation } = require('~/server/middleware/oauthNavigation'); +const { loginLimiter } = require('~/server/middleware/limiters'); +const { getAppConfig } = require('~/server/services/Config'); +const db = require('~/models'); + +module.exports = createPullRequestConnectionRouter({ + store: db, + fetchFn: createProxyAwareFetch(), + env: process.env, + getAppConfig, + getUserById: db.getUserById, + requireJwtAuth, + markNavigation: markOAuthNavigation, + callbackLimiter: loginLimiter, +}); diff --git a/librechat.example.yaml b/librechat.example.yaml index 2a381add6a2..04389625ba3 100644 --- a/librechat.example.yaml +++ b/librechat.example.yaml @@ -950,6 +950,19 @@ endpoints: # # host: "github.example.com" # # Environment variable reference only, never the token itself. Optional, see above. # token: "${GITHUB_PULL_REQUEST_TOKEN}" + # # Let each user connect their own account, so lookups use their token, rate limit and + # # repository access instead of the server's (read from this file only). Register + # # /api/pull-requests/oauth/callback as the app's callback URL. On GitHub a + # # GitHub App with read-only Pull requests, Checks and Contents permissions is recommended. + # # Changing clientId, provider or host asks users to connect again; a changed scope only + # # applies to new connections. Disconnecting removes every grant of the user. + # # oauth: + # # clientId: "${GITHUB_PR_OAUTH_CLIENT_ID}" + # # clientSecret: "${GITHUB_PR_OAUTH_CLIENT_SECRET}" + # # scope: "repo" # only for a classic OAuth app; GitLab defaults to read_api + # # flowTimeoutMinutes: 10 # time to approve at the provider + # # requestTimeoutSeconds: 10 # per code exchange or token refresh + # # retentionDays: 180 # an unused connection is removed after this; each use extends it # # Repositories the token may be used for, as owner/name or owner/*. A worker reports its # # own repository, and only these are ever looked up with the server's token. # allowedRepositories: ["LibreChat-AI/LibreChat"] diff --git a/packages/api/src/admin/users.spec.ts b/packages/api/src/admin/users.spec.ts index acf6a50f470..1b969cfef66 100644 --- a/packages/api/src/admin/users.spec.ts +++ b/packages/api/src/admin/users.spec.ts @@ -68,6 +68,7 @@ function createDeps(overrides: Partial = {}): AdminUsersDeps { deleteConfig: jest.fn().mockResolvedValue(null), deleteAclEntries: jest.fn().mockResolvedValue(undefined), deletePasskeysByUser: jest.fn().mockResolvedValue(0), + deleteTokens: jest.fn().mockResolvedValue({ deletedCount: 0 }), ...overrides, }; } @@ -461,6 +462,34 @@ describe('createAdminUsersHandlers', () => { expect(deps.deletePasskeysByUser).toHaveBeenCalledWith(validUserId); }); + it("removes the user's stored credentials before the user", async () => { + const deps = createDeps(); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(200); + expect(deps.deleteTokens).toHaveBeenCalledWith({ userId: validUserId }); + expect(jest.mocked(deps.deleteTokens).mock.invocationCallOrder[0]).toBeLessThan( + jest.mocked(deps.deleteUserById).mock.invocationCallOrder[0], + ); + }); + + it('keeps the user, and releases the fence, when its credentials cannot be removed', async () => { + const deps = createDeps({ + deleteTokens: jest.fn().mockRejectedValue(new Error('mongo down')), + }); + const handlers = createAdminUsersHandlers(deps); + const { req, res, status } = createReqRes({ params: { id: validUserId } }); + + await handlers.deleteUser(req, res); + + expect(status).toHaveBeenCalledWith(500); + expect(deps.deleteUserById).not.toHaveBeenCalled(); + expect(deps.cancelAgentTriggerUserDeletion).toHaveBeenCalled(); + }); + it('returns success even when cascade cleanup partially fails', async () => { const result: UserDeleteResult = { deletedCount: 1, diff --git a/packages/api/src/admin/users.ts b/packages/api/src/admin/users.ts index e612a6d71da..e6a1e5f431c 100644 --- a/packages/api/src/admin/users.ts +++ b/packages/api/src/admin/users.ts @@ -62,6 +62,12 @@ export interface AdminUsersDeps { * keep authentication material and can collide with a later registration. */ deletePasskeysByUser: (userId: string) => Promise; + /** + * Stored credentials the user holds, such as a connected account's OAuth grant. Removed before + * the user is, as the self-delete flow does, so a failure leaves the account to retry rather + * than its credentials behind. + */ + deleteTokens: (query: { userId: string }) => Promise; } export function createAdminUsersHandlers(deps: AdminUsersDeps): { @@ -85,6 +91,7 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): { deleteConfig, deleteAclEntries, deletePasskeysByUser, + deleteTokens, } = deps; async function listUsersHandler(req: ServerRequest, res: Response) { @@ -202,6 +209,7 @@ export function createAdminUsersHandlers(deps: AdminUsersDeps): { await prepareAgentTriggerUserPurge(id, triggerDeletionFence, targetUser?.tenantId); await drainAgentTriggerDeliveriesForUser(id); + await deleteTokens({ userId: id }); const result = await deleteUserById(id); if (result.deletedCount === 0) { diff --git a/packages/api/src/oauth/csrf.ts b/packages/api/src/oauth/csrf.ts index 1ad1ad64432..0d30a45c1bd 100644 --- a/packages/api/src/oauth/csrf.ts +++ b/packages/api/src/oauth/csrf.ts @@ -148,13 +148,19 @@ function getOAuthCookiePath(cookiePath: string): string { } } -/** Sets a SameSite=Lax CSRF cookie bound to a specific OAuth flow */ -export function setOAuthCsrfCookie(res: Response, flowId: string, cookiePath: string): void { +/** Sets a SameSite=Lax CSRF cookie bound to a specific OAuth flow. `maxAge` follows the flow's own + * approval window when it has one, so the cookie lives as long as the state it binds. */ +export function setOAuthCsrfCookie( + res: Response, + flowId: string, + cookiePath: string, + maxAge: number = OAUTH_CSRF_MAX_AGE, +): void { res.cookie(OAUTH_CSRF_COOKIE, generateOAuthCsrfToken(flowId), { httpOnly: true, secure: shouldUseSecureCookie(), sameSite: 'lax', - maxAge: OAUTH_CSRF_MAX_AGE, + maxAge, path: getOAuthCookiePath(cookiePath), }); } diff --git a/packages/api/src/pulls/bitbucket.spec.ts b/packages/api/src/pulls/bitbucket.spec.ts index 5ab6fbd3984..23e9920fe35 100644 --- a/packages/api/src/pulls/bitbucket.spec.ts +++ b/packages/api/src/pulls/bitbucket.spec.ts @@ -222,6 +222,11 @@ describe('createBitbucketPullRequestSource', () => { expect(urlsOf(fetchFn)[0]).not.toContain('"'); }); + it('maps a 401 to CREDENTIAL_REJECTED, a refusal of the token itself', async () => { + const { source } = sourceFor({ 'state=OPEN': () => new Response('', { status: 401 }) }); + await expect(find(source)).rejects.toMatchObject({ code: 'CREDENTIAL_REJECTED' }); + }); + it.each([ ['another host', 'https://evil.example/w/r/pull-requests/7'], ['plain http', 'http://bitbucket.org/w/r/pull-requests/7'], diff --git a/packages/api/src/pulls/bitbucket.ts b/packages/api/src/pulls/bitbucket.ts index ad8743f9ec1..bca611cc10c 100644 --- a/packages/api/src/pulls/bitbucket.ts +++ b/packages/api/src/pulls/bitbucket.ts @@ -221,6 +221,8 @@ export function createBitbucketPullRequestSource({ } /** A repository the token cannot see is indistinguishable from one without pull requests. */ if (response.status === 404 || absent.includes(response.status)) return null; + /** The provider no longer accepts the token: revoked, or expired without a refresh. */ + if (response.status === 401) throw new PullRequestSourceError('CREDENTIAL_REJECTED'); throw new PullRequestSourceError('UPSTREAM_ERROR'); } diff --git a/packages/api/src/pulls/capabilities.spec.ts b/packages/api/src/pulls/capabilities.spec.ts index 7c93e93b2cc..f4f7085ab2c 100644 --- a/packages/api/src/pulls/capabilities.spec.ts +++ b/packages/api/src/pulls/capabilities.spec.ts @@ -17,6 +17,19 @@ describe('resolvePullRequestCapabilities', () => { }); }); + it('advertises the provider users can connect to, only when an app is configured', () => { + const oauth = { clientId: '${ID}', clientSecret: '${SECRET}' }; + expect( + resolvePullRequestCapabilities(configWith({ provider: 'gitlab', oauth }), { + ID: 'i', + SECRET: 's', + }), + ).toMatchObject({ pullRequestsEnabled: true, pullRequestsConnect: 'gitlab' }); + expect(resolvePullRequestCapabilities(configWith(scoped), env)).not.toHaveProperty( + 'pullRequestsConnect', + ); + }); + it('advertises the configured lookup limit', () => { expect( resolvePullRequestCapabilities(configWith({ ...scoped, maxConcurrentLookups: 2 }), env), diff --git a/packages/api/src/pulls/capabilities.ts b/packages/api/src/pulls/capabilities.ts index 9f481c3bf61..aa0e7385250 100644 --- a/packages/api/src/pulls/capabilities.ts +++ b/packages/api/src/pulls/capabilities.ts @@ -1,7 +1,7 @@ import { EModelEndpoint, PULL_REQUEST_BATCH_VERSION } from 'librechat-data-provider'; -import type { TAgentsEndpoint } from 'librechat-data-provider'; +import type { PullRequestProvider, TAgentsEndpoint } from 'librechat-data-provider'; import type { AppConfig } from '@librechat/data-schemas'; -import { isPullRequestFeatureActive } from './settings'; +import { isPullRequestFeatureActive, resolvePullRequestOAuth } from './settings'; export type PullRequestCapabilities = { pullRequestsEnabled: boolean; @@ -10,6 +10,8 @@ export type PullRequestCapabilities = { /** The lookups one batch runs at once, so a client that falls back to the single route can * keep to the same limit while a replica without the batch route is still serving it. */ pullRequestsMaxConcurrentLookups?: number; + /** The provider users can connect their own account to, when the operator configured one. */ + pullRequestsConnect?: PullRequestProvider; }; /** @@ -23,12 +25,16 @@ export function resolvePullRequestCapabilities( ): PullRequestCapabilities { const agents = appConfig?.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined; const enabled = isPullRequestFeatureActive(agents?.pullRequests, env); + const connect = enabled + ? resolvePullRequestOAuth(agents?.pullRequests, env)?.provider + : undefined; return { pullRequestsEnabled: enabled, ...(enabled ? { pullRequestsBatchVersion: PULL_REQUEST_BATCH_VERSION, pullRequestsMaxConcurrentLookups: agents?.pullRequests?.maxConcurrentLookups ?? 4, + ...(connect != null ? { pullRequestsConnect: connect } : {}), } : {}), }; diff --git a/packages/api/src/pulls/connect.spec.ts b/packages/api/src/pulls/connect.spec.ts new file mode 100644 index 00000000000..fc440d911d3 --- /dev/null +++ b/packages/api/src/pulls/connect.spec.ts @@ -0,0 +1,435 @@ +import { getTenantId, tenantStorage } from '@librechat/data-schemas'; +import type { Request, Response } from 'express'; +import type { PullRequestOAuth, PullRequestOAuthState } from './oauth'; +import type { ServerRequest } from '~/types'; +import { + createPullRequestUserToken, + createPullRequestConnectionRouter, + createPullRequestConnectionHandlers, +} from './connect'; +import { generateOAuthCsrfToken, OAUTH_CSRF_COOKIE, OAUTH_SESSION_COOKIE } from '~/oauth/csrf'; + +const env = { + JWT_SECRET: 'jwt-secret', + GH_CLIENT_ID: 'client-1', + GH_CLIENT_SECRET: 'client-secret', +}; +const oauthSettings = { + allowedRepositories: ['o/r'], + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, +}; +const configWith = (pullRequests?: Record) => ({ + endpoints: { agents: { pullRequests } }, +}); +const claims: PullRequestOAuthState = { + userId: 'u1', + provider: 'github', + host: 'github.com', + nonce: 'n', + flowId: 'pull-request-oauth:u1:n', + generation: 'initial', +}; +/** How a page that started this flow is told it is the one that finished. */ +const flowParam = `&flow=${encodeURIComponent(claims.flowId)}`; + +function makeOAuth(overrides: Partial = {}): jest.Mocked { + return { + authorizationUrl: jest.fn(() => ({ + url: 'https://github.com/login/oauth/authorize?x=1', + flowId: claims.flowId, + })), + verifyState: jest.fn(() => claims), + complete: jest.fn(async () => undefined), + token: jest.fn(async () => 'user-token'), + connected: jest.fn(async () => true), + status: jest.fn(async () => ({ connected: true, flowId: 'pull-request-oauth:u1:n' })), + storedStatus: jest.fn(async () => ({ connected: false })), + reject: jest.fn(async () => undefined), + disconnect: jest.fn(async () => undefined), + ...overrides, + } as jest.Mocked; +} + +function setup({ + settings = oauthSettings, + base = settings, + oauth = makeOAuth(), + user = { role: 'USER' }, +}: { + settings?: Record | undefined; + base?: Record | undefined; + oauth?: jest.Mocked; + /** The stored user a callback's state names; null when it no longer exists. */ + user?: { role?: string; tenantId?: string } | null; +} = {}) { + const getAppConfig = jest + .fn() + .mockImplementation(async (options: { baseOnly?: boolean }) => + configWith(options.baseOnly ? base : settings), + ); + const getUser = jest.fn(async (id: string) => (user == null ? null : { id, ...user })); + const handlers = createPullRequestConnectionHandlers({ oauth, getAppConfig, getUser, env }); + const res = { + status: jest.fn().mockReturnThis(), + json: jest.fn().mockReturnThis(), + end: jest.fn().mockReturnThis(), + cookie: jest.fn().mockReturnThis(), + clearCookie: jest.fn().mockReturnThis(), + redirect: jest.fn(), + }; + return { handlers, oauth, getAppConfig, getUser, res }; +} + +const asRes = (res: unknown) => res as Response; +const signedIn = (user: Record | null = { id: 'u1' }) => + ({ user, params: {}, query: {} }) as unknown as ServerRequest; +const callbackRequest = (query: Record, cookies: Record = {}) => + ({ query, cookies }) as unknown as Request; +const bound = () => ({ + [OAUTH_CSRF_COOKIE]: generateOAuthCsrfToken(claims.flowId, env.JWT_SECRET), +}); + +beforeAll(() => { + process.env.JWT_SECRET = env.JWT_SECRET; +}); + +describe('connect', () => { + it('answers the provider page and binds the flow to this browser with a cookie', async () => { + const { handlers, oauth, res } = setup(); + await handlers.connect(signedIn(), asRes(res)); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ + url: 'https://github.com/login/oauth/authorize?x=1', + expiresInSeconds: 600, + flowId: claims.flowId, + }); + expect(oauth.authorizationUrl).toHaveBeenCalledWith( + { id: 'u1' }, + expect.objectContaining({ provider: 'github', clientId: 'client-1' }), + ); + expect(res.cookie).toHaveBeenCalledWith( + OAUTH_CSRF_COOKIE, + generateOAuthCsrfToken(claims.flowId, env.JWT_SECRET), + expect.objectContaining({ httpOnly: true, path: '/api/pull-requests/oauth/callback' }), + ); + }); + + it.each([ + ['the default flow timeout', {}, 10 * 60_000], + ['a configured flow timeout', { flowTimeoutMinutes: 45 }, 45 * 60_000], + ])('keeps the binding cookie, and tells the client, for %s', async (_label, flow, maxAge) => { + const { handlers, res } = setup({ + settings: { ...oauthSettings, oauth: { ...oauthSettings.oauth, ...flow } }, + }); + await handlers.connect(signedIn(), asRes(res)); + expect(res.cookie).toHaveBeenCalledWith( + OAUTH_CSRF_COOKIE, + expect.any(String), + expect.objectContaining({ maxAge }), + ); + /** The client watches the sign-in for exactly as long. */ + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ expiresInSeconds: maxAge / 1000 }), + ); + }); + + it('is unavailable when no app is configured', async () => { + const { handlers, res } = setup({ settings: { allowedRepositories: ['o/r'] } }); + await handlers.connect(signedIn(), asRes(res)); + expect(res.status).toHaveBeenCalledWith(404); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'NOT_CONFIGURED' })); + }); + + it('answers a failure with a stable code, never the cause', async () => { + const { handlers, res } = setup({ + oauth: makeOAuth({ + authorizationUrl: jest.fn(() => { + throw new Error('mongodb://secret@host'); + }), + }), + }); + await handlers.connect(signedIn(), asRes(res)); + expect(res.status).toHaveBeenCalledWith(500); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'CONNECT_FAILED' })); + expect(JSON.stringify(res.json.mock.calls)).not.toContain('secret@host'); + }); + + it('takes the app from the operator config, never from a database override', async () => { + const { handlers, res } = setup({ + settings: { ...oauthSettings, oauth: { clientId: '${EVIL_ID}', clientSecret: '${EVIL}' } }, + base: { allowedRepositories: ['o/r'] }, + }); + await handlers.connect(signedIn(), asRes(res)); + expect(res.status).toHaveBeenCalledWith(404); + }); + + it('refuses a request without a user', async () => { + const { handlers, res } = setup(); + await handlers.connect(signedIn(null), asRes(res)); + expect(res.status).toHaveBeenCalledWith(401); + }); +}); + +describe('callback', () => { + const run = async ( + query: Record, + cookies: Record = bound(), + options: Parameters[0] = {}, + ) => { + const context = setup(options); + await context.handlers.callback(callbackRequest(query, cookies), asRes(context.res)); + return context; + }; + + it('stores the grant and shows the success page', async () => { + const { oauth, res } = await run({ state: 's', code: 'c' }); + expect(oauth.complete).toHaveBeenCalledWith( + claims, + 'c', + expect.objectContaining({ provider: 'github', host: 'github.com' }), + ); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/success?serverName=GitHub${flowParam}`); + }); + + it('accepts the session cookie of the same user when the flow cookie is gone', async () => { + const { oauth, res } = await run( + { state: 's', code: 'c' }, + { [OAUTH_SESSION_COOKIE]: generateOAuthCsrfToken('u1', env.JWT_SECRET) }, + ); + expect(oauth.complete).toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/success?serverName=GitHub${flowParam}`); + }); + + it.each([ + /** Before the flow is known to be this browser's, the page names no flow. */ + ['no state', { code: 'c' }, bound(), 'missing_state', ''], + ['no code', { state: 's' }, bound(), 'missing_code', flowParam], + ['a browser that did not start the flow', { state: 's', code: 'c' }, {}, 'invalid_state', ''], + [ + "another user's session", + { state: 's', code: 'c' }, + { [OAUTH_SESSION_COOKIE]: generateOAuthCsrfToken('u2', env.JWT_SECRET) }, + 'invalid_state', + '', + ], + [ + 'a refusal by the user', + { state: 's', error: 'access_denied' }, + bound(), + 'callback_failed', + flowParam, + ], + ])('fails for %s, storing nothing', async (_label, query, cookies, code, flow) => { + const { oauth, res } = await run(query, cookies); + expect(oauth.complete).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/error?error=${code}${flow}`); + }); + + it('fails for a state this server did not issue', async () => { + const { oauth, res } = await run({ state: 's', code: 'c' }, bound(), { + oauth: makeOAuth({ verifyState: jest.fn(() => null) }), + }); + expect(oauth.complete).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith('/oauth/error?error=invalid_state'); + }); + + it('fails for a flow started on a host the operator no longer configures', async () => { + const { oauth, res } = await run({ state: 's', code: 'c' }, bound(), { + base: { ...oauthSettings, host: 'ghe.example.com' }, + }); + expect(oauth.complete).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/error?error=invalid_state${flowParam}`); + }); + + it('fails without provider text when the exchange fails', async () => { + const { res } = await run({ state: 's', code: 'c' }, bound(), { + oauth: makeOAuth({ complete: jest.fn().mockRejectedValue(new Error('secret detail')) }), + }); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/error?error=callback_failed${flowParam}`); + }); + + it('refuses a callback already placed in another tenant than the flow started in', async () => { + const oauth = makeOAuth({ verifyState: jest.fn(() => ({ ...claims, tenantId: 'tenant-a' })) }); + const context = setup({ oauth }); + await tenantStorage.run({ tenantId: 'tenant-b' }, () => + context.handlers.callback( + callbackRequest({ state: 's', code: 'c' }, bound()), + asRes(context.res), + ), + ); + expect(oauth.complete).not.toHaveBeenCalled(); + expect(context.res.redirect).toHaveBeenCalledWith( + `/oauth/error?error=invalid_state${flowParam}`, + ); + }); + + it('restores the tenant the flow started in before storing', async () => { + let tenantDuringComplete: string | undefined; + const oauth = makeOAuth({ + verifyState: jest.fn(() => ({ ...claims, tenantId: 'tenant-a' })), + complete: jest.fn(async () => { + tenantDuringComplete = getTenantId(); + }), + }); + const { getAppConfig } = await run({ state: 's', code: 'c' }, bound(), { + oauth, + user: { role: 'USER', tenantId: 'tenant-a' }, + }); + expect(tenantDuringComplete).toBe('tenant-a'); + expect(getAppConfig).toHaveBeenCalledWith( + expect.objectContaining({ baseOnly: true, tenantId: 'tenant-a', userId: 'u1' }), + ); + }); + + it('completes a flow an override turned on for this user while the operator config has it off', async () => { + const { oauth, getAppConfig, res } = await run({ state: 's', code: 'c' }, bound(), { + base: { ...oauthSettings, enabled: false }, + user: { role: 'BETA' }, + }); + expect(oauth.complete).toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/success?serverName=GitHub${flowParam}`); + expect(getAppConfig).toHaveBeenCalledWith( + expect.objectContaining({ role: 'BETA', userId: 'u1', failClosed: true }), + ); + }); + + it.each([ + ['the user no longer exists', { user: null }], + [ + "the user's effective config turns the feature off", + { settings: { ...oauthSettings, enabled: false } }, + ], + ['the user is in another tenant than the flow', { user: { tenantId: 'tenant-b' } }], + ])('refuses the callback when %s, storing nothing', async (_label, options) => { + const { oauth, res } = await run({ state: 's', code: 'c' }, bound(), options); + expect(oauth.complete).not.toHaveBeenCalled(); + expect(res.redirect).toHaveBeenCalledWith(`/oauth/error?error=invalid_state${flowParam}`); + }); +}); + +describe('status and disconnect', () => { + it('reports a connected account', async () => { + const { handlers, res } = setup(); + await handlers.status(signedIn(), asRes(res)); + expect(res.json).toHaveBeenCalledWith({ + configured: true, + provider: 'github', + connected: true, + flowId: 'pull-request-oauth:u1:n', + }); + }); + + it('reports a deployment where users cannot connect', async () => { + const { handlers, oauth, res } = setup({ settings: { allowedRepositories: ['o/r'] } }); + await handlers.status(signedIn(), asRes(res)); + expect(res.json).toHaveBeenCalledWith({ configured: false, connected: false }); + expect(oauth.status).not.toHaveBeenCalled(); + expect(oauth.storedStatus).toHaveBeenCalledWith('u1'); + }); + + it('reports a grant that outlived the app being turned off, so it can be removed', async () => { + const oauth = makeOAuth({ + storedStatus: jest.fn(async () => ({ connected: true, flowId: 'pull-request-oauth:u1:old' })), + }); + const { handlers, res } = setup({ settings: { allowedRepositories: ['o/r'] }, oauth }); + await handlers.status(signedIn(), asRes(res)); + expect(res.json).toHaveBeenCalledWith({ + configured: false, + connected: true, + flowId: 'pull-request-oauth:u1:old', + }); + }); + + it('removes the connection', async () => { + const { handlers, oauth, res } = setup(); + await handlers.disconnect(signedIn(), asRes(res)); + expect(oauth.disconnect).toHaveBeenCalledWith('u1'); + expect(res.status).toHaveBeenCalledWith(204); + }); + + it('removes the connection after the app was turned off', async () => { + const { handlers, oauth, res } = setup({ settings: { allowedRepositories: ['o/r'] } }); + await handlers.disconnect(signedIn(), asRes(res)); + expect(oauth.disconnect).toHaveBeenCalledWith('u1'); + expect(res.status).toHaveBeenCalledWith(204); + }); + + it('answers a status or disconnect failure with a stable code', async () => { + const failing = makeOAuth({ + status: jest.fn(async () => { + throw new Error('key unavailable'); + }), + disconnect: jest.fn(async () => { + throw new Error('db down'); + }), + }); + const { handlers, res } = setup({ oauth: failing }); + await handlers.status(signedIn(), asRes(res)); + expect(res.json).toHaveBeenLastCalledWith(expect.objectContaining({ code: 'STATUS_FAILED' })); + await handlers.disconnect(signedIn(), asRes(res)); + expect(res.json).toHaveBeenLastCalledWith( + expect.objectContaining({ code: 'DISCONNECT_FAILED' }), + ); + }); +}); + +describe('createPullRequestConnectionRouter', () => { + const routesOf = (router: { stack: Array<{ route?: { path: string; methods: object } }> }) => + router.stack.flatMap((layer) => + layer.route ? Object.keys(layer.route.methods).map((m) => `${m} ${layer.route!.path}`) : [], + ); + const callbackLimiter = jest.fn(); + const markNavigation = jest.fn(); + const deps = { + store: { + findToken: jest.fn(), + replaceTokenIfCurrent: jest.fn(), + deleteTokens: jest.fn(), + isAgentTriggerPrincipalActive: jest.fn(), + findTokensOfUser: jest.fn(), + }, + fetchFn: jest.fn(), + getAppConfig: jest.fn(), + getUserById: jest.fn(), + requireJwtAuth: jest.fn(), + markNavigation, + callbackLimiter, + }; + + it('marks the callback as a navigation, then puts it behind the limiter', () => { + const router = createPullRequestConnectionRouter({ ...deps, env }) as unknown as { + stack: Array<{ route?: { path: string; stack: Array<{ handle: unknown }> } }>; + }; + const callback = router.stack.find((layer) => layer.route?.path === '/oauth/callback'); + /** Marked first, so a limited callback lands on the OAuth failure page, not raw JSON. */ + expect(callback?.route?.stack.map((layer) => layer.handle).slice(0, 2)).toEqual([ + markNavigation, + callbackLimiter, + ]); + }); + + it('registers the connection routes when states can be signed', () => { + const router = createPullRequestConnectionRouter({ ...deps, env }); + expect(routesOf(router as never)).toEqual([ + 'get /oauth/callback', + 'get /oauth/connect', + 'get /oauth/status', + 'delete /oauth', + ]); + }); + + it('registers nothing without a signing secret', () => { + const router = createPullRequestConnectionRouter({ ...deps, env: {} }); + expect(routesOf(router as never)).toEqual([]); + }); +}); + +describe('createPullRequestUserToken', () => { + it('reads the token only when an app is configured', async () => { + const oauth = makeOAuth(); + const userToken = createPullRequestUserToken(oauth, env); + await expect(userToken('u1', oauthSettings)).resolves.toBe('user-token'); + await expect(userToken('u1', { allowedRepositories: ['o/r'] })).resolves.toBeNull(); + expect(oauth.token).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/api/src/pulls/connect.ts b/packages/api/src/pulls/connect.ts new file mode 100644 index 00000000000..b4dbecc3097 --- /dev/null +++ b/packages/api/src/pulls/connect.ts @@ -0,0 +1,312 @@ +import express from 'express'; +import { logger, getTenantId, tenantStorage } from '@librechat/data-schemas'; +import type { + PullRequestProvider, + TPullRequestConnection, + TPullRequestConnectStart, +} from 'librechat-data-provider'; +import type { Request, Response, Router, RequestHandler } from 'express'; +import type { AppConfig } from '@librechat/data-schemas'; +import type { AppConfigUserLike, GetAppConfigOptions } from '~/app/service'; +import type { PullRequestOAuth, PullRequestOAuthStore } from './oauth'; +import type { PullRequestOAuthConfig } from './settings'; +import type { PullRequestUserToken } from './handler'; +import type { PullRequestFetch } from './shared'; +import type { ServerRequest } from '~/types'; +import { + PULL_REQUEST_OAUTH_CALLBACK_PATH, + createPullRequestOAuthFromEnv, + DEFAULT_PULL_REQUEST_OAUTH_FLOW_MINUTES, +} from './oauth'; +import { + setOAuthSession, + validateOAuthCsrf, + setOAuthCsrfCookie, + validateOAuthSession, +} from '~/oauth/csrf'; +import { isPullRequestFeatureActive, resolvePullRequestOAuth } from './settings'; +import { readPullRequestSettings } from './handler'; +import { getSafeErrorMetadata } from '~/utils'; +import { getBasePath } from '~/utils/path'; + +const PROVIDER_NAMES: Record = { + github: 'GitHub', + gitlab: 'GitLab', + bitbucket: 'Bitbucket', +}; + +type Env = Readonly>; + +/** + * The routes a user connects and disconnects their own account through. Starting a connection + * and reading or removing it need the signed-in user; the callback arrives from the provider + * without one and is trusted only through its signed `state` plus the CSRF or session cookie that + * binds it to the browser that started the flow. Failures redirect to the shared OAuth error page + * with one of its known codes, never with provider text; the JSON routes answer with a stable + * `code` a client can localize. + */ +export function createPullRequestConnectionHandlers(deps: { + oauth: PullRequestOAuth; + getAppConfig: (options: GetAppConfigOptions) => Promise; + /** The user a callback's signed state names, as config resolution needs it; null if gone. */ + getUser: (userId: string) => Promise; + env: Env; +}) { + /** + * A user's connection settings: the feature as that user's effective config resolves it (role, + * group and user overrides included), with the app taken from the operator's YAML. Starting a + * flow and finishing it both ask this, so a flow that may start may also finish. + */ + async function configFor( + user: AppConfigUserLike | null | undefined, + ): Promise { + const settings = await readPullRequestSettings(deps.getAppConfig, user); + if (settings == null || !isPullRequestFeatureActive(settings, deps.env)) return null; + return resolvePullRequestOAuth(settings, deps.env); + } + + const redirect = (res: Response, page: string) => res.redirect(`${getBasePath()}${page}`); + /** Names the flow once it is known to be this browser's, so the page that started it can stop + * watching that flow alone; an unbound or unreadable state names none. */ + const flowParam = (flowId?: string) => (flowId ? `&flow=${encodeURIComponent(flowId)}` : ''); + const fail = (res: Response, code: string, flowId?: string) => + redirect(res, `/oauth/error?error=${encodeURIComponent(code)}${flowParam(flowId)}`); + + async function finishCallback( + res: Response, + claims: NonNullable>, + code: string, + ): Promise { + try { + /** The user must still exist, in the tenant the flow started in. */ + const user = await deps.getUser(claims.userId); + if (user == null || (user.tenantId ?? null) !== (claims.tenantId ?? null)) { + fail(res, 'invalid_state', claims.flowId); + return; + } + /** The same check as starting the flow; a changed app voids flows started before. */ + const config = await configFor({ ...user, id: claims.userId }); + if (config == null || config.provider !== claims.provider || config.host !== claims.host) { + fail(res, 'invalid_state', claims.flowId); + return; + } + await deps.oauth.complete(claims, code, config); + redirect( + res, + `/oauth/success?serverName=${encodeURIComponent(PROVIDER_NAMES[config.provider])}${flowParam(claims.flowId)}`, + ); + } catch (caught) { + logger.warn('[PullRequests] Connection callback failed', getSafeErrorMetadata(caught)); + fail(res, 'callback_failed', claims.flowId); + } + } + + return { + async connect(req: ServerRequest, res: Response): Promise { + const user = req.user; + if (!user?.id) { + res.status(401).json({ error: 'Unauthorized', code: 'UNAUTHORIZED' }); + return; + } + try { + const config = await configFor(req.user); + if (config == null) { + res + .status(404) + .json({ error: 'Connecting an account is not available', code: 'NOT_CONFIGURED' }); + return; + } + const tenantId = getTenantId() ?? user.tenantId; + const { url, flowId } = await deps.oauth.authorizationUrl( + { id: user.id, ...(tenantId ? { tenantId } : {}) }, + config, + ); + /** How long the signed state stays valid: the binding cookie lives as long, and the + * client watches for exactly that long, so an approval inside the window can finish. */ + const expiresInSeconds = + (config.flowTimeoutMinutes ?? DEFAULT_PULL_REQUEST_OAUTH_FLOW_MINUTES) * 60; + setOAuthCsrfCookie(res, flowId, PULL_REQUEST_OAUTH_CALLBACK_PATH, expiresInSeconds * 1000); + const body: TPullRequestConnectStart = { url, expiresInSeconds, flowId }; + res.status(200).json(body); + } catch (error) { + logger.error('[PullRequests] Could not start a connection', getSafeErrorMetadata(error)); + res.status(500).json({ error: 'Failed to start the connection', code: 'CONNECT_FAILED' }); + } + }, + + async callback(req: Request, res: Response): Promise { + const { code, state, error } = req.query as Record; + if (typeof state !== 'string' || state === '') { + fail(res, 'missing_state'); + return; + } + const claims = deps.oauth.verifyState(state); + if (claims == null) { + fail(res, 'invalid_state'); + return; + } + /** The browser finishing the flow must be the one that started it, for that user. */ + const bound = + validateOAuthCsrf(req, res, claims.flowId, PULL_REQUEST_OAUTH_CALLBACK_PATH) || + validateOAuthSession(req, claims.userId); + if (!bound) { + fail(res, 'invalid_state'); + return; + } + if (typeof error === 'string' && error !== '') { + fail(res, 'callback_failed', claims.flowId); + return; + } + if (typeof code !== 'string' || code === '') { + fail(res, 'missing_code', claims.flowId); + return; + } + /** + * The provider's redirect carries no session, so the tenant the flow started in is restored + * from the signed state before anything is read or stored. A request already placed in + * another tenant (by host, for example) is refused rather than stored there. + */ + const current = getTenantId(); + if (current != null && current !== claims.tenantId) { + fail(res, 'invalid_state', claims.flowId); + return; + } + const finish = () => finishCallback(res, claims, code); + if (claims.tenantId && current == null) { + await tenantStorage.run({ tenantId: claims.tenantId }, finish); + return; + } + await finish(); + }, + + async status(req: ServerRequest, res: Response): Promise { + const userId = req.user?.id; + if (!userId) { + res.status(401).json({ error: 'Unauthorized', code: 'UNAUTHORIZED' }); + return; + } + try { + const config = await configFor(req.user); + /** Without a usable app the grant is read under any app or host, so one made before the + * app was turned off still shows, and the user can still remove it. */ + const stored = + config == null + ? await deps.oauth.storedStatus(userId) + : await deps.oauth.status(userId, config); + const body: TPullRequestConnection = { + configured: config != null, + ...(config != null ? { provider: config.provider } : {}), + connected: stored.connected, + ...(stored.flowId != null ? { flowId: stored.flowId } : {}), + }; + res.status(200).json(body); + } catch (error) { + logger.error('[PullRequests] Could not read the connection', getSafeErrorMetadata(error)); + res.status(500).json({ error: 'Failed to read the connection', code: 'STATUS_FAILED' }); + } + }, + + async disconnect(req: ServerRequest, res: Response): Promise { + const userId = req.user?.id; + if (!userId) { + res.status(401).json({ error: 'Unauthorized', code: 'UNAUTHORIZED' }); + return; + } + try { + /** Not gated on the current configuration: a grant made under an app or host that has + * since changed must still go, or restoring that configuration would revive it. */ + await deps.oauth.disconnect(userId); + res.status(204).end(); + } catch (error) { + logger.error('[PullRequests] Could not disconnect', getSafeErrorMetadata(error)); + res.status(500).json({ error: 'Failed to disconnect', code: 'DISCONNECT_FAILED' }); + } + }, + }; +} + +type ConnectionSettings = Parameters[0]; + +/** A `PullRequestUserToken` that reads only the connection settings it needs. */ +type ConnectionToken = (( + userId: string, + settings: ConnectionSettings, +) => Promise) & { + reject: (userId: string, settings: ConnectionSettings, token: string) => Promise; +}; + +/** The lookups' view of a connection: the user's token, or none, and its invalidation. */ +export function createPullRequestUserToken(oauth: PullRequestOAuth, env: Env): ConnectionToken { + const read = async (userId: string, settings: ConnectionSettings) => { + const config = resolvePullRequestOAuth(settings, env); + return config == null ? null : oauth.token(userId, config); + }; + return Object.assign(read, { + reject: async (userId: string, settings: ConnectionSettings, token: string) => { + const config = resolvePullRequestOAuth(settings, env); + if (config != null) await oauth.reject(userId, config, token); + }, + }); +} + +type ConnectionDeps = { + store: PullRequestOAuthStore; + fetchFn: PullRequestFetch; + env: Env; +}; + +/** The lookups' user tokens for this deployment, or none when users cannot connect at all. */ +export function createPullRequestUserTokenFromEnv( + deps: ConnectionDeps, +): PullRequestUserToken | undefined { + const oauth = createPullRequestOAuthFromEnv(deps); + return oauth && createPullRequestUserToken(oauth, deps.env); +} + +/** + * The connection routes, mounted under `/api/pull-requests`. Without a signing secret no route is + * registered. The callback is unauthenticated by design (see the handlers); the others need the + * caller's `requireJwtAuth`. + */ +export function createPullRequestConnectionRouter( + deps: ConnectionDeps & { + getAppConfig: (options: GetAppConfigOptions) => Promise; + getUserById: ( + userId: string, + fieldsToSelect: string, + ) => Promise<{ role?: string; tenantId?: string; idOnTheSource?: string | null } | null>; + requireJwtAuth: RequestHandler; + /** Marks the callback as a browser navigation, so a limiter or ban answers it with the + * localized OAuth failure page instead of JSON. Runs before the limiter. */ + markNavigation: RequestHandler; + /** The app's limiter for unauthenticated OAuth callbacks: each one costs a token exchange. */ + callbackLimiter: RequestHandler; + }, +): Router { + const router = express.Router(); + const oauth = createPullRequestOAuthFromEnv(deps); + if (oauth == null) return router; + const handlers = createPullRequestConnectionHandlers({ + oauth, + getAppConfig: deps.getAppConfig, + getUser: async (userId) => { + const user = await deps.getUserById(userId, 'role tenantId idOnTheSource'); + if (user == null) return null; + return { + id: userId, + ...(user.role != null ? { role: user.role } : {}), + ...(user.tenantId != null ? { tenantId: user.tenantId } : {}), + ...(user.idOnTheSource !== undefined ? { idOnTheSource: user.idOnTheSource } : {}), + }; + }, + env: deps.env, + }); + /** Behind `requireJwtAuth`, which puts the signed-in user on the request. */ + const authed = (handler: (req: ServerRequest, res: Response) => Promise) => + handler as unknown as RequestHandler; + router.get('/oauth/callback', deps.markNavigation, deps.callbackLimiter, handlers.callback); + router.get('/oauth/connect', deps.requireJwtAuth, setOAuthSession, authed(handlers.connect)); + router.get('/oauth/status', deps.requireJwtAuth, authed(handlers.status)); + router.delete('/oauth', deps.requireJwtAuth, authed(handlers.disconnect)); + return router; +} diff --git a/packages/api/src/pulls/github.spec.ts b/packages/api/src/pulls/github.spec.ts index 3040ef07689..b45bf790dee 100644 --- a/packages/api/src/pulls/github.spec.ts +++ b/packages/api/src/pulls/github.spec.ts @@ -160,6 +160,13 @@ describe('createGitHubPullRequestSource', () => { await expect(find(source)).rejects.toMatchObject({ code: 'RATE_LIMITED' }); }); + it('maps a 401 to CREDENTIAL_REJECTED, a refusal of the token itself', async () => { + const { source } = sourceFor({ + '/pulls?state=open': () => new Response('', { status: 401 }), + }); + await expect(find(source)).rejects.toMatchObject({ code: 'CREDENTIAL_REJECTED' }); + }); + it.each([ ['a 500', () => new Response('boom', { status: 500 })], ['a plain 403', () => new Response('', { status: 403 })], diff --git a/packages/api/src/pulls/github.ts b/packages/api/src/pulls/github.ts index 81c9d3eece8..51d1f7ddee3 100644 --- a/packages/api/src/pulls/github.ts +++ b/packages/api/src/pulls/github.ts @@ -287,6 +287,8 @@ export function createGitHubPullRequestSource({ if ((notFoundIsAbsent && response.status === 404) || absent.includes(response.status)) { return null; } + /** The provider no longer accepts the token: revoked, or expired without a refresh. */ + if (response.status === 401) throw new PullRequestSourceError('CREDENTIAL_REJECTED'); throw new PullRequestSourceError('UPSTREAM_ERROR'); } diff --git a/packages/api/src/pulls/gitlab.spec.ts b/packages/api/src/pulls/gitlab.spec.ts index 178e698a14c..d3f52427414 100644 --- a/packages/api/src/pulls/gitlab.spec.ts +++ b/packages/api/src/pulls/gitlab.spec.ts @@ -105,6 +105,11 @@ describe('createGitLabPullRequestSource', () => { expect(urls.every((url) => url.startsWith(`https://${host}/api/`))).toBe(true); }); + it('maps a 401 to CREDENTIAL_REJECTED, a refusal of the token itself', async () => { + const { source } = sourceFor({ 'state=opened': () => new Response('', { status: 401 }) }); + await expect(find(source)).rejects.toMatchObject({ code: 'CREDENTIAL_REJECTED' }); + }); + it.each([ ['another host', 'https://evil.example/g/r/-/merge_requests/7'], ['plain http', 'http://gitlab.com/g/r/-/merge_requests/7'], diff --git a/packages/api/src/pulls/gitlab.ts b/packages/api/src/pulls/gitlab.ts index 9f552628942..06cce5a00de 100644 --- a/packages/api/src/pulls/gitlab.ts +++ b/packages/api/src/pulls/gitlab.ts @@ -199,6 +199,8 @@ export function createGitLabPullRequestSource({ } /** A project the token cannot see is indistinguishable from one without merge requests. */ if (response.status === 404 || absent.includes(response.status)) return null; + /** The provider no longer accepts the token: revoked, or expired without a refresh. */ + if (response.status === 401) throw new PullRequestSourceError('CREDENTIAL_REJECTED'); throw new PullRequestSourceError('UPSTREAM_ERROR'); } diff --git a/packages/api/src/pulls/handler.spec.ts b/packages/api/src/pulls/handler.spec.ts index a835a9f33c9..eab79bebbf1 100644 --- a/packages/api/src/pulls/handler.spec.ts +++ b/packages/api/src/pulls/handler.spec.ts @@ -47,12 +47,15 @@ function setup( laneGit?: Lane; lookup?: jest.Mock; env?: Record; + userToken?: jest.Mock; } = {}, ) { /** `null` means the block is absent; omitting the option means the enabled default. */ const settings = options.settings === undefined ? enabled : (options.settings ?? undefined); const laneGit: Lane = - options.laneGit === undefined ? { branch: 'feat/x', head: null, repo: 'o/r' } : options.laneGit; + options.laneGit === undefined + ? { branch: 'feat/x', head: 'c0ffee1', repo: 'o/r' } + : options.laneGit; const lookup = options.lookup ?? jest.fn().mockResolvedValue({ ok: true, value: pr }); const env = options.env ?? { GH_TOKEN: 'ghp_secret' }; const getConvoLaneGit = jest.fn().mockResolvedValue(laneGit); @@ -61,6 +64,7 @@ function setup( getAppConfig: jest.fn().mockResolvedValue(configWith(settings)), lookup, env, + ...(options.userToken ? { userToken: options.userToken } : {}), }); const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; const run = (params: { conversationId?: string } = { conversationId: 'c1' }, user = 'u1') => @@ -89,7 +93,7 @@ describe('createConversationPullRequestHandler', () => { repo: 'o/r', branch: 'feat/x', token: 'ghp_secret', - head: null, + head: 'c0ffee1', provider: 'github', host: 'github.com', ttlMs: 30_000, @@ -122,7 +126,9 @@ describe('createConversationPullRequestHandler', () => { async function hostFor(base: Record, override: Record) { const lookup = jest.fn().mockResolvedValue({ ok: true, value: pr }); const handler = createConversationPullRequestHandler({ - getConvoLaneGit: jest.fn().mockResolvedValue({ branch: 'feat/x', head: null, repo: 'o/r' }), + getConvoLaneGit: jest + .fn() + .mockResolvedValue({ branch: 'feat/x', head: 'c0ffee1', repo: 'o/r' }), getAppConfig: jest .fn() .mockImplementation(async (options: { baseOnly?: boolean }) => @@ -142,7 +148,9 @@ describe('createConversationPullRequestHandler', () => { it('takes the provider from the operator config, never from a database override', async () => { const lookup = jest.fn().mockResolvedValue({ ok: true, value: pr }); const handler = createConversationPullRequestHandler({ - getConvoLaneGit: jest.fn().mockResolvedValue({ branch: 'feat/x', head: null, repo: 'o/r' }), + getConvoLaneGit: jest + .fn() + .mockResolvedValue({ branch: 'feat/x', head: 'c0ffee1', repo: 'o/r' }), getAppConfig: jest .fn() .mockImplementation(async (options: { baseOnly?: boolean }) => @@ -161,6 +169,138 @@ describe('createConversationPullRequestHandler', () => { ); }); + describe('with users connecting their own accounts', () => { + const connectable = { + ...enabled, + token: undefined, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }; + const oauthEnv = { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }; + + it("looks up with the user's own token, in any repository that user can see", async () => { + const { run, lookup } = setup({ + settings: connectable, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + laneGit: { branch: 'feat/x', head: 'c0ffee1', repo: 'elsewhere/private' }, + userToken: jest.fn().mockResolvedValue('gho_user'), + }); + await run(); + expect(lookup).toHaveBeenCalledWith( + expect.objectContaining({ + token: 'gho_user', + repo: 'elsewhere/private', + allowedRepositories: ['*/*'], + }), + ); + }); + + it("does not read the user's token for a lane with no commit yet", async () => { + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run, res } = setup({ + settings: connectable, + env: oauthEnv, + laneGit: { branch: 'feat/x', head: null, repo: 'o/r' }, + userToken, + }); + await run(); + expect(userToken).not.toHaveBeenCalled(); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it.each([ + ["the user's own token", true], + ["the server's token", false], + ])('invalidates %s only when the provider refuses it', async (_label, personal) => { + const userToken = Object.assign(jest.fn().mockResolvedValue(personal ? 'gho_user' : null), { + reject: jest.fn().mockResolvedValue(undefined), + }); + const { run, res } = setup({ + settings: personal ? connectable : enabled, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + lookup: jest.fn().mockResolvedValue({ ok: false, error: { code: 'CREDENTIAL_REJECTED' } }), + userToken, + }); + await run(); + expect(res.status).toHaveBeenCalledWith(503); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'UPSTREAM_ERROR' })); + if (personal) { + expect(userToken.reject).toHaveBeenCalledWith('u1', expect.anything(), 'gho_user'); + } else { + expect(userToken.reject).not.toHaveBeenCalled(); + } + }); + + it("falls back to the server's token, still held to its list, for a user who has not connected", async () => { + const { run, lookup, res } = setup({ + settings: connectable, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + laneGit: { branch: 'feat/x', head: 'c0ffee1', repo: 'elsewhere/private' }, + userToken: jest.fn().mockResolvedValue(null), + }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it('answers no pull request, not a configuration error, when nothing can look up', async () => { + const { run, lookup, res } = setup({ + settings: connectable, + env: oauthEnv, + userToken: jest.fn().mockResolvedValue(null), + }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it("does not read the user's token while the feature is turned off", async () => { + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run, res } = setup({ + settings: { ...connectable, enabled: false }, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + userToken, + }); + await run(); + expect(userToken).not.toHaveBeenCalled(); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }); + + it.each([ + ['no lane', null], + ['a lane without a branch', { branch: null, head: 'c0ffee1', repo: 'o/r' }], + ])( + "answers no pull request for %s without reading the user's token", + async (_label, laneGit) => { + const userToken = jest.fn().mockRejectedValue(new Error('key unavailable')); + const { run, lookup, res } = setup({ + settings: connectable, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + laneGit: laneGit as Lane, + userToken, + }); + await run(); + expect(userToken).not.toHaveBeenCalled(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ pullRequest: null }); + }, + ); + + it("answers unavailable, not the server's token or no pull request, when the user's cannot be read", async () => { + const { run, lookup, res } = setup({ + settings: connectable, + env: { ...oauthEnv, GH_TOKEN: 'ghp_server' }, + userToken: jest.fn().mockRejectedValue(new Error('mongodb://secret@host')), + }); + await run(); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(503); + expect(res.json).toHaveBeenCalledWith(expect.objectContaining({ code: 'UPSTREAM_ERROR' })); + expect(JSON.stringify(res.json.mock.calls)).not.toContain('secret@host'); + }); + }); + it('looks a GitLab lane up with the GitLab token on the GitLab host', async () => { const { run, lookup } = setup({ settings: { ...enabled, token: undefined, provider: 'gitlab' }, @@ -192,7 +332,7 @@ describe('createConversationPullRequestHandler', () => { it('passes the pull request the conversation linked to', async () => { const { run, lookup } = setup({ - laneGit: { branch: 'feat/x', head: null, repo: 'o/r', pullRequest: 7 }, + laneGit: { branch: 'feat/x', head: 'c0ffee1', repo: 'o/r', pullRequest: 7 }, }); await run(); expect(lookup).toHaveBeenCalledWith(expect.objectContaining({ pullRequest: 7 })); @@ -275,7 +415,7 @@ describe('createConversationPullRequestHandler', () => { }); describe('allowAllRepositories', () => { - const lane = { branch: 'feat/x', head: null, repo: 'someone/else' }; + const lane = { branch: 'feat/x', head: 'c0ffee1', repo: 'someone/else' }; const open = { allowAllRepositories: true }; it('looks up a repository nobody listed, when the administrator opted in', async () => { @@ -310,7 +450,7 @@ describe('createConversationPullRequestHandler', () => { }); describe('repository allowlist', () => { - const lane = (repo: string) => ({ branch: 'feat/x', head: null, repo }); + const lane = (repo: string) => ({ branch: 'feat/x', head: 'c0ffee1', repo }); it.each([ ['an exact match', ['o/r'], 'o/r'], @@ -374,7 +514,7 @@ describe('createConversationPullRequestHandler', () => { ); const getConvoLaneGit = jest.fn(async () => { order.push('lane:start'); - return { branch: 'feat/x', head: null, repo: 'o/r' }; + return { branch: 'feat/x', head: 'c0ffee1', repo: 'o/r' }; }); const handler = createConversationPullRequestHandler({ getConvoLaneGit, @@ -397,8 +537,8 @@ describe('createConversationPullRequestHandler', () => { it.each([ ['no stored lane', null], - ['a detached lane', { branch: null, head: null, repo: 'o/r' }], - ['a lane with no repository', { branch: 'main', head: null }], + ['a detached lane', { branch: null, head: 'c0ffee1', repo: 'o/r' }], + ['a lane with no repository', { branch: 'main', head: 'c0ffee1' }], ])('answers null for %s without calling GitHub', async (_label, laneGit) => { const { run, res, lookup } = setup({ laneGit }); await run(); @@ -468,7 +608,7 @@ describe('createConversationPullRequestsHandler', () => { }; const lane = (conversationId: string, repo = 'o/r', branch = `feat/${conversationId}`): Row => ({ conversationId, - laneGit: { branch, head: null, repo }, + laneGit: { branch, head: 'c0ffee1', repo }, }); function batch( @@ -477,17 +617,25 @@ describe('createConversationPullRequestsHandler', () => { lanes?: Row[]; lookup?: jest.Mock; env?: Record; + userToken?: jest.Mock; + /** Answers the config read only when this resolves. */ + configGate?: Promise; } = {}, ) { const settings = options.settings === undefined ? enabled : (options.settings ?? undefined); const lanes = options.lanes ?? [lane('a'), lane('b')]; + const gate = options.configGate; const lookup = options.lookup ?? jest.fn().mockResolvedValue({ ok: true, value: pr }); const getConvosLaneGit = jest.fn().mockResolvedValue(lanes); const handler = createConversationPullRequestsHandler({ getConvosLaneGit, - getAppConfig: jest.fn().mockResolvedValue(configWith(settings)), + getAppConfig: jest.fn().mockImplementation(async () => { + await gate; + return configWith(settings); + }), lookup, env: options.env ?? { GH_TOKEN: 'ghp_secret' }, + ...(options.userToken ? { userToken: options.userToken } : {}), }); const res = { status: jest.fn().mockReturnThis(), json: jest.fn() }; const run = (...args: [body?: unknown, user?: string | null]) => { @@ -502,6 +650,165 @@ describe('createConversationPullRequestsHandler', () => { return { run, res, lookup, getConvosLaneGit }; } + describe('a token read the request no longer waits for', () => { + const connectable = { + ...enabled, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }; + const env = { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }; + + it('is not started once the configured deadline has passed', async () => { + let open: () => void = () => undefined; + const configGate = new Promise((resolve) => (open = resolve)); + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run, res } = batch({ + settings: { ...connectable, batchTimeoutSeconds: 1 }, + env, + userToken, + configGate, + }); + const running = run(); + await new Promise((resolve) => setTimeout(resolve, 1_100)); + open(); + await running; + await new Promise((resolve) => setImmediate(resolve)); + expect(res.status).toHaveBeenCalledWith(503); + expect(userToken).not.toHaveBeenCalled(); + }); + + it('is not started after the request was answered for a config read that took too long', async () => { + jest.useFakeTimers({ doNotFake: ['nextTick', 'setImmediate', 'queueMicrotask'] }); + try { + let open: () => void = () => undefined; + const configGate = new Promise((resolve) => (open = resolve)); + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run, res } = batch({ + settings: { ...connectable, batchTimeoutSeconds: 120 }, + env, + userToken, + configGate, + }); + const running = run(); + await jest.advanceTimersByTimeAsync(20_001); + await running; + expect(res.status).toHaveBeenCalledWith(503); + /** The config arrives within its own, longer deadline, but nobody is waiting any more. */ + open(); + for (let i = 0; i < 5; i++) await new Promise((resolve) => setImmediate(resolve)); + expect(userToken).not.toHaveBeenCalled(); + } finally { + jest.useRealTimers(); + } + }); + }); + + it("looks the batch up with the user's own token", async () => { + const { run, lookup } = batch({ + settings: { + ...enabled, + token: undefined, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }, + userToken: jest.fn().mockResolvedValue('gho_user'), + }); + await run({ conversationIds: ['a', 'b'] }); + expect(lookup).toHaveBeenCalledWith(expect.objectContaining({ token: 'gho_user' })); + }); + + it("does not read the user's token for a batch whose lanes have no commit yet", async () => { + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run } = batch({ + settings: { + ...enabled, + token: undefined, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }, + lanes: [{ conversationId: 'a', laneGit: { branch: 'feat/a', head: null, repo: 'o/r' } }], + userToken, + }); + await run({ conversationIds: ['a'] }); + expect(userToken).not.toHaveBeenCalled(); + }); + + it("does not read the user's token for a batch while the feature is turned off", async () => { + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run } = batch({ + settings: { + ...enabled, + enabled: false, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }, + userToken, + }); + await run({ conversationIds: ['a', 'b'] }); + expect(userToken).not.toHaveBeenCalled(); + }); + + it("does not read the user's token for a batch with no lane to look up", async () => { + const userToken = jest.fn().mockResolvedValue('gho_user'); + const { run, res } = batch({ + settings: { + ...enabled, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }, + lanes: [{ conversationId: 'b', laneGit: { branch: null, head: 'c0ffee1' } }], + userToken, + }); + await run({ conversationIds: ['a', 'b'] }); + expect(userToken).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + }); + + it("invalidates the user's own token when a batch lookup is refused with it", async () => { + const userToken = Object.assign(jest.fn().mockResolvedValue('gho_user'), { + reject: jest.fn().mockResolvedValue(undefined), + }); + const { run, res } = batch({ + settings: { + ...enabled, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret' }, + lanes: [lane('a', 'o/r')], + lookup: jest.fn().mockResolvedValue({ ok: false, error: { code: 'CREDENTIAL_REJECTED' } }), + userToken, + }); + await run({ conversationIds: ['a'] }); + expect(userToken.reject).toHaveBeenCalledTimes(1); + expect(userToken.reject).toHaveBeenCalledWith('u1', expect.anything(), 'gho_user'); + expect(res.json).toHaveBeenCalledWith({ + results: [{ conversationId: 'a', error: { code: 'UPSTREAM_ERROR' } }], + }); + }); + + it("answers the batch's lanes unavailable when the user's own token cannot be read", async () => { + const { run, lookup, res } = batch({ + settings: { + ...enabled, + oauth: { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }, + }, + env: { GH_CLIENT_ID: 'id', GH_CLIENT_SECRET: 'secret', GH_TOKEN: 'ghp_server' }, + lanes: [ + lane('a', 'elsewhere/private'), + { conversationId: 'b', laneGit: { branch: null, head: 'c0ffee1' } }, + ], + userToken: jest.fn().mockRejectedValue(new Error('refresh timed out')), + }); + await run({ conversationIds: ['a', 'b'] }); + expect(lookup).not.toHaveBeenCalled(); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith({ + results: [ + { conversationId: 'a', error: { code: 'UPSTREAM_ERROR' } }, + { conversationId: 'b', pullRequest: null }, + ], + }); + }); + it('takes the host from the operator config for the batch too', async () => { const lookup = jest.fn().mockResolvedValue({ ok: true, value: pr }); const handler = createConversationPullRequestsHandler({ @@ -525,7 +832,7 @@ describe('createConversationPullRequestsHandler', () => { it('keeps conversations on one lane apart when they linked different pull requests', async () => { const linked = (conversationId: string, pullRequest: number) => ({ conversationId, - laneGit: { branch: 'feat/x', head: null, repo: 'o/r', pullRequest }, + laneGit: { branch: 'feat/x', head: 'c0ffee1', repo: 'o/r', pullRequest }, }); const { run, lookup } = batch({ lanes: [linked('a', 7), linked('b', 9)] }); await run({ conversationIds: ['a', 'b'] }); @@ -538,7 +845,10 @@ describe('createConversationPullRequestsHandler', () => { const { run, lookup } = batch({ settings: { allowAllRepositories: true }, lanes: [ - { conversationId: 'a', laneGit: { branch: 'feat/x', head: null, repo: 'someone/else' } }, + { + conversationId: 'a', + laneGit: { branch: 'feat/x', head: 'c0ffee1', repo: 'someone/else' }, + }, ], env: { GITHUB_TOKEN: 'ghp_deploy' }, }); @@ -1240,7 +1550,7 @@ describe('createConversationPullRequestsHandler', () => { }); it('uses the same lookup input as the single route', async () => { - const single = setup({ laneGit: { branch: 'feat/a', head: null, repo: 'o/r' } }); + const single = setup({ laneGit: { branch: 'feat/a', head: 'c0ffee1', repo: 'o/r' } }); await single.run(); const many = batch({ lanes: [lane('a')] }); await many.run({ conversationIds: ['a'] }); diff --git a/packages/api/src/pulls/handler.ts b/packages/api/src/pulls/handler.ts index 3402a0bf58f..51fbfd40a91 100644 --- a/packages/api/src/pulls/handler.ts +++ b/packages/api/src/pulls/handler.ts @@ -10,12 +10,13 @@ import type { } from 'librechat-data-provider'; import type { AppConfig } from '@librechat/data-schemas'; import type { Response } from 'express'; -import type { GetAppConfigOptions } from '~/app/service'; +import type { AppConfigUserLike, GetAppConfigOptions } from '~/app/service'; +import type { PullRequestErrorCode, PullRequestLookup } from './types'; import type { PullRequestSettings } from './settings'; -import type { PullRequestLookup } from './types'; import type { ServerRequest } from '~/types'; import { resolvePullRequestHost, + resolvePullRequestOAuth, resolvePullRequestToken, isPullRequestFeatureActive, resolvePullRequestProvider, @@ -167,13 +168,90 @@ async function withDeadline(work: Promise, ms: number): Promise settings.allowAllRepositories === true ? ALL_REPOSITORIES : (settings.allowedRepositories ?? []); -/** A lane the server's token may be used for: it names a branch and an allowed repository. */ +/** + * The token a lookup runs with and the repositories it may be used for. A user's own token sees + * only what that user can see, so it is not held to the list the server's token is. + */ +/** `personal` marks the user's own token, the one a refusal invalidates. */ +type Credential = { token: string; repositories: readonly string[]; personal?: true }; + +/** Clients know the codes a lookup has always had; a refused token is an upstream failure to them. */ +const wireCode = (code: PullRequestErrorCode | string): string => + code === 'CREDENTIAL_REJECTED' ? 'UPSTREAM_ERROR' : code; + +/** A refused personal token is invalidated, so the account reads as disconnected rather than + * failing every lookup; the server's token is the operator's to fix. */ +async function invalidateRejected( + userToken: PullRequestUserToken | undefined, + userId: string, + settings: PullRequestSettings, + credential: Credential, +): Promise { + if (credential.personal !== true || userToken?.reject == null) return; + try { + await userToken.reject(userId, settings, credential.token); + } catch (error) { + logger.warn('[PullRequests] Refused user token not invalidated', getSafeErrorMetadata(error)); + } +} + +function credentialOf( + settings: PullRequestSettings, + env: Readonly>, + userToken: UserTokenRead, +): Credential | null { + if (userToken === USER_TOKEN_FAILED) return null; + if (userToken != null) { + return { token: userToken, repositories: ALL_REPOSITORIES, personal: true }; + } + const token = resolvePullRequestToken( + settings.token, + env, + resolvePullRequestProvider(settings.provider) ?? undefined, + ); + return token == null ? null : { token, repositories: repositoryScope(settings) }; +} + +/** Reads the user's own token, or null when the user has not connected an account. `reject` + * invalidates that token after the provider refused it on a lookup. */ +export type PullRequestUserToken = (( + userId: string, + settings: PullRequestSettings, +) => Promise) & { + reject?: (userId: string, settings: PullRequestSettings, token: string) => Promise; +}; + +/** A connected account whose token could not be read or refreshed (the provider or the key store + * is down). That is an outage, answered as one: neither "no pull request" nor the server's token, + * which may see less than the user's own. */ +const USER_TOKEN_FAILED = Symbol('user token failed'); +type UserTokenRead = string | null | typeof USER_TOKEN_FAILED; + +/** Only with the feature on: reading the token can refresh, extend or delete the stored grant. */ +async function readUserToken( + userToken: PullRequestUserToken | undefined, + userId: string, + settings: PullRequestSettings | undefined, + env: Readonly>, +): Promise { + if (userToken == null || settings == null || !isPullRequestFeatureActive(settings, env)) { + return null; + } + try { + return await userToken(userId, settings); + } catch (error) { + logger.warn('[PullRequests] User token unavailable', getSafeErrorMetadata(error)); + return USER_TOKEN_FAILED; + } +} + +/** A lane the credential may be used for: it names a branch and an allowed repository. */ function eligibleLane( laneGit: ConversationLaneGit | null | undefined, - settings: PullRequestSettings, + repositories: readonly string[], ): EligibleLane | null { if (laneGit?.branch == null || laneGit.repo == null) return null; - if (!isAllowedRepository(laneGit.repo, repositoryScope(settings))) return null; + if (!isAllowedRepository(laneGit.repo, repositories)) return null; return { branch: laneGit.branch, head: laneGit.head, @@ -182,19 +260,24 @@ function eligibleLane( }; } -type ServerUser = ServerRequest['user']; +/** Whether any credential could find a pull request for the lane, so reading the user's token + * (which can refresh and rotate it) is worth it. A lane with a branch and no commit yet (`head` + * null) has nothing to match against: every source answers no pull request for it. */ +const isLookupable = (laneGit: ConversationLaneGit | null | undefined): boolean => + laneGit?.head !== null && eligibleLane(laneGit, ALL_REPOSITORIES) != null; const agentsPullRequests = (config: AppConfig): PullRequestSettings | undefined => (config.endpoints?.[EModelEndpoint.agents] as TAgentsEndpoint | undefined)?.pullRequests; /** - * The user's settings, with `provider` and `host` taken from the operator's YAML alone. Role, group - * and user overrides live in the database and may be edited by delegated administrators; letting - * one choose either would let it send the deployment's token to a server of its choosing. + * The user's settings, with `provider`, `host` and `oauth` taken from the operator's YAML alone. + * Role, group and user overrides live in the database and may be edited by delegated + * administrators; letting one choose any of them would let it send a token to a server of its + * choosing. */ -async function readSettings( +export async function readPullRequestSettings( getAppConfig: (options: GetAppConfigOptions) => Promise, - user: ServerUser, + user: AppConfigUserLike | null | undefined, ): Promise { const options = { ...getAppConfigOptionsFromUser(user), skipRuntimeAugmentation: true }; const [resolved, base] = await Promise.all([ @@ -203,17 +286,18 @@ async function readSettings( ]); const settings = agentsPullRequests(resolved); if (settings == null) return undefined; - const { host: _host, provider: _provider, ...rest } = settings; + const { host: _host, provider: _provider, oauth: _oauth, ...rest } = settings; const operator = agentsPullRequests(base); return { ...rest, ...(operator?.provider != null ? { provider: operator.provider } : {}), ...(operator?.host != null ? { host: operator.host } : {}), + ...(operator?.oauth != null ? { oauth: operator.oauth } : {}), }; } /** Everything one lookup needs from the settings, so the single and batch routes cannot differ. */ -function lookupInput(settings: PullRequestSettings, token: string, lane: EligibleLane) { +function lookupInput(settings: PullRequestSettings, credential: Credential, lane: EligibleLane) { return { repo: lane.repo, branch: lane.branch, @@ -225,14 +309,14 @@ function lookupInput(settings: PullRequestSettings, token: string, lane: Eligibl settings.host, resolvePullRequestProvider(settings.provider) ?? undefined, ) ?? undefined, - token, + token: credential.token, ttlMs: (settings.cacheTtlSeconds ?? 30) * 1000, cacheMaxEntries: settings.cacheMaxEntries ?? 500, cacheMaxCredentials: settings.cacheMaxCredentials ?? 256, sharedCache: settings.sharedCache === true, sharedCacheTimeoutMs: settings.sharedCacheTimeoutMs ?? 250, sharedCacheSlots: settings.sharedCacheSlots ?? 65_536, - allowedRepositories: repositoryScope(settings), + allowedRepositories: credential.repositories, limits: { requestTimeoutMs: (settings.requestTimeoutSeconds ?? 10) * 1000, lookupTimeoutMs: (settings.lookupTimeoutSeconds ?? 30) * 1000, @@ -283,6 +367,8 @@ export function createConversationPullRequestHandler(deps: { getAppConfig: (options: GetAppConfigOptions) => Promise; lookup: PullRequestLookup; env: Readonly>; + /** The user's own token, when users can connect their accounts. */ + userToken?: PullRequestUserToken; }) { return async (req: ServerRequest, res: Response): Promise => { const userId = req.user?.id; @@ -298,39 +384,55 @@ export function createConversationPullRequestHandler(deps: { * answer the same as before. */ const [settings, laneGit] = await Promise.all([ - readSettings(deps.getAppConfig, req.user), + readPullRequestSettings(deps.getAppConfig, req.user), deps.getConvoLaneGit(userId, conversationId), ]); if (!settings || !isPullRequestFeatureActive(settings, deps.env)) { res.status(200).json(NONE); return; } + /** Reading the user's token can refresh and rotate it, so only a lane that some credential + * could look up is worth it; any other conversation has no pull request regardless. */ + const userToken = !isLookupable(laneGit) + ? null + : await readUserToken(deps.userToken, userId, settings, deps.env); - /** The repository comes from the worker, so it is never used with the token unless the - * administrator named it. A repository that is not allowed looks like one without a pull - * request. */ - const lane = eligibleLane(laneGit, settings); - if (lane == null) { + const credential = credentialOf(settings, deps.env, userToken); + if (userToken === USER_TOKEN_FAILED) { + res + .status(503) + .json({ error: 'Pull request lookup is unavailable', code: 'UPSTREAM_ERROR' }); + return; + } + /** Users can connect their own account and this one has not: nothing to look up with. */ + if (credential == null && resolvePullRequestOAuth(settings, deps.env) != null) { res.status(200).json(NONE); return; } - - const token = resolvePullRequestToken( - settings.token, - deps.env, - resolvePullRequestProvider(settings.provider) ?? undefined, - ); - if (token == null) { + if (credential == null) { logger.warn('[PullRequests] Enabled without a usable token reference'); res.status(503).json({ error: 'Pull requests are not configured', code: 'NOT_CONFIGURED' }); return; } - const result = await deps.lookup(lookupInput(settings, token, lane)); + /** The repository comes from the worker, so it is never used with the server's token unless + * the administrator named it. A repository that is not allowed looks like one without a + * pull request. */ + const lane = eligibleLane(laneGit, credential.repositories); + if (lane == null) { + res.status(200).json(NONE); + return; + } + + const result = await deps.lookup(lookupInput(settings, credential, lane)); if (!result.ok) { + if (result.error.code === 'CREDENTIAL_REJECTED') { + /** In the background: the answer does not wait on it, and it fails only to a log. */ + void invalidateRejected(deps.userToken, userId, settings, credential); + } res.status(503).json({ error: 'Pull request lookup is unavailable', - code: result.error.code, + code: wireCode(result.error.code), }); return; } @@ -358,6 +460,8 @@ export function createConversationPullRequestsHandler(deps: { getAppConfig: (options: GetAppConfigOptions) => Promise; lookup: PullRequestLookup; env: Readonly>; + /** The user's own token, when users can connect their accounts. */ + userToken?: PullRequestUserToken; }) { const limiter = createLookupLimiter(); const sharedLookups = createSharedLookups(limiter, deps.lookup); @@ -368,6 +472,8 @@ export function createConversationPullRequestsHandler(deps: { res.status(400).json({ error: 'Invalid conversation list' }); return; } + /** Set once the request is answered, however it ended. */ + let answered = false; try { /** * The clock starts when the request arrives. The configured deadline lives in the config, @@ -376,10 +482,23 @@ export function createConversationPullRequestsHandler(deps: { * overlapped. */ const startedAt = Date.now(); - const configRead = readSettings(deps.getAppConfig, req.user); + const configRead = readPullRequestSettings(deps.getAppConfig, req.user); const laneRead = deps.getConvosLaneGit(userId, ids); - /** A lane read nobody waits for (feature off, config timed out) must not fail unhandled. */ + /** Once the settings and the lanes are in, and only if some lane could be looked up: reading + * the user's token can refresh and rotate it. */ + const userTokenRead = Promise.all([configRead, laneRead]).then(([loaded, lanes]) => { + /** Never started for a request already answered or out of time: nobody would wait for + * it, and it could still refresh, extend or refuse the stored grant. */ + const deadline = + startedAt + (loaded?.batchTimeoutSeconds ?? DEFAULT_BATCH_TIMEOUT_SECONDS) * 1000; + if (answered || Date.now() >= deadline) return null; + return lanes.some(({ laneGit }) => isLookupable(laneGit)) + ? readUserToken(deps.userToken, userId, loaded, deps.env) + : null; + }); + /** A read nobody waits for (feature off, config timed out) must not fail unhandled. */ laneRead.catch(() => undefined); + userTokenRead.catch(() => undefined); const unavailable = () => res .status(503) @@ -400,15 +519,35 @@ export function createConversationPullRequestsHandler(deps: { return; } - const lanes = await withDeadline(laneRead, deadline - Date.now()); - if (lanes === TIMED_OUT) { + const reads = await withDeadline( + Promise.all([laneRead, userTokenRead]), + deadline - Date.now(), + ); + if (reads === TIMED_OUT) { unavailable(); return; } + const [lanes, userToken] = reads; + const credential = credentialOf(settings, deps.env, userToken); + const userTokenFailed = userToken === USER_TOKEN_FAILED; + /** Users can connect their own account and this one has not: nothing to look up with. */ + if ( + credential == null && + !userTokenFailed && + resolvePullRequestOAuth(settings, deps.env) != null + ) { + res.status(200).json(none()); + return; + } + /** The repositories the missing credential would have been used for: every one, for a + * user's own token that failed to load. */ + const repositories = + credential?.repositories ?? + (userTokenFailed ? ALL_REPOSITORIES : repositoryScope(settings)); const eligible = new Map(); for (const { conversationId, laneGit } of lanes) { - const lane = eligibleLane(laneGit, settings); + const lane = eligibleLane(laneGit, repositories); if (lane != null) eligible.set(conversationId, lane); } if (eligible.size === 0) { @@ -420,18 +559,15 @@ export function createConversationPullRequestsHandler(deps: { * A missing token is a fact about the conversations that needed one, not about the request: * those entries say so, and every other conversation keeps its correct "no pull request". */ - const token = resolvePullRequestToken( - settings.token, - deps.env, - resolvePullRequestProvider(settings.provider) ?? undefined, - ); - if (token == null) { - logger.warn('[PullRequests] Enabled without a usable token reference'); + if (credential == null) { + if (!userTokenFailed) + logger.warn('[PullRequests] Enabled without a usable token reference'); + const code = userTokenFailed ? 'UPSTREAM_ERROR' : 'NOT_CONFIGURED'; res.status(200).json({ results: ids.map( (conversationId): TConversationPullRequestsEntry => eligible.has(conversationId) - ? { conversationId, error: { code: 'NOT_CONFIGURED' } } + ? { conversationId, error: { code } } : { conversationId, pullRequest: null }, ), }); @@ -446,7 +582,7 @@ export function createConversationPullRequestsHandler(deps: { * limit and fills the cache for the next request. */ const limit = settings.maxConcurrentLookups ?? DEFAULT_BATCH_CONCURRENCY; - const scope = limiterScope(token); + const scope = limiterScope(credential.token); /** * Conversations that share a repository, branch and head share one answer, and the lookup * already coalesces them, so each distinct lookup takes one slot and one deadline, not one @@ -467,7 +603,7 @@ export function createConversationPullRequestsHandler(deps: { outcomes.set(key, { ok: false, code: 'UPSTREAM_ERROR' }); return; } - const input = lookupInput(settings, token, lane); + const input = lookupInput(settings, credential, lane); /** The same lane under the same policy is one lookup for every request that wants it. */ const shareKey = `${scope}\0${JSON.stringify([ input.provider, @@ -497,18 +633,24 @@ export function createConversationPullRequestsHandler(deps: { result.ok ? { ok: true, value: result.value } : { ok: false, code: result.error.code }, ); }); + if ([...outcomes.values()].some((o) => !o.ok && o.code === 'CREDENTIAL_REJECTED')) { + /** In the background: the answer does not wait on it, and it fails only to a log. */ + void invalidateRejected(deps.userToken, userId, settings, credential); + } const looked = ids.map((conversationId): TConversationPullRequestsEntry => { const lane = eligible.get(conversationId); const outcome = lane == null ? undefined : outcomes.get(keyOf(lane)); if (outcome == null) return { conversationId, pullRequest: null }; return outcome.ok ? { conversationId, pullRequest: outcome.value } - : { conversationId, error: { code: outcome.code } }; + : { conversationId, error: { code: wireCode(outcome.code) } }; }); res.status(200).json({ results: looked }); } catch (error) { logger.error('[PullRequests] Batch handler failed', getSafeErrorMetadata(error)); res.status(500).json({ error: 'Failed to load the pull requests' }); + } finally { + answered = true; } }; } diff --git a/packages/api/src/pulls/index.ts b/packages/api/src/pulls/index.ts index fe3776e1daa..2eb0ab52ad7 100644 --- a/packages/api/src/pulls/index.ts +++ b/packages/api/src/pulls/index.ts @@ -4,6 +4,8 @@ export * from './github'; export * from './gitlab'; export * from './bitbucket'; export * from './sources'; +export * from './oauth'; +export * from './connect'; export * from './lookup'; export * from './handler'; export * from './settings'; diff --git a/packages/api/src/pulls/oauth.spec.ts b/packages/api/src/pulls/oauth.spec.ts new file mode 100644 index 00000000000..e4f87810480 --- /dev/null +++ b/packages/api/src/pulls/oauth.spec.ts @@ -0,0 +1,1650 @@ +import jwt from 'jsonwebtoken'; +import mongoose from 'mongoose'; +import { createHash } from 'node:crypto'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import { createMethods, createModels } from '@librechat/data-schemas'; +import type { PullRequestOAuthConfig } from './settings'; +import type { PullRequestOAuthStore } from './oauth'; +import { + oauthEndpoints, + createPullRequestOAuth, + PULL_REQUEST_OAUTH_TYPE, + PULL_REQUEST_OAUTH_GENERATION_TYPE, +} from './oauth'; + +const secret = 'test-secret'; +const redirectUri = 'https://chat.example.com/api/pull-requests/oauth/callback'; +const github: PullRequestOAuthConfig = { + provider: 'github', + host: 'github.com', + clientId: 'client-1', + clientSecret: 'client-secret', +}; +const gitlab: PullRequestOAuthConfig = { + provider: 'gitlab', + host: 'gitlab.example.com', + clientId: 'gl-client', + clientSecret: 'gl-secret', +}; +/** Reversible, and never the plaintext, so a test can see what was stored and that it was hidden. */ +const encrypt = async (value: string) => `enc:${Buffer.from(value).toString('base64')}`; +const decrypt = async (value: string) => + Buffer.from(value.replace(/^enc:/, ''), 'base64').toString('utf8'); + +let mongoServer: MongoMemoryServer; +let store: PullRequestOAuthStore; + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + createModels(mongoose); + store = createMethods(mongoose) as unknown as PullRequestOAuthStore; +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +beforeEach(async () => { + await mongoose.connection.db?.collection('tokens').deleteMany({}); +}); + +const userId = () => new mongoose.Types.ObjectId().toString(); +const json = (body: unknown, init?: ResponseInit) => Response.json(body, init); + +const settle = () => new Promise((resolve) => setImmediate(resolve)); +async function waitFor(condition: () => boolean): Promise { + for (let i = 0; i < 200 && !condition(); i++) { + await new Promise((resolve) => setTimeout(resolve, 5)); + } + if (!condition()) throw new Error('condition not reached'); +} +const users = () => mongoose.connection.db!.collection('users'); +const tokens = () => mongoose.connection.db!.collection('tokens'); + +function setup(responses: Array<() => Response | Promise>, now = () => Date.now()) { + const fetchFn = jest.fn(async (_input: string, _init?: RequestInit) => { + const next = responses.shift(); + if (next == null) throw new Error('unexpected request'); + return next(); + }); + const oauth = createPullRequestOAuth({ + store, + fetchFn, + secret, + redirectUri, + encrypt, + decrypt, + now, + }); + return { oauth, fetchFn }; +} + +const formOf = (init: RequestInit | undefined) => new URLSearchParams(String(init?.body)); + +async function connect( + oauth: ReturnType['oauth'], + user: string, + config = github, +): Promise { + /** Completion stores only for a user that exists and is not being deleted. */ + await users().updateOne( + { _id: new mongoose.Types.ObjectId(user) }, + { $setOnInsert: { email: `${user}@example.com` } }, + { upsert: true }, + ); + const { url } = await oauth.authorizationUrl({ id: user }, config); + const state = new URL(url).searchParams.get('state') ?? ''; + const claims = oauth.verifyState(state); + if (claims == null) throw new Error('state did not verify'); + await oauth.complete(claims, 'the-code', config); +} + +/** A replica whose next read of the grant row waits until released. */ +function heldReplica(responses: Array<() => Response> = []) { + let release: () => void = () => undefined; + const held = new Promise((resolve) => (release = resolve)); + let reached: () => void = () => undefined; + const waiting = new Promise((resolve) => (reached = resolve)); + let holding = true; + const heldStore: PullRequestOAuthStore = { + ...store, + findToken: async (query) => { + if (holding && query.scope.startsWith(`${PULL_REQUEST_OAUTH_TYPE}:`)) { + holding = false; + const row = await store.findToken(query); + reached(); + await held; + return row; + } + return store.findToken(query); + }, + }; + const oauth = createPullRequestOAuth({ + store: heldStore, + fetchFn: jest.fn(async () => { + const next = responses.shift(); + if (next == null) throw new Error('unexpected request'); + return next(); + }), + secret, + redirectUri, + encrypt, + decrypt, + }); + return { oauth, release, waiting }; +} + +describe('oauthEndpoints', () => { + it.each([ + ['github', 'github.com', 'https://github.com/login/oauth/authorize'], + ['github', 'ghe.example.com', 'https://ghe.example.com/login/oauth/authorize'], + ['gitlab', 'gitlab.example.com', 'https://gitlab.example.com/oauth/authorize'], + ['bitbucket', 'bitbucket.org', 'https://bitbucket.org/site/oauth2/authorize'], + ] as const)('authorizes %s on %s at %s', (provider, host, authorize) => { + expect(oauthEndpoints(provider, host).authorize).toBe(authorize); + }); +}); + +describe('authorizationUrl and verifyState', () => { + it('sends the user to the provider with a signed state and an S256 challenge', async () => { + const { oauth } = setup([]); + const { url, flowId } = await oauth.authorizationUrl({ id: 'u1', tenantId: 't1' }, gitlab); + const parsed = new URL(url); + expect(parsed.origin + parsed.pathname).toBe('https://gitlab.example.com/oauth/authorize'); + expect(parsed.searchParams.get('client_id')).toBe('gl-client'); + expect(parsed.searchParams.get('redirect_uri')).toBe(redirectUri); + expect(parsed.searchParams.get('scope')).toBe('read_api'); + expect(parsed.searchParams.get('code_challenge_method')).toBe('S256'); + expect(parsed.searchParams.get('client_secret')).toBeNull(); + const claims = oauth.verifyState(parsed.searchParams.get('state') ?? ''); + expect(claims).toMatchObject({ + userId: 'u1', + provider: 'gitlab', + host: 'gitlab.example.com', + tenantId: 't1', + flowId, + }); + }); + + it('asks GitHub for no scope unless one is configured', async () => { + const { oauth } = setup([]); + expect( + new URL((await oauth.authorizationUrl({ id: 'u1' }, github)).url).searchParams.has('scope'), + ).toBe(false); + const scoped = await oauth.authorizationUrl({ id: 'u1' }, { ...github, scope: 'repo' }); + expect(new URL(scoped.url).searchParams.get('scope')).toBe('repo'); + }); + + it('starts a new flow each time', async () => { + const { oauth } = setup([]); + const first = await oauth.authorizationUrl({ id: 'u1' }, github); + const second = await oauth.authorizationUrl({ id: 'u1' }, github); + expect(first.flowId).not.toBe(second.flowId); + }); + + it.each([ + ['garbage', 'not-a-token'], + ['another secret', jwt.sign({ sub: 'u1', p: 'github', h: 'github.com', n: 'x' }, 'other')], + [ + 'another audience', + jwt.sign({ sub: 'u1', p: 'github', h: 'github.com', n: 'x' }, secret, { audience: 'x' }), + ], + [ + 'an expired state', + jwt.sign({ sub: 'u1', p: 'github', h: 'github.com', n: 'x' }, secret, { + audience: 'librechat:pull-request-oauth', + expiresIn: -1, + }), + ], + [ + 'an unknown provider', + jwt.sign({ sub: 'u1', p: 'gitea', h: 'x', n: 'x' }, secret, { + audience: 'librechat:pull-request-oauth', + }), + ], + ])('refuses %s', (_label, state) => { + const { oauth } = setup([]); + expect(oauth.verifyState(state)).toBeNull(); + }); +}); + +describe('complete', () => { + it('exchanges the code with the verifier behind the challenge and stores the grant encrypted', async () => { + const { oauth, fetchFn } = setup([ + () => json({ access_token: 'gho_access', token_type: 'bearer' }), + ]); + const user = userId(); + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + }); + const { url } = await oauth.authorizationUrl({ id: user }, github); + const challenge = new URL(url).searchParams.get('code_challenge'); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + await oauth.complete(claims!, 'the-code', github); + + const [endpoint, init] = fetchFn.mock.calls[0]; + expect(endpoint).toBe('https://github.com/login/oauth/access_token'); + const form = formOf(init); + expect(form.get('code')).toBe('the-code'); + expect(form.get('redirect_uri')).toBe(redirectUri); + expect(form.get('client_secret')).toBe('client-secret'); + const verifier = form.get('code_verifier') ?? ''; + expect(createHash('sha256').update(verifier).digest('base64url')).toBe(challenge); + + const row = await mongoose.connection + .db!.collection('tokens') + .findOne({ type: PULL_REQUEST_OAUTH_TYPE }); + expect(row?.identifier).toBe('github:github.com'); + expect(String(row?.token)).not.toContain('gho_access'); + await expect(oauth.token(user, github)).resolves.toBe('gho_access'); + }); + + it('authenticates to Bitbucket with Basic credentials, not in the form', async () => { + const bitbucket: PullRequestOAuthConfig = { + ...github, + provider: 'bitbucket', + host: 'bitbucket.org', + }; + const { oauth, fetchFn } = setup([() => json({ access_token: 'bb', expires_in: 7200 })]); + await connect(oauth, userId(), bitbucket); + const init = fetchFn.mock.calls[0][1]; + expect((init?.headers as Record).Authorization).toBe( + `Basic ${Buffer.from('client-1:client-secret').toString('base64')}`, + ); + expect(formOf(init).has('client_secret')).toBe(false); + }); + + it('replaces an earlier connection of the same user', async () => { + const { oauth } = setup([ + () => json({ access_token: 'first' }), + () => json({ access_token: 'second' }), + ]); + const user = userId(); + await connect(oauth, user); + await connect(oauth, user); + await expect(oauth.token(user, github)).resolves.toBe('second'); + expect( + await mongoose.connection + .db!.collection('tokens') + .countDocuments({ type: PULL_REQUEST_OAUTH_TYPE }), + ).toBe(1); + }); + + it.each([ + ['GitHub style, 200 with an error', () => json({ error: 'bad_verification_code' })], + ['a 400', () => json({ error: 'invalid_grant' }, { status: 400 })], + ['a 500', () => new Response('', { status: 500 })], + ])('stores nothing when the provider refuses the code (%s)', async (_label, response) => { + const { oauth } = setup([response]); + const user = userId(); + await expect(connect(oauth, user)).rejects.toThrow(); + await expect(oauth.connected(user, github)).resolves.toBe(false); + }); +}); + +describe('token', () => { + it('is null for a user who never connected', async () => { + const { oauth } = setup([]); + await expect(oauth.token(userId(), github)).resolves.toBeNull(); + }); + + it('keeps users apart', async () => { + const { oauth } = setup([() => json({ access_token: 'mine' })]); + const user = userId(); + await connect(oauth, user); + await expect(oauth.token(userId(), github)).resolves.toBeNull(); + await expect(oauth.token(user, github)).resolves.toBe('mine'); + }); + + it('treats a grant from a replaced app as no connection', async () => { + const { oauth } = setup([() => json({ access_token: 'old-app' })]); + const user = userId(); + await connect(oauth, user); + await expect(oauth.token(user, { ...github, clientId: 'client-2' })).resolves.toBeNull(); + /** Left alone: a replica still on the other config, mid-rollout, must not destroy it. */ + await expect(oauth.token(user, github)).resolves.toBe('old-app'); + }); + + it('refreshes an expiring token once for concurrent callers and keeps a refresh token that did not rotate', async () => { + let clock = 1_000_000; + const { oauth, fetchFn } = setup( + [ + () => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 7200 }), + () => json({ access_token: 'a2', expires_in: 7200 }), + () => json({ access_token: 'a3', expires_in: 7200 }), + ], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + clock += 7200 * 1000; + const tokens = await Promise.all([oauth.token(user, gitlab), oauth.token(user, gitlab)]); + expect(tokens).toEqual(['a2', 'a2']); + expect(fetchFn).toHaveBeenCalledTimes(2); + const refreshForm = formOf(fetchFn.mock.calls[1][1]); + expect(refreshForm.get('grant_type')).toBe('refresh_token'); + expect(refreshForm.get('refresh_token')).toBe('r1'); + + clock += 7200 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBe('a3'); + expect(formOf(fetchFn.mock.calls[2][1]).get('refresh_token')).toBe('r1'); + }); + + it('serves a grant without a refresh token until it expires, then reports it disconnected', async () => { + let clock = 1_000_000; + const { oauth, fetchFn } = setup( + [() => json({ access_token: 'a1', expires_in: 60 })], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + /** Inside the refresh margin, but with nothing to refresh with it is still the token. */ + clock += 50 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBe('a1'); + await expect(oauth.connected(user, gitlab)).resolves.toBe(true); + clock += 20 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBeNull(); + await expect(oauth.connected(user, gitlab)).resolves.toBe(false); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + it('refreshes once when a concurrent caller reads the grant after the refresh finished', async () => { + let clock = 1_000_000; + const { oauth: connector } = setup( + [() => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 7200 })], + () => clock, + ); + const user = userId(); + await connect(connector, user, gitlab); + /** The second read of the stored grant answers only once the first caller is done. */ + let release: () => void = () => undefined; + const firstDone = new Promise((resolve) => (release = resolve)); + let reads = 0; + const slowStore: PullRequestOAuthStore = { + ...store, + findToken: async (query) => { + const row = await store.findToken(query); + /** Only reads of the grant itself; the generation is read around them. */ + if (!query.scope.startsWith(`${PULL_REQUEST_OAUTH_TYPE}:`)) return row; + reads += 1; + if (reads === 2) await firstDone; + return row; + }, + }; + const fetchFn = jest.fn(async () => json({ access_token: 'a2', expires_in: 7200 })); + const oauth = createPullRequestOAuth({ + store: slowStore, + fetchFn, + secret, + redirectUri, + encrypt, + decrypt, + now: () => clock, + }); + clock += 7200 * 1000; + const first = oauth.token(user, gitlab).finally(release); + const second = oauth.token(user, gitlab); + await expect(Promise.all([first, second])).resolves.toEqual(['a2', 'a2']); + expect(fetchFn).toHaveBeenCalledTimes(1); + }); + + it('disconnects a user whose refresh token the provider rejects', async () => { + let clock = 1_000_000; + const { oauth } = setup( + [ + () => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 }), + () => json({ error: 'invalid_grant' }, { status: 400 }), + ], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + clock += 120 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBeNull(); + await expect(oauth.connected(user, gitlab)).resolves.toBe(false); + }); + + it.each([ + ['GitHub style, 200', () => json({ error: 'bad_refresh_token' })], + ['a 401', () => json({ error: 'invalid_grant' }, { status: 401 })], + ])('treats a refused refresh token as a disconnection (%s)', async (_label, refusal) => { + let clock = 1_000_000; + const { oauth } = setup( + [() => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 }), refusal], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + clock += 120 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBeNull(); + await expect(oauth.connected(user, gitlab)).resolves.toBe(false); + }); + + it.each([ + ['invalid_client, 401', () => json({ error: 'invalid_client' }, { status: 401 })], + ['invalid_client, 400', () => json({ error: 'invalid_client' }, { status: 400 })], + [ + 'GitHub incorrect_client_credentials, 200', + () => json({ error: 'incorrect_client_credentials' }), + ], + ['a 400 without a body', () => new Response('', { status: 400 })], + ])('keeps the grant when the error is not about it (%s)', async (_label, failure) => { + let clock = 1_000_000; + const { oauth } = setup( + [ + () => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 }), + failure, + () => json({ access_token: 'a2', refresh_token: 'r2', expires_in: 7200 }), + ], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + clock += 120 * 1000; + await expect(oauth.token(user, gitlab)).rejects.toThrow('Token endpoint answered'); + await expect(oauth.connected(user, gitlab)).resolves.toBe(true); + /** Once the secret is fixed, the same grant refreshes. */ + await expect(oauth.token(user, gitlab)).resolves.toBe('a2'); + }); + + /** A refresh held at the provider until the test answers it. */ + function heldRefresh(clock: () => number, body: Record) { + let answer: () => void = () => undefined; + const answered = new Promise((resolve) => (answer = resolve)); + /** Resolves once the request is at the provider, which is after the claim is stored. */ + let reached: () => void = () => undefined; + const requested = new Promise((resolve) => (reached = resolve)); + const replica = setup([], clock); + replica.fetchFn.mockImplementationOnce(async () => { + reached(); + await answered; + return json(body); + }); + return { ...replica, answer, requested }; + } + + async function connectExpiring(clock: () => number) { + const { oauth } = setup( + [() => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 })], + clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + return user; + } + + it('lets one replica at a time spend the refresh token', async () => { + let clock = 1_000_000; + const user = await connectExpiring(() => clock); + clock += 120 * 1000; + const replicaA = heldRefresh(() => clock, { + access_token: 'a2', + refresh_token: 'r2', + expires_in: 7200, + }); + const first = replicaA.oauth.token(user, gitlab); + await replicaA.requested; + /** B finds A's claim and does not send the same refresh token. */ + const replicaB = setup([], () => clock); + await expect(replicaB.oauth.token(user, gitlab)).rejects.toThrow('Another refresh'); + expect(replicaB.fetchFn).not.toHaveBeenCalled(); + await expect(replicaB.oauth.connected(user, gitlab)).resolves.toBe(true); + replicaA.answer(); + await expect(first).resolves.toBe('a2'); + await expect(replicaB.oauth.token(user, gitlab)).resolves.toBe('a2'); + }); + + it("stores a stalled replica's refresh over another replica's refusal of the same grant", async () => { + let clock = 1_000_000; + const user = await connectExpiring(() => clock); + clock += 120 * 1000; + const replicaA = heldRefresh(() => clock, { + access_token: 'a2', + refresh_token: 'r2', + expires_in: 7200, + }); + const first = replicaA.oauth.token(user, gitlab); + await replicaA.requested; + /** A outlives its claim; B claims the grant and is refused the token A already rotated. */ + clock += 60 * 1000; + const replicaB = setup([() => json({ error: 'invalid_grant' }, { status: 400 })], () => clock); + await expect(replicaB.oauth.token(user, gitlab)).resolves.toBeNull(); + await expect(replicaB.oauth.connected(user, gitlab)).resolves.toBe(false); + /** A's answer is the only grant the provider still accepts, so it is kept. */ + replicaA.answer(); + await expect(first).resolves.toBe('a2'); + await expect(replicaB.oauth.connected(user, gitlab)).resolves.toBe(true); + await expect(replicaB.oauth.token(user, gitlab)).resolves.toBe('a2'); + }); + + it('does not bring back a connection removed while its refresh was in flight', async () => { + let clock = 1_000_000; + const user = await connectExpiring(() => clock); + clock += 120 * 1000; + const replica = heldRefresh(() => clock, { + access_token: 'a2', + refresh_token: 'r2', + expires_in: 7200, + }); + const pending = replica.oauth.token(user, gitlab); + await replica.requested; + await replica.oauth.disconnect(user); + replica.answer(); + await expect(pending).resolves.toBeNull(); + await expect(replica.oauth.connected(user, gitlab)).resolves.toBe(false); + expect( + await tokens().countDocuments({ + userId: new mongoose.Types.ObjectId(user), + type: PULL_REQUEST_OAUTH_TYPE, + }), + ).toBe(0); + }); + + it('releases its claim after an outage, so the next attempt need not wait it out', async () => { + let clock = 1_000_000; + const user = await connectExpiring(() => clock); + clock += 120 * 1000; + const { oauth } = setup([() => new Response('', { status: 503 })], () => clock); + await expect(oauth.token(user, gitlab)).rejects.toThrow('Token endpoint answered'); + const other = setup( + [() => json({ access_token: 'a2', refresh_token: 'r2', expires_in: 7200 })], + () => clock, + ); + await expect(other.oauth.token(user, gitlab)).resolves.toBe('a2'); + }); + + it('does not hand a read under a shortened retention the answer of one under the old', async () => { + let clock = Date.now(); + const long = { ...github, retentionDays: 180 }; + const { oauth: connector } = setup([() => json({ access_token: 'kept' })], () => clock); + const user = userId(); + await connect(connector, user, long); + clock += 2 * 24 * 60 * 60 * 1000; + /** The read under the old retention is held at the store until the new one has answered. */ + let release: () => void = () => undefined; + const held = new Promise((resolve) => (release = resolve)); + let reads = 0; + const heldStore: PullRequestOAuthStore = { + ...store, + findToken: async (query) => { + reads += 1; + if (reads === 1) await held; + return store.findToken(query); + }, + }; + const oauth = createPullRequestOAuth({ + store: heldStore, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt, + now: () => clock, + }); + const old = oauth.token(user, long); + /** Unused for two days: under a one-day retention it is gone, whatever the old read says. */ + const shortened = oauth.token(user, { ...github, retentionDays: 1 }); + await expect(Promise.race([shortened, held.then(() => 'joined')])).resolves.toBeNull(); + release(); + await old; + }); + + it("does not hand a replaced app's token to a read under the new app", async () => { + let clock = 1_000_000; + const user = await connectExpiring(() => clock); + clock += 120 * 1000; + const replica = heldRefresh(() => clock, { + access_token: 'a2', + refresh_token: 'r2', + expires_in: 7200, + }); + const old = replica.oauth.token(user, gitlab); + /** Same provider and host, another app: its own read, which finds no grant of its own. */ + const replaced = replica.oauth.token(user, { ...gitlab, clientId: 'gl-client-2' }); + await expect(replaced).resolves.toBeNull(); + replica.answer(); + await expect(old).resolves.toBe('a2'); + await expect( + replica.oauth.token(user, { ...gitlab, clientId: 'gl-client-2' }), + ).resolves.toBeNull(); + }); + + it('applies a shortened retention to a grant stored under a longer one', async () => { + let clock = Date.now(); + const { oauth } = setup([() => json({ access_token: 'kept' })], () => clock); + const user = userId(); + await connect(oauth, user, { ...github, retentionDays: 180 }); + const shortened = { ...github, retentionDays: 1 }; + /** Used now: served, and the row is cut down to the new retention. */ + await expect(oauth.token(user, shortened)).resolves.toBe('kept'); + await settle(); + await settle(); + const row = await tokens().findOne({ userId: new mongoose.Types.ObjectId(user) }); + /** The store stamps the row on its own clock. */ + expect(new Date(row?.expiresAt).getTime()).toBeLessThanOrEqual( + Date.now() + 24 * 60 * 60 * 1000, + ); + /** Then left unused past it: gone, and the row removed. */ + clock += 2 * 24 * 60 * 60 * 1000; + await expect(oauth.token(user, shortened)).resolves.toBeNull(); + await expect(oauth.connected(user, shortened)).resolves.toBe(false); + }); + + it('serves a grant that expires without a refresh token without rewriting it on every use', async () => { + const { oauth } = setup([() => json({ access_token: 'fixed', expires_in: 7200 })]); + const user = userId(); + await connect(oauth, user, gitlab); + const writes = jest.spyOn(store, 'replaceTokenIfCurrent'); + for (let i = 0; i < 3; i++) { + await expect(oauth.token(user, gitlab)).resolves.toBe('fixed'); + } + await settle(); + expect(writes).not.toHaveBeenCalled(); + writes.mockRestore(); + }); + + it('applies the current retention again when another replica changed the row first', async () => { + const { oauth } = setup([() => json({ access_token: 'kept' })]); + const user = userId(); + await connect(oauth, user, { ...github, retentionDays: 180 }); + const original = store.replaceTokenIfCurrent.bind(store); + let grantWrites = 0; + const race = jest + .spyOn(store, 'replaceTokenIfCurrent') + .mockImplementation(async (scope, expected, row) => { + if (row.type === PULL_REQUEST_OAUTH_TYPE) { + grantWrites += 1; + /** The first rewrite loses to a replica that wrote the row in between. */ + if (grantWrites === 1) return false; + } + return original(scope, expected, row); + }); + try { + await expect(oauth.token(user, { ...github, retentionDays: 1 })).resolves.toBe('kept'); + } finally { + race.mockRestore(); + } + expect(grantWrites).toBe(2); + const row = await tokens().findOne({ + userId: new mongoose.Types.ObjectId(user), + type: PULL_REQUEST_OAUTH_TYPE, + }); + expect(new Date(row?.expiresAt).getTime()).toBeLessThanOrEqual(Date.now() + 86_400_000); + }); + + it('ends a grant unused for longer than the current retention, whatever its row says', async () => { + let clock = Date.now(); + const { oauth } = setup([() => json({ access_token: 'old' })], () => clock); + const user = userId(); + await connect(oauth, user, { ...github, retentionDays: 180 }); + clock += 2 * 24 * 60 * 60 * 1000; + await expect(oauth.token(user, { ...github, retentionDays: 1 })).resolves.toBeNull(); + await settle(); + expect( + await tokens().countDocuments({ + userId: new mongoose.Types.ObjectId(user), + type: PULL_REQUEST_OAUTH_TYPE, + }), + ).toBe(0); + }); + + it('reports a failed removal of a grant past the retention as a failure, not as absence', async () => { + let clock = Date.now(); + const { oauth: connector } = setup([() => json({ access_token: 'old' })], () => clock); + const user = userId(); + await connect(connector, user, { ...github, retentionDays: 180 }); + clock += 2 * 24 * 60 * 60 * 1000; + const failing = createPullRequestOAuth({ + store: { ...store, deleteTokens: jest.fn().mockRejectedValue(new Error('mongo down')) }, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt, + now: () => clock, + }); + const shortened = { ...github, retentionDays: 1 }; + await expect(failing.token(user, shortened)).rejects.toThrow('mongo down'); + await expect(failing.connected(user, shortened)).rejects.toThrow('mongo down'); + }); + + it('treats a row past its retention as gone, and does not extend it', async () => { + let clock = Date.now(); + const config = { ...github, retentionDays: 1 }; + const { oauth } = setup([() => json({ access_token: 'old' })], () => clock); + const user = userId(); + await connect(oauth, user, config); + const before = await tokens().findOne({ userId: new mongoose.Types.ObjectId(user) }); + expect(before?.expiresAt).toBeInstanceOf(Date); + clock += 2 * 24 * 60 * 60 * 1000; + await expect(oauth.token(user, config)).resolves.toBeNull(); + await expect(oauth.connected(user, config)).resolves.toBe(false); + await settle(); + const after = await tokens().findOne({ userId: new mongoose.Types.ObjectId(user) }); + expect(after?.expiresAt).toEqual(before?.expiresAt); + }); + + it.each([ + ['an empty server error', () => new Response('', { status: 503 })], + [ + 'a server error with an OAuth error body', + () => json({ error: 'temporarily_unavailable' }, { status: 503 }), + ], + ])('keeps the connection and reports the outage on %s', async (_name, outage) => { + let clock = 1_000_000; + const { oauth } = setup( + [() => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 }), outage], + () => clock, + ); + const user = userId(); + await connect(oauth, user, gitlab); + clock += 120 * 1000; + await expect(oauth.token(user, gitlab)).rejects.toThrow('Token endpoint answered 503'); + await expect(oauth.connected(user, gitlab)).resolves.toBe(true); + }); + + it('gives up on a token endpoint slower than the configured request timeout', async () => { + const fetchFn = jest.fn( + (_input: string, init?: RequestInit) => + new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => reject(init.signal?.reason)); + }), + ); + const oauth = createPullRequestOAuth({ store, fetchFn, secret, redirectUri, encrypt, decrypt }); + const { url } = await oauth.authorizationUrl({ id: userId() }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + const started = Date.now(); + await expect( + oauth.complete(claims!, 'the-code', { ...github, requestTimeoutSeconds: 1 }), + ).rejects.toThrow(); + expect(Date.now() - started).toBeLessThan(5_000); + }); + + it('renews the connection generation with a grant still in use, so it never expires first', async () => { + const { oauth } = setup([() => json({ access_token: 'forever' })]); + const user = userId(); + const config = { ...github, retentionDays: 10 }; + await connect(oauth, user, config); + const tokens = mongoose.connection.db!.collection('tokens'); + const owner = { userId: new mongoose.Types.ObjectId(user) }; + /** Nearly ten years on, the generation is about to go while the grant is used every day. */ + const soon = new Date(Date.now() + 86_400_000); + await tokens.updateOne( + { ...owner, type: PULL_REQUEST_OAUTH_GENERATION_TYPE }, + { $set: { expiresAt: soon } }, + ); + await tokens.updateOne( + { ...owner, type: PULL_REQUEST_OAUTH_TYPE }, + { $set: { expiresAt: new Date(Date.now() + 8 * 86_400_000) } }, + ); + await expect(oauth.token(user, config)).resolves.toBe('forever'); + const generation = await tokens.findOne({ ...owner, type: PULL_REQUEST_OAUTH_GENERATION_TYPE }); + expect((generation!.expiresAt as Date).getTime() - Date.now()).toBeGreaterThan( + 3000 * 86_400_000, + ); + await expect(oauth.token(user, config)).resolves.toBe('forever'); + }); + + it('extends a grant that never expires on its first use each day', async () => { + const { oauth } = setup([() => json({ access_token: 'forever' })]); + const user = userId(); + const config = { ...github, retentionDays: 10 }; + await connect(oauth, user, config); + const tokens = mongoose.connection.db!.collection('tokens'); + const first = (await tokens.findOne({ type: PULL_REQUEST_OAUTH_TYPE }))!.expiresAt as Date; + expect(first.getTime() - Date.now()).toBeGreaterThan(9.9 * 86_400_000); + + /** Used early: nothing to extend. */ + await expect(oauth.token(user, config)).resolves.toBe('forever'); + await new Promise((resolve) => setImmediate(resolve)); + expect( + ((await tokens.findOne({ type: PULL_REQUEST_OAUTH_TYPE }))!.expiresAt as Date).getTime(), + ).toBe(first.getTime()); + + /** Two days in, well before half the retention: using it still starts the retention again, so + * the grant stays for the whole window after its last use. */ + await tokens.updateOne( + { type: PULL_REQUEST_OAUTH_TYPE }, + { $set: { expiresAt: new Date(Date.now() + 8 * 86_400_000) } }, + ); + await expect(oauth.token(user, config)).resolves.toBe('forever'); + await new Promise((resolve) => setTimeout(resolve, 100)); + const extended = (await tokens.findOne({ type: PULL_REQUEST_OAUTH_TYPE }))!.expiresAt as Date; + expect(extended.getTime() - Date.now()).toBeGreaterThan(9.9 * 86_400_000); + await expect(oauth.token(user, config)).resolves.toBe('forever'); + }); + + it('reports a failed extension as a failure, not as a successful read', async () => { + const { oauth } = setup([() => json({ access_token: 'forever' })]); + const user = userId(); + const config = { ...github, retentionDays: 10 }; + await connect(oauth, user, config); + const tokens = mongoose.connection.db!.collection('tokens'); + await tokens.updateOne( + { type: PULL_REQUEST_OAUTH_TYPE }, + { $set: { expiresAt: new Date(Date.now() + 4 * 86_400_000) } }, + ); + const write = jest + .spyOn(store, 'replaceTokenIfCurrent') + .mockRejectedValueOnce(new Error('write failed')); + try { + await expect(oauth.token(user, config)).rejects.toThrow('write failed'); + } finally { + write.mockRestore(); + } + }); + + it('takes the grant another replica stored when its refresh lost the race', async () => { + let clock = 1_000_000; + const replicaA = setup( + [() => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 })], + () => clock, + ); + const user = userId(); + await connect(replicaA.oauth, user, gitlab); + clock += 120 * 1000; + /** Replica B refreshed first and rotated the refresh token; A's refresh is then refused. */ + const replicaB = setup( + [() => json({ access_token: 'b2', refresh_token: 'r2', expires_in: 7200 })], + () => clock, + ); + await expect(replicaB.oauth.token(user, gitlab)).resolves.toBe('b2'); + const stale = setup([() => json({ error: 'invalid_grant' }, { status: 400 })], () => clock); + await expect(stale.oauth.token(user, gitlab)).resolves.toBe('b2'); + await expect(stale.oauth.connected(user, gitlab)).resolves.toBe(true); + }); +}); + +describe('a grant that cannot be read', () => { + it('reports a decryption failure as a failure, not as a disconnection', async () => { + const { oauth } = setup([() => json({ access_token: 'kept' })]); + const user = userId(); + await connect(oauth, user); + const broken = createPullRequestOAuth({ + store, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt: async () => { + throw new Error('key unavailable'); + }, + }); + await expect(broken.token(user, github)).rejects.toThrow('key unavailable'); + await expect(broken.connected(user, github)).rejects.toThrow('key unavailable'); + }); + + it.each([ + ['text that is not JSON', 'not json'], + ['JSON that is not a grant', JSON.stringify({ accessToken: 7, clientId: github.clientId })], + ['a grant without its app', JSON.stringify({ accessToken: 'kept' })], + ])('reports %s as a failure, not as a disconnection', async (_label, plaintext) => { + const { oauth } = setup([() => json({ access_token: 'kept' })]); + const user = userId(); + await connect(oauth, user); + const garbled = createPullRequestOAuth({ + store, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt: async () => plaintext, + }); + await expect(garbled.token(user, github)).rejects.toThrow('malformed'); + await expect(garbled.connected(user, github)).rejects.toThrow('malformed'); + }); +}); + +describe('status', () => { + it('names the sign-in that made the connection, and keeps it through a refresh', async () => { + let clock = 1_000_000; + const { oauth } = setup( + [ + () => json({ access_token: 'a1', refresh_token: 'r1', expires_in: 60 }), + () => json({ access_token: 'a2', refresh_token: 'r2', expires_in: 7200 }), + ], + () => clock, + ); + const user = userId(); + await users().updateOne( + { _id: new mongoose.Types.ObjectId(user) }, + { $setOnInsert: { email: `${user}@example.com` } }, + { upsert: true }, + ); + const { url, flowId } = await oauth.authorizationUrl({ id: user }, gitlab); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + await oauth.complete(claims!, 'the-code', gitlab); + await expect(oauth.status(user, gitlab)).resolves.toEqual({ connected: true, flowId }); + clock += 120 * 1000; + await expect(oauth.token(user, gitlab)).resolves.toBe('a2'); + await expect(oauth.status(user, gitlab)).resolves.toEqual({ connected: true, flowId }); + }); + + it('reports a grant without any configuration, whatever app or host made it', async () => { + const { oauth } = setup([() => json({ access_token: 'kept' })]); + const user = userId(); + await connect(oauth, user, gitlab); + await expect(oauth.storedStatus(user)).resolves.toEqual({ + connected: true, + flowId: expect.stringContaining(`pull-request-oauth:${user}:`), + }); + await oauth.disconnect(user); + await expect(oauth.storedStatus(user)).resolves.toEqual({ connected: false }); + }); + + it('does not report a refused grant, or a user who never connected, without a configuration', async () => { + const { oauth } = setup([() => json({ access_token: 'revoked' })]); + const user = userId(); + await expect(oauth.storedStatus(user)).resolves.toEqual({ connected: false }); + await connect(oauth, user); + await oauth.reject(user, github, 'revoked'); + await expect(oauth.storedStatus(user)).resolves.toEqual({ connected: false }); + }); + + it('refreshes an access token a lookup was refused with, and ends the connection only if the refresh is refused', async () => { + const { oauth, fetchFn } = setup([ + () => json({ access_token: 'turned-away', refresh_token: 'r1', expires_in: 7200 }), + () => json({ access_token: 'fresh', refresh_token: 'r2', expires_in: 7200 }), + () => json({ error: 'invalid_grant' }, { status: 400 }), + ]); + const user = userId(); + await connect(oauth, user); + await oauth.reject(user, github, 'turned-away'); + /** The refresh token still works: the connection survives with a new access token. */ + await expect(oauth.token(user, github)).resolves.toBe('fresh'); + expect(formOf(fetchFn.mock.calls[1][1]).get('refresh_token')).toBe('r1'); + await oauth.reject(user, github, 'fresh'); + /** This time the provider refuses the refresh token: now the user is disconnected. */ + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.connected(user, github)).resolves.toBe(false); + }); + + it('still ends a refused grant when another write of the same grant lands first', async () => { + const { oauth } = setup([() => json({ access_token: 'revoked' })]); + const user = userId(); + await connect(oauth, user); + const original = store.replaceTokenIfCurrent.bind(store); + let grantWrites = 0; + const race = jest + .spyOn(store, 'replaceTokenIfCurrent') + .mockImplementation(async (scope, expected, row) => { + if (row.type === PULL_REQUEST_OAUTH_TYPE) { + grantWrites += 1; + /** The first write loses to a replica that rewrote the same grant in between. */ + if (grantWrites === 1) return false; + } + return original(scope, expected, row); + }); + try { + await oauth.reject(user, github, 'revoked'); + } finally { + race.mockRestore(); + } + expect(grantWrites).toBe(2); + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.connected(user, github)).resolves.toBe(false); + }); + + it('reports a reconnect that finishes while the status is being read as connected', async () => { + const { oauth } = setup([ + () => json({ access_token: 'old' }), + () => json({ access_token: 'new' }), + ]); + const user = userId(); + await connect(oauth, user); + const original = store.findToken.bind(store); + let reconnected = false; + const race = jest.spyOn(store, 'findToken').mockImplementation(async (query) => { + const row = await original(query); + /** The status has just read the old grant: the reconnect finishes before it checks. */ + if (!reconnected && query.scope?.startsWith(`${PULL_REQUEST_OAUTH_TYPE}:`)) { + reconnected = true; + await connect(oauth, user); + } + return row; + }); + try { + await expect(oauth.status(user, github)).resolves.toMatchObject({ connected: true }); + } finally { + race.mockRestore(); + } + expect(reconnected).toBe(true); + await expect(oauth.token(user, github)).resolves.toBe('new'); + }); + + it('reports a stored grant that cannot be read as a failure, without a configuration too', async () => { + const { oauth } = setup([() => json({ access_token: 'kept' })]); + const user = userId(); + await connect(oauth, user); + const garbled = createPullRequestOAuth({ + store, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt: async () => 'not json', + }); + await expect(garbled.storedStatus(user)).rejects.toThrow('malformed'); + }); +}); + +describe('the connection generation', () => { + async function started(oauth: ReturnType['oauth'], user: string, config = github) { + const { url, flowId } = await oauth.authorizationUrl({ id: user }, config); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + if (claims == null) throw new Error('state did not verify'); + return { claims, flowId }; + } + + async function existing(): Promise { + const user = userId(); + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + }); + return user; + } + + it('keeps the new connection when only the final check of a sign-in fails to read', async () => { + const { oauth } = setup([ + () => json({ access_token: 'previous' }), + () => json({ access_token: 'current' }), + ]); + const user = await existing(); + await connect(oauth, user); + const { claims } = await started(oauth, user); + const original = store.isAgentTriggerPrincipalActive.bind(store); + let calls = 0; + const check = jest + .spyOn(store, 'isAgentTriggerPrincipalActive') + .mockImplementation(async (id) => { + calls += 1; + /** The check before the exchange reads; the one after storing hits an outage. */ + if (calls === 2) throw new Error('mongo down'); + return original(id); + }); + try { + await expect(oauth.complete(claims, 'code', github)).rejects.toThrow('mongo down'); + } finally { + check.mockRestore(); + } + await expect(oauth.token(user, github)).resolves.toBe('current'); + }); + + it('lets one of two sign-ins completing together connect, and refuses the other as stale', async () => { + /** Both exchanges are under way before either stores, so both passed the check before it. */ + let answer: () => void = () => undefined; + const answered = new Promise((resolve) => (answer = resolve)); + let exchanges = 0; + /** Each sign-in's token follows its own code, whichever reaches the provider first. */ + const oauth = createPullRequestOAuth({ + store, + fetchFn: jest.fn(async (_input: string, init?: RequestInit) => { + exchanges += 1; + await answered; + return json({ access_token: formOf(init).get('code') === 'code-1' ? 'one' : 'two' }); + }), + secret, + redirectUri, + encrypt, + decrypt, + }); + const user = await existing(); + const first = await started(oauth, user); + const second = await started(oauth, user); + const outcomes = Promise.allSettled([ + oauth.complete(first.claims, 'code-1', github), + oauth.complete(second.claims, 'code-2', github), + ]); + await waitFor(() => exchanges === 2); + answer(); + const [one, two] = await outcomes; + expect([one.status, two.status].sort()).toEqual(['fulfilled', 'rejected']); + const winner = one.status === 'fulfilled' ? first : second; + await expect(oauth.status(user, github)).resolves.toEqual({ + connected: true, + flowId: winner.flowId, + }); + await expect(oauth.token(user, github)).resolves.toBe(winner === first ? 'one' : 'two'); + }); + + it("does not answer with the previous account's token once a new sign-in has landed", async () => { + const { oauth: connector } = setup([ + () => json({ access_token: 'previous' }), + () => json({ access_token: 'current' }), + ]); + const user = await existing(); + await connect(connector, user); + const reader = heldReplica(); + const pending = reader.oauth.token(user, github); + await reader.waiting; + const { claims } = await started(connector, user); + await connector.complete(claims, 'code', github); + reader.release(); + /** Read again under the new connection, on an instance that could not drop its pending read. */ + await expect(pending).resolves.toBe('current'); + await expect(reader.oauth.token(user, github)).resolves.toBe('current'); + }); + + it.each([ + ['the first connection', false], + ['a reconnection', true], + ])('keeps the previous state when storing %s fails', async (_label, connectedBefore) => { + const { oauth } = setup([ + () => json({ access_token: 'previous' }), + () => json({ access_token: 'current' }), + ]); + const user = await existing(); + if (connectedBefore) await connect(oauth, user); + const { claims } = await started(oauth, user); + const original = store.replaceTokenIfCurrent.bind(store); + const write = jest + .spyOn(store, 'replaceTokenIfCurrent') + .mockImplementation(async (scope, expected, row) => + row.type === PULL_REQUEST_OAUTH_TYPE + ? Promise.reject(new Error('write failed')) + : original(scope, expected, row), + ); + try { + await expect(oauth.complete(claims, 'code', github)).rejects.toThrow('write failed'); + } finally { + write.mockRestore(); + } + /** The earlier connection answers again, or there is still none; a new sign-in works. */ + await expect(oauth.token(user, github)).resolves.toBe(connectedBefore ? 'previous' : null); + }); + + it.each([ + [ + 'its grant write applied but did not answer, so the sign-in is finished', + PULL_REQUEST_OAUTH_TYPE, + 'current', + true, + ], + [ + 'its generation write applied but did not answer, so the previous connection is back', + PULL_REQUEST_OAUTH_GENERATION_TYPE, + 'previous', + false, + ], + ])('when %s', async (_label, failingType, answer, completes) => { + const { oauth } = setup([ + () => json({ access_token: 'previous' }), + () => json({ access_token: 'current' }), + ]); + const user = await existing(); + await connect(oauth, user); + const { claims } = await started(oauth, user); + const original = store.replaceTokenIfCurrent.bind(store); + let failed = false; + const write = jest + .spyOn(store, 'replaceTokenIfCurrent') + .mockImplementation(async (scope, expected, row) => { + const applied = await original(scope, expected, row); + if (!failed && row.type === failingType) { + failed = true; + throw new Error('no answer'); + } + return applied; + }); + try { + const completion = oauth.complete(claims, 'code', github); + if (completes) await expect(completion).resolves.toBeUndefined(); + else await expect(completion).rejects.toThrow('no answer'); + } finally { + write.mockRestore(); + } + await expect(oauth.token(user, github)).resolves.toBe(answer); + }); + + it('drops reads of this process that started before a sign-in it completed', async () => { + const user = await existing(); + const { oauth: connector } = setup([() => json({ access_token: 'previous' })]); + await connect(connector, user); + const reader = heldReplica([() => json({ access_token: 'current' })]); + const pending = reader.oauth.token(user, github); + await reader.waiting; + const { claims } = await started(reader.oauth, user); + await reader.oauth.complete(claims, 'code', github); + /** A request after the sign-in does not join the read that found the previous account. */ + await expect(reader.oauth.token(user, github)).resolves.toBe('current'); + reader.release(); + await expect(pending).resolves.toBe('current'); + }); + + it('keeps a grant a stale sign-in stored unusable when removing it fails', async () => { + const user = await existing(); + const { oauth: connector } = setup([() => json({ access_token: 'kept' })]); + await connect(connector, user); + /** The sign-in's own cleanup fails after a disconnect made its flow stale mid-exchange. */ + const failingStore: PullRequestOAuthStore = { + ...store, + deleteTokens: async (query) => { + if ('scope' in query && query.token != null) throw new Error('mongo down'); + return store.deleteTokens(query); + }, + }; + const oauth = createPullRequestOAuth({ + store: failingStore, + fetchFn: jest.fn(async () => { + await connector.disconnect(user); + return json({ access_token: 'stale' }); + }), + secret, + redirectUri, + encrypt, + decrypt, + }); + const { claims } = await started(oauth, user); + await expect(oauth.complete(claims, 'code', github)).rejects.toThrow(); + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.status(user, github)).resolves.toEqual({ connected: false }); + await expect(oauth.storedStatus(user)).resolves.toEqual({ connected: false }); + }); + + it('leaves a grant of an earlier generation in place, but no connection', async () => { + const { oauth } = setup([() => json({ access_token: 'old' })]); + const user = userId(); + await connect(oauth, user); + await tokens().updateOne( + { type: PULL_REQUEST_OAUTH_GENERATION_TYPE, userId: new mongoose.Types.ObjectId(user) }, + { $set: { token: 'another-generation' } }, + ); + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.connected(user, github)).resolves.toBe(false); + expect( + await tokens().countDocuments({ + userId: new mongoose.Types.ObjectId(user), + type: PULL_REQUEST_OAUTH_TYPE, + }), + ).toBe(1); + }); + + it('leaves every grant unusable once the generation row is removed, as account deletion does', async () => { + const { oauth } = setup([() => json({ access_token: 'orphan' })]); + const user = userId(); + await connect(oauth, user); + await tokens().deleteMany({ + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + userId: new mongoose.Types.ObjectId(user), + }); + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.storedStatus(user)).resolves.toEqual({ connected: false }); + }); + + it('keeps the generation row as long as the longest retention a grant may have', async () => { + const { oauth } = setup([() => json({ access_token: 'long' })]); + const user = userId(); + await connect(oauth, user, { ...github, retentionDays: 3650 }); + const row = await tokens().findOne({ + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + userId: new mongoose.Types.ObjectId(user), + }); + expect(new Date(row?.expiresAt).getTime() - Date.now()).toBeGreaterThan(3649 * 86_400_000); + }); +}); + +describe('reject', () => { + it('disconnects a grant the provider refused on a lookup', async () => { + const { oauth } = setup([() => json({ access_token: 'revoked' })]); + const user = userId(); + await connect(oauth, user); + await oauth.reject(user, github, 'revoked'); + await expect(oauth.connected(user, github)).resolves.toBe(false); + await expect(oauth.token(user, github)).resolves.toBeNull(); + }); + + it('keeps a grant replaced since the refused token was read', async () => { + const { oauth } = setup([ + () => json({ access_token: 'old' }), + () => json({ access_token: 'new' }), + ]); + const user = userId(); + await connect(oauth, user); + await connect(oauth, user); + await oauth.reject(user, github, 'old'); + await expect(oauth.token(user, github)).resolves.toBe('new'); + }); +}); + +describe('completion during account deletion', () => { + it('spends no code on a flow started for another provider or host', async () => { + const user = userId(); + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + }); + const { oauth, fetchFn } = setup([() => json({ access_token: 'wrong' })]); + const { url } = await oauth.authorizationUrl({ id: user }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + await expect(oauth.complete(claims!, 'the-code', gitlab)).rejects.toThrow('another provider'); + expect(fetchFn).not.toHaveBeenCalled(); + }); + + async function complete(oauth: ReturnType['oauth'], user: string) { + const { url } = await oauth.authorizationUrl({ id: user }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + if (claims == null) throw new Error('state did not verify'); + return oauth.complete(claims, 'the-code', github); + } + + it.each([ + ['whose deletion has started', { agentTriggerDeletionStartedAt: new Date() }], + ['that no longer exists', null], + ])('spends no code and stores nothing for a user %s', async (_label, fields) => { + const user = userId(); + if (fields != null) { + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + ...fields, + }); + } + const { oauth, fetchFn } = setup([() => json({ access_token: 'late' })]); + await expect(complete(oauth, user)).rejects.toThrow('not active'); + expect(fetchFn).not.toHaveBeenCalled(); + expect( + await tokens().countDocuments({ + userId: new mongoose.Types.ObjectId(user), + type: PULL_REQUEST_OAUTH_TYPE, + }), + ).toBe(0); + }); + + it('removes the grant it stored when the deletion started during the exchange', async () => { + const user = userId(); + const _id = new mongoose.Types.ObjectId(user); + await users().insertOne({ _id, email: `${user}@example.com` }); + const { oauth } = setup([ + async () => { + await users().updateOne({ _id }, { $set: { agentTriggerDeletionStartedAt: new Date() } }); + return json({ access_token: 'late' }); + }, + ]); + await expect(complete(oauth, user)).rejects.toThrow('not active'); + expect(await tokens().countDocuments({ userId: _id, type: PULL_REQUEST_OAUTH_TYPE })).toBe(0); + }); + + it('keeps its grant unusable when its removal fails and the deletion is then cancelled', async () => { + const user = userId(); + const _id = new mongoose.Types.ObjectId(user); + await users().insertOne({ _id, email: `${user}@example.com` }); + /** Removing the grant fails; rolling back the generation does not. */ + const failing: PullRequestOAuthStore = { + ...store, + deleteTokens: async (query) => { + if ('scope' in query && query.scope.startsWith(`${PULL_REQUEST_OAUTH_TYPE}:`)) { + throw new Error('mongo down'); + } + return store.deleteTokens(query); + }, + }; + const oauth = createPullRequestOAuth({ + store: failing, + fetchFn: jest.fn(async () => { + /** The deletion starts during the exchange: its fence, then every token of the user. */ + await users().updateOne({ _id }, { $set: { agentTriggerDeletionStartedAt: new Date() } }); + await tokens().deleteMany({ userId: _id }); + return json({ access_token: 'late' }); + }), + secret, + redirectUri, + encrypt, + decrypt, + }); + await expect(complete(oauth, user)).rejects.toThrow('not active'); + /** The deletion fails and is cancelled: the fence goes, the leftover grant stays. */ + await users().updateOne({ _id }, { $unset: { agentTriggerDeletionStartedAt: '' } }); + expect(await tokens().countDocuments({ userId: _id, type: PULL_REQUEST_OAUTH_TYPE })).toBe(1); + await expect(oauth.token(user, github)).resolves.toBeNull(); + }); +}); + +describe('disconnect', () => { + /** A replica whose disconnect pauses at `step` ('delete': before removing the grants; + * 'last': before replacing the generation after it) until resumed. */ + function pausedDisconnect(step: 'delete' | 'last') { + let resume: () => void = () => undefined; + const resumed = new Promise((resolve) => (resume = resolve)); + let paused: () => void = () => undefined; + const reached = new Promise((resolve) => (paused = resolve)); + let deleted = false; + const pausing: PullRequestOAuthStore = { + ...store, + deleteTokens: async (query) => { + if (step === 'delete' && 'type' in query) { + paused(); + await resumed; + } + deleted = true; + return store.deleteTokens(query); + }, + replaceTokenIfCurrent: async (scope, expected, data) => { + if (step === 'last' && deleted && data.type === PULL_REQUEST_OAUTH_GENERATION_TYPE) { + paused(); + await resumed; + } + return store.replaceTokenIfCurrent(scope, expected, data); + }, + }; + const oauth = createPullRequestOAuth({ + store: pausing, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt, + }); + return { oauth, resume, reached }; + } + + it('does not answer with a grant read between the start of a disconnect and its end', async () => { + const { oauth: connector } = setup([() => json({ access_token: 'old' })]); + const user = userId(); + await connect(connector, user); + const disconnecting = pausedDisconnect('delete'); + const disconnected = disconnecting.oauth.disconnect(user); + await disconnecting.reached; + /** Read after the first generation change, while the grant is still there. */ + const reader = heldReplica(); + const pending = reader.oauth.token(user, github); + await reader.waiting; + disconnecting.resume(); + await disconnected; + reader.release(); + await expect(pending).resolves.toBeNull(); + }); + + it('removes a grant stored between the removal of the grants and the end of a disconnect', async () => { + const user = userId(); + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + }); + const { oauth } = setup([() => json({ access_token: 'late' })]); + const { url } = await oauth.authorizationUrl({ id: user }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + const disconnecting = pausedDisconnect('last'); + const disconnected = disconnecting.oauth.disconnect(user); + await disconnecting.reached; + await expect(oauth.complete(claims!, 'the-code', github)).rejects.toThrow( + 'before a disconnect', + ); + disconnecting.resume(); + await disconnected; + expect(await tokens().countDocuments({ type: PULL_REQUEST_OAUTH_TYPE })).toBe(0); + }); + + it.each([ + ['this process', true], + ['another replica', false], + ])( + 'does not answer with a grant a read found before a disconnect in %s finished', + async (_label, sameProcess) => { + const { oauth: connector } = setup([() => json({ access_token: 'old' })]); + const user = userId(); + await connect(connector, user); + const reader = heldReplica(); + const pending = reader.oauth.token(user, github); + await reader.waiting; + await (sameProcess ? reader.oauth : connector).disconnect(user); + /** A request after the disconnect: in this process it does not join the earlier read; on a + * replica that did not run the disconnect it may, and that read then answers null too. */ + const later = reader.oauth.token(user, github); + reader.release(); + await expect(later).resolves.toBeNull(); + await expect(pending).resolves.toBeNull(); + }, + ); + + it('voids a flow started before it, even one whose exchange is under way', async () => { + const { oauth } = setup([ + () => json({ access_token: 'first' }), + () => json({ access_token: 'late' }), + ]); + const user = userId(); + await connect(oauth, user); + /** A flow started while connected, then the user disconnects before it comes back. */ + const { url } = await oauth.authorizationUrl({ id: user }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + await oauth.disconnect(user); + await expect(oauth.complete(claims!, 'the-code', github)).rejects.toThrow( + 'before a disconnect', + ); + await expect(oauth.connected(user, github)).resolves.toBe(false); + }); + + it('removes a grant a flow stored while the disconnect ran', async () => { + const user = userId(); + const { oauth } = setup([ + async () => { + await oauth.disconnect(user); + return json({ access_token: 'late' }); + }, + ]); + await users().insertOne({ + _id: new mongoose.Types.ObjectId(user), + email: `${user}@example.com`, + }); + const { url } = await oauth.authorizationUrl({ id: user }, github); + const claims = oauth.verifyState(new URL(url).searchParams.get('state') ?? ''); + await expect(oauth.complete(claims!, 'the-code', github)).rejects.toThrow( + 'before a disconnect', + ); + expect(await tokens().countDocuments({ type: PULL_REQUEST_OAUTH_TYPE })).toBe(0); + }); + + it('does not report a connection a disconnect on another replica removed during the read', async () => { + const { oauth: connector } = setup([() => json({ access_token: 'old' })]); + const user = userId(); + await connect(connector, user); + const reader = heldReplica(); + const pending = reader.oauth.connected(user, github); + await reader.waiting; + await connector.disconnect(user); + reader.release(); + await expect(pending).resolves.toBe(false); + }); + + it('keeps a read started after a disconnect joinable when an earlier read finishes', async () => { + const { oauth: connector } = setup([ + () => json({ access_token: 'first' }), + () => json({ access_token: 'second' }), + ]); + const user = userId(); + await connect(connector, user); + /** Each read of the grant waits for its own release. */ + const releases: Array<() => void> = []; + let grantReads = 0; + const gated: PullRequestOAuthStore = { + ...store, + findToken: async (query) => { + if (!query.scope.startsWith(`${PULL_REQUEST_OAUTH_TYPE}:`)) return store.findToken(query); + grantReads += 1; + await new Promise((resolve) => releases.push(resolve)); + return store.findToken(query); + }, + }; + const oauth = createPullRequestOAuth({ + store: gated, + fetchFn: jest.fn(), + secret, + redirectUri, + encrypt, + decrypt, + }); + const earlier = oauth.token(user, github); + await waitFor(() => releases.length === 1); + await oauth.disconnect(user); + await connect(connector, user); + const later = oauth.token(user, github); + await waitFor(() => releases.length === 2); + releases[0](); + /** The earlier read saw the connection change under it, so it reads again under the new one. */ + await waitFor(() => releases.length === 3); + releases[2](); + await expect(earlier).resolves.toBe('second'); + /** A third caller joins the later read rather than starting its own. */ + const third = oauth.token(user, github); + await new Promise((resolve) => setImmediate(resolve)); + expect(grantReads).toBe(3); + releases[1](); + await expect(Promise.all([later, third])).resolves.toEqual(['second', 'second']); + }); + + it('lets a flow started after it connect again', async () => { + const { oauth } = setup([ + () => json({ access_token: 'first' }), + () => json({ access_token: 'again' }), + ]); + const user = userId(); + await connect(oauth, user); + await oauth.disconnect(user); + await connect(oauth, user); + await expect(oauth.token(user, github)).resolves.toBe('again'); + }); + + it('removes the stored grant', async () => { + const { oauth } = setup([() => json({ access_token: 'gone' })]); + const user = userId(); + await connect(oauth, user); + await oauth.disconnect(user); + await expect(oauth.token(user, github)).resolves.toBeNull(); + expect(await tokens().countDocuments({ type: PULL_REQUEST_OAUTH_TYPE })).toBe(0); + }); + + it("removes grants made under an earlier app or host, and only that user's", async () => { + const { oauth } = setup([ + () => json({ access_token: 'github-grant' }), + () => json({ access_token: 'gitlab-grant' }), + () => json({ access_token: 'someone-else' }), + ]); + const user = userId(); + const other = userId(); + await connect(oauth, user, github); + await connect(oauth, user, gitlab); + await connect(oauth, other, github); + await oauth.disconnect(user); + await expect(oauth.token(user, github)).resolves.toBeNull(); + await expect(oauth.token(user, gitlab)).resolves.toBeNull(); + await expect(oauth.token(other, github)).resolves.toBe('someone-else'); + }); +}); + +describe('flow timeout', () => { + it('signs the state for the configured number of minutes', async () => { + const { oauth } = setup([]); + const { url } = await oauth.authorizationUrl( + { id: userId() }, + { ...github, flowTimeoutMinutes: 3 }, + ); + const claims = jwt.decode(new URL(url).searchParams.get('state') ?? '') as { + iat: number; + exp: number; + }; + expect(claims.exp - claims.iat).toBe(180); + }); +}); diff --git a/packages/api/src/pulls/oauth.ts b/packages/api/src/pulls/oauth.ts new file mode 100644 index 00000000000..25cef656e2f --- /dev/null +++ b/packages/api/src/pulls/oauth.ts @@ -0,0 +1,997 @@ +import jwt from 'jsonwebtoken'; +import { decryptV2, encryptV2, logger } from '@librechat/data-schemas'; +import { createHash, createHmac, randomBytes, randomUUID } from 'node:crypto'; +import type { PullRequestProvider } from 'librechat-data-provider'; +import type { PullRequestOAuthConfig } from './settings'; +import type { PullRequestFetch } from './shared'; +import { getOAuthCallbackUrl } from '~/oauth/url'; +import { getSafeErrorMetadata } from '~/utils'; +import { isRecord } from './shared'; + +export const PULL_REQUEST_OAUTH_TYPE = 'pr_oauth'; +/** One row per user: the connection generation a disconnect replaces, which every flow binds to. */ +export const PULL_REQUEST_OAUTH_GENERATION_TYPE = 'pr_oauth_generation'; +/** The generation of a user with no generation row: never connected, or deleted. No grant is ever + * stored under it, so reverting to it leaves every stored grant unusable. */ +const FIRST_GENERATION = 'initial'; +export const PULL_REQUEST_OAUTH_CALLBACK_PATH = '/api/pull-requests/oauth/callback'; +const STATE_AUDIENCE = 'librechat:pull-request-oauth'; +/** Defaults for `pullRequests.oauth.flowTimeoutMinutes`, `requestTimeoutSeconds` and + * `retentionDays`. */ +export const DEFAULT_PULL_REQUEST_OAUTH_FLOW_MINUTES = 10; +export const DEFAULT_PULL_REQUEST_OAUTH_REQUEST_SECONDS = 10; +export const DEFAULT_PULL_REQUEST_OAUTH_RETENTION_DAYS = 180; +/** An access token this close to expiring is refreshed first, so a lookup never starts with it. */ +const REFRESH_SKEW_MS = 60_000; +const DAY_SECONDS = 24 * 60 * 60; +/** A generation row outlives every grant made under it: the longest `retentionDays` allowed, and + * renewed whenever a grant under it is (see `renewGeneration`), so a grant in use for longer than + * that never outlives it. If it expired first, the user would be silently disconnected. */ +const GENERATION_LIFETIME_SECONDS = 3650 * DAY_SECONDS; +const MAX_TOKEN_LENGTH = 8_192; +/** + * The token endpoint errors that mean the grant itself is dead: RFC 6749 `invalid_grant`, and the + * codes GitHub answers with for a refresh token or a code it no longer accepts. Any other error, + * such as `invalid_client` after a wrong client secret is deployed, is about the server, and + * deleting every user's grant for it would leave them nothing to recover once it is fixed. + */ +const GRANT_REFUSALS: ReadonlySet = new Set([ + 'invalid_grant', + 'bad_refresh_token', + 'bad_verification_code', +]); + +/** Where users authorize and where the code is exchanged, on the configured host. */ +export function oauthEndpoints( + provider: PullRequestProvider, + host: string, +): { authorize: string; token: string; defaultScope?: string; basicAuth: boolean } { + if (provider === 'gitlab') { + return { + authorize: `https://${host}/oauth/authorize`, + token: `https://${host}/oauth/token`, + defaultScope: 'read_api', + basicAuth: false, + }; + } + if (provider === 'bitbucket') { + return { + authorize: 'https://bitbucket.org/site/oauth2/authorize', + token: 'https://bitbucket.org/site/oauth2/access_token', + basicAuth: true, + }; + } + return { + authorize: `https://${host}/login/oauth/authorize`, + token: `https://${host}/login/oauth/access_token`, + basicAuth: false, + }; +} + +/** The fields of a stored token row this module reads and writes; plain values, no documents. */ +export type PullRequestOAuthRow = { token: string; expiresAt?: Date | string | null }; + +export type PullRequestOAuthRowInput = { + userId: string; + type: string; + identifier: string; + token: string; + /** Seconds until the row may be deleted. */ + expiresIn: number; +}; + +/** The token persistence this module needs, supplied by the caller (the app's token methods). */ +export type PullRequestOAuthStore = { + findToken(query: { scope: string }): Promise; + /** Writes `data` only while the row still holds `expected` (or is absent, for `null`). */ + replaceTokenIfCurrent( + scope: string, + expected: string | null, + data: PullRequestOAuthRowInput, + ): Promise; + deleteTokens( + query: { scope: string; token?: string } | { userId: string; type: string }, + ): Promise; + /** Whether the user exists and no account deletion has started for it (its deletion fence). */ + isAgentTriggerPrincipalActive(userId: string): Promise; + /** Every row of one type the user holds, whatever app or host made it. */ + findTokensOfUser(query: { userId: string; type: string }): Promise; +}; + +export type PullRequestOAuthStatus = { connected: boolean; flowId?: string }; + +/** What a verified `state` carries back to the callback. */ +export type PullRequestOAuthState = { + userId: string; + provider: PullRequestProvider; + host: string; + tenantId?: string; + /** Binds the CSRF cookie to this one flow. */ + flowId: string; + nonce: string; + /** The user's connection generation when the flow started; a disconnect since voids it. */ + generation: string; +}; + +/** What the provider issued. */ +type Issued = { + accessToken: string; + refreshToken?: string; + /** Epoch milliseconds; absent for a token that does not expire. */ + accessExpiresAt?: number; + /** The app that issued it; a grant of a replaced app is treated as no connection. */ + clientId: string; +}; + +/** + * Every stored record carries `lineage`, fixed when the user connects and kept by every refresh, + * so a write can tell a later state of the same connection from a new one. Each write also + * stores a fresh `v`, which makes the ciphertext a version: every write and delete compares and + * swaps on the exact row it read, never on equal content. + */ +type Versioned = { + lineage: string; + v: string; + /** The user's connection generation the record was made under. Only a record of the current + * generation is a connection, so a sign-in, a disconnect or an account deletion, each of + * which replaces or removes the generation, leaves every older record unusable even where + * removing it failed. Absent from a record that predates it, which is therefore never one. */ + generation?: string; +}; + +/** A grant as stored, encrypted, in one row per user and provider host. `leaseUntil` marks the + * one replica allowed to refresh it, so no two spend the same refresh token. */ +type Grant = Issued & + Versioned & { + leaseUntil?: number; + /** Epoch milliseconds of the last write, which every use makes; the retention counts from it + * under the current `retentionDays`, so a shortened retention applies to existing grants. */ + usedAt?: number; + /** The sign-in that made the connection, kept through refreshes, so a client can tell its + * own sign-in landed from an older connection still being there. */ + flowId?: string; + }; + +/** What a definite refusal leaves behind: no connection, but the lineage, so a slower replica + * whose refresh of the same grant succeeded can still store its result over it. */ +type Refused = Versioned & { refused: true; clientId: string }; + +type Stored = Grant | Refused; + +const isRefused = (record: Stored): record is Refused => 'refused' in record; + +/** A stored grant that decrypts but is not one this service wrote: corruption, not absence. */ +class MalformedGrantError extends Error { + constructor() { + super('The stored pull request grant is malformed'); + this.name = 'MalformedGrantError'; + } +} + +/** Thrown for a grant the provider refused, as opposed to one it could not be asked about. A + * refusal disconnects the user, so only a definite one is: a server error, or an error about the + * client rather than the grant, is an outage, whatever its status. */ +class GrantRejectedError extends Error { + constructor() { + super('The provider rejected the grant'); + this.name = 'GrantRejectedError'; + } +} + +/** Another replica holds the refresh of this grant, so its token is not known yet. */ +class GrantRefreshPendingError extends Error { + constructor() { + super('Another refresh of the grant is in progress'); + this.name = 'GrantRefreshPendingError'; + } +} + +/** The user disconnected after this flow started, so it may not connect them again. */ +class StaleFlowError extends Error { + constructor() { + super('The flow started before a disconnect, or for another provider'); + this.name = 'StaleFlowError'; + } +} + +/** The user was deleted, or its deletion started, while the connection was being made. */ +class InactivePrincipalError extends Error { + constructor() { + super('The user is not active'); + this.name = 'InactivePrincipalError'; + } +} + +const base64url = (value: Buffer): string => value.toString('base64url'); + +const isToken = (value: unknown): value is string => + typeof value === 'string' && value.length > 0 && value.length <= MAX_TOKEN_LENGTH; + +/** + * A stored record, from its decrypted text. Only this service writes the row, so text that is + * not one of its records is a failure (corruption), never an absent connection. + */ +function parseStored(plaintext: string): Stored { + let parsed: unknown; + try { + parsed = JSON.parse(plaintext); + } catch { + throw new MalformedGrantError(); + } + if ( + !isRecord(parsed) || + typeof parsed.clientId !== 'string' || + typeof parsed.lineage !== 'string' || + typeof parsed.v !== 'string' || + (parsed.generation !== undefined && typeof parsed.generation !== 'string') + ) { + throw new MalformedGrantError(); + } + if (parsed.refused === true) return parsed as Refused; + if ( + !isToken(parsed.accessToken) || + (parsed.refreshToken !== undefined && !isToken(parsed.refreshToken)) || + (parsed.accessExpiresAt !== undefined && typeof parsed.accessExpiresAt !== 'number') || + (parsed.leaseUntil !== undefined && typeof parsed.leaseUntil !== 'number') || + (parsed.usedAt !== undefined && typeof parsed.usedAt !== 'number') || + (parsed.flowId !== undefined && typeof parsed.flowId !== 'string') + ) { + throw new MalformedGrantError(); + } + return parsed as Grant; +} + +const isSeconds = (value: unknown): value is number => + typeof value === 'number' && Number.isFinite(value) && value > 0; + +export type PullRequestOAuth = { + /** Where to send the user, and the flow id its CSRF cookie must be bound to. */ + authorizationUrl( + user: { id: string; tenantId?: string }, + config: PullRequestOAuthConfig, + ): Promise<{ url: string; flowId: string }>; + /** The claims of a `state` this server issued and that has not expired; null otherwise. */ + verifyState(state: string): PullRequestOAuthState | null; + /** Exchanges the code and stores the grant for the user the state names. */ + complete( + state: PullRequestOAuthState, + code: string, + config: PullRequestOAuthConfig, + ): Promise; + /** The user's access token, refreshed when it is about to expire; null when not connected. + * Rejects when it cannot be known: an outage, a grant that cannot be read, or a refusal that + * another replica's refresh may still overtake. */ + token(userId: string, config: PullRequestOAuthConfig): Promise; + connected(userId: string, config: PullRequestOAuthConfig): Promise; + /** `connected`, with the sign-in that made the connection. */ + status(userId: string, config: PullRequestOAuthConfig): Promise; + /** Whether the user holds a grant under any app or host, read without a configuration, so a + * grant made before the app was turned off still shows, and can still be disconnected. */ + storedStatus(userId: string): Promise; + /** Marks the grant refused when the provider rejected `accessToken` on a lookup, only while it + * is still the stored one: a grant refreshed or reconnected since stands. */ + reject(userId: string, config: PullRequestOAuthConfig, accessToken: string): Promise; + /** Removes every connection the user has, whatever app or host it was made with, so a later + * configuration can never bring back one the user removed. */ + disconnect(userId: string): Promise; +}; + +/** + * Per-user connections to the configured provider, so pull request lookups run on each user's own + * token, rate limit and repository access. The `state` is a short-lived signed token, so the + * callback needs no server-side flow record, and the PKCE verifier is derived from it with the + * server secret, so it never travels through the browser. Grants are stored encrypted, one row per + * user and provider host, and replaced only when unchanged since read, so two replicas refreshing + * at once cannot overwrite each other's newer grant. + */ +export function createPullRequestOAuth({ + store, + fetchFn, + secret, + redirectUri, + encrypt = encryptV2, + decrypt = decryptV2, + now = Date.now, +}: { + store: PullRequestOAuthStore; + fetchFn: PullRequestFetch; + /** Signs the state and derives the PKCE verifier. */ + secret: string; + redirectUri: string; + encrypt?: (value: string) => Promise; + decrypt?: (value: string) => Promise; + now?: () => number; +}): PullRequestOAuth { + const scopeOf = (userId: string, config: PullRequestOAuthConfig) => + `${PULL_REQUEST_OAUTH_TYPE}:${userId}:${config.provider}:${config.host}`; + const verifierOf = (nonce: string) => + base64url(createHmac('sha256', secret).update(`pull-request-oauth:${nonce}`).digest()); + + /** Token reads in flight in this process, so concurrent lookups share one read and at most one + * refresh. Keyed by the user and every setting of the app config, since any of them (the app, + * the retention, the timeouts) can change the answer. */ + const reading = new Map>(); + const configKeyOf = (config: PullRequestOAuthConfig): string => + createHash('sha256') + .update(JSON.stringify(config, Object.keys(config).sort())) + .digest('hex'); + + const generationScopeOf = (userId: string) => `${PULL_REQUEST_OAUTH_GENERATION_TYPE}:${userId}`; + async function generationOf(userId: string): Promise { + const row = await store.findToken({ scope: generationScopeOf(userId) }); + return row?.token ?? FIRST_GENERATION; + } + + async function replaceGeneration(userId: string): Promise { + const scope = generationScopeOf(userId); + for (let attempt = 0; attempt < 3; attempt++) { + const current = await store.findToken({ scope }); + const replaced = await store.replaceTokenIfCurrent(scope, current?.token ?? null, { + userId, + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + identifier: 'generation', + token: randomUUID(), + expiresIn: GENERATION_LIFETIME_SECONDS, + }); + if (replaced) return; + } + throw new Error('The connection generation could not be replaced'); + } + + /** + * After a sign-in's writes failed without a known outcome: returns the stored grant's row token + * when both the generation and the grant landed, and otherwise puts the previous generation back + * if this sign-in's generation is the current one. + */ + async function reconcileSignIn( + state: PullRequestOAuthState, + config: PullRequestOAuthConfig, + generation: string, + lineage: string, + ): Promise { + if ((await generationOf(state.userId)) !== generation) return null; + const row = await store.findToken({ scope: scopeOf(state.userId, config) }); + if (row != null) { + const record = parseStored(await decrypt(row.token)); + if (!isRefused(record) && record.lineage === lineage) return row.token; + } + await restoreGeneration(state.userId, generation, state.generation); + return null; + } + + /** Moves the user back from `from` to `to`, only while `from` is still current. */ + async function restoreGeneration(userId: string, from: string, to: string): Promise { + const scope = generationScopeOf(userId); + if (to === FIRST_GENERATION) { + await store.deleteTokens({ scope, token: from }); + return; + } + await store.replaceTokenIfCurrent(scope, from, { + userId, + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + identifier: 'generation', + token: to, + expiresIn: GENERATION_LIFETIME_SECONDS, + }); + } + + /** + * Starts the generation's lifetime again, at most once a day, whenever a grant under it is + * extended or refreshed. The same value is written back only while it is current, so a + * generation replaced meanwhile (a disconnect, a new sign-in) is never restored. + */ + async function renewGeneration(userId: string, generation: string | undefined): Promise { + if (generation == null || generation === FIRST_GENERATION) return; + const scope = generationScopeOf(userId); + const row = await store.findToken({ scope }); + if (row?.token !== generation || row.expiresAt == null) return; + const remaining = new Date(row.expiresAt).getTime() - now(); + if (remaining > (GENERATION_LIFETIME_SECONDS - DAY_SECONDS) * 1000) return; + await store.replaceTokenIfCurrent(scope, generation, { + userId, + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + identifier: 'generation', + token: generation, + expiresIn: GENERATION_LIFETIME_SECONDS, + }); + } + + /** + * Moves the user from the generation a flow started in to a new one, or fails as stale: one + * sign-in at a time, and none across a disconnect. Returns the new generation. + */ + async function advanceGeneration(userId: string, from: string, next: string): Promise { + const advanced = await store.replaceTokenIfCurrent( + generationScopeOf(userId), + from === FIRST_GENERATION ? null : from, + { + userId, + type: PULL_REQUEST_OAUTH_GENERATION_TYPE, + identifier: 'generation', + token: next, + expiresIn: GENERATION_LIFETIME_SECONDS, + }, + ); + if (!advanced) throw new StaleFlowError(); + return next; + } + + const dropPendingReads = (userId: string): void => { + for (const key of reading.keys()) { + if (key.startsWith(`${userId}\0`)) reading.delete(key); + } + }; + + /** + * A token, or null when the user disconnected while it was being read. The generation is read + * before the grant and again after: a grant found before a disconnect removed it is answered + * only if that disconnect had not finished, since its last step replaces the generation. + */ + async function fencedToken( + userId: string, + config: PullRequestOAuthConfig, + ): Promise { + for (let attempt = 0; attempt < 2; attempt++) { + const before = await generationOf(userId); + const token = await currentToken(userId, config, before); + /** + * The connection changed under this read: a sign-in or a disconnect, on any replica or any + * instance, including one whose pending reads this instance cannot drop. It is read again + * under the new one, so neither this caller nor any that joined it answers for the old. + */ + if ((await generationOf(userId)) === before) return token; + } + return null; + } + + /** A flow may store a grant only for an active user still in the generation it started in. */ + async function assertCurrent(state: PullRequestOAuthState): Promise { + const [active, generation] = await Promise.all([ + store.isAgentTriggerPrincipalActive(state.userId), + generationOf(state.userId), + ]); + if (!active) throw new InactivePrincipalError(); + if (generation !== state.generation) throw new StaleFlowError(); + } + + async function requestGrant( + config: PullRequestOAuthConfig, + form: Record, + clientId: string, + ): Promise { + const endpoints = oauthEndpoints(config.provider, config.host); + const body = new URLSearchParams(form); + const headers: Record = { + Accept: 'application/json', + 'Content-Type': 'application/x-www-form-urlencoded', + 'User-Agent': 'LibreChat-Pull-Requests', + }; + if (endpoints.basicAuth) { + const credentials = Buffer.from(`${config.clientId}:${config.clientSecret}`); + headers.Authorization = `Basic ${credentials.toString('base64')}`; + } else { + body.set('client_id', config.clientId); + body.set('client_secret', config.clientSecret); + } + const response = await fetchFn(endpoints.token, { + method: 'POST', + headers, + body: body.toString(), + signal: AbortSignal.timeout( + (config.requestTimeoutSeconds ?? DEFAULT_PULL_REQUEST_OAUTH_REQUEST_SECONDS) * 1000, + ), + }); + const parsed: unknown = await response.json().catch(() => null); + const error = isRecord(parsed) && typeof parsed.error === 'string' ? parsed.error : undefined; + /** GitHub answers a refused grant with 200 and an `error` field; others use 400 or 401. A + * server error is an outage whatever its body says. */ + const refused = + (response.ok || response.status === 400 || response.status === 401) && + error != null && + GRANT_REFUSALS.has(error); + if (refused) throw new GrantRejectedError(); + if (!response.ok || error != null || !isRecord(parsed) || !isToken(parsed.access_token)) { + throw new Error(`Token endpoint answered ${response.status}`); + } + return { + accessToken: parsed.access_token, + ...(isToken(parsed.refresh_token) ? { refreshToken: parsed.refresh_token } : {}), + ...(isSeconds(parsed.expires_in) + ? { accessExpiresAt: now() + parsed.expires_in * 1000 } + : {}), + clientId, + }; + } + + const retentionOf = (config: PullRequestOAuthConfig): number => + (config.retentionDays ?? DEFAULT_PULL_REQUEST_OAUTH_RETENTION_DAYS) * DAY_SECONDS; + + /** One replica's claim on a refresh: its provider request, with room to store the answer. A + * claim that outlives it (a stalled replica) lets another replica claim the grant; the stalled + * one's answer is then still stored, as it carries the same lineage. */ + const leaseOf = (config: PullRequestOAuthConfig): number => + 2 * (config.requestTimeoutSeconds ?? DEFAULT_PULL_REQUEST_OAUTH_REQUEST_SECONDS) * 1000; + + /** How long the row may live: an access token that expires without a refresh token lives that + * long; a refusal lives a day, long enough for a slower refresh of the same grant to land; any + * other grant lives the retention, which every use extends (see `token`). */ + const lifetimeOf = (record: Stored, config: PullRequestOAuthConfig): number => { + if (isRefused(record)) return DAY_SECONDS; + if (record.refreshToken == null && record.accessExpiresAt != null) { + const remaining = Math.max(1, Math.ceil((record.accessExpiresAt - now()) / 1000)); + return Math.min(remaining, retentionOf(config)); + } + return retentionOf(config); + }; + + /** Whether using a grant should rewrite its row: once a write would extend its deadline by more + * than a day (half the retention, if that is shorter), so the retention runs from the last use + * within a day without a write per use, or when it is set to outlive what the current retention + * allows (past the second `lifetimeOf` rounds up to), so a shortened retention takes hold. A + * grant whose access token expires without a refresh token already lives until that expiry, so + * using it never needs a write. */ + const needsRewrite = ( + row: PullRequestOAuthRow, + grant: Grant, + config: PullRequestOAuthConfig, + ): boolean => { + if (row.expiresAt == null) return false; + const remaining = new Date(row.expiresAt).getTime() - now(); + const lifetime = lifetimeOf(grant, config) * 1000; + const interval = Math.min(DAY_SECONDS * 1000, (retentionOf(config) * 1000) / 2); + return ( + Number.isFinite(remaining) && (remaining < lifetime - interval || remaining > lifetime + 1000) + ); + }; + + /** Whether a grant can answer now, or never will without a refresh. */ + const isLive = (grant: Grant, skewMs: number): boolean => + grant.accessExpiresAt == null || grant.accessExpiresAt - skewMs > now(); + + /** The stored record for this app, or null when there is none: no row, a row past its + * retention (the TTL reaper removes it late, never on time), or a record of a replaced app. */ + async function read( + userId: string, + config: PullRequestOAuthConfig, + generation: string, + ): Promise<{ row: PullRequestOAuthRow; record: Stored } | null> { + const row = await store.findToken({ scope: scopeOf(userId, config) }); + if (row == null) return null; + if (row.expiresAt != null) { + const expiresAt = new Date(row.expiresAt).getTime(); + if (Number.isFinite(expiresAt) && expiresAt <= now()) return null; + } + /** Only this service writes the row, so a value that does not decrypt (a missing or rotated + * key) or is not one of its records is a failure, not an absent connection, and it + * propagates. A record of another OAuth app, from before the app was replaced, is no + * connection to this one. */ + const parsed = parseStored(await decrypt(row.token)); + /** A record of another app is no connection to this one, and is left alone: during a rollout + * this replica's config may be the stale one, and deleting would destroy the user's new + * connection. It expires with the retention, and disconnecting removes it. */ + if (parsed.clientId !== config.clientId) return null; + /** A record of an earlier generation is no connection, and is left alone like one of + * another app: the disconnect or sign-in that replaced its generation owns its removal. */ + if (parsed.generation !== generation) return null; + if (isRefused(parsed)) return { row, record: parsed }; + /** Unused for longer than the retention now allows: gone, and removed if still this row. + * Awaited, and a failure propagates: the caller hears there is no connection only once the + * credential is actually gone. */ + if (typeof parsed.usedAt === 'number' && parsed.usedAt + retentionOf(config) * 1000 <= now()) { + await store.deleteTokens({ scope: scopeOf(userId, config), token: row.token }); + return null; + } + return { row, record: parsed }; + } + + async function statusOf( + userId: string, + config: PullRequestOAuthConfig, + attempt = 0, + ): Promise { + /** Fenced like a token read, so it never reports a connection a finished disconnect removed. */ + const before = await generationOf(userId); + const current = await read(userId, config, before); + /** The generation moved during the read (a disconnect or a sign-in finished): the answer is + * read again under the new one, as `fencedToken` does, so a reconnect is not reported as + * disconnected. */ + if ((await generationOf(userId)) !== before) { + return attempt < 2 ? statusOf(userId, config, attempt + 1) : { connected: false }; + } + if (current == null || isRefused(current.record)) return { connected: false }; + const grant = current.record; + /** The row of a grant that cannot be refreshed outlives it until the TTL reaper runs. */ + if (grant.refreshToken == null && !isLive(grant, 0)) return { connected: false }; + return grant.flowId != null ? { connected: true, flowId: grant.flowId } : { connected: true }; + } + + /** + * Stores `record` as a new version, only while the row is still `expected` (absent, for null). + * Returns the new version, or null when the row had moved on. + */ + async function write( + userId: string, + config: PullRequestOAuthConfig, + expected: string | null, + record: Omit | Omit, + ): Promise { + const versioned = ( + 'refused' in record + ? { ...record, v: randomUUID() } + : { ...record, usedAt: now(), v: randomUUID() } + ) as Stored; + const token = await encrypt(JSON.stringify(versioned)); + const stored = await store.replaceTokenIfCurrent(scopeOf(userId, config), expected, { + userId, + type: PULL_REQUEST_OAUTH_TYPE, + identifier: `${config.provider}:${config.host}`, + token, + expiresIn: lifetimeOf(versioned, config), + }); + return stored ? token : null; + } + + /** + * Stores a refreshed grant over the version this replica claimed. When that version is gone, + * the row's state decides: removed (a disconnect or an account deletion) stays removed; a newer + * connection wins; a later state of this same connection (another replica's claim after this + * one's lease ran out, or its refusal of the refresh token this one already used) is replaced, + * since only this answer holds the refresh token the provider now accepts. + */ + async function settle( + userId: string, + config: PullRequestOAuthConfig, + expected: string, + next: Omit, + ): Promise { + let version = expected; + for (let attempt = 0; attempt < 3; attempt++) { + if ((await write(userId, config, version, next)) != null) { + await renewGeneration(userId, next.generation); + return next.accessToken; + } + const current = await read(userId, config, next.generation ?? FIRST_GENERATION); + if (current == null) return null; + if (current.record.lineage !== next.lineage) { + return isRefused(current.record) || !isLive(current.record, 0) + ? null + : current.record.accessToken; + } + version = current.row.token; + } + throw new Error('The refreshed grant could not be stored'); + } + + async function refresh( + userId: string, + config: PullRequestOAuthConfig, + row: PullRequestOAuthRow, + grant: Grant, + ): Promise { + if (grant.refreshToken == null) return null; + if (grant.leaseUntil != null && grant.leaseUntil > now()) throw new GrantRefreshPendingError(); + const { v: _v, leaseUntil: _lease, ...unleased } = grant; + /** Claim the refresh: only the replica whose claim lands spends the refresh token. */ + const claimed = await write(userId, config, row.token, { + ...unleased, + leaseUntil: now() + leaseOf(config), + }); + if (claimed == null) { + const current = await read(userId, config, grant.generation ?? FIRST_GENERATION); + if (current == null || isRefused(current.record)) return null; + if (current.record.leaseUntil == null && isLive(current.record, REFRESH_SKEW_MS)) { + return current.record.accessToken; + } + throw new GrantRefreshPendingError(); + } + let issued: Issued; + try { + issued = await requestGrant( + config, + { grant_type: 'refresh_token', refresh_token: grant.refreshToken }, + config.clientId, + ); + } catch (error) { + if (error instanceof GrantRejectedError) { + /** Refuses only the version this replica claimed; anything newer stands. */ + const refused = await write(userId, config, claimed, { + refused: true, + clientId: grant.clientId, + lineage: grant.lineage, + ...(grant.generation != null ? { generation: grant.generation } : {}), + }); + if (refused != null) return null; + const current = await read(userId, config, grant.generation ?? FIRST_GENERATION); + return current == null || isRefused(current.record) || !isLive(current.record, 0) + ? null + : current.record.accessToken; + } + /** An outage: release the claim so the next attempt need not wait it out. */ + await write(userId, config, claimed, unleased).catch((cause: unknown) => { + logger.warn('[PullRequests] OAuth refresh claim not released', getSafeErrorMetadata(cause)); + }); + throw error; + } + /** A provider that does not rotate refresh tokens keeps the one it was given. */ + return settle(userId, config, claimed, { + ...issued, + refreshToken: issued.refreshToken ?? grant.refreshToken, + lineage: grant.lineage, + ...(grant.flowId != null ? { flowId: grant.flowId } : {}), + ...(grant.generation != null ? { generation: grant.generation } : {}), + }); + } + + async function currentToken( + userId: string, + config: PullRequestOAuthConfig, + generation: string, + attempt = 0, + ): Promise { + const current = await read(userId, config, generation); + if (current == null || isRefused(current.record)) return null; + const { row, record: grant } = current; + /** Without a refresh token there is nothing to refresh with, so it serves until it expires. */ + if (isLive(grant, grant.refreshToken == null ? 0 : REFRESH_SKEW_MS)) { + if (needsRewrite(row, grant, config) && grant.leaseUntil == null) { + const { v: _v, ...same } = grant; + /** Rare (past half the retention, or a shortened one), so the read waits for it. A failed + * write is an outage, not a successful read: the grant would keep its old deadline. */ + if ((await write(userId, config, row.token, same)) == null) { + /** Another replica changed the row first (an old-config one may have extended it), so the + * current policy is read and applied again rather than assumed to have landed. */ + if (attempt >= 2) throw new Error('The grant retention could not be updated'); + return currentToken(userId, config, generation, attempt + 1); + } + await renewGeneration(userId, grant.generation); + } + return grant.accessToken; + } + return refresh(userId, config, row, grant); + } + + return { + async authorizationUrl(user, config) { + const nonce = base64url(randomBytes(16)); + const generation = await generationOf(user.id); + const state = jwt.sign( + { + sub: user.id, + p: config.provider, + h: config.host, + n: nonce, + g: generation, + ...(user.tenantId ? { t: user.tenantId } : {}), + }, + secret, + { + audience: STATE_AUDIENCE, + expiresIn: (config.flowTimeoutMinutes ?? DEFAULT_PULL_REQUEST_OAUTH_FLOW_MINUTES) * 60, + }, + ); + const endpoints = oauthEndpoints(config.provider, config.host); + const scope = config.scope ?? endpoints.defaultScope; + const challenge = base64url(createHash('sha256').update(verifierOf(nonce)).digest()); + const query = new URLSearchParams({ + client_id: config.clientId, + redirect_uri: redirectUri, + response_type: 'code', + state, + code_challenge: challenge, + code_challenge_method: 'S256', + ...(scope ? { scope } : {}), + }); + return { + url: `${endpoints.authorize}?${query.toString()}`, + flowId: flowIdOf(user.id, nonce), + }; + }, + + verifyState(state) { + let claims: unknown; + try { + claims = jwt.verify(state, secret, { audience: STATE_AUDIENCE }); + } catch { + return null; + } + if ( + !isRecord(claims) || + typeof claims.sub !== 'string' || + typeof claims.h !== 'string' || + typeof claims.n !== 'string' || + typeof claims.g !== 'string' || + (claims.p !== 'github' && claims.p !== 'gitlab' && claims.p !== 'bitbucket') || + (claims.t !== undefined && typeof claims.t !== 'string') + ) { + return null; + } + return { + userId: claims.sub, + provider: claims.p, + host: claims.h, + nonce: claims.n, + generation: claims.g, + flowId: flowIdOf(claims.sub, claims.n), + ...(typeof claims.t === 'string' ? { tenantId: claims.t } : {}), + }; + }, + + async complete(state, code, config) { + /** The code is exchanged only with the provider the flow was started for. */ + if (state.provider !== config.provider || state.host !== config.host) { + throw new StaleFlowError(); + } + /** No code is spent for a user that is gone, being deleted, or disconnected since. */ + await assertCurrent(state); + const response = await requestGrant( + config, + { + grant_type: 'authorization_code', + code, + redirect_uri: redirectUri, + code_verifier: verifierOf(state.nonce), + }, + config.clientId, + ); + /** + * The sign-in is an identity boundary: the user moves to a new generation before the grant is + * stored under it, so every earlier grant, and every read that found one, stops answering. + * After the exchange, so a failed sign-in leaves the existing connection alone. + */ + const generation = randomUUID(); + const record = { ...response, lineage: randomUUID(), flowId: state.flowId, generation }; + /** Replaces whatever the user had connected before, read first so the swap is exact. */ + let stored: string | null = null; + try { + await advanceGeneration(state.userId, state.generation, generation); + dropPendingReads(state.userId); + for (let attempt = 0; attempt < 3 && stored == null; attempt++) { + const current = await store.findToken({ scope: scopeOf(state.userId, config) }); + stored = await write(state.userId, config, current?.token ?? null, record); + } + if (stored == null) throw new Error('The grant could not be stored'); + } catch (error) { + if (error instanceof StaleFlowError) throw error; + /** + * A write can apply and still fail to answer, so what was stored decides, not the error: + * the new grant under the new generation is a finished sign-in; the new generation without + * it goes back to the previous one, whose grant is untouched; anything else moved on. + */ + stored = await reconcileSignIn(state, config, generation, record.lineage); + if (stored == null) throw error; + } + /** + * Account deletion sets its fence and then removes the user's tokens; a disconnect replaces + * the generation and then removes the grants. A grant stored before that removal goes with + * it; one stored after it finds the fence, no user, or a new generation here and removes + * itself, so neither a deletion nor a disconnect is undone by a flow started before it. + */ + try { + await assertCurrent({ ...state, generation }); + } catch (error) { + /** Only a confirmed fence undoes the sign-in; a failed check read is an outage, and the + * stored grant under the new generation stays the user's connection. */ + if (!(error instanceof InactivePrincipalError || error instanceof StaleFlowError)) { + throw error; + } + /** + * Both undone, each attempted whatever the other does: the grant, and the generation this + * sign-in set (only while it is still current). Either alone keeps the grant unusable, so + * a deletion that is later cancelled cannot make it a connection. + */ + await Promise.allSettled([ + store.deleteTokens({ scope: scopeOf(state.userId, config), token: stored }), + restoreGeneration(state.userId, generation, state.generation), + ]); + throw error; + } + }, + + async token(userId, config) { + /** Concurrent callers share one read and at most one refresh. Joining only the refresh is + * not enough: a caller whose read answered after another's refresh had already finished + * would refresh the grant that refresh replaced. */ + const key = `${userId}\0${configKeyOf(config)}`; + const pending = reading.get(key); + if (pending != null) return pending; + const started = fencedToken(userId, config); + reading.set(key, started); + try { + return await started; + } finally { + /** Only its own entry: a disconnect may have cleared it and a newer read taken the key. */ + if (reading.get(key) === started) reading.delete(key); + } + }, + + async connected(userId, config) { + return (await statusOf(userId, config)).connected; + }, + + status: statusOf, + + async storedStatus(userId) { + const before = await generationOf(userId); + const rows = await store.findTokensOfUser({ userId, type: PULL_REQUEST_OAUTH_TYPE }); + for (const row of rows) { + if (row.expiresAt != null && new Date(row.expiresAt).getTime() <= now()) continue; + /** A row that cannot be read is a failure, as for a configured read. */ + const parsed = parseStored(await decrypt(row.token)); + if (isRefused(parsed) || parsed.generation !== before) continue; + if ((await generationOf(userId)) !== before) return { connected: false }; + return parsed.flowId != null + ? { connected: true, flowId: parsed.flowId } + : { connected: true }; + } + return { connected: false }; + }, + + async reject(userId, config, accessToken) { + /** A write that loses to another (a retention rewrite of the same grant) is read again, so + * the rejection still lands while the row holds that access token; a row that moved on to + * another token or connection is left alone. */ + for (let attempt = 0; attempt < 3; attempt++) { + const current = await read(userId, config, await generationOf(userId)); + if (current == null || isRefused(current.record)) return; + if (current.record.accessToken !== accessToken) return; + /** Only the access token was turned away: with a refresh token the next read refreshes it, + * and only a refused refresh ends the connection. */ + const { v: _v, ...grant } = current.record; + const written = + current.record.refreshToken != null + ? await write(userId, config, current.row.token, { ...grant, accessExpiresAt: 0 }) + : await write(userId, config, current.row.token, { + refused: true, + clientId: current.record.clientId, + lineage: current.record.lineage, + ...(current.record.generation != null + ? { generation: current.record.generation } + : {}), + }); + if (written != null) return; + } + throw new Error('The rejected grant could not be updated'); + }, + + async disconnect(userId) { + /** + * The generation is replaced before the grants are removed, so a flow started before this + * cannot store a grant after it (see `complete`), and again after, so a token read that + * found the grant before its removal does not answer with it once this has returned (see + * `fencedToken`). Reads of this process are not joined by later requests either. + */ + await replaceGeneration(userId); + await store.deleteTokens({ userId, type: PULL_REQUEST_OAUTH_TYPE }); + await replaceGeneration(userId); + dropPendingReads(userId); + }, + }; +} + +const flowIdOf = (userId: string, nonce: string): string => `pull-request-oauth:${userId}:${nonce}`; + +/** + * The connection service for this deployment, from its environment: the state is signed with + * `JWT_SECRET` and the callback is on `DOMAIN_SERVER`. Without a secret no user can connect. + */ +export function createPullRequestOAuthFromEnv({ + store, + fetchFn, + env, +}: { + store: PullRequestOAuthStore; + fetchFn: PullRequestFetch; + env: Readonly>; +}): PullRequestOAuth | undefined { + const secret = env.JWT_SECRET; + if (!secret) return undefined; + return createPullRequestOAuth({ + store, + fetchFn, + secret, + redirectUri: getOAuthCallbackUrl( + env.DOMAIN_SERVER || 'http://localhost:3080', + PULL_REQUEST_OAUTH_CALLBACK_PATH, + ), + }); +} diff --git a/packages/api/src/pulls/settings.spec.ts b/packages/api/src/pulls/settings.spec.ts index 2104436adf8..b03cab78714 100644 --- a/packages/api/src/pulls/settings.spec.ts +++ b/packages/api/src/pulls/settings.spec.ts @@ -1,4 +1,5 @@ import { + resolvePullRequestOAuth, resolveTokenReference, resolvePullRequestHost, resolvePullRequestToken, @@ -7,6 +8,38 @@ import { resolvePullRequestProvider, } from './settings'; +describe('resolvePullRequestOAuth', () => { + const oauth = { clientId: '${ID}', clientSecret: '${SECRET}' }; + const env = { ID: 'client', SECRET: 'secret' }; + + it('resolves the app on the provider host', () => { + expect( + resolvePullRequestOAuth({ provider: 'gitlab', host: 'GitLab.Example.com', oauth }, env), + ).toEqual({ + provider: 'gitlab', + host: 'gitlab.example.com', + clientId: 'client', + clientSecret: 'secret', + }); + expect(resolvePullRequestOAuth({ oauth: { ...oauth, scope: ' repo ' } }, env)).toMatchObject({ + provider: 'github', + host: 'github.com', + scope: 'repo', + }); + }); + + it('is null without an app, or when a reference does not resolve', () => { + expect(resolvePullRequestOAuth({}, env)).toBeNull(); + expect(resolvePullRequestOAuth({ oauth }, { ID: 'client' })).toBeNull(); + expect(resolvePullRequestOAuth({ oauth: { ...oauth, clientId: 'literal' } }, env)).toBeNull(); + }); + + it('turns the feature on without a server token or a repository list', () => { + expect(isPullRequestFeatureActive({ oauth }, env)).toBe(true); + expect(isPullRequestFeatureActive({ oauth, enabled: false }, env)).toBe(false); + }); +}); + describe('resolvePullRequestProvider', () => { it('is GitHub when none is configured', () => { expect(resolvePullRequestProvider(undefined)).toBe('github'); diff --git a/packages/api/src/pulls/settings.ts b/packages/api/src/pulls/settings.ts index c1409fd63c4..ab132765ff0 100644 --- a/packages/api/src/pulls/settings.ts +++ b/packages/api/src/pulls/settings.ts @@ -78,6 +78,48 @@ export function resolvePullRequestToken( return null; } +/** A user connection the operator configured: the app users authorize, on the configured host. */ +export type PullRequestOAuthConfig = { + provider: PullRequestProvider; + host: string; + clientId: string; + clientSecret: string; + scope?: string; + flowTimeoutMinutes?: number; + requestTimeoutSeconds?: number; + retentionDays?: number; +}; + +/** + * The OAuth app users connect their accounts through, or `null` when none is configured or its + * references do not resolve, in which case users cannot connect and only the server token is used. + */ +export function resolvePullRequestOAuth( + settings: Partial | null | undefined, + env: Env, +): PullRequestOAuthConfig | null { + const oauth = settings?.oauth; + if (oauth == null) return null; + const provider = resolvePullRequestProvider(settings?.provider); + if (provider == null) return null; + const host = resolvePullRequestHost(settings?.host, provider); + const clientId = resolveTokenReference(oauth.clientId, env); + const clientSecret = resolveTokenReference(oauth.clientSecret, env); + if (host == null || clientId == null || clientSecret == null) return null; + const scope = oauth.scope?.trim(); + const { flowTimeoutMinutes, requestTimeoutSeconds, retentionDays } = oauth; + return { + provider, + host, + clientId, + clientSecret, + ...(scope ? { scope } : {}), + ...(flowTimeoutMinutes != null ? { flowTimeoutMinutes } : {}), + ...(requestTimeoutSeconds != null ? { requestTimeoutSeconds } : {}), + ...(retentionDays != null ? { retentionDays } : {}), + }; +} + /** * Whether pull requests can do anything on this deployment. The feature is on unless an * administrator turns it off, and it stays dormant, advertising nothing and recording nothing, @@ -91,6 +133,9 @@ export function isPullRequestFeatureActive( if (settings?.enabled === false) return false; const provider = resolvePullRequestProvider(settings?.provider); if (provider == null || resolvePullRequestHost(settings?.host, provider) == null) return false; + /** Users who connect their own account need no repository list: their token sees only what + * they can already see. */ + if (resolvePullRequestOAuth(settings, env) != null) return true; if (settings?.token == null && resolvePullRequestToken(undefined, env, provider) == null) { return false; } diff --git a/packages/api/src/pulls/types.ts b/packages/api/src/pulls/types.ts index 5b48ec8de87..a8dd769fe28 100644 --- a/packages/api/src/pulls/types.ts +++ b/packages/api/src/pulls/types.ts @@ -5,6 +5,9 @@ export const PULL_REQUEST_ERROR_CODES = [ 'NOT_CONFIGURED', 'RATE_LIMITED', 'UPSTREAM_ERROR', + /** The provider refused the token itself (401). Answered to clients as `UPSTREAM_ERROR`; a + * user's own token is also invalidated, so the account reads as disconnected. */ + 'CREDENTIAL_REJECTED', ] as const; export type PullRequestErrorCode = (typeof PULL_REQUEST_ERROR_CODES)[number]; diff --git a/packages/data-provider/src/config.spec.ts b/packages/data-provider/src/config.spec.ts index 614d4285d17..6e06d485ea5 100644 --- a/packages/data-provider/src/config.spec.ts +++ b/packages/data-provider/src/config.spec.ts @@ -1199,6 +1199,22 @@ describe('agent pull request config', () => { expect(parse({ ...enabled, provider }).success).toBe(true); }); + it('accepts an OAuth app given by environment references', () => { + const oauth = { clientId: '${GH_CLIENT_ID}', clientSecret: '${GH_CLIENT_SECRET}' }; + expect(parse({ ...enabled, oauth }).success).toBe(true); + expect(parse({ ...enabled, oauth: { ...oauth, scope: 'read_api read_user' } }).success).toBe( + true, + ); + }); + + it.each([ + ['a literal secret', { clientId: '${ID}', clientSecret: 'shhh' }], + ['a missing secret', { clientId: '${ID}' }], + ['a scope with odd characters', { clientId: '${ID}', clientSecret: '${S}', scope: 'a;b' }], + ])('rejects an OAuth app with %s', (_label, oauth) => { + expect(parse({ ...enabled, oauth }).success).toBe(false); + }); + it('rejects a provider it does not know', () => { expect(parse({ ...enabled, provider: 'gitea' }).success).toBe(false); }); diff --git a/packages/data-provider/src/config.ts b/packages/data-provider/src/config.ts index 4e246a384fc..a01a4c6932f 100644 --- a/packages/data-provider/src/config.ts +++ b/packages/data-provider/src/config.ts @@ -67,6 +67,7 @@ import { PULL_REQUEST_BATCH_VERSION, PULL_REQUEST_BATCH_TIMEOUT_MAX_SECONDS, } from './types/pullRequest'; +import type { PullRequestProvider } from './types/pullRequest'; import { isActionTool } from './types/tools'; import { apiBaseUrl } from './api-endpoints'; import { FileSources } from './types/files'; @@ -114,11 +115,13 @@ export const BASE_ONLY_CONFIG_SECTIONS = ['filters', 'mcpAppSandbox'] as const; export const BASE_PRINCIPAL_CONFIG_SECTIONS = ['langfuse'] as const; /** Fields inside overridable sections that only the operator's YAML sets: a role, group, user * or tenant config document can neither change nor remove them. The pull request provider and - * host decide where the deployment's token is sent, and the shared cache's slot count where - * every replica looks for a shared answer. */ + * host decide where the deployment's token is sent, the OAuth app which credentials users grant + * and which environment secret is sent with them, and the shared cache's slot count where every + * replica looks for a shared answer. */ export const BASE_ONLY_CONFIG_PATHS = [ 'endpoints.agents.pullRequests.provider', 'endpoints.agents.pullRequests.host', + 'endpoints.agents.pullRequests.oauth', 'endpoints.agents.pullRequests.sharedCacheSlots', ] as const; @@ -1918,6 +1921,36 @@ export const agentsEndpointSchema = baseEndpointSchema * `${GITHUB_PULL_REQUEST_TOKEN}`. Never the token itself. Optional: without it the first * of `GITHUB_PULL_REQUEST_TOKEN`, `GITHUB_TOKEN` and `GH_TOKEN` that is set is used. */ token: pullRequestTokenReferenceSchema.optional(), + /** Lets each user connect their own account, so lookups use that user's token, rate + * limit and repository access instead of the server's. Read from the YAML only, as + * `provider` and `host` are. Register `/api/pull-requests/oauth/callback` + * as the callback URL. On GitHub, a GitHub App with read-only permissions is the + * recommended client: its user tokens need no scope. A connection belongs to the app + * it was made with: changing `clientId`, `provider` or `host` asks users to connect + * again. A grant made with another `clientId`, provider or host is never used and + * expires with `retentionDays`. A changed `scope` applies to new connections only. + * Disconnecting removes every grant of the user. */ + oauth: z + .object({ + /** Environment variable references, never the values. */ + clientId: pullRequestTokenReferenceSchema, + clientSecret: pullRequestTokenReferenceSchema, + /** Space separated scopes to request. Defaults: none on GitHub, `read_api` on + * GitLab, and the consumer's own on Bitbucket. */ + scope: z + .string() + .trim() + .max(256) + .regex(/^[A-Za-z0-9_:.\- ]*$/) + .optional(), + /** Minutes a user may take to approve the connection at the provider. Default 10. */ + flowTimeoutMinutes: z.number().int().min(1).max(60).optional(), + /** Seconds a code exchange or token refresh may take. Default 10. */ + requestTimeoutSeconds: z.number().int().min(1).max(60).optional(), + /** Days a connection is kept without being used; every use extends it. Default 180. */ + retentionDays: z.number().int().min(1).max(3650).optional(), + }) + .optional(), /** Repositories the token may be used for, as `owner/name` or `owner/*`. A worker reports * its own repository, so without this list a user could point the server's token at any * repository it can read. */ @@ -3259,6 +3292,8 @@ export type TStartupConfig = { /** `endpoints.agents.pullRequests.maxConcurrentLookups`, so the single-route fallback of an * upgrade in progress keeps to the limit the operator configured. */ pullRequestsMaxConcurrentLookups?: number; + /** The provider users can connect their own account to, from `pullRequests.oauth`. */ + pullRequestsConnect?: PullRequestProvider; /** Manual context compaction, gated by the same `summarization.enabled` * switch that governs the automatic detour. */ compactionEnabled?: boolean; diff --git a/packages/data-provider/src/types/pullRequest.ts b/packages/data-provider/src/types/pullRequest.ts index ce8e5240b3a..4dcdcfeb979 100644 --- a/packages/data-provider/src/types/pullRequest.ts +++ b/packages/data-provider/src/types/pullRequest.ts @@ -27,6 +27,27 @@ export type TConversationPullRequest = { checks: PullRequestChecks; }; +/** A started connection: the provider page to send the user to, and how long the flow lasts. */ +export type TPullRequestConnectStart = { + url: string; + /** Seconds the signed flow stays valid. Absent from a server that predates it: 600. */ + expiresInSeconds?: number; + /** Names this sign-in, so the client knows when the grant it produced is the stored one. */ + flowId?: string; +}; + +/** Whether the signed-in user has connected their own account for pull request lookups. */ +export type TPullRequestConnection = { + /** Whether users can connect an account on this deployment at all. */ + configured: boolean; + provider?: PullRequestProvider; + /** Whether the user has a stored grant. It can be true while `configured` is false: a grant + * made before the app was turned off can, and should, still be disconnected. */ + connected: boolean; + /** The sign-in that produced the stored grant, so replacing it with another is a change. */ + flowId?: string; +}; + /** Most conversations one batch lookup may name. It bounds the fan-out to GitHub of a single * request; the lookups themselves stay governed by the configured limits and cache. */ export const PULL_REQUEST_BATCH_MAX = 50; diff --git a/packages/data-schemas/src/app/resolution.spec.ts b/packages/data-schemas/src/app/resolution.spec.ts index c9d371fe5b7..ba054547f25 100644 --- a/packages/data-schemas/src/app/resolution.spec.ts +++ b/packages/data-schemas/src/app/resolution.spec.ts @@ -173,6 +173,20 @@ describe('mergeConfigOverrides', () => { ]); expect(slots.sharedCacheSlots).toBeUndefined(); + const oauth = read([ + fakeConfig( + { + endpoints: { + agents: { + pullRequests: { oauth: { clientId: 'x', clientSecret: '${OTHER_SECRET}' } }, + }, + }, + }, + 10, + ), + ]); + expect(oauth.oauth).toBeUndefined(); + const removed = read([fakeConfig({}, 10, ['endpoints.agents.pullRequests.host'])]); expect(removed.host).toBe('ghe.example.com'); diff --git a/packages/data-schemas/src/methods/index.ts b/packages/data-schemas/src/methods/index.ts index 682adf8d82b..0f5752dd454 100644 --- a/packages/data-schemas/src/methods/index.ts +++ b/packages/data-schemas/src/methods/index.ts @@ -30,6 +30,7 @@ import { createSessionMethods, DEFAULT_REFRESH_TOKEN_EXPIRY, type SessionMethods import { createPasskeyMethods, type PasskeyMethods } from './passkey'; import { createUserMethods, DEFAULT_SESSION_EXPIRY, type UserMethods } from './user'; import { createTokenMethods, type TokenMethods } from './token'; +import { createTokenListMethods, type TokenListMethods } from './tokenList'; import { createRoleMethods, RoleConflictError } from './role'; import { createKeyMethods, type KeyMethods } from './key'; import { createFileMethods } from './file'; @@ -252,6 +253,7 @@ export type AllMethods = ToolApprovalGrantStorage & UserMethods & SessionMethods & TokenMethods & + TokenListMethods & RefreshTokenBridgeMethods & OpenIDRefreshFlightMethods & PasskeyMethods & @@ -490,6 +492,7 @@ export function createMethods( ...createToolApprovalGrantMethods(mongoose), ...createSessionMethods(mongoose), ...createTokenMethods(mongoose), + ...createTokenListMethods(mongoose), ...createRefreshTokenBridgeMethods(mongoose), ...createOpenIDRefreshFlightMethods(mongoose), ...createPasskeyMethods(mongoose), diff --git a/packages/data-schemas/src/methods/tokenList.spec.ts b/packages/data-schemas/src/methods/tokenList.spec.ts new file mode 100644 index 00000000000..89f27fccad8 --- /dev/null +++ b/packages/data-schemas/src/methods/tokenList.spec.ts @@ -0,0 +1,65 @@ +import mongoose from 'mongoose'; +import { MongoMemoryServer } from 'mongodb-memory-server'; +import { createTokenListMethods } from './tokenList'; +import { createTokenModel } from '~/models/token'; +import { createTokenMethods } from './token'; + +let mongoServer: MongoMemoryServer; +let methods: ReturnType; +let tokens: ReturnType; + +beforeAll(async () => { + mongoServer = await MongoMemoryServer.create(); + await mongoose.connect(mongoServer.getUri()); + createTokenModel(mongoose); + methods = createTokenListMethods(mongoose); + tokens = createTokenMethods(mongoose); +}); + +afterAll(async () => { + await mongoose.disconnect(); + await mongoServer.stop(); +}); + +const add = (userId: mongoose.Types.ObjectId, type: string, identifier: string) => + tokens.createToken({ userId, type, identifier, token: `${type}:${identifier}`, expiresIn: 60 }); + +describe('findTokensOfUser', () => { + it("returns every token of one type the user holds, and only that user's", async () => { + const user = new mongoose.Types.ObjectId(); + const other = new mongoose.Types.ObjectId(); + await add(user, 'pr_oauth', 'github:github.com'); + await add(user, 'pr_oauth', 'gitlab:gitlab.com'); + await add(user, 'mcp_oauth', 'server'); + await add(other, 'pr_oauth', 'github:github.com'); + const found = await methods.findTokensOfUser({ userId: user.toString(), type: 'pr_oauth' }); + expect(found.map((token) => token.identifier).sort()).toEqual([ + 'github:github.com', + 'gitlab:gitlab.com', + ]); + }); + + it('leaves out expired rows and returns the newest first', async () => { + const user = new mongoose.Types.ObjectId(); + await add(user, 'pr_oauth', 'older'); + await new Promise((resolve) => setTimeout(resolve, 5)); + await add(user, 'pr_oauth', 'newer'); + await tokens.createToken({ + userId: user, + type: 'pr_oauth', + identifier: 'expired', + token: 'pr_oauth:expired', + expiresIn: -60, + }); + const found = await methods.findTokensOfUser({ userId: user.toString(), type: 'pr_oauth' }); + expect(found.map((token) => token.identifier)).toEqual(['newer', 'older']); + }); + + it('returns nothing for a user without any', async () => { + const found = await methods.findTokensOfUser({ + userId: new mongoose.Types.ObjectId().toString(), + type: 'pr_oauth', + }); + expect(found).toEqual([]); + }); +}); diff --git a/packages/data-schemas/src/methods/tokenList.ts b/packages/data-schemas/src/methods/tokenList.ts new file mode 100644 index 00000000000..839e1611b08 --- /dev/null +++ b/packages/data-schemas/src/methods/tokenList.ts @@ -0,0 +1,23 @@ +import type { IToken } from '~/types'; + +/** A user holds a handful of tokens of one type at most; the bound keeps a read cheap regardless. */ +const MAX_TOKENS_OF_USER = 100; + +export interface TokenListMethods { + /** The unexpired tokens of one type a user holds, such as one per connected provider host, + * newest first and at most a hundred. */ + findTokensOfUser(query: { userId: string; type: string }): Promise; +} + +export function createTokenListMethods(mongoose: typeof import('mongoose')): TokenListMethods { + async function findTokensOfUser(query: { userId: string; type: string }): Promise { + const Token = mongoose.models.Token; + /** Unexpired rows, the most recently written first: a grant in use is rewritten as it is + * used, so it is never the row the bound leaves out. */ + return Token.find({ userId: query.userId, type: query.type, expiresAt: { $gt: new Date() } }) + .sort({ createdAt: -1 }) + .limit(MAX_TOKENS_OF_USER) + .lean(); + } + return { findTokensOfUser }; +}