Use a post link to logout

This avoids needing to access the session id, which is currently
only working with the memcache store.

The fallback page is preserved for anyone who wants to logout without
using javascript.

Refs #2488

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index a61a10d94f5c098606a3dffd7d59e5854fc61a88..514b3f8ee73f41d8d2284886ac11c1dde1addf47 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -269,7 +269,7 @@ class UsersController < ApplicationController
   def logout
     @title = t "users.logout.title"
 
-    if params[:session] == session.id
+    if request.post?
       if session[:token]
         token = UserToken.find_by(:token => session[:token])
         token&.destroy

diff --git a/app/views/layouts/_header.html.erb b/app/views/layouts/_header.html.erb
index 6df8f02da2a1e43a0227a9a9ff583aae19e667de..3963c211e10e95c3e803e96f648d85b26437013a 100644 (file)
--- a/app/views/layouts/_header.html.erb
+++ b/app/views/layouts/_header.html.erb
@@ -102,7 +102,7 @@
             <%= yield :greeting %>
           </li>
           <li>
-            <%= link_to t("layouts.logout"), logout_path(:session => session.id, :referer => request.fullpath), :class => "geolink" %>
+            <%= link_to t("layouts.logout"), logout_path(:referer => request.fullpath), :method => "post", :class => "geolink" %>
           </li>
         </ul>
       </div>

diff --git a/app/views/users/logout.html.erb b/app/views/users/logout.html.erb
index 273c7e1b94d0c7f39fd042481a2fbbb058e9c0c5..5d8e2de492420246e5c5b086937897a846db86c5 100644 (file)
--- a/app/views/users/logout.html.erb
+++ b/app/views/users/logout.html.erb
@@ -4,6 +4,5 @@
 
 <%= form_tag :action => "logout" do %>
   <%= hidden_field_tag("referer", h(params[:referer])) %>
-  <%= hidden_field_tag("session", session.id) %>
   <%= submit_tag t(".logout_button") %>
 <% end %>

diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index feca92df56dbcbe594bdbc2b756b2df737d3c398..4417d353facae1e55092a6dbb86e4b8f03b2886f 100644 (file)
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -344,27 +344,13 @@ class UsersControllerTest < ActionController::TestCase
   end
 
   def test_logout_without_referer
-    get :logout
-    assert_response :success
-    assert_template :logout
-    assert_select "input[name=referer][value=?]", ""
-
-    session_id = assert_select("input[name=session]").first["value"]
-
-    get :logout, :params => { :session => session_id }
+    post :logout
     assert_response :redirect
     assert_redirected_to root_path
   end
 
   def test_logout_with_referer
-    get :logout, :params => { :referer => "/test" }
-    assert_response :success
-    assert_template :logout
-    assert_select "input[name=referer][value=?]", "/test"
-
-    session_id = assert_select("input[name=session]").first["value"]
-
-    get :logout, :params => { :session => session_id, :referer => "/test" }
+    post :logout, :params => { :referer => "/test" }
     assert_response :redirect
     assert_redirected_to "/test"
   end
@@ -374,16 +360,7 @@ class UsersControllerTest < ActionController::TestCase
 
     session[:token] = token.token
 
-    get :logout
-    assert_response :success
-    assert_template :logout
-    assert_select "input[name=referer][value=?]", ""
-    assert_equal token.token, session[:token]
-    assert_not_nil UserToken.where(:id => token.id).first
-
-    session_id = assert_select("input[name=session]").first["value"]
-
-    get :logout, :params => { :session => session_id }
+    post :logout
     assert_response :redirect
     assert_redirected_to root_path
     assert_nil session[:token]

diff --git a/test/system/user_logout_test.rb b/test/system/user_logout_test.rb
new file mode 100644 (file)
index 0000000..a2e145f
--- /dev/null
+++ b/test/system/user_logout_test.rb
@@ -0,0 +1,22 @@
+require "application_system_test_case"
+
+class UserLogoutTest < ApplicationSystemTestCase
+  test "Sign out via link" do
+    user = create(:user)
+    sign_in_as(user)
+
+    click_on user.display_name
+    click_on "Log Out"
+    assert page.has_content? "Log In"
+  end
+
+  test "Sign out via fallback page" do
+    sign_in_as(create(:user))
+
+    visit logout_path
+    assert page.has_content? "Logout from OpenStreetMap"
+
+    click_button "Logout"
+    assert page.has_content? "Log In"
+  end
+end