diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts index cf3fdefc36..fda90c376a 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.spec.ts @@ -313,93 +313,6 @@ describe('OAuthController', () => { expect(result.authorizationUrl).toContain('code_challenge='); expect(result.authorizationUrl).toContain('code_challenge_method=S256'); }); - - it('should return a GitHub App install URL (state only) for appInstallFlow providers', async () => { - const manifest = { - id: 'github-app', - name: 'GitHub App', - auth: { - type: 'oauth2', - config: { - authorizeUrl: - 'https://github.com/apps/{APP_SLUG}/installations/new', - tokenUrl: 'https://github.com/login/oauth/access_token', - pkce: false, - appInstallFlow: true, - additionalOAuthSettings: [{ id: 'appSlug', token: '{APP_SLUG}' }], - }, - }, - category: 'Development', - capabilities: [], - isActive: true, - }; - mockedGetManifest.mockReturnValue(manifest as never); - mockOAuthCredentialsService.getCredentials.mockResolvedValue({ - clientId: 'client_123', - clientSecret: 'secret_456', - scopes: [], - source: 'platform', - customSettings: { appSlug: 'comp-ai' }, - }); - mockProviderRepository.upsert.mockResolvedValue(undefined); - mockOAuthStateRepository.create.mockResolvedValue({ - state: 'state_install', - }); - - const result = await controller.startOAuth('org_1', 'user_1', { - providerSlug: 'github-app', - }); - - // Install URL with the slug substituted and only `state` appended. - expect(result.authorizationUrl).toContain( - 'https://github.com/apps/comp-ai/installations/new', - ); - expect(result.authorizationUrl).toContain('state=state_install'); - // GitHub ignores redirect_uri on the install URL and routes to the App's - // first registered callback, so we deliberately do NOT set it here. - expect(result.authorizationUrl).not.toContain('redirect_uri='); - // OAuth authorize params must NOT be on an install URL. - expect(result.authorizationUrl).not.toContain('client_id='); - expect(result.authorizationUrl).not.toContain('response_type='); - expect(result.authorizationUrl).not.toContain('scope='); - }); - - it('should throw PRECONDITION_FAILED for an install-flow provider when the app slug is not configured', async () => { - const manifest = { - id: 'github-app', - name: 'GitHub App', - auth: { - type: 'oauth2', - config: { - authorizeUrl: - 'https://github.com/apps/{APP_SLUG}/installations/new', - tokenUrl: 'https://github.com/login/oauth/access_token', - pkce: false, - appInstallFlow: true, - additionalOAuthSettings: [{ id: 'appSlug', token: '{APP_SLUG}' }], - }, - }, - category: 'Development', - capabilities: [], - isActive: true, - }; - mockedGetManifest.mockReturnValue(manifest as never); - // No customSettings.appSlug → {APP_SLUG} cannot be resolved. - mockOAuthCredentialsService.getCredentials.mockResolvedValue({ - clientId: 'client_123', - clientSecret: 'secret_456', - scopes: [], - source: 'organization', - }); - mockProviderRepository.upsert.mockResolvedValue(undefined); - mockOAuthStateRepository.create.mockResolvedValue({ state: 's' }); - - await expect( - controller.startOAuth('org_1', 'user_1', { - providerSlug: 'github-app', - }), - ).rejects.toThrow(HttpException); - }); }); describe('oauthCallback', () => { @@ -692,6 +605,132 @@ describe('OAuthController', () => { fetchSpy.mockRestore(); }); + const githubAppManifest = { + id: 'github-app', + name: 'GitHub App', + category: 'Development', + auth: { + type: 'oauth2', + config: { + authorizeUrl: 'https://github.com/login/oauth/authorize', + tokenUrl: 'https://github.com/login/oauth/access_token', + }, + }, + capabilities: [], + isActive: true, + }; + + it('blocks the GitHub App connect when the App is not installed', async () => { + const futureDate = new Date(Date.now() + 600000); + mockOAuthStateRepository.findByState.mockResolvedValue({ + state: 'gh_state', + providerSlug: 'github-app', + organizationId: 'org_1', + userId: 'user_1', + codeVerifier: null, + redirectUrl: null, + expiresAt: futureDate, + }); + mockedGetManifest.mockReturnValue(githubAppManifest as never); + mockOAuthCredentialsService.getCredentials.mockResolvedValue({ + clientId: 'c', + clientSecret: 's', + scopes: [], + source: 'platform', + }); + + const fetchSpy = jest + .spyOn(global, 'fetch') + // 1) token exchange + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve({ access_token: 'gh_token' }), + text: () => Promise.resolve(''), + } as unknown as Response) + // 2) GET /user/installations -> none + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve({ total_count: 0, installations: [] }), + text: () => Promise.resolve(''), + } as unknown as Response); + + await controller.oauthCallback( + { code: 'auth_code', state: 'gh_state' }, + mockRequest, + mockResponse, + ); + + // The connection must NOT be activated when the App isn't installed. + expect(mockConnectionService.activateConnection).not.toHaveBeenCalled(); + const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; + expect(redirectUrl).toContain('error=github_app_not_installed'); + + fetchSpy.mockRestore(); + }); + + it('completes the GitHub App connect when an installation exists', async () => { + const futureDate = new Date(Date.now() + 600000); + mockOAuthStateRepository.findByState.mockResolvedValue({ + state: 'gh_state', + providerSlug: 'github-app', + organizationId: 'org_1', + userId: 'user_1', + codeVerifier: null, + redirectUrl: null, + expiresAt: futureDate, + }); + mockedGetManifest.mockReturnValue(githubAppManifest as never); + mockOAuthCredentialsService.getCredentials.mockResolvedValue({ + clientId: 'c', + clientSecret: 's', + scopes: [], + source: 'platform', + }); + mockProviderRepository.findBySlug.mockResolvedValue({ id: 'p_gh' }); + mockConnectionRepository.findByProviderAndOrg.mockResolvedValue({ + id: 'conn_gh', + metadata: {}, + variables: {}, + lastSyncAt: null, + }); + mockConnectionService.activateConnection.mockResolvedValue({ + id: 'conn_gh', + }); + + const fetchSpy = jest + .spyOn(global, 'fetch') + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => Promise.resolve({ access_token: 'gh_token' }), + text: () => Promise.resolve(''), + } as unknown as Response) + .mockResolvedValueOnce({ + ok: true, + status: 200, + json: () => + Promise.resolve({ total_count: 1, installations: [{ id: 1 }] }), + text: () => Promise.resolve(''), + } as unknown as Response); + + await controller.oauthCallback( + { code: 'auth_code', state: 'gh_state' }, + mockRequest, + mockResponse, + ); + await new Promise((resolve) => setImmediate(resolve)); + + expect(mockConnectionService.activateConnection).toHaveBeenCalledWith( + 'conn_gh', + ); + const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0]; + expect(redirectUrl).toContain('success=true'); + + fetchSpy.mockRestore(); + }); + describe('session defense-in-depth', () => { const futureDate = new Date(Date.now() + 600000); const validState = { diff --git a/apps/api/src/integration-platform/controllers/oauth.controller.ts b/apps/api/src/integration-platform/controllers/oauth.controller.ts index 8007d2249b..0b7673d86d 100644 --- a/apps/api/src/integration-platform/controllers/oauth.controller.ts +++ b/apps/api/src/integration-platform/controllers/oauth.controller.ts @@ -182,44 +182,6 @@ export class OAuthController { } const authUrl = new URL(authorizeUrl); - // GitHub App installation flow: the connect step is an App *installation* - // (github.com/apps/{slug}/installations/new), not a standard OAuth authorize. - // GitHub redirects to the App's FIRST registered callback URL after install - // and IGNORES any redirect_uri passed to the install URL (confirmed GitHub - // behavior), so only `state` is set here — client_id, response_type and - // scope do not apply either. Per-environment routing is therefore handled by - // using a separate GitHub App per environment (each with its own single - // callback URL), NOT via redirect_uri. The OAuth `code` still comes back on - // that callback (with "Request user authorization during installation" - // enabled), so token exchange proceeds normally afterwards. - if (oauthConfig.appInstallFlow) { - // Every placeholder in the install URL (e.g. {APP_SLUG}) must have been - // resolved from credentials above. If a required setting like the GitHub - // App slug was never persisted (e.g. org-scoped credentials saved without - // customSettings), fail loudly with a clear error instead of redirecting - // the user to a broken GitHub URL. - const unresolvedTokens = (oauthConfig.additionalOAuthSettings ?? []) - .map((setting) => setting.token) - .filter( - (token): token is string => !!token && authorizeUrl.includes(token), - ); - if (unresolvedTokens.length > 0) { - throw new HttpException( - { - message: `GitHub App is not fully configured for ${providerSlug} (missing: ${unresolvedTokens.join(', ')}). Set the App slug in the integration credentials.`, - setupInstructions: oauthConfig.setupInstructions, - createAppUrl: oauthConfig.createAppUrl, - }, - HttpStatus.PRECONDITION_FAILED, - ); - } - authUrl.searchParams.set('state', oauthState.state); - this.logger.log( - `Starting GitHub App install flow for ${providerSlug}, org: ${organizationId} (credentials from ${credentials.source})`, - ); - return { authorizationUrl: authUrl.toString() }; - } - // Standard OAuth2 params authUrl.searchParams.set('client_id', credentials.clientId); authUrl.searchParams.set('response_type', 'code'); @@ -373,6 +335,33 @@ export class OAuthController { oauthState.codeVerifier, ); + // GitHub App: a user can complete OAuth *without* installing the App, + // which yields a token that cannot read any repositories. If GitHub + // definitively reports no installation for this user, stop here and tell + // them to install the App instead of creating a "connected" but unusable + // integration. Fails open on any uncertainty so a valid connection is + // never blocked by a transient error. + if ( + oauthState.providerSlug === 'github-app' && + (await this.githubAppInstallationMissing(tokens.access_token)) + ) { + await this.oauthStateRepository.delete(state); + this.logger.warn( + `GitHub App authorized but not installed for org ${oauthState.organizationId}`, + ); + const errorUrl = this.buildRedirectUrl( + oauthState.redirectUrl, + { + error: 'github_app_not_installed', + error_description: + 'You authorized Comp AI, but the GitHub App is not installed on your organization yet. Install the App on GitHub, then reconnect.', + }, + oauthState.organizationId, + ); + res.redirect(errorUrl); + return; + } + // Get or create provider const provider = await this.providerRepository.findBySlug( oauthState.providerSlug, @@ -639,6 +628,37 @@ export class OAuthController { return tokens; } + /** + * Returns true ONLY when GitHub definitively reports that the user has no + * installation of the GitHub App. A GitHub App user token can be obtained + * without the App being installed (install and user-authorization are separate + * steps), and such a token cannot read any repositories. Fails open — on any + * non-OK response or error we return false so a valid connection is never + * blocked by a transient failure. + */ + private async githubAppInstallationMissing( + accessToken: string, + ): Promise { + try { + const response = await fetch( + 'https://api.github.com/user/installations', + { + headers: { + Authorization: `Bearer ${accessToken}`, + Accept: 'application/vnd.github+json', + 'User-Agent': 'CompAI-Integration', + }, + }, + ); + if (!response.ok) return false; + const data = (await response.json()) as { total_count?: number }; + return data?.total_count === 0; + } catch (error) { + this.logger.warn(`Failed to verify GitHub App installation: ${error}`); + return false; + } + } + /** * Mark Rippling app as installed (required by Rippling) * See: https://developer.rippling.com/documentation/developer-portal/v2-guides/installation diff --git a/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts b/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts index 05a489229c..32d4d5d6c7 100644 --- a/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts +++ b/packages/integration-platform/src/manifests/github-app/__tests__/manifest.test.ts @@ -5,8 +5,9 @@ import { githubAppManifest } from '../index'; /** * CS-710: the new `github-app` integration must request read-only, fine-grained - * access (a GitHub App install flow) while leaving the legacy `github` OAuth - * integration completely untouched so existing connections keep working. + * access (a GitHub App via the standard OAuth authorize flow) while leaving the + * legacy `github` OAuth integration completely untouched so existing connections + * keep working. */ describe('github-app manifest (CS-710)', () => { it('is registered in the registry', () => { @@ -14,27 +15,16 @@ describe('github-app manifest (CS-710)', () => { expect(getAllManifests().some((m) => m.id === 'github-app')).toBe(true); }); - it('uses a read-only GitHub App install flow (no `repo` scope requested)', () => { + it('uses the standard OAuth authorize flow with no `repo` scope (read-only via the App)', () => { const { auth } = githubAppManifest; expect(auth.type).toBe('oauth2'); if (auth.type !== 'oauth2') return; - expect(auth.config.appInstallFlow).toBe(true); // GitHub Apps ignore scopes — permissions come from the App config. expect(auth.config.scopes).toEqual([]); - expect(auth.config.authorizeUrl).toContain('{APP_SLUG}'); - expect(auth.config.authorizeUrl).toContain('installations/new'); + expect(auth.config.authorizeUrl).toBe('https://github.com/login/oauth/authorize'); expect(auth.config.tokenUrl).toBe('https://github.com/login/oauth/access_token'); }); - it('exposes an admin-configurable app slug setting', () => { - const { auth } = githubAppManifest; - if (auth.type !== 'oauth2') throw new Error('expected oauth2 auth'); - const appSlug = auth.config.additionalOAuthSettings?.find((s) => s.id === 'appSlug'); - expect(appSlug).toBeDefined(); - expect(appSlug?.token).toBe('{APP_SLUG}'); - expect(appSlug?.required).toBe(true); - }); - it('reuses the exact same checks as the legacy github manifest', () => { const appCheckIds = githubAppManifest.checks?.map((c) => c.id).sort(); const legacyCheckIds = githubManifest.checks?.map((c) => c.id).sort(); @@ -49,6 +39,5 @@ describe('github-app manifest (CS-710)', () => { throw new Error('expected oauth2 auth'); } expect(githubManifest.auth.config.scopes).toContain('repo'); - expect(githubManifest.auth.config.appInstallFlow).toBeUndefined(); }); }); diff --git a/packages/integration-platform/src/manifests/github-app/index.ts b/packages/integration-platform/src/manifests/github-app/index.ts index a4a1092422..dba88a881a 100644 --- a/packages/integration-platform/src/manifests/github-app/index.ts +++ b/packages/integration-platform/src/manifests/github-app/index.ts @@ -15,17 +15,19 @@ * is left completely untouched so current connections keep working. New/concerned * customers opt into this one. * - * Connect flow (user-to-server token): - * 1. The customer is sent to the App's installation page - * (`https://github.com/apps/{APP_SLUG}/installations/new`). - * 2. They install the App on their org/account and pick repositories. - * 3. With "Request user authorization (OAuth) during installation" enabled on - * the App, GitHub redirects back through the OAuth flow and returns a `code` - * (plus `installation_id`). - * 4. The shared OAuth callback exchanges the `code` for a user-to-server access + * Connect flow (user-to-server token via the standard OAuth authorize URL): + * 1. The customer is sent to `https://github.com/login/oauth/authorize` with + * the App's client id. First-time users get an "Install & Authorize" screen + * (install the App + pick repositories); already-installed users just + * authorize. Either way GitHub returns to our callback with a `code`. + * 2. The shared OAuth callback exchanges the `code` for a user-to-server access * token (read-only, capped by the App's permissions) and stores it like any - * other OAuth token. The `installation_id` is persisted on the connection so - * a future server-to-server (installation token) upgrade needs no re-connect. + * other OAuth token. + * + * We deliberately use the authorize URL rather than the App's install URL: the + * install URL dead-ends on a "manage" page (no `code`) for orgs that already have + * the App installed, whereas the authorize URL works in both cases and honors + * `redirect_uri` for per-environment callback routing. * * The five checks are reused verbatim from the `github` manifest — only the way * the token is obtained differs (read-only App vs read/write OAuth App), so there @@ -60,21 +62,21 @@ export const githubAppManifest: IntegrationManifest = { auth: { type: 'oauth2', config: { - // "Connect" installs the GitHub App. {APP_SLUG} is replaced at runtime with - // customSettings.appSlug from the configured platform/org credentials - // (reuses the same token-substitution mechanism as Rippling's {APP_NAME}). - authorizeUrl: 'https://github.com/apps/{APP_SLUG}/installations/new', + // Standard OAuth user-authorization flow — but the client id/secret belong + // to a GitHub *App*, not an OAuth App. GitHub Apps ignore OAuth scopes (the + // App's own read-only permissions apply), so no scope is requested. + // + // We use login/oauth/authorize rather than the App's install URL on + // purpose: the authorize URL returns a `code` whether or not the App is + // already installed (the install URL dead-ends on the "manage" page for + // already-installed orgs), shows an "Install & Authorize" screen for + // first-time users, and honors redirect_uri so each environment routes to + // its own callback. + authorizeUrl: 'https://github.com/login/oauth/authorize', tokenUrl: 'https://github.com/login/oauth/access_token', - // GitHub Apps ignore OAuth scopes — permissions come from the App's own - // configuration (set to read-only when the App is created). Intentionally - // empty so no scope is requested at connect time. scopes: [], pkce: false, clientAuthMethod: 'body', - // This is a GitHub App *installation* flow, not a standard OAuth authorize. - // The connect step redirects to the install URL; the callback still returns - // an OAuth `code` (with user-authorization-during-installation enabled). - appInstallFlow: true, // With "Expire user authorization tokens" DISABLED on the App, the token is // long-lived (like the legacy GitHub OAuth token), so no refresh is needed. supportsRefreshToken: false, @@ -88,29 +90,17 @@ export const githubAppManifest: IntegrationManifest = { }, setupInstructions: `Create the GitHub App once (a GitHub organization owner must do this): 1. GitHub > Settings > Developer settings > GitHub Apps > "New GitHub App". -2. Set "Callback URL" to the callback URL shown below, and enable - "Request user authorization (OAuth) during installation". +2. Set "Callback URL" to the callback URL shown below (add one per environment), + and enable "Request user authorization (OAuth) during installation". 3. Disable "Expire user authorization tokens" (so tokens stay valid, like the old integration). 4. Under Permissions, grant READ-ONLY access: • Repository → Metadata, Contents, Pull requests, Administration, Dependabot alerts • Organization → Members -5. Create the App, then generate a client secret. -6. Enter the Client ID, Client Secret, and the App slug (the "" in the - App's public URL github.com/apps/) in the credentials form.`, +5. Set "Where can this GitHub App be installed?" to "Any account". +6. Create the App, then generate a client secret. +7. Enter the Client ID and Client Secret in the credentials form.`, createAppUrl: 'https://github.com/settings/apps/new', - additionalOAuthSettings: [ - { - id: 'appSlug', - label: 'GitHub App Slug', - type: 'text', - required: true, - placeholder: 'comp-ai', - helpText: - "Your GitHub App's slug from its public page (github.com/apps/). Used to build the installation URL.", - token: '{APP_SLUG}', - }, - ], }, }, diff --git a/packages/integration-platform/src/types.ts b/packages/integration-platform/src/types.ts index 7f6bed8845..acbbef67da 100644 --- a/packages/integration-platform/src/types.ts +++ b/packages/integration-platform/src/types.ts @@ -32,17 +32,6 @@ export const OAuthConfigSchema = z.object({ * Default: true */ supportsRefreshToken: z.boolean().default(true), - /** - * GitHub App installation flow. When true, the "connect" step sends the user - * to a GitHub App *installation* URL (e.g. - * `https://github.com/apps/{slug}/installations/new`) instead of a standard - * OAuth authorize URL. Only `state` is appended to that URL — - * client_id/response_type/scope do not apply to an install URL. The OAuth - * `code` still comes back on the callback (with "Request user authorization - * during installation" enabled on the App), so token exchange is unchanged. - * Default: false (standard OAuth authorize flow). - */ - appInstallFlow: z.boolean().optional(), /** * Separate URL for token refresh (if different from tokenUrl). * Most providers use the same tokenUrl for both.