Skip to content

Commit

Permalink
[Brent] Allow different OIDC config for WasteWorks host
Browse files Browse the repository at this point in the history 
For FD-3395
  • Loading branch information
@davea
davea committed Nov 17, 2023
1 parent ca0d5ff commit dfd28a0
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 12 deletions.
20 changes: 13 additions & 7 deletions perllib/FixMyStreet/App/Controller/Auth/Social.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -162,10 +162,16 @@ sub twitter_callback: Path('/auth/Twitter') : Args(0) {
$c->forward('oauth_success', [ 'twitter', $info->{id}, $info->{name} ]);
}

sub oidc_config : Private {
my ($self, $c) = @_;

return $c->cobrand->call_hook('oidc_config') || $c->cobrand->feature('oidc_login');
}

sub oidc : Private {
my ($self, $c) = @_;

my $config = $c->cobrand->feature('oidc_login');
my $config = $c->forward('oidc_config');

OIDC::Lite::Client::WebServer::AuthCodeFlow->new(
id => $config->{client_id},
Expand All @@ -180,7 +186,7 @@ sub oidc_sign_in : Private {

$c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED');

my $cfg = $c->cobrand->feature('oidc_login');
my $cfg = $c->forward('oidc_config');
$c->detach( '/page_error_400_bad_request', [] ) unless $cfg;

my $oidc = $c->forward('oidc');
Expand Down Expand Up @@ -242,7 +248,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {

if ($c->get_param('error')) {
my $error_desc = $c->get_param('error_description');
my $password_reset_uri = $c->cobrand->feature('oidc_login')->{password_reset_uri};
my $password_reset_uri = $c->forward('oidc_config')->{password_reset_uri};
if ($password_reset_uri && $error_desc =~ /^AADB2C90118:/) {
my $url = $oidc->uri_to_redirect(
uri => $password_reset_uri,
Expand Down Expand Up @@ -304,8 +310,8 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
}

# sanity check the token audience is us...
unless ($id_token->payload->{aud} eq $c->cobrand->feature('oidc_login')->{client_id}) {
$c->log->info("Social::oidc_callback invalid id_token: expected aud to be " . $c->cobrand->feature('oidc_login')->{client_id} . " but it was " . $id_token->payload->{aud});
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});
$c->detach('/page_error_500_internal_error', ['invalid id_token']);
}

Expand All @@ -315,7 +321,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
$c->detach('/page_error_500_internal_error', ['invalid id_token']);
}

if (my $domains = $c->cobrand->feature('oidc_login')->{allowed_domains}) {
if (my $domains = $c->forward('oidc_config')->{allowed_domains}) {
# Check that the hd payload is present in the token and matches the
# list of allowed domains from the config
my $hd = $id_token->payload->{hd};
Expand All @@ -331,7 +337,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
$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
my $uid = join(":", $c->cobrand->moniker, $c->cobrand->feature('oidc_login')->{client_id}, $id_token->payload->{sub});
my $uid = join(":", $c->cobrand->moniker, $c->forward('oidc_config')->{client_id}, $id_token->payload->{sub});

# The cobrand may want to set values in the user extra field, e.g. a CRM ID
# which is passed to Open311 with reports made by this user.
Expand Down
23 changes: 22 additions & 1 deletion perllib/FixMyStreet/Cobrand/Brent.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ sub categories_restriction {
return $rs->search( { 'me.category' => { '-not_like' => 'River Piers%' } } );
}

=head2 social_auth_enabled and user_from_oidc
=head2 social_auth_enabled, user_from_oidc, and oidc_config
=over 4
Expand Down Expand Up @@ -214,6 +214,27 @@ sub user_from_oidc {
=cut

=item * Brent FMS and WasteWorks have separate OIDC configurations
This code figures out the correct OIDC config based on the hostname used
for the request.
=cut

sub oidc_config {
my $self = shift;

my $cfg = $self->{c}->cobrand->feature('oidc_login');
my $host = $self->{c}->req->uri->host;

if ($cfg->{hosts} && $cfg->{hosts}->{$host}) {
return $cfg->{hosts}->{$host};
}

return $cfg;
}


=head2 dashboard_export_problems_add_columns
Brent have various additional columns for extra report data.
Expand Down
8 changes: 7 additions & 1 deletion t/Mock/OpenIDConnect.pm
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ has cobrand => (
default => '',
);

has host => (
is => 'rw',
isa => Str,
default => '',
);

sub dispatch_request {
my $self = shift;

Expand Down Expand Up @@ -57,7 +63,7 @@ sub dispatch_request {
ver => "1.0",
iss => "https://login.example.org/12345-6789-4321-abcd-12309812309/v2.0/",
sub => "my_cool_user_id",
aud => "example_client_id",
aud => $self->host eq "brent-wasteworks-oidc.example.org" ? "wasteworks_client_id": "example_client_id",
iat => $now,
auth_time => $now,
tfp => "B2C_1_default",
Expand Down
49 changes: 48 additions & 1 deletion t/app/controller/auth_social.t
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,53 @@ for my $test (
report_email => $test_email3,
pc => 'HA9 0FJ',
},
{
type => 'oidc',
config => {
ALLOWED_COBRANDS => 'brent',
MAPIT_URL => 'http://mapit.uk/',
COBRAND_FEATURES => {
anonymous_account => {
brent => 'test',
},
oidc_login => {
brent => {
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',
password_change_uri => 'http://oidc.example.org/oauth2/v2.0/password_change',
display_name => 'MyAccount',
hosts => {
'brent-wasteworks-oidc.example.org' => {
client_id => 'wasteworks_client_id',
secret => 'wasteworks_secret_key',
auth_uri => 'http://brent-wasteworks-oidc.example.org/oauth2/v2.0/authorize',
token_uri => 'http://brent-wasteworks-oidc.example.org/oauth2/v2.0/token',
logout_uri => 'http://brent-wasteworks-oidc.example.org/oauth2/v2.0/logout',
password_change_uri => 'http://brent-wasteworks-oidc.example.org/oauth2/v2.0/password_change',
display_name => 'MyAccount - WasteWorks',
}
}
}
}
}
},
email => $mech->uniquify_email('[email protected]'),
uid => "brent:wasteworks_client_id:my_cool_user_id",
mock => 't::Mock::OpenIDConnect',
mock_hosts => ['brent-wasteworks-oidc.example.org'],
host => 'brent-wasteworks-oidc.example.org',
error_callback => '/auth/OIDC?error=ERROR',
success_callback => '/auth/OIDC?code=response-code&state=login',
redirect_pattern => qr{brent-wasteworks-oidc\.example\.org/oauth2/v2\.0/authorize},
logout_redirect_pattern => qr{brent-wasteworks-oidc\.example\.org/oauth2/v2\.0/logout},
password_change_pattern => qr{brent-wasteworks-oidc\.example\.org/oauth2/v2\.0/password_change},
report => $report3,
report_email => $test_email3,
pc => 'HA9 0FJ',
},
{
type => 'oidc',
config => {
Expand Down Expand Up @@ -207,7 +254,7 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
}

# Set up a mock to catch (most, see below) requests to the OAuth API
my $mock_api = $test->{mock}->new;
my $mock_api = $test->{mock}->new( host => $test->{host} );

if ($test->{uid} =~ /:/) {
my ($cobrand) = $test->{uid} =~ /^(.*?):/;
Expand Down
5 changes: 3 additions & 2 deletions templates/web/base/auth/general.html
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,11 @@ <h1>
</button>
</div>
[% END %]
[% IF c.cobrand.feature('oidc_login') %]
[% oidc_config = c.cobrand.call_hook('oidc_config') OR c.cobrand.feature('oidc_login') %]
[% IF oidc_config %]
<div class="form-box">
<button name="social_sign_in" id="oidc_sign_in" value="oidc" class="btn btn--block btn--social btn--oidc">
[% tprintf(loc('Login with %s'), c.cobrand.feature('oidc_login').display_name) %]
[% tprintf(loc('Login with %s'), oidc_config.display_name) %]
</button>
</div>
[% END %]
Expand Down

0 comments on commit dfd28a0

Please sign in to comment.