Avoid CSP issues with OpenID login

To avoid Chrom getting upset about sending form data to sites
that our policy doesn't allow, even when it isn't, use Javascript
to jump straight to Omniauth as the direct OpenID based login
buttons were already doing.

Fixes #1909

app/assets/javascripts/login.js

@@ -22,4 +22,18 @@ $(document).ready(function() {
   // Hide OpenID field for now
   $("#login_openid_url").hide();
   $("#login_openid_submit").hide();
+
+  // Handle OpenID submission by redirecting to omniauth
+  $("#openid_login_form").submit(function() {
+    var action = $(this).prop("action"),
+        openid_url = $(this).find("#openid_url").val(),
+        referer = $(this).find("#openid_referer").val(),
+        args = {};
+    args.openid_url = openid_url;
+    if (referer) {
+      args.referer = referer;
+    }
+    window.location = action + "?" + querystring.stringify(args);
+    return false;
+  });
 });

app/controllers/users_controller.rb

@@ -264,9 +264,6 @@ class UsersController < ApplicationController
     if params[:username].present? && params[:password].present?
       session[:remember_me] ||= params[:remember_me]
       password_authentication(params[:username], params[:password])
-    elsif params[:openid_url].present?
-      session[:remember_me] ||= params[:remember_me_openid]
-      redirect_to auth_url("openid", params[:openid_url], params[:referer])
     end
   end
 

app/views/users/login.html.erb

@@ -40,6 +40,13 @@
         <%= submit_tag t('.login_button'), :tabindex => 4 %>
       </fieldset>
 
+    </div>
+
+  <% end %>
+
+  <%= form_tag(auth_path(:provider => "openid"), { :id => "openid_login_form" }) do %>
+    <div id="loginForm" class="standard-form">
+
       <fieldset class='form-divider'>
 
         <p class='standard-label'><%= t '.with external' %></p>
@@ -68,15 +75,11 @@
 
         <div id='login_openid_url' class='form-row'>
           <label for='openid_url' class="standard-label"><%= raw t '.openid', :logo => openid_logo %></label>
+          <%= hidden_field_tag("openid_referer", params[:referer]) if params[:referer] %>
           <%= text_field_tag("openid_url", "", { :tabindex => 3, :class => "openid_url" }) %>
           <span class="minorNote">(<a href="<%= t 'users.account.openid.link' %>" target="_new"><%= t 'users.account.openid.link text' %></a>)</span>
         </div>
 
-        <div class='form-row'>
-          <%= check_box_tag "remember_me_openid", "yes", false, :tabindex => 5 %>
-          <label class="standard-label" for="remember_me_openid"><%= t '.remember' %></label>
-        </div>
-
         <%= submit_tag t('.login_button'), :tabindex => 6, :id => "login_openid_submit" %>
 
       </fieldset>

test/integration/user_login_test.rb

@@ -359,10 +359,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_response :success
     assert_template "users/login"
-    post "/login", :params => { :openid_url => "http://localhost:1123/john.doe", :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
+    get auth_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history")
     assert_response :redirect
     assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/john.doe", :origin => "/login?referer=%2Fhistory", :referer => "/history")
     follow_redirect!
@@ -373,31 +370,6 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     assert_select "span.username", user.display_name
   end
 
-  def test_login_openid_remember_me
-    user = create(:user, :auth_provider => "openid", :auth_uid => "http://example.com/john.doe")
-    OmniAuth.config.add_mock(:openid, :uid => user.auth_uid)
-
-    get "/login", :params => { :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to :controller => :users, :action => :login, :cookie_test => true, :referer => "/history"
-    follow_redirect!
-    assert_response :success
-    assert_template "users/login"
-    post "/login", :params => { :openid_url => user.auth_uid, :remember_me_openid => true, :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
-    assert_response :redirect
-    assert_redirected_to auth_success_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
-    assert_response :redirect
-    follow_redirect!
-    assert_response :success
-    assert_template "changesets/history"
-    assert_select "span.username", user.display_name
-    assert session.key?(:_remember_for)
-  end
-
   def test_login_openid_connection_failed
     user = create(:user, :auth_provider => "openid", :auth_uid => "http://example.com/john.doe")
     OmniAuth.config.mock_auth[:openid] = :connection_failed
@@ -408,10 +380,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_response :success
     assert_template "users/login"
-    post "/login", :params => { :openid_url => user.auth_uid, :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
+    get auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
     assert_response :redirect
     assert_redirected_to auth_success_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
     follow_redirect!
@@ -436,10 +405,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_response :success
     assert_template "users/login"
-    post "/login", :params => { :openid_url => user.auth_uid, :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
+    get auth_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
     assert_response :redirect
     assert_redirected_to auth_success_path(:provider => "openid", :openid_url => user.auth_uid, :origin => "/login?referer=%2Fhistory", :referer => "/history")
     follow_redirect!
@@ -463,10 +429,7 @@ class UserLoginTest < ActionDispatch::IntegrationTest
     follow_redirect!
     assert_response :success
     assert_template "users/login"
-    post "/login", :params => { :openid_url => "http://localhost:1123/fred.bloggs", :referer => "/history" }
-    assert_response :redirect
-    assert_redirected_to auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history")
-    follow_redirect!
+    get auth_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history")
     assert_response :redirect
     assert_redirected_to auth_success_path(:provider => "openid", :openid_url => "http://localhost:1123/fred.bloggs", :origin => "/login?referer=%2Fhistory", :referer => "/history")
     follow_redirect!