diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth.pm | 30 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Auth/Social.pm | 129 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/New.pm | 15 | ||||
| -rw-r--r-- | perllib/FixMyStreet/App/Controller/Report/Update.pm | 15 | ||||
| -rw-r--r-- | perllib/FixMyStreet/Cobrand/Default.pm | 2 | ||||
| -rw-r--r-- | perllib/OIDC/Lite/Client/IDTokenResponseParser.pm | 72 | ||||
| -rw-r--r-- | perllib/OIDC/Lite/Client/WebServer/Azure.pm | 39 | ||||
| -rw-r--r-- | t/Mock/OpenIDConnect.pm | 70 | ||||
| -rw-r--r-- | t/app/controller/auth_social.t | 144 | ||||
| -rw-r--r-- | templates/web/base/auth/general.html | 11 | ||||
| -rw-r--r-- | templates/web/base/report/form/user.html | 9 | ||||
| -rw-r--r-- | web/cobrands/fixmystreet/fixmystreet.js | 2 |
13 files changed, 454 insertions, 85 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index 3afd39446..1190c81eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Add new roles system, to group permissions and apply to users. #2483 - New features: - Categories can be listed under more than one group #2475 + - OpenID Connect login support. #2523 - Bugfixes: - Prevent creation of two templates with same title. #2471 - Fix bug going between report/new pages client side. #2484 diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm index 50d11a10c..b2c001566 100644 --- a/perllib/FixMyStreet/App/Controller/Auth.pm +++ b/perllib/FixMyStreet/App/Controller/Auth.pm @@ -44,13 +44,12 @@ sub general : Path : Args(0) { # decide which action to take $c->detach('code_sign_in') if $clicked_sign_in_by_code || ($data_email && !$data_password); - if (!$data_username && !$data_password && !$data_email) { - $c->detach('social/facebook_sign_in') if $c->get_param('facebook_sign_in'); - $c->detach('social/twitter_sign_in') if $c->get_param('twitter_sign_in'); + if (!$data_username && !$data_password && !$data_email && $c->get_param('social_sign_in')) { + $c->forward('social/handle_sign_in'); } - $c->forward( 'sign_in', [ $data_username ] ) - && $c->detach( 'redirect_on_signin', [ $c->get_param('r') ] ); + $c->forward( 'sign_in', [ $data_username ] ) + && $c->detach( 'redirect_on_signin', [ $c->get_param('r') ] ); } @@ -180,13 +179,13 @@ sub email_sign_in : Private { name => $c->get_param('name'), password => $user->password, }; - $token_data->{name} = $c->session->{oauth}{name} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{name} && !$token_data->{name}; - $token_data->{facebook_id} = $c->session->{oauth}{facebook_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{facebook_id}; - $token_data->{twitter_id} = $c->session->{oauth}{twitter_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{twitter_id}; + if ($c->get_param('oauth_need_email')) { + $token_data->{name} = $c->session->{oauth}{name} + if $c->session->{oauth}{name} && !$token_data->{name}; + $c->forward('set_oauth_token_data', [ $token_data ]); + } + if ($c->stash->{current_user}) { $token_data->{old_user_id} = $c->stash->{current_user}->id; $token_data->{r} = 'auth/change_email/success'; @@ -217,6 +216,14 @@ sub get_token : Private { return $data; } +sub set_oauth_token_data : Private { + my ( $self, $c, $token_data ) = @_; + + foreach (qw/facebook_id twitter_id oidc_id/) { + $token_data->{$_} = $c->session->{oauth}{$_} if $c->session->{oauth}{$_}; + } +} + =head2 token Handle the 'email_sign_in' tokens. Find the account for the email address @@ -275,6 +282,7 @@ sub process_login : Private { $user->password( $data->{password}, 1 ) if $data->{password}; $user->facebook_id( $data->{facebook_id} ) if $data->{facebook_id}; $user->twitter_id( $data->{twitter_id} ) if $data->{twitter_id}; + $user->add_oidc_id( $data->{oidc_id} ) if $data->{oidc_id}; $user->update_or_insert; $c->authenticate( { $type => $data->{$type}, $ver => 1 }, 'no_password' ); diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm index d1a2c37cb..e0abf5c60 100644 --- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm +++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm @@ -6,6 +6,7 @@ BEGIN { extends 'Catalyst::Controller'; } use Net::Facebook::Oauth2; use Net::Twitter::Lite::WithAPIv1_1; +use OIDC::Lite::Client::WebServer::Azure; =head1 NAME @@ -13,10 +14,26 @@ FixMyStreet::App::Controller::Auth::Social - Catalyst Controller =head1 DESCRIPTION -Controller for the Facebook/Twitter authentication. +Controller for the Facebook/Twitter/OpenID Connect authentication. =head1 METHODS +=head2 handle_sign_in + +Forwards to the appropriate (facebook|twitter|oidc)_sign_in method +based on the social_sign_in parameter + +=cut + +sub handle_sign_in : Private { + my ($self, $c) = @_; + + $c->detach('facebook_sign_in') if $c->get_param('social_sign_in') eq 'facebook'; + $c->detach('twitter_sign_in') if $c->get_param('social_sign_in') eq 'twitter'; + $c->detach('oidc_sign_in') if $c->get_param('social_sign_in') eq 'oidc'; + +} + =head2 facebook_sign_in Starts the Facebook authentication sequence. @@ -142,6 +159,84 @@ sub twitter_callback: Path('/auth/Twitter') : Args(0) { $c->forward('oauth_success', [ 'twitter', $info->{id}, $info->{name} ]); } +sub oidc : Private { + my ($self, $c) = @_; + + my $config = $c->cobrand->feature('oidc_login'); + + OIDC::Lite::Client::WebServer::Azure->new( + id => $config->{client_id}, + secret => $config->{secret}, + authorize_uri => $config->{auth_uri}, + access_token_uri => $config->{token_uri}, + ); +} + +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 $oidc = $c->forward('oidc'); + my $url = $oidc->uri_to_redirect( + redirect_uri => $c->uri_for('/auth/OIDC'), + scope => 'openid', + state => 'test', + extra => { + response_mode => 'form_post', + }, + ); + + my %oauth; + $oauth{return_url} = $c->get_param('r'); + $oauth{detach_to} = $c->stash->{detach_to}; + $oauth{detach_args} = $c->stash->{detach_args}; + $c->session->{oauth} = \%oauth; + $c->res->redirect($url); +} + +sub oidc_callback: Path('/auth/OIDC') : Args(0) { + my ( $self, $c ) = @_; + + $c->detach('oauth_failure') if $c->get_param('error'); + $c->detach('/page_error_400_bad_request', []) unless $c->get_param('code'); + + my $oidc = $c->forward('oidc'); + + my $id_token; + eval { + $id_token = $oidc->get_access_token( + code => $c->get_param('code'), + ); + }; + if ($@) { + (my $message = $@) =~ s/at [^ ]*Auth.pm.*//; + $c->detach('/page_error_500_internal_error', [ $message ]); + } + + $c->detach('oauth_failure') unless $id_token; + + # 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}; + + # 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 $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) { + $email = $emails->[0]; + } + + # 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}); + + $c->forward('oauth_success', [ 'oidc', $uid, $name, $email ]); +} + + sub oauth_failure : Private { my ( $self, $c ) = @_; @@ -159,19 +254,39 @@ sub oauth_success : Private { my $user; if ($email) { - # Only Facebook gets here + # Only Facebook & OIDC get here # We've got an ID and an email address + # Remove any existing mention of this ID - my $existing = $c->model('DB::User')->find( { facebook_id => $uid } ); - $existing->update( { facebook_id => undef } ) if $existing; - # Get or create a user, give it this Facebook ID + my $existing; + if ($type eq 'facebook') { + $existing = $c->model('DB::User')->find( { $type . '_id' => $uid } ); + $existing->update( { $type . '_id' => undef } ) if $existing; + } elsif ( $type eq 'oidc' ) { + $existing = $c->model('DB::User')->find( { oidc_ids => \[ + '&& ?', [ oidc_ids => [ $uid ] ] + ] } ); + $existing->remove_oidc_id( $uid ) if $existing; + } + + # Get or create a user, give it this Facebook/OIDC ID $user = $c->model('DB::User')->find_or_new( { email => $email } ); - $user->facebook_id($uid); + if ( $type eq 'facebook' ) { + $user->facebook_id($uid); + } elsif ( $type eq 'oidc' ) { + $user->add_oidc_id($uid); + } $user->name($name); $user->in_storage() ? $user->update : $user->insert; } else { # We've got an ID, but no email - $user = $c->model('DB::User')->find( { $type . '_id' => $uid } ); + if ($type eq 'oidc') { + $user = $c->model('DB::User')->find( { oidc_ids => \[ + '&& ?', [ oidc_ids => [ $uid ] ] + ] } ); + } else { + $user = $c->model('DB::User')->find( { $type . '_id' => $uid } ); + } if ($user) { # Matching ID in our database $user->name($name); diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm index 120467905..7a3e94f95 100644 --- a/perllib/FixMyStreet/App/Controller/Report/New.pm +++ b/perllib/FixMyStreet/App/Controller/Report/New.pm @@ -1155,7 +1155,7 @@ sub check_for_errors : Private { } # if using social login then we don't care about other errors - $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); + $c->stash->{is_social_user} = $c->get_param('social_sign_in') ? 1 : 0; if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; delete $field_errors{username}; @@ -1197,10 +1197,8 @@ sub tokenize_user : Private { password => $report->user->password, title => $report->user->title, }; - $c->stash->{token_data}{facebook_id} = $c->session->{oauth}{facebook_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{facebook_id}; - $c->stash->{token_data}{twitter_id} = $c->session->{oauth}{twitter_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{twitter_id}; + $c->forward('/auth/set_oauth_token_data', [ $c->stash->{token_data} ]) + if $c->get_param('oauth_need_email'); } sub send_problem_confirm_email : Private { @@ -1308,6 +1306,7 @@ sub process_confirmation : Private { for (qw(name title facebook_id twitter_id)) { $problem->user->$_( $data->{$_} ) if $data->{$_}; } + $problem->user->add_oidc_id($data->{oidc_id}) if $data->{oidc_id}; $problem->user->update; } if ($problem->user->email_verified) { @@ -1368,11 +1367,7 @@ sub save_user_and_report : Private { $c->stash->{detach_to} = '/report/new/oauth_callback'; $c->stash->{detach_args} = [$token->token]; - if ( $c->get_param('facebook_sign_in') ) { - $c->detach('/auth/social/facebook_sign_in'); - } elsif ( $c->get_param('twitter_sign_in') ) { - $c->detach('/auth/social/twitter_sign_in'); - } + $c->forward('/auth/social/handle_sign_in') if $c->get_param('social_sign_in'); } # Save or update the user if appropriate diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm index cbedf7a01..68c8a41ce 100644 --- a/perllib/FixMyStreet/App/Controller/Report/Update.pm +++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm @@ -366,7 +366,7 @@ sub check_for_errors : Private { ); # if using social login then we don't care about name and email errors - $c->stash->{is_social_user} = $c->get_param('facebook_sign_in') || $c->get_param('twitter_sign_in'); + $c->stash->{is_social_user} = $c->get_param('social_sign_in') ? 1 : 0; if ( $c->stash->{is_social_user} ) { delete $field_errors{name}; delete $field_errors{username}; @@ -404,10 +404,8 @@ sub tokenize_user : Private { name => $update->user->name, password => $update->user->password, }; - $c->stash->{token_data}{facebook_id} = $c->session->{oauth}{facebook_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{facebook_id}; - $c->stash->{token_data}{twitter_id} = $c->session->{oauth}{twitter_id} - if $c->get_param('oauth_need_email') && $c->session->{oauth}{twitter_id}; + $c->forward('/auth/set_oauth_token_data', [ $c->stash->{token_data} ]) + if $c->get_param('oauth_need_email'); } =head2 save_update @@ -440,11 +438,7 @@ sub save_update : Private { $c->stash->{detach_to} = '/report/update/oauth_callback'; $c->stash->{detach_args} = [$token->token]; - if ( $c->get_param('facebook_sign_in') ) { - $c->detach('/auth/social/facebook_sign_in'); - } elsif ( $c->get_param('twitter_sign_in') ) { - $c->detach('/auth/social/twitter_sign_in'); - } + $c->forward('/auth/social/handle_sign_in') if $c->get_param('social_sign_in'); } if ( $c->cobrand->never_confirm_updates ) { @@ -585,6 +579,7 @@ sub process_confirmation : Private { for (qw(name facebook_id twitter_id)) { $comment->user->$_( $data->{$_} ) if $data->{$_}; } + $comment->user->add_oidc_id($data->{oidc_id}) if $data->{oidc_id}; $comment->user->password( $data->{password}, 1 ) if $data->{password}; $comment->user->update; } diff --git a/perllib/FixMyStreet/Cobrand/Default.pm b/perllib/FixMyStreet/Cobrand/Default.pm index eaf27e3bc..7115bc66b 100644 --- a/perllib/FixMyStreet/Cobrand/Default.pm +++ b/perllib/FixMyStreet/Cobrand/Default.pm @@ -1264,7 +1264,7 @@ sub allow_report_extra_fields { 0 } sub social_auth_enabled { my $self = shift; - my $key_present = FixMyStreet->config('FACEBOOK_APP_ID') or FixMyStreet->config('TWITTER_KEY'); + my $key_present = FixMyStreet->config('FACEBOOK_APP_ID') || FixMyStreet->config('TWITTER_KEY'); return $key_present && !$self->call_hook("social_auth_disabled"); } diff --git a/perllib/OIDC/Lite/Client/IDTokenResponseParser.pm b/perllib/OIDC/Lite/Client/IDTokenResponseParser.pm new file mode 100644 index 000000000..41db1bbdb --- /dev/null +++ b/perllib/OIDC/Lite/Client/IDTokenResponseParser.pm @@ -0,0 +1,72 @@ +package OIDC::Lite::Client::IDTokenResponseParser; + +use strict; +use warnings; + +use Try::Tiny qw/try catch/; +use OIDC::Lite::Model::IDToken; +use OAuth::Lite2::Formatters; +use OAuth::Lite2::Client::Error; + +=head1 NAME + +OIDC::Lite::Client::IDTokenResponseParser - parse id_token JWT into an +L<OIDC::Lite::Model::IDToken> object. + +Acts the same as L<OIDC::Lite::Client::TokenResponseParser> but looks for an +id_token in the HTTP response instead of access_token. + +=cut + +sub new { + bless {}, $_[0]; +} + +sub parse { + my ($self, $http_res) = @_; + + my $formatter = + OAuth::Lite2::Formatters->get_formatter_by_type( + $http_res->content_type); + + my $token; + + if ($http_res->is_success) { + + OAuth::Lite2::Client::Error::InvalidResponse->throw( + message => sprintf(q{Invalid response content-type: %s}, + $http_res->content_type||'') + ) unless $formatter; + + my $result = try { + return $formatter->parse($http_res->content); + } catch { + OAuth::Lite2::Client::Error::InvalidResponse->throw( + message => sprintf(q{Invalid response format: %s}, $_), + ); + }; + + OAuth::Lite2::Client::Error::InvalidResponse->throw( + message => sprintf("Response doesn't include 'id_token'") + ) unless exists $result->{id_token}; + + $token = OIDC::Lite::Model::IDToken->load($result->{id_token}); + + } else { + + my $errmsg = $http_res->content || $http_res->status_line; + if ($formatter && $http_res->content) { + try { + my $result = $formatter->parse($http_res->content); + $errmsg = $result->{error} + if exists $result->{error}; + } catch { + return OAuth::Lite2::Client::Error::InvalidResponse->throw; + }; + } + OAuth::Lite2::Client::Error::InvalidResponse->throw( message => $errmsg ); + } + return $token; +} + +1; diff --git a/perllib/OIDC/Lite/Client/WebServer/Azure.pm b/perllib/OIDC/Lite/Client/WebServer/Azure.pm new file mode 100644 index 000000000..b19dce90e --- /dev/null +++ b/perllib/OIDC/Lite/Client/WebServer/Azure.pm @@ -0,0 +1,39 @@ +package OIDC::Lite::Client::WebServer::Azure; + +use strict; +use warnings; +use parent 'OIDC::Lite::Client::WebServer'; + +use OIDC::Lite::Client::IDTokenResponseParser; + +=head1 NAME + +OIDC::Lite::Client::WebServer::Azure - extension to auth against Azure AD B2C + +OIDC::Lite doesn't appear to support the authorisation code flow to get an +ID token - only an access token. Azure returns all its claims in the id_token +and doesn't support a UserInfo endpoint, so this extension adds support for +parsing the id_token when calling get_access_token. + +=cut + +=head2 new + +Overrides response_parser so that get_access_token returns a +L<OIDC::Lite::Model::IDToken> object. + +NB this does not perform any verification of the id_token. It's assumed to be +safe as it's come directly from the OpenID IdP and not an untrusted user's +browser. + +=cut + +sub new { + my $self = shift->next::method(@_); + + $self->{response_parser} = OIDC::Lite::Client::IDTokenResponseParser->new; + + return $self; +} + +1; diff --git a/t/Mock/OpenIDConnect.pm b/t/Mock/OpenIDConnect.pm new file mode 100644 index 000000000..ca0da48cd --- /dev/null +++ b/t/Mock/OpenIDConnect.pm @@ -0,0 +1,70 @@ +package t::Mock::OpenIDConnect; + +use JSON::MaybeXS; +use Web::Simple; +use DateTime; +use MIME::Base64 qw(encode_base64); +use MooX::Types::MooseLike::Base qw(:all); + +has json => ( + is => 'lazy', + default => sub { + JSON->new->pretty->allow_blessed->convert_blessed; + }, +); + +has returns_email => ( + is => 'rw', + isa => Bool, + default => 1, +); + +sub dispatch_request { + my $self = shift; + + sub (GET + /oauth2/v2.0/authorize + ?*) { + my ($self) = @_; + return [ 200, [ 'Content-Type' => 'text/html' ], [ 'OpenID Connect login page' ] ]; + }, + + sub (POST + /oauth2/v2.0/token + ?*) { + my ($self) = @_; + my $header = { + typ => "JWT", + alg => "RS256", + kid => "XXXfakeKEY1234", + }; + my $now = DateTime->now->epoch; + my $payload = { + exp => $now + 3600, + nbf => $now, + ver => "1.0", + iss => "https://login.example.org/12345-6789-4321-abcd-12309812309/v2.0/", + sub => "my_cool_user_id", + aud => "example_client_id", + iat => $now, + auth_time => $now, + given_name => "Andy", + family_name => "Dwyer", + tfp => "B2C_1_default" + }; + $payload->{emails} = ['oidc@example.org'] if $self->returns_email; + my $signature = "dummy"; + my $id_token = join(".", ( + encode_base64($self->json->encode($header), ''), + encode_base64($self->json->encode($payload), ''), + encode_base64($signature, '') + )); + my $data = { + id_token => $id_token, + token_type => "Bearer", + not_before => $now, + id_token_expires_in => 3600, + profile_info => encode_base64($self->json->encode({}), ''), + }; + my $json = $self->json->encode($data); + return [ 200, [ 'Content-Type' => 'application/json' ], [ $json ] ]; + }, +} + +__PACKAGE__->run_if_script; diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t index 75eabfc43..0f092b480 100644 --- a/t/app/controller/auth_social.t +++ b/t/app/controller/auth_social.t @@ -5,10 +5,11 @@ use JSON::MaybeXS; use t::Mock::Facebook; use t::Mock::Twitter; +use t::Mock::OpenIDConnect; use FixMyStreet::TestMech; my $mech = FixMyStreet::TestMech->new; - + # disable info logs for this test run FixMyStreet::App->log->disable('info'); END { FixMyStreet::App->log->enable('info'); } @@ -30,44 +31,86 @@ my $contact2 = $mech->create_contact_ok( body_id => $body->id, category => 'Whatever', email => 'WHATEVER', ); -FixMyStreet::override_config { - FACEBOOK_APP_ID => 'facebook-app-id', - TWITTER_KEY => 'twitter-key', - ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], - MAPIT_URL => 'http://mapit.uk/', -}, sub { +my $resolver = Test::MockModule->new('Email::Valid'); -my $fb_email = 'facebook@example.org'; -my $fb_uid = 123456789; +for my $test ( + { + type => 'facebook', + config => { + FACEBOOK_APP_ID => 'facebook-app-id', + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + }, + email => 'facebook@example.org', + uid => 123456789, + mock => 't::Mock::Facebook', + mock_hosts => ['www.facebook.com', 'graph.facebook.com'], + host => 'www.facebook.com', + error_callback => '/auth/Facebook?error_code=ERROR', + success_callback => '/auth/Facebook?code=response-code', + redirect_pattern => qr{facebook\.com.*dialog/oauth.*facebook-app-id}, +}, { + type => 'oidc', + config => { + ALLOWED_COBRANDS => [ { westminster => '.' } ], + MAPIT_URL => 'http://mapit.uk/', + COBRAND_FEATURES => { + oidc_login => { + westminster => { + 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', + display_name => 'MyWestminster' + } + } + } + }, + email => 'oidc@example.org', + uid => "westminster:example_client_id:my_cool_user_id", + mock => 't::Mock::OpenIDConnect', + mock_hosts => ['oidc.example.org'], + host => 'oidc.example.org', + error_callback => '/auth/OIDC?error=ERROR', + success_callback => '/auth/OIDC?code=response-code', + redirect_pattern => qr{oidc\.example\.org/oauth2/v2\.0/authorize}, +} +) { -my $resolver = Test::MockModule->new('Email::Valid'); -$resolver->mock('address', sub { 'facebook@example.org' }); +FixMyStreet::override_config $test->{config}, sub { + +$resolver->mock('address', sub { $test->{email} }); -for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { +for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) { for my $page ( 'my', 'report', 'update' ) { - subtest "test FB '$fb_state' login for page '$page'" => sub { + subtest "test $test->{type} '$state' login for page '$page'" => sub { # Lots of user changes happening here, make sure we don't confuse # Catalyst with a cookie session user that no longer exists $mech->log_out_ok; $mech->cookie_jar({}); - if ($fb_state eq 'existing UID') { - my $user = $mech->create_user_ok($fb_email); - $user->update({ facebook_id => $fb_uid }); + if ($state eq 'existing UID') { + my $user = $mech->create_user_ok($test->{email}); + if ($test->{type} eq 'facebook') { + $user->update({ facebook_id => $test->{uid} }); + } elsif ($test->{type} eq 'oidc') { + $user->update({ oidc_ids => [ $test->{uid} ] }); + } } else { - $mech->delete_user($fb_email); + $mech->delete_user($test->{email}); } - # Set up a mock to catch (most, see below) requests to Facebook - my $fb = t::Mock::Facebook->new; - $fb->returns_email(0) if $fb_state eq 'no email' || $fb_state eq 'existing UID'; - LWP::Protocol::PSGI->register($fb->to_psgi_app, host => 'www.facebook.com'); - LWP::Protocol::PSGI->register($fb->to_psgi_app, host => 'graph.facebook.com'); + # Set up a mock to catch (most, see below) requests to the OAuth API + my $mock_api = $test->{mock}->new; + $mock_api->returns_email(0) if $state eq 'no email' || $state eq 'existing UID'; + for my $host (@{ $test->{mock_hosts} }) { + LWP::Protocol::PSGI->register($mock_api->to_psgi_app, host => $host); + } # Due to https://metacpan.org/pod/Test::WWW::Mechanize::Catalyst#External-Redirects-and-allow_external - # the redirect to Facebook's OAuth page can mess up the session - # cookie. So let's pretend we always on www.facebook.com, which + # the redirect to the OAuth page can mess up the session + # cookie. So let's pretend we're always on the API host, which # sorts that out. - $mech->host('www.facebook.com'); + $mech->host($test->{host}); # Fetch the page with the form via which we wish to log in my $fields; @@ -91,20 +134,23 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { update => 'Test update', }; } - $mech->submit_form(with_fields => $fields, button => 'facebook_sign_in'); + $mech->submit_form(with_fields => $fields, button => 'social_sign_in'); # As well as the cookie issue above, caused by this external # redirect rewriting the host, the redirect gets handled directly # by Catalyst, not our mocked handler, so will be a 404. Check # the redirect happened instead. - is $mech->res->previous->code, 302, 'FB button redirected'; - like $mech->res->previous->header('Location'), qr{facebook\.com.*dialog/oauth.*facebook-app-id}, 'FB redirect to oauth URL'; - - # Okay, now call the callback Facebook would send us to - if ($fb_state eq 'refused') { - $mech->get_ok('/auth/Facebook?error_code=ERROR'); + is $mech->res->previous->code, 302, "$test->{type} button redirected"; + like $mech->res->previous->header('Location'), $test->{redirect_pattern}, "$test->{type} redirect to oauth URL"; + + # Okay, now call the callback we'd be sent to + # NB: for OIDC these should be post_ok, but that doesn't work because + # the session cookie doesn't seem to be included (related to the + # cookie issue above perhaps). + if ($state eq 'refused') { + $mech->get_ok($test->{error_callback}); } else { - $mech->get_ok('/auth/Facebook?code=response-code'); + $mech->get_ok($test->{success_callback}); } # Check we're showing the right form, regardless of what came back @@ -115,14 +161,14 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { $mech->content_contains('/report/update'); } - if ($fb_state eq 'refused') { + if ($state eq 'refused') { $mech->content_contains('Sorry, we could not log you in. Please fill in the form below.'); $mech->not_logged_in_ok; - } elsif ($fb_state eq 'no email') { + } elsif ($state eq 'no email') { $mech->content_contains('We need your email address, please give it below.'); # We don't have an email, so check that we can still submit it, # and the ID carries through the confirmation - $fields->{username} = $fb_email; + $fields->{username} = $test->{email}; $fields->{name} = 'Ffion Tester' unless $page eq 'my'; $mech->submit_form(with_fields => $fields, $page eq 'my' ? (button => 'sign_in_by_code') : ()); $mech->content_contains('Nearly done! Now check your email'); @@ -131,15 +177,23 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { $mech->clear_emails_ok; ok $url, "extracted confirm url '$url'"; - my $user = FixMyStreet::App->model( 'DB::User' )->find( { email => $fb_email } ); + my $user = FixMyStreet::App->model( 'DB::User' )->find( { email => $test->{email} } ); if ($page eq 'my') { is $user, undef, 'No user yet exists'; } else { - is $user->facebook_id, undef, 'User has no facebook ID'; + if ($test->{type} eq 'facebook') { + is $user->facebook_id, undef, 'User has no facebook ID'; + } elsif ($test->{type} eq 'oidc') { + is $user->oidc_ids, undef, 'User has no OIDC IDs'; + } } $mech->get_ok( $url ); - $user = FixMyStreet::App->model( 'DB::User' )->find( { email => $fb_email } ); - is $user->facebook_id, $fb_uid, 'User now has correct facebook ID'; + $user = FixMyStreet::App->model( 'DB::User' )->find( { email => $test->{email} } ); + if ($test->{type} eq 'facebook') { + is $user->facebook_id, $test->{uid}, 'User now has correct facebook ID'; + } elsif ($test->{type} eq 'oidc') { + is_deeply $user->oidc_ids, [ $test->{uid} ], 'User now has correct OIDC IDs'; + } } elsif ($page ne 'my') { # /my auth login goes directly there, no message like this @@ -151,6 +205,14 @@ for my $fb_state ( 'refused', 'no email', 'existing UID', 'okay' ) { } } } +} +}; + +FixMyStreet::override_config { + TWITTER_KEY => 'twitter-key', + ALLOWED_COBRANDS => [ { fixmystreet => '.' } ], + MAPIT_URL => 'http://mapit.uk/', +}, sub { $resolver->mock('address', sub { 'twitter@example.org' }); @@ -204,7 +266,7 @@ for my $tw_state ( 'refused', 'existing UID', 'no email' ) { update => 'Test update', }; } - $mech->submit_form(with_fields => $fields, button => 'twitter_sign_in'); + $mech->submit_form(with_fields => $fields, button => 'social_sign_in'); # As well as the cookie issue above, caused by this external # redirect rewriting the host, the redirect gets handled directly diff --git a/templates/web/base/auth/general.html b/templates/web/base/auth/general.html index e2e880871..30b1bf63f 100644 --- a/templates/web/base/auth/general.html +++ b/templates/web/base/auth/general.html @@ -24,15 +24,22 @@ [% IF NOT oauth_need_email AND c.cobrand.social_auth_enabled %] [% IF c.config.FACEBOOK_APP_ID %] <div class="form-box"> - <button name="facebook_sign_in" id="facebook_sign_in" value="facebook_sign_in" class="btn btn--block btn--social btn--facebook"> + <button name="social_sign_in" id="facebook_sign_in" value="facebook" class="btn btn--block btn--social btn--facebook"> <img alt="" src="/i/facebook-icon-32.png" width="17" height="32"> [% loc('Log in with Facebook') %] </button> </div> [% END %] + [% IF c.cobrand.feature('oidc_login') %] + <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) %] + </button> + </div> + [% END %] [% IF c.config.TWITTER_KEY %] <div class="form-box"> - <button name="twitter_sign_in" id="twitter_sign_in" value="twitter_sign_in" class="btn btn--block btn--social btn--twitter"> + <button name="social_sign_in" id="twitter_sign_in" value="twitter" class="btn btn--block btn--social btn--twitter"> <img alt="" src="/i/twitter-icon-32.png" width="17" height="32"> [% loc('Log in with Twitter') %] </button> diff --git a/templates/web/base/report/form/user.html b/templates/web/base/report/form/user.html index 6381d2928..5f14d9adc 100644 --- a/templates/web/base/report/form/user.html +++ b/templates/web/base/report/form/user.html @@ -8,13 +8,18 @@ <button class="btn btn--block hidden-nojs js-new-report-user-show">[% loc('Continue') %]</button> [% ELSE %] [% IF c.config.FACEBOOK_APP_ID %] - <button name="facebook_sign_in" id="facebook_sign_in" value="facebook_sign_in" class="btn btn--block btn--social btn--facebook"> + <button name="social_sign_in" id="facebook_sign_in" value="facebook" class="btn btn--block btn--social btn--facebook"> <img alt="" src="/i/facebook-icon-32.png" width="17" height="32"> [% loc('Log in with Facebook') %] </button> [% END %] + [% IF c.cobrand.feature('oidc_login') %] + <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) %] + </button> + [% END %] [% IF c.config.TWITTER_KEY %] - <button name="twitter_sign_in" id="twitter_sign_in" value="twitter_sign_in" class="btn btn--block btn--social btn--twitter"> + <button name="social_sign_in" id="twitter_sign_in" value="twitter" class="btn btn--block btn--social btn--twitter"> <img alt="" src="/i/twitter-icon-32.png" width="17" height="32"> [% loc('Log in with Twitter') %] </button> diff --git a/web/cobrands/fixmystreet/fixmystreet.js b/web/cobrands/fixmystreet/fixmystreet.js index 7aac072ea..ac2d1e2da 100644 --- a/web/cobrands/fixmystreet/fixmystreet.js +++ b/web/cobrands/fixmystreet/fixmystreet.js @@ -347,7 +347,7 @@ $.extend(fixmystreet.set_up, { $('.js-form-name').addClass('required'); } ); - $('#facebook_sign_in, #twitter_sign_in').click(function(e){ + $('#facebook_sign_in, #twitter_sign_in, #oidc_sign_in').click(function(e){ $('#username, #form_username_register, #form_username_sign_in').removeClass('required'); }); |