diff options
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 7 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 62 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/User.pm | 27 | ||||
-rw-r--r-- | t/app/controller/auth.t | 54 | ||||
-rw-r--r-- | templates/email/default/change_email.txt | 11 | ||||
-rw-r--r-- | templates/web/base/auth/change_email.html | 37 | ||||
-rw-r--r-- | templates/web/base/auth/change_password.html | 4 | ||||
-rw-r--r-- | templates/web/base/my/my.html | 1 |
8 files changed, 184 insertions, 19 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index 72c6baad3..e68eb00a5 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -1120,12 +1120,7 @@ sub user_edit : Path('user_edit') : Args(1) { my $existing_user = $c->model('DB::User')->search({ email => $user->email, id => { '!=', $user->id } })->first; if ($existing_user) { - foreach (qw(Problem Comment Alert)) { - $c->model("DB::$_") - ->search({ user_id => $user->id }) - ->update({ user_id => $existing_user->id }); - } - $user->delete; + $existing_user->adopt($user); $c->forward( 'log_edit', [ $id, 'user', 'merge' ] ); $c->res->redirect( $c->uri_for( 'user_edit', $existing_user->id ) ); } else { diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 65533b1d2..b564a988c 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -139,6 +139,10 @@ sub email_sign_in : Private { if $c->get_param('oauth_need_email') && $c->session->{oauth}{facebook_id}; $token_data->{twitter_id} = $c->session->{oauth}{twitter_id} if $c->get_param('oauth_need_email') && $c->session->{oauth}{twitter_id}; + if ($c->stash->{current_user}) { + $token_data->{old_email} = $c->stash->{current_user}->email; + $token_data->{r} = 'auth/change_email/success'; + } my $token_obj = $c->model('DB::Token')->create({ scope => 'email_sign_in', @@ -146,7 +150,8 @@ sub email_sign_in : Private { }); $c->stash->{token} = $token_obj->token; - $c->send_email( 'login.txt', { to => $good_email } ); + my $template = $c->stash->{email_template} || 'login.txt'; + $c->send_email( $template, { to => $good_email } ); $c->stash->{template} = 'auth/token.html'; } @@ -177,17 +182,40 @@ sub token : Path('/M') : Args(1) { return; } - # Sign out in case we are another user - $c->logout(); - # find or create the user related to the token. my $data = $token_obj->data; - my $user = $c->model('DB::User')->find_or_create( { email => $data->{email} } ); + + if ($data->{old_email} && (!$c->user_exists || $c->user->email ne $data->{old_email})) { + $c->stash->{token_not_found} = 1; + return; + } + + # sign out in case we are another user + $c->logout(); + + my $user = $c->model('DB::User')->find_or_new({ email => $data->{email} }); + + if ($data->{old_email}) { + # Were logged in as old_email, want to switch to email ($user) + if ($user->in_storage) { + my $old_user = $c->model('DB::User')->find({ email => $data->{old_email} }); + if ($old_user) { + $old_user->adopt($user); + $user = $old_user; + $user->email($data->{email}); + } + } else { + # Updating to a new (to the db) email address, easier! + $user = $c->model('DB::User')->find({ email => $data->{old_email} }); + $user->email($data->{email}); + } + } + $user->name( $data->{name} ) if $data->{name}; $user->password( $data->{password}, 1 ) if $data->{password}; $user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; $user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; - $user->update; + $user->update_or_insert; $c->authenticate( { email => $user->email }, 'no_password' ); # send the user to their page @@ -446,6 +474,28 @@ sub change_password : Local { } +=head2 change_email + +Let the user change their email. + +=cut + +sub change_email : Local { + my ( $self, $c ) = @_; + + $c->detach( 'redirect' ) unless $c->user; + + $c->forward('get_csrf_token'); + + # If not a post then no submission + return unless $c->req->method eq 'POST'; + + $c->forward('check_csrf_token'); + $c->stash->{current_user} = $c->user; + $c->stash->{email_template} = 'change_email.txt'; + $c->forward('email_sign_in'); +} + sub get_csrf_token : Private { my ( $self, $c ) = @_; diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 054be3644..7356969d1 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -239,9 +239,30 @@ sub has_permission_to { return $permission ? 1 : undef; } -sub print { - my $self = shift; - return '[' . (join '-', @_) . ']'; +sub adopt { + my ($self, $other) = @_; + + return if $self->id == $other->id; + + # Move most things from $other to $self + foreach (qw(Problem Comment Alert AdminLog )) { + $self->result_source->schema->resultset($_) + ->search({ user_id => $other->id }) + ->update({ user_id => $self->id }); + } + + # It's possible the user permissions for the other user exist, so + # try updating, and then delete anyway. + foreach ($self->result_source->schema->resultset("UserBodyPermission") + ->search({ user_id => $other->id })->all) { + eval { + $_->update({ user_id => $self->id }); + }; + $_->delete if $@; + } + + # Delete the now empty user + $other->delete; } 1; diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t index 9b3d9468a..60f22acfb 100644 --- a/t/app/controller/auth.t +++ b/t/app/controller/auth.t @@ -7,11 +7,13 @@ use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; my $test_email = 'test@example.com'; +my $test_email2 = 'test@example.net'; my $test_password = 'foobar'; $mech->delete_user($test_email); END { $mech->delete_user($test_email); + $mech->delete_user($test_email2); done_testing(); } @@ -63,7 +65,6 @@ $mech->not_logged_in_ok; # check that we got one email { - $mech->email_count_is(1); my $email = $mech->get_email; $mech->clear_emails_ok; is $email->header('Subject'), "Your FixMyStreet account details", @@ -116,7 +117,6 @@ $mech->not_logged_in_ok; # follow link and change password - check not prompted for old password $mech->not_logged_in_ok; - $mech->email_count_is(1); my $email = $mech->get_email; $mech->clear_emails_ok; my ($link) = $email->body =~ m{(http://\S+)}; @@ -175,6 +175,54 @@ $mech->not_logged_in_ok; ok $user->password, "user now has a password"; } +subtest "Test change email page" => sub { + # Still signed in from the above test + $mech->get_ok('/my'); + $mech->follow_link_ok({url => '/auth/change_email'}); + $mech->submit_form_ok( + { with_fields => { email => "" } }, + "submit blank change email form" + ); + $mech->content_contains( 'Please enter your email', "found expected error" ); + $mech->submit_form_ok({ with_fields => { email => $test_email2 } }, "change_email to $test_email2"); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + my $email = $mech->get_email; + $mech->clear_emails_ok; + my ($link) = $email->body =~ m{(http://\S+)}; + $mech->get_ok($link); + is $mech->uri->path, '/auth/change_email/success', "redirected to the change_email page"; + $mech->content_contains('successfully confirmed'); + ok(FixMyStreet::App->model('DB::User')->find( { email => $test_email2 } ), "got a user"); + + ok(FixMyStreet::App->model('DB::User')->create( { email => $test_email } ), "created old user"); + $mech->submit_form_ok({ with_fields => { email => $test_email } }, + "change_email back to $test_email" + ); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + $email = $mech->get_email; + $mech->clear_emails_ok; + ($link) = $email->body =~ m{(http://\S+)}; + $mech->get_ok($link); + is $mech->uri->path, '/auth/change_email/success', "redirected to the change_email page"; + $mech->content_contains('successfully confirmed'); + + # Test you can't click the link if logged out + $mech->submit_form_ok({ with_fields => { email => $test_email } }, + "change_email back to $test_email" + ); + is $mech->uri->path, '/auth/change_email', "still on change email page"; + $mech->content_contains( 'Now check your email', "found check your email" ); + $email = $mech->get_email; + $mech->clear_emails_ok; + ($link) = $email->body =~ m{(http://\S+)}; + $mech->log_out_ok; + $mech->get_ok($link); + isnt $mech->uri->path, '/auth/change_email/success', "not redirected to the change_email page"; + $mech->content_contains('Sorry'); +}; + foreach my $remember_me ( '1', '0' ) { subtest "sign in using valid details (remember_me => '$remember_me')" => sub { $mech->get_ok('/auth'); @@ -188,7 +236,7 @@ foreach my $remember_me ( '1', '0' ) { }, button => 'sign_in', }, - "sign in with '$test_email' & '$test_password" + "sign in with '$test_email' & '$test_password'" ); is $mech->uri->path, '/my', "redirected to correct page"; diff --git a/templates/email/default/change_email.txt b/templates/email/default/change_email.txt new file mode 100644 index 000000000..0c5aeac14 --- /dev/null +++ b/templates/email/default/change_email.txt @@ -0,0 +1,11 @@ +Subject: Updating your [% INCLUDE 'site-name.txt' | trim %] email address + +Please click on the link below to confirm you wish to update your +email address on [% INCLUDE 'site-name.txt' | trim %]. + +[% c.uri_for_action( 'auth/token', token ) %] + +[% INCLUDE 'signature.txt' %] + +This email was sent automatically, from an unmonitored email account - so +please do not reply to it. diff --git a/templates/web/base/auth/change_email.html b/templates/web/base/auth/change_email.html new file mode 100644 index 000000000..58c864929 --- /dev/null +++ b/templates/web/base/auth/change_email.html @@ -0,0 +1,37 @@ +[% INCLUDE 'header.html', title = loc('Change email address'), bodyclass = 'authpage' %] + +<h1>[% loc('Change email address') %]</h1> + +[% IF c.req.args.0 == 'success' %] + <p class="form-success">[% loc('You have successfully confirmed your email address.') %]</p> +[% END %] + +[% loc('Your email address') %]: [% c.user.email %] + +<form action="[% c.uri_for('change_email') %]" method="post" name="change_email"> + <input type="hidden" name="token" value="[% csrf_token %]"> + + <fieldset> + [% IF email_error; + errors = { + missing = loc('Please enter your email'), + other = loc('Please check your email address is correct') + }; + loc_email_error = errors.$email_error || errors.other; + %] + <div class="form-error">[% loc_email_error %]</div> + [% END %] + + <div class="form-field"> + <label for="email">[% loc('New email address:') %]</label> + <input type="email" name="email" id="email" value="[% email | html %]"> + </div> + <div class="final-submit"> + <input type="submit" value="[% loc('Change email address') %]"> + </div> + + </fieldset> +</form> + + +[% INCLUDE 'footer.html' %] diff --git a/templates/web/base/auth/change_password.html b/templates/web/base/auth/change_password.html index be0dc69b4..44b695e0d 100644 --- a/templates/web/base/auth/change_password.html +++ b/templates/web/base/auth/change_password.html @@ -10,6 +10,7 @@ <form action="[% c.uri_for('change_password') %]" method="post" name="change_password" class="fieldset"> <input type="hidden" name="token" value="[% csrf_token %]"> + <fieldset> [% IF password_error; errors = { @@ -30,10 +31,11 @@ <label for="confirm">[% loc('Again:') %]</label> <input type="password" name="confirm" value="[% confirm | html %]"> </div> - <div class="checkbox"> + <div class="final-submit"> <input type="submit" value="[% loc('Change password') %]"> </div> + </fieldset> </form> diff --git a/templates/web/base/my/my.html b/templates/web/base/my/my.html index b93a837ad..9ba9533f8 100644 --- a/templates/web/base/my/my.html +++ b/templates/web/base/my/my.html @@ -20,6 +20,7 @@ <p class="my-account-buttons"> <a href="/auth/change_password">[% loc('Change password') %]</a> + <a href="/auth/change_email">[% loc('Change email') %]</a> <a href="/auth/sign_out">[% loc('Sign out') %]</a> </p> |