Fix inability to set visibility_level on project via APIsh-fix-issue-63158

Consider the scenario:

1. The default visibility level is set to internal
2. A user attempts to create a private project within a private group

Previously this would always fail because default_value_for would
overwrite the private visibility setting, no matter what
visibility_level were specified. This was happening because
default_value_for was confused by the default value of 0 specified by
the database schema.

default_value_for attempts to assign the default value in the block by
checking whether the attribute has changed. The problem is that since
the default value by the database was 0, and the user requested 0, this
appeared as though no changes were made. As a result, default_value_for
would always overwrite the user's preference.

To fix this, we remove the use of default_value_for and only set the
visibility level to the default application setting when no preference
has been given at creation time.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63158

5 files changed, 79 insertions, 1 deletions

diff --git a/app/models/project.rb b/app/models/project.rb
index 9d17d68eee2..fb06af8e97e 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -72,7 +72,6 @@ class Project < ApplicationRecord
   delegate :no_import?, to: :import_state, allow_nil: true
 
   default_value_for :archived, false
-  default_value_for(:visibility_level) { Gitlab::CurrentSettings.default_project_visibility }
   default_value_for :resolve_outdated_diff_discussions, false
   default_value_for :container_registry_enabled, gitlab_config_features.container_registry
   default_value_for(:repository_storage) { Gitlab::CurrentSettings.pick_repository_storage }
@@ -613,6 +612,23 @@ class Project < ApplicationRecord
     end
   end
 
+  def initialize(attributes = {})
+    # We can't use default_value_for because the database has a default
+    # value of 0 for visibility_level. If someone attempts to create a
+    # private project, default_value_for will assume that the
+    # visibility_level hasn't changed and will use the application
+    # setting default, which could be internal or public. For projects
+    # inside a private group, those levels are invalid.
+    #
+    # To fix the problem, we assign the actual default in the application if
+    # no explicit visibility has been initialized.
+    unless visibility_attribute_present?(attributes)
+      attributes[:visibility_level] = Gitlab::CurrentSettings.default_project_visibility
+    end
+
+    super
+  end
+
   def all_pipelines
     if builds_enabled?
       super
diff --git a/changelogs/unreleased/sh-fix-issue-63158.yml b/changelogs/unreleased/sh-fix-issue-63158.yml
new file mode 100644
index 00000000000..1a79166b6a2
--- /dev/null
+++ b/changelogs/unreleased/sh-fix-issue-63158.yml
@@ -0,0 +1,5 @@
+---
+title: Fix inability to set visibility_level on project via API
+merge_request: 29578
+author:
+type: fixed
diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb
index 8f9d5cf1e63..e2787744f09 100644
--- a/lib/gitlab/visibility_level.rb
+++ b/lib/gitlab/visibility_level.rb
@@ -138,5 +138,18 @@ module Gitlab
     def visibility=(level)
       self[visibility_level_field] = Gitlab::VisibilityLevel.level_value(level)
     end
+
+    def visibility_attribute_present?(attributes)
+      visibility_level_attributes.each do |attr|
+        return true if attributes[attr].present?
+      end
+
+      false
+    end
+
+    def visibility_level_attributes
+      [visibility_level_field, visibility_level_field.to_s,
+       :visibility, 'visibility']
+    end
   end
 end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index aad08b9d4aa..673a1d7936f 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1479,11 +1479,28 @@ describe Project do
     end
 
     context 'when set to INTERNAL in application settings' do
+      using RSpec::Parameterized::TableSyntax
+
       before do
         stub_application_setting(default_project_visibility: Gitlab::VisibilityLevel::INTERNAL)
       end
 
       it { is_expected.to eq(Gitlab::VisibilityLevel::INTERNAL) }
+
+      where(:attribute_name, :value) do
+        :visibility | 'public'
+        :visibility_level | Gitlab::VisibilityLevel::PUBLIC
+        'visibility' | 'public'
+        'visibility_level' | Gitlab::VisibilityLevel::PUBLIC
+      end
+
+      with_them do
+        it 'sets the visibility level' do
+          proj = described_class.new(attribute_name => value, name: 'test', path: 'test')
+
+          expect(proj.visibility_level).to eq(Gitlab::VisibilityLevel::PUBLIC)
+        end
+      end
     end
   end
 
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index f54f9200661..a4c48991807 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -152,6 +152,33 @@ describe Projects::CreateService, '#execute' do
     end
   end
 
+  context 'default visibility level' do
+    let(:group) { create(:group, :private) }
+
+    before do
+      stub_application_setting(default_project_visibility: Gitlab::VisibilityLevel::INTERNAL)
+      group.add_developer(user)
+
+      opts.merge!(
+        visibility: 'private',
+        name: 'test',
+        namespace: group,
+        path: 'foo'
+      )
+    end
+
+    it 'creates a private project' do
+      project = create_project(user, opts)
+
+      expect(project).to respond_to(:errors)
+
+      expect(project.errors.any?).to be(false)
+      expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::PRIVATE)
+      expect(project.saved?).to be(true)
+      expect(project.valid?).to be(true)
+    end
+  end
+
   context 'restricted visibility level' do
     before do
       stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::PUBLIC])