Commit 082a6567 authored by John Jarvis's avatar John Jarvis

Merge branch 'security-master-group-cicd-settings-accessible-to-maintainer' into 'master'

[master] 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!2721
parents 5d550fa5 e264677b
...@@ -4,7 +4,7 @@ module Groups ...@@ -4,7 +4,7 @@ module Groups
module Settings module Settings
class CiCdController < Groups::ApplicationController class CiCdController < Groups::ApplicationController
skip_cross_project_access_check :show skip_cross_project_access_check :show
before_action :authorize_admin_pipeline! before_action :authorize_admin_group!
def show def show
define_ci_variables define_ci_variables
...@@ -26,8 +26,8 @@ module Groups ...@@ -26,8 +26,8 @@ module Groups
.map { |variable| variable.present(current_user: current_user) } .map { |variable| variable.present(current_user: current_user) }
end end
def authorize_admin_pipeline! def authorize_admin_group!
return render_404 unless can?(current_user, :admin_pipeline, group) return render_404 unless can?(current_user, :admin_group, group)
end end
end end
end end
......
---
title: Allow changing group CI/CD settings only for owners.
merge_request:
author:
type: security
...@@ -5,11 +5,15 @@ describe Groups::Settings::CiCdController do ...@@ -5,11 +5,15 @@ describe Groups::Settings::CiCdController do
let(:user) { create(:user) } let(:user) { create(:user) }
before do before do
group.add_maintainer(user)
sign_in(user) sign_in(user)
end end
describe 'GET #show' do describe 'GET #show' do
context 'when user is owner' do
before do
group.add_owner(user)
end
it 'renders show with 200 status code' do it 'renders show with 200 status code' do
get :show, params: { group_id: group } get :show, params: { group_id: group }
...@@ -18,9 +22,27 @@ describe Groups::Settings::CiCdController do ...@@ -18,9 +22,27 @@ describe Groups::Settings::CiCdController do
end end
end end
context 'when user is not owner' do
before do
group.add_maintainer(user)
end
it 'renders a 404' do
get :show, params: { group_id: group }
expect(response).to have_gitlab_http_status(404)
end
end
end
describe 'PUT #reset_registration_token' do describe 'PUT #reset_registration_token' do
subject { put :reset_registration_token, params: { group_id: group } } subject { put :reset_registration_token, params: { group_id: group } }
context 'when user is owner' do
before do
group.add_owner(user)
end
it 'resets runner registration token' do it 'resets runner registration token' do
expect { subject }.to change { group.reload.runners_token } expect { subject }.to change { group.reload.runners_token }
end end
...@@ -31,4 +53,17 @@ describe Groups::Settings::CiCdController do ...@@ -31,4 +53,17 @@ describe Groups::Settings::CiCdController do
expect(response).to redirect_to(group_settings_ci_cd_path) expect(response).to redirect_to(group_settings_ci_cd_path)
end end
end end
context 'when user is not owner' do
before do
group.add_maintainer(user)
end
it 'renders a 404' do
subject
expect(response).to have_gitlab_http_status(404)
end
end
end
end end
...@@ -7,7 +7,7 @@ describe 'Group variables', :js do ...@@ -7,7 +7,7 @@ describe 'Group variables', :js do
let(:page_path) { group_settings_ci_cd_path(group) } let(:page_path) { group_settings_ci_cd_path(group) }
before do before do
group.add_maintainer(user) group.add_owner(user)
gitlab_sign_in(user) gitlab_sign_in(user)
visit page_path visit page_path
......
...@@ -259,8 +259,9 @@ describe 'Runners' do ...@@ -259,8 +259,9 @@ describe 'Runners' do
context 'group runners in group settings' do context 'group runners in group settings' do
let(:group) { create(:group) } let(:group) { create(:group) }
before do before do
group.add_maintainer(user) group.add_owner(user)
end end
context 'group with no runners' do context 'group with no runners' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment