From b4c917ac0792990b401c40bf7efde350884b95c6 Mon Sep 17 00:00:00 2001 From: Philipinho <16838612+Philipinho@users.noreply.github.com> Date: Sat, 23 May 2026 12:38:43 +0100 Subject: [PATCH] feat(integrations): defer workspace-scoped install until OAuth succeeds --- .../integration/pages/integrations.tsx | 34 +++++-- .../services/integration-service.ts | 15 +++ .../core/integration/dto/integration.dto.ts | 6 ++ .../integration/oauth/oauth.controller.ts | 29 +++++- .../core/integration/oauth/oauth.service.ts | 97 ++++++++++++++++++- 5 files changed, 166 insertions(+), 15 deletions(-) diff --git a/apps/client/src/features/integration/pages/integrations.tsx b/apps/client/src/features/integration/pages/integrations.tsx index 4141d5f0a..508ed6d6f 100644 --- a/apps/client/src/features/integration/pages/integrations.tsx +++ b/apps/client/src/features/integration/pages/integrations.tsx @@ -14,7 +14,11 @@ import { useUpdateIntegrationSettings, } from "../queries/integration-query"; import { Integration } from "../types/integration.types"; -import { getOAuthAuthorizeUrl } from "../services/integration-service"; +import { + getOAuthAuthorizeUrl, + getOAuthInstallUrl, +} from "../services/integration-service"; +import { notifications } from "@mantine/notifications"; export default function Integrations() { const { t } = useTranslation(); @@ -31,19 +35,29 @@ export default function Integrations() { const handleInstall = useCallback( async (type: string) => { const definition = available?.find((d) => d.type === type); - try { - const integration = await installMutation.mutateAsync({ type }); - if (definition?.oauth?.connectionScope === 'workspace') { - const { authorizationUrl } = await getOAuthAuthorizeUrl({ - integrationId: integration.id, - }); + + // Workspace-scoped (Slack): the install row is only persisted when the + // OAuth callback succeeds. Skip the upfront install API call entirely. + if (definition?.oauth?.connectionScope === "workspace") { + try { + const { authorizationUrl } = await getOAuthInstallUrl({ type }); window.location.href = authorizationUrl; + } catch (err: any) { + notifications.show({ + message: + err?.response?.data?.message ?? t("Failed to start installation"), + color: "red", + }); } - } catch (err) { - // installMutation's onError already shows a notification + return; } + + // Per-user OAuth providers (Linear, Jira, GitHub, ...): keep existing + // two-step flow — create the integration row, then individual users + // OAuth-connect from /settings/account/connections. + installMutation.mutate({ type }); }, - [installMutation, available], + [installMutation, available, t], ); const handleUninstall = useCallback( diff --git a/apps/client/src/features/integration/services/integration-service.ts b/apps/client/src/features/integration/services/integration-service.ts index 3d2d54989..44e501fe1 100644 --- a/apps/client/src/features/integration/services/integration-service.ts +++ b/apps/client/src/features/integration/services/integration-service.ts @@ -68,6 +68,21 @@ export async function getOAuthAuthorizeUrl(data: { return req.data; } +/** + * For workspace-scoped providers: returns the authorize URL WITHOUT creating + * the integration row. The row is created atomically when the OAuth callback + * succeeds; a cancelled OAuth leaves no half-installed state. + */ +export async function getOAuthInstallUrl(data: { + type: string; +}): Promise<{ authorizationUrl: string }> { + const req = await api.post<{ authorizationUrl: string }>( + "/integrations/oauth/install", + data, + ); + return req.data; +} + export async function disconnectIntegration(data: { integrationId: string; }): Promise { diff --git a/apps/server/src/core/integration/dto/integration.dto.ts b/apps/server/src/core/integration/dto/integration.dto.ts index f5cb3ca88..f7edef072 100644 --- a/apps/server/src/core/integration/dto/integration.dto.ts +++ b/apps/server/src/core/integration/dto/integration.dto.ts @@ -49,3 +49,9 @@ export class OAuthDisconnectDto { @IsString() integrationId: string; } + +export class OAuthInstallDto { + @IsNotEmpty() + @IsString() + type: string; +} diff --git a/apps/server/src/core/integration/oauth/oauth.controller.ts b/apps/server/src/core/integration/oauth/oauth.controller.ts index 30ba86a07..5628c3dad 100644 --- a/apps/server/src/core/integration/oauth/oauth.controller.ts +++ b/apps/server/src/core/integration/oauth/oauth.controller.ts @@ -18,7 +18,11 @@ import { AuthUser } from '../../../common/decorators/auth-user.decorator'; import { AuthWorkspace } from '../../../common/decorators/auth-workspace.decorator'; import { User, Workspace } from '@docmost/db/types/entity.types'; import { OAuthService } from './oauth.service'; -import { OAuthAuthorizeDto, OAuthDisconnectDto } from '../dto/integration.dto'; +import { + OAuthAuthorizeDto, + OAuthDisconnectDto, + OAuthInstallDto, +} from '../dto/integration.dto'; import { IntegrationConnectionService } from '../integration-connection.service'; import { EnvironmentService } from '../../../integrations/environment/environment.service'; @@ -49,6 +53,29 @@ export class OAuthController { return { authorizationUrl }; } + /** + * Install-and-authorize for workspace-scoped providers (Slack model). + * Returns the authorize URL without first creating the integration row; + * the row is created atomically on successful OAuth callback so a cancelled + * OAuth flow leaves no half-installed state. + */ + @UseGuards(JwtAuthGuard) + @HttpCode(HttpStatus.OK) + @Post('install') + async installAndAuthorize( + @Body() dto: OAuthInstallDto, + @AuthUser() user: User, + @AuthWorkspace() workspace: Workspace, + ) { + const { authorizationUrl } = await this.oauthService.getInstallAuthorizationUrl( + dto.type, + workspace.id, + user.id, + ); + + return { authorizationUrl }; + } + @Get(':type/callback') async callback( @Param('type') type: string, diff --git a/apps/server/src/core/integration/oauth/oauth.service.ts b/apps/server/src/core/integration/oauth/oauth.service.ts index c8795479c..9892e0339 100644 --- a/apps/server/src/core/integration/oauth/oauth.service.ts +++ b/apps/server/src/core/integration/oauth/oauth.service.ts @@ -24,7 +24,12 @@ type OAuthTokenResponse = { }; export type OAuthStatePayload = { - integrationId: string; + // For "authorize-only" flows (per-user OAuth on an already-installed + // integration) integrationId is set; for "install-and-authorize" flows + // (workspace-scoped providers like Slack) it's null until the callback + // resolves-or-creates the row atomically with token exchange success. + integrationId: string | null; + type: string; userId: string; workspaceId: string; // Workspace's canonical URL at authorize time. Cloud workspaces are routed @@ -76,6 +81,7 @@ export class OAuthService { const state = this.createSignedState({ integrationId, + type: integration.type, userId, workspaceId, returnUrl, @@ -98,6 +104,75 @@ export class OAuthService { }; } + /** + * Install-and-authorize for workspace-scoped providers (Slack model). + * + * Skips creating the integration row up front. The callback (atomic with + * token exchange success) is what actually persists the integration; if the + * user cancels at Slack's consent screen, nothing is written. Refusing the + * already-installed case here keeps the install button idempotent. + */ + async getInstallAuthorizationUrl( + type: string, + workspaceId: string, + userId: string, + ): Promise<{ authorizationUrl: string }> { + const provider = this.registry.getProvider(type); + if (!provider || !provider.definition.oauth) { + throw new BadRequestException('Integration does not support OAuth'); + } + if (provider.definition.oauth.connectionScope !== 'workspace') { + throw new BadRequestException( + 'This integration uses per-user OAuth; use the standard install + authorize flow', + ); + } + + const existing = await this.integrationRepo.findByWorkspaceAndType( + workspaceId, + type, + ); + if (existing) { + throw new BadRequestException( + `Integration "${type}" is already installed`, + ); + } + + const oauthConfig = provider.getOAuthConfig + ? provider.getOAuthConfig({}) + : provider.definition.oauth; + + const callbackUrl = this.buildCallbackUrl(type); + + const workspace = await this.workspaceRepo.findById(workspaceId); + const returnUrl = this.domainService.getWorkspaceUrl( + workspace ?? { hostname: null, customDomain: null }, + ); + + const state = this.createSignedState({ + integrationId: null, + type, + userId, + workspaceId, + returnUrl, + exp: Date.now() + 10 * 60 * 1000, + }); + + const params = new URLSearchParams({ + client_id: this.getClientId(type), + redirect_uri: callbackUrl, + response_type: 'code', + state, + }); + + const scope = oauthConfig.scopes + .map((s) => encodeURIComponent(s)) + .join('%20'); + + return { + authorizationUrl: `${oauthConfig.authUrl}?${params.toString()}&scope=${scope}`, + }; + } + verifySignedState(state: string): OAuthStatePayload | null { const dotIndex = state.lastIndexOf('.'); if (dotIndex === -1) return null; @@ -129,7 +204,7 @@ export class OAuthService { async exchangeCodeForTokens( type: string, code: string, - integrationId: string, + integrationId: string | null, userId: string, workspaceId: string, ): Promise { @@ -138,8 +213,22 @@ export class OAuthService { throw new BadRequestException('Integration does not support OAuth'); } - const integration = await this.integrationRepo.findById(integrationId); - const settings = (integration?.settings as Record) ?? {}; + // Install-and-authorize flow (workspace-scoped providers): no integration + // row exists yet. Create or restore it now that OAuth has succeeded. + let integration = integrationId + ? await this.integrationRepo.findById(integrationId) + : null; + + if (!integration) { + integration = await this.integrationRepo.insertOrRestore({ + type, + workspaceId, + installedById: userId, + }); + integrationId = integration.id; + } + + const settings = (integration.settings as Record) ?? {}; const oauthConfig = provider.getOAuthConfig ? provider.getOAuthConfig(settings)