Merge branch '2fa-improvements'

7 files changed, 203 insertions, 63 deletions

diff --git a/t/app/controller/admin/reportextrafields.t b/t/app/controller/admin/reportextrafields.t
index 1714e8521..1aa4c6a42 100644
--- a/t/app/controller/admin/reportextrafields.t
+++ b/t/app/controller/admin/reportextrafields.t
@@ -9,6 +9,8 @@ sub allow_report_extra_fields { 1 }
 
 sub area_types { [ 'UTA' ] }
 
+sub must_have_2fa { 0 }
+
 
 package FixMyStreet::Cobrand::SecondTester;
 
diff --git a/t/app/controller/admin/users.t b/t/app/controller/admin/users.t
index 7361ab619..17256a214 100644
--- a/t/app/controller/admin/users.t
+++ b/t/app/controller/admin/users.t
@@ -249,7 +249,6 @@ my %default_perms = (
     "permissions[template_edit]" => undef,
     "permissions[responsepriority_edit]" => undef,
     "permissions[category_edit]" => undef,
-    trusted_bodies => undef,
 );
 
 # Start this section with user having no name
diff --git a/t/app/controller/auth.t b/t/app/controller/auth.t
index ffabc75f3..cd72ab550 100644
--- a/t/app/controller/auth.t
+++ b/t/app/controller/auth.t
@@ -1,3 +1,10 @@
+package FixMyStreet::Cobrand::Dummy;
+use parent 'FixMyStreet::Cobrand::Default';
+
+sub must_have_2fa { 1 }
+
+package main;
+
 use Test::MockModule;
 
 use FixMyStreet::TestMech;
@@ -7,10 +14,6 @@ my $test_email    = 'test@example.com';
 my $test_email3   = 'newuser@example.org';
 my $test_password = 'foobar123';
 
-END {
-    done_testing();
-}
-
 $mech->get_ok('/auth');
 
 # check that we can't reach a page that is only available to authenticated users
@@ -290,7 +293,6 @@ subtest "Test two-factor authentication login" => sub {
     my $wrong_code = $auth->code(undef, time() - 120);
 
     my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
-    $user->is_superuser(1);
     $user->password('password');
     $user->set_extra_metadata('2fa_secret', $auth->secret32);
     $user->update;
@@ -305,3 +307,97 @@ subtest "Test two-factor authentication login" => sub {
     $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
     $mech->logged_in_ok;
 };
+
+subtest "Test enforced two-factor authentication" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => 'dummy',
+    }, sub {
+        my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+        $user->unset_extra_metadata('2fa_secret');
+        $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('requires two-factor');
+        $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2FA activation");
+        my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/;
+
+        use Auth::GoogleAuth;
+        my $auth = Auth::GoogleAuth->new({ secret32 => $token });
+        my $code = $auth->code;
+        my $wrong_code = $auth->code(undef, time() - 120);
+
+        $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" );
+        $mech->content_contains('Try again');
+        $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
+        $mech->content_contains('successfully enabled two-factor authentication', "2FA activated");
+
+        $user->discard_changes();
+        my $user_token = $user->get_extra_metadata('2fa_secret');
+        is $token, $user_token, '2FA secret set';
+
+        $mech->logged_in_ok;
+    };
+};
+
+subtest "Test enforced two-factor authentication, no password yet set" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => 'dummy',
+    }, sub {
+        my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+        $user->unset_extra_metadata('2fa_secret');
+        $user->update;
+
+        $mech->clear_emails_ok;
+        $mech->get_ok('/auth');
+        $mech->submit_form_ok({
+            fields => { username => $test_email, password_register => $test_password },
+            button => 'sign_in_by_code',
+        }, "log in by email");
+
+        my $link = $mech->get_link_from_email;
+        $mech->get_ok($link);
+
+        $mech->content_contains('requires two-factor');
+        $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2fa activation");
+        my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/;
+
+        my $auth = Auth::GoogleAuth->new({ secret32 => $token });
+        my $code = $auth->code;
+        $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2fa code" );
+
+        $user->discard_changes();
+        my $user_token = $user->get_extra_metadata('2fa_secret');
+        is $token, $user_token, '2FA secret set';
+
+        $mech->logged_in_ok;
+    };
+};
+
+subtest "Check two-factor log in by email works" => sub {
+    use Auth::GoogleAuth;
+    my $auth = Auth::GoogleAuth->new;
+    my $code = $auth->code;
+
+    my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+    $user->set_extra_metadata('2fa_secret', $auth->secret32);
+    $user->update;
+
+    $mech->clear_emails_ok;
+    $mech->get_ok('/auth');
+    $mech->submit_form_ok({
+        fields => { username => $test_email, password_register => $test_password },
+        button => 'sign_in_by_code',
+    }, "log in by email");
+
+    my $link = $mech->get_link_from_email;
+    $mech->get_ok($link);
+    $mech->content_contains('Please generate a two-factor code');
+    $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
+    $mech->logged_in_ok;
+};
+
+done_testing();
diff --git a/t/app/controller/auth_profile.t b/t/app/controller/auth_profile.t
index 2f0ef6a11..4b7db19b5 100644
--- a/t/app/controller/auth_profile.t
+++ b/t/app/controller/auth_profile.t
@@ -1,3 +1,10 @@
+package FixMyStreet::Cobrand::Dummy;
+use parent 'FixMyStreet::Cobrand::Default';
+
+sub must_have_2fa { 1 }
+
+package main;
+
 use Test::MockModule;
 use FixMyStreet::TestMech;
 my $mech = FixMyStreet::TestMech->new;
