Completed auth section (main parts at least)

10 files changed, 314 insertions, 188 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index b21981417..2069b3903 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -34,7 +34,7 @@ sub general : Path : Args(0) {
     return unless $req->method eq 'POST';
 
     # check that the email is valid - otherwise flag an error
-    my $raw_email = $req->param('email') || '';
+    my $raw_email = lc( $req->param('email') || '' );
     my $email_checker = Email::Valid->new(
         -mxcheck  => 1,
         -tldcheck => 1,
@@ -52,7 +52,8 @@ sub general : Path : Args(0) {
     }
 
     # decide which action to take
-    $c->detach('create_account') if $req->param('create_account');
+    $c->detach('login')       if $req->param('login');
+    $c->detach('email_login') if $req->param('email_login');
 
     # hmm - should not get this far. 404 so that user knows there is a problem
     # rather than it silently not working.
@@ -60,78 +61,131 @@ sub general : Path : Args(0) {
 
 }
 
-=head2 create_account
+=head2 login
 
-Create an account for the user, send them an email with confirm link and log
-them in straight away. If the email address already has an account send them an
-email with a password reset link (slightly leaks privacy information but
-required to allow instant logins).
+Allow the user to legin with a username and a password.
 
 =cut
 
-sub create_account : Private {
+sub login : Private {
     my ( $self, $c ) = @_;
-    my $email = $c->stash->{email};
 
-    # get account from the database
-    my $account = $c->model('DB::User')->find_or_new( { email => $email } );
+    my $email    = $c->stash->{email}         || '';
+    my $password = $c->req->param('password') || '';
 
-    # Deal with existing accounts by treating it like a password reset link
-    if ( $account->in_storage ) {
-        $c->stash->{tried_to_create_account} = 1;
-        $c->detach('email_reset');
-    }
+    # logout just in case
+    $c->logout();
 
-    # we have a new account
-    my $password = mySociety::AuthToken::random_token();
-    $account->password( sha1_hex($password) );
-    $account->insert;    # save to database
+    if ( $c->authenticate( { email => $email, password => $password } ) ) {
+        $c->res->redirect( $c->uri_for('/my') );
+        return;
+    }
 
-    # log the user in, send them an email and redirect to the welcome page
-    $c->authenticate( { email => $email, password => $password } );
-    $c->send_email( 'auth_new_account_welcome', { to => $email } );
-    $c->res->redirect( $c->uri_for('welcome') );
+    # could not authenticate - show an error
+    $c->stash->{login_error} = 1;
 }
 
-=head2 welcome
+=head2 email_login
 
-Page that new users are redirected to after they have created an account.
+Email the user the details they need to log in. Don't check for an account - if
+there isn't one we can create it when they come back with a token (which
+contains the email addresss).
 
 =cut
 
-sub welcome : Local {
+sub email_login : Private {
     my ( $self, $c ) = @_;
+    my $email = $c->stash->{email};
 
-    # FIXME - check that user is logged in!
-    # pass thru
-}
+    my $token_obj = $c->model('DB::Token')    #
+      ->create(
+        {
+            scope => 'email_login',
+            data  => { email => $email }
+        }
+      );
 
-=head2 confirm
+    # log the user in, send them an email and redirect to the welcome page
+    $c->stash->{token} = $token_obj->token;
+    $c->send_email( 'login', { to => $email } );
+    $c->res->redirect( $c->uri_for('token') );
+}
 
-Confirm that a user can receive email - url is .../confirm/$token
+=head2 token
 
-We don't assume that the user is logged in, but if they are they are logged out
-and then logged in as the user they are confirming. The token is destroyed at
-the end of the request so it cannot be reused.
+Handle the 'email_login' tokens. Find the account for the email address
+(creating if needed), authenticate the user and delete the token.
 
 =cut
 
-sub confirm : Local {
+sub token : Local {
     my ( $self, $c, $url_token ) = @_;
 
-    # Use the token to confirm the user and return them.
-    my $user = $c->model('DB::User')->confirm_user_from_token($url_token);
+    # check for a token - if none found then return
+    return unless $url_token;
 
-    # If we did not get a user back then the token was not valid
-    return if !$user;
+    # retrieve the token or return
+    my $token_obj =
+      $c->model('DB::Token')
+      ->find( { scope => 'email_login', token => $url_token, } );
+
+    if ( !$token_obj ) {
+        $c->stash->{token_not_found} = 1;
+        return;
+    }
 
-    # got a user back which is now confirmed - auth as them
+    # logout in case we are another user
     $c->logout();
+
+    # get the email and scrap the token
+    my $email = $token_obj->data->{email};
+    $token_obj->delete;
+
+    # find or create the user related to the token and delete the token
+    my $user = $c->model('DB::User')->find_or_create( { email => $email } );
     $c->authenticate( { email => $user->email }, 'no_password' );
-    $c->stash->{user_now_confirmed} = 1;
 
-    # TODO - should we redirect somewhere - perhaps to pending problems?
-    return;
+    # send the user to their page
+    $c->res->redirect( $c->uri_for('/my') );
+}
+
+=head2 change_password
+
+Let the user change their password.
+
+=cut
+
+sub change_password : Local {
+    my ( $self, $c ) = @_;
+
+    # FIXME - should be logged in
+    # FIXME - CSRF check here
+    # FIXME - minimum criteria for passwords (length, contain number, etc)
+
+    # If not a post then no submission
+    return unless $c->req->method eq 'POST';
+
+    # get the passwords
+    my $new     = $c->req->param('new_password') // '';
+    my $confirm = $c->req->param('confirm')      // '';
+
+    # check for errors
+    my $password_error =
+       !$new && !$confirm ? 'missing'
+      : $new ne $confirm ? 'mismatch'
+      :                    '';
+
+    if ($password_error) {
+        $c->stash->{password_error} = $password_error;
+        $c->stash->{new_password}   = $new;
+        $c->stash->{confirm}        = $confirm;
+        return;
+    }
+
+    # we should have a usable password - save it to the user
+    $c->user->obj->update( { password => sha1_hex($new) } );
+    $c->stash->{password_changed} = 1;
+
 }
 
 =head2 logout
diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm
index 099ec2349..d88de5e7d 100644
--- a/perllib/FixMyStreet/DB/Result/User.pm
+++ b/perllib/FixMyStreet/DB/Result/User.pm
@@ -32,30 +32,5 @@ __PACKAGE__->add_unique_constraint( "users_email_key", ["email"] );
 # Created by DBIx::Class::Schema::Loader v0.07009 @ 2011-03-03 10:05:03
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:4dpBN1I88nB1BYtHT/AfKA
 
-=head2 create_confirm_token
-
-    $token = $user->create_confirm_token();
-
-Create a token that can be emailed to the user. When it is returned it can be
-used to confirm that the email address works.
-
-See also the ::ResultSet::User method 'confirm_user_from_token'.
-
-=cut
-
-sub create_confirm_token {
-    my $self = shift;
-
-    my $token_rs = $self->result_source->schema->resultset('Token');
-
-    my $token_obj = $token_rs->create(
-        {
-            scope => 'user_confirm',            #
-            data => { email => $self->email }
-        }
-    );
-
-    return $token_obj->token;
-}
 
 1;
diff --git a/perllib/FixMyStreet/DB/ResultSet/User.pm b/perllib/FixMyStreet/DB/ResultSet/User.pm
index a84286a78..7e657a936 100644
--- a/perllib/FixMyStreet/DB/ResultSet/User.pm
+++ b/perllib/FixMyStreet/DB/ResultSet/User.pm
@@ -4,42 +4,5 @@ use base 'DBIx::Class::ResultSet';
 use strict;
 use warnings;
 
-=head2 confirm_user_from_token
-
-    $user = $rs->confirm_user_from_token( $token );
-
-Given a token retrieve it from the database, find the user it relates to and
-confirm them. Return the user an the end. If anything goes wrong return undef.
-
-Delete the token afterwards.
-
-See also the ::Result::User method 'create_confirm_token'
-
-=cut
-
-sub confirm_user_from_token {
-    my $self = shift;
-    my $token_string = shift || return;
-
-    # retrieve the token or return
-    my $token_rs = $self->result_source->schema->resultset('Token');
-    my $token_obj =
-      $token_rs->find( { scope => 'user_confirm', token => $token_string, } )
-      || return;
-
-    # find the user related to the token
-    my $user = $self->find( { email => $token_obj->data->{email} } );
-
-    # If we found a user confirm them and delete the token - in transaction
-    $self->result_source->schema->txn_do(
-        sub {
-            $user->update( { is_confirmed => 1 } ) if $user;
-            $token_obj->delete;
-        }
-    );
-
-    # return the user (possibly undef if none found)
-    return $user;
-}
 
 1;
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 0a0280494..43f83db13 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -6,7 +6,7 @@ BEGIN {
     FixMyStreet->test_mode(1);
 }
 
-use Test::More tests => 44;
+use Test::More tests => 90;
 use Email::Send::Test;
 
 use FixMyStreet::App;
@@ -14,14 +14,13 @@ use FixMyStreet::App;
 use Test::WWW::Mechanize::Catalyst 'FixMyStreet::App';
 my $mech = Test::WWW::Mechanize::Catalyst->new;
 
-my $test_email = 'test@example.com';
+my $test_email    = 'test@example.com';
+my $test_password = 'foobar';
 
 END {
-    ok(
-        FixMyStreet::App->model('DB::User')->find( { email => $test_email } )
-          ->delete,
-        "delete test user"
-    );
+    ok( FixMyStreet::App->model('DB::User')->find( { email => $_ } )->delete,
+        "delete test user '$_'" )
+      for ($test_email);
 }
 
 $mech->get_ok('/auth');
@@ -47,7 +46,7 @@ for my $test (
         {
             form_name => 'general_auth',
             fields    => { email => $email, },
-            button    => 'create_account',
+            button    => 'email_login',
         },
         "try to create an account with email '$email'"
     );
@@ -62,14 +61,14 @@ $mech->submit_form_ok(
     {
         form_name => 'general_auth',
         fields    => { email => $test_email, },
-        button    => 'create_account',
+        button    => 'email_login',
     },
     "create an account for '$test_email'"
 );
-is $mech->uri->path, '/auth/welcome', "redirected to welcome page";
+is $mech->uri->path, '/auth/token', "redirected to welcome page";
 
-# check that we are now logged in
-$mech->get_ok("/auth/check_auth");
+# check that we are not logged in yet
+is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
 
 # check that we got one email
 {
@@ -77,7 +76,7 @@ $mech->get_ok("/auth/check_auth");
     Email::Send::Test->clear;
 
     is scalar(@emails), 1, "got one email";
-    is $emails[0]->header('Subject'), "Your new FixMyStreet.com account",
+    is $emails[0]->header('Subject'), "Your FixMyStreet.com account details",
       "subject is correct";
     is $emails[0]->header('To'), $test_email, "to is correct";
 
@@ -85,43 +84,143 @@ $mech->get_ok("/auth/check_auth");
     my ($link) = $emails[0]->body =~ m{(http://\S+)};
     ok $link, "Found a link in email '$link'";
 
-    # check that the user is currently not confirmed
-    my $user =
-      FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
-    ok $user, "got a user";
-    ok !$user->is_confirmed, "user has not been confirmed";
+    # check that the user does not exist
+    sub get_user {
+        FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+    }
+    ok !get_user(), "no user exists";
 
     # visit the confirm link (with bad token) and check user no confirmed
     $mech->get_ok( $link . 'XXX' );
-    $user->discard_changes;
-    ok !$user->is_confirmed, "user has not been confirmed";
+    ok !get_user(), "no user exists";
+    is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
 
     # visit the confirm link and check user is confirmed
     $mech->get_ok($link);
-    $user->discard_changes;
-    ok $user->is_confirmed, "user has been confirmed";
+    ok get_user(), "user created";
+    is $mech->uri->path, '/my', "redirected to the 'my' section of site";
+    $mech->get_ok('/auth/check_auth');
+
+    # logout and try to use the token again
+    $mech->get_ok("/auth/logout");
+    is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
+    $mech->get_ok($link);
+    is $mech->uri, $link, "not logged in";
+    $mech->content_contains( 'Link too old or already used',
+        'token now invalid' );
+    is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
 }
 
-# logout
-$mech->get_ok("/auth/logout");
-is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
+# get a login email and change password
+{
+    Email::Send::Test->clear;
+    $mech->get_ok('/auth');
+    $mech->submit_form_ok(
+        {
+            form_name => 'general_auth',
+            fields    => { email => "$test_email", },
+            button    => 'email_login',
+        },
+        "email_login with '$test_email'"
+    );
+    is $mech->uri->path, '/auth/token', "redirected to token page";
 
-# login using valid details
+    # rest is as before so no need to test
 
-# logout
+    # follow link and change password - check not prompted for old password
+    is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
 
-# try to login with bad details
+    my @emails = Email::Send::Test->emails;
+    my ($link) = $emails[0]->body =~ m{(http://\S+)};
+    $mech->get_ok($link);
+
+    $mech->follow_link_ok( { url => '/auth/change_password' } );
+
+    ok my $form = $mech->form_name('change_password'),
+      "found change password form";
+    is_deeply [ sort grep { $_ } map { $_->name } $form->inputs ],    #
+      [ 'confirm', 'new_password' ],
+      "check we got expected fields (ie not old_password)";
+
+    # check the various ways the form can be wrong
+    for my $test (
+        { new => '',       conf => '',           err => 'enter a password', },
+        { new => 'secret', conf => '',           err => 'do not match', },
+        { new => '',       conf => 'secret',     err => 'do not match', },
+        { new => 'secret', conf => 'not_secret', err => 'do not match', },
+      )
+    {
+        $mech->get_ok('/auth/change_password');
+        $mech->content_lacks( $test->{err}, "did not find expected error" );
+        $mech->submit_form_ok(
+            {
+                form_name => 'change_password',
+                fields =>
+                  { new_password => $test->{new}, confirm => $test->{conf}, },
+            },
+            "change_password with '$test->{new}' and '$test->{conf}'"
+        );
+        $mech->content_contains( $test->{err}, "found expected error" );
+    }
+
+    my $user =
+      FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+    ok $user, "got a user";
+    ok !$user->password, "user has no password";
 
-# try to create an account with bad details
+    $mech->get_ok('/auth/change_password');
+    $mech->submit_form_ok(
+        {
+            form_name => 'change_password',
+            fields =>
+              { new_password => $test_password, confirm => $test_password, },
+        },
+        "change_password with '$test_password' and '$test_password'"
+    );
+    is $mech->uri->path, '/auth/change_password',
+      "still on change password page";
+    $mech->content_contains( 'password has been changed',
+        "found password changed" );
 
-# get a password reset email (for bad email address)
+    $user->discard_changes();
+    ok $user->password, "user now has a password";
+}
 
-# get a password reminder (for good email address)
+# login using valid details
+$mech->get_ok('/auth');
+$mech->submit_form_ok(
+    {
+        form_name => 'general_auth',
+        fields    => {
+            email    => $test_email,
+            password => $test_password,
+        },
+        button => 'login',
+    },
+    "login with '$test_email' & '$test_password"
+);
+is $mech->uri->path, '/my', "redirected to correct page";
 
-# try using  bad reset token
+# logout
+$mech->get_ok("/auth/logout");
+is $mech->get('/auth/check_auth')->code, 401, "got 401 at check_auth";
 
-# use the good reset token and change the password
+# try to login with bad details
+$mech->get_ok('/auth');
+$mech->submit_form_ok(
+    {
+        form_name => 'general_auth',
+        fields    => {
+            email    => $test_email,
+            password => 'not the password',
+        },
+        button => 'login',
+    },
+    "login with '$test_email' & '$test_password"
+);
+is $mech->uri->path, '/auth', "redirected to correct page";
+$mech->content_contains( 'Email or password wrong', 'found error message' );
 
-# try to use the good token again
+# more test:
+# TODO: test that email are always lowercased
 
-# delete the test user
diff --git a/templates/email/default/auth_new_account_welcome b/templates/email/default/auth_new_account_welcome
deleted file mode 100644
index 1270c7d96..000000000
--- a/templates/email/default/auth_new_account_welcome
+++ /dev/null
@@ -1,13 +0,0 @@
-Subject: [% loc('Your new FixMyStreet.com account') %]
-
-Hello,
-
-We've just created an account on FIXME for you.
-
-Please use the following link to confirm that your email works - we need to do
-this before sending reports to the council on your behalf.
-
-  [% c.uri_for( '/auth/confirm', c.user.create_confirm_token ) %]
-
-Yours,
-  FIXME team.
\ No newline at end of file
diff --git a/templates/email/default/login b/templates/email/default/login
new file mode 100644
index 000000000..c873e82af
--- /dev/null
+++ b/templates/email/default/login
@@ -0,0 +1,12 @@
+Subject: [% loc('Your FixMyStreet.com account details') %]
+
+Please click on the link below to confirm your email address. Then you will be able to view your problem reports.
+
+[% c.uri_for( '/auth/token', token ) %]
+
+We will never give away or sell your email address to anyone else without your permission.
+
+Yours,
+  the FixMyStreet.com team
+
+
diff --git a/templates/web/default/auth/change_password.html b/templates/web/default/auth/change_password.html
new file mode 100644
index 000000000..d4a7f107b
--- /dev/null
+++ b/templates/web/default/auth/change_password.html
@@ -0,0 +1,41 @@
+[% INCLUDE 'header.html', title => loc('Change Password') %]
+
+<h1>[% loc('Change Password') %]</h1>
+
+[% IF password_changed %]
+    <p>Your password has been changed!</p>
+[% END %]
+
+
+<form action="[% c.uri_for('change_password') %]" method="post" name="change_password">
+    
+    [% IF password_error;
+
+        errors = {
+            missing  => loc('Please enter a password'),
+            mismatch => loc('The passwords do not match'),
+            other    => loc('Please check the passwords and try again'),
+        };
+        
+        loc_password_error = errors.$password_error || errors.other;
+    END %]
+    
+    
+    <div>
+        <span class="error">[% loc_password_error %]</span><br>
+        <label for="new_password">[% loc('Password:') %]</label>
+        <input type="password" name="new_password" value="[% new_password | html %]">
+        <br>
+
+        <label for="confirm">[% loc('Again:') %]</label>
+        <input type="password" name="confirm" value="[% confirm | html %]">
+        <br>
+
+        <label for="login">&nbsp;</label>
+        <input type="submit" value="[% loc('Change Password') %]">
+    </div>
+    
+</form>
+
+
+[% INCLUDE 'footer.html' %]
diff --git a/templates/web/default/auth/general.html b/templates/web/default/auth/general.html
index f68843f41..44f203193 100644
--- a/templates/web/default/auth/general.html
+++ b/templates/web/default/auth/general.html
@@ -19,27 +19,31 @@
     
     
     <div>
-        <span class="error">[% loc_email_error %]</span><br>
-        <label for="email">[% loc('Your email:') %]</label>
-        <input type="text" name="email" value="[% email || '' | html %]">
-    </div>    
 
+        [% IF loc_email_error  %]
+            <span class="error">[% loc_email_error %]</span><br>
+        [% ELSIF login_error %]
+            <span class="error">Email or password wrong - please try again.</span><br>
+        [% END %]
 
-    <div>
-        <h3>I already have an account</h3>
-        <label for="password">[% loc('Your password:') %]</label>
+        <label for="email">[% loc('Email:') %]</label>
+        <input type="text" name="email" value="[% email || '' | html %]">
+        <br>
+
+        <label for="password">[% loc('Password:') %]</label>
         <input type="password" name="password" value="">
+        <br>
+        <!-- FIXME - implement session length choosing
+        <label for="remember_me">&nbsp;</label>
+        <input type="checkbox" name="remember_me">
+        Remember me - do not use on a public computer
+        <br> -->
+        <label for="login">&nbsp;</label>
         <input type="submit" name="login" value="[% loc('Log me in') %]">
-    </div>
 
-    <div>
-        <h3>I don't have an account...</h3>
-        <input type="submit" name="create_account" value="[% loc('...create a new account now') %]">
-    </div>
-
-    <div>
-        <h3>I've forgotten my password...</h3>
-        <input type="submit" name="email_reset" value="[% loc('...email me a reset link') %]">
+        <h3>I don't have an account, or I've forgotten my password...</h3>
+        <label for="email_login">&nbsp;</label>
+        <input type="submit" name="email_login" value="[% loc('Email the details I need to the address I entered above') %]">
     </div>
     
 </form>
diff --git a/templates/web/default/auth/confirm.html b/templates/web/default/auth/token.html
index fc3abeeba..ecbf94b83 100644
--- a/templates/web/default/auth/confirm.html
+++ b/templates/web/default/auth/token.html
@@ -1,12 +1,6 @@
 [% INCLUDE 'header.html', title => loc('Confirm account') %]
 
-[% IF user_now_confirmed %]
-
-<h1>[% loc('Account now confirmed') %]</h1>
-
-<p>Your account has been confirmed - thank you!</p>
-
-[% ELSE %]
+[% IF token_not_found %]
 
 <h1>[% loc('Error') %]</h1>
 
@@ -18,6 +12,14 @@
     <li>FIXME - reasons here</li>
 </ul>
 
+[% ELSE %]
+
+<h1>[% loc('Please check you email') %]</h1>
+
+<p>We have sent you an email containing a link to confirm your account.</p>
+
+<p>If you do not receive the email in the next few minutes please check your spam folder.</p>
+
 [% END %]
 
 [% INCLUDE 'footer.html' %]
\ No newline at end of file
diff --git a/templates/web/default/auth/welcome.html b/templates/web/default/auth/welcome.html
deleted file mode 100644
index 5727f82ad..000000000
--- a/templates/web/default/auth/welcome.html
+++ /dev/null
@@ -1,11 +0,0 @@
-[% INCLUDE 'header.html', title => loc('Welcome') %]
-
-<h1>[% loc('Welcome') %]</h1>
-
-<p>Your new account on FIXME has been created!</p>
-
-FIXME - add blurb about being sent an email with a confirmation link to confirm
-account.
-
-
-[% INCLUDE 'footer.html' %]
\ No newline at end of file