improved error handling and tests added

This commit is contained in:
milnerTill
2026-04-20 13:38:17 -03:00
parent f3c1fad50a
commit 844c79ae72
7 changed files with 84 additions and 29 deletions

View File

@@ -55,7 +55,7 @@ class OidcController extends Controller
}
try {
$this->_catchCustomErrorAndFail($request);
$this->throwIfAuthorizationError($request);
$this->oidcService->processAuthorizeResponse($request->query('code'));
} catch (OidcException $oidcException) {
$this->showErrorNotification($oidcException->getMessage());
@@ -74,19 +74,22 @@ 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);
/**
*
* @throws OidcException
*/
private function throwIfAuthorizationError(Request $request): void
{
$errorCode = $request->query('error');
if (!$errorCode) {
return;
}
$errorDescription = $request->query('error_description');
if ($errorDescription) {
throw new OidcException($errorDescription);
}
throw new OidcException(trans('errors.oidc_fail_authed', ['system' => config('oidc.name')]));
}
}

View File

@@ -26,7 +26,6 @@ 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.',

View File

@@ -26,7 +26,6 @@ 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 .',

View File

@@ -26,7 +26,6 @@ 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 .',

2
package-lock.json generated
View File

@@ -1,5 +1,5 @@
{
"name": "openId-HS256-support",
"name": "bookstack",
"lockfileVersion": 3,
"requires": true,
"packages": {

View File

@@ -2,6 +2,7 @@
namespace Tests\Helpers;
use BookStack\Access\Oidc\OidcProviderSettings;
use phpseclib3\Crypt\RSA;
/**
@@ -119,6 +120,41 @@ q/1PY4iJviGKddtmfClH3v4=
-----END PRIVATE KEY-----';
}
public static function defaultSecret(): string
{
return 'test-client-secret-for-hs256';
}
/**
* Build a minimal OidcProviderSettings for use in token validation tests.
*/
public static function defaultProviderSettings(array $overrides = []): OidcProviderSettings
{
return new OidcProviderSettings(array_merge([
'issuer' => static::defaultIssuer(),
'clientId' => static::defaultClientId(),
'clientSecret' => static::defaultSecret(),
], $overrides));
}
/**
* Build an HS256-signed ID token using the given secret.
*/
public static function hs256IdToken(string $secret, array $payloadOverrides = []): string
{
$payload = array_merge(static::defaultPayload(), $payloadOverrides);
$header = ['alg' => 'HS256', 'typ' => 'JWT'];
$top = implode('.', [
static::base64UrlEncode(json_encode($header)),
static::base64UrlEncode(json_encode($payload)),
]);
$signature = hash_hmac('sha256', $top, $secret, true);
return $top . '.' . static::base64UrlEncode($signature);
}
public static function publicJwkKeyArray(): array
{
return [

View File

@@ -15,7 +15,7 @@ class OidcIdTokenTest extends TestCase
OidcJwtHelper::publicJwkKeyArray(),
]);
$this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123'));
$this->assertTrue($token->validate(OidcJwtHelper::defaultProviderSettings()));
}
public function test_get_claim_returns_value_if_existing()
@@ -56,7 +56,7 @@ class OidcIdTokenTest extends TestCase
$err = null;
try {
$token->validate('abc');
$token->validate(OidcJwtHelper::defaultProviderSettings());
} catch (\Exception $exception) {
$err = $exception;
}
@@ -71,7 +71,7 @@ class OidcIdTokenTest extends TestCase
$token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), []);
$this->expectException(OidcInvalidTokenException::class);
$this->expectExceptionMessage('Token signature could not be validated using the provided keys');
$token->validate('abc');
$token->validate(OidcJwtHelper::defaultProviderSettings());
}
public function test_error_thrown_if_token_signature_not_validated_from_non_matching_key()
@@ -83,7 +83,7 @@ class OidcIdTokenTest extends TestCase
]);
$this->expectException(OidcInvalidTokenException::class);
$this->expectExceptionMessage('Token signature could not be validated using the provided keys');
$token->validate('abc');
$token->validate(OidcJwtHelper::defaultProviderSettings());
}
public function test_error_thrown_if_invalid_key_provided()
@@ -91,15 +91,34 @@ class OidcIdTokenTest extends TestCase
$token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), ['url://example.com']);
$this->expectException(OidcInvalidTokenException::class);
$this->expectExceptionMessage('Unexpected type of key value provided');
$token->validate('abc');
$token->validate(OidcJwtHelper::defaultProviderSettings());
}
public function test_error_thrown_if_token_algorithm_is_not_rs256()
public function test_error_thrown_if_token_algorithm_is_not_supported()
{
$token = new OidcIdToken(OidcJwtHelper::idToken([], ['alg' => 'HS256']), OidcJwtHelper::defaultIssuer(), []);
$token = new OidcIdToken(OidcJwtHelper::idToken([], ['alg' => 'ES256']), OidcJwtHelper::defaultIssuer(), []);
$this->expectException(OidcInvalidTokenException::class);
$this->expectExceptionMessage('Only RS256 signature validation is supported. Token reports using HS256');
$token->validate('abc');
$this->expectExceptionMessage('Only HS256, RS256 signatures validation are supported. Token reports using ES256');
$token->validate(OidcJwtHelper::defaultProviderSettings());
}
public function test_hs256_token_passes_validation_with_correct_secret()
{
$secret = OidcJwtHelper::defaultSecret();
$token = new OidcIdToken(OidcJwtHelper::hs256IdToken($secret), OidcJwtHelper::defaultIssuer(), []);
$settings = OidcJwtHelper::defaultProviderSettings(['clientSecret' => $secret]);
$this->assertTrue($token->validate($settings));
}
public function test_hs256_token_fails_validation_with_wrong_secret()
{
$token = new OidcIdToken(OidcJwtHelper::hs256IdToken('correct-secret'), OidcJwtHelper::defaultIssuer(), []);
$settings = OidcJwtHelper::defaultProviderSettings(['clientSecret' => 'wrong-secret']);
$this->expectException(OidcInvalidTokenException::class);
$this->expectExceptionMessage('Token signature could not be validated using the provided secret');
$token->validate($settings);
}
public function test_token_claim_error_cases()
@@ -141,7 +160,7 @@ class OidcIdTokenTest extends TestCase
$err = null;
try {
$token->validate('xxyyzz.aaa.bbccdd.123');
$token->validate(OidcJwtHelper::defaultProviderSettings());
} catch (\Exception $exception) {
$err = $exception;
}
@@ -160,7 +179,7 @@ class OidcIdTokenTest extends TestCase
$testFilePath,
]);
$this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123'));
$this->assertTrue($token->validate(OidcJwtHelper::defaultProviderSettings()));
unlink($testFilePath);
}
}