From 3b958bc30c5ccb6ea3143c08d1ca65dc0bf4b9bc Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Wed, 13 May 2020 16:38:09 +0100 Subject: Rename O::L::C::W::Azure to O::L::C::W::AuthCodeFlow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Turns out there’s nothing strictly Azure-specific about it. --- perllib/FixMyStreet/App/Controller/Auth/Social.pm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller/Auth/Social.pm') diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index 06e67573f..54cf35315 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -6,7 +6,7 @@ BEGIN { extends 'Catalyst::Controller'; } use Net::Facebook::Oauth2; use Net::Twitter::Lite::WithAPIv1_1; -use OIDC::Lite::Client::WebServer::Azure; +use OIDC::Lite::Client::WebServer::AuthCodeFlow; use URI::Escape; use mySociety::AuthToken; @@ -167,7 +167,7 @@ sub oidc : Private { my $config = $c->cobrand->feature('oidc_login'); - OIDC::Lite::Client::WebServer::Azure->new( + OIDC::Lite::Client::WebServer::AuthCodeFlow->new( id => $config->{client_id}, secret => $config->{secret}, authorize_uri => $config->{auth_uri}, -- cgit v1.2.3 From c505cbca2576f9e658692d8e1c512f645069985a Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Wed, 13 May 2020 16:41:26 +0100 Subject: OIDC scope/token parsing improvements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Cobrand config can now specify custom scope and other params e.g. G Suite supports per-domain customisation and the ‘prompt’ param to always ask the user to select the account they want to login with. - Token may have an ‘name’ claim instead of needing to concat given_/family_name claims --- perllib/FixMyStreet/App/Controller/Auth/Social.pm | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'perllib/FixMyStreet/App/Controller/Auth/Social.pm') diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index 54cf35315..2468cfad2 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -179,7 +179,9 @@ sub oidc_sign_in : Private { my ( $self, $c ) = @_; $c->detach( '/page_error_403_access_denied', [] ) if FixMyStreet->config('SIGNUPS_DISABLED'); - $c->detach( '/page_error_400_bad_request', [] ) unless $c->cobrand->feature('oidc_login'); + + my $cfg = $c->cobrand->feature('oidc_login'); + $c->detach( '/page_error_400_bad_request', [] ) unless $cfg; my $oidc = $c->forward('oidc'); my $nonce = $self->generate_nonce(); @@ -190,6 +192,15 @@ sub oidc_sign_in : Private { extra => { response_mode => 'form_post', nonce => $nonce, + # auth_extra_params provides a way to pass custom parameters + # to the OIDC endpoint for the intial authentication request. + # This allows, for example, a custom scope to be used, + # or the `hd` parameter which customises the appearance of + # the login form. + # This is primarily useful for Google G Suite authentication - see + # available parameters here: + # https://developers.google.com/identity/protocols/oauth2/openid-connect#authenticationuriparameters + %{ $cfg->{auth_extra_params} || {} } , }, ); @@ -201,14 +212,14 @@ sub oidc_sign_in : Private { # The OIDC endpoint may require a specific URI to be called to log the user # out when they log out of FMS. - if ( my $redirect_uri = $c->cobrand->feature('oidc_login')->{logout_uri} ) { + if ( my $redirect_uri = $cfg->{logout_uri} ) { $redirect_uri .= "?post_logout_redirect_uri="; $redirect_uri .= URI::Escape::uri_escape( $c->uri_for('/auth/sign_out') ); $oauth{logout_redirect_uri} = $redirect_uri; } # The OIDC endpoint may provide a specific URI for changing the user's password. - if ( my $password_change_uri = $c->cobrand->feature('oidc_login')->{password_change_uri} ) { + if ( my $password_change_uri = $cfg->{password_change_uri} ) { $oauth{change_password_uri} = $oidc->uri_to_redirect( uri => $password_change_uri, redirect_uri => $c->uri_for('/auth/OIDC'), @@ -295,9 +306,9 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { $c->detach('/page_error_500_internal_error', ['invalid id_token']) unless $id_token->payload->{nonce} eq $c->session->{oauth}{nonce}; # Some claims need parsing into a friendlier format - # XXX check how much of this is Westminster/Azure-specific - my $name = join(" ", $id_token->payload->{given_name}, $id_token->payload->{family_name}); + my $name = $id_token->payload->{name} || join(" ", $id_token->payload->{given_name}, $id_token->payload->{family_name}); my $email = $id_token->payload->{email}; + # WCC Azure provides a single email address as an array for some reason my $emails = $id_token->payload->{emails}; if ($emails && @$emails) { -- cgit v1.2.3 From 7e4ebd6568a836683f5f374ce6f0d9ed97d38c54 Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Wed, 13 May 2020 16:42:04 +0100 Subject: Include redirect URI when fetching OIDC access token --- perllib/FixMyStreet/App/Controller/Auth/Social.pm | 1 + 1 file changed, 1 insertion(+) (limited to 'perllib/FixMyStreet/App/Controller/Auth/Social.pm') diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index 2468cfad2..c9e0381a4 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -290,6 +290,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { eval { $id_token = $oidc->get_access_token( code => $c->get_param('code'), + redirect_uri => $c->uri_for('/auth/OIDC') ); }; if ($@) { -- cgit v1.2.3 From aad79063d6e01f7596da564b274b3bf78413fdfa Mon Sep 17 00:00:00 2001 From: Dave Arter Date: Mon, 1 Jun 2020 13:49:52 +0100 Subject: Add allowed_domains OIDC config to limit logins to specific domains --- perllib/FixMyStreet/App/Controller/Auth/Social.pm | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'perllib/FixMyStreet/App/Controller/Auth/Social.pm') diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index c9e0381a4..ce94fe256 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -306,6 +306,14 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { # check that the nonce matches what we set in the user session $c->detach('/page_error_500_internal_error', ['invalid id_token']) unless $id_token->payload->{nonce} eq $c->session->{oauth}{nonce}; + if (my $domains = $c->cobrand->feature('oidc_login')->{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}; + my %allowed_domains = map { $_ => 1} @$domains; + $c->detach('oauth_failure') unless $allowed_domains{$hd}; + } + # Some claims need parsing into a friendlier format my $name = $id_token->payload->{name} || join(" ", $id_token->payload->{given_name}, $id_token->payload->{family_name}); my $email = $id_token->payload->{email}; -- cgit v1.2.3