diff options
author | Mayra Cabrera <mcabrera@gitlab.com> | 2018-06-06 19:45:45 +0000 |
---|---|---|
committer | Mayra Cabrera <mcabrera@gitlab.com> | 2018-06-06 19:45:45 +0000 |
commit | 5dfec1ce48f45805037675e286800df3b146f8a8 (patch) | |
tree | 537c1848415ec38b8fefc3ff819133f1f6448216 | |
parent | 564c3424d6f9175cf5f2d522e10d20d781511bf1 (diff) | |
parent | 94badb642739eae6bf6f2d15fc585402a4a514d9 (diff) | |
download | gitlab-ce-5dfec1ce48f45805037675e286800df3b146f8a8.tar.gz |
Merge branch '10-8-stable-patch-4' into '10-8-stable'
Prepare 10.8.4 release
See merge request gitlab-org/gitlab-ce!19472
-rw-r--r-- | app/controllers/application_controller.rb | 9 | ||||
-rw-r--r-- | app/services/projects/create_service.rb | 3 | ||||
-rw-r--r-- | app/services/projects/update_service.rb | 17 | ||||
-rw-r--r-- | spec/controllers/application_controller_spec.rb | 24 | ||||
-rw-r--r-- | spec/controllers/concerns/controller_with_cross_project_access_check_spec.rb | 12 | ||||
-rw-r--r-- | spec/controllers/search_controller_spec.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/settings_spec.rb | 4 |
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 |