@@ -359,6 +366,8 @@ subtest "Test superuser can access generate token page" => sub {
     $mech->get_ok('/auth/generate_token');
 };
 
+my $body = $mech->create_body_ok(2237, 'Oxfordshire');
+
 subtest "Test staff user can access generate token page" => sub {
     my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
     ok $user->update({ is_superuser => 0 }), 'user not superuser';
@@ -374,8 +383,6 @@ subtest "Test staff user can access generate token page" => sub {
 
     $mech->content_lacks('Security');
 
-    my $body = $mech->create_body_ok(2237, 'Oxfordshire');
-
     $mech->get('/auth/generate_token');
     is $mech->res->code, 403, "access denied";
 
@@ -425,29 +432,80 @@ subtest "Test generate token page" => sub {
     $mech->log_out_ok;
     $mech->add_header('Authorization', "Bearer $token");
     $mech->logged_in_ok;
+    $mech->delete_header('Authorization');
 };
 
 subtest "Test two-factor authentication admin" => sub {
+  for (0, 1) {
     my $user = $mech->log_in_ok($test_email);
-    ok $user->update({ is_superuser => 1 }), 'user set to superuser';
+    if ($_) {
+        ok $user->update({ is_superuser => 1, from_body => undef }), 'user set to superuser';
+    } else {
+        ok $user->update({ is_superuser => 0, from_body => $body }), 'user set to staff user';
+    }
 
     $mech->get_ok('/auth/generate_token');
     ok !$user->get_extra_metadata('2fa_secret');
 
-    $mech->submit_form_ok({ button => 'toggle_2fa' }, "submit 2FA activation");
+    $mech->submit_form_ok({ button => '2fa_activate' }, "submit 2FA activation");
+    my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/;
+
+    use Auth::GoogleAuth;
+    my $auth = Auth::GoogleAuth->new({ secret32 => $token });
+    my $code = $auth->code;
+    my $wrong_code = $auth->code(undef, time() - 120);
+
+    $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" );
+    $mech->content_contains('Try again');
+    $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
+
     $mech->content_contains('has been activated', "2FA activated");
 
     $user->discard_changes();
-    my $token = $user->get_extra_metadata('2fa_secret');
-    ok $token, '2FA secret set';
-
-    $mech->content_contains($token, 'secret displayed');
+    my $user_token = $user->get_extra_metadata('2fa_secret');
+    is $token, $user_token, '2FA secret set';
 
     $mech->get_ok('/auth/generate_token');
     $mech->content_lacks($token, 'secret no longer displayed');
 
-    $mech->submit_form_ok({ button => 'toggle_2fa' }, "submit 2FA deactivation");
+    $mech->submit_form_ok({ button => '2fa_deactivate' }, "submit 2FA deactivation");
     $mech->content_contains('has been deactivated', "2FA deactivated");
+  }
+};
+
+subtest "Test enforced two-factor authentication" => sub {
+    FixMyStreet::override_config {
+        ALLOWED_COBRANDS => 'dummy',
+    }, sub {
+        use Auth::GoogleAuth;
+        my $auth = Auth::GoogleAuth->new;
+        my $code = $auth->code;
+
+        # Sign in with 2FA
+        my $user = FixMyStreet::App->model('DB::User')->find( { email => $test_email } );
+        $user->password('password');
+        $user->set_extra_metadata('2fa_secret', $auth->secret32);
+        $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('Please generate a two-factor code');
+        $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
+
+        $mech->get_ok('/auth/generate_token');
+        $mech->content_contains('Change two-factor');
+        $mech->content_lacks('Deactivate two-factor');
+
+        my ($csrf) = $mech->content =~ /meta content="([^"]*)" name="csrf-token"/;
+        $mech->post_ok('/auth/generate_token', {
+            '2fa_deactivate' => 1,
+            'token' => $csrf,
+        });
+        $mech->content_lacks('has been deactivated', "2FA not deactivated");
+        $mech->content_contains('Please scan this image', 'Change 2FA page shown instead');
+    };
 };
 
 done_testing();
