Commit 32a622bc authored by Erick Bajao's avatar Erick Bajao

Update policy for group and project for updating max artifacts size

Adds specific rules for admins under the group and project policies
instead of manually checking `admin?` everytime we try to update
the setting.
parent 22762832
...@@ -4,8 +4,8 @@ module Groups ...@@ -4,8 +4,8 @@ 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_group!, except: [:update] before_action :authorize_admin_group!
before_action :authorize_admin!, only: [:update] before_action :authorize_update_max_artifacts_size!, only: [:update]
def show def show
define_ci_variables define_ci_variables
...@@ -51,8 +51,8 @@ module Groups ...@@ -51,8 +51,8 @@ module Groups
return render_404 unless can?(current_user, :admin_group, group) return render_404 unless can?(current_user, :admin_group, group)
end end
def authorize_admin! def authorize_update_max_artifacts_size!
return render_404 unless current_user&.admin? return render_404 unless can?(current_user, :update_max_artifacts_size, group)
end end
def auto_devops_params def auto_devops_params
......
...@@ -57,7 +57,7 @@ module Projects ...@@ -57,7 +57,7 @@ module Projects
auto_devops_attributes: [:id, :domain, :enabled, :deploy_strategy], auto_devops_attributes: [:id, :domain, :enabled, :deploy_strategy],
ci_cd_settings_attributes: [:default_git_depth] ci_cd_settings_attributes: [:default_git_depth]
].tap do |list| ].tap do |list|
list << :max_artifacts_size if current_user.admin? list << :max_artifacts_size if can?(current_user, :update_max_artifacts_size, project)
end end
end end
......
...@@ -53,7 +53,10 @@ class GroupPolicy < BasePolicy ...@@ -53,7 +53,10 @@ class GroupPolicy < BasePolicy
enable :upload_file enable :upload_file
end end
rule { admin }.enable :read_group rule { admin }.policy do
enable :read_group
enable :update_max_artifacts_size
end
rule { has_projects }.policy do rule { has_projects }.policy do
enable :read_group enable :read_group
......
...@@ -137,6 +137,8 @@ class ProjectPolicy < BasePolicy ...@@ -137,6 +137,8 @@ class ProjectPolicy < BasePolicy
# not. # not.
rule { guest | admin }.enable :read_project_for_iids rule { guest | admin }.enable :read_project_for_iids
rule { admin }.enable :update_max_artifacts_size
rule { guest }.enable :guest_access rule { guest }.enable :guest_access
rule { reporter }.enable :reporter_access rule { reporter }.enable :reporter_access
rule { developer }.enable :developer_access rule { developer }.enable :developer_access
......
...@@ -6,7 +6,7 @@ ...@@ -6,7 +6,7 @@
-# Given we only have one field in this form which is also admin-only, -# Given we only have one field in this form which is also admin-only,
-# we don't want to show an empty section to non-admin users, -# we don't want to show an empty section to non-admin users,
- if current_user.admin? - if can?(current_user, :update_max_artifacts_size, @group)
%section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) } %section.settings#js-general-pipeline-settings.no-animate{ class: ('expanded' if general_expanded) }
.settings-header .settings-header
%h4 %h4
......
...@@ -40,7 +40,7 @@ ...@@ -40,7 +40,7 @@
= _('If any job surpasses this timeout threshold, it will be marked as failed. Human readable time input language is accepted like "1 hour". Values without specification represent seconds.') = _('If any job surpasses this timeout threshold, it will be marked as failed. Human readable time input language is accepted like "1 hour". Values without specification represent seconds.')
= link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank' = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank'
- if current_user.admin? - if can?(current_user, :update_max_artifacts_size, @project)
%hr %hr
.form-group .form-group
= f.label :max_artifacts_size, _('Maximum artifacts size (MB)'), class: 'label-bold' = f.label :max_artifacts_size, _('Maximum artifacts size (MB)'), class: 'label-bold'
......
...@@ -547,4 +547,28 @@ describe GroupPolicy do ...@@ -547,4 +547,28 @@ describe GroupPolicy do
groups: [clusterable]) groups: [clusterable])
end end
end end
describe 'update_max_artifacts_size' do
let(:group) { create(:group, :public) }
context 'when no user' do
let(:current_user) { nil }
it { expect_disallowed(:update_max_artifacts_size) }
end
context 'admin' do
let(:current_user) { admin }
it { expect_allowed(:update_max_artifacts_size) }
end
%w(guest reporter developer maintainer owner).each do |role|
context role do
let(:current_user) { send(role) }
it { expect_disallowed(:update_max_artifacts_size) }
end
end
end
end end
...@@ -478,4 +478,28 @@ describe ProjectPolicy do ...@@ -478,4 +478,28 @@ describe ProjectPolicy do
end end
end end
end end
describe 'update_max_artifacts_size' do
subject { described_class.new(current_user, project) }
context 'when no user' do
let(:current_user) { nil }
it { expect_disallowed(:update_max_artifacts_size) }
end
context 'admin' do
let(:current_user) { admin }
it { expect_allowed(:update_max_artifacts_size) }
end
%w(guest reporter developer maintainer owner).each do |role|
context role do
let(:current_user) { send(role) }
it { expect_disallowed(:update_max_artifacts_size) }
end
end
end
end end
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