Prevent CSRF bypass with login form

diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb
index c2cbca4ae8defe29675e24ab933c37c732e49aa2..aa504e3131a23380572d4f4d70d8e20aa9e59f3a 100644 (file)
--- a/app/controllers/users_controller.rb
+++ b/app/controllers/users_controller.rb
@@ -276,7 +276,7 @@ class UsersController < ApplicationController
 
     session[:referer] = safe_referer(params[:referer]) if params[:referer]
 
-    if params[:username].present? && params[:password].present?
+    if request.post?
       session[:remember_me] ||= params[:remember_me]
       password_authentication(params[:username], params[:password])
     end

diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb
index ff75df548e827b9bb2986be4620c9de713d96687..54c737b97e39ebac266287f537600e0fa3176973 100644 (file)
--- a/test/controllers/users_controller_test.rb
+++ b/test/controllers/users_controller_test.rb
@@ -406,6 +406,25 @@ class UsersControllerTest < ActionDispatch::IntegrationTest
     ActionMailer::Base.deliveries.clear
   end
 
+  def test_login
+    user = create(:user)
+
+    get login_path
+    assert_response :redirect
+    assert_redirected_to login_path(:cookie_test => true)
+    follow_redirect!
+    assert_response :success
+    assert_template "login"
+
+    get login_path, :params => { :username => user.display_name, :password => "test" }
+    assert_response :success
+    assert_template "login"
+
+    post login_path, :params => { :username => user.display_name, :password => "test" }
+    assert_response :redirect
+    assert_redirected_to root_path
+  end
+
   def test_logout_without_referer
     post logout_path
     assert_response :redirect

diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb
index 3242df9e2619d96878c13cc96fd4ca561724073a..1505cb34ab4fcc06b30d99057fd6a193236d79b8 100644 (file)
--- a/test/integration/oauth_test.rb
+++ b/test/integration/oauth_test.rb
@@ -6,8 +6,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
   def test_oauth10_web_app
     client = create(:client_application, :callback_url => "http://some.web.app.example.org/callback", :allow_read_prefs => true, :allow_write_api => true, :allow_read_gpx => true)
 
-    post "/login", :params => { :username => client.user.email, :password => "test" }
+    get "/login"
     follow_redirect!
+    post "/login", :params => { :username => client.user.email, :password => "test" }
     follow_redirect!
     assert_response :success
 
@@ -19,8 +20,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
   def test_oauth10_desktop_app
     client = create(:client_application, :allow_read_prefs => true, :allow_write_api => true, :allow_read_gpx => true)
 
-    post "/login", :params => { :username => client.user.email, :password => "test" }
+    get "/login"
     follow_redirect!
+    post "/login", :params => { :username => client.user.email, :password => "test" }
     follow_redirect!
     assert_response :success
 
@@ -31,8 +33,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
   def test_oauth10a_web_app
     client = create(:client_application, :callback_url => "http://some.web.app.example.org/callback", :allow_read_prefs => true, :allow_write_api => true, :allow_read_gpx => true)
 
-    post "/login", :params => { :username => client.user.email, :password => "test" }
+    get "/login"
     follow_redirect!
+    post "/login", :params => { :username => client.user.email, :password => "test" }
     follow_redirect!
     assert_response :success
 
@@ -44,8 +47,9 @@ class OAuthTest < ActionDispatch::IntegrationTest
   def test_oauth10a_desktop_app
     client = create(:client_application, :allow_read_prefs => true, :allow_write_api => true, :allow_read_gpx => true)
 
-    post "/login", :params => { :username => client.user.email, :password => "test" }
+    get "/login"
     follow_redirect!
+    post "/login", :params => { :username => client.user.email, :password => "test" }
     follow_redirect!
     assert_response :success
 

diff --git a/test/integration/page_locale_test.rb b/test/integration/page_locale_test.rb
index 2c4939b054fddf456565505dbd48802e4215c4f1..239c1d847b20da20878d080def9a54ea5888f8d5 100644 (file)
--- a/test/integration/page_locale_test.rb
+++ b/test/integration/page_locale_test.rb
@@ -12,6 +12,8 @@ class PageLocaleTest < ActionDispatch::IntegrationTest
   def test_defaulting
     user = create(:user, :languages => [])
 
+    get "/login"
+    follow_redirect!
     post "/login", :params => { :username => user.email, :password => "test" }
     follow_redirect!
 
@@ -33,6 +35,8 @@ class PageLocaleTest < ActionDispatch::IntegrationTest
     get "/diary", :params => { :locale => "es" }
     assert_select "html[lang=?]", "es"
 
+    get "/login"
+    follow_redirect!
     post "/login", :params => { :username => user.email, :password => "test" }
     follow_redirect!
 

diff --git a/test/test_helper.rb b/test/test_helper.rb
index a023dd61026ebfd671b25854d7486f645f9237ed..da36be28b7f10bff08912e68b2252e740747a82f 100644 (file)
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -243,6 +243,7 @@ module ActiveSupport
     end
 
     def session_for(user)
+      get login_path
       post login_path, :params => { :username => user.display_name, :password => "test" }
       follow_redirect!
     end