diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/models/project.rb | 5 | ||||
| -rw-r--r-- | app/services/projects/base_service.rb | 18 | ||||
| -rw-r--r-- | app/services/projects/create_service.rb | 11 | ||||
| -rw-r--r-- | app/services/projects/update_service.rb | 11 | ||||
| -rw-r--r-- | doc/public_access/public_access.md | 2 | ||||
| -rw-r--r-- | lib/api/helpers.rb | 2 | ||||
| -rw-r--r-- | lib/api/projects.rb | 6 | ||||
| -rw-r--r-- | spec/requests/api/projects_spec.rb | 26 | ||||
| -rw-r--r-- | spec/services/projects/create_service_spec.rb | 27 | ||||
| -rw-r--r-- | spec/services/projects/update_service_spec.rb | 6 |
11 files changed, 95 insertions, 20 deletions
diff --git a/CHANGELOG b/CHANGELOG index b26e83d7269..5e9f69f3e9c 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -11,6 +11,7 @@ v 7.9.0 (unreleased) - Improve error messages for file edit failures - Improve UI for commits, issues and merge request lists - Fix commit comments on first line of diff not rendering in Merge Request Discussion view. + - Allow admins to override restricted project visibility settings. - Move restricted visibility settings from gitlab.yml into the web UI. - Improve trigger merge request hook when source project branch has been updated (Kirill Zaitsev) - Save web edit in new branch diff --git a/app/models/project.rb b/app/models/project.rb index 16b68453f5c..dae2b6425c4 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -34,8 +34,6 @@ require 'file_size_validator' class Project < ActiveRecord::Base include Sortable - include Gitlab::CurrentSettings - extend Gitlab::CurrentSettings include Gitlab::ShellAdapter include Gitlab::VisibilityLevel include Gitlab::ConfigHelper @@ -133,9 +131,6 @@ class Project < ActiveRecord::Base message: Gitlab::Regex.path_regex_message } validates :issues_enabled, :merge_requests_enabled, :wiki_enabled, inclusion: { in: [true, false] } - validates :visibility_level, - exclusion: { in: current_application_settings.restricted_visibility_levels }, - if: -> { current_application_settings.restricted_visibility_levels.any? } validates :issues_tracker_id, length: { maximum: 255 }, allow_blank: true validates :namespace, presence: true validates_uniqueness_of :name, scope: :namespace_id diff --git a/app/services/projects/base_service.rb b/app/services/projects/base_service.rb new file mode 100644 index 00000000000..2a683e0d40a --- /dev/null +++ b/app/services/projects/base_service.rb @@ -0,0 +1,18 @@ +module Projects + class BaseService < ::BaseService + # Add an error to the project for restricted visibility levels + def deny_visibility_level(project, denied_visibility_level = nil) + denied_visibility_level ||= project.visibility_level + + level_name = 'Unknown' + Gitlab::VisibilityLevel.options.each do |name, level| + level_name = name if level == denied_visibility_level + end + + project.errors.add( + :visibility_level, + "#{level_name} visibility has been restricted by your GitLab administrator" + ) + end + end +end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 4fe790b98f1..5f166a9a30b 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -1,5 +1,5 @@ module Projects - class CreateService < BaseService + class CreateService < Projects::BaseService def initialize(user, params) @current_user, @params = user, params.dup end @@ -7,9 +7,12 @@ module Projects def execute @project = Project.new(params) - # Reset visibility level if is not allowed to set it - unless Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) - @project.visibility_level = default_features.visibility_level + # Make sure that the user is allowed to use the specified visibility + # level + unless Gitlab::VisibilityLevel.allowed_for?(current_user, + params[:visibility_level]) + deny_visibility_level(@project) + return @project end # Set project name from path diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb index 36877a61679..823afadc186 100644 --- a/app/services/projects/update_service.rb +++ b/app/services/projects/update_service.rb @@ -1,9 +1,14 @@ module Projects - class UpdateService < BaseService + class UpdateService < Projects::BaseService def execute # check that user is allowed to set specified visibility_level - unless can?(current_user, :change_visibility_level, project) && Gitlab::VisibilityLevel.allowed_for?(current_user, params[:visibility_level]) - params[:visibility_level] = project.visibility_level + new_visibility = params[:visibility_level] + if new_visibility && new_visibility.to_i != project.visibility_level + unless can?(current_user, :change_visibility_level, project) && + Gitlab::VisibilityLevel.allowed_for?(current_user, new_visibility) + deny_visibility_level(project, new_visibility) + return project + end end new_branch = params[:default_branch] diff --git a/doc/public_access/public_access.md b/doc/public_access/public_access.md index 4712c387021..7c5a6c04639 100644 --- a/doc/public_access/public_access.md +++ b/doc/public_access/public_access.md @@ -41,4 +41,4 @@ When visiting the public page of an user, you will only see listed projects whic ## Restricting the use of public or internal projects -In [gitlab.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/dbd88d453b8e6c78a423fa7e692004b1db6ea069/config/gitlab.yml.example#L64) you can disable public projects or public and internal projects for the entire GitLab installation to prevent people making code public by accident. +In [gitlab.yml](https://gitlab.com/gitlab-org/gitlab-ce/blob/dbd88d453b8e6c78a423fa7e692004b1db6ea069/config/gitlab.yml.example#L64) you can disable public projects or public and internal projects for the entire GitLab installation to prevent people making code public by accident. The restricted visibility settings do not apply to admin users. diff --git a/lib/api/helpers.rb b/lib/api/helpers.rb index 228a719fbdf..f46dc8b456e 100644 --- a/lib/api/helpers.rb +++ b/lib/api/helpers.rb @@ -204,7 +204,7 @@ module API end def render_validation_error!(model) - unless model.valid? + if model.errors.any? render_api_error!(model.errors.messages || '400 Bad Request', 400) end end diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 0677e85beab..83f65eec6cc 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -233,10 +233,10 @@ module API ::Projects::UpdateService.new(user_project, current_user, attrs).execute - if user_project.valid? - present user_project, with: Entities::Project - else + if user_project.errors.any? render_validation_error!(user_project) + else + present user_project, with: Entities::Project end end diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index 0b3a47e3273..98b31a6e0af 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -3,6 +3,7 @@ require 'spec_helper' describe API::API, api: true do include ApiHelpers + include Gitlab::CurrentSettings let(:user) { create(:user) } let(:user2) { create(:user) } let(:user3) { create(:user) } @@ -202,6 +203,31 @@ describe API::API, api: true do expect(json_response['public']).to be_falsey expect(json_response['visibility_level']).to eq(Gitlab::VisibilityLevel::PRIVATE) end + + context 'when a visibility level is restricted' do + before do + @project = attributes_for(:project, { public: true }) + allow_any_instance_of(ApplicationSetting).to( + receive(:restricted_visibility_levels).and_return([20]) + ) + end + + it 'should not allow a non-admin to use a restricted visibility level' do + post api('/projects', user), @project + expect(response.status).to eq(400) + expect(json_response['message']['visibility_level'].first).to( + match('restricted by your GitLab administrator') + ) + end + + it 'should allow an admin to override restricted visibility settings' do + post api('/projects', admin), @project + expect(json_response['public']).to be_truthy + expect(json_response['visibility_level']).to( + eq(Gitlab::VisibilityLevel::PUBLIC) + ) + end + end end describe 'POST /projects/user/:id' do diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb index 8bb48346202..337dae592dd 100644 --- a/spec/services/projects/create_service_spec.rb +++ b/spec/services/projects/create_service_spec.rb @@ -55,6 +55,33 @@ describe Projects::CreateService do it { expect(File.exists?(@path)).to be_falsey } end end + + context 'restricted visibility level' do + before do + allow_any_instance_of(ApplicationSetting).to( + receive(:restricted_visibility_levels).and_return([20]) + ) + + @opts.merge!( + visibility_level: Gitlab::VisibilityLevel.options['Public'] + ) + end + + it 'should not allow a restricted visibility level for non-admins' do + project = create_project(@user, @opts) + expect(project).to respond_to(:errors) + expect(project.errors.messages).to have_key(:visibility_level) + expect(project.errors.messages[:visibility_level].first).to( + match('restricted by your GitLab administrator') + ) + end + + it 'should allow a restricted visibility level for admins' do + project = create_project(@admin, @opts) + expect(project.errors.any?).to be(false) + expect(project.saved?).to be(true) + end + end end def create_project(user, opts) diff --git a/spec/services/projects/update_service_spec.rb b/spec/services/projects/update_service_spec.rb index 10dbc548e86..ea5b8813105 100644 --- a/spec/services/projects/update_service_spec.rb +++ b/spec/services/projects/update_service_spec.rb @@ -47,9 +47,9 @@ describe Projects::UpdateService do context 'respect configured visibility restrictions setting' do before(:each) do - @restrictions = double("restrictions") - allow(@restrictions).to receive(:restricted_visibility_levels) { [ "public" ] } - Settings.stub_chain(:gitlab).and_return(@restrictions) + allow_any_instance_of(ApplicationSetting).to( + receive(:restricted_visibility_levels).and_return([20]) + ) end context 'should be private when updated to private' do |