diff options
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Admin.pm | 63 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth/Phone.pm | 8 | ||||
-rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth/Profile.pm | 4 | ||||
-rw-r--r-- | perllib/FixMyStreet/DB/Result/User.pm | 2 | ||||
-rw-r--r-- | perllib/FixMyStreet/SMS.pm | 4 | ||||
-rw-r--r-- | t/app/controller/auth_phone.t | 8 |
7 files changed, 47 insertions, 44 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Admin.pm b/perllib/FixMyStreet/App/Controller/Admin.pm index b854f16f5..719b04cf6 100644 --- a/perllib/FixMyStreet/App/Controller/Admin.pm +++ b/perllib/FixMyStreet/App/Controller/Admin.pm @@ -797,6 +797,19 @@ sub reports : Path('reports') { } +sub update_user : Private { + my ($self, $c, $object) = @_; + my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username')); + if ($parsed->{email} || ($parsed->{phone} && $parsed->{may_be_mobile})) { + my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} }); + if ($user->id && $user->id != $object->user->id) { + $object->user( $user ); + return 1; + } + } + return 0; +} + sub report_edit : Path('report_edit') : Args(1) { my ( $self, $c, $id ) = @_; @@ -901,14 +914,7 @@ sub report_edit : Path('report_edit') : Args(1) { $problem->set_inflated_columns(\%columns); $c->forward( '/admin/report_edit_category', [ $problem ] ); - - my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username')); - if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) { - my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} }); - if ($user->id && $user->id != $problem->user->id) { - $problem->user( $user ); - } - } + $c->forward('update_user', [ $problem ]); # Deal with photos my $remove_photo_param = $self->_get_remove_photo_param($c); @@ -1256,14 +1262,7 @@ sub update_edit : Path('update_edit') : Args(1) { $update->anonymous( $c->get_param('anonymous') ); $update->state( $new_state ); - my $parsed = FixMyStreet::SMS->parse_username($c->get_param('username')); - if ($parsed->{email} || ($parsed->{phone} && $parsed->{phone}->is_mobile)) { - my $user = $c->model('DB::User')->find_or_create({ $parsed->{type} => $parsed->{username} }); - if ($user->id && $user->id != $update->user->id) { - $edited = 1; - $update->user( $user ); - } - } + $edited = 1 if $c->forward('update_user', [ $update ]); if ( $new_state eq 'confirmed' and $old_state eq 'unconfirmed' ) { $update->confirmed( \'current_timestamp' ); @@ -1303,6 +1302,18 @@ sub update_edit : Path('update_edit') : Args(1) { return 1; } +sub phone_check : Private { + my ($self, $c, $phone) = @_; + my $parsed = FixMyStreet::SMS->parse_username($phone); + if ($parsed->{phone} && $parsed->{may_be_mobile}) { + return $parsed->{username}; + } elsif ($parsed->{phone}) { + $c->stash->{field_errors}->{phone} = _('Please enter a mobile number'); + } else { + $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct'); + } +} + sub user_add : Path('user_edit') : Args(0) { my ( $self, $c ) = @_; @@ -1334,14 +1345,8 @@ sub user_add : Path('user_edit') : Args(0) { } if ($phone_v) { - my $parsed = FixMyStreet::SMS->parse_username($phone); - if ($parsed->{phone} && $parsed->{phone}->is_mobile) { - $phone = $parsed->{username}; - } elsif ($parsed->{phone}) { - $c->stash->{field_errors}->{phone} = _('Please enter a mobile number'); - } else { - $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct'); - } + my $parsed_phone = $c->forward('phone_check', [ $phone ]); + $phone = $parsed_phone if $parsed_phone; } my $existing_email = $email_v && $c->model('DB::User')->find( { email => $email } ); @@ -1422,14 +1427,8 @@ sub user_edit : Path('user_edit') : Args(1) { } if ($phone_v) { - my $parsed = FixMyStreet::SMS->parse_username($phone); - if ($parsed->{phone} && $parsed->{phone}->is_mobile) { - $phone = $parsed->{username}; - } elsif ($parsed->{phone}) { - $c->stash->{field_errors}->{phone} = _('Please enter a mobile number'); - } else { - $c->stash->{field_errors}->{phone} = _('Please check your phone number is correct'); - } + my $parsed_phone = $c->forward('phone_check', [ $phone ]); + $phone = $parsed_phone if $parsed_phone; } unless ($user->name) { diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index b453f593b..80e407147 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -119,7 +119,7 @@ sub code_sign_in : Private { my $parsed = FixMyStreet::SMS->parse_username($username); if ($parsed->{type} eq 'phone' && FixMyStreet->config('SMS_AUTHENTICATION')) { - $c->forward('phone/sign_in', [ $parsed->{phone} ]); + $c->forward('phone/sign_in', [ $parsed ]); } else { $c->forward('email_sign_in', [ $parsed->{username} ]); } diff --git a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm index 4e9f92596..e4ffc2205 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm @@ -45,19 +45,19 @@ and sets up the token/etc to deal with the response. =cut sub sign_in : Private { - my ( $self, $c, $phone ) = @_; + my ( $self, $c, $parsed ) = @_; - unless ($phone) { + unless ($parsed->{phone}) { $c->stash->{username_error} = 'other_phone'; return; } - unless ($phone->is_mobile) { + unless ($parsed->{may_be_mobile}) { $c->stash->{username_error} = 'nonmobile'; return; } - (my $number = $phone->format) =~ s/\s+//g; + (my $number = $parsed->{phone}->format) =~ s/\s+//g; if ( FixMyStreet->config('SIGNUPS_DISABLED') && !$c->model('DB::User')->find({ phone => $number }) diff --git a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm index ecf009150..acffd3019 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Profile.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Profile.pm @@ -118,14 +118,14 @@ sub change_phone : Path('/auth/change_phone') { # If we've not used a mobile and we're not specifically verifying, # and phone isn't our only verified way of logging in, # then allow change of number (for e.g. landline). - if (!FixMyStreet->config('SMS_AUTHENTICATION') || (!$parsed->{phone}->is_mobile && !$c->stash->{verifying} && $c->user->email_verified)) { + if (!FixMyStreet->config('SMS_AUTHENTICATION') || (!$parsed->{may_be_mobile} && !$c->stash->{verifying} && $c->user->email_verified)) { $c->user->update({ phone => $phone, phone_verified => 0 }); $c->flash->{flash_message} = _('You have successfully added your phone number.'); $c->res->redirect('/my'); $c->detach; } - $c->forward('/auth/phone/sign_in', [ $parsed->{phone} ]); + $c->forward('/auth/phone/sign_in', [ $parsed ]); } sub verify_item : Path('/auth/verify') : Args(1) { diff --git a/perllib/FixMyStreet/DB/Result/User.pm b/perllib/FixMyStreet/DB/Result/User.pm index 296cf997d..d02039ac3 100644 --- a/perllib/FixMyStreet/DB/Result/User.pm +++ b/perllib/FixMyStreet/DB/Result/User.pm @@ -196,7 +196,7 @@ sub check_for_errors { my $parsed = FixMyStreet::SMS->parse_username($self->phone); if (!$parsed->{phone}) { $errors{username} = _('Please check your phone number is correct'); - } elsif (!$parsed->{phone}->is_mobile) { + } elsif (!$parsed->{may_be_mobile}) { $errors{username} = _('Please enter a mobile number'); } } diff --git a/perllib/FixMyStreet/SMS.pm b/perllib/FixMyStreet/SMS.pm index ec9251a1a..c71cceadc 100644 --- a/perllib/FixMyStreet/SMS.pm +++ b/perllib/FixMyStreet/SMS.pm @@ -94,15 +94,19 @@ sub parse_username { } }; + my $may_be_mobile = 0; if ($phone) { $type = 'phone'; # Store phone without spaces ($username = $phone->format) =~ s/\s+//g; + # Is this mobile definitely or possibly a mobile? (+1 numbers) + $may_be_mobile = 1 if $phone->is_mobile || (!defined $phone->is_mobile && $phone->is_geographic); } return { type => $type, phone => $phone, + may_be_mobile => $may_be_mobile, username => $username, }; } diff --git a/t/app/controller/auth_phone.t b/t/app/controller/auth_phone.t index 8673f5c62..435ea7552 100644 --- a/t/app/controller/auth_phone.t +++ b/t/app/controller/auth_phone.t @@ -47,7 +47,7 @@ subtest 'Log in using mobile, by text' => sub { }, sub { $mech->submit_form_ok({ form_name => 'general_auth', - fields => { username => '+61491570156', password_register => 'secret' }, + fields => { username => '+18165550100', password_register => 'secret' }, button => 'sign_in_by_code', }, "sign in using mobile"); @@ -61,7 +61,7 @@ subtest 'Log in using mobile, by text' => sub { with_fields => { code => $code } }, 'submit correct code'); - my $user = FixMyStreet::App->model('DB::User')->find( { phone => '+61491570156' } ); + my $user = FixMyStreet::App->model('DB::User')->find( { phone => '+18165550100' } ); ok $user, "user created"; is $mech->uri->path, '/my', "redirected to the 'my' section of site"; $mech->logged_in_ok; @@ -76,13 +76,13 @@ subtest 'Log in using mobile, by password' => sub { $mech->get_ok('/auth'); $mech->submit_form_ok({ form_name => 'general_auth', - fields => { username => '+61491570156', password_sign_in => 'incorrect' }, + fields => { username => '+18165550100', password_sign_in => 'incorrect' }, button => 'sign_in_by_password', }, "sign in using wrong password"); $mech->content_contains('There was a problem'); $mech->submit_form_ok({ form_name => 'general_auth', - fields => { username => '+61491570156', password_sign_in => 'secret' }, + fields => { username => '+18165550100', password_sign_in => 'secret' }, button => 'sign_in_by_password', }, "sign in using password"); |