Allow admins to override restricted visibility

Allow admins to use restricted visibility levels when creating or
updating projects.

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