Prevent CSRF bypass with login form

app/controllers/users_controller.rb

@@ -276,7 +276,7 @@ def login
 
     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

test/controllers/users_controller_test.rb

@@ -406,6 +406,25 @@ def test_save_referer_params
     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

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 @@ def test_oauth10_web_app
   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 @@ def test_oauth10_desktop_app
   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 @@ def test_oauth10a_web_app
   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
 

test/integration/page_locale_test.rb

@@ -12,6 +12,8 @@ def teardown
   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 @@ def test_override
     get "/diary", :params => { :locale => "es" }
     assert_select "html[lang=?]", "es"
 
+    get "/login"
+    follow_redirect!
     post "/login", :params => { :username => user.email, :password => "test" }
     follow_redirect!
 

test/test_helper.rb

@@ -243,6 +243,7 @@ def sign_in_as(user)
     end
 
     def session_for(user)
+      get login_path
       post login_path, :params => { :username => user.display_name, :password => "test" }
       follow_redirect!
     end