From f3c1fad50a54395bb61f828841399fa98989684b Mon Sep 17 00:00:00 2001 From: milnerTill Date: Mon, 20 Apr 2026 11:35:35 -0300 Subject: [PATCH] add support to HS256 algorithm --- app/Access/Controllers/OidcController.php | 17 +++++ ...idcHttpBasicWithClientIdOptionProvider.php | 27 +++++++ app/Access/Oidc/OidcIdToken.php | 6 +- app/Access/Oidc/OidcJwtWithClaims.php | 73 +++++++++++-------- app/Access/Oidc/OidcProviderSettings.php | 2 +- app/Access/Oidc/OidcService.php | 5 +- lang/en/errors.php | 1 + lang/es/errors.php | 1 + lang/es_AR/errors.php | 1 + package-lock.json | 2 +- 10 files changed, 97 insertions(+), 38 deletions(-) create mode 100644 app/Access/Oidc/OidcHttpBasicWithClientIdOptionProvider.php diff --git a/app/Access/Controllers/OidcController.php b/app/Access/Controllers/OidcController.php index 654ed6928..203130602 100644 --- a/app/Access/Controllers/OidcController.php +++ b/app/Access/Controllers/OidcController.php @@ -55,6 +55,7 @@ class OidcController extends Controller } try { + $this->_catchCustomErrorAndFail($request); $this->oidcService->processAuthorizeResponse($request->query('code')); } catch (OidcException $oidcException) { $this->showErrorNotification($oidcException->getMessage()); @@ -72,4 +73,20 @@ class OidcController extends Controller { return redirect($this->oidcService->logout()); } + + private function _catchCustomErrorAndFail(Request $request):void { + $_errorCode = $request->query('error'); + if ($_errorCode) { + if ($_errorCode === 'need_auth') { + if ( null === ($errorMsg = $request->query('error_description')) ){ + $customError = trans('errors.oidc_need_auth', ['system' => config('oidc.name')]); + }else { + $customError = $errorMsg; + } + } + throw new OidcException($customError); + } + } + + } diff --git a/app/Access/Oidc/OidcHttpBasicWithClientIdOptionProvider.php b/app/Access/Oidc/OidcHttpBasicWithClientIdOptionProvider.php new file mode 100644 index 000000000..b5aca63f3 --- /dev/null +++ b/app/Access/Oidc/OidcHttpBasicWithClientIdOptionProvider.php @@ -0,0 +1,27 @@ +validateTokenClaims($clientId); + parent::validateCommonTokenDetails($settings); + $this->validateTokenClaims($settings->clientId); return true; } diff --git a/app/Access/Oidc/OidcJwtWithClaims.php b/app/Access/Oidc/OidcJwtWithClaims.php index 9d7eeead1..b3da04ee5 100644 --- a/app/Access/Oidc/OidcJwtWithClaims.php +++ b/app/Access/Oidc/OidcJwtWithClaims.php @@ -9,7 +9,9 @@ class OidcJwtWithClaims implements ProvidesClaims protected string $signature; protected string $issuer; protected array $tokenParts = []; - + protected array $acceptedSignatures = [self::hs256Signature, self::rs256Signature]; + private const hs256Signature = 'HS256' + , rs256Signature = 'RS256'; /** * @var array[]|string[] */ @@ -59,11 +61,11 @@ class OidcJwtWithClaims implements ProvidesClaims * * @throws OidcInvalidTokenException */ - public function validateCommonTokenDetails(string $clientId): bool + public function validateCommonTokenDetails(OidcProviderSettings $settings): bool { $this->validateTokenStructure(); - $this->validateTokenSignature(); - $this->validateCommonClaims($clientId); + $this->validateTokenSignature($settings); + $this->validateCommonClaims($settings->clientId); return true; } @@ -102,12 +104,12 @@ class OidcJwtWithClaims implements ProvidesClaims protected function validateTokenStructure(): void { foreach (['header', 'payload'] as $prop) { - if (empty($this->$prop)) { + if (empty($this->$prop) || !is_array($this->$prop)) { throw new OidcInvalidTokenException("Could not parse out a valid {$prop} within the provided token"); } } - if (empty($this->signature)) { + if (empty($this->signature) || !is_string($this->signature)) { throw new OidcInvalidTokenException('Could not parse out a valid signature within the provided token'); } } @@ -117,31 +119,42 @@ class OidcJwtWithClaims implements ProvidesClaims * * @throws OidcInvalidTokenException */ - protected function validateTokenSignature(): void - { - if ($this->header['alg'] !== 'RS256') { - throw new OidcInvalidTokenException("Only RS256 signature validation is supported. Token reports using {$this->header['alg']}"); + protected function validateTokenSignature(OidcProviderSettings $settings): void { + $validSignatures = implode(', ',$this->acceptedSignatures); + switch ($this->header['alg']) { + case self::rs256Signature: + $parsedKeys = array_map(function ($key) { + try { + return new OidcJwtSigningKey($key); + } catch (OidcInvalidKeyException $e) { + throw new OidcInvalidTokenException('Failed to read signing key with error: ' . $e->getMessage()); + } + }, $this->keys); + + $parsedKeys = array_filter($parsedKeys); + + $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1]; + /** @var OidcJwtSigningKey $parsedKey */ + foreach ($parsedKeys as $parsedKey) { + if ($parsedKey->verify($contentToSign, $this->signature)) { + return; + } + } + + throw new OidcInvalidTokenException('Token signature could not be validated using the provided keys'); + case self::hs256Signature: + $secret = $settings->clientSecret; + $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1]; + $expectedSignature = hash_hmac('sha256', $contentToSign, $secret, true); + + if (hash_equals($expectedSignature, $this->signature)) { + return; + } + + throw new OidcInvalidTokenException('Token signature could not be validated using the provided secret'); + default: + throw new OidcInvalidTokenException("Only $validSignatures signatures validation are supported. Token reports using {$this->header['alg']}"); } - - $parsedKeys = array_map(function ($key) { - try { - return new OidcJwtSigningKey($key); - } catch (OidcInvalidKeyException $e) { - throw new OidcInvalidTokenException('Failed to read signing key with error: ' . $e->getMessage()); - } - }, $this->keys); - - $parsedKeys = array_filter($parsedKeys); - - $contentToSign = $this->tokenParts[0] . '.' . $this->tokenParts[1]; - /** @var OidcJwtSigningKey $parsedKey */ - foreach ($parsedKeys as $parsedKey) { - if ($parsedKey->verify($contentToSign, $this->signature)) { - return; - } - } - - throw new OidcInvalidTokenException('Token signature could not be validated using the provided keys'); } /** diff --git a/app/Access/Oidc/OidcProviderSettings.php b/app/Access/Oidc/OidcProviderSettings.php index 71c3b5734..573d2beef 100644 --- a/app/Access/Oidc/OidcProviderSettings.php +++ b/app/Access/Oidc/OidcProviderSettings.php @@ -74,7 +74,7 @@ class OidcProviderSettings { $this->validateInitial(); - $required = ['keys', 'tokenEndpoint', 'authorizationEndpoint']; + $required = ['tokenEndpoint', 'authorizationEndpoint']; foreach ($required as $prop) { if (empty($this->$prop)) { throw new InvalidArgumentException("Missing required configuration \"{$prop}\" value"); diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php index a84bd3205..60eb0a4d1 100644 --- a/app/Access/Oidc/OidcService.php +++ b/app/Access/Oidc/OidcService.php @@ -14,7 +14,6 @@ use BookStack\Theming\ThemeEvents; use BookStack\Uploads\UserAvatars; use BookStack\Users\Models\User; use Illuminate\Support\Facades\Cache; -use League\OAuth2\Client\OptionProvider\HttpBasicAuthOptionProvider; use League\OAuth2\Client\Provider\Exception\IdentityProviderException; /** @@ -140,7 +139,7 @@ class OidcService 'redirectUri' => url('/oidc/callback'), ], [ 'httpClient' => $this->http->buildClient(5), - 'optionProvider' => new HttpBasicAuthOptionProvider(), + 'optionProvider' => new OidcHttpBasicWithClientIdOptionProvider(), ]); foreach ($this->getAdditionalScopes() as $scope) { @@ -199,7 +198,7 @@ class OidcService } try { - $idToken->validate($settings->clientId); + $idToken->validate($settings); } catch (OidcInvalidTokenException $exception) { throw new OidcException("ID token validation failed with error: {$exception->getMessage()}"); } diff --git a/lang/en/errors.php b/lang/en/errors.php index 20537d59f..89225c9fe 100644 --- a/lang/en/errors.php +++ b/lang/en/errors.php @@ -26,6 +26,7 @@ return [ 'oidc_already_logged_in' => 'Already logged in', 'oidc_no_email_address' => 'Could not find an email address, for this user, in the data provided by the external authentication system', 'oidc_fail_authed' => 'Login using :system failed, system did not provide successful authorization', + 'oidc_need_auth' => 'You need to be authenticated to perform this action.', 'social_no_action_defined' => 'No action defined', 'social_login_bad_response' => "Error received during :socialAccount login: \n:error", 'social_account_in_use' => 'This :socialAccount account is already in use, Try logging in via the :socialAccount option.', diff --git a/lang/es/errors.php b/lang/es/errors.php index 25f82a5d8..fd16c3163 100644 --- a/lang/es/errors.php +++ b/lang/es/errors.php @@ -26,6 +26,7 @@ return [ 'oidc_already_logged_in' => 'Ya tenías la sesión iniciada', 'oidc_no_email_address' => 'No se pudo encontrar una dirección de correo electrónico, para este usuario, en los datos proporcionados por el sistema de autenticación externo', 'oidc_fail_authed' => 'El inicio de sesión con :system falló, el sistema no proporcionó una autorización correcta', + 'oidc_need_auth' => 'Necesita estar autenticado para realizar esta acción.', 'social_no_action_defined' => 'Acción no definida', 'social_login_bad_response' => "Se ha recibido un error durante el acceso con :socialAccount error: \n:error", 'social_account_in_use' => 'la cuenta :socialAccount ya se encuentra en uso, intente acceder a través de la opción :socialAccount .', diff --git a/lang/es_AR/errors.php b/lang/es_AR/errors.php index 6f6b25bcf..c24fde88c 100644 --- a/lang/es_AR/errors.php +++ b/lang/es_AR/errors.php @@ -26,6 +26,7 @@ return [ 'oidc_already_logged_in' => 'Ya está conectado', 'oidc_no_email_address' => 'No se pudo encontrar una dirección de correo electrónico para este usuario en los datos proporcionados por el sistema de autenticación externo', 'oidc_fail_authed' => 'El inicio de sesión con :system falló, el sistema no proporcionó una autorización correcta', + 'oidc_need_auth' => 'Necesita estar autenticado para realizar esta acción.', 'social_no_action_defined' => 'Acción no definida', 'social_login_bad_response' => "SE recibió un Error durante el acceso con :socialAccount : \n:error", 'social_account_in_use' => 'la cuenta :socialAccount ya se encuentra en uso, intente loguearse a través de la opcón :socialAccount .', diff --git a/package-lock.json b/package-lock.json index d239426ec..97bb4b7f2 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,5 +1,5 @@ { - "name": "bookstack", + "name": "openId-HS256-support", "lockfileVersion": 3, "requires": true, "packages": {