From 5844a21a0acae08a19fa82984dcc0feb1b8777c5 Mon Sep 17 00:00:00 2001 From: Robert Speicher Date: Mon, 15 Feb 2016 21:17:20 -0500 Subject: [PATCH] Use a custom Devise failure app to handle unauthenticated .zip requests Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/12944 --- config/initializers/devise.rb | 10 ++--- lib/gitlab/devise_failure.rb | 23 +++++++++++ .../projects/repositories_controller_spec.rb | 41 ++++++++++++------- 3 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 lib/gitlab/devise_failure.rb diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index d82cfb3ec0c..31dceaebcad 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 00000000000..a78fde9d782 --- /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 09ec4f18f9d..0ddbec9eac2 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] + 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" + + expect(response.content_type).to start_with 'text/html' + expect(response).to be_redirect + end end - it "uses Gitlab::Workhorse" do - expect(Gitlab::Workhorse).to receive(:send_git_archive).with(project, "master", "zip") - - get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" - 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") + + get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" end - it "renders Not Found" do - get :archive, namespace_id: project.namespace.path, project_id: project.path, ref: "master", format: "zip" + context "when the service raises an error" do - expect(response.status).to eq(404) + 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) + end end end end