aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--perllib/FixMyStreet/App/Controller/Admin.pm63
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth.pm2
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth/Phone.pm8
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth/Profile.pm4
-rw-r--r--perllib/FixMyStreet/DB/Result/User.pm2
-rw-r--r--perllib/FixMyStreet/SMS.pm4
-rw-r--r--t/app/controller/auth_phone.t8
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");