Skip to content

Commit

Permalink
Allow two-factor to work during creation flow.
Browse files Browse the repository at this point in the history
  • Loading branch information
@dracos
dracos committed Feb 7, 2018
1 parent b4b6679 commit 3e721dd
Show file tree
Hide file tree
Showing 6 changed files with 84 additions and 18 deletions.
20 changes: 19 additions & 1 deletion perllib/Catalyst/Authentication/Credential/2FA.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions perllib/FixMyStreet/App/Controller/Auth.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
29 changes: 20 additions & 9 deletions perllib/FixMyStreet/App/Controller/Report/New.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down Expand Up @@ -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')) {
Expand Down Expand Up @@ -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');
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
13 changes: 9 additions & 4 deletions perllib/FixMyStreet/App/Controller/Report/Update.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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');
}

Expand All @@ -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) {
Expand Down
34 changes: 30 additions & 4 deletions t/app/controller/report_new.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -742,14 +755,23 @@ 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',
}
},
"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:' );

Expand All @@ -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};
Expand All @@ -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);
Expand Down
1 change: 1 addition & 0 deletions templates/web/base/auth/2faform.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ <h1>[% loc("Nearly done! Now check your phone&hellip;") %]</h1>
<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">
Expand Down

0 comments on commit 3e721dd

Please sign in to comment.