Make SSO button use separate form.

This reduces confusion if a browser auto-fills the email form but someone still clicks the SSO button, or similar.
mysociety · Sep 19, 2024 · 67eadeb · 67eadeb
1 parent 9e00579
commit 67eadeb
Show file tree

Hide file tree

Showing 4 changed files with 40 additions and 46 deletions.
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -44,10 +44,7 @@ sub general : Path : Args(0) {
 
     # decide which action to take
     $c->detach('code_sign_in') if $clicked_sign_in_by_code || ($data_email && !$data_password);
-    if (!$data_username && !$data_password && !$data_email && $c->get_param('social_sign_in')) {
-        $c->forward('social/handle_sign_in');
-    }
-
+    $c->detach('social/handle_sign_in') if $c->get_param('social_sign_in');
     $c->forward( 'sign_in', [ $data_username ] )
         && $c->detach( 'redirect_on_signin', [ $c->get_param('r') ] );
 

diff --git a/templates/web/base/auth/general.html b/templates/web/base/auth/general.html
@@ -18,11 +18,9 @@ <h1>
     <p class="form-error">[% loc('Sorry, we could not log you in. Please fill in the form below.') %]</p>
 [% END %]
 
+[% IF NOT oauth_need_email AND c.cobrand.social_auth_enabled %]
 <form action="/auth" method="post" name="general_auth" class="validate">
-
     <input type="hidden" name="r" value="[% c.req.params.r | html %]">
-
-[% IF NOT oauth_need_email AND c.cobrand.social_auth_enabled %]
     [% IF c.config.FACEBOOK_APP_ID %]
       <div class="form-box">
         <button name="social_sign_in" id="facebook_sign_in" value="facebook" class="btn btn--block btn--social btn--facebook">
@@ -47,10 +45,13 @@ <h1>
         </button>
       </div>
     [% END %]
-      <div id="js-social-email-hide">
+</form>
 [% END %]
 
-        [% loc_username_error = INCLUDE 'auth/_username_error.html' default='email' %]
+<form action="/auth" method="post" name="general_auth" class="validate">
+    <input type="hidden" name="r" value="[% c.req.params.r | html %]">
+
+    [% loc_username_error = INCLUDE 'auth/_username_error.html' default='email' %]
 
 [% IF c.cobrand.sms_authentication %]
     [% SET username_label = loc('Your email or mobile') %]
@@ -60,26 +61,22 @@ <h1>
     [% SET username_type = 'email' %]
 [% END %]
 
-        <label class="n" for="username">[% username_label %]</label>
-      [% IF loc_username_error %]
-        <div class="form-error">[% loc_username_error %]</div>
+    <label class="n" for="username">[% username_label %]</label>
+  [% IF loc_username_error %]
+    <div class="form-error">[% loc_username_error %]</div>
+  [% END %]
+    <input type="[% username_type %]" class="form-control required" id="username" name="username" value="[% username | html %]" autocomplete="username"
+    [%~ IF c.cobrand.moniker != 'borsetshire' %] autofocus[% END %]>
+
+    <div id="form_sign_in">
+      [% IF oauth_need_email %]
+        [% INCLUDE form_sign_in_no %]
+        <input type="hidden" name="oauth_need_email" value="1">
+      [% ELSE %]
+        [% INCLUDE form_sign_in_yes %]
+        [% INCLUDE form_sign_in_no %]
       [% END %]
-        <input type="[% username_type %]" class="form-control required" id="username" name="username" value="[% username | html %]" autocomplete="username"
-        [%~ IF c.cobrand.moniker != 'borsetshire' %] autofocus[% END %]>
-
-        <div id="form_sign_in">
-          [% IF oauth_need_email %]
-            [% INCLUDE form_sign_in_no %]
-            <input type="hidden" name="oauth_need_email" value="1">
-          [% ELSE %]
-            [% INCLUDE form_sign_in_yes %]
-            [% INCLUDE form_sign_in_no %]
-          [% END %]
-        </div>
-
-[% IF NOT oauth_need_email AND c.cobrand.social_auth_enabled %]
-      </div>
-[% END %]
+    </div>
 
 </form>
 

diff --git a/templates/web/camden/auth/general.html b/templates/web/camden/auth/general.html
@@ -42,11 +42,19 @@ <h1>
       [% ELSE %]
         [% INCLUDE form_sign_in_yes %]
         [% INCLUDE form_sign_in_no %]
-        [% INCLUDE form_sign_in_camden_staff %]
       [% END %]
     </div>
 </form>
 
+[% IF c.cobrand.feature('oidc_login') AND NOT oauth_need_email %]
+<form action="/auth" method="post" name="general_auth" class="validate">
+    <input type="hidden" name="r" value="[% c.req.params.r | html %]">
+    <button name="social_sign_in" id="oidc_sign_in" value="oidc" class="fake-link sso-staff-sign-in">
+        Camden Staff Sign-in
+    </button>
+</form>
+[% END %]
+
 [% INCLUDE 'footer.html' %]
 
 [% BLOCK form_sign_in_yes %]
@@ -76,11 +84,3 @@ <h1>
       [%~ END ~%]
       "></p>
 [% END %]
-
-[% BLOCK form_sign_in_camden_staff %]
-  [% IF c.cobrand.feature('oidc_login') %]
-      <button name="social_sign_in" id="oidc_sign_in" value="oidc" class="fake-link sso-staff-sign-in">
-          Camden Staff Sign-in
-      </button>
-  [% END %]
-[% END %]
diff --git a/templates/web/hackney/auth/general.html b/templates/web/hackney/auth/general.html
@@ -44,11 +44,19 @@ <h1>
       [% ELSE %]
         [% INCLUDE form_sign_in_yes %]
         [% INCLUDE form_sign_in_no %]
-        [% INCLUDE form_sign_in_hackney_staff %]
       [% END %]
     </div>
 </form>
 
+[% IF c.cobrand.feature('oidc_login') AND NOT oauth_need_email %]
+<form action="/auth" method="post" name="general_auth" class="validate">
+    <input type="hidden" name="r" value="[% c.req.params.r | html %]">
+    <button name="social_sign_in" id="oidc_sign_in" value="oidc" class="fake-link sso-staff-sign-in">
+        Hackney Staff Sign-in
+    </button>
+</form>
+[% END %]
+
 [% INCLUDE 'footer.html' %]
 
 [% BLOCK form_sign_in_yes %]
@@ -78,11 +86,3 @@ <h1>
       [%~ END ~%]
       "></p>
 [% END %]
-
-[% BLOCK form_sign_in_hackney_staff %]
-  [% IF c.cobrand.feature('oidc_login') %]
-      <button name="social_sign_in" id="oidc_sign_in" value="oidc" class="fake-link sso-staff-sign-in">
-          Hackney Staff Sign-in
-      </button>
-  [% END %]
-[% END %]