Add test for OIDC logout redirection

6 files changed, 44 insertions, 9 deletions

diff --git a/perllib/FixMyStreet/App/Controller/Auth.pm b/perllib/FixMyStreet/App/Controller/Auth.pm
index 6b2b29044..964d8f19a 100644
--- a/perllib/FixMyStreet/App/Controller/Auth.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth.pm
@@ -219,7 +219,7 @@ sub get_token : Private {
 sub set_oauth_token_data : Private {
     my ( $self, $c, $token_data ) = @_;
 
-    foreach (qw/facebook_id twitter_id oidc_id extra/) {
+    foreach (qw/facebook_id twitter_id oidc_id extra logout_redirect_uri/) {
         $token_data->{$_} = $c->session->{oauth}{$_} if $c->session->{oauth}{$_};
     }
 }
@@ -291,6 +291,12 @@ sub process_login : Private {
     $user->update_or_insert;
     $c->authenticate( { $type => $data->{$type}, $ver => 1 }, 'no_password' );
 
+    if ($data->{logout_redirect_uri}) {
+        $c->session->{oauth} ||= ();
+        $c->session->{oauth}{logout_redirect_uri} = $data->{logout_redirect_uri};
+    }
+
+
     # send the user to their page
     $c->detach( 'redirect_on_signin', [ $data->{r}, $data->{p} ] );
 }
diff --git a/perllib/FixMyStreet/App/Controller/Auth/Social.pm b/perllib/FixMyStreet/App/Controller/Auth/Social.pm
index 4bd7dcd4d..56bae96d2 100644
--- a/perllib/FixMyStreet/App/Controller/Auth/Social.pm
+++ b/perllib/FixMyStreet/App/Controller/Auth/Social.pm
@@ -198,6 +198,15 @@ sub oidc_sign_in : Private {
     $oauth{detach_to} = $c->stash->{detach_to};
     $oauth{detach_args} = $c->stash->{detach_args};
     $oauth{nonce} = $nonce;
+
+    # 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} ) {
+        $redirect_uri .= "?post_logout_redirect_uri=";
+        $redirect_uri .= URI::Escape::uri_escape( $c->uri_for('/auth/sign_out') );
+        $oauth{logout_redirect_uri} = $redirect_uri;
+    }
+
     $c->session->{oauth} = \%oauth;
     $c->res->redirect($url);
 }
@@ -271,14 +280,6 @@ sub oidc_callback: Path('/auth/OIDC') : Args(0) {
     # which is passed to Open311 with reports made by this user.
     my $extra = $c->cobrand->call_hook(oidc_user_extra => $id_token);
 
-    # 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} ) {
-        $redirect_uri .= "?post_logout_redirect_uri=";
-        $redirect_uri .= URI::Escape::uri_escape( $c->uri_for('/auth/sign_out') );
-        $c->session->{oauth}{logout_redirect_uri} = $redirect_uri;
-    }
-
     $c->forward('oauth_success', [ 'oidc', $uid, $name, $email, $extra ]);
 }
 
diff --git a/perllib/FixMyStreet/App/Controller/Report/New.pm b/perllib/FixMyStreet/App/Controller/Report/New.pm
index c6ecea79e..53459baee 100644
--- a/perllib/FixMyStreet/App/Controller/Report/New.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/New.pm
@@ -1330,6 +1330,12 @@ sub process_confirmation : Private {
         }) if $data->{extra};
 
         $problem->user->update;
+
+        # Make sure OIDC logout redirection happens, if applicable
+        if ($data->{logout_redirect_uri}) {
+            $c->session->{oauth} ||= ();
+            $c->session->{oauth}{logout_redirect_uri} = $data->{logout_redirect_uri};
+        }
     }
     if ($problem->user->email_verified) {
         $c->authenticate( { email => $problem->user->email, email_verified => 1 }, 'no_password' );
diff --git a/perllib/FixMyStreet/App/Controller/Report/Update.pm b/perllib/FixMyStreet/App/Controller/Report/Update.pm
index 0f3f8c098..d67ead82d 100644
--- a/perllib/FixMyStreet/App/Controller/Report/Update.pm
+++ b/perllib/FixMyStreet/App/Controller/Report/Update.pm
@@ -586,6 +586,11 @@ sub process_confirmation : Private {
         }) if $data->{extra};
         $comment->user->password( $data->{password}, 1 ) if $data->{password};
         $comment->user->update;
+        # Make sure OIDC logout redirection happens, if applicable
+        if ($data->{logout_redirect_uri}) {
+            $c->session->{oauth} ||= ();
+            $c->session->{oauth}{logout_redirect_uri} = $data->{logout_redirect_uri};
+        }
     }
 
     if ($comment->user->email_verified) {
diff --git a/t/Mock/OpenIDConnect.pm b/t/Mock/OpenIDConnect.pm
index 77ed88817..079c354fc 100644
--- a/t/Mock/OpenIDConnect.pm
+++ b/t/Mock/OpenIDConnect.pm
@@ -27,6 +27,11 @@ sub dispatch_request {
         return [ 200, [ 'Content-Type' => 'text/html' ], [ 'OpenID Connect login page' ] ];
     },
 
+    sub (GET + /oauth2/v2.0/logout + ?*) {
+        my ($self) = @_;
+        return [ 200, [ 'Content-Type' => 'text/html' ], [ 'OpenID Connect logout page' ] ];
+    },
+
     sub (POST + /oauth2/v2.0/token + ?*) {
         my ($self) = @_;
         my $header = {
diff --git a/t/app/controller/auth_social.t b/t/app/controller/auth_social.t
index 3a0539452..62ab0b60f 100644
--- a/t/app/controller/auth_social.t
+++ b/t/app/controller/auth_social.t
@@ -63,6 +63,7 @@ for my $test (
                     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 => 'MyWestminster'
                 }
             }
@@ -76,6 +77,7 @@ for my $test (
     error_callback => '/auth/OIDC?error=ERROR',
     success_callback => '/auth/OIDC?code=response-code&state=login',
     redirect_pattern => qr{oidc\.example\.org/oauth2/v2\.0/authorize},
+    logout_redirect_pattern => qr{oidc\.example\.org/oauth2/v2\.0/logout},
     user_extras => [
         [westminster_account_id => "1c304134-ef12-c128-9212-123908123901"],
     ],
@@ -227,6 +229,16 @@ for my $state ( 'refused', 'no email', 'existing UID', 'okay' ) {
                     }
                 }
             }
+
+            $mech->get('/auth/sign_out');
+            if ($test->{type} eq 'oidc' && $state ne 'refused' && $state ne 'no email') {
+                # XXX the 'no email' situation is skipped because of some confusion
+                # with the hosts/sessions that I've not been able to get to the bottom of.
+                # The code does behave as expected when testing manually, however.
+                is $mech->res->previous->code, 302, "$test->{type} sign out redirected";
+                like $mech->res->previous->header('Location'), $test->{logout_redirect_pattern}, "$test->{type} sign out redirect to oauth logout URL";
+            }
+            $mech->not_logged_in_ok;
         }
     }
 }