Skip to content

Commit

Permalink
[TfL] Remove FMS log in options for TfL
Browse files Browse the repository at this point in the history
TfL has requested staff can only log in via single sign on.

Create a call hook and add the call to TfL to
disallow logging in on an email address not supplied
for single sign on.
  • Loading branch information
MorayMySoc committed Mar 6, 2024
1 parent 847aceb commit 395ea66
Show file tree
Hide file tree
Showing 5 changed files with 73 additions and 18 deletions.
4 changes: 3 additions & 1 deletion perllib/FixMyStreet/App/Controller/Auth.pm
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ sub sign_in : Private {
$c->logout();

my $parsed = FixMyStreet::SMS->parse_username($username);

$c->cobrand->call_hook('disable_login_for_email', $parsed->{username}) unless $c->stash->{oauth_need_email};
$c->forward('throttle_username', [$parsed]);

if ($parsed->{username} && $password && $c->forward('authenticate', [ $parsed->{type}, $parsed->{username}, $password ])) {
Expand Down Expand Up @@ -193,6 +193,8 @@ sub email_sign_in : Private {
return;
}

$c->cobrand->call_hook('disable_login_for_email', $good_email) unless $c->get_param('oauth_need_email');

my $password = $c->get_param('password_register');
if ($password) {
return unless $c->forward('/auth/test_password', [ $password ]);
Expand Down
1 change: 1 addition & 0 deletions perllib/FixMyStreet/App/Controller/Report/New.pm
Original file line number Diff line number Diff line change
Expand Up @@ -969,6 +969,7 @@ sub process_user : Private {
}
$params{username} ||= '';

$c->cobrand->call_hook('disable_login_for_email', $params{username}) unless $c->get_param('oauth_need_email');
my $anon_button = $c->cobrand->allow_anonymous_reports eq 'button' && $c->get_param('report_anonymously');
my $anon_fallback = $c->cobrand->allow_anonymous_reports eq '1' && !$c->user_exists && !$params{username};
if ($anon_button || $anon_fallback) {
Expand Down
1 change: 1 addition & 0 deletions perllib/FixMyStreet/App/Controller/Report/Update.pm
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ sub process_user : Private {
$params{username} = $c->get_param('username_register');
}
$params{username} ||= '';
$c->cobrand->call_hook('disable_login_for_email', $params{username}) unless $c->get_param('oauth_need_email');

my $anon_button = $c->cobrand->allow_anonymous_updates eq 'button' && $c->get_param('report_anonymously');
if ($anon_button) {
Expand Down
10 changes: 10 additions & 0 deletions perllib/FixMyStreet/Cobrand/TfL.pm
Original file line number Diff line number Diff line change
Expand Up @@ -479,6 +479,16 @@ sub munge_reports_area_list {

sub munge_report_new_contacts { }

sub disable_login_for_email {
my ($self, $email) = @_;

my $staff_email = $self->admin_user_domain;

if ($email =~ /$staff_email$/) {
$self->{c}->detach('/page_error_403_access_denied', ['Please use the staff login option']);
}
}

sub munge_report_new_bodies {
my ($self, $bodies) = @_;

Expand Down
75 changes: 58 additions & 17 deletions t/cobrand/tfl.t
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,8 @@ FixMyStreet::override_config {
$mech->host("tfl.fixmystreet.com");

subtest "creating a user on TfL creates tfl_password extra_metadata" => sub {
my $tfl_mock = Test::MockModule->new('FixMyStreet::Cobrand::TfL');
$tfl_mock->mock('disable_login_for_email', sub {});
for my $email ('[email protected]', '[email protected]') {
$mech->get_ok('/auth/create');
$mech->submit_form_ok({ with_fields => { username => $email, password_register => 'xpasswordx'} });
Expand Down Expand Up @@ -316,23 +318,6 @@ subtest "test report creation anonymously by button" => sub {
$mech->not_logged_in_ok;
};

subtest "test users with tfl email not asked to update their FMS password" => sub {
my $tfl_staff = FixMyStreet::DB->resultset("User")->find({ email => '[email protected]'});
$tfl_staff->set_extra_metadata('last_password_change', DateTime->now->subtract(years => 2)->epoch);
$tfl_staff->update;
$mech->get_ok('/auth');
FixMyStreet->test_mode(0);
$mech->submit_form_ok(
{ with_fields => { username => $tfl_staff->email, password_sign_in => 'xpasswordx' } },
"sign in using form"
);
FixMyStreet->test_mode(1);
$tfl_staff->set_extra_metadata('last_password_change', time());
$tfl_staff->update;
is $mech->uri->path, '/my', "logged in to My Account page";
$mech->content_lacks('Your password has expired, please create a new one below');
};

subtest "test users without tfl email asked to update their FMS password" => sub {
my $tfl_non_staff = FixMyStreet::DB->resultset("User")->find({ email => '[email protected]'});
$tfl_non_staff->set_extra_metadata('last_password_change', DateTime->now->subtract(years => 2)->epoch);
Expand Down Expand Up @@ -433,6 +418,46 @@ subtest "test report creation anonymously by staff user" => sub {

FixMyStreet::DB->resultset("Problem")->delete_all;

my $tfl_staff = FixMyStreet::DB->resultset("User")->find({ email => '[email protected]'});

subtest "test user with tfl email address can't login through standard login" => sub {
$mech->get_ok('/auth');
$mech->submit_form(
form_name => 'general_auth',
fields => { username => $tfl_staff->email, },
button => 'sign_in_by_code',
);
is $mech->status, '403', "status forbidden";
$mech->content_contains('Please use the staff login option');
$mech->get_ok('/auth');
$mech->submit_form(
form_name => 'general_auth',
fields => {
username => $tfl_staff->email,
password_sign_in => 'password'
},
);
is $mech->status, '403', "status forbidden";
$mech->content_contains('Please use the staff login option');
};

subtest "test user with tfl email address can't login by creating a report" => sub {
$mech->get_ok('/around');
$mech->submit_form_ok( { with_fields => { pc => 'BR1 3UH', } }, "submit location" );
$mech->follow_link_ok( { text_regex => qr/skip this step/i, }, "follow 'skip this step' link" );
$mech->submit_form(
fields => {
title => 'Test Report 1',
detail => 'Test report details.',
name => 'TfL Staff',
username_register => $tfl_staff->email,
category => 'Bus stops',
},
);
is $mech->status, '403', "status forbidden";
$mech->content_contains('Please use the staff login option');
};

subtest "test report creation and reference number" => sub {
my $tfl_mock = Test::MockModule->new('FixMyStreet::Cobrand::TfL');
$tfl_mock->mock('password_expiry', sub {});
Expand Down Expand Up @@ -467,7 +492,23 @@ subtest "test report creation and reference number" => sub {
is $report->name, 'Joe Bloggs';
};

subtest "test user with tfl email address can't login by updating a report" => sub {
$mech->log_out_ok;
my $report = FixMyStreet::DB->resultset("Problem")->find({ title => 'Test Report 1'});
$mech->get_ok('/report/' . $report->id);
$mech->submit_form(
with_fields => {
update => 'This is an update',
username => $tfl_staff->email,
password_sign_in => 'xpasswordx',
}
);
is $mech->status, '403', "status forbidden";
$mech->content_contains('Please use the staff login option');
};

subtest "test bus report creation outside London, .com" => sub {
$mech->log_in_ok( $user->email );
$mech->host('www.fixmystreet.com');
$mech->get_ok('/report/new?latitude=51.345714&longitude=-0.227959');
$mech->content_lacks('Bus things');
Expand Down

0 comments on commit 395ea66

Please sign in to comment.