Merge branch 'rs-issue-12944' into 'master'

Use a custom Devise failure app to handle unauthenticated .zip requests

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/12944

See merge request !2828

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
+  config.warden do |manager|
+    manager.failure_app = Gitlab::DeviseFailure
     # manager.intercept_401 = false
     # manager.default_strategies(scope: :user).unshift :some_external_strategy
-  # end
+  end
 
   if Gitlab::LDAP::Config.enabled?
     Gitlab.config.ldap.servers.values.each do |server|

lib/gitlab/devise_failure.rb

+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

spec/controllers/projects/repositories_controller_spec.rb

@@ -2,14 +2,24 @@ require "spec_helper"
 
 describe Projects::RepositoriesController do
   let(:project) { create(:project) }
-  let(:user)    { create(:user) }
 
   describe "GET archive" do
+    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
+
+    context 'as a user' do
+      let(:user) { create(:user) }
+
       before do
-      sign_in(user)
         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")
 
@@ -29,4 +39,5 @@ describe Projects::RepositoriesController do
         end
       end
     end
+  end
 end