Add latest changes from gitlab-org/security/gitlab@14-4-stable-ee

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) }