Allow two-factor to work during creation flow.

6 files changed, 84 insertions, 18 deletions

diff --git a/perllib/Catalyst/Authentication/Credential/2FA.pm b/perllib/Catalyst/Authentication/Credential/2FA.pm
index 2c2054c66..6cb1dd297 100644
--- a/perllib/Catalyst/Authentication/Credential/2FA.pm
+++ b/perllib/Catalyst/Authentication/Credential/2FA.pm
@@ -24,7 +24,25 @@ sub authenticate {
         # We don't care unless user is a superuser and has a 2FA secret
         return $user_obj unless $user_obj->is_superuser;
         return $user_obj unless $user_obj->get_extra_metadata('2fa_secret');
-        return $user_obj if $self->check_2fa($c, $user_obj);
+
+        $c->stash->{token} = $c->get_param('token');
+
+        if ($self->check_2fa($c, $user_obj)) {
+            if ($c->stash->{token}) {
+                my $token = $c->forward('/tokens/load_auth_token', [ $c->stash->{token}, '2fa' ]);
+                # Will contain a detach_to and report/update data
+                $c->stash($token->data);
+            }
+            return $user_obj;
+        }
+
+        if ($c->stash->{tfa_data}) {
+            my $token = $c->model("DB::Token")->create( {
+                scope => '2fa',
+                data => $c->stash->{tfa_data},
+            });
+            $c->stash->{token} = $token->token;
+        }
 
         $c->stash->{template} = 'auth/2faform.html';
         $c->detach;
diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 06448afde..b2d909f9c 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -273,6 +273,11 @@ Used after signing in to take the person back to where they were.
 
 sub redirect_on_signin : Private {
     my ( $self, $c, $redirect, $params ) = @_;
+
+    if ($c->stash->{detach_to}) {
+        $c->detach($c->stash->{detach_to}, $c->stash->{detach_args});
+    }
+
     unless ( $redirect ) {
         $c->detach('redirect_to_categories') if $c->user->from_body && scalar @{ $c->user->categories };
         $redirect = 'my';
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index 09430110d..1083f843b 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -109,8 +109,8 @@ sub report_new : Path : Args(0) {
     return unless $c->forward('check_form_submitted');
 
     $c->forward('/auth/check_csrf_token');
-    $c->forward('process_user');
     $c->forward('process_report');
+    $c->forward('process_user');
     $c->forward('/photo/process_photo');
     return unless $c->forward('check_for_errors');
     $c->forward('save_user_and_report');
@@ -147,8 +147,8 @@ sub report_new_ajax : Path('mobile') : Args(0) {
 
     $c->forward('setup_categories_and_bodies');
     $c->forward('setup_report_extra_fields');
-    $c->forward('process_user');
     $c->forward('process_report');
+    $c->forward('process_user');
     $c->forward('/photo/process_photo');
 
     unless ($c->forward('check_for_errors')) {
@@ -418,7 +418,9 @@ sub report_import : Path('/import') {
 
 sub oauth_callback : Private {
     my ( $self, $c, $token_code ) = @_;
-    $c->stash->{oauth_report} = $token_code;
+    my $auth_token = $c->forward(
+        '/tokens/load_auth_token', [ $token_code, 'problem/social' ]);
+    $c->stash->{oauth_report} = $auth_token->data;
     $c->detach('report_new');
 }
 
@@ -475,9 +477,7 @@ sub initialize_report : Private {
     }
 
     if (!$report && $c->stash->{oauth_report}) {
-        my $auth_token = $c->forward( '/tokens/load_auth_token',
-            [ $c->stash->{oauth_report}, 'problem/social' ] );
-        $report = $c->model("DB::Problem")->new($auth_token->data);
+        $report = $c->model("DB::Problem")->new($c->stash->{oauth_report});
     }
 
     if ($report) {
@@ -772,7 +772,7 @@ sub process_user : Private {
     if ( $c->user_exists ) { {
         my $user = $c->user->obj;
 
-        if ($c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $c->stash->{bodies})) {
+        if ($c->stash->{contributing_as_another_user}) {
             # Act as if not logged in (and it will be auto-confirmed later on)
             $report->user(undef);
             last;
@@ -781,8 +781,7 @@ sub process_user : Private {
         $report->user( $user );
         $c->forward('update_user', [ \%params ]);
 
-        if ($c->stash->{contributing_as_body} = $user->contributing_as('body', $c, $c->stash->{bodies}) or
-            $c->stash->{contributing_as_anonymous_user} = $user->contributing_as('anonymous_user', $c, $c->stash->{bodies})) {
+        if ($c->stash->{contributing_as_body} or $c->stash->{contributing_as_anonymous_user}) {
             $report->name($user->from_body->name);
             $user->name($user->from_body->name) unless $user->name;
             $c->stash->{no_reporter_alert} = 1;
@@ -799,6 +798,11 @@ sub process_user : Private {
 
     # The user is trying to sign in. We only care about username from the params.
     if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) {
+        $c->stash->{tfa_data} = {
+            detach_to => '/report/new/report_new',
+            login_success => 1,
+            oauth_report => { $report->get_inflated_columns }
+        };
         unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) {
             $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.');
             return 1;
@@ -867,6 +871,13 @@ sub process_report : Private {
     $report->longitude( $c->stash->{longitude} );
     $report->send_questionnaire( $c->cobrand->send_questionnaires() );
 
+    if ( $c->user_exists ) {
+        my $user = $c->user->obj;
+        $c->stash->{contributing_as_another_user} = $user->contributing_as('another_user', $c, $c->stash->{bodies});
+        $c->stash->{contributing_as_body} = $user->contributing_as('body', $c, $c->stash->{bodies});
+        $c->stash->{contributing_as_anonymous_user} = $user->contributing_as('anonymous_user', $c, $c->stash->{bodies});
+    }
+
     # set some simple bool values (note they get inverted)
     if ($c->stash->{contributing_as_body}) {
         $report->anonymous(0);
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 2f0ef8c0f..8b96b02d1 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -131,6 +131,11 @@ sub process_user : Private {
 
     # The user is trying to sign in. We only care about username from the params.
     if ( $c->get_param('submit_sign_in') || $c->get_param('password_sign_in') ) {
+        $c->stash->{tfa_data} = {
+            detach_to => '/report/update/report_update',
+            login_success => 1,
+            oauth_update => { $update->get_inflated_columns }
+        };
         unless ( $c->forward( '/auth/sign_in', [ $params{username} ] ) ) {
             $c->stash->{field_errors}->{password} = _('There was a problem with your login information. If you cannot remember your password, or do not have one, please fill in the ‘No’ section of the form.');
             return 1;
@@ -161,7 +166,9 @@ what we have so far.
 
 sub oauth_callback : Private {
     my ( $self, $c, $token_code ) = @_;
-    $c->stash->{oauth_update} = $token_code;
+    my $auth_token = $c->forward('/tokens/load_auth_token',
+        [ $token_code, 'update/social' ]);
+    $c->stash->{oauth_update} = $auth_token->data;
     $c->detach('report_update');
 }
 
@@ -176,9 +183,7 @@ sub initialize_update : Private {
 
     my $update;
     if ($c->stash->{oauth_update}) {
-        my $auth_token = $c->forward( '/tokens/load_auth_token',
-            [ $c->stash->{oauth_update}, 'update/social' ] );
-        $update = $c->model("DB::Comment")->new($auth_token->data);
+        $update = $c->model("DB::Comment")->new($c->stash->{oauth_update});
     }
 
     if ($update) {
diff --git a/t/app/controller/report_new.t b/t/app/controller/report_new.t
index e0fe205bd..f5af6f082 100644
--- a/t/app/controller/report_new.t
+++ b/t/app/controller/report_new.t
@@ -705,7 +705,11 @@ subtest "test password errors for a user who is signing in as they report" => su
     ], "check there were errors";
 };
 
-subtest "test report creation for a user who is signing in as they report" => sub {
+foreach my $test (
+  { two_factor => 0, desc => '', },
+  { two_factor => 1, desc => ' with two-factor', },
+) {
+  subtest "test report creation for a user who is signing in as they report$test->{desc}" => sub {
     $mech->log_out_ok;
     $mech->cookie_jar({});
     $mech->clear_emails_ok;
@@ -722,6 +726,15 @@ subtest "test report creation for a user who is signing in as they report" => su
         password => 'secret2',
     } ), "set user details";
 
+    my $auth;
+    if ($test->{two_factor}) {
+        use Auth::GoogleAuth;
+        $auth = Auth::GoogleAuth->new;
+        $user->is_superuser(1);
+        $user->set_extra_metadata('2fa_secret', $auth->generate_secret32);
+        $user->update;
+    }
+
     # submit initial pc form
     $mech->get_ok('/around');
     FixMyStreet::override_config {
@@ -742,7 +755,7 @@ subtest "test report creation for a user who is signing in as they report" => su
                     title         => 'Test Report',
                     detail        => 'Test report details.',
                     photo1        => '',
-                    username      => 'test-2@example.com',
+                    username      => $test_email,
                     password_sign_in => 'secret2',
                     category      => 'Street lighting',
                 }
@@ -750,6 +763,15 @@ subtest "test report creation for a user who is signing in as they report" => su
             "submit good details"
         );
 
+        if ($test->{two_factor}) {
+            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" );
+        }
+
         # check that we got the message expected
         $mech->content_contains( 'You have successfully signed in; please check and confirm your details are accurate:' );
 
@@ -768,7 +790,10 @@ subtest "test report creation for a user who is signing in as they report" => su
     my $report = $user->problems->first;
     ok $report, "Found the report";
 
-    $mech->content_contains('Thank you for reporting this issue');
+    if (!$test->{two_factor}) {
+        # The superuser account will be immediately redirected
+        $mech->content_contains('Thank you for reporting this issue');
+    }
 
     # Check the report has been assigned appropriately
     is $report->bodies_str, $body_ids{2651};
@@ -793,7 +818,8 @@ subtest "test report creation for a user who is signing in as they report" => su
 
     # cleanup
     $mech->delete_user($user)
-};
+  };
+}
 
 #### test report creation for user with account and logged in
 my ($saved_lat, $saved_lon);
diff --git a/templates/web/base/auth/2faform.html b/templates/web/base/auth/2faform.html
index f9257bf61..bd8d60cdb 100644
--- a/templates/web/base/auth/2faform.html
+++ b/templates/web/base/auth/2faform.html
@@ -13,6 +13,7 @@
         <input type="hidden" name="password_sign_in" value="[% c.get_param('password_sign_in') | html %]">
         <input type="hidden" name="r" value="[% c.get_param('r') | html %]">
         <input type="hidden" name="remember_me" value="[% c.get_param('remember_me') | html %]">
+        <input type="hidden" name="token" value="[% token | html %]">
 
         <label for="2fa_code">[% loc('Code') %]</label>
         <div class="form-txt-submit-box">