Merge branch 'feature-change-signout-route' into 'master'

Change Sign Out route from a DELETE to a GET

Closes #39708

See merge request gitlab-org/gitlab-ce!15231
(cherry picked from commit b579cc7620dad1d406e974cce2d9ad5a4ce58a57)

6 files changed, 12 insertions, 16 deletions

diff --git a/app/views/layouts/header/_default.html.haml b/app/views/layouts/header/_default.html.haml
index 5ff6ac5fc00..1eca412aff9 100644
--- a/app/views/layouts/header/_default.html.haml
+++ b/app/views/layouts/header/_default.html.haml
@@ -61,7 +61,7 @@
                       = link_to "Help", help_path
                   %li.divider
                   %li
-                    = link_to "Sign out", destroy_user_session_path, method: :delete, class: "sign-out-link"
+                    = link_to "Sign out", destroy_user_session_path, class: "sign-out-link"
             - if session[:impersonator_id]
               %li.impersonation
                 = link_to admin_impersonation_path, class: 'impersonation-btn', method: :delete, title: "Stop impersonation", aria: { label: 'Stop impersonation' }, data: { toggle: 'tooltip', placement: 'bottom', container: 'body' } do
diff --git a/changelogs/unreleased/feature-change-signout-route.yml b/changelogs/unreleased/feature-change-signout-route.yml
new file mode 100644
index 00000000000..bccb85b3eaf
--- /dev/null
+++ b/changelogs/unreleased/feature-change-signout-route.yml
@@ -0,0 +1,5 @@
+---
+title: Change 'Sign Out' route from a DELETE to a GET
+merge_request: 39708
+author: Joe Marty
+type: changed
diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb
index c6ec0aeda7b..958859be6cf 100644
--- a/config/initializers/devise.rb
+++ b/config/initializers/devise.rb
@@ -195,7 +195,7 @@ Devise.setup do |config|
   config.navigational_formats = [:"*/*", "*/*", :html, :zip]
 
   # The default HTTP method used to sign out a resource. Default is :delete.
-  config.sign_out_via = :delete
+  config.sign_out_via = :get
 
   # ==> OmniAuth
   # To configure a new OmniAuth provider copy and edit omniauth.rb.sample
diff --git a/lib/gitlab/middleware/read_only.rb b/lib/gitlab/middleware/read_only.rb
index 8853dfa3d2d..5e4932e4e57 100644
--- a/lib/gitlab/middleware/read_only.rb
+++ b/lib/gitlab/middleware/read_only.rb
@@ -66,11 +66,7 @@ module Gitlab
       end
 
       def whitelisted_routes
-        logout_route || grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route
-      end
-
-      def logout_route
-        route_hash[:controller] == 'sessions' && route_hash[:action] == 'destroy'
+        grack_route || @whitelisted.any? { |path| request.path.include?(path) } || lfs_route || sidekiq_route
       end
 
       def sidekiq_route
diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb
index 86be06ff595..b14735943a5 100644
--- a/spec/lib/gitlab/middleware/read_only_spec.rb
+++ b/spec/lib/gitlab/middleware/read_only_spec.rb
@@ -91,13 +91,6 @@ describe Gitlab::Middleware::ReadOnly do
     end
 
     context 'whitelisted requests' do
-      it 'expects DELETE request to logout to be allowed' do
-        response = request.delete('/users/sign_out')
-
-        expect(response).not_to be_a_redirect
-        expect(subject).not_to disallow_request
-      end
-
       it 'expects a POST internal request to be allowed' do
         response = request.post("/api/#{API::API.version}/internal")
 
diff --git a/spec/routing/routing_spec.rb b/spec/routing/routing_spec.rb
index 32aa6e5ad52..91aefa84d0e 100644
--- a/spec/routing/routing_spec.rb
+++ b/spec/routing/routing_spec.rb
@@ -257,8 +257,10 @@ describe "Authentication", "routing" do
     expect(post("/users/sign_in")).to route_to('sessions#create')
   end
 
-  it "DELETE /users/sign_out" do
-    expect(delete("/users/sign_out")).to route_to('sessions#destroy')
+  # sign_out with GET instead of DELETE facilitates ad-hoc single-sign-out processes
+  # (https://gitlab.com/gitlab-org/gitlab-ce/issues/39708)
+  it "GET /users/sign_out" do
+    expect(get("/users/sign_out")).to route_to('sessions#destroy')
   end
 
   it "POST /users/password" do