OIDC RP Logout: Added autodiscovery support and test cases

diff --git a/app/Access/Controllers/LoginController.php b/app/Access/Controllers/LoginController.php
index 9047366566d8082de71249e602c7cdaebbeecd91..ce872ba88dcb7bc457508ac2dcb59f86b67d7d01 100644 (file)
--- a/app/Access/Controllers/LoginController.php
+++ b/app/Access/Controllers/LoginController.php
@@ -10,27 +10,19 @@ use BookStack\Facades\Activity;
 use BookStack\Http\Controller;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;
 use BookStack\Http\Controller;
 use Illuminate\Http\RedirectResponse;
 use Illuminate\Http\Request;

-use Illuminate\Support\Facades\Auth;

 use Illuminate\Validation\ValidationException;
 
 class LoginController extends Controller
 {
     use ThrottlesLogins;
 
 use Illuminate\Validation\ValidationException;
 
 class LoginController extends Controller
 {
     use ThrottlesLogins;
 

-    protected SocialDriverManager $socialDriverManager;
-    protected LoginService $loginService;
-
-    /**
-     * Create a new controller instance.
-     */
-    public function __construct(SocialDriverManager $driverManager, LoginService $loginService)
-    {
+    public function __construct(
+        protected SocialDriverManager $socialDriverManager,
+        protected LoginService $loginService,
+    ) {

         $this->middleware('guest', ['only' => ['getLogin', 'login']]);
         $this->middleware('guard:standard,ldap', ['only' => ['login']]);
         $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]);
         $this->middleware('guest', ['only' => ['getLogin', 'login']]);
         $this->middleware('guard:standard,ldap', ['only' => ['login']]);
         $this->middleware('guard:standard,ldap,oidc', ['only' => ['logout']]);

-
-        $this->socialDriverManager = $driverManager;
-        $this->loginService = $loginService;

     }
 
     /**
     }
 
     /**


@@ -52,7 +44,7 @@ class LoginController extends Controller
         // Store the previous location for redirect after login
         $this->updateIntendedFromPrevious();
 
         // Store the previous location for redirect after login
         $this->updateIntendedFromPrevious();
 

-        if (!$preventInitiation && $this->shouldAutoInitiate()) {
+        if (!$preventInitiation && $this->loginService->shouldAutoInitiate()) {

             return view('auth.login-initiate', [
                 'authMethod'    => $authMethod,
             ]);
             return view('auth.login-initiate', [
                 'authMethod'    => $authMethod,
             ]);


@@ -194,7 +186,7 @@ class LoginController extends Controller
     {
         // Store the previous location for redirect after login
         $previous = url()->previous('');
     {
         // Store the previous location for redirect after login
         $previous = url()->previous('');

-        $isPreviousFromInstance = (strpos($previous, url('/')) === 0);
+        $isPreviousFromInstance = str_starts_with($previous, url('/'));

         if (!$previous || !setting('app-public') || !$isPreviousFromInstance) {
             return;
         }
         if (!$previous || !setting('app-public') || !$isPreviousFromInstance) {
             return;
         }


@@ -205,7 +197,7 @@ class LoginController extends Controller
         ];
 
         foreach ($ignorePrefixList as $ignorePrefix) {
         ];
 
         foreach ($ignorePrefixList as $ignorePrefix) {

-            if (strpos($previous, url($ignorePrefix)) === 0) {
+            if (str_starts_with($previous, url($ignorePrefix))) {

                 return;
             }
         }
                 return;
             }
         }

diff --git a/app/Access/LoginService.php b/app/Access/LoginService.php
index f0f6ad4d3a0500b3c4f68266006529a1e6179fc5..cc48e0f9babed3777b713f6fdf551f70e8603090 100644 (file)
--- a/app/Access/LoginService.php
+++ b/app/Access/LoginService.php
@@ -176,14 +176,18 @@ class LoginService
     }
 
     /**
     }
 
     /**

-     * Check if login auto-initiate should be valid based upon authentication config.
+     * Check if login auto-initiate should be active based upon authentication config.

      */
      */

-    protected function shouldAutoInitiate(): bool
+    public function shouldAutoInitiate(): bool

     {
     {

+        $autoRedirect = config('auth.auto_initiate');
+        if (!$autoRedirect) {
+            return false;
+        }
+

         $socialDrivers = $this->socialDriverManager->getActive();
         $authMethod = config('auth.method');
         $socialDrivers = $this->socialDriverManager->getActive();
         $authMethod = config('auth.method');

-        $autoRedirect = config('auth.auto_initiate');

 
 

-        return $autoRedirect && count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']);
+        return count($socialDrivers) === 0 && in_array($authMethod, ['oidc', 'saml2']);

     }
 }
     }
 }

diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php
index fa3f579b18a8db4a667fb4597d71f00c85743db6..bea6a523e773987612aefae83aeb903ee0d1ae93 100644 (file)
--- a/app/Access/Oidc/OidcProviderSettings.php
+++ b/app/Access/Oidc/OidcProviderSettings.php
@@ -21,6 +21,7 @@ class OidcProviderSettings
     public ?string $redirectUri;
     public ?string $authorizationEndpoint;
     public ?string $tokenEndpoint;
     public ?string $redirectUri;
     public ?string $authorizationEndpoint;
     public ?string $tokenEndpoint;

+    public ?string $endSessionEndpoint;

 
     /**
      * @var string[]|array[]
 
     /**
      * @var string[]|array[]


@@ -132,6 +133,10 @@ class OidcProviderSettings
             $discoveredSettings['keys'] = $this->filterKeys($keys);
         }
 
             $discoveredSettings['keys'] = $this->filterKeys($keys);
         }
 

+        if (!empty($result['end_session_endpoint'])) {
+            $discoveredSettings['endSessionEndpoint'] = $result['end_session_endpoint'];
+        }
+

         return $discoveredSettings;
     }
 
         return $discoveredSettings;
     }
 

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index be869b1797faa2efe183a1de10402c76228fea5b..3f9cd41b4b36dbefbe75f786f1e7175332f9303c 100644 (file)
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -84,6 +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'],

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


@@ -100,6 +101,11 @@ class OidcService
             }
         }
 
             }
         }
 

+        // Prevent use of RP-initiated logout if specifically disabled
+        if ($config['end_session_endpoint'] === false) {
+            $settings->endSessionEndpoint = null;
+        }
+

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


@@ -291,20 +297,23 @@ class OidcService
      * Start the RP-initiated logout flow if active, otherwise start a standard logout flow.
      * Returns a post-app-logout redirect URL.
      * Reference: https://openid.net/specs/openid-connect-rpinitiated-1_0.html
      * Start the RP-initiated logout flow if active, otherwise start a standard logout flow.
      * Returns a post-app-logout redirect URL.
      * Reference: https://openid.net/specs/openid-connect-rpinitiated-1_0.html

+     * @throws OidcException

      */
     public function logout(): string
     {
      */
     public function logout(): string
     {

-        $endSessionEndpoint = $this->config()["end_session_endpoint"];
-
-        // TODO - Add autodiscovery and false/null config value support.
-

         $oidcToken = session()->pull("oidc_id_token");
         $defaultLogoutUrl = url($this->loginService->logout());
         $oidcToken = session()->pull("oidc_id_token");
         $defaultLogoutUrl = url($this->loginService->logout());

+        $oidcSettings = $this->getProviderSettings();
+
+        if (!$oidcSettings->endSessionEndpoint) {
+            return $defaultLogoutUrl;
+        }
+

         $endpointParams = [
             'id_token_hint' => $oidcToken,
             'post_logout_redirect_uri' => $defaultLogoutUrl,
         ];
 
         $endpointParams = [
             'id_token_hint' => $oidcToken,
             'post_logout_redirect_uri' => $defaultLogoutUrl,
         ];
 

-        return $endSessionEndpoint . '?' . http_build_query($endpointParams);
+        return $oidcSettings->endSessionEndpoint . '?' . http_build_query($endpointParams);

     }
 }
     }
 }

diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index 5f61063f63377be49bf4b16eb259b89894f82e44..ed9302d1032cd7dd0d62405eeb1e01a7ccb490b2 100644 (file)
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -36,10 +36,9 @@ return [
     'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null),
     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null),
 
     'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null),
     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null),
 

