From 8b803f500476ce26b53fc718f26e57b8d3ccabc2 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Wed, 17 Jun 2020 15:06:06 -0700 Subject: [PATCH 01/13] updated version to 2.1.1-beta.1 for org apps beta release --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 3ea0c54cc..8136510b0 100644 --- a/package.json +++ b/package.json @@ -43,7 +43,7 @@ }, "dependencies": { "@slack/logger": "^2.0.0", - "@slack/oauth": "^1.2.0", + "@slack/oauth": "feat-org-apps", "@slack/types": "^1.9.0", "@slack/web-api": "^5.12.0", "@types/express": "^4.16.1", From 9e89cee14523412eba340d3e937c13351b224edb Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Thu, 18 Jun 2020 16:51:48 -0700 Subject: [PATCH 02/13] updated interactive payloads for org apps --- src/App.ts | 52 +++++++++++++++++------- src/types/actions/block-action.ts | 10 ++++- src/types/actions/dialog-action.ts | 9 +++- src/types/actions/interactive-message.ts | 9 +++- src/types/options/index.ts | 9 +++- src/types/shortcuts/global-shortcut.ts | 10 ++++- src/types/shortcuts/message-shortcut.ts | 10 ++++- src/types/view/index.ts | 16 +++++++- 8 files changed, 103 insertions(+), 22 deletions(-) diff --git a/src/App.ts b/src/App.ts index b9f1a5903..0b3317574 100644 --- a/src/App.ts +++ b/src/App.ts @@ -86,7 +86,7 @@ export interface Authorize { /** Authorization function inputs - authenticated data about an event for the authorization function */ export interface AuthorizeSourceData { - teamId: string; + teamId: string | undefined; enterpriseId?: string; userId?: string; conversationId?: string; @@ -631,10 +631,10 @@ export default class App { let { client } = this; const token = selectToken(context); if (token !== undefined) { - let pool = this.clients[source.teamId]; + let pool = this.clients[source.teamId!]; if (pool === undefined) { // eslint-disable-next-line no-multi-assign - pool = this.clients[source.teamId] = new WebClientPool(); + pool = this.clients[source.teamId!] = new WebClientPool(); } client = pool.getOrCreate(token, this.clientOptions); } @@ -717,30 +717,54 @@ function buildSource( teamId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) - : type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.user !== undefined ? ((body as ( | SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs | SlackShortcutMiddlewareArgs - )['body']).team.id as string) - : assertNever(type), + )['body']).user.team_id as string) + : undefined, enterpriseId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) - : type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.enterprise_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.enterprise !== undefined ? ((body as ( | SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs | SlackShortcutMiddlewareArgs - )['body']).team.enterprise_id as string) + )['body']).enterprise!.id as string) : undefined, userId: type === IncomingEventType.Event diff --git a/src/types/actions/block-action.ts b/src/types/actions/block-action.ts index 01836647c..55ecf5843 100644 --- a/src/types/actions/block-action.ts +++ b/src/types/actions/block-action.ts @@ -209,7 +209,7 @@ export interface BlockAction ex domain: string; enterprise_id?: string; // undocumented enterprise_name?: string; // undocumented - }; + } | null; channel?: { id: string; name: string; @@ -52,6 +52,13 @@ export interface OptionsRequest ex // this appears in the block_suggestions schema, but we're not sure when its present or what its type would be app_unfurl?: any; + + // exists for enterprise installs + is_enterprise_install?: boolean; + enterprise?: { + id: string; + name: string; + }; } /** diff --git a/src/types/shortcuts/global-shortcut.ts b/src/types/shortcuts/global-shortcut.ts index 73437d109..93c4fd9f9 100644 --- a/src/types/shortcuts/global-shortcut.ts +++ b/src/types/shortcuts/global-shortcut.ts @@ -12,12 +12,20 @@ export interface GlobalShortcut { username: string; team_id: string; }; + // team is null for org apps shortcut payload team: { id: string; domain: string; enterprise_id?: string; enterprise_name?: string; - }; + } | null; token: string; action_ts: string; + + // exists for enterprise installs + is_enterprise_install?: boolean; + enterprise?: { + id: string; + name: string; + }; } diff --git a/src/types/shortcuts/message-shortcut.ts b/src/types/shortcuts/message-shortcut.ts index a16a48caa..9cd07de1b 100644 --- a/src/types/shortcuts/message-shortcut.ts +++ b/src/types/shortcuts/message-shortcut.ts @@ -21,6 +21,7 @@ export interface MessageShortcut { id: string; name: string; team_id?: string; // undocumented + username?: string; // shows up on org app msg actions }; channel: { id: string; @@ -31,7 +32,14 @@ export interface MessageShortcut { domain: string; enterprise_id?: string; // undocumented enterprise_name?: string; // undocumented - }; + } | null; token: string; action_ts: string; // undocumented + + // exists for enterprise installs + is_enterprise_install?: boolean; + enterprise?: { + id: string; + name: string; + }; } diff --git a/src/types/view/index.ts b/src/types/view/index.ts index 336ca1951..774931dc6 100644 --- a/src/types/view/index.ts +++ b/src/types/view/index.ts @@ -39,7 +39,7 @@ export interface ViewSubmitAction { domain: string; enterprise_id?: string; // undocumented enterprise_name?: string; // undocumented - }; + } | null; user: { id: string; name: string; @@ -48,6 +48,12 @@ export interface ViewSubmitAction { view: ViewOutput; api_app_id: string; token: string; + // exists for enterprise installs + is_enterprise_install?: boolean; + enterprise?: { + id: string; + name: string; + }; } /** @@ -62,7 +68,7 @@ export interface ViewClosedAction { domain: string; enterprise_id?: string; // undocumented enterprise_name?: string; // undocumented - }; + } | null; user: { id: string; name: string; @@ -72,6 +78,12 @@ export interface ViewClosedAction { api_app_id: string; token: string; is_cleared: boolean; + // exists for enterprise installs + is_enterprise_install?: boolean; + enterprise?: { + id: string; + name: string; + }; } /** From 857b1361d3760d1213d6f992af9092b46984859c Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 14 Jul 2020 16:31:13 -0700 Subject: [PATCH 03/13] updated org app support based on proposal --- src/App.ts | 321 +++++++++++++++++++++++++++++++++++++---------------- 1 file changed, 224 insertions(+), 97 deletions(-) diff --git a/src/App.ts b/src/App.ts index 0b3317574..5685c3a97 100644 --- a/src/App.ts +++ b/src/App.ts @@ -70,6 +70,7 @@ export interface AppOptions { botId?: AuthorizeResult['botId']; // only used when authorize is not defined, shortcut for fetching botUserId?: AuthorizeResult['botUserId']; // only used when authorize is not defined, shortcut for fetching authorize?: Authorize; // either token or authorize + orgAuthorize?: Authorize; receiver?: Receiver; logger?: Logger; logLevel?: LogLevel; @@ -81,17 +82,25 @@ export { LogLevel, Logger } from '@slack/logger'; /** Authorization function - seeds the middleware processing and listeners with an authorization context */ export interface Authorize { - (source: AuthorizeSourceData, body?: AnyMiddlewareArgs['body']): Promise; + (source: AuthorizeSourceData | OrgAuthorizeSourceData, body?: AnyMiddlewareArgs['body']): Promise; } /** Authorization function inputs - authenticated data about an event for the authorization function */ export interface AuthorizeSourceData { - teamId: string | undefined; + teamId: string; enterpriseId?: string; userId?: string; conversationId?: string; } +/** Authorization function inputs - authenticated data about an event for the authorization function */ +export interface OrgAuthorizeSourceData { + enterpriseId: string; + teamId?: string; + userId?: string; + conversationId?: string; +} + /** Authorization function outputs - data that will be available as part of event processing */ export interface AuthorizeResult { // one of either botToken or userToken are required @@ -99,6 +108,7 @@ export interface AuthorizeResult { userToken?: string; // used by `say` (overridden by botToken) botId?: string; // required for `ignoreSelf` global middleware botUserId?: string; // optional but allows `ignoreSelf` global middleware be more filter more than just message events + teamId?: string; [key: string]: any; } @@ -146,7 +156,7 @@ export default class App { private clientOptions: WebClientOptions; - private clients: { [teamId: string]: WebClientPool } = {}; + private clients: { [teamOrEnterpriseId: string]: WebClientPool } = {}; /** Receiver - ingests events from the Slack platform */ private receiver: Receiver; @@ -157,6 +167,9 @@ export default class App { /** Authorize */ private authorize!: Authorize; + /** Org Authorize */ + private orgAuthorize!: Authorize; + /** Global middleware chain */ private middleware: Middleware[]; @@ -180,6 +193,7 @@ export default class App { botId = undefined, botUserId = undefined, authorize = undefined, + orgAuthorize = undefined, logger = undefined, logLevel = undefined, ignoreSelf = true, @@ -265,22 +279,30 @@ export default class App { } if (token !== undefined) { - if (authorize !== undefined || usingOauth) { + if (authorize !== undefined || orgAuthorize !== undefined || usingOauth) { throw new AppInitializationError( - `token as well as authorize options or oauth installer options were provided. ${tokenUsage}`, + `token as well as authorize, orgAuthorize, or oauth installer options were provided. ${tokenUsage}`, ); } this.authorize = singleTeamAuthorization(this.client, { botId, botUserId, botToken: token }); - } else if (authorize === undefined && !usingOauth) { + // Todo: what should we do with orgAuthorize in a singeTeamAuthorize/token provided world? + this.orgAuthorize = singleTeamAuthorization(this.client, { botId, botUserId, botToken: token }); + } else if (authorize === undefined && orgAuthorize === undefined && !usingOauth) { throw new AppInitializationError( - `No token, no authorize options, and no oauth installer options provided. ${tokenUsage}`, + `No token, no authorize, no orgAuthorize, and no oauth installer options provided. ${tokenUsage}`, ); - } else if (authorize !== undefined && usingOauth) { + } else if ((authorize !== undefined || orgAuthorize !== undefined) && usingOauth) { throw new AppInitializationError(`Both authorize options and oauth installer options provided. ${tokenUsage}`); - } else if (authorize === undefined && usingOauth) { + } else if (authorize === undefined && orgAuthorize === undefined && usingOauth) { this.authorize = (this.receiver as ExpressReceiver).installer!.authorize as Authorize; - } else if (authorize !== undefined && !usingOauth) { + this.orgAuthorize = (this.receiver as ExpressReceiver).installer!.orgAuthorize as Authorize; + } else if (authorize === undefined && orgAuthorize !== undefined && !usingOauth) { + this.orgAuthorize = orgAuthorize; + } else if (authorize !== undefined && orgAuthorize === undefined && !usingOauth) { + this.authorize = authorize; + } else if (authorize !== undefined && orgAuthorize !== undefined && !usingOauth) { this.authorize = authorize; + this.orgAuthorize = orgAuthorize; } else { this.logger.error('Never should have reached this point, please report to the team'); assertNever(); @@ -523,15 +545,29 @@ export default class App { // From this point on, we assume that body is not just a key-value map, but one of the types of bodies we expect const bodyArg = body as AnyMiddlewareArgs['body']; - // Initialize context (shallow copy to enforce object identity separation) - const source = buildSource(type, conversationId, bodyArg); let authorizeResult; - - try { - authorizeResult = await this.authorize(source, bodyArg); - } catch (error) { - this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); - return this.handleError(error); + let source; + if (bodyArg.is_enterprise_install) { + // This is an org app + // Initialize context (shallow copy to enforce object identity separation) + source = buildSource(type, conversationId, bodyArg); + + try { + authorizeResult = await this.orgAuthorize(source, bodyArg); + } catch (error) { + this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); + return this.handleError(error); + } + } else { + // Initialize context (shallow copy to enforce object identity separation) + source = buildSource(type, conversationId, bodyArg); + + try { + authorizeResult = await this.authorize(source, bodyArg); + } catch (error) { + this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); + return this.handleError(error); + } } const context: Context = { ...authorizeResult }; @@ -631,12 +667,21 @@ export default class App { let { client } = this; const token = selectToken(context); if (token !== undefined) { - let pool = this.clients[source.teamId!]; - if (pool === undefined) { - // eslint-disable-next-line no-multi-assign - pool = this.clients[source.teamId!] = new WebClientPool(); + let pool; + if (source.teamId !== undefined) { + pool = this.clients[source.teamId]; + if (pool === undefined) { + pool = this.clients[source.teamId] = new WebClientPool(); + } + } else if (source.enterpriseId !== undefined) { + pool = this.clients[source.enterpriseId]; + if (pool === undefined) { + pool = this.clients[source.enterpriseId] = new WebClientPool(); + } + } + if (pool !== undefined) { + client = pool.getOrCreate(token, this.clientOptions); } - client = pool.getOrCreate(token, this.clientOptions); } // Dispatch event through the global middleware chain @@ -708,88 +753,170 @@ function buildSource( type: IncomingEventType, channelId: string | undefined, body: AnyMiddlewareArgs['body'], -): AuthorizeSourceData { +): AuthorizeSourceData | OrgAuthorizeSourceData { // NOTE: potentially something that can be optimized, so that each of these conditions isn't evaluated more than once. // if this makes it prettier, great! but we should probably check perf before committing to any specific optimization. + let source: AuthorizeSourceData | OrgAuthorizeSourceData; + // let source: OrgAuthorizeSourceData; // tslint:disable:max-line-length - const source: AuthorizeSourceData = { - teamId: - type === IncomingEventType.Event || type === IncomingEventType.Command - ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.team !== null - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).team!.id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.user !== undefined - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).user.team_id as string) - : undefined, - enterpriseId: - type === IncomingEventType.Event || type === IncomingEventType.Command - ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) - : (type === IncomingEventType.Action || + if (body.is_enterprise_install) { + source = { + teamId: + type === IncomingEventType.Event || type === IncomingEventType.Command + ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.user !== undefined + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).user.team_id as string) + : undefined, + enterpriseId: + type === IncomingEventType.Event || type === IncomingEventType.Command + ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.enterprise_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.enterprise !== undefined + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).enterprise!.id as string) + : '', // TODO: empty string for enterpriseID is wrong, should be assertNever + userId: + type === IncomingEventType.Event + ? typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'string' + ? ((body as SlackEventMiddlewareArgs['body']).event.user as string) + : typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'object' + ? ((body as SlackEventMiddlewareArgs['body']).event.user.id as string) + : (body as SlackEventMiddlewareArgs['body']).event.channel !== undefined && + (body as SlackEventMiddlewareArgs['body']).event.channel.creator !== undefined + ? ((body as SlackEventMiddlewareArgs['body']).event.channel.creator as string) + : (body as SlackEventMiddlewareArgs['body']).event.subteam !== undefined && + (body as SlackEventMiddlewareArgs['body']).event.subteam.created_by !== undefined + ? ((body as SlackEventMiddlewareArgs['body']).event.subteam.created_by as string) + : undefined + : type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.team !== null - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).team!.enterprise_id as string) - : (type === IncomingEventType.Action || + type === IncomingEventType.Shortcut + ? ((body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).user + .id as string) + : type === IncomingEventType.Command + ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) + : undefined, + conversationId: channelId, + }; + } else { + source = { + teamId: + type === IncomingEventType.Event || type === IncomingEventType.Command + ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.user !== undefined + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).user.team_id as string) + : '', // TODO: empty string for teamID is wrong, should be assertNever + enterpriseId: + type === IncomingEventType.Event || type === IncomingEventType.Command + ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.team !== null + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).team!.enterprise_id as string) + : (type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut) && + body.enterprise !== undefined + ? ((body as ( + | SlackActionMiddlewareArgs + | SlackOptionsMiddlewareArgs + | SlackViewMiddlewareArgs + | SlackShortcutMiddlewareArgs + )['body']).enterprise!.id as string) + : undefined, + userId: + type === IncomingEventType.Event + ? typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'string' + ? ((body as SlackEventMiddlewareArgs['body']).event.user as string) + : typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'object' + ? ((body as SlackEventMiddlewareArgs['body']).event.user.id as string) + : (body as SlackEventMiddlewareArgs['body']).event.channel !== undefined && + (body as SlackEventMiddlewareArgs['body']).event.channel.creator !== undefined + ? ((body as SlackEventMiddlewareArgs['body']).event.channel.creator as string) + : (body as SlackEventMiddlewareArgs['body']).event.subteam !== undefined && + (body as SlackEventMiddlewareArgs['body']).event.subteam.created_by !== undefined + ? ((body as SlackEventMiddlewareArgs['body']).event.subteam.created_by as string) + : undefined + : type === IncomingEventType.Action || type === IncomingEventType.Options || type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.enterprise !== undefined - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).enterprise!.id as string) - : undefined, - userId: - type === IncomingEventType.Event - ? typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'string' - ? ((body as SlackEventMiddlewareArgs['body']).event.user as string) - : typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'object' - ? ((body as SlackEventMiddlewareArgs['body']).event.user.id as string) - : (body as SlackEventMiddlewareArgs['body']).event.channel !== undefined && - (body as SlackEventMiddlewareArgs['body']).event.channel.creator !== undefined - ? ((body as SlackEventMiddlewareArgs['body']).event.channel.creator as string) - : (body as SlackEventMiddlewareArgs['body']).event.subteam !== undefined && - (body as SlackEventMiddlewareArgs['body']).event.subteam.created_by !== undefined - ? ((body as SlackEventMiddlewareArgs['body']).event.subteam.created_by as string) - : undefined - : type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut - ? ((body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).user - .id as string) - : type === IncomingEventType.Command - ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) - : undefined, - conversationId: channelId, - }; + type === IncomingEventType.Shortcut + ? ((body as (SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs)['body']).user + .id as string) + : type === IncomingEventType.Command + ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) + : undefined, + conversationId: channelId, + }; + } // tslint:enable:max-line-length return source; From e4a961cfaee119101fb2fc8ec82b391755a98c58 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Thu, 30 Jul 2020 17:02:00 -0700 Subject: [PATCH 04/13] fixed type issue --- src/App.ts | 45 +++++++++++---------------------------------- 1 file changed, 11 insertions(+), 34 deletions(-) diff --git a/src/App.ts b/src/App.ts index 5685c3a97..3eb8473c5 100644 --- a/src/App.ts +++ b/src/App.ts @@ -788,32 +788,21 @@ function buildSource( | SlackShortcutMiddlewareArgs )['body']).user.team_id as string) : undefined, + // TODO: double check payloads for event and command below enterpriseId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.team !== null - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).team!.enterprise_id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.enterprise !== undefined + : type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut ? ((body as ( | SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs | SlackShortcutMiddlewareArgs )['body']).enterprise!.id as string) - : '', // TODO: empty string for enterpriseID is wrong, should be assertNever + : assertNever(type), userId: type === IncomingEventType.Event ? typeof (body as SlackEventMiddlewareArgs['body']).event.user === 'string' @@ -843,29 +832,17 @@ function buildSource( teamId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.team !== null + : type === IncomingEventType.Action || + type === IncomingEventType.Options || + type === IncomingEventType.ViewAction || + type === IncomingEventType.Shortcut ? ((body as ( | SlackActionMiddlewareArgs | SlackOptionsMiddlewareArgs | SlackViewMiddlewareArgs | SlackShortcutMiddlewareArgs )['body']).team!.id as string) - : (type === IncomingEventType.Action || - type === IncomingEventType.Options || - type === IncomingEventType.ViewAction || - type === IncomingEventType.Shortcut) && - body.user !== undefined - ? ((body as ( - | SlackActionMiddlewareArgs - | SlackOptionsMiddlewareArgs - | SlackViewMiddlewareArgs - | SlackShortcutMiddlewareArgs - )['body']).user.team_id as string) - : '', // TODO: empty string for teamID is wrong, should be assertNever + : assertNever(type), // TODO: empty string for teamID is wrong, should be assertNever enterpriseId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) From 050ba6730958daee0689703a436be1f02cdc4f52 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 4 Aug 2020 15:10:15 -0700 Subject: [PATCH 05/13] fixed bugs with org app installations --- src/App.ts | 16 ++++++++++++---- src/types/actions/block-action.ts | 1 - src/types/actions/workflow-step-edit.ts | 7 +++++++ 3 files changed, 19 insertions(+), 5 deletions(-) diff --git a/src/App.ts b/src/App.ts index 3eb8473c5..9ebb4ae1c 100644 --- a/src/App.ts +++ b/src/App.ts @@ -297,10 +297,13 @@ export default class App { this.authorize = (this.receiver as ExpressReceiver).installer!.authorize as Authorize; this.orgAuthorize = (this.receiver as ExpressReceiver).installer!.orgAuthorize as Authorize; } else if (authorize === undefined && orgAuthorize !== undefined && !usingOauth) { + // only supporting org installs this.orgAuthorize = orgAuthorize; } else if (authorize !== undefined && orgAuthorize === undefined && !usingOauth) { + // only supporting non org installs this.authorize = authorize; } else if (authorize !== undefined && orgAuthorize !== undefined && !usingOauth) { + // supporting both org installs and non org installs this.authorize = authorize; this.orgAuthorize = orgAuthorize; } else { @@ -547,7 +550,12 @@ export default class App { let authorizeResult; let source; - if (bodyArg.is_enterprise_install) { + if ( + type === IncomingEventType.Event && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0] !== undefined && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0].is_enterprise_install + ) { // This is an org app // Initialize context (shallow copy to enforce object identity separation) source = buildSource(type, conversationId, bodyArg); @@ -671,11 +679,13 @@ export default class App { if (source.teamId !== undefined) { pool = this.clients[source.teamId]; if (pool === undefined) { + // eslint-disable-next-line no-multi-assign pool = this.clients[source.teamId] = new WebClientPool(); } } else if (source.enterpriseId !== undefined) { pool = this.clients[source.enterpriseId]; if (pool === undefined) { + // eslint-disable-next-line no-multi-assign pool = this.clients[source.enterpriseId] = new WebClientPool(); } } @@ -758,7 +768,6 @@ function buildSource( // if this makes it prettier, great! but we should probably check perf before committing to any specific optimization. let source: AuthorizeSourceData | OrgAuthorizeSourceData; - // let source: OrgAuthorizeSourceData; // tslint:disable:max-line-length if (body.is_enterprise_install) { source = { @@ -842,7 +851,7 @@ function buildSource( | SlackViewMiddlewareArgs | SlackShortcutMiddlewareArgs )['body']).team!.id as string) - : assertNever(type), // TODO: empty string for teamID is wrong, should be assertNever + : assertNever(type), enterpriseId: type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) @@ -895,7 +904,6 @@ function buildSource( }; } // tslint:enable:max-line-length - return source; } diff --git a/src/types/actions/block-action.ts b/src/types/actions/block-action.ts index 55ecf5843..e78c8327a 100644 --- a/src/types/actions/block-action.ts +++ b/src/types/actions/block-action.ts @@ -244,7 +244,6 @@ export interface BlockAction Date: Fri, 2 Oct 2020 12:01:56 -0700 Subject: [PATCH 06/13] v2.4.1-orgAppsBeta.3 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 8136510b0..51c281c2b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@slack/bolt", - "version": "2.4.1", + "version": "2.4.1-orgAppsBeta.3", "description": "A framework for building Slack apps, fast.", "author": "Slack Technologies, Inc.", "license": "MIT", From 886e2effa69204a34a260bab9827c2765a4f81cb Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 13 Oct 2020 15:30:00 -0700 Subject: [PATCH 07/13] fixed error with org installation store not being accessed for non event type payloads --- src/App.ts | 12 ++++++++---- src/types/command/index.ts | 2 ++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/App.ts b/src/App.ts index 9ebb4ae1c..74ce33223 100644 --- a/src/App.ts +++ b/src/App.ts @@ -550,11 +550,14 @@ export default class App { let authorizeResult; let source; + // Check if type event with the authorizations object or if it has a top level is_enterprise_install property if ( - type === IncomingEventType.Event && - (bodyArg as SlackEventMiddlewareArgs['body']).authorizations !== undefined && - (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0] !== undefined && - (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0].is_enterprise_install + (type === IncomingEventType.Event && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0] !== undefined && + (bodyArg as SlackEventMiddlewareArgs['body']).authorizations![0].is_enterprise_install) || + bodyArg.is_enterprise_install === true || + bodyArg.is_enterprise_install === 'true' // command payloads have this as a string ) { // This is an org app // Initialize context (shallow copy to enforce object identity separation) @@ -567,6 +570,7 @@ export default class App { return this.handleError(error); } } else { + // This is not an org app // Initialize context (shallow copy to enforce object identity separation) source = buildSource(type, conversationId, bodyArg); diff --git a/src/types/command/index.ts b/src/types/command/index.ts index 808f4074c..ebd35b4c6 100644 --- a/src/types/command/index.ts +++ b/src/types/command/index.ts @@ -33,4 +33,6 @@ export interface SlashCommand extends StringIndexed { api_app_id: string; enterprise_id?: string; enterprise_name?: string; + // exists for enterprise installs + is_enterprise_install?: string; // This should be a boolean, but payload for commands gives string 'true' } From d8b715e583ea3b2745c3d4d762e478cfd3f57a2e Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 13 Oct 2020 16:29:15 -0700 Subject: [PATCH 08/13] v2.4.1-orgAppsBeta.4 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 51c281c2b..7b5be9d1f 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@slack/bolt", - "version": "2.4.1-orgAppsBeta.3", + "version": "2.4.1-orgAppsBeta.4", "description": "A framework for building Slack apps, fast.", "author": "Slack Technologies, Inc.", "license": "MIT", From 4d676279f873402435a8e001f55131a1229c99cd Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Fri, 13 Nov 2020 11:42:44 -0800 Subject: [PATCH 09/13] passed team_id to web-api --- src/App.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/App.ts b/src/App.ts index 74ce33223..1c2ec3d0d 100644 --- a/src/App.ts +++ b/src/App.ts @@ -680,12 +680,14 @@ export default class App { const token = selectToken(context); if (token !== undefined) { let pool; + const clientOptionsCopy = { ...this.clientOptions }; if (source.teamId !== undefined) { pool = this.clients[source.teamId]; if (pool === undefined) { // eslint-disable-next-line no-multi-assign pool = this.clients[source.teamId] = new WebClientPool(); } + clientOptionsCopy.teamId = source.teamId; } else if (source.enterpriseId !== undefined) { pool = this.clients[source.enterpriseId]; if (pool === undefined) { @@ -694,7 +696,8 @@ export default class App { } } if (pool !== undefined) { - client = pool.getOrCreate(token, this.clientOptions); + // Question: should teamId from source or authResult be passed in via clientOptionsCopy + client = pool.getOrCreate(token, clientOptionsCopy); } } From 2bd7bdb3891556d5fefada380d77e35c907ba845 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Fri, 13 Nov 2020 16:34:07 -0800 Subject: [PATCH 10/13] added org app tests --- src/App.spec.ts | 428 ++++++++++++++++++++++++++++++++++++++++++++++++ src/App.ts | 3 + 2 files changed, 431 insertions(+) diff --git a/src/App.spec.ts b/src/App.spec.ts index be71c2074..dc77c0df4 100644 --- a/src/App.spec.ts +++ b/src/App.spec.ts @@ -68,6 +68,32 @@ describe('App', () => { assert(authorizeCallback.notCalled, 'Should not call the authorize callback on instantiation'); assert.instanceOf(app, App); }); + it('should succeed with an orgAuthorize callback', async () => { + // Arrange + const authorizeCallback = sinon.fake(); + const App = await importApp(); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + + // Act + const app = new App({ orgAuthorize: authorizeCallback, signingSecret: '' }); + + // Assert + assert(authorizeCallback.notCalled, 'Should not call the orgAuthorize callback on instantiation'); + assert.instanceOf(app, App); + }); + it('should succeed with an authorize and orgAuthorize callback', async () => { + // Arrange + const authorizeCallback = sinon.fake(); + const orgAuthorizeCallback = sinon.fake(); + const App = await importApp(); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + + // Act + const app = new App({ orgAuthorize: orgAuthorizeCallback, authorize: authorizeCallback, signingSecret: '' }); + + // Assert + assert(authorizeCallback.notCalled, 'Should not call the authorize callback on instantiation'); + assert(orgAuthorizeCallback.notCalled, 'Should not call the orgAuthorize callback on instantiation'); + assert.instanceOf(app, App); + }); it('should fail without a token for single team authorization or authorize callback or oauth installer', async () => { // Arrange const App = await importApp(); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match @@ -97,6 +123,22 @@ describe('App', () => { assert(authorizeCallback.notCalled); } }); + it('should fail when both a token and orgAuthorize callback are specified', async () => { + // Arrange + const authorizeCallback = sinon.fake(); + const App = await importApp(); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + + // Act + try { + // eslint-disable-line @typescript-eslint/no-unused-expressions + new App({ token: '', orgAuthorize: authorizeCallback, signingSecret: '' }); + assert.fail(); + } catch (error) { + // Assert + assert.propertyVal(error, 'code', ErrorCode.AppInitializationError); + assert(authorizeCallback.notCalled); + } + }); it('should fail when both a token is specified and OAuthInstaller is initialized', async () => { // Arrange const authorizeCallback = sinon.fake(); @@ -129,6 +171,28 @@ describe('App', () => { assert(authorizeCallback.notCalled); } }); + it('should fail when both a orgAuthorize callback is specified and OAuthInstaller is initialized', async () => { + // Arrange + const authorizeCallback = sinon.fake(); + const App = await importApp(); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + + // Act + try { + // eslint-disable-line @typescript-eslint/no-unused-expressions + new App({ + orgAuthorize: authorizeCallback, + clientId: '', + clientSecret: '', + stateSecret: '', + signingSecret: '', + }); + assert.fail(); + } catch (error) { + // Assert + assert.propertyVal(error, 'code', ErrorCode.AppInitializationError); + assert(authorizeCallback.notCalled); + } + }); describe('with a custom receiver', () => { it('should succeed with no signing secret', async () => { // Arrange @@ -770,6 +834,225 @@ describe('App', () => { ]; } + function createOrgAppReceiverEvents(): ReceiverEvent[] { + return [ + { + // IncomingEventType.Event (app.event) + ...baseEvent, + body: { + event: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Command (app.command) + ...baseEvent, + body: { + command: '/COMMAND_NAME', + is_enterprise_install: 'true', + enterprise_id: 'E12345678', + }, + }, + { + // IncomingEventType.Action (app.action) + ...baseEvent, + body: { + type: 'block_actions', + actions: [ + { + action_id: 'block_action_id', + }, + ], + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Shortcut (app.shortcut) + ...baseEvent, + body: { + type: 'message_action', + callback_id: 'message_action_callback_id', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Shortcut (app.shortcut) + ...baseEvent, + body: { + type: 'message_action', + callback_id: 'another_message_action_callback_id', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Shortcut (app.shortcut) + ...baseEvent, + body: { + type: 'shortcut', + callback_id: 'shortcut_callback_id', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Shortcut (app.shortcut) + ...baseEvent, + body: { + type: 'shortcut', + callback_id: 'another_shortcut_callback_id', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Action (app.action) + ...baseEvent, + body: { + type: 'interactive_message', + callback_id: 'interactive_message_callback_id', + actions: [{}], + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Action with dialog submission (app.action) + ...baseEvent, + body: { + type: 'dialog_submission', + callback_id: 'dialog_submission_callback_id', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Action for an external_select block (app.options) + ...baseEvent, + body: { + type: 'block_suggestion', + action_id: 'external_select_action_id', + channel: {}, + user: {}, + team: {}, + actions: [], + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.Action for "data_source": "external" in dialogs (app.options) + ...baseEvent, + body: { + type: 'dialog_suggestion', + callback_id: 'dialog_suggestion_callback_id', + name: 'the name', + channel: {}, + user: {}, + team: {}, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + // IncomingEventType.ViewSubmitAction (app.view) + ...baseEvent, + body: { + type: 'view_submission', + channel: {}, + user: {}, + team: {}, + view: { + callback_id: 'view_callback_id', + }, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + ...baseEvent, + body: { + type: 'view_closed', + channel: {}, + user: {}, + team: {}, + view: { + callback_id: 'view_callback_id', + }, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + { + ...baseEvent, + body: { + type: 'event_callback', + token: 'XXYYZZ', + team_id: 'TXXXXXXXX', + api_app_id: 'AXXXXXXXXX', + event: { + type: 'message', + event_ts: '1234567890.123456', + user: 'UXXXXXXX1', + text: 'hello friends!', + }, + is_enterprise_install: true, + enterprise: { + id: 'E12345678', + }, + }, + }, + ]; + } + it('should acknowledge any of possible events', async () => { // Arrange const ackFn = sinon.fake.resolves({}); @@ -877,6 +1160,151 @@ describe('App', () => { assert.equal(ackFn.callCount, dummyReceiverEvents.length); assert(fakeErrorHandler.notCalled); }); + + // This test confirms orgAuthorize is being used for org events + it('should acknowledge any of possible org events', async () => { + // Arrange + const ackFn = sinon.fake.resolves({}); + const actionFn = sinon.fake.resolves({}); + const shortcutFn = sinon.fake.resolves({}); + const viewFn = sinon.fake.resolves({}); + const optionsFn = sinon.fake.resolves({}); + const overrides = buildOverrides([withNoopWebClient()]); + const App = await importApp(overrides); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + const dummyReceiverEvents = createOrgAppReceiverEvents(); + + // Act + const fakeLogger = createFakeLogger(); + const app = new App({ + logger: fakeLogger, + receiver: fakeReceiver, + orgAuthorize: sinon.fake.resolves(dummyAuthorizationResult), + }); + + app.use(async ({ next }) => { + await ackFn(); + await next!(); + }); + app.shortcut({ callback_id: 'message_action_callback_id' }, async ({}) => { + await shortcutFn(); + }); + app.shortcut({ type: 'message_action', callback_id: 'another_message_action_callback_id' }, async ({}) => { + await shortcutFn(); + }); + app.shortcut({ type: 'message_action', callback_id: 'does_not_exist' }, async ({}) => { + await shortcutFn(); + }); + app.shortcut({ callback_id: 'shortcut_callback_id' }, async ({}) => { + await shortcutFn(); + }); + app.shortcut({ type: 'shortcut', callback_id: 'another_shortcut_callback_id' }, async ({}) => { + await shortcutFn(); + }); + app.shortcut({ type: 'shortcut', callback_id: 'does_not_exist' }, async ({}) => { + await shortcutFn(); + }); + app.action('block_action_id', async ({}) => { + await actionFn(); + }); + app.action({ callback_id: 'interactive_message_callback_id' }, async ({}) => { + await actionFn(); + }); + app.action({ callback_id: 'dialog_submission_callback_id' }, async ({}) => { + await actionFn(); + }); + app.view('view_callback_id', async ({}) => { + await viewFn(); + }); + app.view({ callback_id: 'view_callback_id', type: 'view_closed' }, async ({}) => { + await viewFn(); + }); + app.options('external_select_action_id', async ({}) => { + await optionsFn(); + }); + app.options({ callback_id: 'dialog_suggestion_callback_id' }, async ({}) => { + await optionsFn(); + }); + + app.event('app_home_opened', async ({}) => { + /* noop */ + }); + app.message('hello', async ({}) => { + /* noop */ + }); + app.command('/echo', async ({}) => { + /* noop */ + }); + + // invalid view constraints + const invalidViewConstraints1 = ({ + callback_id: 'foo', + type: 'view_submission', + unknown_key: 'should be detected', + } as any) as ViewConstraints; + app.view(invalidViewConstraints1, async ({}) => { + /* noop */ + }); + assert.isTrue(fakeLogger.error.called); + + fakeLogger.error = sinon.fake(); + + const invalidViewConstraints2 = ({ + callback_id: 'foo', + type: undefined, + unknown_key: 'should be detected', + } as any) as ViewConstraints; + app.view(invalidViewConstraints2, async ({}) => { + /* noop */ + }); + assert.isTrue(fakeLogger.error.called); + + app.error(fakeErrorHandler); + await Promise.all(dummyReceiverEvents.map((event) => fakeReceiver.sendEvent(event))); + + // Assert + assert.equal(actionFn.callCount, 3); + assert.equal(shortcutFn.callCount, 4); + assert.equal(viewFn.callCount, 2); + assert.equal(optionsFn.callCount, 2); + assert.equal(ackFn.callCount, dummyReceiverEvents.length); + assert(fakeErrorHandler.notCalled); + }); + + it('should fail because no orgAuthorize was defined to handle org install events', async () => { + // Arrange + const ackFn = sinon.fake.resolves({}); + const actionFn = sinon.fake.resolves({}); + const overrides = buildOverrides([withNoopWebClient()]); + const App = await importApp(overrides); // eslint-disable-line @typescript-eslint/naming-convention, no-underscore-dangle, id-blacklist, id-match + const dummyReceiverEvents = createOrgAppReceiverEvents(); + + // Act + const fakeLogger = createFakeLogger(); + // only passed in authorize and not orgAuthorize + const app = new App({ + logger: fakeLogger, + receiver: fakeReceiver, + authorize: sinon.fake.resolves(dummyAuthorizationResult), + }); + + app.use(async ({ next }) => { + await ackFn(); + await next!(); + }); + app.action('block_action_id', async ({}) => { + await actionFn(); + }); + + app.error(fakeErrorHandler); + await Promise.all(dummyReceiverEvents.map((event) => fakeReceiver.sendEvent(event))); + + // Assert + assert.equal(actionFn.callCount, 0); + assert.equal(ackFn.callCount, 0); + assert.equal(fakeErrorHandler.callCount, dummyReceiverEvents.length); + assert.instanceOf(fakeErrorHandler.firstCall.args[0], Error); + assert.propertyVal(fakeErrorHandler.firstCall.args[0], 'code', ErrorCode.AuthorizationError); + }); }); describe('respond()', () => { diff --git a/src/App.ts b/src/App.ts index 1c2ec3d0d..ad98f727b 100644 --- a/src/App.ts +++ b/src/App.ts @@ -156,6 +156,7 @@ export default class App { private clientOptions: WebClientOptions; + // Some payloads don't have teamId anymore. So we use EnterpriseId in those scenarios private clients: { [teamOrEnterpriseId: string]: WebClientPool } = {}; /** Receiver - ingests events from the Slack platform */ @@ -567,6 +568,7 @@ export default class App { authorizeResult = await this.orgAuthorize(source, bodyArg); } catch (error) { this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); + error.code = 'slack_bolt_authorization_error'; return this.handleError(error); } } else { @@ -578,6 +580,7 @@ export default class App { authorizeResult = await this.authorize(source, bodyArg); } catch (error) { this.logger.warn('Authorization of incoming event did not succeed. No listeners will be called.'); + error.code = 'slack_bolt_authorization_error'; return this.handleError(error); } } From 8f9c40cdd9e8e2a7d5ab2be600b1f7b1ea8ac2d7 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Mon, 23 Nov 2020 15:54:38 -0800 Subject: [PATCH 11/13] added enterpriseId to authorizeResult, removed oauth orgAuthorize --- src/App.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/App.ts b/src/App.ts index ad98f727b..bd08334a6 100644 --- a/src/App.ts +++ b/src/App.ts @@ -91,6 +91,7 @@ export interface AuthorizeSourceData { enterpriseId?: string; userId?: string; conversationId?: string; + isEnterpriseInstall?: boolean; } /** Authorization function inputs - authenticated data about an event for the authorization function */ @@ -99,6 +100,7 @@ export interface OrgAuthorizeSourceData { teamId?: string; userId?: string; conversationId?: string; + isEnterpriseInstall?: boolean; } /** Authorization function outputs - data that will be available as part of event processing */ @@ -109,6 +111,7 @@ export interface AuthorizeResult { botId?: string; // required for `ignoreSelf` global middleware botUserId?: string; // optional but allows `ignoreSelf` global middleware be more filter more than just message events teamId?: string; + enterpriseId?: string; [key: string]: any; } @@ -296,7 +299,7 @@ export default class App { throw new AppInitializationError(`Both authorize options and oauth installer options provided. ${tokenUsage}`); } else if (authorize === undefined && orgAuthorize === undefined && usingOauth) { this.authorize = (this.receiver as ExpressReceiver).installer!.authorize as Authorize; - this.orgAuthorize = (this.receiver as ExpressReceiver).installer!.orgAuthorize as Authorize; + this.orgAuthorize = (this.receiver as ExpressReceiver).installer!.authorize as Authorize; } else if (authorize === undefined && orgAuthorize !== undefined && !usingOauth) { // only supporting org installs this.orgAuthorize = orgAuthorize; @@ -845,6 +848,7 @@ function buildSource( ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) : undefined, conversationId: channelId, + isEnterpriseInstall: body.is_enterprise_install, }; } else { source = { @@ -911,6 +915,7 @@ function buildSource( ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) : undefined, conversationId: channelId, + isEnterpriseInstall: false, }; } // tslint:enable:max-line-length From 2ecb016634ce18740c122a89dad49f09c48b53e3 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 24 Nov 2020 12:25:00 -0800 Subject: [PATCH 12/13] fixed #687 by extracting team_id and enterprise_id from authorizations --- src/App.ts | 38 +++++++++++++++++++++++++++++--------- 1 file changed, 29 insertions(+), 9 deletions(-) diff --git a/src/App.ts b/src/App.ts index bd08334a6..4c289a1aa 100644 --- a/src/App.ts +++ b/src/App.ts @@ -565,7 +565,7 @@ export default class App { ) { // This is an org app // Initialize context (shallow copy to enforce object identity separation) - source = buildSource(type, conversationId, bodyArg); + source = buildSource(type, conversationId, bodyArg, true); try { authorizeResult = await this.orgAuthorize(source, bodyArg); @@ -577,7 +577,7 @@ export default class App { } else { // This is not an org app // Initialize context (shallow copy to enforce object identity separation) - source = buildSource(type, conversationId, bodyArg); + source = buildSource(type, conversationId, bodyArg, false); try { authorizeResult = await this.authorize(source, bodyArg); @@ -776,16 +776,21 @@ function buildSource( type: IncomingEventType, channelId: string | undefined, body: AnyMiddlewareArgs['body'], + orgInstall: boolean, ): AuthorizeSourceData | OrgAuthorizeSourceData { // NOTE: potentially something that can be optimized, so that each of these conditions isn't evaluated more than once. // if this makes it prettier, great! but we should probably check perf before committing to any specific optimization. - let source: AuthorizeSourceData | OrgAuthorizeSourceData; // tslint:disable:max-line-length - if (body.is_enterprise_install) { + if (orgInstall) { source = { teamId: - type === IncomingEventType.Event || type === IncomingEventType.Command + type === IncomingEventType.Event && + (body as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (body as SlackEventMiddlewareArgs['body']).authorizations!.length > 0 && + (body as SlackEventMiddlewareArgs['body']).authorizations![0].team_id !== null + ? ((body as SlackEventMiddlewareArgs['body']).authorizations![0].team_id as string) + : type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) : (type === IncomingEventType.Action || type === IncomingEventType.Options || @@ -812,7 +817,12 @@ function buildSource( : undefined, // TODO: double check payloads for event and command below enterpriseId: - type === IncomingEventType.Event || type === IncomingEventType.Command + type === IncomingEventType.Event && + (body as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (body as SlackEventMiddlewareArgs['body']).authorizations!.length > 0 && + (body as SlackEventMiddlewareArgs['body']).authorizations![0].enterprise_id !== null + ? ((body as SlackEventMiddlewareArgs['body']).authorizations![0].enterprise_id as string) + : type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) : type === IncomingEventType.Action || type === IncomingEventType.Options || @@ -848,12 +858,17 @@ function buildSource( ? ((body as SlackCommandMiddlewareArgs['body']).user_id as string) : undefined, conversationId: channelId, - isEnterpriseInstall: body.is_enterprise_install, + isEnterpriseInstall: true, }; } else { source = { teamId: - type === IncomingEventType.Event || type === IncomingEventType.Command + type === IncomingEventType.Event && + (body as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (body as SlackEventMiddlewareArgs['body']).authorizations!.length > 0 && + (body as SlackEventMiddlewareArgs['body']).authorizations![0].team_id !== null + ? ((body as SlackEventMiddlewareArgs['body']).authorizations![0].team_id as string) + : type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).team_id as string) : type === IncomingEventType.Action || type === IncomingEventType.Options || @@ -867,7 +882,12 @@ function buildSource( )['body']).team!.id as string) : assertNever(type), enterpriseId: - type === IncomingEventType.Event || type === IncomingEventType.Command + type === IncomingEventType.Event && + (body as SlackEventMiddlewareArgs['body']).authorizations !== undefined && + (body as SlackEventMiddlewareArgs['body']).authorizations!.length > 0 && + (body as SlackEventMiddlewareArgs['body']).authorizations![0].enterprise_id !== null + ? ((body as SlackEventMiddlewareArgs['body']).authorizations![0].enterprise_id as string) + : type === IncomingEventType.Event || type === IncomingEventType.Command ? ((body as (SlackEventMiddlewareArgs | SlackCommandMiddlewareArgs)['body']).enterprise_id as string) : (type === IncomingEventType.Action || type === IncomingEventType.Options || From 8a147451f476876f67ed57a62c9b4b08aadce8b1 Mon Sep 17 00:00:00 2001 From: Steve Gill Date: Tue, 24 Nov 2020 12:41:58 -0800 Subject: [PATCH 13/13] updated webclientPool logic to use teamId & enterpriseId from authResult first --- src/App.ts | 33 +++++++++++++++++++++++++-------- 1 file changed, 25 insertions(+), 8 deletions(-) diff --git a/src/App.ts b/src/App.ts index 4c289a1aa..2f5fc01d7 100644 --- a/src/App.ts +++ b/src/App.ts @@ -684,25 +684,42 @@ export default class App { // Get the client arg let { client } = this; const token = selectToken(context); + let teamId; + let enterpriseId; + + // Try to set teamId from AuthorizeResult before using one from source + if (authorizeResult.teamId !== undefined) { + teamId = authorizeResult.teamId; + } else if (source.teamId !== undefined) { + teamId = source.teamId; + } + + // Try to set enterpriseId from AuthorizeResult before using one from source + if (authorizeResult.enterpriseId !== undefined) { + enterpriseId = authorizeResult.enterpriseId; + } else if (source.enterpriseId !== undefined) { + enterpriseId = source.enterpriseId; + } + if (token !== undefined) { let pool; const clientOptionsCopy = { ...this.clientOptions }; - if (source.teamId !== undefined) { - pool = this.clients[source.teamId]; + if (teamId !== undefined) { + pool = this.clients[teamId]; if (pool === undefined) { // eslint-disable-next-line no-multi-assign - pool = this.clients[source.teamId] = new WebClientPool(); + pool = this.clients[teamId] = new WebClientPool(); } - clientOptionsCopy.teamId = source.teamId; - } else if (source.enterpriseId !== undefined) { - pool = this.clients[source.enterpriseId]; + // Add teamId to clientOptions so it can be automatically added to web-api calls + clientOptionsCopy.teamId = teamId; + } else if (enterpriseId !== undefined) { + pool = this.clients[enterpriseId]; if (pool === undefined) { // eslint-disable-next-line no-multi-assign - pool = this.clients[source.enterpriseId] = new WebClientPool(); + pool = this.clients[enterpriseId] = new WebClientPool(); } } if (pool !== undefined) { - // Question: should teamId from source or authResult be passed in via clientOptionsCopy client = pool.getOrCreate(token, clientOptionsCopy); } }