diff --git a/apps/web/src/components/integrations/GitLabIntegrationDetails.tsx b/apps/web/src/components/integrations/GitLabIntegrationDetails.tsx index 182d03e1b9b..4dcb7c6ecbe 100644 --- a/apps/web/src/components/integrations/GitLabIntegrationDetails.tsx +++ b/apps/web/src/components/integrations/GitLabIntegrationDetails.tsx @@ -157,14 +157,18 @@ export function GitLabIntegrationDetails({ // Connect with PAT mutation const { mutate: connectWithPATMutate, isPending: isConnectingWithPAT } = useMutation( trpc.gitlab.connectWithPAT.mutationOptions({ - onSuccess: () => { - toast.success('GitLab connected successfully!'); + onSuccess: result => { + const warning = result.warnings?.join(' '); + toast.success('GitLab connected successfully!', warning ? { description: warning } : {}); // Reset PAT form setPatToken(''); setPATValidation({ status: 'idle' }); setConnectionMethod('oauth'); - // Reload the page to refresh the installation data - window.location.reload(); + // Refresh the installation in place. A reload here would replace the + // document and discard the toast before it can be read. + void queryClient.invalidateQueries({ + queryKey: trpc.gitlab.getInstallation.queryKey(input), + }); }, onError: err => { toast.error('Failed to connect', { description: err.message }); diff --git a/apps/web/src/lib/code-reviews/action-required.test.ts b/apps/web/src/lib/code-reviews/action-required.test.ts index 93c00d12b06..d7467f49d63 100644 --- a/apps/web/src/lib/code-reviews/action-required.test.ts +++ b/apps/web/src/lib/code-reviews/action-required.test.ts @@ -129,6 +129,11 @@ describe('classifyCodeReviewActionRequiredFailure', () => { it('does not classify unrelated auth, rate-limit, or BYOK quota failures', () => { expect(classifyCodeReviewActionRequiredFailure('GitHub returned 401 Unauthorized')).toBeNull(); expect(classifyCodeReviewActionRequiredFailure('GitHub returned 403 Forbidden')).toBeNull(); + expect( + classifyCodeReviewActionRequiredFailure( + 'GitLab rejected the connected credential while preparing this review of owner/repo. Reconnect the GitLab integration, then retry.' + ) + ).toBeNull(); expect(classifyCodeReviewActionRequiredFailure('Rate limit exceeded: 429')).toBeNull(); expect( classifyCodeReviewActionRequiredFailure('[BYOK] Your account quota is exhausted.') diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts index 51ae8382f8e..fde65680cce 100644 --- a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.test.ts @@ -39,6 +39,7 @@ jest.mock('@/lib/integrations/platforms/gitlab/adapter', () => ({ fetchGitLabRootTextFileAtRef: (...args: unknown[]) => mockFetchGitLabRootTextFileAtRef(...args), fetchGitLabRepositorySize: (...args: unknown[]) => mockFetchGitLabRepositorySize(...args), GitLabProjectAccessTokenPermissionError: class GitLabProjectAccessTokenPermissionError extends Error {}, + GitLabAuthenticationError: class GitLabAuthenticationError extends Error {}, })); jest.mock('@/lib/integrations/gitlab-service', () => ({ @@ -75,6 +76,11 @@ import { type User, } from '@kilocode/db/schema'; import { eq, or } from 'drizzle-orm'; +import { TRPCError } from '@trpc/server'; +import { + GitLabAuthenticationError, + GitLabProjectAccessTokenPermissionError, +} from '@/lib/integrations/platforms/gitlab/adapter'; import { prepareReviewPayload } from './prepare-review-payload'; const REPO = `test-org/prepare-review-payload-${Date.now()}`; @@ -725,6 +731,97 @@ describe('prepareReviewPayload', () => { ).rejects.toThrow('is missing its integration'); }); + it('reports a rejected GitLab credential as a reconnect problem', async () => { + const [review] = await db + .insert(cloud_agent_code_reviews) + .values( + defineReview(testUser.id, gitlabIntegration.id, { + platform: 'gitlab', + platform_project_id: 456, + pr_url: `https://gitlab.example.com/${REPO}/-/merge_requests/123`, + }) + ) + .returning(); + // The service wraps the adapter refusal, so the cause carries the kind. + mockGetOrCreateProjectAccessToken.mockRejectedValueOnce( + new TRPCError({ + code: 'UNAUTHORIZED', + message: + 'GitLab rejected the connected credential. Reconnect the GitLab integration and try again.', + cause: new GitLabAuthenticationError(456, 401, 'GitLab rejected the credential.'), + }) + ); + + const failure = await prepareReviewPayload({ + reviewId: review.id, + owner: { type: 'user', id: testUser.id, userId: testUser.id }, + agentConfig: { config: baseAgentConfig }, + platform: 'gitlab', + }).catch((error: unknown) => error); + + expect((failure as Error).message).toBe( + `GitLab rejected the connected credential while preparing this review of ${REPO}. Reconnect the GitLab integration, then retry.` + ); + expect((failure as Error).message).not.toContain('Maintainer'); + }); + + it('reports a broker reconnect refusal as a reconnect problem', async () => { + const [review] = await db + .insert(cloud_agent_code_reviews) + .values( + defineReview(testUser.id, gitlabIntegration.id, { + platform: 'gitlab', + platform_project_id: 456, + pr_url: `https://gitlab.example.com/${REPO}/-/merge_requests/123`, + }) + ) + .returning(); + mockGetOrCreateProjectAccessToken.mockRejectedValueOnce( + new TRPCError({ + code: 'UNAUTHORIZED', + message: 'GitLab integration must be reconnected', + }) + ); + + await expect( + prepareReviewPayload({ + reviewId: review.id, + owner: { type: 'user', id: testUser.id, userId: testUser.id }, + agentConfig: { config: baseAgentConfig }, + platform: 'gitlab', + }) + ).rejects.toThrow('Reconnect the GitLab integration, then retry.'); + }); + + it('keeps Maintainer-role guidance when GitLab refuses the role', async () => { + const [review] = await db + .insert(cloud_agent_code_reviews) + .values( + defineReview(testUser.id, gitlabIntegration.id, { + platform: 'gitlab', + platform_project_id: 456, + pr_url: `https://gitlab.example.com/${REPO}/-/merge_requests/123`, + }) + ) + .returning(); + mockGetOrCreateProjectAccessToken.mockRejectedValueOnce( + new GitLabProjectAccessTokenPermissionError( + 456, + 403, + 'Insufficient permissions to create Project Access Token for project 456. Requires Maintainer role or higher.' + ) + ); + + await expect( + prepareReviewPayload({ + reviewId: review.id, + owner: { type: 'user', id: testUser.id, userId: testUser.id }, + agentConfig: { config: baseAgentConfig }, + platform: 'gitlab', + }) + ).rejects.toThrow('You need Maintainer role or higher'); + }); + it('prepares a fresh tokenless Bitbucket review from exact organization integration identity', async () => { const [review] = await db .insert(cloud_agent_code_reviews) diff --git a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts index a84933ddb62..040e4a03b94 100644 --- a/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts +++ b/apps/web/src/lib/code-reviews/triggers/prepare-review-payload.ts @@ -47,6 +47,7 @@ import { getOrCreateProjectAccessToken, type GitLabIntegrationMetadata, } from '@/lib/integrations/gitlab-service'; +import { isGitLabReconnectRequiredError } from '@/lib/integrations/platforms/gitlab/credential-errors'; import type { ExistingReviewState, PreviousReviewStatus, @@ -557,6 +558,12 @@ export async function prepareReviewPayload( projectId, }); } catch (pratError) { + if (isGitLabReconnectRequiredError(pratError)) { + throw new Error( + `GitLab rejected the connected credential while preparing this review of ` + + `${review.repo_full_name}. Reconnect the GitLab integration, then retry.` + ); + } if (pratError instanceof GitLabProjectAccessTokenPermissionError) { throw new Error( `Cannot create Project Access Token for GitLab code review. ` + diff --git a/apps/web/src/lib/integrations/gitlab-service.credentials.test.ts b/apps/web/src/lib/integrations/gitlab-service.credentials.test.ts index 616a3a6e582..5e4caa50ad9 100644 --- a/apps/web/src/lib/integrations/gitlab-service.credentials.test.ts +++ b/apps/web/src/lib/integrations/gitlab-service.credentials.test.ts @@ -19,6 +19,10 @@ import { buildGitLabProjectAccessTokenAad, } from '@kilocode/worker-utils/gitlab-credential'; import type * as GitLabAdapter from '@/lib/integrations/platforms/gitlab/adapter'; +import { + GitLabAuthenticationError, + GitLabProjectAccessTokenPermissionError, +} from '@/lib/integrations/platforms/gitlab/adapter'; import type * as GitLabService from './gitlab-service'; import type { fetchGitLabCredential as FetchGitLabCredential } from '@/lib/integrations/platforms/gitlab/credential-broker-client'; @@ -960,4 +964,216 @@ describe('GitLab encrypted credential persistence', () => { expect(disconnectedIntegration.metadata).not.toHaveProperty('access_token'); expect(disconnectedIntegration.metadata).not.toHaveProperty('project_tokens'); }); + + it('keeps a new connection when the repository list cannot be loaded', async () => { + const user = await insertTestUser(); + mockFetchGitLabProjects.mockRejectedValueOnce(new Error('GitLab projects fetch failed: 500')); + + const result = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-primary-token', + 'https://gitlab.com', + user.id + ); + + const [integration] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, result.integration.id)); + const credentials = await db + .select() + .from(platform_access_token_credentials) + .where(eq(platform_access_token_credentials.platform_integration_id, integration.id)); + + expect(result.success).toBe(true); + expect(result.integration.id).toBe(integration.id); + expect(result.warnings).toEqual([ + 'Connected, but the repository list could not be loaded. Refresh the project list from the GitLab integration page.', + ]); + expect(integration.integration_status).toBe('active'); + expect(integration.repositories).toBeNull(); + expect(integration.repositories_synced_at).toBeNull(); + expect(credentials).toHaveLength(1); + }); + + it('keeps an existing connection and its webhook secret when the repository list cannot be loaded', async () => { + const user = await insertTestUser(); + const initial = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-initial-token', + 'https://gitlab.com', + user.id + ); + const [before] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, initial.integration.id)); + mockFetchGitLabProjects.mockRejectedValueOnce(new Error('GitLab projects fetch failed: 502')); + + const result = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-replacement-token', + 'https://gitlab.com', + user.id + ); + + const [after] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, initial.integration.id)); + + expect(result.success).toBe(true); + expect(result.warnings).toEqual([ + 'Connected, but the repository list could not be loaded. Refresh the project list from the GitLab integration page.', + ]); + expect(after.integration_status).toBe('active'); + // The cache from the earlier successful sync survives the failed refresh. + expect(after.repositories).toEqual(before.repositories); + expect(after.repositories_synced_at).toEqual(before.repositories_synced_at); + expect((after.metadata as Record).webhook_secret).toBe( + (before.metadata as Record).webhook_secret + ); + }); + + it('restores credential health on reconnect even when repository sync fails', async () => { + const user = await insertTestUser(); + const owner = { type: 'user', id: user.id } as const; + const initial = await connectWithPAT( + owner, + 'glpat-initial-token', + 'https://gitlab.com', + user.id + ); + await db + .update(platform_integrations) + .set({ + integration_status: 'suspended', + auth_invalid_at: new Date().toISOString(), + auth_invalid_reason: 'authorizing_user_deleted', + }) + .where(eq(platform_integrations.id, initial.integration.id)); + await db + .delete(platform_access_token_credentials) + .where(eq(platform_access_token_credentials.platform_integration_id, initial.integration.id)); + mockFetchGitLabProjects.mockRejectedValueOnce(new Error('GitLab projects fetch failed: 502')); + + const result = await connectWithPAT( + owner, + 'glpat-replacement-token', + 'https://gitlab.com', + user.id + ); + const [integration] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, initial.integration.id)); + const [credential] = await db + .select() + .from(platform_access_token_credentials) + .where(eq(platform_access_token_credentials.platform_integration_id, initial.integration.id)); + + expect(result.success).toBe(true); + expect(result.warnings).toEqual([ + 'Connected, but the repository list could not be loaded. Refresh the project list from the GitLab integration page.', + ]); + expect(integration.integration_status).toBe('active'); + expect(integration.auth_invalid_at).toBeNull(); + expect(integration.auth_invalid_reason).toBeNull(); + expect(credential.authorized_by_user_id).toBe(user.id); + }); + + it('discards the previous instance cache when the repository list cannot be loaded', async () => { + const user = await insertTestUser(); + mockFetchGitLabProjects.mockResolvedValueOnce([ + { id: 123, name: 'repo', full_name: 'group/sub/repo', private: true }, + ]); + const initial = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-initial-token', + 'https://gitlab.com', + user.id + ); + const [before] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, initial.integration.id)); + mockFetchGitLabProjects.mockRejectedValueOnce(new Error('GitLab projects fetch failed: 500')); + + const result = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-replacement-token', + 'https://gitlab.example.com', + user.id + ); + + const [after] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, initial.integration.id)); + + expect(before.repositories).toHaveLength(1); + expect(result.success).toBe(true); + expect(result.warnings).toEqual([ + 'Connected, but the repository list could not be loaded. Refresh the project list from the GitLab integration page.', + ]); + expect(after.repositories).toBeNull(); + expect(after.repositories_synced_at).toBeNull(); + }); + + it('reports a rejected project token creation as a reconnect error', async () => { + const user = await insertTestUser(); + const connected = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-primary-token', + 'https://gitlab.com', + user.id + ); + const [integration] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, connected.integration.id)); + mockCreateProjectAccessToken.mockRejectedValueOnce( + new GitLabAuthenticationError( + 42, + 401, + 'GitLab rejected the credential while creating a Project Access Token for project 42 (HTTP 401). Reconnect the GitLab integration and try again.' + ) + ); + + await expect( + getOrCreateProjectAccessToken(integration, 42, { userId: user.id }) + ).rejects.toMatchObject({ + code: 'UNAUTHORIZED', + message: + 'GitLab rejected the connected credential. Reconnect the GitLab integration and try again.', + }); + }); + + it('reports a refused project token role as a permission error', async () => { + const user = await insertTestUser(); + const connected = await connectWithPAT( + { type: 'user', id: user.id }, + 'glpat-primary-token', + 'https://gitlab.com', + user.id + ); + const [integration] = await db + .select() + .from(platform_integrations) + .where(eq(platform_integrations.id, connected.integration.id)); + mockCreateProjectAccessToken.mockRejectedValueOnce( + new GitLabProjectAccessTokenPermissionError( + 42, + 403, + 'Insufficient permissions to create Project Access Token for project 42. Requires Maintainer role or higher.' + ) + ); + + await expect( + getOrCreateProjectAccessToken(integration, 42, { userId: user.id }) + ).rejects.toMatchObject({ + code: 'FORBIDDEN', + message: expect.stringContaining('You need Maintainer role or higher'), + }); + }); }); diff --git a/apps/web/src/lib/integrations/gitlab-service.ts b/apps/web/src/lib/integrations/gitlab-service.ts index 6645384d334..716602a37fc 100644 --- a/apps/web/src/lib/integrations/gitlab-service.ts +++ b/apps/web/src/lib/integrations/gitlab-service.ts @@ -26,6 +26,7 @@ import { validatePersonalAccessToken, type GitLabProjectAccessToken, type GitLabPATValidationResult, + GitLabAuthenticationError, GitLabProjectAccessTokenPermissionError, } from '@/lib/integrations/platforms/gitlab/adapter'; import { randomBytes, randomUUID } from 'crypto'; @@ -35,6 +36,7 @@ import { GitLabInstanceUrlError, normalizeGitLabInstanceUrl, } from '@/lib/integrations/platforms/gitlab/instance-url'; +import { GITLAB_RECONNECT_REQUIRED_MESSAGE } from '@/lib/integrations/platforms/gitlab/credential-errors'; import { mutateGitLabMetadataInTransaction, readGitLabMetadataInTransaction, @@ -228,7 +230,7 @@ function requireAvailableGitLabCredential( case 'reconnect_required': throw new TRPCError({ code: 'UNAUTHORIZED', - message: 'GitLab integration must be reconnected', + message: GITLAB_RECONNECT_REQUIRED_MESSAGE, }); case 'temporarily_unavailable': throw new TRPCError({ @@ -713,6 +715,14 @@ export async function getOrCreateProjectAccessToken( return newToken.token; } catch (error) { + if (error instanceof GitLabAuthenticationError) { + throw new TRPCError({ + code: 'UNAUTHORIZED', + message: + 'GitLab rejected the connected credential. Reconnect the GitLab integration and try again.', + cause: error, + }); + } if (error instanceof GitLabProjectAccessTokenPermissionError) { throw new TRPCError({ code: 'FORBIDDEN', @@ -971,6 +981,61 @@ export async function importExistingProjectAccessToken( */ export { validatePersonalAccessToken, type GitLabPATValidationResult }; +const GITLAB_REPOSITORY_SYNC_WARNING = + 'Connected, but the repository list could not be loaded. Refresh the project list from the GitLab integration page.'; + +/** + * Caches the connected account's repositories after the integration row and + * credential are committed. + * + * A provider refusal here says nothing about the connection itself, so it is + * reported as a warning on the successful result instead of failing the + * connect. A new connection, or one whose previous list was dropped, then has + * an empty cache, so the next repository-list read refetches it (see + * `listGitLabRepositories`); a same-instance reconnect keeps its last + * successful list until the user refreshes it. + * + * `discardOnFailure` is for an instance change: the cached rows belong to the + * previous GitLab instance, so they are dropped rather than shown against a + * credential that cannot reach them. + */ +async function cacheConnectedGitLabRepositories( + integrationId: string, + token: string, + instanceUrl: string, + discardOnFailure: boolean +): Promise { + try { + const repos = await fetchGitLabProjects(token, instanceUrl); + await updateRepositoriesForIntegration(integrationId, repos); + logExceptInTest('[connectWithPAT] Repositories cached', { + integrationId, + repoCount: repos.length, + }); + return undefined; + } catch (error) { + logExceptInTest('[connectWithPAT] Repository sync failed after commit', { + integrationId, + error: error instanceof Error ? error.message : String(error), + discardOnFailure, + }); + if (discardOnFailure) { + try { + await db + .update(platform_integrations) + .set({ repositories: null, repositories_synced_at: null }) + .where(eq(platform_integrations.id, integrationId)); + } catch (clearError) { + logExceptInTest('[connectWithPAT] Failed to discard stale repositories', { + integrationId, + error: clearError instanceof Error ? clearError.message : String(clearError), + }); + } + } + return GITLAB_REPOSITORY_SYNC_WARNING; + } +} + /** * Connects GitLab using a Personal Access Token * @@ -986,6 +1051,8 @@ export { validatePersonalAccessToken, type GitLabPATValidationResult }; * If an existing integration exists, this function will update it instead of * creating a new one. This preserves webhook secrets and configured webhooks * so existing webhook configurations continue to work. + * Replacing a credential clears its authentication invalidation in the same + * transaction; connection health does not depend on the repository-cache sync. * * @param owner - User or organization owner * @param token - Personal Access Token @@ -1098,6 +1165,8 @@ export async function connectWithPAT( platform_account_login: validatedUser.username, scopes: validation.tokenInfo?.scopes ?? ['api'], integration_status: INTEGRATION_STATUS.ACTIVE, + auth_invalid_at: null, + auth_invalid_reason: null, updated_at: new Date().toISOString(), }) .where(eq(platform_integrations.id, existingIntegration.id)); @@ -1190,9 +1259,14 @@ export async function connectWithPAT( : countMetadataObjectEntries(existingMetadata.configured_webhooks), }); - // Fetch and cache repositories - const repos = await fetchGitLabProjects(token, normalizedInstanceUrl); - await updateRepositoriesForIntegration(existingIntegration.id, repos); + // Fetch and cache repositories. The connection is already committed, so a + // repository-list failure is a warning from here on, never a failed connect. + const syncWarning = await cacheConnectedGitLabRepositories( + existingIntegration.id, + token, + normalizedInstanceUrl, + isInstanceChange + ); return { success: true, @@ -1202,7 +1276,7 @@ export async function connectWithPAT( accountId: String(validatedUser.id), instanceUrl: normalizedInstanceUrl, }, - warnings: validation.warnings, + warnings: [...(validation.warnings ?? []), ...(syncWarning ? [syncWarning] : [])], }; } @@ -1282,14 +1356,14 @@ export async function connectWithPAT( authType: 'pat', }); - // 6. Fetch and cache repositories - const repos = await fetchGitLabProjects(token, normalizedInstanceUrl); - await updateRepositoriesForIntegration(integration.id, repos); - - logExceptInTest('[connectWithPAT] Repositories cached', { - integrationId: integration.id, - repoCount: repos.length, - }); + // 6. Fetch and cache repositories. The integration is committed above, so a + // repository-list failure is a warning from here on, never a failed connect. + const syncWarning = await cacheConnectedGitLabRepositories( + integration.id, + token, + normalizedInstanceUrl, + false + ); return { success: true, @@ -1299,6 +1373,6 @@ export async function connectWithPAT( accountId: String(validatedUser.id), instanceUrl: normalizedInstanceUrl, }, - warnings: validation.warnings, + warnings: [...(validation.warnings ?? []), ...(syncWarning ? [syncWarning] : [])], }; } diff --git a/apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts b/apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts index 4ad2dda5d38..751320e4604 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/adapter.test.ts @@ -15,12 +15,20 @@ import { validateGitLabInstance, validatePersonalAccessToken, createProjectWebhook, + createProjectAccessToken, deleteProjectWebhook, fetchGitLabProjects, fetchGitLabRootTextFileAtRef, fetchGitLabRepositorySize, + GitLabAuthenticationError, + GitLabProjectAccessTokenPermissionError, + GitLabWebhookPermissionError, getMRDiffRefs, getMRHeadCommit, + listProjectAccessTokens, + listProjectWebhooks, + revokeProjectAccessToken, + rotateProjectAccessToken, updateProjectWebhook, } from './adapter'; @@ -784,3 +792,70 @@ describe('validatePersonalAccessToken', () => { expect(mockFetch).not.toHaveBeenCalled(); }); }); + +describe('webhook and project access token refusals', () => { + const webhookOperations = [ + ['listProjectWebhooks', () => listProjectWebhooks('token', 123)], + [ + 'createProjectWebhook', + () => createProjectWebhook('token', 123, 'https://example.com/webhook', 'webhook-secret'), + ], + [ + 'updateProjectWebhook', + () => updateProjectWebhook('token', 123, 456, 'https://example.com/webhook', 'secret'), + ], + ] as const; + + const projectAccessTokenOperations = [ + [ + 'createProjectAccessToken', + () => createProjectAccessToken('token', 123, 'Kilo bot', '2027-01-01'), + ], + ['listProjectAccessTokens', () => listProjectAccessTokens('token', 123)], + ['rotateProjectAccessToken', () => rotateProjectAccessToken('token', 123, 456, '2027-01-01')], + ['revokeProjectAccessToken', () => revokeProjectAccessToken('token', 123, 456)], + ] as const; + + const allOperations = [...webhookOperations, ...projectAccessTokenOperations] as const; + + function mockRefusal(status: number) { + mockFetch.mockResolvedValueOnce({ + ok: false, + status, + headers: new Headers(), + text: async () => `refused: ${status}`, + json: async () => ({}), + }); + } + + beforeEach(() => { + mockFetch.mockReset(); + }); + + it.each(allOperations)( + '%s reports a 401 as an authentication failure that needs a reconnect', + async (_name, run) => { + mockRefusal(401); + + const refusal = await run().catch((error: unknown) => error); + + expect(refusal).toBeInstanceOf(GitLabAuthenticationError); + expect(refusal).toMatchObject({ statusCode: 401, projectId: 123 }); + expect((refusal as Error).message).toContain('Reconnect the GitLab integration'); + expect((refusal as Error).message).not.toContain('Maintainer'); + } + ); + + it.each(allOperations)('%s keeps a 403 as a permission failure', async (name, run) => { + const expectedError = webhookOperations.some(([operation]) => operation === name) + ? GitLabWebhookPermissionError + : GitLabProjectAccessTokenPermissionError; + mockRefusal(403); + + const refusal = await run().catch((error: unknown) => error); + + expect(refusal).toBeInstanceOf(expectedError); + expect(refusal).toMatchObject({ statusCode: 403, projectId: 123 }); + expect((refusal as Error).message).toContain('Requires Maintainer role or higher'); + }); +}); diff --git a/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts b/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts index aa2750c796a..6945ab1a4f3 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/adapter.ts @@ -573,6 +573,22 @@ export function verifyGitLabWebhookToken(token: string, expectedToken?: string): } } +/** + * GitLab rejected the credential itself (HTTP 401), before any role or + * permission check. Callers must send the user to reconnect the integration; + * role guidance cannot fix it. + */ +export class GitLabAuthenticationError extends Error { + constructor( + public projectId: string | number, + readonly statusCode: number, + message: string + ) { + super(message); + this.name = 'GitLabAuthenticationError'; + } +} + /** * Custom error class for webhook permission issues * Thrown when user doesn't have Maintainer+ role on a project @@ -643,7 +659,8 @@ function buildProjectWebhookPayload(webhookUrl: string, webhookSecret: string) { * @param accessToken - OAuth access token * @param projectId - GitLab project ID or path (URL-encoded) * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project (HTTP 403) */ export async function listProjectWebhooks( accessToken: string, @@ -670,7 +687,15 @@ export async function listProjectWebhooks( projectId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while listing webhooks for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabWebhookPermissionError( projectId, response.status, @@ -692,7 +717,8 @@ export async function listProjectWebhooks( * @param webhookUrl - URL to receive webhook events * @param webhookSecret - Secret token for webhook verification * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project (HTTP 403) */ export async function createProjectWebhook( accessToken: string, @@ -724,7 +750,15 @@ export async function createProjectWebhook( projectId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while creating a webhook for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabWebhookPermissionError( projectId, response.status, @@ -755,7 +789,8 @@ export async function createProjectWebhook( * @param webhookUrl - URL to receive webhook events * @param webhookSecret - Secret token for webhook verification * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabWebhookPermissionError} When user doesn't have Maintainer+ role on the project (HTTP 403) */ export async function updateProjectWebhook( accessToken: string, @@ -789,7 +824,15 @@ export async function updateProjectWebhook( hookId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while updating a webhook for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabWebhookPermissionError( projectId, response.status, @@ -1694,7 +1737,8 @@ export class GitLabProjectAccessTokenPermissionError extends Error { * @param scopes - Token scopes (default: ['api', 'self_rotate']) * @param accessLevel - Access level for the token (default: DEVELOPER = 30) * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role (HTTP 403) * * @see https://docs.gitlab.com/ee/api/project_access_tokens.html#create-a-project-access-token */ @@ -1735,7 +1779,15 @@ export async function createProjectAccessToken( projectId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while creating a Project Access Token for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabProjectAccessTokenPermissionError( projectId, response.status, @@ -1773,7 +1825,8 @@ export async function createProjectAccessToken( * @param accessToken - OAuth access token (requires Maintainer+ role) * @param projectId - GitLab project ID or path (URL-encoded) * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) - * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role (HTTP 403) * * @see https://docs.gitlab.com/ee/api/project_access_tokens.html#list-project-access-tokens */ @@ -1802,7 +1855,15 @@ export async function listProjectAccessTokens( projectId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while listing Project Access Tokens for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabProjectAccessTokenPermissionError( projectId, response.status, @@ -1885,6 +1946,8 @@ export async function getProjectAccessToken( * @param tokenId - ID of the Project Access Token to rotate * @param expiresAt - New expiration date in YYYY-MM-DD format (max 1 year from now) * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role (HTTP 403) * * @see https://docs.gitlab.com/ee/api/project_access_tokens.html#rotate-a-project-access-token */ @@ -1924,7 +1987,15 @@ export async function rotateProjectAccessToken( tokenId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while rotating a Project Access Token for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabProjectAccessTokenPermissionError( projectId, response.status, @@ -1959,6 +2030,8 @@ export async function rotateProjectAccessToken( * @param projectId - GitLab project ID or path (URL-encoded) * @param tokenId - ID of the Project Access Token to revoke * @param instanceUrl - GitLab instance URL (defaults to gitlab.com) + * @throws {GitLabAuthenticationError} When GitLab rejects the credential (HTTP 401) + * @throws {GitLabProjectAccessTokenPermissionError} When user doesn't have Maintainer+ role (HTTP 403) * * @see https://docs.gitlab.com/ee/api/project_access_tokens.html#revoke-a-project-access-token */ @@ -1991,7 +2064,15 @@ export async function revokeProjectAccessToken( tokenId, }); - if (response.status === 401 || response.status === 403) { + if (response.status === 401) { + throw new GitLabAuthenticationError( + projectId, + 401, + `GitLab rejected the credential while revoking a Project Access Token for project ${projectId} (HTTP 401). Reconnect the GitLab integration and try again.` + ); + } + + if (response.status === 403) { throw new GitLabProjectAccessTokenPermissionError( projectId, response.status, diff --git a/apps/web/src/lib/integrations/platforms/gitlab/credential-errors.ts b/apps/web/src/lib/integrations/platforms/gitlab/credential-errors.ts new file mode 100644 index 00000000000..2bd5807b154 --- /dev/null +++ b/apps/web/src/lib/integrations/platforms/gitlab/credential-errors.ts @@ -0,0 +1,26 @@ +import { TRPCError } from '@trpc/server'; +import { GitLabAuthenticationError } from './adapter'; + +/** + * The credential broker's definitive refusal: the stored integration credential + * cannot be used until the owner reconnects. + */ +export const GITLAB_RECONNECT_REQUIRED_MESSAGE = 'GitLab integration must be reconnected'; + +/** + * True when the error says the stored GitLab credential cannot be used as-is: + * a provider 401 (directly, or wrapped by a service call) or the credential + * broker's definitive reconnect refusal. Role guidance cannot fix either case, + * so callers route them to a reconnect message. + */ +export function isGitLabReconnectRequiredError(error: unknown): boolean { + if (error instanceof GitLabAuthenticationError) return true; + if (error instanceof TRPCError) { + if (error.code !== 'UNAUTHORIZED') return false; + return ( + error.message === GITLAB_RECONNECT_REQUIRED_MESSAGE || + error.cause instanceof GitLabAuthenticationError + ); + } + return error instanceof Error && error.cause instanceof GitLabAuthenticationError; +} diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.test.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.test.ts new file mode 100644 index 00000000000..29864b61a29 --- /dev/null +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.test.ts @@ -0,0 +1,75 @@ +import { + createProjectWebhook, + findKiloWebhook, + GitLabAuthenticationError, + GitLabWebhookPermissionError, +} from './adapter'; +import { syncWebhooksForRepositories } from './webhook-sync'; + +jest.mock('./adapter', () => ({ + ...jest.requireActual('./adapter'), + findKiloWebhook: jest.fn(), + createProjectWebhook: jest.fn(), + updateProjectWebhook: jest.fn(), + deleteProjectWebhook: jest.fn(), +})); + +const mockFindKiloWebhook = jest.mocked(findKiloWebhook); +const mockCreateProjectWebhook = jest.mocked(createProjectWebhook); + +describe('syncWebhooksForRepositories', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockFindKiloWebhook.mockResolvedValue(null); + }); + + it('asks the user to reconnect when GitLab rejects the credential', async () => { + mockCreateProjectWebhook.mockRejectedValueOnce( + new GitLabAuthenticationError( + 123, + 401, + 'GitLab rejected the credential while creating a webhook for project 123 (HTTP 401). Reconnect the GitLab integration and try again.' + ) + ); + + const { result, updatedWebhooks } = await syncWebhooksForRepositories( + 'token', + 'webhook-secret', + [123], + [], + {} + ); + + expect(result.created).toEqual([]); + expect(result.errors).toEqual([ + { + projectId: 123, + operation: 'create', + error: + 'GitLab rejected the connected credential: reconnect the GitLab integration, then retry. You can still configure the webhook manually in GitLab.', + }, + ]); + expect(updatedWebhooks).toEqual({}); + }); + + it('keeps Maintainer-role guidance when GitLab refuses the role', async () => { + mockCreateProjectWebhook.mockRejectedValueOnce( + new GitLabWebhookPermissionError( + 123, + 403, + 'Insufficient permissions to create webhook for project 123. Requires Maintainer role or higher.' + ) + ); + + const { result } = await syncWebhooksForRepositories('token', 'webhook-secret', [123], [], {}); + + expect(result.errors).toEqual([ + { + projectId: 123, + operation: 'create', + error: + 'Permission denied: You need Maintainer role or higher on this project to configure webhooks automatically. You can still configure the webhook manually in GitLab.', + }, + ]); + }); +}); diff --git a/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts index ad3bb163385..d511b956f38 100644 --- a/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts +++ b/apps/web/src/lib/integrations/platforms/gitlab/webhook-sync.ts @@ -5,6 +5,7 @@ import { deleteProjectWebhook, findKiloWebhook, updateProjectWebhook, + GitLabAuthenticationError, GitLabWebhookPermissionError, } from './adapter'; @@ -123,7 +124,9 @@ export async function syncWebhooksForRepositories( } } catch (error) { let errorMessage: string; - if (error instanceof GitLabWebhookPermissionError) { + if (error instanceof GitLabAuthenticationError) { + errorMessage = `GitLab rejected the connected credential: reconnect the GitLab integration, then retry. You can still configure the webhook manually in GitLab.`; + } else if (error instanceof GitLabWebhookPermissionError) { errorMessage = `Permission denied: You need Maintainer role or higher on this project to configure webhooks automatically. You can still configure the webhook manually in GitLab.`; } else { errorMessage = error instanceof Error ? error.message : String(error); @@ -139,6 +142,7 @@ export async function syncWebhooksForRepositories( projectId, error: errorMessage, isPermissionError: error instanceof GitLabWebhookPermissionError, + isAuthenticationError: error instanceof GitLabAuthenticationError, }); } }