Skip to content

Commit 0885e23

Browse files
authored
fix(slack): prevent duplicate workspace installs (#2989)
* fix(slack): prevent duplicate workspace installs * fix(slack): show suspended integration state * fix(slack): suspend detached duplicate installs * fix(slack): allow disconnecting suspended installs * fix(slack): preserve existing duplicate lookup order
1 parent f626eb5 commit 0885e23

9 files changed

Lines changed: 18389 additions & 38 deletions

File tree

apps/web/src/app/api/integrations/slack/callback/route.ts

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,10 @@ import { getUserFromAuth } from '@/lib/user.server';
44
import { ensureOrganizationAccess } from '@/routers/organizations/utils';
55
import type { Owner } from '@/lib/integrations/core/types';
66
import { captureException, captureMessage } from '@sentry/nextjs';
7-
import { upsertSlackInstallation } from '@/lib/integrations/slack-service';
7+
import {
8+
SlackWorkspaceAlreadyConnectedError,
9+
upsertSlackInstallation,
10+
} from '@/lib/integrations/slack-service';
811
import { verifyOAuthState } from '@/lib/integrations/oauth-state';
912
import { APP_URL } from '@/lib/constants';
1013
import { bot } from '@/lib/bot';
@@ -128,7 +131,17 @@ export async function GET(request: NextRequest) {
128131
const { teamId, installation } = await slackAdapter.handleOAuthCallback(patchedRequest);
129132

130133
// 8. Store installation in database
131-
await upsertSlackInstallation({ owner, teamId, installation });
134+
try {
135+
await upsertSlackInstallation({ owner, teamId, installation });
136+
} catch (error) {
137+
if (error instanceof SlackWorkspaceAlreadyConnectedError) {
138+
return NextResponse.redirect(
139+
new URL(buildSlackRedirectPath(state, 'error=workspace_already_connected'), APP_URL)
140+
);
141+
}
142+
143+
throw error;
144+
}
132145

133146
// 9. Redirect to success page
134147
const successPath =

apps/web/src/components/integrations/SlackIntegrationDetails.tsx

Lines changed: 71 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,25 @@ type SlackIntegrationDetailsProps = {
2626
error?: string;
2727
};
2828

29+
const slackConnectionErrorMessages: Record<string, string> = {
30+
workspace_already_connected:
31+
'This Slack workspace is already connected to another Kilo account or organization. Disconnect it there before connecting it here.',
32+
};
33+
34+
const duplicateSlackWorkspaceMigration = 'duplicate_slack_workspace_migration';
35+
36+
function getSlackConnectionErrorMessage(error: string): string {
37+
return slackConnectionErrorMessages[error] ?? `Connection failed: ${error}`;
38+
}
39+
40+
function getSuspendedSlackIntegrationMessage(suspendedBy: string | null): string {
41+
if (suspendedBy === duplicateSlackWorkspaceMigration) {
42+
return 'This Slack integration was suspended because the workspace was also connected to another Kilo account or organization. It will not receive Slack messages. Disconnect the other Kilo connection for this Slack workspace, then re-install Slack here.';
43+
}
44+
45+
return 'This Slack integration is suspended and will not receive Slack messages. Re-install Slack to restore the connection.';
46+
}
47+
2948
export function SlackIntegrationDetails({
3049
organizationId,
3150
success,
@@ -127,7 +146,7 @@ export function SlackIntegrationDetails({
127146
toast.success('Slack connected successfully!');
128147
}
129148
if (error) {
130-
toast.error(`Connection failed: ${error}`);
149+
toast.error(getSlackConnectionErrorMessage(error));
131150
}
132151
}, [success, error]);
133152

@@ -239,10 +258,19 @@ export function SlackIntegrationDetails({
239258

240259
const isInstalled = installationData?.installed;
241260
const installation = installationData?.installation;
261+
const isSuspended = installation?.status === 'suspended';
242262
const missingScopes = installation?.missingScopes ?? [];
243263

244264
return (
245265
<div className="space-y-6">
266+
{error && (
267+
<Alert variant="destructive">
268+
<XCircle className="h-4 w-4" />
269+
<AlertTitle>Could not connect Slack</AlertTitle>
270+
<AlertDescription>{getSlackConnectionErrorMessage(error)}</AlertDescription>
271+
</Alert>
272+
)}
273+
246274
{isInstalled && missingScopes.length > 0 && (
247275
<Alert variant="warning">
248276
<AlertTriangle className="h-4 w-4" />
@@ -264,6 +292,24 @@ export function SlackIntegrationDetails({
264292
</Alert>
265293
)}
266294

295+
{installation && isSuspended && (
296+
<Alert variant="warning">
297+
<AlertTriangle className="h-4 w-4" />
298+
<AlertTitle>Slack integration suspended</AlertTitle>
299+
<AlertDescription className="gap-3">
300+
<p>{getSuspendedSlackIntegrationMessage(installation.suspendedBy)}</p>
301+
<Button
302+
onClick={handleInstall}
303+
disabled={isStartingSlackConnection || isFetchingOAuthUrl}
304+
className="bg-yellow-400 text-yellow-950 hover:bg-yellow-300"
305+
>
306+
<RefreshCw className="mr-2 h-4 w-4" />
307+
{isStartingSlackConnection || isFetchingOAuthUrl ? 'Loading...' : 'Re-install Slack'}
308+
</Button>
309+
</AlertDescription>
310+
</Alert>
311+
)}
312+
267313
{/* Installation Status Card */}
268314
<Card>
269315
<CardHeader>
@@ -277,7 +323,12 @@ export function SlackIntegrationDetails({
277323
Create PRs, debug code, ask questions about your repos, etc. directly from Slack
278324
</CardDescription>
279325
</div>
280-
{isInstalled ? (
326+
{isSuspended ? (
327+
<Badge variant="destructive" className="flex items-center gap-1">
328+
<AlertTriangle className="h-3 w-3" />
329+
Suspended
330+
</Badge>
331+
) : isInstalled ? (
281332
<Badge variant="default" className="flex items-center gap-1">
282333
<CheckCircle2 className="h-3 w-3" />
283334
Connected
@@ -291,7 +342,7 @@ export function SlackIntegrationDetails({
291342
</div>
292343
</CardHeader>
293344
<CardContent className="space-y-4">
294-
{isInstalled && installation ? (
345+
{installation ? (
295346
<>
296347
{/* Installation Details */}
297348
<div className="space-y-3 rounded-lg border p-4">
@@ -319,6 +370,16 @@ export function SlackIntegrationDetails({
319370
: 'Unknown'}
320371
</span>
321372
</div>
373+
{isSuspended && (
374+
<div className="flex items-center justify-between">
375+
<span className="text-sm font-medium">Suspended:</span>
376+
<span className="text-sm">
377+
{installation.suspendedAt
378+
? new Date(installation.suspendedAt).toLocaleDateString()
379+
: 'Unknown'}
380+
</span>
381+
</div>
382+
)}
322383
</div>
323384

324385
{/* Model Selection */}
@@ -345,11 +406,13 @@ export function SlackIntegrationDetails({
345406
<RefreshCw className="mr-2 h-4 w-4" />
346407
{isStartingSlackConnection || isFetchingOAuthUrl ? 'Loading...' : 'Re-install'}
347408
</Button>
348-
<TestConnectionButton
349-
isPending={testConnection.isPending}
350-
state={connectionCheck}
351-
onClick={handleTestConnection}
352-
/>
409+
{!isSuspended && (
410+
<TestConnectionButton
411+
isPending={testConnection.isPending}
412+
state={connectionCheck}
413+
onClick={handleTestConnection}
414+
/>
415+
)}
353416
<Button
354417
variant="destructive"
355418
onClick={handleUninstall}

apps/web/src/lib/integrations/slack-service.test.ts

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ const mockUpdateSet = jest.fn();
66
const mockUpdateWhere = jest.fn();
77
const mockUpdateReturning = jest.fn();
88
const mockDeleteWhere = jest.fn();
9+
const mockInsertValues = jest.fn();
10+
const mockInsertReturning = jest.fn();
911
const mockAuthRevoke = jest.fn();
1012
const mockAuthTest = jest.fn();
1113

@@ -24,6 +26,9 @@ jest.mock('@/lib/drizzle', () => ({
2426
update: jest.fn(() => ({
2527
set: mockUpdateSet,
2628
})),
29+
insert: jest.fn(() => ({
30+
values: mockInsertValues,
31+
})),
2732
},
2833
}));
2934

@@ -41,6 +46,7 @@ import type { SlackInstallation } from '@chat-adapter/slack';
4146
import {
4247
deleteInstallationByTeamId,
4348
getMissingSlackScopes,
49+
SlackWorkspaceAlreadyConnectedError,
4450
SLACK_SCOPES,
4551
testConnection,
4652
uninstallApp,
@@ -56,6 +62,8 @@ function buildSlackIntegration(overrides: Record<string, unknown> = {}) {
5662
metadata: { access_token: 'xoxb-token' },
5763
platform_installation_id: 'T123',
5864
platform_account_id: 'T123',
65+
owned_by_user_id: owner.id,
66+
owned_by_organization_id: null,
5967
...overrides,
6068
};
6169
}
@@ -136,6 +144,27 @@ describe('slack-service uninstallApp', () => {
136144
expect(deleteChatSdkInstallation).toHaveBeenCalledWith('T456');
137145
expect(mockDeleteWhere).toHaveBeenCalledTimes(1);
138146
});
147+
148+
it('disconnects suspended integrations without deleting shared Slack installation state', async () => {
149+
mockLimit.mockResolvedValue([
150+
buildSlackIntegration({
151+
integration_status: 'suspended',
152+
platform_installation_id: null,
153+
platform_account_id: 'T456',
154+
}),
155+
]);
156+
const deleteChatSdkInstallation = jest.fn(async (_teamId: string): Promise<void> => {});
157+
const deleteChatSdkIdentityCache = jest.fn(async (_teamId: string): Promise<void> => {});
158+
159+
await expect(
160+
uninstallApp(owner, { deleteChatSdkInstallation, deleteChatSdkIdentityCache })
161+
).resolves.toEqual({ success: true });
162+
163+
expect(mockAuthRevoke).not.toHaveBeenCalled();
164+
expect(deleteChatSdkInstallation).not.toHaveBeenCalled();
165+
expect(deleteChatSdkIdentityCache).not.toHaveBeenCalled();
166+
expect(mockDeleteWhere).toHaveBeenCalledTimes(1);
167+
});
139168
});
140169

141170
describe('slack-service deleteInstallationByTeamId', () => {
@@ -233,6 +262,10 @@ describe('upsertSlackInstallation', () => {
233262
mockUpdateSet.mockReturnValue({ where: mockUpdateWhere });
234263
mockUpdateWhere.mockReturnValue({ returning: mockUpdateReturning });
235264
mockUpdateReturning.mockResolvedValue([buildSlackIntegration()]);
265+
mockInsertValues.mockReset();
266+
mockInsertReturning.mockReset();
267+
mockInsertValues.mockReturnValue({ returning: mockInsertReturning });
268+
mockInsertReturning.mockResolvedValue([buildSlackIntegration()]);
236269
});
237270

238271
it('preserves the selected model when refreshing an existing installation', async () => {
@@ -263,7 +296,44 @@ describe('upsertSlackInstallation', () => {
263296
incoming_webhook: { channel: '#general', channelId: 'C123', url: 'https://example.com' },
264297
model_slug: 'anthropic/claude-sonnet-4.5',
265298
}),
299+
platform_installation_id: 'T123',
266300
})
267301
);
268302
});
303+
304+
it('rejects installing a Slack workspace connected to another owner', async () => {
305+
mockLimit
306+
.mockResolvedValueOnce([])
307+
.mockResolvedValueOnce([buildSlackIntegration({ owned_by_user_id: 'user-2' })]);
308+
309+
const installation = {
310+
botToken: 'xoxb-new-token',
311+
botUserId: 'U_NEW_BOT',
312+
teamName: 'Kilo Team',
313+
} satisfies SlackInstallation;
314+
315+
await expect(upsertSlackInstallation({ owner, teamId: 'T123', installation })).rejects.toThrow(
316+
SlackWorkspaceAlreadyConnectedError
317+
);
318+
319+
expect(mockInsertValues).not.toHaveBeenCalled();
320+
expect(mockUpdateSet).not.toHaveBeenCalled();
321+
});
322+
323+
it('maps Slack workspace unique violations to a helpful install error', async () => {
324+
mockLimit.mockResolvedValueOnce([]).mockResolvedValueOnce([]);
325+
mockInsertReturning.mockRejectedValue({
326+
constraint: 'UQ_platform_integrations_slack_platform_inst',
327+
});
328+
329+
const installation = {
330+
botToken: 'xoxb-new-token',
331+
botUserId: 'U_NEW_BOT',
332+
teamName: 'Kilo Team',
333+
} satisfies SlackInstallation;
334+
335+
await expect(upsertSlackInstallation({ owner, teamId: 'T123', installation })).rejects.toThrow(
336+
'Kilo Team is already connected to another Kilo account or organization'
337+
);
338+
});
269339
});

0 commit comments

Comments
 (0)