diff options
author | John Jarvis <jarv@gitlab.com> | 2018-12-27 08:45:24 +0000 |
---|---|---|
committer | John Jarvis <jarv@gitlab.com> | 2018-12-27 08:45:24 +0000 |
commit | 1617cfb104b696e5225b95923010b8794f10a47c (patch) | |
tree | 7d85145398f7f1c54317dc63b2a4b704303c3de1 | |
parent | 28abad5fd6a195c900bb6577a6781069ec91a1ce (diff) | |
parent | 19a735fc0c4b3a983717176e957098cd1a8a7c17 (diff) | |
download | gitlab-ce-1617cfb104b696e5225b95923010b8794f10a47c.tar.gz |
Merge branch 'security-11-6-group-cicd-settings-accessible-to-maintainer' into 'security-11-6'
[11.6] Group Ex-Maintainer Could maintain Access to Project's Source Code/Jobs/Pipelines/Artifacts if it had Shared Group Runner Configured
See merge request gitlab/gitlabhq!2750
5 files changed, 56 insertions, 15 deletions
diff --git a/app/controllers/groups/settings/ci_cd_controller.rb b/app/controllers/groups/settings/ci_cd_controller.rb index c1dcc463de7..f476f428fdb 100644 --- a/app/controllers/groups/settings/ci_cd_controller.rb +++ b/app/controllers/groups/settings/ci_cd_controller.rb @@ -4,7 +4,7 @@ module Groups module Settings class CiCdController < Groups::ApplicationController skip_cross_project_access_check :show - before_action :authorize_admin_pipeline! + before_action :authorize_admin_group! def show define_ci_variables @@ -26,8 +26,8 @@ module Groups .map { |variable| variable.present(current_user: current_user) } end - def authorize_admin_pipeline! - return render_404 unless can?(current_user, :admin_pipeline, group) + def authorize_admin_group! + return render_404 unless can?(current_user, :admin_group, group) end end end diff --git a/changelogs/unreleased/security-11-6-group-cicd-settings-accessible-to-maintainer.yml b/changelogs/unreleased/security-11-6-group-cicd-settings-accessible-to-maintainer.yml new file mode 100644 index 00000000000..5586fa6cd8e --- /dev/null +++ b/changelogs/unreleased/security-11-6-group-cicd-settings-accessible-to-maintainer.yml @@ -0,0 +1,5 @@ +--- +title: Allow changing group CI/CD settings only for owners. +merge_request: +author: +type: security diff --git a/spec/controllers/groups/settings/ci_cd_controller_spec.rb b/spec/controllers/groups/settings/ci_cd_controller_spec.rb index 06ccace8242..1bcc30915a1 100644 --- a/spec/controllers/groups/settings/ci_cd_controller_spec.rb +++ b/spec/controllers/groups/settings/ci_cd_controller_spec.rb @@ -5,30 +5,65 @@ describe Groups::Settings::CiCdController do let(:user) { create(:user) } before do - group.add_maintainer(user) sign_in(user) end describe 'GET #show' do - it 'renders show with 200 status code' do - get :show, group_id: group + context 'when user is owner' do + before do + group.add_owner(user) + end - expect(response).to have_gitlab_http_status(200) - expect(response).to render_template(:show) + it 'renders show with 200 status code' do + get :show, group_id: group + + expect(response).to have_gitlab_http_status(200) + expect(response).to render_template(:show) + end + end + + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + get :show, group_id: group + + expect(response).to have_gitlab_http_status(404) + end end end describe 'PUT #reset_registration_token' do subject { put :reset_registration_token, group_id: group } - it 'resets runner registration token' do - expect { subject }.to change { group.reload.runners_token } + context 'when user is owner' do + before do + group.add_owner(user) + end + + it 'resets runner registration token' do + expect { subject }.to change { group.reload.runners_token } + end + + it 'redirects the user to admin runners page' do + subject + + expect(response).to redirect_to(group_settings_ci_cd_path) + end end - it 'redirects the user to admin runners page' do - subject + context 'when user is not owner' do + before do + group.add_maintainer(user) + end + + it 'renders a 404' do + subject - expect(response).to redirect_to(group_settings_ci_cd_path) + expect(response).to have_gitlab_http_status(404) + end end end end diff --git a/spec/features/group_variables_spec.rb b/spec/features/group_variables_spec.rb index 89e0cdd8ed7..57e3ddfb39c 100644 --- a/spec/features/group_variables_spec.rb +++ b/spec/features/group_variables_spec.rb @@ -7,7 +7,7 @@ describe 'Group variables', :js do let(:page_path) { group_settings_ci_cd_path(group) } before do - group.add_maintainer(user) + group.add_owner(user) gitlab_sign_in(user) visit page_path diff --git a/spec/features/runners_spec.rb b/spec/features/runners_spec.rb index cb7a912946c..09de983f669 100644 --- a/spec/features/runners_spec.rb +++ b/spec/features/runners_spec.rb @@ -259,8 +259,9 @@ describe 'Runners' do context 'group runners in group settings' do let(:group) { create(:group) } + before do - group.add_maintainer(user) + group.add_owner(user) end context 'group with no runners' do |