diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth/Profile.pm | 34 | ||||
| -rw-r--r-- | t/app/controller/auth_profile.t | 63 | ||||
| -rw-r--r-- | templates/web/base/auth/change_password.html | 16 |
4 files changed, 103 insertions, 11 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 680d4b412..e0ae5ceed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - Front end improvements: - Zoom out as much as necessary on body map page, even on mobile. #1958 - Show loading message on initial /around map load #1976 + - Ask for current password/send email on password change. #1974 - Bugfixes: - Fix bug specifying category in URL on /around. #1950 - Fix bug with multiple select-multiples on a page. #1951 diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm index 5e6fe6266..441f222d1 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm @@ -19,7 +19,7 @@ verifying email, phone, password. =cut -sub auto { +sub auto : Private { my ( $self, $c ) = @_; $c->detach( '/auth/redirect' ) unless $c->user; @@ -49,8 +49,17 @@ sub change_password : Path('/auth/change_password') { my $new = $c->get_param('new_password') // ''; my $confirm = $c->get_param('confirm') // ''; + my $password_error; + + # Check existing password, if available + if ($c->user->password) { + my $current = $c->get_param('current_password') // ''; + $c->stash->{current_password} = $current; + $password_error = 'incorrect' unless $c->user->check_password($current); + } + # check for errors - my $password_error = + $password_error ||= !$new && !$confirm ? 'missing' : $new ne $confirm ? 'mismatch' : ''; @@ -62,10 +71,17 @@ sub change_password : Path('/auth/change_password') { return; } - # we should have a usable password - save it to the user - $c->user->obj->update( { password => $new } ); - $c->stash->{password_changed} = 1; - + if ($c->user->password) { + # we should have a usable password - save it to the user + $c->user->obj->update( { password => $new } ); + $c->stash->{password_changed} = 1; + } else { + # Set up arguments for code sign in + $c->set_param('username', $c->user->username); + $c->set_param('password_register', $new); + $c->set_param('r', 'auth/change_password/success'); + $c->detach('/auth/code_sign_in'); + } } =head2 change_email @@ -148,6 +164,12 @@ sub change_phone_success : Path('/auth/change_phone/success') { $c->res->redirect('/my'); } +sub change_password_success : Path('/auth/change_password/success') { + my ( $self, $c ) = @_; + $c->flash->{flash_message} = _('Your password has been changed'); + $c->res->redirect('/my'); +} + sub generate_token : Path('/auth/generate_token') { my ($self, $c) = @_; diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t index 74edccfe6..6040406ef 100644 --- a/t/app/controller/auth_profile.t +++ b/t/app/controller/auth_profile.t @@ -75,9 +75,68 @@ subtest "Test change password page" => sub { { form_name => 'change_password', fields => - { new_password => $test_password, confirm => $test_password, }, + { new_password => 'new_password', confirm => 'new_password', }, }, - "change_password with '$test_password' and '$test_password'" + "change_password with 'new_password' and 'new_password'" + ); + is $mech->uri->path, '/auth/change_password', + "still on change password page"; + $mech->content_contains('check your email'); + + $link = $mech->get_link_from_email; + $mech->get_ok($link); + is $mech->uri->path, '/my', "redirected to /my"; + + $mech->content_contains( 'password has been changed', + "found password changed" ); + + $user->discard_changes(); + ok $user->password, "user now has a password"; +}; + +# Change password, when already got one +subtest "Test change password page with current password" => sub { + $mech->get_ok('/auth/change_password'); + + ok my $form = $mech->form_name('change_password'), + "found change password form"; + is_deeply [ sort grep { $_ } map { $_->name } $form->inputs ], # + [ 'confirm', 'current_password', 'new_password', 'token' ], + "check we got expected fields (ie not old_password)"; + + # check the various ways the form can be wrong + for my $test ( + { current => '', new => '', conf => '', err => 'check the passwords', }, + { current => 'new_password', new => '', conf => '', err => 'enter a password', }, + { current => 'new_password', new => 'secret', conf => '', err => 'do not match', }, + { current => 'new_password', new => '', conf => 'secret', err => 'do not match', }, + { current => 'new_password', 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 => + { current_password => $test->{current}, new_password => $test->{new}, confirm => $test->{conf}, }, + }, + "change_password with '$test->{current}', '$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"; + + $mech->get_ok('/auth/change_password'); + $mech->submit_form_ok( + { + form_name => 'change_password', + fields => + { current_password => 'new_password', new_password => $test_password, confirm => $test_password }, + }, + "change_password with 'new_password' and '$test_password'" ); is $mech->uri->path, '/auth/change_password', "still on change password page"; diff --git a/templates/web/base/auth/change_password.html b/templates/web/base/auth/change_password.html index a32dbaf9c..bf361ff2d 100644 --- a/templates/web/base/auth/change_password.html +++ b/templates/web/base/auth/change_password.html @@ -1,7 +1,9 @@ [% SET bclass = 'authpage'; SET bclass = 'fullwidthpage' IF password_changed; -INCLUDE 'header.html', title = loc('Change password'), bodyclass = bclass +SET title = loc('Set password'); +SET title = loc('Change password') IF c.user.password; +INCLUDE 'header.html', title = title bodyclass = bclass %] [% IF password_changed %] @@ -13,7 +15,7 @@ INCLUDE 'header.html', title = loc('Change password'), bodyclass = bclass [% ELSE %] -<h1>[% loc('Change password') %]</h1> +<h1>[% title %]</h1> <form action="[% c.uri_for_action('/auth/profile/change_password') %]" method="post" name="change_password" class="fieldset"> <input type="hidden" name="token" value="[% csrf_token %]"> @@ -31,6 +33,14 @@ INCLUDE 'header.html', title = loc('Change password'), bodyclass = bclass <div class="form-error">[% loc_password_error %]</div> [% END %] +[% IF c.user.password %] + <div class="form-field"> + <label for="current_password">[% loc('Current password:') %]</label> + <input class="form-control" type="password" name="current_password" value="[% current_password | html %]"> + </div> + <hr> +[% END %] + <div class="form-field"> <label for="new_password">[% loc('New password:') %]</label> <input class="form-control" type="password" name="new_password" value="[% new_password | html %]"> @@ -40,7 +50,7 @@ INCLUDE 'header.html', title = loc('Change password'), bodyclass = bclass <input class="form-control" type="password" name="confirm" value="[% confirm | html %]"> </div> <div class="final-submit"> - <input type="submit" class="btn" value="[% loc('Change password') %]"> + <input type="submit" class="btn" value="[% title %]"> </div> </fieldset> |