fix: Interactions from private channels not working (#4688)

* i18n fix

* grammar fix

* impl for /commands

* impl IChannel

* checkPermission fix

* impl

* Update admin.json

* fb fix wip

* fb fix

* fb fix

* fb fix

* boolean fix

* whitespace

* fix

Co-authored-by: Yuki Takei <yuki@weseek.co.jp>

packages/app/resource/locales/en_US/admin/admin.json

@@ -342,7 +342,7 @@
       "manage_commands": "Manage GROWI commands",
       "multiple_growi_command": "Commands that could be sent to multiple GROWI instances at once",
       "single_growi_command": "Commands that could be sent to single GROWI instance at a time",
-      "allowed_channels_description": "Input allowed channels for \"{{commandName}}\" command. Separate each channel with \",\" . Users can will be able to use \"{{commandName}}\" command from channels written here.",
+      "allowed_channels_description": "Enter the allowed channel ID or channel name for \"{{commandName}}\" command. Only channel IDs are allowed for private channels. Separate each channel with \",\" . Users will be able to use \"{{commandName}}\" command from channels written here.",
       "allow_all": "Allow all",
       "deny_all": "Deny all",
       "allow_specified": "Allow specified",

packages/app/resource/locales/ja_JP/admin/admin.json

@@ -341,7 +341,7 @@
       "manage_commands": "使用可能なGROWIコマンドを設定する",
       "multiple_growi_command": "複数のGROWIに対して送信できるコマンド",
       "single_growi_command": "一つのGROWIに対して送信できるコマンド",
-      "allowed_channels_description": "\"{{commandName}}\" コマンドの使用を許可するチャンネルを \",\" 区切りで入力してください。ユーザーはここに記入されているチャンネルから \"{{commandName}}\" コマンドを使用することができます。",
+      "allowed_channels_description": "\"{{commandName}}\" コマンドの使用を許可するチャンネルの ID またはチャンネル名を \",\" 区切りで入力してください。プライベートチャンネルの場合はチャンネル ID を入力する必要があります。ユーザーはここに記入されているチャンネルから \"{{commandName}}\" コマンドを使用することができます。",
       "allow_all": "全てのチャンネルを許可",
       "deny_all": "全てのチャンネルを拒否",
       "allow_specified": "特定のチャンネルを許可",

packages/app/resource/locales/zh_CN/admin/admin.json

@@ -351,7 +351,7 @@
       "manage_commands": "管理 GROWI 命令",
       "multiple_growi_command": "可以一次发送到多个 GROWI 实例的命令",
       "single_growi_command": "可以一次发送到一个 GROWI 实例的命令",
-      "allowed_channels_description": "为 \"{{commandName}}\" 命令输入允许的通道。每个通道之间用 \",\" 隔开。用户可以从这里写入的通道中使用 \"{{commandName}}\"。",
+      "allowed_channels_description": "为 \"{{commandName}}\" 命令输入允许的频道ID或频道名称。对于私人频道，只允许输入频道ID。每个频道之间用\",\"隔开。用户可以从这里写的频道中使用 \"{{commandName}}\" 命令。",
       "allow_all": "允许所有",
       "deny_all": "拒绝所有",
       "allow_specified": "允许指定",

packages/app/src/server/routes/apiv3/slack-integration.js

@@ -95,7 +95,10 @@ module.exports = (crowi) => {
 
     const tokenPtoG = req.headers['x-growi-ptog-tokens'];
     const extractPermissions = await extractPermissionsCommands(tokenPtoG);
-    const fromChannel = req.body.channel_name;
+    const fromChannel = {
+      id: req.body.channel_id,
+      name: req.body.channel_name,
+    };
     const siteUrl = crowi.appService.getSiteUrl();
 
     let commandPermission;
@@ -141,7 +144,7 @@ module.exports = (crowi) => {
 
     const { actionId, callbackId } = interactionPayloadAccessor.getActionIdAndCallbackIdFromPayLoad();
     const callbacIdkOrActionId = callbackId || actionId;
-    const fromChannel = interactionPayloadAccessor.getChannelName();
+    const fromChannel = interactionPayloadAccessor.getChannel();
 
     const tokenPtoG = req.headers['x-growi-ptog-tokens'];
     const extractPermissions = await extractPermissionsCommands(tokenPtoG);

packages/app/src/server/util/slack-integration.ts

@@ -1,9 +1,9 @@
-import { getSupportedGrowiActionsRegExp } from '@growi/slack';
+import { getSupportedGrowiActionsRegExp, IChannelOptionalId } from '@growi/slack';
 
 type CommandPermission = { [key:string]: string[] | boolean }
 
 export const checkPermission = (
-    commandPermission:CommandPermission, commandOrActionIdOrCallbackId:string, fromChannel:string,
+    commandPermission: CommandPermission, commandOrActionIdOrCallbackId: string, fromChannel: IChannelOptionalId,
 ):boolean => {
   let isPermitted = false;
 
@@ -23,9 +23,19 @@ export const checkPermission = (
       isPermitted = true;
       return;
     }
-    if (Array.isArray(permission) && permission.includes(fromChannel)) {
-      isPermitted = true;
-      return;
+
+    if (Array.isArray(permission)) {
+      if (permission.includes(fromChannel.name)) {
+        isPermitted = true;
+        return;
+      }
+
+      if (fromChannel.id == null) return;
+
+      if (permission.includes(fromChannel.id)) {
+        isPermitted = true;
+        return;
+      }
     }
   });
 

packages/slack/src/index.ts

@@ -22,6 +22,7 @@ export const defaultSupportedCommandsNameForSingleUse: string[] = [
   'keep',
 ];
 
+export * from './interfaces/channel';
 export * from './interfaces/growi-command-processor';
 export * from './interfaces/growi-interaction-processor';
 export * from './interfaces/growi-command';

packages/slack/src/interfaces/channel.ts

@@ -0,0 +1,6 @@
+export type IChannel = {
+  id: string,
+  name: string,
+}
+
+export type IChannelOptionalId = Omit<IChannel, 'id'> & Partial<IChannel>;

packages/slack/src/utils/interaction-payload-accessor.ts

@@ -1,5 +1,6 @@
 import assert from 'assert';
 import { IInteractionPayloadAccessor } from '../interfaces/request-from-slack';
+import { IChannel } from '../interfaces/channel';
 import loggerFactory from './logger';
 
 const logger = loggerFactory('@growi/slack:utils:interaction-payload-accessor');
@@ -68,16 +69,15 @@ export class InteractionPayloadAccessor implements IInteractionPayloadAccessor {
     return { actionId, callbackId };
   }
 
-  getChannelName(): string | null {
+  getChannel(): IChannel | null {
     // private_metadata should have the channelName parameter when view_submission
     const privateMetadata = this.getViewPrivateMetaData();
     if (privateMetadata != null && privateMetadata.channelName != null) {
-      return privateMetadata.channelName;
+      throw new Error('PrivateMetaDatas are not implemented after removal of modal from slash commands. Use payload instead.');
     }
-
     const channel = this.payload.channel;
     if (channel != null) {
-      return this.payload.channel.name;
+      return channel;
     }
 
     return null;

packages/slackbot-proxy/src/controllers/slack.ts

@@ -12,7 +12,7 @@ import {
   markdownSectionBlock, GrowiCommand, parseSlashCommand, respondRejectedErrors, generateWebClient,
   InvalidGrowiCommandError, requiredScopes, REQUEST_TIMEOUT_FOR_PTOG,
   parseSlackInteractionRequest, verifySlackRequest,
-  respond, supportedGrowiCommands,
+  respond, supportedGrowiCommands, IChannelOptionalId,
 } from '@growi/slack';
 
 import { Relation } from '~/entities/relation';
@@ -227,16 +227,21 @@ export class SlackCtrl {
     const allowedRelationsForBroadcastUse:Relation[] = [];
     const disallowedGrowiUrls: Set<string> = new Set();
 
+    const channel: IChannelOptionalId = {
+      id: body.channel_id,
+      name: body.channel_name,
+    };
+
     // check permission
     await Promise.all(relations.map(async(relation) => {
       const isSupportedForSingleUse = await this.relationsService.isPermissionsForSingleUseCommands(
-        relation, growiCommand.growiCommandType, body.channel_name,
+        relation, growiCommand.growiCommandType, channel,
       );
 
       let isSupportedForBroadcastUse = false;
       if (!isSupportedForSingleUse) {
         isSupportedForBroadcastUse = await this.relationsService.isPermissionsUseBroadcastCommands(
-          relation, growiCommand.growiCommandType, body.channel_name,
+          relation, growiCommand.growiCommandType, channel,
         );
       }
 
@@ -346,8 +351,13 @@ export class SlackCtrl {
 
     const privateMeta = interactionPayloadAccessor.getViewPrivateMetaData();
 
-    const channelName = interactionPayload.channel?.name || privateMeta?.body?.channel_name || privateMeta?.channelName;
-    const permission = await this.relationsService.checkPermissionForInteractions(relations, actionId, callbackId, channelName);
+    const channelFromMeta = {
+      name: privateMeta?.body?.channel_name || privateMeta?.channelName,
+    };
+
+    const channel: IChannelOptionalId = interactionPayload.channel || channelFromMeta;
+    const permission = await this.relationsService.checkPermissionForInteractions(relations, actionId, callbackId, channel);
+
     const {
       allowedRelations, disallowedGrowiUrls, commandName, rejectedResults,
     } = permission;

packages/slackbot-proxy/src/services/RelationsService.ts

@@ -3,7 +3,7 @@ import { Inject, Service } from '@tsed/di';
 import axios from 'axios';
 import { addHours } from 'date-fns';
 
-import { REQUEST_TIMEOUT_FOR_PTOG, getSupportedGrowiActionsRegExp } from '@growi/slack';
+import { REQUEST_TIMEOUT_FOR_PTOG, getSupportedGrowiActionsRegExp, IChannelOptionalId } from '@growi/slack';
 import { Relation, PermissionSettingsInterface } from '~/entities/relation';
 import { RelationRepository } from '~/repositories/relation';
 
@@ -24,6 +24,7 @@ type CheckEachRelationResult = {
   eachRelationCommandName:string,
 }
 
+
 @Service()
 export class RelationsService {
 
@@ -76,7 +77,7 @@ export class RelationsService {
     return relation;
   }
 
-  private isPermitted(permissionSettings: PermissionSettingsInterface, growiCommandType: string, channelName: string): boolean {
+  private isPermitted(permissionSettings: PermissionSettingsInterface, growiCommandType: string, channel: IChannelOptionalId): boolean {
     // TODO assert (permissionSettings != null)
 
     const permissionForCommand = permissionSettings[growiCommandType];
@@ -86,13 +87,23 @@ export class RelationsService {
     }
 
     if (Array.isArray(permissionForCommand)) {
-      return permissionForCommand.includes(channelName);
+      if (permissionForCommand.includes(channel.name)) {
+        return true;
+      }
+
+      if (channel.id == null) {
+        return false;
+      }
+
+      if (permissionForCommand.includes(channel.id)) {
+        return true;
+      }
     }
+    return permissionForCommand as boolean;
 
-    return permissionForCommand;
   }
 
-  async isPermissionsForSingleUseCommands(relation: Relation, growiCommandType: string, channelName: string): Promise<boolean> {
+  async isPermissionsForSingleUseCommands(relation: Relation, growiCommandType: string, channel: IChannelOptionalId): Promise<boolean> {
     // TODO assert (relation != null)
     if (relation == null) {
       return false;
@@ -110,10 +121,10 @@ export class RelationsService {
 
     // TODO assert (relationToEval.permissionsForSingleUseCommands != null) because syncRelation success
 
-    return this.isPermitted(relationToEval.permissionsForSingleUseCommands, growiCommandType, channelName);
+    return this.isPermitted(relationToEval.permissionsForSingleUseCommands, growiCommandType, channel);
   }
 
-  async isPermissionsUseBroadcastCommands(relation: Relation, growiCommandType: string, channelName: string):Promise<boolean> {
+  async isPermissionsUseBroadcastCommands(relation: Relation, growiCommandType: string, channel: IChannelOptionalId):Promise<boolean> {
     // TODO assert (relation != null)
     if (relation == null) {
       return false;
@@ -131,11 +142,11 @@ export class RelationsService {
 
     // TODO assert (relationToEval.permissionsForSingleUseCommands != null) because syncRelation success
 
-    return this.isPermitted(relationToEval.permissionsForBroadcastUseCommands, growiCommandType, channelName);
+    return this.isPermitted(relationToEval.permissionsForBroadcastUseCommands, growiCommandType, channel);
   }
 
   async checkPermissionForInteractions(
-      relations:Relation[], actionId:string, callbackId:string, channelName:string,
+      relations: Relation[], actionId: string, callbackId: string, channel: IChannelOptionalId,
   ):Promise<CheckPermissionForInteractionsResults> {
 
     const allowedRelations:Relation[] = [];
@@ -143,7 +154,7 @@ export class RelationsService {
     let commandName = '';
 
     const results = await Promise.allSettled(relations.map((relation) => {
-      const relationResult = this.checkEachRelation(relation, actionId, callbackId, channelName);
+      const relationResult = this.checkEachRelation(relation, actionId, callbackId, channel);
       const { allowedRelation, disallowedGrowiUrl, eachRelationCommandName } = relationResult;
 
       if (allowedRelation != null) {
@@ -164,8 +175,7 @@ export class RelationsService {
     };
   }
 
-  checkEachRelation(relation:Relation, actionId:string, callbackId:string, channelName:string):CheckEachRelationResult {
-
+  checkEachRelation(relation:Relation, actionId:string, callbackId:string, channel: IChannelOptionalId): CheckEachRelationResult {
     let allowedRelation:Relation|null = null;
     let disallowedGrowiUrl:string|null = null;
     let eachRelationCommandName = '';
@@ -198,9 +208,18 @@ export class RelationsService {
       }
 
       // check permission at channel level
-      if (Array.isArray(permissionForInteractions) && permissionForInteractions.includes(channelName)) {
-        allowedRelation = relation;
-        return;
+      if (Array.isArray(permissionForInteractions)) {
+        if (permissionForInteractions.includes(channel.name)) {
+          allowedRelation = relation;
+          return;
+        }
+
+        if (channel.id == null) return;
+
+        if (permissionForInteractions.includes(channel.id)) {
+          allowedRelation = relation;
+          return;
+        }
       }
 
       disallowedGrowiUrl = relation.growiUri;

packages/slackbot-proxy/src/services/SelectGrowiService.ts

@@ -28,6 +28,8 @@ type SendCommandBody = {
   // eslint-disable-next-line camelcase
   trigger_id: string,
   // eslint-disable-next-line camelcase
+  channel_id: string,
+  // eslint-disable-next-line camelcase
   channel_name: string,
 }
 
@@ -209,9 +211,9 @@ export class SelectGrowiService implements GrowiCommandProcessor<SelectGrowiComm
     }
 
     // increment sendCommandBody
-    const channelName = interactionPayloadAccessor.getChannelName();
-    if (channelName == null) {
-      logger.error('Growi command failed: channelName not found.');
+    const channel = interactionPayloadAccessor.getChannel();
+    if (channel == null) {
+      logger.error('Growi command failed: channel not found.');
       await respond(responseUrl, {
         text: 'Growi command failed',
         blocks: [
@@ -222,7 +224,8 @@ export class SelectGrowiService implements GrowiCommandProcessor<SelectGrowiComm
     }
     const sendCommandBody: SendCommandBody = {
       trigger_id: interactionPayload.trigger_id,
-      channel_name: channelName,
+      channel_id: channel.id,
+      channel_name: channel.name,
     };
 
     return {