diff options
author | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-07-24 20:46:45 +0300 |
---|---|---|
committer | GitLab Release Tools Bot <robert+release-tools@gitlab.com> | 2019-07-24 20:46:45 +0300 |
commit | 08d4019405bc60f8b585f39bb325bb4c2933d4dd (patch) | |
tree | 2d68d729eb3e0a2e3d75c8f3aba81176e47bb2e3 | |
parent | ac93ef9409ee96aa88b0b513b411979a86c1c613 (diff) | |
parent | 51e607156c0a7676ffac8a647077b0934d946123 (diff) |
Merge branch 'security-2873-restrict-slash-commands-to-users-who-can-log-in-12-0' into '12-0-stable'
Restrict slash commands to users who can log in
See merge request gitlab/gitlabhq!3238
5 files changed, 51 insertions, 0 deletions
diff --git a/app/models/project_services/slash_commands_service.rb b/app/models/project_services/slash_commands_service.rb index bfabc6d262c..46925f6704d 100644 --- a/app/models/project_services/slash_commands_service.rb +++ b/app/models/project_services/slash_commands_service.rb @@ -35,6 +35,8 @@ class SlashCommandsService < Service chat_user = find_chat_user(params) if chat_user&.user + return Gitlab::SlashCommands::Presenters::Access.new.access_denied unless chat_user.user.can?(:use_slash_commands) + Gitlab::SlashCommands::Command.new(project, chat_user, params).execute else url = authorize_chat_name_url(params) diff --git a/app/policies/global_policy.rb b/app/policies/global_policy.rb index 134de1c9ace..311aab0dcd4 100644 --- a/app/policies/global_policy.rb +++ b/app/policies/global_policy.rb @@ -33,6 +33,7 @@ class GlobalPolicy < BasePolicy enable :access_git enable :receive_notifications enable :use_quick_actions + enable :use_slash_commands end rule { blocked | internal }.policy do @@ -40,6 +41,7 @@ class GlobalPolicy < BasePolicy prevent :access_api prevent :access_git prevent :receive_notifications + prevent :use_slash_commands end rule { required_terms_not_accepted }.policy do @@ -57,6 +59,7 @@ class GlobalPolicy < BasePolicy rule { access_locked }.policy do prevent :log_in + prevent :use_slash_commands end rule { ~(anonymous & restricted_public_level) }.policy do diff --git a/changelogs/unreleased/security-2873-blocked-user-slash-command-bypass-master.yml b/changelogs/unreleased/security-2873-blocked-user-slash-command-bypass-master.yml new file mode 100644 index 00000000000..cd31fe0f35c --- /dev/null +++ b/changelogs/unreleased/security-2873-blocked-user-slash-command-bypass-master.yml @@ -0,0 +1,5 @@ +--- +title: Restrict slash commands to users who can log in +merge_request: +author: +type: security diff --git a/spec/policies/global_policy_spec.rb b/spec/policies/global_policy_spec.rb index 12be3927e18..df6cc526eb0 100644 --- a/spec/policies/global_policy_spec.rb +++ b/spec/policies/global_policy_spec.rb @@ -226,4 +226,32 @@ describe GlobalPolicy do it { is_expected.not_to be_allowed(:read_instance_statistics) } end end + + describe 'slash commands' do + context 'regular user' do + it { is_expected.to be_allowed(:use_slash_commands) } + end + + context 'when internal' do + let(:current_user) { User.ghost } + + it { is_expected.not_to be_allowed(:use_slash_commands) } + end + + context 'when blocked' do + before do + current_user.block + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + end + + context 'when access locked' do + before do + current_user.lock_access! + end + + it { is_expected.not_to be_allowed(:use_slash_commands) } + end + end end diff --git a/spec/support/shared_examples/chat_slash_commands_shared_examples.rb b/spec/support/shared_examples/chat_slash_commands_shared_examples.rb index dc97a39f051..ef40287fd6e 100644 --- a/spec/support/shared_examples/chat_slash_commands_shared_examples.rb +++ b/spec/support/shared_examples/chat_slash_commands_shared_examples.rb @@ -91,6 +91,19 @@ RSpec.shared_examples 'chat slash commands service' do subject.trigger(params) end + + context 'when user is blocked' do + before do + chat_name.user.block + end + + it 'blocks command execution' do + expect_any_instance_of(Gitlab::SlashCommands::Command).not_to receive(:execute) + + result = subject.trigger(params) + expect(result).to include(text: /^Whoops! This action is not allowed/) + end + end end end end |