diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index d82cfb3ec0c787022f6b61fcab2f5104f0fee8f0..31dceaebcadb5b2afa9fffba38ebe7ee556f7bd3 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -203,11 +203,11 @@ Devise.setup do |config| # If you want to use other strategies, that are not supported by Devise, or # change the failure app, you can configure them inside the config.warden block. # - # config.warden do |manager| - # manager.failure_app = AnotherApp - # manager.intercept_401 = false - # manager.default_strategies(scope: :user).unshift :some_external_strategy - # end + config.warden do |manager| + manager.failure_app = Gitlab::DeviseFailure + # manager.intercept_401 = false + # manager.default_strategies(scope: :user).unshift :some_external_strategy + end if Gitlab::LDAP::Config.enabled? Gitlab.config.ldap.servers.values.each do |server| diff --git a/lib/gitlab/devise_failure.rb b/lib/gitlab/devise_failure.rb new file mode 100644 index 0000000000000000000000000000000000000000..a78fde9d7829136ba49633de4cb56a0bcabf65b7 --- /dev/null +++ b/lib/gitlab/devise_failure.rb @@ -0,0 +1,23 @@ +module Gitlab + class DeviseFailure < Devise::FailureApp + protected + + # Override `Devise::FailureApp#request_format` to handle a special case + # + # This tells Devise to handle an unauthenticated `.zip` request as an HTML + # request (i.e., redirect to sign in). + # + # Otherwise, Devise would respond with a 401 Unauthorized with + # `Content-Type: application/zip` and a response body in plaintext, and the + # browser would freak out. + # + # See https://gitlab.com/gitlab-org/gitlab-ce/issues/12944 + def request_format + if request.format == :zip + Mime::Type.lookup_by_extension(:html).ref + else + super + end + end + end +end diff --git a/spec/controllers/projects/repositories_controller_spec.rb b/spec/controllers/projects/repositories_controller_spec.rb index 09ec4f18f9d2394dc2276e6fe1d603b121d12c1c..0ddbec9eac21cee9bba8ff16f8e90cc310a04f2e 100644 --- a/spec/controllers/projects/repositories_controller_spec.rb +++ b/spec/controllers/projects/repositories_controller_spec.rb @@ -2,30 +2,41 @@ require "spec_helper" describe Projects::RepositoriesController do let(:project) { create(:project) } - let(:user) { create(:user) } describe "GET archive" do - before do - sign_in(user) - project.team << [user, :developer] - end - - it "uses Gitlab::Workhorse" do - expect(Gitlab::Workhorse).to receive(:send_git_archive).with(project, "master", "zip") + context 'as a guest' do + it 'responds with redirect in correct format' do + get :archive, namespace_id: project.namespace.path, project_id: project.path, format: "zip" - get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + expect(response.content_type).to start_with 'text/html' + expect(response).to be_redirect + end end - context "when the service raises an error" do + context 'as a user' do + let(:user) { create(:user) } before do - allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed") + project.team << [user, :developer] + sign_in(user) end + it "uses Gitlab::Workhorse" do + expect(Gitlab::Workhorse).to receive(:send_git_archive).with(project, "master", "zip") - it "renders Not Found" do get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + end + + context "when the service raises an error" do + + before do + allow(Gitlab::Workhorse).to receive(:send_git_archive).and_raise("Archive failed") + end + + it "renders Not Found" do + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" - expect(response.status).to eq(404) + expect(response.status).to eq(404) + end end end end