OIDC RP Logout: Fixed issues during testing

- Disabled by default due to strict rejection by auth systems.
- Fixed issue when autoloading logout URL, but not provided in
  autodiscovery response.
- Added proper handling for if the logout URL contains a query string
  already.
- Added extra tests to cover.
- Forced config endpoint to be used, if set as a string, instead of
  autodiscovery endpoint.

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index 3f9cd41b4b36dbefbe75f786f1e7175332f9303c..f1e5b25af1490b9fa2c7c5eed7568d3e5e6ab23d 100644 (file)
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -84,7 +84,7 @@ class OidcService
             'redirectUri'           => url('/oidc/callback'),
             'authorizationEndpoint' => $config['authorization_endpoint'],
             'tokenEndpoint'         => $config['token_endpoint'],
             'redirectUri'           => url('/oidc/callback'),
             'authorizationEndpoint' => $config['authorization_endpoint'],
             'tokenEndpoint'         => $config['token_endpoint'],

-            'endSessionEndpoint'    => $config['end_session_endpoint'],
+            'endSessionEndpoint'    => is_string($config['end_session_endpoint']) ? $config['end_session_endpoint'] : null,

         ]);
 
         // Use keys if configured
         ]);
 
         // Use keys if configured


@@ -102,8 +102,11 @@ class OidcService
         }
 
         // Prevent use of RP-initiated logout if specifically disabled
         }
 
         // Prevent use of RP-initiated logout if specifically disabled

+        // Or force use of a URL if specifically set.

         if ($config['end_session_endpoint'] === false) {
             $settings->endSessionEndpoint = null;
         if ($config['end_session_endpoint'] === false) {
             $settings->endSessionEndpoint = null;

+        } else if (is_string($config['end_session_endpoint'])) {
+            $settings->endSessionEndpoint = $config['end_session_endpoint'];

         }
 
         $settings->validate();
         }
 
         $settings->validate();


@@ -314,6 +317,8 @@ class OidcService
             'post_logout_redirect_uri' => $defaultLogoutUrl,
         ];
 
             'post_logout_redirect_uri' => $defaultLogoutUrl,
         ];
 

-        return $oidcSettings->endSessionEndpoint . '?' . http_build_query($endpointParams);
+        $joiner = str_contains($oidcSettings->endSessionEndpoint, '?') ? '&' : '?';
+
+        return $oidcSettings->endSessionEndpoint . $joiner . http_build_query($endpointParams);

     }
 }
     }
 }

diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index ed9302d1032cd7dd0d62405eeb1e01a7ccb490b2..7f8f40d419090af498907ee4bb2b8c59dee6e926 100644 (file)
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -37,9 +37,10 @@ return [
     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null),
 
     // OIDC RP-Initiated Logout endpoint URL.
     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null),
 
     // OIDC RP-Initiated Logout endpoint URL.

-    // A null value gets the URL from discovery, if active.

     // A false value force-disables RP-Initiated Logout.
     // A false value force-disables RP-Initiated Logout.

-    'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null),
+    // A true value gets the URL from discovery, if active.
+    // A string value is used as the URL.
+    'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', false),

 
     // Add extra scopes, upon those required, to the OIDC authentication request
     // Multiple values can be provided comma seperated.
 
     // Add extra scopes, upon those required, to the OIDC authentication request
     // Multiple values can be provided comma seperated.

diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index d10582d8c7da8951fb5b3683fbcf2376bf56d4c7..dbf26f1bd302b39009cf776c5673afaa6812379f 100644 (file)
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -44,7 +44,7 @@ class OidcTest extends TestCase
             'oidc.groups_claim'           => 'group',
             'oidc.remove_from_groups'     => false,
             'oidc.external_id_claim'      => 'sub',
             'oidc.groups_claim'           => 'group',
             'oidc.remove_from_groups'     => false,
             'oidc.external_id_claim'      => 'sub',

-            'oidc.end_session_endpoint'   => null,
+            'oidc.end_session_endpoint'   => false,

         ]);
     }
 
         ]);
     }
 


@@ -486,8 +486,9 @@ class OidcTest extends TestCase
         $resp = $this->get('/');
         $this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button');
     }
         $resp = $this->get('/');
         $this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button');
     }

