Commit dcba5279 authored by Stan Hu's avatar Stan Hu

Fix inability to set visibility_level on project via API

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
parent 89fa2538
......@@ -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
......
---
title: Fix inability to set visibility_level on project via API
merge_request: 29578
author:
type: fixed
......@@ -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
......@@ -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
......
......@@ -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])
......
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