Allow cobrands to skip 2FA requirement.

3 files changed, 29 insertions, 6 deletions

diff --git a/perllib/Catalyst/Authentication/Credential/2FA.pm b/perllib/Catalyst/Authentication/Credential/2FA.pm
index 3f59ada06..f77f56bea 100644
--- a/perllib/Catalyst/Authentication/Credential/2FA.pm
+++ b/perllib/Catalyst/Authentication/Credential/2FA.pm
@@ -23,7 +23,9 @@ sub authenticate {
     if (ref($user_obj)) {
 
         # We don't care unless user has a 2FA secret, or the cobrand mandates it
-        return $user_obj unless $user_obj->has_2fa || $c->cobrand->call_hook('must_have_2fa', $user_obj);
+        # We also don't care if the cobrand says we don't
+        my $must_have_2fa = $c->cobrand->call_hook('must_have_2fa', $user_obj) || '';
+        return $user_obj if $must_have_2fa eq 'skip' || !($user_obj->has_2fa || $must_have_2fa);
 
         $c->stash->{token} = $c->get_param('token');
 
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 6badbf518..cecfa318c 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -277,10 +277,13 @@ sub process_login : Private {
         if FixMyStreet->config('SIGNUPS_DISABLED') && !$user->in_storage && !$data->{old_user_id};
 
     # People using 2FA need to supply a code
-    if ($user->has_2fa) {
-        $c->forward( 'token_2fa', [ $user, $url_token ] );
-    } elsif ($c->cobrand->call_hook('must_have_2fa', $user)) {
-        $c->forward( 'signup_2fa', [ $user ] );
+    my $must_have_2fa = $c->cobrand->call_hook('must_have_2fa', $user) || '';
+    if ($must_have_2fa ne 'skip') {
+        if ($user->has_2fa) {
+            $c->forward( 'token_2fa', [ $user, $url_token ] );
+        } elsif ($c->cobrand->call_hook('must_have_2fa', $user)) {
+            $c->forward( 'signup_2fa', [ $user ] );
+        }
     }
 
     if ($data->{old_user_id}) {
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index 20bc5595c..24deb8cab 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -1,7 +1,11 @@
 package FixMyStreet::Cobrand::Dummy;
 use parent 'FixMyStreet::Cobrand::Default';
 
-sub must_have_2fa { 1 }
+sub must_have_2fa {
+    my ($self, $user) = @_;
+    return 'skip' if $user->name eq 'skip';
+    return 1;
+}
 
 package FixMyStreet::Cobrand::Expiring;
 use parent 'FixMyStreet::Cobrand::Default';
@@ -348,6 +352,14 @@ subtest "Test enforced two-factor authentication" => sub {
         is $token, $user_token, '2FA secret set';
 
         $mech->logged_in_ok;
+
+        $user->name('skip');
+        $user->update;
+        $mech->get_ok('/auth');
+        $mech->submit_form_ok(
+            { with_fields => { username => $test_email, password_sign_in => 'password' } },
+            "sign in using form" );
+        $mech->content_contains('<h1>Your account');
     };
 };
 
@@ -357,6 +369,7 @@ subtest "Test enforced two-factor authentication, no password yet set" => sub {
     }, sub {
         my $user = FixMyStreet::DB->resultset('User')->find( { email => $test_email } );
         $user->unset_extra_metadata('2fa_secret');
+        $user->name('Test User');
         $user->update;
 
         $mech->clear_emails_ok;
@@ -387,6 +400,11 @@ subtest "Test enforced two-factor authentication, no password yet set" => sub {
         $mech->content_contains('Please generate a two-factor code');
         $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
         $mech->content_lacks('requires two-factor');
+
+        $user->name('skip');
+        $user->update;
+        $mech->get_ok($link);
+        $mech->content_contains('Your account');
     };
 };