-    public function test_logout_with_autodiscovery()
+    public function test_logout_with_autodiscovery_with_oidc_logout_enabled()

     {
     {

+        config()->set(['oidc.end_session_endpoint' => true]);

         $this->withAutodiscovery();
 
         $transactions = $this->mockHttpClient([
         $this->withAutodiscovery();
 
         $transactions = $this->mockHttpClient([


@@ -499,9 +500,10 @@ class OidcTest extends TestCase
         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/')));
 
         $this->assertEquals(2, $transactions->requestCount());
         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/')));
 
         $this->assertEquals(2, $transactions->requestCount());

+        $this->assertFalse(auth()->check());

     }
 
     }
 

-    public function test_logout_with_autodiscovery_but_oidc_logout_disabled()
+    public function test_logout_with_autodiscovery_with_oidc_logout_disabled()

     {
         $this->withAutodiscovery();
         config()->set(['oidc.end_session_endpoint' => false]);
     {
         $this->withAutodiscovery();
         config()->set(['oidc.end_session_endpoint' => false]);


@@ -513,6 +515,7 @@ class OidcTest extends TestCase
 
         $resp = $this->asEditor()->post('/oidc/logout');
         $resp->assertRedirect('/');
 
         $resp = $this->asEditor()->post('/oidc/logout');
         $resp->assertRedirect('/');

+        $this->assertFalse(auth()->check());

     }
 
     public function test_logout_without_autodiscovery_but_with_endpoint_configured()
     }
 
     public function test_logout_without_autodiscovery_but_with_endpoint_configured()


@@ -521,6 +524,16 @@ class OidcTest extends TestCase
 
         $resp = $this->asEditor()->post('/oidc/logout');
         $resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/')));
 
         $resp = $this->asEditor()->post('/oidc/logout');
         $resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/')));

+        $this->assertFalse(auth()->check());
+    }
+
+    public function test_logout_without_autodiscovery_with_configured_endpoint_adds_to_query_if_existing()
+    {
+        config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout?a=b']);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('https://example.com/logout?a=b&post_logout_redirect_uri=' . urlencode(url('/')));
+        $this->assertFalse(auth()->check());

     }
 
     public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login()
     }
 
     public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login()


@@ -530,6 +543,7 @@ class OidcTest extends TestCase
             'auth.auto_initiate' => true,
             'services.google.client_id' => false,
             'services.github.client_id' => false,
             'auth.auto_initiate' => true,
             'services.google.client_id' => false,
             'services.github.client_id' => false,

+            'oidc.end_session_endpoint' => true,

         ]);
 
         $this->mockHttpClient([
         ]);
 
         $this->mockHttpClient([


@@ -541,6 +555,39 @@ class OidcTest extends TestCase
 
         $redirectUrl = url('/login?prevent_auto_init=true');
         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl));
 
         $redirectUrl = url('/login?prevent_auto_init=true');
         $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl));

+        $this->assertFalse(auth()->check());
+    }
+
+    public function test_logout_endpoint_url_overrides_autodiscovery_endpoint()
+    {
+        config()->set(['oidc.end_session_endpoint' => 'https://a.example.com']);
+        $this->withAutodiscovery();
+
+        $transactions = $this->mockHttpClient([
+            $this->getAutoDiscoveryResponse(),
+            $this->getJwksResponse(),
+        ]);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('https://a.example.com?post_logout_redirect_uri=' . urlencode(url('/')));
+
+        $this->assertEquals(2, $transactions->requestCount());
+        $this->assertFalse(auth()->check());
+    }
+
+    public function test_logout_with_autodiscovery_does_not_use_rp_logout_if_no_url_via_autodiscovery()
+    {
+        config()->set(['oidc.end_session_endpoint' => true]);
+        $this->withAutodiscovery();
+
+        $this->mockHttpClient([
+            $this->getAutoDiscoveryResponse(['end_session_endpoint' => null]),
+            $this->getJwksResponse(),
+        ]);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('/');
+        $this->assertFalse(auth()->check());

     }
 
     public function test_logout_redirect_contains_id_token_hint_if_existing()
     }
 
     public function test_logout_redirect_contains_id_token_hint_if_existing()