Avoid CSP issues with OpenID login
authorTom Hughes <tom@compton.nu>
Mon, 25 Feb 2019 11:44:24 +0000 (11:44 +0000)
committerTom Hughes <tom@compton.nu>
Mon, 25 Feb 2019 11:46:12 +0000 (11:46 +0000)
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
app/controllers/users_controller.rb
app/views/users/login.html.erb
test/integration/user_login_test.rb

index b7a540f..3496587 100644 (file)
@@ -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;
+  });
 });
index d1a94b9..e872712 100644 (file)
@@ -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
 
index 0944d78..04a0415 100644 (file)
         <%= 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>
 
         <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>
index 89c3a8a..b83b90a 100644 (file)
@@ -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!