diff options
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth/Social.pm | 28 | ||||
| -rw-r--r-- | t/Mock/OpenIDConnect.pm | 1 | ||||
| -rw-r--r-- | t/app/controller/auth_social.t | 4 |
3 files changed, 29 insertions, 4 deletions
diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index 8ac44c14d..4bd7dcd4d 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -9,6 +9,8 @@ use Net::Twitter::Lite::WithAPIv1_1; use OIDC::Lite::Client::WebServer::Azure; use URI::Escape; +use mySociety::AuthToken; + =head1 NAME FixMyStreet::App::Controller::Auth::Social - Catalyst Controller @@ -180,12 +182,14 @@ sub oidc_sign_in : Private { $c->detach( '/page_error_400_bad_request', [] ) unless $c->cobrand->feature('oidc_login'); my $oidc = $c->forward('oidc'); + my $nonce = $self->generate_nonce(); my $url = $oidc->uri_to_redirect( redirect_uri => $c->uri_for('/auth/OIDC'), scope => 'openid', - state => 'test', + state => 'login', extra => { response_mode => 'form_post', + nonce => $nonce, }, ); @@ -193,6 +197,7 @@ sub oidc_sign_in : Private { $oauth{return_url} = $c->get_param('r'); $oauth{detach_to} = $c->stash->{detach_to}; $oauth{detach_args} = $c->stash->{detach_args}; + $oauth{nonce} = $nonce; $c->session->{oauth} = \%oauth; $c->res->redirect($url); } @@ -210,7 +215,7 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { uri => $password_reset_uri, redirect_uri => $c->uri_for('/auth/OIDC'), scope => 'openid', - state => 'test', + state => 'password_reset', extra => { response_mode => 'form_post', }, @@ -221,7 +226,14 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { $c->detach('oauth_failure'); } } - $c->detach('/page_error_400_bad_request', []) unless $c->get_param('code'); + $c->detach('/page_error_400_bad_request', []) unless $c->get_param('code') && $c->get_param('state'); + + # After a password reset on the OIDC endpoint the user isn't properly logged + # in, so redirect them to the usual OIDC login process. + $c->detach('oidc_sign_in', []) if $c->get_param('state') eq 'password_reset'; + + # 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 { @@ -239,6 +251,9 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { # sanity check the token audience is us... $c->detach('/page_error_500_internal_error', ['invalid id_token']) unless $id_token->payload->{aud} eq $c->cobrand->feature('oidc_login')->{client_id}; + # 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}; + # 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}); @@ -267,6 +282,13 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) { $c->forward('oauth_success', [ 'oidc', $uid, $name, $email, $extra ]); } +# Just a wrapper around random_token to make mocking easier. +sub generate_nonce : Private { + my ($self, $c) = @_; + + return mySociety::AuthToken::random_token(); +} + sub oauth_failure : Private { my ( $self, $c ) = @_; diff --git a/t/Mock/OpenIDConnect.pm b/t/Mock/OpenIDConnect.pm index 3fb15ff70..77ed88817 100644 --- a/t/Mock/OpenIDConnect.pm +++ b/t/Mock/OpenIDConnect.pm @@ -48,6 +48,7 @@ sub dispatch_request { family_name => "Dwyer", tfp => "B2C_1_default", extension_CrmContactId => "1c304134-ef12-c128-9212-123908123901", + nonce => 'MyAwesomeRandomValue', }; $payload->{emails} = ['oidc@example.org'] if $self->returns_email; my $signature = "dummy"; diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t index 6c80c9857..3a0539452 100644 --- a/t/app/controller/auth_social.t +++ b/t/app/controller/auth_social.t @@ -32,6 +32,8 @@ my $contact2 = $mech->create_contact_ok( ); my $resolver = Test::MockModule->new('Email::Valid'); +my $social = Test::MockModule->new('FixMyStreet::App::Controller::Auth::Social'); +$social->mock('generate_nonce', sub { 'MyAwesomeRandomValue' }); for my $test ( { @@ -72,7 +74,7 @@ for my $test ( mock_hosts => ['oidc.example.org'], host => 'oidc.example.org', error_callback => '/auth/OIDC?error=ERROR', - success_callback => '/auth/OIDC?code=response-code', + success_callback => '/auth/OIDC?code=response-code&state=login', redirect_pattern => qr{oidc\.example\.org/oauth2/v2\.0/authorize}, user_extras => [ [westminster_account_id => "1c304134-ef12-c128-9212-123908123901"], |