Skip to content
Merged
175 changes: 175 additions & 0 deletions apps/api/src/integration-platform/controllers/oauth.controller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,93 @@ 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', () => {
Expand Down Expand Up @@ -605,6 +692,94 @@ describe('OAuthController', () => {
fetchSpy.mockRestore();
});

it('persists installation_id on the connection for the GitHub App install flow', async () => {
const futureDate = new Date(Date.now() + 600000);
mockOAuthStateRepository.findByState.mockResolvedValue({
state: 'valid_gh_state',
providerSlug: 'github-app',
organizationId: 'org_1',
userId: 'user_1',
codeVerifier: null,
redirectUrl: null,
expiresAt: futureDate,
});
mockedGetManifest.mockReturnValue({
id: 'github-app',
name: 'GitHub App',
category: 'Development',
auth: {
type: 'oauth2',
config: {
authorizeUrl:
'https://github.com/apps/{APP_SLUG}/installations/new',
tokenUrl: 'https://github.com/login/oauth/access_token',
appInstallFlow: true,
},
},
capabilities: [],
isActive: true,
} as never);
mockOAuthCredentialsService.getCredentials.mockResolvedValue({
clientId: 'client_123',
clientSecret: 'secret_456',
scopes: [],
source: 'platform',
});
mockProviderRepository.findBySlug.mockResolvedValue({
id: 'provider_gh',
});
mockConnectionRepository.findByProviderAndOrg.mockResolvedValue({
id: 'conn_gh',
metadata: {},
variables: {},
lastSyncAt: null,
});
mockConnectionRepository.update.mockResolvedValue({
id: 'conn_gh',
metadata: {},
variables: {},
lastSyncAt: null,
});
mockConnectionService.activateConnection.mockResolvedValue({
id: 'conn_gh',
});

const fetchSpy = jest.spyOn(global, 'fetch').mockResolvedValue({
ok: true,
status: 200,
json: () => Promise.resolve({ access_token: 'gh_user_token' }),
text: () => Promise.resolve(''),
} as unknown as Response);

await controller.oauthCallback(
{
code: 'auth_code',
state: 'valid_gh_state',
installation_id: '987654',
setup_action: 'install',
},
mockRequest,
mockResponse,
);

await new Promise<void>((resolve) => setImmediate(resolve));

expect(mockConnectionRepository.update).toHaveBeenCalledWith(
'conn_gh',
expect.objectContaining({
metadata: expect.objectContaining({
githubInstallationId: '987654',
githubSetupAction: 'install',
}),
}),
);
const redirectUrl = (mockResponse.redirect as jest.Mock).mock.calls[0][0];
expect(redirectUrl).toContain('success=true');
expect(redirectUrl).toContain('provider=github-app');

fetchSpy.mockRestore();
});

