diff options
| -rw-r--r-- | app/services/concerns/update_visibility_level.rb | 12 | ||||
| -rw-r--r-- | app/services/groups/update_service.rb | 4 | ||||
| -rw-r--r-- | app/services/projects/update_service.rb | 2 | ||||
| -rw-r--r-- | lib/api/projects.rb | 2 | ||||
| -rw-r--r-- | lib/gitlab/visibility_level.rb | 8 | ||||
| -rw-r--r-- | spec/requests/api/groups_spec.rb | 18 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 22 |
7 files changed, 60 insertions, 8 deletions
diff --git a/app/services/concerns/update_visibility_level.rb b/app/services/concerns/update_visibility_level.rb index b7a161f5089..4cd14a2fb53 100644 --- a/app/services/concerns/update_visibility_level.rb +++ b/app/services/concerns/update_visibility_level.rb @@ -1,13 +1,17 @@ # frozen_string_literal: true module UpdateVisibilityLevel + # check that user is allowed to set specified visibility_level def valid_visibility_level_change?(target, new_visibility) - # check that user is allowed to set specified visibility_level - if new_visibility && new_visibility.to_i != target.visibility_level + return true unless new_visibility + + new_visibility_level = Gitlab::VisibilityLevel.level_value(new_visibility) + + if new_visibility_level != target.visibility_level_value unless can?(current_user, :change_visibility_level, target) && - Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility_level) - deny_visibility_level(target, new_visibility) + deny_visibility_level(target, new_visibility_level) return false end end diff --git a/app/services/groups/update_service.rb b/app/services/groups/update_service.rb index 1ad43b051be..2d6334251ad 100644 --- a/app/services/groups/update_service.rb +++ b/app/services/groups/update_service.rb @@ -15,7 +15,7 @@ module Groups return false end - return false unless valid_visibility_level_change?(group, params[:visibility_level]) + return false unless valid_visibility_level_change?(group, group.visibility_attribute_value(params)) return false unless valid_share_with_group_lock_change? @@ -77,7 +77,7 @@ module Groups end def after_update - if group.previous_changes.include?(:visibility_level) && group.private? + if group.previous_changes.include?(group.visibility_level_field) && group.private? # don't enqueue immediately to prevent todos removal in case of a mistake TodosDestroyer::GroupPrivateWorker.perform_in(Todo::WAIT_FOR_DELETE, group.id) end diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index a32e80af4b1..b34ecf06e52 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -49,7 +49,7 @@ module Projects private def validate! - unless valid_visibility_level_change?(project, params[:visibility_level]) + unless valid_visibility_level_change?(project, project.visibility_attribute_value(params)) raise ValidationError, s_('UpdateProject|New visibility level not allowed!') end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index e8a48d6c9f4..bb74849a98a 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -429,7 +429,7 @@ module API authorize_admin_project attrs = declared_params(include_missing: false) authorize! :rename_project, user_project if attrs[:name].present? - authorize! :change_visibility_level, user_project if attrs[:visibility].present? + authorize! :change_visibility_level, user_project if user_project.visibility_attribute_present?(attrs) attrs = translate_params_for_compatibility(attrs) filter_attributes_using_license!(attrs) diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index 64029d4d3fe..d378e558b8a 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -155,6 +155,14 @@ module Gitlab false end + def visibility_attribute_value(attributes) + visibility_level_attributes.each do |attr| + return attributes[attr] if attributes.has_key?(attr) + end + + nil + end + def visibility_level_attributes [visibility_level_field, visibility_level_field.to_s, :visibility, 'visibility'] diff --git a/spec/requests/api/groups_spec.rb b/spec/requests/api/groups_spec.rb index 2c7e2ecff85..cee727ae6fe 100644 --- a/spec/requests/api/groups_spec.rb +++ b/spec/requests/api/groups_spec.rb @@ -879,6 +879,15 @@ RSpec.describe API::Groups do expect(json_response['prevent_sharing_groups_outside_hierarchy']).to eq(true) end + it 'does not update visibility_level if it is restricted' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/groups/#{group1.id}", user1), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator') + end + context 'updating the `default_branch_protection` attribute' do subject do put api("/groups/#{group1.id}", user1), params: { default_branch_protection: ::Gitlab::Access::PROTECTION_NONE } @@ -966,6 +975,15 @@ RSpec.describe API::Groups do expect(response).to have_gitlab_http_status(:ok) expect(json_response['name']).to eq(new_group_name) end + + it 'ignores visibility level restrictions' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/groups/#{group1.id}", admin), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['visibility']).to eq('internal') + end end context 'when authenticated as an user that can see the group' do diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 9b23c008ae7..dd6afa869e0 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3203,6 +3203,15 @@ RSpec.describe API::Projects do expect(json_response['visibility']).to eq('private') end + it 'does not update visibility_level if it is restricted' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/projects/#{project3.id}", user), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']['visibility_level']).to include('internal has been restricted by your GitLab administrator') + end + it 'does not update name to existing name' do project_param = { name: project3.name } @@ -3526,6 +3535,19 @@ RSpec.describe API::Projects do end end + context 'when authenticated as the admin' do + let_it_be(:admin) { create(:admin) } + + it 'ignores visibility level restrictions' do + stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL]) + + put api("/projects/#{project3.id}", admin), params: { visibility: 'internal' } + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['visibility']).to eq('internal') + end + end + context 'when updating repository storage' do let(:unknown_storage) { 'new-storage' } let(:new_project) { create(:project, :repository, namespace: user.namespace) } |