diff --git a/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts b/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts index 72be1deea27..ce4cc24e046 100644 --- a/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts +++ b/packages/cli/src/modules/agents/__tests__/agent-integrations.controller.test.ts @@ -647,6 +647,42 @@ describe('AgentIntegrationsController integration credentials', () => { ); }); + it.each([ + ['unpublished', null, true], + ['published', 'v1', false], + ])( + 'defaults external resource deletion to %s agent behavior', + async (_status, activeVersionId, expectedDeleteExternalResource) => { + const agentRepository = mock(); + agentRepository.findByIdAndProjectId.mockResolvedValue({ + id: 'agent-1', + projectId: 'project-1', + activeVersionId, + integrations: [{ type: 'slack', credentialId: 'cred-slack' }], + } as never); + const slackAppSetupService = mock(); + const { controller } = makeController({ agentRepository, slackAppSetupService }); + + await controller.disconnectIntegration( + { + params: { projectId: 'project-1' }, + user: { id: 'user-1' }, + } as never, + undefined as never, + 'agent-1', + { type: 'slack', credentialId: 'cred-slack' }, + ); + + expect(slackAppSetupService.deleteManagedAppForCredential).toHaveBeenCalledWith({ + projectId: 'project-1', + agentId: 'agent-1', + credentialId: 'cred-slack', + user: { id: 'user-1' }, + deleteExternalResource: expectedDeleteExternalResource, + }); + }, + ); + it('returns a warning while still disconnecting when the managed Slack app remains', async () => { const agentRepository = mock(); const agent = { diff --git a/packages/cli/src/modules/agents/agent-integration-management.service.ts b/packages/cli/src/modules/agents/agent-integration-management.service.ts index 41661558dba..fae881adae8 100644 --- a/packages/cli/src/modules/agents/agent-integration-management.service.ts +++ b/packages/cli/src/modules/agents/agent-integration-management.service.ts @@ -97,13 +97,16 @@ export class AgentIntegrationManagementService { credentialId: options.credentialId, }); const integration = persisted ?? (parsed.success ? parsed.data : undefined); + // if agent is not published fallback to delete external resource + const deleteExternalResource = + options.deleteExternalResource ?? options.agent.activeVersionId === null; const warning = persisted ? await this.registry.get(options.type)?.onRemove?.({ agentId: options.agent.id, projectId: options.agent.projectId, credentialId: options.credentialId, user: options.user, - deleteExternalResource: options.deleteExternalResource, + deleteExternalResource, }) : undefined; diff --git a/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts b/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts index 49f2b82ac0a..8a1e8bba9ac 100644 --- a/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts +++ b/packages/cli/src/modules/agents/integrations/__tests__/slack-app-setup.service.test.ts @@ -675,7 +675,7 @@ describe('Slack setup services', () => { }); }); - it('refreshes the manager token and auto-installs with a project-owned bot credential', async () => { + it('refreshes the manager token during icon setup and auto-installs the bot credential', async () => { credentialsOverwrites.getOverwrites.mockReturnValue({ clientId: 'client', clientSecret: 'secret', @@ -703,7 +703,6 @@ describe('Slack setup services', () => { }); requestMock .mockResolvedValueOnce(slackAppCreatedResponse()) - .mockResolvedValueOnce(slackResponse({ ok: true })) .mockResolvedValueOnce(slackResponse({ ok: false, error: 'token_expired' })) .mockResolvedValueOnce( slackResponse({ @@ -714,6 +713,7 @@ describe('Slack setup services', () => { }, }), ) + .mockResolvedValueOnce(slackResponse({ ok: true })) .mockResolvedValueOnce( slackResponse({ ok: true, @@ -759,8 +759,14 @@ describe('Slack setup services', () => { expect(iconRequest.body.get('app_id')).toBe('A123'); expect(iconRequest.body.get('token')).toBe('xoxp-manager'); expect(iconRequest.body.get('file')).toBeInstanceOf(Blob); - expect(fetchParams(requestMock, 2).get('bot_scopes')).toContain('chat:write'); - expect(fetchParams(requestMock, 3).get('grant_type')).toBe('refresh_token'); + expect(fetchParams(requestMock, 2).get('grant_type')).toBe('refresh_token'); + const retriedIconRequest = requestMock.mock.calls[3]?.[0] as { + url: string; + body: FormData; + }; + expect(retriedIconRequest.url).toBe('https://slack.com/api/apps.icon.set'); + expect(retriedIconRequest.body.get('token')).toBe('xoxp-refreshed'); + expect(fetchParams(requestMock, 4).get('bot_scopes')).toContain('chat:write'); expect(fetchParams(requestMock, 4).get('token')).toBe('xoxp-refreshed'); expect(credentialsService.update).toHaveBeenCalledWith( 'manager', @@ -788,6 +794,55 @@ describe('Slack setup services', () => { ); }); + it('deletes a newly created managed app when icon setup fails', async () => { + credentialsOverwrites.getOverwrites.mockReturnValue({ + clientId: 'client', + clientSecret: 'secret', + userScope: 'app_configurations:read app_configurations:write managed_apps:install', + }); + credentialsOverwrites.usesManagedAuth.mockReturnValue(true); + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + { id: 'manager', type: 'slackManagerOAuth2Api' }, + ] as never); + credentialsFinderService.findCredentialForUser.mockResolvedValue({ + id: 'manager', + name: 'Slack manager', + type: 'slackManagerOAuth2Api', + } as CredentialsEntity); + credentialsService.decrypt.mockResolvedValue({ + oauthTokenData: { + authed_user: { + access_token: 'xoxp-manager', + scope: 'app_configurations:read app_configurations:write managed_apps:install', + }, + team: { id: 'T123', name: 'Example workspace' }, + }, + }); + requestMock + .mockResolvedValueOnce(slackAppCreatedResponse()) + .mockResolvedValueOnce(slackResponse({ ok: false, error: 'slack_request_failed' })) + .mockResolvedValueOnce(slackResponse({ ok: true })); + + await expect( + service.installManagedApp({ + projectId: 'project-1', + agentId: 'agent-1', + managerCredentialId: 'manager', + workspaceId: 'T123', + user, + }), + ).rejects.toThrow('Slack could not set the Slack app icon: slack_request_failed'); + + expect(requestMock.mock.calls[2]?.[0]).toEqual( + expect.objectContaining({ url: 'https://slack.com/api/apps.manifest.delete' }), + ); + expect(fetchParams(requestMock, 2).get('app_id')).toBe('A123'); + expect(fetchParams(requestMock, 2).get('token')).toBe('xoxp-manager'); + expect(cacheService.delete).toHaveBeenCalledWith( + 'agents:slack-managed-app:project-1:agent-1:manager:T123', + ); + }); + it('returns the child OAuth URL when managed installation needs approval', async () => { credentialsOverwrites.getOverwrites.mockReturnValue({ clientId: 'client', @@ -846,6 +901,77 @@ describe('Slack setup services', () => { expect(credentialsService.createManagedCredential).not.toHaveBeenCalled(); }); + it('preserves managed app provenance after fallback OAuth installation', async () => { + credentialsOverwrites.getOverwrites.mockReturnValue({ + clientId: 'client', + clientSecret: 'secret', + userScope: 'app_configurations:read app_configurations:write managed_apps:install', + }); + credentialsOverwrites.usesManagedAuth.mockReturnValue(true); + credentialsService.getCredentialsAUserCanUseInAWorkflow.mockResolvedValue([ + { id: 'manager', type: 'slackManagerOAuth2Api' }, + ] as never); + credentialsFinderService.findCredentialForUser.mockResolvedValue({ + id: 'manager', + name: 'Slack manager', + type: 'slackManagerOAuth2Api', + } as CredentialsEntity); + credentialsService.decrypt.mockResolvedValue({ + oauthTokenData: { + authed_user: { + access_token: 'xoxp-manager', + scope: 'app_configurations:read app_configurations:write managed_apps:install', + }, + team: { id: 'T123', name: 'Example workspace' }, + }, + }); + requestMock + .mockResolvedValueOnce(slackAppCreatedResponse()) + .mockResolvedValueOnce(slackResponse({ ok: true })) + .mockResolvedValueOnce(slackResponse({ ok: false, error: 'app_approval_request_eligible' })) + .mockResolvedValueOnce(slackOAuthResponse()); + userRepository.findOne.mockResolvedValue(user); + credentialsService.createManagedCredential.mockResolvedValue({ + id: 'bot-credential', + } as never); + agentIntegrationPersistenceService.saveCredentialIntegration.mockResolvedValue(agent as never); + + const result = await service.installManagedApp({ + projectId: 'project-1', + agentId: 'agent-1', + managerCredentialId: 'manager', + workspaceId: 'T123', + user, + }); + expect(result.status).toBe('manual_install_required'); + if (result.status !== 'manual_install_required') return; + + const state = new URL(result.installUrl).searchParams.get('state') ?? ''; + await service.completeInstall({ + projectId: 'project-1', + agentId: 'agent-1', + code: 'slack-code', + state, + }); + + expect(credentialsService.createManagedCredential).toHaveBeenCalledWith( + { + name: 'Example workspace - Support Agent', + type: 'slackApi', + data: { + accessToken: 'xoxb-installed-token', + signatureSecret: 'signing-secret', + managedAppId: 'A123', + teamId: 'T123', + managerCredentialId: 'manager', + }, + projectId: 'project-1', + }, + user, + ); + expect(credentialsService.createUnmanagedCredential).not.toHaveBeenCalled(); + }); + it('exports settings for a managed Slack bot credential', async () => { credentialsOverwrites.getOverwrites.mockReturnValue({ clientId: 'client', diff --git a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-managed-setup.service.ts b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-managed-setup.service.ts index ea1ca6a4242..a4f06b17ef4 100644 --- a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-managed-setup.service.ts +++ b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-managed-setup.service.ts @@ -25,14 +25,10 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { CacheService } from '@/services/cache/cache.service'; import { SlackMethodsService } from './slack-methods.service'; -import { - childRecord, - SLACK_BOT_SCOPES, - type SlackAppSetupSession, - stringProperty, -} from './slack-setup.types'; +import { childRecord, SLACK_BOT_SCOPES, type SlackAppSetupSession } from './slack-setup.types'; import type { Agent } from '../../../entities/agent.entity'; import { AgentRepository } from '../../../repositories/agent.repository'; +import { stringProperty } from '../../integration-helpers'; const SLACK_MANAGED_APP_CACHE_PREFIX = 'agents:slack-managed-app:'; const SLACK_APP_SETUP_TTL_MS = 60 * 60 * 1000; @@ -111,6 +107,9 @@ interface ManagedBotCredentialContext { teamId: string; } +type SlackApiParams = Record | FormData; +type SlackApiParamsFactory = (accessToken: string) => SlackApiParams; + function stringsFromScope(value: unknown): Set { if (typeof value !== 'string') return new Set(); return new Set(value.split(/[\s,]+/).filter(Boolean)); @@ -715,31 +714,41 @@ export class SlackManagedSetupService { const clientSecret = stringProperty(credentials, 'client_secret'); const signingSecret = stringProperty(credentials, 'signing_secret'); const oauthAuthorizeUrl = stringProperty(response, 'oauth_authorize_url'); - if (!appId || !clientId || !clientSecret || !signingSecret || !oauthAuthorizeUrl) { - throw new BadRequestError('Slack returned an incomplete app setup response'); - } - await this.setManagedAppIcon(manager, appId); + try { + if (!appId || !clientId || !clientSecret || !signingSecret || !oauthAuthorizeUrl) { + throw new BadRequestError('Slack returned an incomplete app setup response'); + } + await this.setManagedAppIcon(manager, appId); - const session = { - projectId: options.projectId, - agentId: options.agentId, - userId: options.user.id, - appId, - clientId, - clientSecret, - signingSecret, - redirectUrl, - managerCredentialId: options.managerCredentialId, - teamId: options.workspaceId, - teamName: workspaceName, - oauthAuthorizeUrl, - } satisfies ManagedSlackAppSession; - await this.cacheService.set( - key, - await this.cipher.encryptV2(JSON.stringify(session)), - SLACK_APP_SETUP_TTL_MS, - ); - return { session, created: true }; + const session = { + projectId: options.projectId, + agentId: options.agentId, + userId: options.user.id, + appId, + clientId, + clientSecret, + signingSecret, + redirectUrl, + managerCredentialId: options.managerCredentialId, + teamId: options.workspaceId, + teamName: workspaceName, + oauthAuthorizeUrl, + } satisfies ManagedSlackAppSession; + await this.cacheService.set( + key, + await this.cipher.encryptV2(JSON.stringify(session)), + SLACK_APP_SETUP_TTL_MS, + ); + return { session, created: true }; + } catch (error) { + if (appId) { + await Promise.allSettled([ + this.callManagerSlackApi(manager, 'apps.manifest.delete', { app_id: appId }), + this.cacheService.delete(key), + ]); + } + throw error; + } } private async decryptManagedAppSession( @@ -758,12 +767,11 @@ export class SlackManagedSetupService { private async callManagerSlackApi( manager: ManagerCredentialContext, method: string, - params: Record, + params: Record | SlackApiParamsFactory, ): Promise> { - let response = await this.methods.callSlackApi(method, { - ...params, - token: manager.accessToken, - }); + const paramsForToken = (accessToken: string): SlackApiParams => + typeof params === 'function' ? params(accessToken) : { ...params, token: accessToken }; + let response = await this.methods.callSlackApi(method, paramsForToken(manager.accessToken)); const error = stringProperty(response, 'error'); if (!['invalid_auth', 'token_expired'].includes(error ?? '')) return response; @@ -805,7 +813,7 @@ export class SlackManagedSetupService { await this.credentialsService.update(manager.credential.id, encrypted, updatedData); manager.oauthTokenData = oauthTokenData; manager.accessToken = refreshedAccessToken; - response = await this.methods.callSlackApi(method, { ...params, token: refreshedAccessToken }); + response = await this.methods.callSlackApi(method, paramsForToken(refreshedAccessToken)); return response; } @@ -823,15 +831,17 @@ export class SlackManagedSetupService { managedAppId: string, ): Promise { const image = await readFile(join(__dirname, 'assets', 'n8n-bot-icon.png')); - const formData = new FormData(); - formData.set('token', manager.accessToken); - formData.set('app_id', managedAppId); - formData.set( - 'file', - new Blob([new Uint8Array(image)], { type: 'image/png' }), - 'n8n-bot-icon.png', - ); - const response = await this.methods.callSlackApi('apps.icon.set', formData); + const response = await this.callManagerSlackApi(manager, 'apps.icon.set', (accessToken) => { + const formData = new FormData(); + formData.set('token', accessToken); + formData.set('app_id', managedAppId); + formData.set( + 'file', + new Blob([new Uint8Array(image)], { type: 'image/png' }), + 'n8n-bot-icon.png', + ); + return formData; + }); if (response.ok !== true) { throw this.methods.slackError('set the Slack app icon', response); } diff --git a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-manual-setup.service.ts b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-manual-setup.service.ts index c2eaa186a53..3dd56568252 100644 --- a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-manual-setup.service.ts +++ b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-manual-setup.service.ts @@ -12,12 +12,8 @@ import { NotFoundError } from '@/errors/response-errors/not-found.error'; import { CacheService } from '@/services/cache/cache.service'; import { SlackMethodsService } from './slack-methods.service'; -import { - childRecord, - type SlackAppSetupSession, - slackSetupCacheKey, - stringProperty, -} from './slack-setup.types'; +import { childRecord, type SlackAppSetupSession, slackSetupCacheKey } from './slack-setup.types'; +import { stringProperty } from '../../integration-helpers'; export interface CreateSlackAppOptions { projectId: string; diff --git a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-methods.service.ts b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-methods.service.ts index 303587be5cc..b411836a9ef 100644 --- a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-methods.service.ts +++ b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-methods.service.ts @@ -19,12 +19,12 @@ import { SLACK_BOT_SCOPES, type SlackAppSetupSession, slackSetupCacheKey, - stringProperty, } from './slack-setup.types'; import { AgentIntegrationPersistenceService } from '../../../agent-integration-persistence.service'; import type { Agent } from '../../../entities/agent.entity'; import { AgentRepository } from '../../../repositories/agent.repository'; import { ChatIntegrationService } from '../../chat-integration.service'; +import { stringProperty } from '../../integration-helpers'; const SLACK_MANAGED_APP_CACHE_PREFIX = 'agents:slack-managed-app:'; const SLACK_APP_SETUP_TTL_MS = 60 * 60 * 1000; diff --git a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-setup.types.ts b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-setup.types.ts index d7368aca481..9da7b45eb58 100644 --- a/packages/cli/src/modules/agents/integrations/platforms/slack/slack-setup.types.ts +++ b/packages/cli/src/modules/agents/integrations/platforms/slack/slack-setup.types.ts @@ -49,11 +49,3 @@ export function childRecord( const child = record[key]; return isRecord(child) ? child : undefined; } - -export function stringProperty( - record: Record | undefined, - key: string, -): string | undefined { - const value = record?.[key]; - return typeof value === 'string' ? value : undefined; -} diff --git a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSettings.test.ts b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSettings.test.ts index 97b611e4cf0..cbbb003db40 100644 --- a/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSettings.test.ts +++ b/packages/frontend/editor-ui/src/features/agents/__tests__/AgentChannelSlackManagedSettings.test.ts @@ -34,18 +34,18 @@ function mountForm( template: '', }, - Switch2: { + Switch: { props: ['modelValue'], emits: ['update:modelValue'], template: '