From 10737905f6444799878f0c28d5d1fc197ed90ccd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 02:21:09 +0000 Subject: [PATCH 1/4] fix(slack): require organization management for organization Slack changes - The organization Slack OAuth connect entry point and callback require an owner or admin, matching the authority other organization-scoped integration changes use. - Organization Slack disconnect and model changes require the same authority. Members keep a read-only card that still runs the connection test. - upsertSlackInstallation takes an explicit permission argument so the bound workspace only changes from a flow that verified organization management authority. - Access denials from the shared OAuth connect handler redirect with a specific error code rather than a generic initialization failure. Tests cover a member, an owner/admin, a repeated install for the same workspace, a personal connection, and the service-level permission check. --- .../integrations/slack/reinstall/route.ts | 7 +- .../integrations/IntegrationDetailPage.tsx | 11 +- .../integrations/SlackIntegrationDetails.tsx | 109 ++++++++----- apps/web/src/lib/integrations/oauth/common.ts | 27 ++-- .../oauth/platforms/slack-callback.test.ts | 152 ++++++++++++++++++ .../oauth/platforms/slack-callback.ts | 29 +++- apps/web/src/lib/integrations/oauth/routes.ts | 6 +- .../lib/integrations/slack-service.test.ts | 94 +++++++++++ .../web/src/lib/integrations/slack-service.ts | 25 +++ apps/web/src/routers/slack-router.ts | 13 +- 10 files changed, 410 insertions(+), 63 deletions(-) create mode 100644 apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts diff --git a/apps/web/src/app/(app)/organizations/[id]/integrations/slack/reinstall/route.ts b/apps/web/src/app/(app)/organizations/[id]/integrations/slack/reinstall/route.ts index 2a206104fd2..db9780bbcee 100644 --- a/apps/web/src/app/(app)/organizations/[id]/integrations/slack/reinstall/route.ts +++ b/apps/web/src/app/(app)/organizations/[id]/integrations/slack/reinstall/route.ts @@ -2,6 +2,7 @@ import type { NextRequest } from 'next/server'; import { NextResponse } from 'next/server'; import { getUserFromAuth } from '@kilocode/web-shared/lib/user/server'; import { ensureOrganizationAccess } from '@kilocode/web-shared/routers/organizations/utils'; +import { ORGANIZATION_MANAGE_ROLES } from '@kilocode/app-shared/organizations'; import { PLATFORM } from '@/lib/integrations/core/constants'; import { getPlatformOAuthConnectPath } from '@/lib/integrations/oauth/paths'; @@ -19,9 +20,11 @@ export async function GET(request: NextRequest, context: RouteContext) { } try { - await ensureOrganizationAccess({ user }, id); + await ensureOrganizationAccess({ user }, id, ORGANIZATION_MANAGE_ROLES); } catch { - return NextResponse.redirect(new URL('/integrations?error=unauthorized', request.url)); + return NextResponse.redirect( + new URL(`/organizations/${id}/integrations/slack?error=permission_required`, request.url) + ); } return NextResponse.redirect( diff --git a/apps/web/src/components/integrations/IntegrationDetailPage.tsx b/apps/web/src/components/integrations/IntegrationDetailPage.tsx index df17c3ccf4b..0c18df0a37a 100644 --- a/apps/web/src/components/integrations/IntegrationDetailPage.tsx +++ b/apps/web/src/components/integrations/IntegrationDetailPage.tsx @@ -5,6 +5,7 @@ import Link from 'next/link'; import { notFound } from 'next/navigation'; import { ArrowLeft } from 'lucide-react'; import { getUserFromAuthOrRedirect } from '@kilocode/web-shared/lib/user/server'; +import type { OrganizationRole } from '@kilocode/web-shared/lib/organizations/organization-types'; import { PLATFORM } from '@/lib/integrations/core/constants'; import { Button } from '@/components/ui/button'; import { Card, CardContent } from '@/components/ui/card'; @@ -23,6 +24,7 @@ export type IntegrationDetailSearchParams = Promise<{ type DetailRenderProps = { organizationId?: string; + role?: OrganizationRole; search: Awaited; }; @@ -90,7 +92,7 @@ const integrationDetailRegistry = { title: 'Slack Integration', userSubtitle: 'Connect your Slack workspace to receive notifications', organizationSubtitle: organizationName => `Manage Slack integration for ${organizationName}`, - render: async ({ organizationId, search }) => { + render: async ({ organizationId, search, role }) => { const { SlackIntegrationDetails } = await import('@/components/integrations/SlackIntegrationDetails'); return ( @@ -98,6 +100,7 @@ const integrationDetailRegistry = { organizationId={organizationId} success={search.success === 'installed'} error={search.error} + canManage={!role || role === 'owner' || role === 'admin'} /> ); }, @@ -203,9 +206,10 @@ function IntegrationDetailsFallback() { async function PlatformIntegrationDetails({ platform, organizationId, + role, search, }: DetailRenderProps & { platform: IntegrationDetailPlatform }) { - return getIntegrationDetailEntry(platform).render({ organizationId, search }); + return getIntegrationDetailEntry(platform).render({ organizationId, role, search }); } function SuspendedIntegrationDetails( @@ -268,7 +272,7 @@ export async function OrganizationIntegrationDetailPage({ return ( ( + render={({ role, organization }) => ( <>
diff --git a/apps/web/src/components/integrations/SlackIntegrationDetails.tsx b/apps/web/src/components/integrations/SlackIntegrationDetails.tsx index 3d9a3a17712..899d992301f 100644 --- a/apps/web/src/components/integrations/SlackIntegrationDetails.tsx +++ b/apps/web/src/components/integrations/SlackIntegrationDetails.tsx @@ -27,11 +27,17 @@ type SlackIntegrationDetailsProps = { organizationId?: string; success?: boolean; error?: string; + /** Organization members without management authority get a read-only view. */ + canManage?: boolean; }; const slackConnectionErrorMessages: Record = { workspace_already_connected: 'This Slack workspace is already connected to another Kilo account or organization. Disconnect it there before connecting it here.', + permission_required: + 'Only organization owners and admins can connect or change the organization Slack integration.', + organization_access_required: + 'You no longer have access to this organization. Ask an organization owner or admin for access.', }; const duplicateSlackWorkspaceMigration = 'duplicate_slack_workspace_migration'; @@ -52,6 +58,7 @@ export function SlackIntegrationDetails({ organizationId, success, error, + canManage = true, }: SlackIntegrationDetailsProps) { const trpc = useTRPC(); const queryClient = useQueryClient(); @@ -289,14 +296,16 @@ export function SlackIntegrationDetails({ This Slack installation is missing required permissions. Re-install the Slack app to refresh its scopes.

- + {canManage && ( + + )} )} @@ -307,14 +316,25 @@ export function SlackIntegrationDetails({ Slack integration suspended

{getSuspendedSlackIntegrationMessage(installation.suspendedBy)}

- + {canManage && ( + + )} + + + )} + + {!canManage && ( + + + Only organization owners and admins can connect, re-install, or disconnect this Slack + integration. You can still test the connection. )} @@ -401,20 +421,23 @@ export function SlackIntegrationDetails({ onValueChange={handleModelChange} isLoading={isLoadingModels} placeholder="Select a model" + disabled={!canManage} />
- + {canManage && ( + + )} {!isSuspended && ( )} - + {canManage && ( + + )} {IS_DEVELOPMENT && (
- + {canManage && ( + + )} )} diff --git a/apps/web/src/lib/integrations/oauth/common.ts b/apps/web/src/lib/integrations/oauth/common.ts index a48ef7cef4b..294576db643 100644 --- a/apps/web/src/lib/integrations/oauth/common.ts +++ b/apps/web/src/lib/integrations/oauth/common.ts @@ -234,19 +234,26 @@ export async function handleStatefulPlatformOAuthConnect( return NextResponse.redirect(buildOAuthUrl(state)); } catch (error) { - captureException(error, { - tags: { - endpoint: `${platform}/connect`, - source, - }, - extra: { - organizationId, - }, - }); + const denialCode = organizationAccessDenialErrorCode(error); + if (!denialCode) { + captureException(error, { + tags: { + endpoint: `${platform}/connect`, + source, + }, + extra: { + organizationId, + }, + }); + } return NextResponse.redirect( new URL( - buildIntegrationOAuthConnectErrorPath(platform, organizationId, 'oauth_init_failed'), + buildIntegrationOAuthConnectErrorPath( + platform, + organizationId, + denialCode ?? 'oauth_init_failed' + ), APP_URL ) ); diff --git a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts new file mode 100644 index 00000000000..d47d385359b --- /dev/null +++ b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts @@ -0,0 +1,152 @@ +process.env.NEXTAUTH_SECRET ||= 'test-nextauth-secret'; + +const mockEnsureOrganizationAccess = jest.fn(); +const mockConsumeProviderOAuthAttempt = jest.fn(); +const mockUpsertSlackInstallation = jest.fn(); +const mockHandleOAuthCallback = jest.fn(); +const mockCaptureException = jest.fn(); +const mockCaptureMessage = jest.fn(); + +jest.mock('@kilocode/web-shared/lib/config.server', () => ({ + NEXTAUTH_SECRET: 'test-nextauth-secret', +})); +jest.mock('@kilocode/web-shared/lib/user/server'); +jest.mock('@kilocode/web-shared/routers/organizations/utils', () => ({ + ensureOrganizationAccess: (...args: unknown[]) => mockEnsureOrganizationAccess(...args), +})); +jest.mock('@sentry/nextjs', () => ({ + captureException: (...args: unknown[]) => mockCaptureException(...args), + captureMessage: (...args: unknown[]) => mockCaptureMessage(...args), +})); +jest.mock('@/lib/bot', () => ({ + bot: { + initialize: jest.fn(async () => {}), + getAdapter: () => ({ + handleOAuthCallback: (...args: unknown[]) => mockHandleOAuthCallback(...args), + }), + }, +})); +jest.mock('@/lib/integrations/provider-oauth-attempts', () => ({ + beginProviderOAuthAttempt: jest.fn(), + cancelProviderOAuthAttempt: jest.fn(), + consumeProviderOAuthAttempt: (...args: unknown[]) => mockConsumeProviderOAuthAttempt(...args), +})); +jest.mock('@/lib/integrations/slack-service', () => { + class SlackWorkspaceAlreadyConnectedError extends Error { + constructor(teamName: string) { + super(`${teamName} is already connected to another Kilo account or organization.`); + this.name = 'SlackWorkspaceAlreadyConnectedError'; + } + } + return { + SlackWorkspaceAlreadyConnectedError, + upsertSlackInstallation: (...args: unknown[]) => mockUpsertSlackInstallation(...args), + }; +}); + +import { TRPCError } from '@trpc/server'; +import { NextRequest } from 'next/server'; +import { getUserFromAuth } from '@kilocode/web-shared/lib/user/server'; +import { ORGANIZATION_MANAGE_ROLES } from '@kilocode/app-shared/organizations'; +import { createOAuthState } from '@/lib/integrations/oauth-state'; +import { handleSlackOAuthCallback } from './slack-callback'; + +const mockedGetUserFromAuth = jest.mocked(getUserFromAuth); + +const USER_ID = '034489e8-19e0-4479-9d69-2edad719e847'; +const ORGANIZATION_ID = '7e3011af-e99d-444f-8171-54c2225b87dc'; +const installation = { botToken: 'xoxb-token', botUserId: 'U_BOT', teamName: 'Kilo Team' }; + +const permissionDeniedError = new TRPCError({ + code: 'UNAUTHORIZED', + message: 'You do not have the required organizational role to access this feature', +}); + +async function callCallback( + owner: string, + statePurpose: 'provider_install' | undefined = 'provider_install' +) { + const state = createOAuthState(owner, USER_ID, undefined, statePurpose); + const request = new NextRequest( + `http://localhost:3000/api/integrations/slack/callback?code=code-1&state=${encodeURIComponent(state)}` + ); + const response = await handleSlackOAuthCallback(request); + return { response, state }; +} + +describe('handleSlackOAuthCallback organization authority', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockedGetUserFromAuth.mockResolvedValue({ + user: { id: USER_ID, is_admin: false }, + authFailedResponse: null, + } as never); + mockEnsureOrganizationAccess.mockResolvedValue('owner'); + mockConsumeProviderOAuthAttempt.mockResolvedValue(true); + mockUpsertSlackInstallation.mockResolvedValue({ id: 'integration-1' }); + mockHandleOAuthCallback.mockResolvedValue({ teamId: 'T123', installation }); + }); + + it('refuses an organization completion by a plain member', async () => { + mockEnsureOrganizationAccess.mockRejectedValue(permissionDeniedError); + + const { response } = await callCallback(`org_${ORGANIZATION_ID}`); + + expect(response.status).toBe(307); + expect(new URL(response.headers.get('location') ?? '').searchParams.get('error')).toBe( + 'permission_required' + ); + expect(mockEnsureOrganizationAccess).toHaveBeenCalledWith( + { user: expect.objectContaining({ id: USER_ID }) }, + ORGANIZATION_ID, + ORGANIZATION_MANAGE_ROLES + ); + expect(mockUpsertSlackInstallation).not.toHaveBeenCalled(); + expect(mockHandleOAuthCallback).not.toHaveBeenCalled(); + expect(mockCaptureException).not.toHaveBeenCalled(); + }); + + it('completes an organization install for an admin and authorizes a workspace change', async () => { + mockEnsureOrganizationAccess.mockResolvedValue('admin'); + + const { response } = await callCallback(`org_${ORGANIZATION_ID}`); + + expect(response.status).toBe(307); + expect(new URL(response.headers.get('location') ?? '').pathname).toBe( + '/organizations/' + ORGANIZATION_ID + '/integrations/slack' + ); + expect(mockUpsertSlackInstallation).toHaveBeenCalledWith({ + owner: { type: 'org', id: ORGANIZATION_ID }, + teamId: 'T123', + installation, + allowWorkspaceRebind: true, + }); + }); + + it('keeps a same-workspace re-install working for an organization admin', async () => { + mockEnsureOrganizationAccess.mockResolvedValue('owner'); + mockHandleOAuthCallback.mockResolvedValue({ teamId: 'T123', installation }); + + const { response } = await callCallback(`org_${ORGANIZATION_ID}`); + + expect(new URL(response.headers.get('location') ?? '').searchParams.get('success')).toBe( + 'installed' + ); + expect(mockUpsertSlackInstallation).toHaveBeenCalledWith( + expect.objectContaining({ teamId: 'T123', allowWorkspaceRebind: true }) + ); + }); + + it('does not require organization authority for a personal completion', async () => { + const { response } = await callCallback(`user_${USER_ID}`); + + expect(new URL(response.headers.get('location') ?? '').pathname).toBe('/integrations/slack'); + expect(mockEnsureOrganizationAccess).not.toHaveBeenCalled(); + expect(mockUpsertSlackInstallation).toHaveBeenCalledWith({ + owner: { type: 'user', id: USER_ID }, + teamId: 'T123', + installation, + allowWorkspaceRebind: false, + }); + }); +}); diff --git a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts index e679e699a4c..3e06bf85815 100644 --- a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts +++ b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.ts @@ -1,5 +1,6 @@ import type { NextRequest } from 'next/server'; import { NextResponse } from 'next/server'; +import { ORGANIZATION_MANAGE_ROLES } from '@kilocode/app-shared/organizations'; import { getUserFromAuth } from '@kilocode/web-shared/lib/user/server'; import { ensureOrganizationAccess } from '@kilocode/web-shared/routers/organizations/utils'; import { captureException, captureMessage } from '@sentry/nextjs'; @@ -18,6 +19,7 @@ import { buildIntegrationOAuthRedirectPathFromState, parseOAuthStateOwner, cancelMissingCodeProviderOAuthAttempt, + organizationAccessDenialErrorCode, } from '@/lib/integrations/oauth/common'; import { consumeProviderOAuthAttempt } from '@/lib/integrations/provider-oauth-attempts'; @@ -113,9 +115,11 @@ export async function handleSlackOAuthCallback(request: NextRequest) { return NextResponse.redirect(new URL('/integrations?error=invalid_state', APP_URL)); } - // 6. Verify user has access to the owner + // 6. Verify the user may change the owner's Slack connection. An organization + // connection carries a shared bot token and model policy, so it needs + // organization management authority, not just membership. if (owner.type === 'org') { - await ensureOrganizationAccess({ user }, owner.id); + await ensureOrganizationAccess({ user }, owner.id, ORGANIZATION_MANAGE_ROLES); } else { // For user-owned integrations, verify it's the same user if (user.id !== owner.id) { @@ -148,9 +152,16 @@ export async function handleSlackOAuthCallback(request: NextRequest) { const patchedRequest = new Request(url, request); const { teamId, installation } = await slackAdapter.handleOAuthCallback(patchedRequest); - // 8. Store installation in database + // 8. Store installation in database. The organization path is the only caller + // allowed to move an existing organization connection to another workspace, + // and only after the organization-management check above. try { - await upsertSlackInstallation({ owner, teamId, installation }); + await upsertSlackInstallation({ + owner, + teamId, + installation, + allowWorkspaceRebind: owner.type === 'org', + }); } catch (error) { if (error instanceof SlackWorkspaceAlreadyConnectedError) { return NextResponse.redirect( @@ -181,6 +192,16 @@ export async function handleSlackOAuthCallback(request: NextRequest) { const searchParams = request.nextUrl.searchParams; const state = searchParams.get('state'); + const denialCode = organizationAccessDenialErrorCode(error); + if (denialCode) { + return NextResponse.redirect( + new URL( + buildIntegrationOAuthRedirectPathFromState(PLATFORM.SLACK, state, `error=${denialCode}`), + APP_URL + ) + ); + } + captureException(error, { tags: { endpoint: 'slack/callback', diff --git a/apps/web/src/lib/integrations/oauth/routes.ts b/apps/web/src/lib/integrations/oauth/routes.ts index 47b987e14d1..6f3e58a36bf 100644 --- a/apps/web/src/lib/integrations/oauth/routes.ts +++ b/apps/web/src/lib/integrations/oauth/routes.ts @@ -1,7 +1,10 @@ import 'server-only'; import type { NextRequest } from 'next/server'; import { NextResponse } from 'next/server'; -import { ORGANIZATION_BILLING_ROLES } from '@kilocode/app-shared/organizations'; +import { + ORGANIZATION_BILLING_ROLES, + ORGANIZATION_MANAGE_ROLES, +} from '@kilocode/app-shared/organizations'; import { PLATFORM } from '@/lib/integrations/core/constants'; import { handleStatefulPlatformOAuthConnect, @@ -49,6 +52,7 @@ const statefulOAuthConnectRouteConfigEntries = [ source: 'slack_oauth', loadBuildOAuthUrl: async () => (await import('@/lib/integrations/slack-service')).getSlackOAuthUrl, + organizationRoles: ORGANIZATION_MANAGE_ROLES, }, ], ] as const satisfies readonly (readonly [StandardOAuthPlatform, StatefulOAuthConnectRouteConfig])[]; diff --git a/apps/web/src/lib/integrations/slack-service.test.ts b/apps/web/src/lib/integrations/slack-service.test.ts index 277d834451b..e0284df79bf 100644 --- a/apps/web/src/lib/integrations/slack-service.test.ts +++ b/apps/web/src/lib/integrations/slack-service.test.ts @@ -45,6 +45,11 @@ jest.mock('@/lib/integrations/platforms/slack/credential-store', () => ({ writeSlackCredential: (...args: unknown[]) => mockWriteSlackCredential(...args), })); +const mockGetDefaultAllowedModel = jest.fn(); +jest.mock('@/lib/slack-bot/model-allow-list', () => ({ + getDefaultAllowedModel: (...args: unknown[]) => mockGetDefaultAllowedModel(...args), +})); + const mockCaptureException = jest.fn(); jest.mock('@sentry/nextjs', () => ({ captureException: (...args: unknown[]) => mockCaptureException(...args), @@ -57,6 +62,7 @@ import { deleteInstallationByTeamId, getMissingSlackScopes, SlackWorkspaceAlreadyConnectedError, + SlackWorkspaceRebindNotAllowedError, SLACK_SCOPES, testConnection, uninstallApp, @@ -430,3 +436,91 @@ describe('upsertSlackInstallation', () => { expect(JSON.stringify(mockCaptureException.mock.calls[0])).not.toContain('xoxb-new-token'); }); }); + +describe('upsertSlackInstallation organization workspace binding', () => { + const organizationOwner = { type: 'org', id: 'org-1' } satisfies Owner; + const installation = { + botToken: 'xoxb-new-token', + botUserId: 'U_NEW_BOT', + teamName: 'Kilo Team', + } satisfies SlackInstallation; + + function organizationIntegration(platformInstallationId: string | null) { + return buildSlackIntegration({ + owned_by_user_id: null, + owned_by_organization_id: organizationOwner.id, + platform_installation_id: platformInstallationId, + }); + } + + beforeEach(() => { + mockLimit.mockReset(); + mockUpdateSet.mockReset(); + mockUpdateWhere.mockReset(); + mockUpdateReturning.mockReset(); + mockUpdateSet.mockReturnValue({ where: mockUpdateWhere }); + mockUpdateWhere.mockReturnValue({ returning: mockUpdateReturning }); + mockUpdateReturning.mockResolvedValue([buildSlackIntegration()]); + mockWriteSlackCredential.mockReset(); + mockWriteSlackCredential.mockResolvedValue({ id: 'credential-1' }); + mockGetDefaultAllowedModel.mockReset(); + mockGetDefaultAllowedModel.mockResolvedValue(DEFAULT_BOT_MODEL); + }); + + it('refuses to move an organization connection without caller authorization', async () => { + mockLimit.mockResolvedValueOnce([organizationIntegration('T_OLD')]).mockResolvedValueOnce([]); + + await expect( + upsertSlackInstallation({ owner: organizationOwner, teamId: 'T_NEW', installation }) + ).rejects.toThrow(SlackWorkspaceRebindNotAllowedError); + + expect(mockUpdateSet).not.toHaveBeenCalled(); + }); + + it('moves an organization connection when the caller authorized the change', async () => { + mockLimit.mockResolvedValueOnce([organizationIntegration('T_OLD')]).mockResolvedValueOnce([]); + + await upsertSlackInstallation({ + owner: organizationOwner, + teamId: 'T_NEW', + installation, + allowWorkspaceRebind: true, + }); + + expect(mockUpdateSet).toHaveBeenCalledWith( + expect.objectContaining({ platform_installation_id: 'T_NEW' }) + ); + }); + + it('refreshes an organization connection bound to the same workspace', async () => { + mockLimit.mockResolvedValueOnce([organizationIntegration('T123')]).mockResolvedValueOnce([]); + + await upsertSlackInstallation({ owner: organizationOwner, teamId: 'T123', installation }); + + expect(mockUpdateSet).toHaveBeenCalledWith( + expect.objectContaining({ platform_installation_id: 'T123' }) + ); + }); + + it('binds an organization row that has no workspace yet', async () => { + mockLimit.mockResolvedValueOnce([organizationIntegration(null)]).mockResolvedValueOnce([]); + + await upsertSlackInstallation({ owner: organizationOwner, teamId: 'T123', installation }); + + expect(mockUpdateSet).toHaveBeenCalledWith( + expect.objectContaining({ platform_installation_id: 'T123' }) + ); + }); + + it('keeps personal workspace changes allowed', async () => { + mockLimit + .mockResolvedValueOnce([buildSlackIntegration({ platform_installation_id: 'T_OLD' })]) + .mockResolvedValueOnce([]); + + await upsertSlackInstallation({ owner, teamId: 'T_NEW', installation }); + + expect(mockUpdateSet).toHaveBeenCalledWith( + expect.objectContaining({ platform_installation_id: 'T_NEW' }) + ); + }); +}); diff --git a/apps/web/src/lib/integrations/slack-service.ts b/apps/web/src/lib/integrations/slack-service.ts index d15957509f4..e1fbbecb268 100644 --- a/apps/web/src/lib/integrations/slack-service.ts +++ b/apps/web/src/lib/integrations/slack-service.ts @@ -25,6 +25,15 @@ export class SlackWorkspaceAlreadyConnectedError extends Error { } } +export class SlackWorkspaceRebindNotAllowedError extends Error { + constructor() { + super( + 'This organization is already connected to a different Slack workspace. Disconnect it before connecting another workspace.' + ); + this.name = 'SlackWorkspaceRebindNotAllowedError'; + } +} + // Slack OAuth scopes for the integration // These should be kept in sync with the scopes requested in the Slack app configuration export const SLACK_SCOPES = [ @@ -231,15 +240,21 @@ async function mirrorSlackCredential({ /** * Create or update Slack installation from the Chat SDK OAuth callback result. + * + * An organization connection holds a shared bot token and model policy, so moving + * it to another Slack workspace is refused unless the caller passes + * `allowWorkspaceRebind` after verifying organization-management authority. */ export async function upsertSlackInstallation({ owner, teamId, installation, + allowWorkspaceRebind = false, }: { owner: Owner; teamId: string; installation: SlackInstallation; + allowWorkspaceRebind?: boolean; }): Promise { const existing = await getInstallation(owner); const teamName = installation.teamName || 'Unknown Team'; @@ -249,6 +264,16 @@ export async function upsertSlackInstallation({ throw new SlackWorkspaceAlreadyConnectedError(teamName); } + if ( + existing && + !allowWorkspaceRebind && + owner.type === 'org' && + existing.platform_installation_id && + existing.platform_installation_id !== teamId + ) { + throw new SlackWorkspaceRebindNotAllowedError(); + } + // For org integrations, get a model that respects org access policy. // For user integrations, use the shared bot default model. const defaultModel = diff --git a/apps/web/src/routers/slack-router.ts b/apps/web/src/routers/slack-router.ts index 79d8839a7cc..79e1689ec0e 100644 --- a/apps/web/src/routers/slack-router.ts +++ b/apps/web/src/routers/slack-router.ts @@ -9,6 +9,7 @@ import { optionalOrgInput, } from '@/lib/integrations/resolve-owner'; import { ensureOrganizationAccess } from '@kilocode/web-shared/routers/organizations/utils'; +import { ORGANIZATION_MANAGE_ROLES } from '@kilocode/app-shared/organizations'; import { requireActiveSubscriptionOrTrial } from '@kilocode/web-shared/lib/organizations/trial-middleware'; import { createAuditLog } from '@kilocode/web-shared/lib/organizations/organization-audit-logs'; import { unlinkTeamKiloUsers } from '@/lib/bot-identity'; @@ -70,7 +71,11 @@ export const slackRouter = createTRPCRouter({ await ensureOrganizationAccess(ctx, input.organizationId); await requireActiveSubscriptionOrTrial(input.organizationId); } - const owner = await resolveAuthorizedOwner(ctx, input?.organizationId); + const owner = await resolveAuthorizedOwner( + ctx, + input?.organizationId, + ORGANIZATION_MANAGE_ROLES + ); const result = await slackService.uninstallApp(owner, { deleteChatSdkInstallation: deleteChatSdkSlackInstallation, deleteChatSdkIdentityCache: deleteChatSdkSlackIdentityCache, @@ -113,7 +118,11 @@ export const slackRouter = createTRPCRouter({ await ensureOrganizationAccess(ctx, input.organizationId); await requireActiveSubscriptionOrTrial(input.organizationId); } - const owner = await resolveAuthorizedOwner(ctx, input.organizationId); + const owner = await resolveAuthorizedOwner( + ctx, + input.organizationId, + ORGANIZATION_MANAGE_ROLES + ); const result = await slackService.updateModel(owner, input.modelSlug); if (input.organizationId) { From 5ec2f913ba7ff02ccbf41bd0d75d75981de4830a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 02:49:37 +0000 Subject: [PATCH 2/4] fix(slack): cover legacy workspace identity and state-aware member copy - The organization workspace check compares the installation id with the account id fallback, so rows that record their workspace only in the account id are covered too. - Add a regression case for that legacy shape and make the unbound fixture a row with no workspace identity at all. - The member notice mentions the connection test only when the test control is rendered. --- .../integrations/SlackIntegrationDetails.tsx | 3 ++- .../lib/integrations/slack-service.test.ts | 22 +++++++++++++++++-- .../web/src/lib/integrations/slack-service.ts | 6 +++-- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/apps/web/src/components/integrations/SlackIntegrationDetails.tsx b/apps/web/src/components/integrations/SlackIntegrationDetails.tsx index 899d992301f..ac715440fb8 100644 --- a/apps/web/src/components/integrations/SlackIntegrationDetails.tsx +++ b/apps/web/src/components/integrations/SlackIntegrationDetails.tsx @@ -334,7 +334,8 @@ export function SlackIntegrationDetails({ Only organization owners and admins can connect, re-install, or disconnect this Slack - integration. You can still test the connection. + integration. + {installation && !isSuspended && ' You can still test the connection.'} )} diff --git a/apps/web/src/lib/integrations/slack-service.test.ts b/apps/web/src/lib/integrations/slack-service.test.ts index e0284df79bf..c8e478dfd42 100644 --- a/apps/web/src/lib/integrations/slack-service.test.ts +++ b/apps/web/src/lib/integrations/slack-service.test.ts @@ -445,11 +445,15 @@ describe('upsertSlackInstallation organization workspace binding', () => { teamName: 'Kilo Team', } satisfies SlackInstallation; - function organizationIntegration(platformInstallationId: string | null) { + function organizationIntegration( + platformInstallationId: string | null, + platformAccountId: string | null = platformInstallationId + ) { return buildSlackIntegration({ owned_by_user_id: null, owned_by_organization_id: organizationOwner.id, platform_installation_id: platformInstallationId, + platform_account_id: platformAccountId, }); } @@ -503,7 +507,9 @@ describe('upsertSlackInstallation organization workspace binding', () => { }); it('binds an organization row that has no workspace yet', async () => { - mockLimit.mockResolvedValueOnce([organizationIntegration(null)]).mockResolvedValueOnce([]); + mockLimit + .mockResolvedValueOnce([organizationIntegration(null, null)]) + .mockResolvedValueOnce([]); await upsertSlackInstallation({ owner: organizationOwner, teamId: 'T123', installation }); @@ -512,6 +518,18 @@ describe('upsertSlackInstallation organization workspace binding', () => { ); }); + it('refuses a legacy organization row bound only by its account id', async () => { + mockLimit + .mockResolvedValueOnce([organizationIntegration(null, 'T_OLD')]) + .mockResolvedValueOnce([]); + + await expect( + upsertSlackInstallation({ owner: organizationOwner, teamId: 'T_NEW', installation }) + ).rejects.toThrow(SlackWorkspaceRebindNotAllowedError); + + expect(mockUpdateSet).not.toHaveBeenCalled(); + }); + it('keeps personal workspace changes allowed', async () => { mockLimit .mockResolvedValueOnce([buildSlackIntegration({ platform_installation_id: 'T_OLD' })]) diff --git a/apps/web/src/lib/integrations/slack-service.ts b/apps/web/src/lib/integrations/slack-service.ts index e1fbbecb268..dd3518cb5de 100644 --- a/apps/web/src/lib/integrations/slack-service.ts +++ b/apps/web/src/lib/integrations/slack-service.ts @@ -258,6 +258,8 @@ export async function upsertSlackInstallation({ }): Promise { const existing = await getInstallation(owner); const teamName = installation.teamName || 'Unknown Team'; + // Legacy rows can carry the workspace id only in `platform_account_id`. + const boundTeamId = existing?.platform_installation_id ?? existing?.platform_account_id ?? null; const conflicting = await getConflictingSlackInstallation(owner, teamId); if (conflicting) { @@ -268,8 +270,8 @@ export async function upsertSlackInstallation({ existing && !allowWorkspaceRebind && owner.type === 'org' && - existing.platform_installation_id && - existing.platform_installation_id !== teamId + boundTeamId && + boundTeamId !== teamId ) { throw new SlackWorkspaceRebindNotAllowedError(); } From cf925c99b119ececdac0a4de75aac570270e35c0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 03:04:04 +0000 Subject: [PATCH 3/4] test(slack): cover an organization completion without an install purpose The organization role check runs before the install-purpose and attempt checks, so the same authority applies to states created without that purpose. Pin it with a callback case. --- .../oauth/platforms/slack-callback.test.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts index d47d385359b..2145d39f61d 100644 --- a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts +++ b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts @@ -106,6 +106,18 @@ describe('handleSlackOAuthCallback organization authority', () => { expect(mockCaptureException).not.toHaveBeenCalled(); }); + it('refuses a member completion that carries no install purpose', async () => { + mockEnsureOrganizationAccess.mockRejectedValue(permissionDeniedError); + + const { response } = await callCallback(`org_${ORGANIZATION_ID}`, undefined); + + expect(new URL(response.headers.get('location') ?? '').searchParams.get('error')).toBe( + 'permission_required' + ); + expect(mockUpsertSlackInstallation).not.toHaveBeenCalled(); + expect(mockHandleOAuthCallback).not.toHaveBeenCalled(); + }); + it('completes an organization install for an admin and authorizes a workspace change', async () => { mockEnsureOrganizationAccess.mockResolvedValue('admin'); From 88632ef111a87aa5f9abcbdd55cba717ec3b3c87 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Igor=20=C5=A0=C4=87eki=C4=87?= <4062236+iscekic@users.noreply.github.com> Date: Tue, 20 Oct 2026 03:09:12 +0000 Subject: [PATCH 4/4] test(slack): build the purpose-less state explicitly The helper defaulted a passed `undefined` back to the install purpose, so the new case re-ran the purpose-bearing state. Take `null` for "no purpose" and assert the signed state carries none. --- .../integrations/oauth/platforms/slack-callback.test.ts | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts index 2145d39f61d..4edd5eb0b30 100644 --- a/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts +++ b/apps/web/src/lib/integrations/oauth/platforms/slack-callback.test.ts @@ -64,9 +64,9 @@ const permissionDeniedError = new TRPCError({ async function callCallback( owner: string, - statePurpose: 'provider_install' | undefined = 'provider_install' + statePurpose: 'provider_install' | null = 'provider_install' ) { - const state = createOAuthState(owner, USER_ID, undefined, statePurpose); + const state = createOAuthState(owner, USER_ID, undefined, statePurpose ?? undefined); const request = new NextRequest( `http://localhost:3000/api/integrations/slack/callback?code=code-1&state=${encodeURIComponent(state)}` ); @@ -109,8 +109,11 @@ describe('handleSlackOAuthCallback organization authority', () => { it('refuses a member completion that carries no install purpose', async () => { mockEnsureOrganizationAccess.mockRejectedValue(permissionDeniedError); - const { response } = await callCallback(`org_${ORGANIZATION_ID}`, undefined); + const { response, state } = await callCallback(`org_${ORGANIZATION_ID}`, null); + expect(JSON.parse(Buffer.from(state.split('.')[0], 'base64url').toString())).not.toHaveProperty( + 'purpose' + ); expect(new URL(response.headers.get('location') ?? '').searchParams.get('error')).toBe( 'permission_required' );