Skip to content

Commit

Permalink
[NH] Look up department in Entra.
Browse files Browse the repository at this point in the history
  • Loading branch information
dracos committed Sep 13, 2024
1 parent aaea3a8 commit 9e00579
Show file tree
Hide file tree
Showing 8 changed files with 146 additions and 44 deletions.
21 changes: 13 additions & 8 deletions perllib/FixMyStreet/App/Controller/Auth/Social.pm
Original file line number Diff line number Diff line change
Expand Up @@ -292,9 +292,8 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
# The only other valid state param is 'login' at this point.
$c->detach('/page_error_400_bad_request', []) unless $c->get_param('state') eq 'login';

my $id_token;
eval {
$id_token = $oidc->get_access_token(
my $token = eval {
$oidc->get_access_token(
code => $c->get_param('code'),
redirect_uri => $c->uri_for('/auth/OIDC')
);
Expand All @@ -303,16 +302,22 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
(my $message = $@) =~ s/at [^ ]*Auth.pm.*//;
$c->detach('/page_error_500_internal_error', [ $message ]);
}
my $id_token = $token->{id_token};
my $access_token = $token->{access_token};

if (!$id_token) {
$c->log->info("Social::oidc_callback no id_token: " . $oidc->{last_response}->{_content});
$c->detach('oauth_failure');
}
my $message = '';
for my $key (sort keys %{$id_token->payload}) {
$message .= $key . " : " . $id_token->payload->{$key} . "\n" if $id_token->payload->{$key};

if (FixMyStreet->config('STAGING_SITE')) {
my $message = '';
for my $key (sort keys %{$id_token->payload}) {
$message .= $key . " : " . $id_token->payload->{$key} . "\n" if $id_token->payload->{$key};
}
$c->log->info($message) if $message;
}
$c->log->info($message) if $message;

# sanity check the token audience is us...
unless ($id_token->payload->{aud} eq $c->forward('oidc_config')->{client_id}) {
$c->log->info("Social::oidc_callback invalid id_token: expected aud to be " . $c->forward('oidc_config')->{client_id} . " but it was " . $id_token->payload->{aud});
Expand All @@ -337,7 +342,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
$c->session->{oauth}{id_token} = $id_token->token_string;

# Cobrands can use different fields for name and email
my ($name, $email) = $c->cobrand->call_hook(user_from_oidc => $id_token->payload);
my ($name, $email) = $c->cobrand->call_hook(user_from_oidc => $id_token->payload, $access_token);
$name = '' if $name && $name !~ /\w/;

# There's a chance that a user may have multiple OIDC logins, so build a namespaced uid to prevent collisions
Expand Down
14 changes: 13 additions & 1 deletion perllib/FixMyStreet/Cobrand/HighwaysEngland.pm
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ use strict;
use warnings;
use utf8;
use DateTime;
use JSON::MaybeXS;
use LWP::UserAgent;

sub council_name { 'National Highways' }

Expand Down Expand Up @@ -215,11 +217,21 @@ sub social_auth_enabled {
=cut

sub user_from_oidc {
my ($self, $payload) = @_;
my ($self, $payload, $access_token) = @_;

Check warning on line 220 in perllib/FixMyStreet/Cobrand/HighwaysEngland.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/HighwaysEngland.pm#L220

Added line #L220 was not covered by tests

my $name = $payload->{name} ? $payload->{name} : '';
my $email = $payload->{email} ? lc($payload->{email}) : '';

if ($payload->{oid} && $access_token) {
my $ua = LWP::UserAgent->new;

Check warning on line 226 in perllib/FixMyStreet/Cobrand/HighwaysEngland.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/HighwaysEngland.pm#L226

Added line #L226 was not covered by tests
my $response = $ua->get(
'https://graph.microsoft.com/v1.0/users/' . $payload->{oid} . '?$select=displayName,department',

Check warning on line 228 in perllib/FixMyStreet/Cobrand/HighwaysEngland.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/HighwaysEngland.pm#L228

Added line #L228 was not covered by tests
Authorization => 'Bearer ' . $access_token,
);
my $user = decode_json($response->decoded_content);

Check warning on line 231 in perllib/FixMyStreet/Cobrand/HighwaysEngland.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/HighwaysEngland.pm#L231

Added line #L231 was not covered by tests
$payload->{roles} = [ $user->{department} ] if $user->{department};
}

return ($name, $email);

Check warning on line 235 in perllib/FixMyStreet/Cobrand/HighwaysEngland.pm

View check run for this annotation

Codecov / codecov/patch

perllib/FixMyStreet/Cobrand/HighwaysEngland.pm#L235

Added line #L235 was not covered by tests
}

Expand Down
34 changes: 0 additions & 34 deletions perllib/FixMyStreet/Cobrand/TfL.pm
Original file line number Diff line number Diff line change
Expand Up @@ -366,40 +366,6 @@ sub user_from_oidc {
return ($name, $email);
}

=item * TfL sends the user role in the single sign-on payload, which we use to set the FMS role
=cut

sub roles_from_oidc {
my ($self, $user, $roles) = @_;

return unless $roles && @$roles;

$user->user_roles->delete;
$user->from_body($self->body->id);

my $cfg = $self->feature('oidc_login') || {};
my $role_map = $cfg->{role_map} || {};

my @body_roles;
for ($user->from_body->roles->order_by('name')->all) {
push @body_roles, {
id => $_->id,
name => $_->name,
}
}

for my $assign_role (@$roles) {
my ($body_role) = grep { $role_map->{$assign_role} && $_->{name} eq $role_map->{$assign_role} } @body_roles;

if ($body_role) {
$user->user_roles->find_or_create({
role_id => $body_role->{id},
});
}
}
}

sub state_groups_inspect {
my $rs = FixMyStreet::DB->resultset("State");
my @open = grep { $_ !~ /^(planned|investigating|for triage)$/ } FixMyStreet::DB::Result::Problem->open_states;
Expand Down
34 changes: 34 additions & 0 deletions perllib/FixMyStreet/Cobrand/UK.pm
Original file line number Diff line number Diff line change
Expand Up @@ -647,4 +647,38 @@ sub _email_to_body {
return $body;
}

=item * Some OIDC users send the user role in the single sign-on payload, which we use to set the FMS role
=cut

sub roles_from_oidc {
my ($self, $user, $roles) = @_;

return unless $roles && @$roles;

$user->user_roles->delete;
$user->from_body($self->body->id);

my $cfg = $self->feature('oidc_login') || {};
my $role_map = $cfg->{role_map} || {};

my @body_roles;
for ($user->from_body->roles->order_by('name')->all) {
push @body_roles, {
id => $_->id,
name => $_->name,
}
}

for my $assign_role (@$roles) {
my ($body_role) = grep { $role_map->{$assign_role} && $_->{name} eq $role_map->{$assign_role} } @body_roles;

if ($body_role) {
$user->user_roles->find_or_create({
role_id => $body_role->{id},
});
}
}
}

1;
5 changes: 4 additions & 1 deletion perllib/OIDC/Lite/Client/IDTokenResponseParser.pm
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@ sub parse {
message => sprintf("Response doesn't include 'id_token'")
) unless exists $result->{id_token};

$token = OIDC::Lite::Model::IDToken->load($result->{id_token});
$token = {
id_token => OIDC::Lite::Model::IDToken->load($result->{id_token}),
access_token => $result->{access_token},
};

} else {

Expand Down
18 changes: 18 additions & 0 deletions t/Mock/MicrosoftGraph.pm
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package t::Mock::MicrosoftGraph;

use Web::Simple;

sub dispatch_request {
my $self = shift;

sub (GET + /v1.0/users/*) {
my ($self, $oid) = @_;
my $json = '{"department":"Department","displayName":"Name"}';
return [ 200, [ 'Content-Type' => 'application/json' ], [ $json ] ];
},

}

LWP::Protocol::PSGI->register(t::Mock::MicrosoftGraph->to_psgi_app, host => 'graph.microsoft.com');

__PACKAGE__->run_if_script;
6 changes: 6 additions & 0 deletions t/Mock/OpenIDConnect.pm
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,11 @@ sub dispatch_request {
$payload->{family_name} = "Dwyer";
$payload->{email} = '[email protected]' if $self->returns_email;
$payload->{roles} = $self->roles;
} elsif ($self->cobrand eq 'highwaysengland') {
$payload->{given_name} = "Andy";
$payload->{family_name} = "Dwyer";
$payload->{email} = '[email protected]' if $self->returns_email;
$payload->{oid} = 'OID-OID-OID';
}
my $signature = "dummy";
my $id_token = join(".", (
Expand All @@ -97,6 +102,7 @@ sub dispatch_request {
encode_base64($signature, '')
));
my $data = {
access_token => 'AccessToken',
id_token => $id_token,
token_type => "Bearer",
not_before => $now,
Expand Down
58 changes: 58 additions & 0 deletions t/cobrand/highwaysengland.t
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use HighwaysEngland;
use DateTime;
use File::Temp 'tempdir';
use Test::MockModule;
use t::Mock::OpenIDConnect;
use t::Mock::MicrosoftGraph;

my $he_mock = Test::MockModule->new('HighwaysEngland');
$he_mock->mock('database_file', sub { FixMyStreet->path_to('t/geocode/roads.sqlite'); });
Expand Down Expand Up @@ -47,6 +49,12 @@ my $highways = $mech->create_body_ok(164186, 'National Highways', { send_method

$mech->create_contact_ok(email => '[email protected]', body_id => $highways->id, category => 'Pothole (NH)');

FixMyStreet::DB->resultset("Role")->create({
body => $highways,
name => 'Inspector',
permissions => ['moderate', 'user_edit'],
});

FixMyStreet::override_config {
ALLOWED_COBRANDS => [ 'highwaysengland', 'fixmystreet' ],
MAPIT_URL => 'http://mapit.uk/',
Expand Down Expand Up @@ -302,4 +310,54 @@ FixMyStreet::override_config {
};
};

FixMyStreet::override_config {
ALLOWED_COBRANDS => 'highwaysengland',
MAPIT_URL => 'http://mapit.uk/',
COBRAND_FEATURES => {
oidc_login => {
highwaysengland => {
client_id => 'example_client_id',
secret => 'example_secret_key',
auth_uri => 'http://oidc.example.org/oauth2/v2.0/authorize',
token_uri => 'http://oidc.example.org/oauth2/v2.0/token',
logout_uri => 'http://oidc.example.org/oauth2/v2.0/logout',
display_name => 'MyAccount',
role_map => {
'Department' => 'Inspector',
},
}
}
}
}, sub {
subtest 'National Highways department lookup' => sub {
my $email = $mech->uniquify_email('[email protected]');
my $redirect_pattern = qr{oidc\.example\.org/oauth2/v2\.0/authorize};

$mech->log_out_ok;
$mech->cookie_jar({});
$mech->host('oidc.example.org');

my $resolver = Test::MockModule->new('Email::Valid');
$resolver->mock('address', sub { $email });
my $social = Test::MockModule->new('FixMyStreet::App::Controller::Auth::Social');
$social->mock('generate_nonce', sub { 'MyAwesomeRandomValue' });

my $mock_api = t::Mock::OpenIDConnect->new( host => 'oidc.example.org' );
LWP::Protocol::PSGI->register($mock_api->to_psgi_app, host => 'oidc.example.org');
$mock_api->cobrand('highwaysengland');

$mech->get_ok('/my');
$mech->submit_form(button => 'social_sign_in');
is $mech->res->previous->code, 302, "button redirected";
like $mech->res->previous->header('Location'), $redirect_pattern, "redirect to oauth URL";

$mech->get_ok('/auth/OIDC?code=response-code&state=login');
is $mech->uri->path, '/my', 'Successfully on /my page';

my $user = FixMyStreet::DB->resultset( 'User' )->find( { email => $email } );
my @roles = sort map { $_->name } $user->roles->all;
is_deeply ['Inspector'], \@roles, 'Correct roles assigned to user';
};
};

done_testing();

0 comments on commit 9e00579

Please sign in to comment.