-    // OIDC RP-Initiated Logout endpoint
+    // 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 null value gets the URL from discovery, if active.
     // A false value force-disables RP-Initiated Logout.

-    // A string value forces the given URL to be used.

     'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null),
 
     // Add extra scopes, upon those required, to the OIDC authentication request
     'end_session_endpoint' => env('OIDC_END_SESSION_ENDPOINT', null),
 
     // Add extra scopes, upon those required, to the OIDC authentication request

diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 204a3bb5f960243e99ff30cc3f1427759b6a9470..d10582d8c7da8951fb5b3683fbcf2376bf56d4c7 100644 (file)
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -44,6 +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,

         ]);
     }
 
         ]);
     }
 


@@ -478,6 +479,81 @@ class OidcTest extends TestCase
         $this->assertTrue($user->hasRole($roleA->id));
     }
 
         $this->assertTrue($user->hasRole($roleA->id));
     }
 

+    public function test_oidc_logout_form_active_when_oidc_active()
+    {
+        $this->runLogin();
+
+        $resp = $this->get('/');
+        $this->withHtml($resp)->assertElementExists('header form[action$="/oidc/logout"] button');
+    }
+    public function test_logout_with_autodiscovery()
+    {
+        $this->withAutodiscovery();
+
+        $transactions = $this->mockHttpClient([
+            $this->getAutoDiscoveryResponse(),
+            $this->getJwksResponse(),
+        ]);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode(url('/')));
+
+        $this->assertEquals(2, $transactions->requestCount());
+    }
+
+    public function test_logout_with_autodiscovery_but_oidc_logout_disabled()
+    {
+        $this->withAutodiscovery();
+        config()->set(['oidc.end_session_endpoint' => false]);
+
+        $this->mockHttpClient([
+            $this->getAutoDiscoveryResponse(),
+            $this->getJwksResponse(),
+        ]);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('/');
+    }
+
+    public function test_logout_without_autodiscovery_but_with_endpoint_configured()
+    {
+        config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout']);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $resp->assertRedirect('https://example.com/logout?post_logout_redirect_uri=' . urlencode(url('/')));
+    }
+
+    public function test_logout_with_autodiscovery_and_auto_initiate_returns_to_auto_prevented_login()
+    {
+        $this->withAutodiscovery();
+        config()->set([
+            'auth.auto_initiate' => true,
+            'services.google.client_id' => false,
+            'services.github.client_id' => false,
+        ]);
+
+        $this->mockHttpClient([
+            $this->getAutoDiscoveryResponse(),
+            $this->getJwksResponse(),
+        ]);
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+
+        $redirectUrl = url('/login?prevent_auto_init=true');
+        $resp->assertRedirect('https://auth.example.com/oidc/logout?post_logout_redirect_uri=' . urlencode($redirectUrl));
+    }
+
+    public function test_logout_redirect_contains_id_token_hint_if_existing()
+    {
+        config()->set(['oidc.end_session_endpoint' => 'https://example.com/logout']);
+
+        $this->runLogin();
+
+        $resp = $this->asEditor()->post('/oidc/logout');
+        $query = 'id_token_hint=' . urlencode(OidcJwtHelper::idToken()) .  '&post_logout_redirect_uri=' . urlencode(url('/'));
+        $resp->assertRedirect('https://example.com/logout?' . $query);
+    }
+

     public function test_oidc_id_token_pre_validate_theme_event_without_return()
     {
         $args = [];
     public function test_oidc_id_token_pre_validate_theme_event_without_return()
     {
         $args = [];


@@ -563,6 +639,7 @@ class OidcTest extends TestCase
             'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize',
             'jwks_uri'               => OidcJwtHelper::defaultIssuer() . '/oidc/keys',
             'issuer'                 => OidcJwtHelper::defaultIssuer(),
             'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize',
             'jwks_uri'               => OidcJwtHelper::defaultIssuer() . '/oidc/keys',
             'issuer'                 => OidcJwtHelper::defaultIssuer(),

+            'end_session_endpoint'   => OidcJwtHelper::defaultIssuer() . '/oidc/logout',

         ], $responseOverrides)));
     }
 
         ], $responseOverrides)));
     }