Merge branch '10-8-stable-patch-4' into '10-8-stable'

Prepare 10.8.4 release

See merge request gitlab-org/gitlab-ce!19472

7 files changed, 57 insertions, 14 deletions

diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb
index 2843d70c645..4ad062e5155 100644
--- a/app/controllers/application_controller.rb
+++ b/app/controllers/application_controller.rb
@@ -130,12 +130,17 @@ class ApplicationController < ActionController::Base
   end
 
   def access_denied!(message = nil)
+    # If we display a custom access denied message to the user, we don't want to
+    # hide existence of the resource, rather tell them they cannot access it using
+    # the provided message
+    status = message.present? ? :forbidden : :not_found
+
     respond_to do |format|
-      format.any { head :not_found }
+      format.any { head status }
       format.html do
         render "errors/access_denied",
                layout: "errors",
-               status: 404,
+               status: status,
                locals: { message: message }
       end
     end
diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb
index d16ecdb7b9b..e3b36e8be18 100644
--- a/app/services/projects/create_service.rb
+++ b/app/services/projects/create_service.rb
@@ -46,6 +46,9 @@ module Projects
 
       yield(@project) if block_given?
 
+      # If the block added errors, don't try to save the project
+      return @project if @project.errors.any?
+
       @project.creator = current_user
 
       if forked_from_project_id
diff --git a/app/services/projects/update_service.rb b/app/services/projects/update_service.rb
index 679f4a9cb62..b2414a39e72 100644
--- a/app/services/projects/update_service.rb
+++ b/app/services/projects/update_service.rb
@@ -17,6 +17,11 @@ module Projects
 
       ensure_wiki_exists if enabling_wiki?
 
+      yield if block_given?
+
+      # If the block added errors, don't try to save the project
+      return validation_failed! if project.errors.any?
+
       if project.update_attributes(params.except(:default_branch))
         if project.previous_changes.include?('path')
           project.rename_repo
@@ -28,10 +33,7 @@ module Projects
 
         success
       else
-        model_errors = project.errors.full_messages.to_sentence
-        error_message = model_errors.presence || 'Project could not be updated!'
-
-        error(error_message)
+        validation_failed!
       end
     end
 
@@ -43,6 +45,13 @@ module Projects
 
     private
 
+    def validation_failed!
+      model_errors = project.errors.full_messages.to_sentence
+      error_message = model_errors.presence || 'Project could not be updated!'
+
+      error(error_message)
+    end
+
     def renaming_project_with_container_registry_tags?
       new_path = params[:path]
 
diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb
index f0caac40afd..a16e1923639 100644
--- a/spec/controllers/application_controller_spec.rb
+++ b/spec/controllers/application_controller_spec.rb
@@ -469,4 +469,28 @@ describe ApplicationController do
       end
     end
   end
+
+  describe '#access_denied' do
+    controller(described_class) do
+      def index
+        access_denied!(params[:message])
+      end
+    end
+
+    before do
+      sign_in user
+    end
+
+    it 'renders a 404 without a message' do
+      get :index
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'renders a 403 when a message is passed to access denied' do
+      get :index, message: 'None shall pass'
+
+      expect(response).to have_gitlab_http_status(403)
+    end
+  end
 end
diff --git a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb
index 27f558e1b5d..d20471ef603 100644
--- a/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb
+++ b/spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb
@@ -43,13 +43,13 @@ describe ControllerWithCrossProjectAccessCheck do
         end
       end
 
-      it 'renders a 404 with trying to access a cross project page' do
+      it 'renders a 403 with trying to access a cross project page' do
         message = "This page is unavailable because you are not allowed to read "\
                   "information across multiple projects."
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
         expect(response.body).to match(/#{message}/)
       end
 
@@ -119,7 +119,7 @@ describe ControllerWithCrossProjectAccessCheck do
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'is executed when the `unless` condition returns true' do
@@ -127,19 +127,19 @@ describe ControllerWithCrossProjectAccessCheck do
 
         get :index
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'does not skip the check on an action that is not skipped' do
         get :show, id: 'hello'
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
 
       it 'does not skip the check on an action that was not defined to skip' do
         get :edit, id: 'hello'
 
-        expect(response).to have_gitlab_http_status(404)
+        expect(response).to have_gitlab_http_status(403)
       end
     end
   end
diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb
index 30c06ddf744..416a09e1684 100644
--- a/spec/controllers/search_controller_spec.rb
+++ b/spec/controllers/search_controller_spec.rb
@@ -32,7 +32,7 @@ describe SearchController do
     it 'still blocks searches without a project_id' do
       get :show, search: 'hello'
 
-      expect(response).to have_gitlab_http_status(404)
+      expect(response).to have_gitlab_http_status(403)
     end
 
     it 'allows searches with a project_id' do
diff --git a/spec/requests/api/settings_spec.rb b/spec/requests/api/settings_spec.rb
index 8b22d1e72f3..26af56a12fc 100644
--- a/spec/requests/api/settings_spec.rb
+++ b/spec/requests/api/settings_spec.rb
@@ -30,7 +30,9 @@ describe API::Settings, 'Settings' do
   describe "PUT /application/settings" do
     context "custom repository storage type set in the config" do
       before do
-        storages = { 'custom' => 'tmp/tests/custom_repositories' }
+        # Add a possible storage to the config
+        storages = Gitlab.config.repositories.storages
+                     .merge({ 'custom' => 'tmp/tests/custom_repositories' })
         allow(Gitlab.config.repositories).to receive(:storages).and_return(storages)
       end