Simplify /auth sign in page.

10 files changed, 157 insertions, 102 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index b2a4d15df..a1d45dc70 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -3,6 +3,7 @@
 * Unreleased
     - Front end improvements:
         - Clearer relocation options while you’re reporting a problem #2238
+        - Simplify /auth sign in page. #2208
     - Admin improvements:
         - Allow moderation to potentially change category. #2320
     - Open311 improvements:
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 9f948e0f9..c194045b9 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -54,6 +54,20 @@ sub general : Path : Args(0) {
 
 }
 
+sub create : Path('create') : Args(0) {
+    my ( $self, $c ) = @_;
+    return unless $c->req->method eq 'POST';
+    $c->detach('code_sign_in');
+}
+
+sub forgot : Path('forgot') : Args(0) {
+    my ( $self, $c ) = @_;
+    $c->stash->{forgotten} = 1;
+    $c->stash->{template} = 'auth/create.html';
+    return unless $c->req->method eq 'POST';
+    $c->detach('code_sign_in');
+}
+
 sub authenticate : Private {
     my ($self, $c, $type, $username, $password) = @_;
     return 1 if $type eq 'email' && $c->authenticate({ email => $username, email_verified => 1, password => $password });
@@ -72,7 +86,6 @@ sub sign_in : Private {
 
     $username ||= '';
     my $password = $c->get_param('password_sign_in') || '';
-    my $remember_me = $c->get_param('remember_me') || 0;
 
     # Sign out just in case
     $c->logout();
@@ -86,10 +99,6 @@ sub sign_in : Private {
             $c->user->update({ password => $password });
         }
 
-        # unless user asked to be remembered limit the session to browser
-        $c->set_session_cookie_expire(0)
-          unless $remember_me;
-
         # Regenerate CSRF token as session ID changed
         $c->forward('get_csrf_token');
 
@@ -99,7 +108,6 @@ sub sign_in : Private {
     $c->stash(
         sign_in_error => 1,
         username => $username,
-        remember_me => $remember_me,
     );
     return;
 }
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 8cc7e4154..ffabc75f3 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -100,33 +100,6 @@ $mech->not_logged_in_ok;
     $mech->log_out_ok;
 }
 
-foreach my $remember_me ( '1', '0' ) {
-    subtest "sign in using valid details (remember_me => '$remember_me')" => sub {
-        $mech->get_ok('/auth');
-        $mech->submit_form_ok(
-            {
-                form_name => 'general_auth',
-                fields    => {
-                    username => $test_email,
-                    password_sign_in => $test_password,
-                    remember_me => ( $remember_me ? 1 : undef ),
-                },
-                button => 'sign_in_by_password',
-            },
-            "sign in with '$test_email' & '$test_password'"
-        );
-        is $mech->uri->path, '/my', "redirected to correct page";
-
-        my $expiry = $mech->session_cookie_expiry;
-        $remember_me
-          ? cmp_ok( $expiry, '>', 86400, "long expiry time" )
-          : is( $expiry, 0, "no expiry time" );
-
-        # logout
-        $mech->log_out_ok;
-    };
-}
-
 # try to sign in with bad details
 $mech->get_ok('/auth');
 $mech->submit_form_ok(
@@ -278,7 +251,7 @@ subtest "check logging in with token" => sub {
 };
 
 subtest 'check password length/common' => sub {
-    $mech->get_ok('/auth');
+    $mech->get_ok('/auth/create');
     $mech->submit_form_ok({
         form_name => 'general_auth',
         fields => { username => $test_email, password_register => 'short' },
@@ -300,6 +273,16 @@ subtest 'check common password AJAX call' => sub {
     $mech->content_contains("true");
 };
 
+subtest 'test forgotten password page' => sub {
+    $mech->get_ok('/auth/forgot');
+    $mech->content_contains('Forgot password');
+    $mech->submit_form_ok({
+        form_name => 'general_auth',
+        fields => { username => $test_email, password_register => 'squirblewirble' },
+        button => 'sign_in_by_code',
+    });
+};
+
 subtest "Test two-factor authentication login" => sub {
     use Auth::GoogleAuth;
     my $auth = Auth::GoogleAuth->new;
diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t
index 031fb8d9e..ac3d98b15 100644
--- a/t/app/controller/auth_social.t
+++ b/t/app/controller/auth_social.t
@@ -103,8 +103,8 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) {
                 # We don't have an email, so check that we can still submit it,
                 # and the ID carries through the confirmation
                 $fields->{username} = $fb_email;
-                $fields->{name} = 'Ffion Tester';
-                $mech->submit_form(with_fields => $fields);
+                $fields->{name} = 'Ffion Tester' unless $page eq 'my';
+                $mech->submit_form(with_fields => $fields, $page eq 'my' ? (button => 'sign_in_by_code') : ());
                 $mech->content_contains('Nearly done! Now check your email');
 
                 my $url = $mech->get_link_from_email;
@@ -211,8 +211,8 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) {
                 # We don't have an email, so check that we can still submit it,
                 # and the ID carries through the confirmation
                 $fields->{username} = $tw_email;
-                $fields->{name} = 'Ffion Tester';
-                $mech->submit_form(with_fields => $fields);
+                $fields->{name} = 'Ffion Tester' unless $page eq 'my';
+                $mech->submit_form(with_fields => $fields, $page eq 'my' ? (button => 'sign_in_by_code') : ());
                 $mech->content_contains('Nearly done! Now check your email');
 
                 my $url = $mech->get_link_from_email;
diff --git a/templates/web/base/auth/_username_error.html b/templates/web/base/auth/_username_error.html
index c0ddc135a..5454317b3 100644
--- a/templates/web/base/auth/_username_error.html
+++ b/templates/web/base/auth/_username_error.html
@@ -10,4 +10,6 @@
     };
     default = "other_$default";
     errors.$username_error || errors.$default;
+ELSIF sign_in_error;
+    loc('There was a problem with your login information.');
 END ~%]
diff --git a/templates/web/base/auth/create.html b/templates/web/base/auth/create.html
new file mode 100644
index 000000000..1886da95b
--- /dev/null
+++ b/templates/web/base/auth/create.html
@@ -0,0 +1,63 @@
+[%
+IF forgotten;
+    title = loc('Forgot password');
+ELSE;
+    title = loc('Create an account');
+END;
+
+INCLUDE 'header.html', bodyclass='authpage' %]
+
+<h1>
+    [% title %]
+    <small>
+        [% tprintf(loc('or <a href="%s">sign in</a>'), '/auth') %]
+    </small>
+</h1>
+
+[% IF forgotten %]
+<p>
+  [% IF c.config.SMS_AUTHENTICATION %]
+    [% loc('Sign in by email or text, providing a new password. When you click the link in your email or enter the SMS authentication code, your password will be updated.') %]</p>
+  [% ELSE %]
+    [% loc('Sign in by email instead, providing a new password. When you click the link in your email, your password will be updated.') %]</p>
+  [% END %]
+</p>
+[% END %]
+
+<form action="/auth/[% forgotten ? 'forgot' : 'create' %]" method="post" name="general_auth" class="validate">
+    <fieldset>
+
+        <input type="hidden" name="r" value="[% c.req.params.r | html %]">
+
+        [% loc_username_error = INCLUDE 'auth/_username_error.html' default='email' %]
+
+[% IF c.config.SMS_AUTHENTICATION %]
+    [% SET username_label = loc('Your email or mobile') %]
+[% ELSE %]
+    [% SET username_label = loc('Your email') %]
+[% END %]
+
+        <label class="n" for="username">[% username_label %]</label>
+      [% IF loc_username_error %]
+        <div class="form-error">[% loc_username_error %]</div>
+      [% END %]
+        <input type="text" class="form-control required" id="username" name="username" value="[% username | html %]" autofocus autocomplete="username">
+
+      [% IF field_errors.password_register %]
+        <p class='form-error'>[% field_errors.password_register %]</p>
+      [% END %]
+        <label for="password_register">[% forgotten ? loc('New password:') : loc('Your password') %]</label>
+
+        <div class="general-notes">
+            <p>[% tprintf(loc('Your password should include %d or more characters.'), c.cobrand.password_minimum_length) %]</p>
+        </div>
+
+        <div class="form-txt-submit-box">
+            <input class="required form-control js-password-validate" type="password" name="password_register" id="password_register" value="" autocomplete="new-password">
+            <input class="green-btn" type="submit" name="sign_in_by_code" value="[% loc('Sign in') %]">
+        </div>
+
+    </fieldset>
+</form>
+
+[% INCLUDE 'footer.html' %]
diff --git a/templates/web/base/auth/general.html b/templates/web/base/auth/general.html
index 4dbdb179f..e2e880871 100644
--- a/templates/web/base/auth/general.html
+++ b/templates/web/base/auth/general.html
@@ -1,6 +1,11 @@
 [% INCLUDE 'header.html', bodyclass='authpage', title = loc('Sign in or create an account') %]
 
-<h1>[% loc('Sign in') %]</h1>
+<h1>
+    [% loc('Sign in') %]
+    <small>
+        [% tprintf(loc('or <a href="%s">create an account</a>'), '/auth/create') %]
+    </small>
+</h1>
 
 [% TRY %][% INCLUDE 'auth/_general_top.html' %][% CATCH file %][% END %]
 
@@ -45,24 +50,18 @@
 [% END %]
 
         <label class="n" for="username">[% username_label %]</label>
-      [% IF loc_username_error  %]
+      [% IF loc_username_error %]
         <div class="form-error">[% loc_username_error %]</div>
-      [% ELSIF sign_in_error %]
-        <div class="form-error">[% loc('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the &lsquo;No&rsquo; section of the form.') %]</div>
       [% END %]
-        <input type="text" class="form-control required" id="username" name="username" value="[% username | html %]"
+        <input type="text" 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">
-            <h3>[% tprintf(loc("Do you have a %s password?", "%s is the site name"), site_name)  %]</h3>
           [% IF oauth_need_email %]
             [% INCLUDE form_sign_in_no %]
-            [% INCLUDE form_sign_in_yes %]
             <input type="hidden" name="oauth_need_email" value="1">
           [% ELSE %]
-          [% IF NOT field_errors.password_register %]
             [% INCLUDE form_sign_in_yes %]
-          [% END %]
             [% INCLUDE form_sign_in_no %]
           [% END %]
         </div>
@@ -77,61 +76,29 @@
 [% INCLUDE 'footer.html' %]
 
 [% BLOCK form_sign_in_yes %]
-    <div id="form_sign_in_yes" class="form-box">
-        <h5>[% loc('<strong>Yes</strong> I have a password') %]</h5>
-
+    <p class="hidden-nojs js-sign-in-password-hide">
+        <input class="btn btn--primary btn--block js-sign-in-password-btn" type="submit" name="sign_in_by_password" value="[% loc('Sign in with a password') %]">
+    </p>
+    <div class="hidden-js js-sign-in-password">
         <label for="password_sign_in">[% loc('Your password') %]</label>
 
         <div class="form-txt-submit-box">
-            <input type="password" name="password_sign_in" class="form-control" id="password_sign_in" value="">
+            <input type="password" name="password_sign_in" class="form-control" id="password_sign_in" value="" autocomplete="current-password">
             <input class="green-btn" type="submit" name="sign_in_by_password" value="[% loc('Sign in') %]">
         </div>
 
-        <div class="checkbox-group">
-            <input type="checkbox" id="remember_me" name="remember_me" value='1'[% ' checked' IF remember_me %]>
-            <label class="inline n" for="remember_me">[% loc('Keep me signed in on this computer') %]</label>
-        </div>
-
-        <div class="general-notes">
-            <p><strong>[% loc('Forgotten your password?') %]</strong>
-          [% IF c.config.SMS_AUTHENTICATION %]
-            [% loc('Sign in by email or text, providing a new password. When you click the link in your email or enter the SMS authentication code, your password will be updated.') %]</p>
-          [% ELSE %]
-            [% loc('Sign in by email instead, providing a new password. When you click the link in your email, your password will be updated.') %]</p>
-          [% END %]
-        </div>
-
+        <p>
+            <a href="/auth/forgot">[% loc('Forgotten your password?') %]</a>
+        </p>
     </div>
 [% END %]
 
 [% BLOCK form_sign_in_no %]
-    <div id="form_sign_in_no" class="form-box">
-      [% IF c.config.SMS_AUTHENTICATION %]
-        <h5>[% loc('<strong>No</strong> let me sign in by email or text') %]</h5>
-      [% ELSE %]
-        <h5>[% loc('<strong>No</strong> let me sign in by email') %]</h5>
-      [% END %]
-
-        <label for="name">[% loc('Your name') %]</label>
-        <input class="form-control" type="text" name="name" value="">
-
-        <label for="password_register">[% loc('Password (optional)') %]</label>
-      [% IF field_errors.password_register %]
-        <p class='form-error'>[% field_errors.password_register %]</p>
-      [% END %]
-
-        <div class="general-notes">
-            <p>[% loc('Providing a name and password is optional, but doing so will allow you to more easily report problems, leave updates and manage your reports.') %]</p>
-        </div>
-
-        <div class="form-txt-submit-box">
-            <input class="form-control js-password-validate" type="password" name="password_register" id="password_register" value="">
-            <input class="green-btn" type="submit" name="sign_in_by_code" value="[% loc('Sign in') %]">
-        </div>
-
-        <div class="general-notes">
-            <p>[% tprintf(loc('Your password should include %d or more characters.'), c.cobrand.password_minimum_length) %]</p>
-        </div>
-
-    </div>
+    <p><input class="fake-link" type="submit" name="sign_in_by_code" value="
+      [%~ IF c.config.SMS_AUTHENTICATION %]
+        [%~ loc('Email me a link or text me a code to sign in') %]
+      [%~ ELSE %]
+        [%~ loc('Email me a link to sign in') %]
+      [%~ END ~%]
+      "></p>
 [% END %]
diff --git a/templates/web/zurich/auth/general.html b/templates/web/zurich/auth/general.html
index 2b183b953..29fff2600 100644
--- a/templates/web/zurich/auth/general.html
+++ b/templates/web/zurich/auth/general.html
@@ -14,8 +14,6 @@
             <label class="n" for="username">[% loc('Email') %]</label>
           [% IF loc_username_error  %]
             <div class="form-error">[% loc_username_error %]</div>
-          [% ELSIF sign_in_error %]
-            <div class="form-error">[% loc('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the &lsquo;No&rsquo; section of the form.') %]</div>
           [% END %]
             <input type="email" class="required email" id="username" name="username" value="[% username | html %]" autofocus>
 
@@ -25,11 +23,6 @@
                 <input class="green-btn" type="submit" name="sign_in_by_password" value="[% loc('Sign in') %]">
             </div>
 
-            <div class="form-txt-submit-box">
-                <input type="checkbox" id="remember_me" name="remember_me" value='1'[% ' checked' IF remember_me %]>
-                <label class="inline n" for="remember_me">[% loc('Keep me signed in on this computer') %]</label>
-            </div>
-
         </div>
     </fieldset>
 </form>
diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js
index 97d53dd01..ac064de99 100644
--- a/web/cobrands/fixmystreet/fixmystreet.js
+++ b/web/cobrands/fixmystreet/fixmystreet.js
@@ -814,6 +814,22 @@ $.extend(fixmystreet.set_up, {
   },
 
   email_login_form: function() {
+    // Password form split up
+    $('.js-sign-in-password-btn').click(function(e) {
+        if ($('.js-sign-in-password').is(':visible')) {
+        } else {
+            e.preventDefault();
+            $('.js-sign-in-password-hide').hide();
+            $('.js-sign-in-password').show().css('visibility', 'visible');
+            $('#password_sign_in').focus();
+        }
+    });
+    // This is if the password box is filled programmatically (by
+    // e.g. 1Password), show it so that it will auto-submit.
+    $('#password_sign_in').change(function() {
+        $('.js-sign-in-password').show().css('visibility', 'visible');
+    });
+
     // Log in with email button
     var email_form = $('#js-social-email-hide'),
         button = $('<button class="btn btn--social btn--social-email">'+translation_strings.login_with_email+'</button>'),
@@ -826,6 +842,7 @@ $.extend(fixmystreet.set_up, {
     form_box.append(button).insertBefore(email_form);
     if ($('.form-error').length) {
         button.click();
+        $('.js-sign-in-password-btn').click();
     }
   },
 
diff --git a/web/cobrands/sass/_base.scss b/web/cobrands/sass/_base.scss
index 24eed252b..7715ff5e3 100644
--- a/web/cobrands/sass/_base.scss
+++ b/web/cobrands/sass/_base.scss
@@ -48,7 +48,17 @@ h1 {
   font-weight: normal;
   margin-top: 0.5em;
   margin-bottom: 0.5em;
+
+  // Useful for extra inline text after a heading
+  small {
+    display: inline-block; // break onto its own line if not enough horizontal space
+    font-style: inherit;
+    font-family: inherit;
+    font-size: 0.5em; // back down to 1em from the 2em parent
+    line-height: 1em;
+  }
 }
+
 h1#reports_heading {
   margin-bottom: 0;
 }
@@ -172,7 +182,8 @@ select {
   width: 100%;
 }
 
-a {
+a,
+.fake-link {
   text-decoration: none;
   color: #0BA7D1;
 
@@ -187,6 +198,16 @@ a {
   }
 }
 
+.fake-link {
+  display: inline-block;
+  background: transparent;
+  border: none;
+  font-family: inherit;
+  font-size: 1em;
+  padding: 0;
+  margin: 0;
+}
+
 // custom type
 .small-print {
   @extend small;