diff --git a/t/app/controller/dashboard.t b/t/app/controller/dashboard.t
index 061809aaf..15c718c74 100644
--- a/t/app/controller/dashboard.t
+++ b/t/app/controller/dashboard.t
@@ -1,5 +1,9 @@
 use Test::MockTime ':all';
 
+package FixMyStreet::Cobrand::No2FA;
+use parent 'FixMyStreet::Cobrand::FixMyStreet';
+sub must_have_2fa { 0 }
+
 package FixMyStreet::Cobrand::Tester;
 use parent 'FixMyStreet::Cobrand::Default';
 # Allow access if CSV export for a body, otherwise deny
@@ -38,13 +42,13 @@ my $area_id = '60705';
 my $alt_area_id = '62883';
 
 my $last_month = DateTime->now->subtract(months => 2);
-$mech->create_problems_for_body(2, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'fixmystreet' });
-$mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet', dt => $last_month });
-$mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$alt_area_id,2651,", category => 'Litter', cobrand => 'fixmystreet' });
+$mech->create_problems_for_body(2, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fat' });
+$mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa', dt => $last_month });
+$mech->create_problems_for_body(1, $body->id, 'Title', { areas => ",$alt_area_id,2651,", category => 'Litter', cobrand => 'no2fa' });
 
-my @scheduled_problems = $mech->create_problems_for_body(7, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet' });
-my @fixed_problems = $mech->create_problems_for_body(4, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'fixmystreet' });
-my @closed_problems = $mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'fixmystreet' });
+my @scheduled_problems = $mech->create_problems_for_body(7, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa' });
+my @fixed_problems = $mech->create_problems_for_body(4, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Potholes', cobrand => 'no2fa' });
+my @closed_problems = $mech->create_problems_for_body(3, $body->id, 'Title', { areas => ",$area_id,2651,", category => 'Traffic lights', cobrand => 'no2fa' });
 
 my $first_problem_id;
 my $first_update_id;
@@ -73,7 +77,7 @@ my $categories = scraper {
 };
 
 FixMyStreet::override_config {
-    ALLOWED_COBRANDS => [ { fixmystreet => '.' } ],
+    ALLOWED_COBRANDS => 'no2fa',
     MAPIT_URL => 'http://mapit.uk/',
 }, sub {
 
diff --git a/t/app/controller/report_inspect.t b/t/app/controller/report_inspect.t
index b6498e840..61fa821b2 100644
--- a/t/app/controller/report_inspect.t
+++ b/t/app/controller/report_inspect.t
@@ -132,7 +132,6 @@ FixMyStreet::override_config {
         $user->user_body_permissions->create({ body => $oxon, permission_type => 'planned_reports' });
         $report->state('confirmed');
         $report->update;
-        my $reputation = $report->user->get_extra_metadata("reputation");
         $mech->get_ok("/report/$report_id");
         $mech->submit_form_ok({ button => 'save', with_fields => {
             public_update => "This is a public update.", include_update => "1",
@@ -145,7 +144,6 @@ FixMyStreet::override_config {
         $report->discard_changes;
         my $comment = ($report->comments( undef, { order_by => { -desc => 'id' } } )->all)[1]->text;
         is $comment, "This is a public update.", 'Update was created';
-        is $report->user->get_extra_metadata('reputation'), $reputation, "User reputation wasn't changed";
         $mech->get_ok("/report/$report_id");
         my $meta = $mech->extract_update_metas;
         like $meta->[0], qr/State changed to: Action scheduled/, 'First update mentions action scheduled';
@@ -583,38 +581,6 @@ FixMyStreet::override_config {
 
         return $perms;
     });
-    subtest "test negative reputation" => sub {
-        my $reputation = $report->user->get_extra_metadata("reputation") || 0;
-
-        $mech->get_ok("/report/$report_id");
-        $mech->submit_form( button => 'remove_from_site' );
-
-        $report->discard_changes;
-        is $report->user->get_extra_metadata('reputation'), $reputation-1, "User reputation was decreased";
-        $report->update({ state => 'confirmed' });
-    };
-
-    subtest "test positive reputation" => sub {
-        $user->user_body_permissions->create({ body => $oxon, permission_type => 'report_instruct' });
-        $report->update;
-        $report->inspection_log_entry->delete if $report->inspection_log_entry;
-        my $reputation = $report->user->get_extra_metadata("reputation") || 0;
-        $mech->get_ok("/report/$report_id");
-        $mech->submit_form_ok({ button => 'save', with_fields => {
-            state => 'in progress', include_update => undef,
-        } });
-        $report->discard_changes;
-
-        $mech->submit_form_ok({ button => 'save', with_fields => {
-            state => 'action scheduled', include_update => undef,
-        } });
-        $report->discard_changes;
-        is $report->user->get_extra_metadata('reputation'), $reputation+1, "User reputation was increased";
-
-        $mech->submit_form_ok({ button => 'save', with_fields => {
-            state => 'action scheduled', include_update => undef,
-        } });
-    };
 
     subtest "Oxfordshire-specific traffic management options are shown" => sub {
         $report->update({ state => 'confirmed' });
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index 20eecb50e..48169c148 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -914,8 +914,9 @@ subtest "test password errors for a user who is signing in as they report" => su
 };
 
 foreach my $test (
-  { two_factor => 0, desc => '', },
-  { two_factor => 1, desc => ' with two-factor', },
+  { two_factor => '', desc => '', },
+  { two_factor => 'yes', desc => ' with two-factor', },
+  { two_factor => 'new', desc => ' with mandated two-factor, not yet set up', },
 ) {
   subtest "test report creation for a user who is signing in as they report$test->{desc}" => sub {
     $mech->log_out_ok;
@@ -932,21 +933,25 @@ foreach my $test (
         name     => 'Joe Bloggs',
         phone    => '01234 567 890',
         password => 'secret2',
+        $test->{two_factor} ? (is_superuser => 1) : (),
     } ), "set user details";
 
     my $auth;
-    if ($test->{two_factor}) {
+    my $mock;
+    if ($test->{two_factor} eq 'yes') {
         use Auth::GoogleAuth;
         $auth = Auth::GoogleAuth->new;
-        $user->is_superuser(1);
         $user->set_extra_metadata('2fa_secret', $auth->generate_secret32);
         $user->update;
+    } elsif ($test->{two_factor} eq 'new') {
+        $mock = Test::MockModule->new('FixMyStreet::Cobrand::FixMyStreet');
+        $mock->mock(must_have_2fa => sub { 1 });
     }
 
     # submit initial pc form
     $mech->get_ok('/around');
     FixMyStreet::override_config {
-        ALLOWED_COBRANDS => [ { fixmystreet => '.' } ],
+        ALLOWED_COBRANDS => 'fixmystreet',
         MAPIT_URL => 'http://mapit.uk/',
     }, sub {
         $mech->submit_form_ok( { with_fields => { pc => 'EH1 1BB', } },
@@ -971,13 +976,23 @@ foreach my $test (
             "submit good details"
         );
 
-        if ($test->{two_factor}) {
+        if ($test->{two_factor} eq 'yes') {
             my $code = $auth->code;
             my $wrong_code = $auth->code(undef, time() - 120);
             $mech->content_contains('Please generate a two-factor code');
             $mech->submit_form_ok({ with_fields => { '2fa_code' => $wrong_code } }, "provide wrong 2FA code" );
             $mech->content_contains('Try again');
             $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
+        } elsif ($test->{two_factor} eq 'new') {
+            $mech->content_contains('requires two-factor');
+            $mech->submit_form_ok({ with_fields => { '2fa_action' => 'activate' } }, "submit 2FA activation");
+            my ($token) = $mech->content =~ /name="secret32" value="([^"]*)">/;
+
+            use Auth::GoogleAuth;
+            my $auth = Auth::GoogleAuth->new({ secret32 => $token });
+            my $code = $auth->code;
+            print $mech->encoded_content;
+            $mech->submit_form_ok({ with_fields => { '2fa_code' => $code } }, "provide correct 2FA code" );
         }
 
         # check that we got the message expected
@@ -998,7 +1013,7 @@ foreach my $test (
     my $report = $user->problems->first;
     ok $report, "Found the report";
 
-    if (!$test->{two_factor}) {
+    if ($test->{two_factor} eq '') {
         # The superuser account will be immediately redirected
         $mech->content_contains('Thank you for reporting this issue');
     }