describe('session defense-in-depth', () => {
const futureDate = new Date(Date.now() + 600000);
const validState = {
Expand Down
83 changes: 76 additions & 7 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;
// GitHub App installation flow returns these alongside `code`.
installation_id?: string;
setup_action?: string;
}

@Controller({ path: 'integrations/oauth', version: '1' })
Expand Down Expand Up @@ -182,6 +185,44 @@ 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');
Expand Down Expand Up @@ -287,7 +328,10 @@ export class OAuthController {
// is the one completing it. The `state` token alone provides CSRF
// protection, but a session match guards against state-token leakage and
// ensures the completing user still has an active session for the org.
const sessionMismatch = await this.checkSessionMatchesState(req, oauthState);
const sessionMismatch = await this.checkSessionMatchesState(
req,
oauthState,
);
if (sessionMismatch) {
await this.oauthStateRepository.delete(state);
this.logger.warn(
Expand Down Expand Up @@ -355,11 +399,15 @@ export class OAuthController {
}

// Store tokens and mark connection as active
await this.credentialVaultService.storeOAuthTokens(connection.id, tokens, {
preserveExistingRefreshToken:
oauthState.providerSlug === 'gcp' ||
oauthState.providerSlug === 'google-workspace',
});
await this.credentialVaultService.storeOAuthTokens(
connection.id,
tokens,
{
preserveExistingRefreshToken:
oauthState.providerSlug === 'gcp' ||
oauthState.providerSlug === 'google-workspace',
},
);

// Mark cloud OAuth reconnect completion so reconnect banners clear after successful OAuth.
if (manifest.category === 'Cloud') {
Expand All @@ -377,6 +425,28 @@ export class OAuthController {
});
}

// GitHub App installation flow: persist the installation id (and setup
// action) on the connection. The token stored above is a user-to-server
// token, but recording the installation id means a future server-to-server
// (installation access token) upgrade needs no re-connect.
if (query.installation_id) {
const metadata =
connection.metadata &&
typeof connection.metadata === 'object' &&
!Array.isArray(connection.metadata)
? (connection.metadata as Record<string, unknown>)
: {};
connection = await this.connectionRepository.update(connection.id, {
metadata: {
...metadata,
githubInstallationId: query.installation_id,
Comment thread
cubic-dev-ai[bot] marked this conversation as resolved.
Outdated
...(query.setup_action
? { githubSetupAction: query.setup_action }
: {}),
},
});
}

await this.connectionService.activateConnection(connection.id);

// Provider-specific post-OAuth actions
Expand Down Expand Up @@ -643,5 +713,4 @@ export class OAuthController {
}
return url.toString();
}

}
27 changes: 15 additions & 12 deletions apps/api/src/isms/documents/generate.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,10 @@ function buildTx() {
ismsInterestedParty: registerTable(),
ismsInterestedPartyRequirement: registerTable(),
ismsObjective: registerTable(),
ismsDocument: { findFirst: jest.fn().mockResolvedValue(null) },
ismsDocumentVersion: {
ismsDocument: {
findFirst: jest.fn().mockResolvedValue(null),
create: jest.fn().mockResolvedValue({}),
// CS-701: the draft narrative lives on IsmsDocument, not a version row.
findUnique: jest.fn().mockResolvedValue(null),
update: jest.fn().mockResolvedValue({}),
},
};
Expand Down Expand Up @@ -132,24 +132,27 @@ describe('runDerivation', () => {
expect(tx.ismsObjective.createMany).toHaveBeenCalled();
});

it('creates a version with the derived narrative for isms_scope', async () => {
it('writes the derived narrative onto an empty document draft (isms_scope)', async () => {
const tx = buildTx();
await runDerivation({ tx: asTx(tx), type: 'isms_scope', ...baseArgs });
expect(tx.ismsDocumentVersion.create).toHaveBeenCalled();
const created = tx.ismsDocumentVersion.create.mock.calls[0][0].data;
expect(created.narrative.certificateScopeSentence).toBeDefined();
expect(tx.ismsDocument.update).toHaveBeenCalledWith(
expect.objectContaining({ where: { id: 'doc_1' } }),
);
const updated = tx.ismsDocument.update.mock.calls[0][0].data;
expect(updated.draftNarrative.certificateScopeSentence).toBeDefined();
});

it('updates the existing version narrative for leadership', async () => {
it('preserves a non-empty existing draft narrative (CS-437 override)', async () => {
const tx = buildTx();
tx.ismsDocumentVersion.findFirst.mockResolvedValue({ id: 'ver_1' });
tx.ismsDocument.findUnique.mockResolvedValue({
draftNarrative: { certificateScopeSentence: 'Manual edit' },
});
await runDerivation({
tx: asTx(tx),
type: 'leadership_commitment',
...baseArgs,
});
expect(tx.ismsDocumentVersion.update).toHaveBeenCalledWith(
expect.objectContaining({ where: { id: 'ver_1' } }),
);
// A regenerate must never clobber the customer's manual narrative.
expect(tx.ismsDocument.update).not.toHaveBeenCalled();
});
});
25 changes: 10 additions & 15 deletions apps/api/src/isms/documents/generate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,22 +174,17 @@ async function generateNarrative({
if (!derived) return;
const narrative: Prisma.InputJsonValue = JSON.parse(JSON.stringify(derived));

const latest = await tx.ismsDocumentVersion.findFirst({
where: { documentId, isLatest: true },
// The draft narrative lives on IsmsDocument (CS-701). Preserve a non-empty
// existing draft so a regenerate never clobbers the customer's manual edits
// (CS-437 override); seed an absent/empty draft with the derived narrative.
const document = await tx.ismsDocument.findUnique({
where: { id: documentId },
select: { draftNarrative: true },
});
if (latest) {
// Preserve a non-empty narrative so a regenerate never clobbers the
// customer's manual edits (CS-437 override). An absent or empty ({}) value —
// e.g. a snapshot-only version — is still seeded with the derived narrative.
if (hasNarrativeContent(latest.narrative)) return;
await tx.ismsDocumentVersion.update({
where: { id: latest.id },
data: { narrative },
});
return;
}
await tx.ismsDocumentVersion.create({
data: { documentId, version: 1, isLatest: true, narrative },
if (document && hasNarrativeContent(document.draftNarrative)) return;
await tx.ismsDocument.update({
where: { id: documentId },
data: { draftNarrative: narrative },
});
}

Expand Down
Loading
Loading