aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
Diffstat
-rw-r--r--perllib/FixMyStreet/App/Controller/Auth/Phone.pm6
-rw-r--r--perllib/FixMyStreet/SMS.pm16
-rw-r--r--t/Mock/Twilio.pm4
-rw-r--r--t/app/controller/auth_phone.t46
-rw-r--r--templates/web/base/auth/_username_error.html13
-rw-r--r--templates/web/base/auth/change_phone.html10
-rw-r--r--templates/web/base/auth/general.html15
7 files changed, 54 insertions, 56 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm
index e4ffc2205..6c6d7006e 100644
--- a/perllib/FixMyStreet/App/Controller/Auth/Phone.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth/Phone.pm
@@ -90,6 +90,12 @@ sub send_token : Private {
my ( $self, $c, $token_data, $token_scope, $to ) = @_;
my $result = FixMyStreet::SMS->send_token($token_data, $token_scope, $to);
+ if ($result->{error}) {
+ $c->log->debug("Failure sending text containing code *$result->{random}*");
+ $c->stash->{sms_error} = $result->{error};
+ $c->stash->{username_error} = 'sms_failed';
+ return;
+ }
$c->stash->{token} = $result->{token};
$c->log->debug("Sending text containing code *$result->{random}*");
$c->stash->{template} = 'auth/smsform.html';
diff --git a/perllib/FixMyStreet/SMS.pm b/perllib/FixMyStreet/SMS.pm
index c71cceadc..dff27e018 100644
--- a/perllib/FixMyStreet/SMS.pm
+++ b/perllib/FixMyStreet/SMS.pm
@@ -3,7 +3,7 @@ package FixMyStreet::SMS;
use strict;
use warnings;
-# use JSON::MaybeXS;
+use JSON::MaybeXS;
use Moo;
use Number::Phone::Lib;
use WWW::Twilio::API;
@@ -44,7 +44,7 @@ sub send_token {
return {
random => $random,
token => $token_obj->token,
- result => $result,
+ %$result,
};
}
@@ -55,13 +55,11 @@ sub send {
To => $params{to},
Body => $params{body},
);
- # At present, we do nothing and assume sent okay.
- # TODO add error checking
- # my $data = decode_json($output->{content});
- # if ($output->{code} != 200) {
- # return { error => "$data->{message} ($data->{code})" };
- # }
- # return { success => $data->{sid} };
+ my $data = decode_json($output->{content});
+ if ($output->{code} >= 400) {
+ return { error => "$data->{message} ($data->{code})" };
+ }
+ return { success => $data->{sid} };
}
=head2 parse_username
diff --git a/t/Mock/Twilio.pm b/t/Mock/Twilio.pm
index eaad30b76..125daa55f 100644
--- a/t/Mock/Twilio.pm
+++ b/t/Mock/Twilio.pm
@@ -20,6 +20,10 @@ sub dispatch_request {
sub (POST + /2010-04-01/Accounts/*/Messages.json + %*) {
my ($self, $sid, $data) = @_;
+ if ($data->{To} eq '+18165550101') {
+ return [ 400, [ 'Content-Type' => 'application/json' ],
+ [ '{"code":"21408", "message": "Unable to send"}' ] ];
+ }
push @{$self->texts}, $data;
return [ 200, [ 'Content-Type' => 'application/json' ], [ '{}' ] ];
},
diff --git a/t/app/controller/auth_phone.t b/t/app/controller/auth_phone.t
index 435ea7552..dea1c3493 100644
--- a/t/app/controller/auth_phone.t
+++ b/t/app/controller/auth_phone.t
@@ -7,12 +7,13 @@ LWP::Protocol::PSGI->register($twilio->to_psgi_app, host => 'api.twilio.com');
my $mech = FixMyStreet::TestMech->new;
-subtest 'Log in with invalid number, fail' => sub {
- FixMyStreet::override_config {
- SMS_AUTHENTICATION => 1,
- PHONE_COUNTRY => 'GB',
- TWILIO_ACCOUNT_SID => 'AC123',
- }, sub {
+FixMyStreet::override_config {
+ SMS_AUTHENTICATION => 1,
+ PHONE_COUNTRY => 'GB',
+ TWILIO_ACCOUNT_SID => 'AC123',
+}, sub {
+
+ subtest 'Log in with invalid number, fail' => sub {
$mech->get_ok('/auth');
$mech->submit_form_ok({
form_name => 'general_auth',
@@ -21,14 +22,8 @@ subtest 'Log in with invalid number, fail' => sub {
}, "sign in using bad number");
$mech->content_contains('Please check your phone number is correct');
};
-};
-subtest 'Log in using landline, fail' => sub {
- FixMyStreet::override_config {
- SMS_AUTHENTICATION => 1,
- PHONE_COUNTRY => 'GB',
- TWILIO_ACCOUNT_SID => 'AC123',
- }, sub {
+ subtest 'Log in using landline, fail' => sub {
$mech->get_ok('/auth');
$mech->submit_form_ok({
form_name => 'general_auth',
@@ -37,14 +32,18 @@ subtest 'Log in using landline, fail' => sub {
}, "sign in using landline");
$mech->content_contains('Please enter a mobile number');
};
-};
-subtest 'Log in using mobile, by text' => sub {
- FixMyStreet::override_config {
- SMS_AUTHENTICATION => 1,
- PHONE_COUNTRY => 'GB',
- TWILIO_ACCOUNT_SID => 'AC123',
- }, sub {
+ subtest 'Log in using number that fails at Twilio' => sub {
+ $mech->get_ok('/auth');
+ $mech->submit_form_ok({
+ form_name => 'general_auth',
+ fields => { username => '+18165550101' },
+ button => 'sign_in_by_code',
+ }, "sign in using failing number");
+ $mech->content_contains('Sending a confirmation text failed');
+ };
+
+ subtest 'Log in using mobile, by text' => sub {
$mech->submit_form_ok({
form_name => 'general_auth',
fields => { username => '+18165550100', password_register => 'secret' },
@@ -67,12 +66,8 @@ subtest 'Log in using mobile, by text' => sub {
$mech->logged_in_ok;
$mech->log_out_ok;
};
-};
-subtest 'Log in using mobile, by password' => sub {
- FixMyStreet::override_config {
- SMS_AUTHENTICATION => 1,
- }, sub {
+ subtest 'Log in using mobile, by password' => sub {
$mech->get_ok('/auth');
$mech->submit_form_ok({
form_name => 'general_auth',
@@ -89,6 +84,7 @@ subtest 'Log in using mobile, by password' => sub {
is $mech->uri->path, '/my', "redirected to the 'my' section of site";
$mech->logged_in_ok;
};
+
};
done_testing();
diff --git a/templates/web/base/auth/_username_error.html b/templates/web/base/auth/_username_error.html
new file mode 100644
index 000000000..c0ddc135a
--- /dev/null
+++ b/templates/web/base/auth/_username_error.html
@@ -0,0 +1,13 @@
+[% IF username_error;
+ # other keys include fqdn, mxcheck if you'd like to write a custom error message
+ errors = {
+ nonmobile = loc('Please enter a mobile number'),
+ sms_failed = tprintf(loc('Sending a confirmation text failed: "%s"'), sms_error),
+ missing_phone = loc('Please enter your phone number'),
+ other_phone = loc('Please check your phone number is correct'),
+ missing_email = loc('Please enter your email'),
+ other_email = loc('Please check your email address is correct')
+ };
+ default = "other_$default";
+ errors.$username_error || errors.$default;
+END ~%]
diff --git a/templates/web/base/auth/change_phone.html b/templates/web/base/auth/change_phone.html
index 4b181f6c5..27a2f63dd 100644
--- a/templates/web/base/auth/change_phone.html
+++ b/templates/web/base/auth/change_phone.html
@@ -25,14 +25,8 @@ END
<input type="hidden" name="token" value="[% csrf_token %]">
<fieldset>
- [% IF username_error;
- errors = {
- nonmobile = loc('Please enter a mobile number'),
- missing_phone = loc('Please enter your phone number'),
- other_phone = loc('Please check your phone number is correct')
- };
- loc_username_error = errors.$username_error || errors.other_phone;
- %]
+ [% loc_username_error = INCLUDE 'auth/_username_error.html' default='phone' %]
+ [% IF loc_username_error %]
<div class="form-error">[% loc_username_error %]</div>
[% END %]
diff --git a/templates/web/base/auth/general.html b/templates/web/base/auth/general.html
index e64230b41..d630dd415 100644
--- a/templates/web/base/auth/general.html
+++ b/templates/web/base/auth/general.html
@@ -36,20 +36,7 @@
<div id="js-social-email-hide">
[% END %]
- [% IF username_error;
-
- # other keys include fqdn, mxcheck if you'd like to write a custom error message
-
- errors = {
- nonmobile = loc('Please enter a mobile number'),
- missing_email = loc('Please enter your email'),
- other_email = loc('Please check your email address is correct')
- missing_phone = loc('Please enter your phone number'),
- other_phone = loc('Please check your phone number is correct')
- };
-
- loc_username_error = errors.$username_error || errors.other_email;
- END %]
+ [% loc_username_error = INCLUDE 'auth/_username_error.html' default='email' %]
[% IF c.config.SMS_AUTHENTICATION %]
[% SET username_label = loc('Your email or mobile') %]
generated by cgit v1.2.3 (git 2.25.1) at 2025-07-18 04:02:24 +0000