From a4f000694ca9f8778c453b20f2cdc81552c97634 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Fri, 21 Apr 2023 13:37:10 +1200 Subject: [PATCH 1/2] Move Slack trigger API endpoint to Core This change is part of https://gitlab.com/gitlab-org/gitlab/-/issues/355896 to move all GitLab for Slack app code to Core. This MR moves: - The `slack/trigger` API endpoint - Related handling code --- .rubocop_todo/rspec/any_instance_of.yml | 1 - .rubocop_todo/rspec/context_wording.yml | 1 - .rubocop_todo/rspec/verified_doubles.yml | 1 - .rubocop_todo/style/if_unless_modifier.yml | 1 - .../slash_commands/global_slack_handler.rb | 71 ------------ ee/lib/ee/api/integrations.rb | 29 ----- .../global_slack_handler_spec.rb | 104 ------------------ lib/api/integrations.rb | 24 +++- .../slash_commands/global_slack_handler.rb | 70 ++++++++++++ .../global_slack_handler_spec.rb | 91 +++++++++++++++ spec/requests/api/integrations_spec.rb | 12 ++ spec/support/rspec_order_todo.yml | 1 - 12 files changed, 193 insertions(+), 213 deletions(-) delete mode 100644 ee/app/services/slash_commands/global_slack_handler.rb delete mode 100644 ee/lib/ee/api/integrations.rb delete mode 100644 ee/spec/services/slash_commands/global_slack_handler_spec.rb create mode 100644 lib/gitlab/slash_commands/global_slack_handler.rb create mode 100644 spec/lib/gitlab/slash_commands/global_slack_handler_spec.rb diff --git a/.rubocop_todo/rspec/any_instance_of.yml b/.rubocop_todo/rspec/any_instance_of.yml index b278eb355d5e3c..e8cc0e77ca9d4a 100644 --- a/.rubocop_todo/rspec/any_instance_of.yml +++ b/.rubocop_todo/rspec/any_instance_of.yml @@ -55,7 +55,6 @@ RSpec/AnyInstanceOf: - 'ee/spec/services/projects/destroy_service_spec.rb' - 'ee/spec/services/projects/group_links/destroy_service_spec.rb' - 'ee/spec/services/projects/update_service_spec.rb' - - 'ee/spec/services/slash_commands/global_slack_handler_spec.rb' - 'ee/spec/support/helpers/ee/stub_configuration.rb' - 'ee/spec/support/shared_examples/controllers/analytics/cycle_analytics/shared_stage_shared_examples.rb' - 'ee/spec/support/shared_examples/features/ultimate_trial_callout_shared_examples.rb' diff --git a/.rubocop_todo/rspec/context_wording.yml b/.rubocop_todo/rspec/context_wording.yml index e63d573108fd72..8cbc6ca9b18ff6 100644 --- a/.rubocop_todo/rspec/context_wording.yml +++ b/.rubocop_todo/rspec/context_wording.yml @@ -826,7 +826,6 @@ RSpec/ContextWording: - 'ee/spec/services/security/store_grouped_scans_service_spec.rb' - 'ee/spec/services/security/track_scan_service_spec.rb' - 'ee/spec/services/security/vulnerability_counting_service_spec.rb' - - 'ee/spec/services/slash_commands/global_slack_handler_spec.rb' - 'ee/spec/services/software_license_policies/update_service_spec.rb' - 'ee/spec/services/status_page/publish_attachments_service_spec.rb' - 'ee/spec/services/status_page/publish_details_service_spec.rb' diff --git a/.rubocop_todo/rspec/verified_doubles.yml b/.rubocop_todo/rspec/verified_doubles.yml index e72c47c45a603f..479272962daf67 100644 --- a/.rubocop_todo/rspec/verified_doubles.yml +++ b/.rubocop_todo/rspec/verified_doubles.yml @@ -169,7 +169,6 @@ RSpec/VerifiedDoubles: - 'ee/spec/services/security/ingestion/ingest_report_slice_service_spec.rb' - 'ee/spec/services/security/orchestration/assign_service_spec.rb' - 'ee/spec/services/security/security_orchestration_policies/on_demand_scan_pipeline_configuration_service_spec.rb' - - 'ee/spec/services/slash_commands/global_slack_handler_spec.rb' - 'ee/spec/services/status_page/publish_details_service_spec.rb' - 'ee/spec/services/status_page/publish_service_spec.rb' - 'ee/spec/services/status_page/trigger_publish_service_spec.rb' diff --git a/.rubocop_todo/style/if_unless_modifier.yml b/.rubocop_todo/style/if_unless_modifier.yml index 4d63d6a6883f2e..43cfa9b269d379 100644 --- a/.rubocop_todo/style/if_unless_modifier.yml +++ b/.rubocop_todo/style/if_unless_modifier.yml @@ -548,7 +548,6 @@ Style/IfUnlessModifier: - 'ee/app/services/security/security_orchestration_policies/process_policy_service.rb' - 'ee/app/services/security/security_orchestration_policies/project_create_service.rb' - 'ee/app/services/security/security_orchestration_policies/validate_policy_service.rb' - - 'ee/app/services/slash_commands/global_slack_handler.rb' - 'ee/app/services/start_pull_mirroring_service.rb' - 'ee/app/services/system_notes/epics_service.rb' - 'ee/app/services/timebox_report_service.rb' diff --git a/ee/app/services/slash_commands/global_slack_handler.rb b/ee/app/services/slash_commands/global_slack_handler.rb deleted file mode 100644 index a2f3b0ffb8b422..00000000000000 --- a/ee/app/services/slash_commands/global_slack_handler.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -module SlashCommands - class GlobalSlackHandler - attr_reader :project_alias, :params - - def initialize(params) - @project_alias, command = parse_command_text(params) - @params = params.merge(text: command, original_command: params[:text]) - end - - def trigger - return false unless valid_token? - - if help_command? - return Gitlab::SlashCommands::ApplicationHelp.new(nil, params).execute - end - - unless slack_integration = find_slack_integration - error_message = 'GitLab error: project or alias not found' - return Gitlab::SlashCommands::Presenters::Error.new(error_message).message - end - - chat_user = ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute - integration = slack_integration.integration - - if chat_user&.user - Gitlab::SlashCommands::Command.new(integration.project, chat_user, params).execute - else - url = ChatNames::AuthorizeUserService.new(params).execute - Gitlab::SlashCommands::Presenters::Access.new(url).authorize - end - end - - private - - def valid_token? - ActiveSupport::SecurityUtils.secure_compare( - Gitlab::CurrentSettings.current_application_settings - .slack_app_verification_token, - params[:token] - ) - end - - def help_command? - params[:original_command] == 'help' - end - - # rubocop: disable CodeReuse/ActiveRecord - def find_slack_integration - if project_alias.nil? - SlackIntegration.find_by(team_id: params[:team_id]) - else - SlackIntegration.find_by(team_id: params[:team_id], alias: project_alias) - end - end - # rubocop: enable CodeReuse/ActiveRecord - - # Splits the command - # '/gitlab help' => [nil, 'help'] - # '/gitlab group/project issue new some title' => ['group/project', 'issue new some title'] - def parse_command_text(params) - if params[:text] == 'incident declare' - [nil, params[:text]] - else - fragments = params[:text].split(/\s/, 2) - fragments.size == 1 ? [nil, fragments.first] : fragments - end - end - end -end diff --git a/ee/lib/ee/api/integrations.rb b/ee/lib/ee/api/integrations.rb deleted file mode 100644 index 151cf3dfb52334..00000000000000 --- a/ee/lib/ee/api/integrations.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module EE - module API - module Integrations - extend ActiveSupport::Concern - - prepended do - desc "Trigger a global slack command" do - detail 'Added in GitLab 9.4' - failure [ - { code: 401, message: 'Unauthorized' } - ] - end - params do - requires :text, type: String, desc: 'Text of the slack command' - end - post 'slack/trigger' do - if result = SlashCommands::GlobalSlackHandler.new(params).trigger - status result[:status] || 200 - present result - else - not_found! - end - end - end - end - end -end diff --git a/ee/spec/services/slash_commands/global_slack_handler_spec.rb b/ee/spec/services/slash_commands/global_slack_handler_spec.rb deleted file mode 100644 index 8f3026b87205a7..00000000000000 --- a/ee/spec/services/slash_commands/global_slack_handler_spec.rb +++ /dev/null @@ -1,104 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -RSpec.describe SlashCommands::GlobalSlackHandler, feature_category: :integrations do - let(:project) { create(:project) } - let(:user) { create(:user) } - let(:chat_name) { double(:chat_name, user: user) } - let(:verification_token) { '123' } - - before do - stub_application_setting( - slack_app_verification_token: verification_token - ) - end - - def enable_slack_application(project) - create(:gitlab_slack_application_integration, project: project) - end - - def handler(params) - SlashCommands::GlobalSlackHandler.new(params) - end - - def handler_with_valid_token(params) - handler(params.merge(token: verification_token)) - end - - it 'does not serve a request if token is invalid' do - result = handler(token: '123456', text: 'help').trigger - - expect(result).to be_falsey - end - - context 'Valid token' do - context 'with incident declare command' do - it 'calls command handler with no project alias' do - expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) - expect_any_instance_of(ChatNames::FindUserService).to receive(:execute).and_return(chat_name) - - enable_slack_application(project) - - slack_integration = create(:slack_integration, integration: project.gitlab_slack_application_integration) - - handler_with_valid_token( - text: "incident declare", - team_id: slack_integration.team_id - ).trigger - end - end - - it 'calls command handler if project alias is valid' do - expect_any_instance_of(Gitlab::SlashCommands::Command).to receive(:execute) - expect_any_instance_of(ChatNames::FindUserService).to receive(:execute).and_return(chat_name) - - enable_slack_application(project) - - slack_integration = create(:slack_integration, integration: project.gitlab_slack_application_integration) - slack_integration.update!(alias: project.full_path) - - handler_with_valid_token( - text: "#{project.full_path} issue new title", - team_id: slack_integration.team_id - ).trigger - end - - it 'returns error if project alias not found' do - expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Error).to receive(:message) - - enable_slack_application(project) - - slack_integration = create(:slack_integration, integration: project.gitlab_slack_application_integration) - - handler_with_valid_token( - text: "fake/fake issue new title", - team_id: slack_integration.team_id - ).trigger - end - - it 'returns authorization request' do - expect_any_instance_of(ChatNames::AuthorizeUserService).to receive(:execute) - expect_any_instance_of(Gitlab::SlashCommands::Presenters::Access).to receive(:authorize) - - enable_slack_application(project) - - slack_integration = create(:slack_integration, integration: project.gitlab_slack_application_integration) - slack_integration.update!(alias: project.full_path) - - handler_with_valid_token( - text: "#{project.full_path} issue new title", - team_id: slack_integration.team_id - ).trigger - end - - it 'calls help presenter' do - expect_any_instance_of(Gitlab::SlashCommands::ApplicationHelp).to receive(:execute) - - handler_with_valid_token( - text: "help" - ).trigger - end - end -end diff --git a/lib/api/integrations.rb b/lib/api/integrations.rb index 408fa038b0de99..4691488c19ed16 100644 --- a/lib/api/integrations.rb +++ b/lib/api/integrations.rb @@ -37,7 +37,7 @@ class Integrations < ::API::Base end end - TRIGGER_INTEGRATIONS = { + SLASH_COMMAND_INTEGRATIONS = { 'mattermost-slash-commands' => [ { name: :token, @@ -173,7 +173,7 @@ def integration_attributes(integration) end end - TRIGGER_INTEGRATIONS.each do |integration_slug, settings| + SLASH_COMMAND_INTEGRATIONS.each do |integration_slug, settings| helpers do def slash_command_integration(project, integration_slug, params) project.integrations.active.find do |integration| @@ -218,7 +218,23 @@ def slash_command_integration(project, integration_slug, params) end end end + + desc "Trigger a global slack command" do + detail 'Added in GitLab 9.4' + failure [ + { code: 401, message: 'Unauthorized' } + ] + end + params do + requires :text, type: String, desc: 'Text of the slack command' + end + post 'slack/trigger' do + if result = Gitlab::SlashCommands::GlobalSlackHandler.new(params).trigger + status result[:status] || 200 + present result + else + not_found! + end + end end end - -API::Integrations.prepend_mod_with('API::Integrations') diff --git a/lib/gitlab/slash_commands/global_slack_handler.rb b/lib/gitlab/slash_commands/global_slack_handler.rb new file mode 100644 index 00000000000000..9bcc1f72b96be2 --- /dev/null +++ b/lib/gitlab/slash_commands/global_slack_handler.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +module Gitlab + module SlashCommands + class GlobalSlackHandler + attr_reader :project_alias, :params + + def initialize(params) + @project_alias, command = parse_command_text(params) + @params = params.merge(text: command, original_command: params[:text]) + end + + def trigger + return false unless valid_token? + return Gitlab::SlashCommands::ApplicationHelp.new(nil, params).execute if help_command? + + unless slack_integration = find_slack_integration + error_message = 'GitLab error: project or alias not found' + return Gitlab::SlashCommands::Presenters::Error.new(error_message).message + end + + chat_user = ChatNames::FindUserService.new(params[:team_id], params[:user_id]).execute + integration = slack_integration.integration + + if chat_user&.user + Gitlab::SlashCommands::Command.new(integration.project, chat_user, params).execute + else + url = ChatNames::AuthorizeUserService.new(params).execute + Gitlab::SlashCommands::Presenters::Access.new(url).authorize + end + end + + private + + def valid_token? + ActiveSupport::SecurityUtils.secure_compare( + Gitlab::CurrentSettings.current_application_settings + .slack_app_verification_token, + params[:token] + ) + end + + def help_command? + params[:original_command] == 'help' + end + + # rubocop: disable CodeReuse/ActiveRecord + def find_slack_integration + if project_alias.nil? + SlackIntegration.find_by(team_id: params[:team_id]) + else + SlackIntegration.find_by(team_id: params[:team_id], alias: project_alias) + end + end + # rubocop: enable CodeReuse/ActiveRecord + + # Splits the command + # '/gitlab help' => [nil, 'help'] + # '/gitlab group/project issue new some title' => ['group/project', 'issue new some title'] + def parse_command_text(params) + if params[:text] == 'incident declare' + [nil, params[:text]] + else + fragments = params[:text].split(/\s/, 2) + fragments.size == 1 ? [nil, fragments.first] : fragments + end + end + end + end +end diff --git a/spec/lib/gitlab/slash_commands/global_slack_handler_spec.rb b/spec/lib/gitlab/slash_commands/global_slack_handler_spec.rb new file mode 100644 index 00000000000000..4a58d65fc4a4ef --- /dev/null +++ b/spec/lib/gitlab/slash_commands/global_slack_handler_spec.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::SlashCommands::GlobalSlackHandler, feature_category: :integrations do + include AfterNextHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + let_it_be_with_reload(:slack_integration) do + create(:gitlab_slack_application_integration, project: project).slack_integration + end + + let(:chat_name) { instance_double('ChatName', user: user) } + let(:verification_token) { '123' } + + before do + stub_application_setting(slack_app_verification_token: verification_token) + end + + def handler(params) + described_class.new(params) + end + + def handler_with_valid_token(params) + handler(params.merge(token: verification_token)) + end + + it 'does not serve a request if token is invalid' do + result = handler(token: '123456', text: 'help').trigger + + expect(result).to be_falsey + end + + context 'with valid token' do + context 'with incident declare command' do + it 'calls command handler with no project alias' do + expect_next(Gitlab::SlashCommands::Command).to receive(:execute) + expect_next(ChatNames::FindUserService).to receive(:execute).and_return(chat_name) + + handler_with_valid_token( + text: "incident declare", + team_id: slack_integration.team_id + ).trigger + end + end + + it 'calls command handler if project alias is valid' do + expect_next(Gitlab::SlashCommands::Command).to receive(:execute) + expect_next(ChatNames::FindUserService).to receive(:execute).and_return(chat_name) + + slack_integration.update!(alias: project.full_path) + + handler_with_valid_token( + text: "#{project.full_path} issue new title", + team_id: slack_integration.team_id + ).trigger + end + + it 'returns error if project alias not found' do + expect_next(Gitlab::SlashCommands::Command).not_to receive(:execute) + expect_next(Gitlab::SlashCommands::Presenters::Error).to receive(:message) + + handler_with_valid_token( + text: "fake/fake issue new title", + team_id: slack_integration.team_id + ).trigger + end + + it 'returns authorization request' do + expect_next(ChatNames::AuthorizeUserService).to receive(:execute) + expect_next(Gitlab::SlashCommands::Presenters::Access).to receive(:authorize) + + slack_integration.update!(alias: project.full_path) + + handler_with_valid_token( + text: "#{project.full_path} issue new title", + team_id: slack_integration.team_id + ).trigger + end + + it 'calls help presenter' do + expect_next(Gitlab::SlashCommands::ApplicationHelp).to receive(:execute) + + handler_with_valid_token( + text: "help" + ).trigger + end + end +end diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index af14d557f88aab..2476e70483e91d 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -428,4 +428,16 @@ def assert_secret_fields_filtered(response_keys, integration) expect(response_keys).not_to include(*integration.secret_fields) end end + + describe 'POST /slack/trigger' do + it 'returns status 200' do + create(:gitlab_slack_application_integration, project: project) + stub_application_setting(slack_app_verification_token: 'token') + + post api('/slack/trigger'), params: { token: 'token', text: 'help' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['response_type']).to eq("ephemeral") + end + end end diff --git a/spec/support/rspec_order_todo.yml b/spec/support/rspec_order_todo.yml index 852ecfa556da85..77f813aeffbe5a 100644 --- a/spec/support/rspec_order_todo.yml +++ b/spec/support/rspec_order_todo.yml @@ -2964,7 +2964,6 @@ - './ee/spec/services/security/update_training_service_spec.rb' - './ee/spec/services/security/vulnerability_counting_service_spec.rb' - './ee/spec/services/sitemap/create_service_spec.rb' -- './ee/spec/services/slash_commands/global_slack_handler_spec.rb' - './ee/spec/services/software_license_policies/create_service_spec.rb' - './ee/spec/services/software_license_policies/update_service_spec.rb' - './ee/spec/services/start_pull_mirroring_service_spec.rb' -- GitLab From a89d6dfd67d79927e99c69d6b94774f51fcc0617 Mon Sep 17 00:00:00 2001 From: Luke Duncalfe Date: Mon, 1 May 2023 16:02:01 +1200 Subject: [PATCH 2/2] Add reviewer feedback --- spec/requests/api/integrations_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/requests/api/integrations_spec.rb b/spec/requests/api/integrations_spec.rb index 2476e70483e91d..4e8f8810128223 100644 --- a/spec/requests/api/integrations_spec.rb +++ b/spec/requests/api/integrations_spec.rb @@ -430,14 +430,26 @@ def assert_secret_fields_filtered(response_keys, integration) end describe 'POST /slack/trigger' do - it 'returns status 200' do + before_all do create(:gitlab_slack_application_integration, project: project) + end + + before do stub_application_setting(slack_app_verification_token: 'token') + end + it 'returns status 200' do post api('/slack/trigger'), params: { token: 'token', text: 'help' } expect(response).to have_gitlab_http_status(:ok) expect(json_response['response_type']).to eq("ephemeral") end + + it 'returns status 404 when token is invalid' do + post api('/slack/trigger'), params: { token: 'invalid', text: 'foo' } + + expect(response).to have_gitlab_http_status(:not_found) + expect(json_response['response_type']).to be_blank + end end end -- GitLab