diff options
-rw-r--r-- | app/views/layouts/header/_default.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/feature-change-signout-route.yml | 5 | ||||
-rw-r--r-- | config/initializers/devise.rb | 2 | ||||
-rw-r--r-- | lib/gitlab/middleware/read_only.rb | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/middleware/read_only_spec.rb | 7 | ||||
-rw-r--r-- | spec/routing/routing_spec.rb | 6 |
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 |