Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 112 additions & 18 deletions apps/api/src/integration-platform/controllers/oauth.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -614,13 +614,33 @@ describe('OAuthController', () => {
config: {
authorizeUrl: 'https://github.com/login/oauth/authorize',
tokenUrl: 'https://github.com/login/oauth/access_token',
installUrl:
'https://github.com/apps/comp-ai-compliance/installations/new',
},
},
capabilities: [],
isActive: true,
};

it('blocks the GitHub App connect when the App is not installed', async () => {
const notInstalledFetch = () =>
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);

it('redirects to the install URL when the App is not installed (first pass)', async () => {
const futureDate = new Date(Date.now() + 600000);
mockOAuthStateRepository.findByState.mockResolvedValue({
state: 'gh_state',
Expand All @@ -638,31 +658,105 @@ describe('OAuthController', () => {
scopes: [],
source: 'platform',
});
mockOAuthStateRepository.create.mockResolvedValue({
state: 'install_state',
});

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);
const fetchSpy = notInstalledFetch();

await controller.oauthCallback(
{ code: 'auth_code', state: 'gh_state' },
mockRequest,
mockResponse,
);

// No installation yet → send them to install, don't finalize.
expect(mockConnectionService.activateConnection).not.toHaveBeenCalled();
const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0];
expect(redirectUrl).toContain(
'https://github.com/apps/comp-ai-compliance/installations/new',
);
expect(redirectUrl).toContain('state=install_state');

// The install-handoff state must be created BEFORE the original state is
// deleted, so a create failure can't leave the outer catch double-deleting
// an already-removed record (which would hang the request).
const createOrder =
mockOAuthStateRepository.create.mock.invocationCallOrder[0];
const deleteOrder =
mockOAuthStateRepository.delete.mock.invocationCallOrder[0];
expect(createOrder).toBeLessThan(deleteOrder);

fetchSpy.mockRestore();
});

it('still emits an error redirect if creating the install-handoff state fails', 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',
});
// Creating the install-handoff state fails (e.g. DB error).
mockOAuthStateRepository.create.mockRejectedValue(new Error('db down'));

const fetchSpy = notInstalledFetch();

await controller.oauthCallback(
{ code: 'auth_code', state: 'gh_state' },
mockRequest,
mockResponse,
);

// The connection must NOT be activated when the App isn't installed.
// The original state was NOT deleted before the failed create, so the
// outer catch can still clean it up and emit an error redirect (no hang).
expect(mockOAuthStateRepository.delete).toHaveBeenCalledWith('gh_state');
const errRedirect = (mockResponse.redirect as jest.Mock).mock.calls[0][0];
expect(errRedirect).toContain('error=');
expect(mockConnectionService.activateConnection).not.toHaveBeenCalled();

fetchSpy.mockRestore();
});

it('errors (no loop) when still not installed after an install attempt', 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 = notInstalledFetch();

// installation_id present → we already came back from an install attempt.
await controller.oauthCallback(
{ code: 'auth_code', state: 'gh_state', installation_id: '999' },
mockRequest,
mockResponse,
);

expect(mockConnectionService.activateConnection).not.toHaveBeenCalled();
const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0];
expect(redirectUrl).toContain('error=github_app_not_installed');
Expand Down
46 changes: 38 additions & 8 deletions apps/api/src/integration-platform/controllers/oauth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ interface OAuthCallbackQuery {
state: string;
error?: string;
error_description?: string;
// Present when GitHub returns from an App installation (install-time OAuth).
// Used only as a loop guard for the install redirect — never persisted.
installation_id?: string;
}

@Controller({ path: 'integrations/oauth', version: '1' })
Expand Down Expand Up @@ -335,26 +338,53 @@ 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.
// GitHub App: user *authorization* and app *installation* are separate on
// GitHub. A user can complete OAuth without installing the App, producing a
// token that can read no repositories. If GitHub definitively reports no
// installation, don't finalize a useless connection — send the user to
// install the App (choose org + repositories). They return through
// install-time OAuth (with an installation_id) and this check then passes.
// 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))
) {
// First pass (came from plain authorize, no installation_id): send the
// user to install the App, then they return here via install-time OAuth.
if (!query.installation_id && oauthConfig.installUrl) {
// Create the install-handoff state BEFORE deleting the original one.
// If creation fails, the original state survives so the outer catch can
// clean it up and still emit an error redirect — deleting first would
// make the catch's delete throw (record gone) and hang the request.
const installState = await this.oauthStateRepository.create({
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
providerSlug: oauthState.providerSlug,
organizationId: oauthState.organizationId,
userId: oauthState.userId,
redirectUrl: oauthState.redirectUrl ?? undefined,
});
await this.oauthStateRepository.delete(state);
const installRedirect = new URL(oauthConfig.installUrl);
installRedirect.searchParams.set('state', installState.state);
this.logger.log(
`GitHub App authorized but not installed; redirecting org ${oauthState.organizationId} to install`,
);
res.redirect(installRedirect.toString());
return;
}

// We already came back from an install attempt but still see no
// installation — stop rather than loop.
await this.oauthStateRepository.delete(state);
this.logger.warn(
`GitHub App authorized but not installed for org ${oauthState.organizationId}`,
`GitHub App still not installed after install attempt 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.',
'The GitHub App was not installed on your organization. Please install it (selecting at least one repository) and try again.',
},
oauthState.organizationId,
);
Expand Down
35 changes: 20 additions & 15 deletions packages/integration-platform/src/manifests/github-app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,21 @@
* is left completely untouched so current connections keep working. New/concerned
* customers opt into this one.
*
* Connect flow (user-to-server token via the standard OAuth authorize URL):
* Connect flow (user-to-server token — GitHub separates user *authorization*
* from app *installation*, so the callback bridges the two):
* 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.
*
* 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 App's client id. This returns a `code` to our callback whether or not
* the App is installed (unlike the install URL, which dead-ends on a
* "manage" page for already-installed orgs), and it honors `redirect_uri`
* for per-environment callback routing.
* 2. The callback exchanges the `code` for a user-to-server token, then checks
* whether the user actually has an installation (GET /user/installations):
* - Installed → finalize the connection.
* - NOT installed → redirect to `installUrl` so the user installs the App
* on their org and picks repositories. GitHub then returns through
* install-time OAuth (with an `installation_id`), the check passes, and
* the connection is finalized. The `installation_id` on the return trip
* is used only as a loop guard, never persisted.
*
* 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
Expand Down Expand Up @@ -69,11 +71,14 @@ export const githubAppManifest: IntegrationManifest = {
// 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.
// already-installed orgs) 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',
// Where to send a user who authorized but has NOT installed the App yet, so
// they can install it on their org and choose repositories. The public app
// slug is the same across environments (one App, multiple callback URLs).
installUrl: 'https://github.com/apps/comp-ai-compliance/installations/new',
scopes: [],
pkce: false,
clientAuthMethod: 'body',
Expand Down
8 changes: 8 additions & 0 deletions packages/integration-platform/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,14 @@ export const OAuthConfigSchema = z.object({
* Default: true
*/
supportsRefreshToken: z.boolean().default(true),
/**
* App-installation URL for providers (e.g. GitHub Apps) where user
* authorization and app installation are separate steps. When set, the OAuth
* callback can redirect a user who authorized but has NOT installed the app to
* this URL to complete installation (choosing account/repositories) before the
* connection is finalized.
*/
installUrl: z.string().url().optional(),
/**
* Separate URL for token refresh (if different from tokenUrl).
* Most providers use the same tokenUrl for both.
Expand Down
Loading