diff --git a/app/Auth/Access/GroupSyncService.php b/app/Auth/Access/GroupSyncService.php index ddd539b7773..db19b007ac3 100644 --- a/app/Auth/Access/GroupSyncService.php +++ b/app/Auth/Access/GroupSyncService.php @@ -73,4 +73,4 @@ public function syncUserWithFoundGroups(User $user, array $userGroups, bool $det $user->roles()->syncWithoutDetaching($groupsAsRoles); } } -} \ No newline at end of file +} diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php index ddd6ada97fe..e3a38537ac3 100644 --- a/app/Auth/Access/LdapService.php +++ b/app/Auth/Access/LdapService.php @@ -285,6 +285,7 @@ public function getUserGroups(string $userName): array } $userGroups = $this->groupFilter($user); + return $this->getGroupsRecursive($userGroups, []); } diff --git a/app/Auth/Access/Oidc/OidcAccessToken.php b/app/Auth/Access/Oidc/OidcAccessToken.php index 63853e08ab0..520966f6ae4 100644 --- a/app/Auth/Access/Oidc/OidcAccessToken.php +++ b/app/Auth/Access/Oidc/OidcAccessToken.php @@ -11,7 +11,8 @@ class OidcAccessToken extends AccessToken * Constructs an access token. * * @param array $options An array of options returned by the service provider - * in the access token request. The `access_token` option is required. + * in the access token request. The `access_token` option is required. + * * @throws InvalidArgumentException if `access_token` is not provided in `$options`. */ public function __construct(array $options = []) @@ -20,7 +21,6 @@ public function __construct(array $options = []) $this->validate($options); } - /** * Validate this access token response for OIDC. * As per https://openid.net/specs/openid-connect-basic-1_0.html#TokenOK. @@ -50,5 +50,4 @@ public function getIdToken(): string { return $this->getValues()['id_token']; } - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcIdToken.php b/app/Auth/Access/Oidc/OidcIdToken.php index de9c42ab277..c955c3b0915 100644 --- a/app/Auth/Access/Oidc/OidcIdToken.php +++ b/app/Auth/Access/Oidc/OidcIdToken.php @@ -60,6 +60,7 @@ protected function parseEncodedTokenPart(string $part): array { $json = $this->base64UrlDecode($part) ?: '{}'; $decoded = json_decode($json, true); + return is_array($decoded) ? $decoded : []; } @@ -74,6 +75,7 @@ protected function base64UrlDecode(string $encoded): string /** * Validate all possible parts of the id token. + * * @throws OidcInvalidTokenException */ public function validate(string $clientId): bool @@ -81,12 +83,14 @@ public function validate(string $clientId): bool $this->validateTokenStructure(); $this->validateTokenSignature(); $this->validateTokenClaims($clientId); + return true; } /** * Fetch a specific claim from this token. * Returns null if it is null or does not exist. + * * @return mixed|null */ public function getClaim(string $claim) @@ -104,7 +108,8 @@ public function getAllClaims(): array /** * Validate the structure of the given token and ensure we have the required pieces. - * As per https://datatracker.ietf.org/doc/html/rfc7519#section-7.2 + * As per https://datatracker.ietf.org/doc/html/rfc7519#section-7.2. + * * @throws OidcInvalidTokenException */ protected function validateTokenStructure(): void @@ -116,12 +121,13 @@ protected function validateTokenStructure(): void } if (empty($this->signature) || !is_string($this->signature)) { - throw new OidcInvalidTokenException("Could not parse out a valid signature within the provided token"); + throw new OidcInvalidTokenException('Could not parse out a valid signature within the provided token'); } } /** * Validate the signature of the given token and ensure it validates against the provided key. + * * @throws OidcInvalidTokenException */ protected function validateTokenSignature(): void @@ -130,7 +136,7 @@ protected function validateTokenSignature(): void throw new OidcInvalidTokenException("Only RS256 signature validation is supported. Token reports using {$this->header['alg']}"); } - $parsedKeys = array_map(function($key) { + $parsedKeys = array_map(function ($key) { try { return new OidcJwtSigningKey($key); } catch (OidcInvalidKeyException $e) { @@ -153,7 +159,8 @@ protected function validateTokenSignature(): void /** * Validate the claims of the token. - * As per https://openid.net/specs/openid-connect-basic-1_0.html#IDTokenValidation + * As per https://openid.net/specs/openid-connect-basic-1_0.html#IDTokenValidation. + * * @throws OidcInvalidTokenException */ protected function validateTokenClaims(string $clientId): void @@ -228,5 +235,4 @@ protected function validateTokenClaims(string $clientId): void throw new OidcInvalidTokenException('Missing token subject value'); } } - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcInvalidKeyException.php b/app/Auth/Access/Oidc/OidcInvalidKeyException.php index 17c32f41685..1b3310e06d9 100644 --- a/app/Auth/Access/Oidc/OidcInvalidKeyException.php +++ b/app/Auth/Access/Oidc/OidcInvalidKeyException.php @@ -4,5 +4,4 @@ class OidcInvalidKeyException extends \Exception { - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcInvalidTokenException.php b/app/Auth/Access/Oidc/OidcInvalidTokenException.php index 30ac79280b5..4f47eb08fc0 100644 --- a/app/Auth/Access/Oidc/OidcInvalidTokenException.php +++ b/app/Auth/Access/Oidc/OidcInvalidTokenException.php @@ -6,5 +6,4 @@ class OidcInvalidTokenException extends Exception { - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcIssuerDiscoveryException.php b/app/Auth/Access/Oidc/OidcIssuerDiscoveryException.php index 47c49c62596..e2f364e89db 100644 --- a/app/Auth/Access/Oidc/OidcIssuerDiscoveryException.php +++ b/app/Auth/Access/Oidc/OidcIssuerDiscoveryException.php @@ -4,5 +4,4 @@ class OidcIssuerDiscoveryException extends \Exception { - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcJwtSigningKey.php b/app/Auth/Access/Oidc/OidcJwtSigningKey.php index 3e77cf33173..9a5b3833ade 100644 --- a/app/Auth/Access/Oidc/OidcJwtSigningKey.php +++ b/app/Auth/Access/Oidc/OidcJwtSigningKey.php @@ -18,15 +18,17 @@ class OidcJwtSigningKey * Can be created either from a JWK parameter array or local file path to load a certificate from. * Examples: * 'file:///var/www/cert.pem' - * ['kty' => 'RSA', 'alg' => 'RS256', 'n' => 'abc123...'] + * ['kty' => 'RSA', 'alg' => 'RS256', 'n' => 'abc123...']. + * * @param array|string $jwkOrKeyPath + * * @throws OidcInvalidKeyException */ public function __construct($jwkOrKeyPath) { if (is_array($jwkOrKeyPath)) { $this->loadFromJwkArray($jwkOrKeyPath); - } else if (is_string($jwkOrKeyPath) && strpos($jwkOrKeyPath, 'file://') === 0) { + } elseif (is_string($jwkOrKeyPath) && strpos($jwkOrKeyPath, 'file://') === 0) { $this->loadFromPath($jwkOrKeyPath); } else { throw new OidcInvalidKeyException('Unexpected type of key value provided'); @@ -47,7 +49,7 @@ protected function loadFromPath(string $path) } if (!($this->key instanceof RSA)) { - throw new OidcInvalidKeyException("Key loaded from file path is not an RSA key as expected"); + throw new OidcInvalidKeyException('Key loaded from file path is not an RSA key as expected'); } } @@ -104,5 +106,4 @@ public function toPem(): string { return $this->key->toString('PKCS8'); } - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcOAuthProvider.php b/app/Auth/Access/Oidc/OidcOAuthProvider.php index 03230e373df..9b9d0524c0d 100644 --- a/app/Auth/Access/Oidc/OidcOAuthProvider.php +++ b/app/Auth/Access/Oidc/OidcOAuthProvider.php @@ -30,7 +30,6 @@ class OidcOAuthProvider extends AbstractProvider */ protected $tokenEndpoint; - /** * Returns the base URL for authorizing a client. */ @@ -66,7 +65,6 @@ protected function getDefaultScopes(): array return ['openid', 'profile', 'email']; } - /** * Returns the string that should be used to separate scopes when building * the URL for requesting an access token. @@ -80,9 +78,11 @@ protected function getScopeSeparator(): string * Checks a provider response for errors. * * @param ResponseInterface $response - * @param array|string $data Parsed response data - * @return void + * @param array|string $data Parsed response data + * * @throws IdentityProviderException + * + * @return void */ protected function checkResponse(ResponseInterface $response, $data) { @@ -99,8 +99,9 @@ protected function checkResponse(ResponseInterface $response, $data) * Generates a resource owner object from a successful resource owner * details request. * - * @param array $response + * @param array $response * @param AccessToken $token + * * @return ResourceOwnerInterface */ protected function createResourceOwner(array $response, AccessToken $token) @@ -114,14 +115,13 @@ protected function createResourceOwner(array $response, AccessToken $token) * The grant that was used to fetch the response can be used to provide * additional context. * - * @param array $response + * @param array $response * @param AbstractGrant $grant + * * @return OidcAccessToken */ protected function createAccessToken(array $response, AbstractGrant $grant) { return new OidcAccessToken($response); } - - -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcProviderSettings.php b/app/Auth/Access/Oidc/OidcProviderSettings.php index 2b72c54b00b..32946d058cf 100644 --- a/app/Auth/Access/Oidc/OidcProviderSettings.php +++ b/app/Auth/Access/Oidc/OidcProviderSettings.php @@ -70,6 +70,7 @@ protected function applySettingsFromArray(array $settingsArray) /** * Validate any core, required properties have been set. + * * @throws InvalidArgumentException */ protected function validateInitial() @@ -82,12 +83,13 @@ protected function validateInitial() } if (strpos($this->issuer, 'https://') !== 0) { - throw new InvalidArgumentException("Issuer value must start with https://"); + throw new InvalidArgumentException('Issuer value must start with https://'); } } /** * Perform a full validation on these settings. + * * @throws InvalidArgumentException */ public function validate(): void @@ -103,13 +105,14 @@ public function validate(): void /** * Discover and autoload settings from the configured issuer. + * * @throws OidcIssuerDiscoveryException */ public function discoverFromIssuer(ClientInterface $httpClient, Repository $cache, int $cacheMinutes) { try { $cacheKey = 'oidc-discovery::' . $this->issuer; - $discoveredSettings = $cache->remember($cacheKey, $cacheMinutes * 60, function() use ($httpClient) { + $discoveredSettings = $cache->remember($cacheKey, $cacheMinutes * 60, function () use ($httpClient) { return $this->loadSettingsFromIssuerDiscovery($httpClient); }); $this->applySettingsFromArray($discoveredSettings); @@ -134,7 +137,7 @@ protected function loadSettingsFromIssuerDiscovery(ClientInterface $httpClient): } if ($result['issuer'] !== $this->issuer) { - throw new OidcIssuerDiscoveryException("Unexpected issuer value found on discovery response"); + throw new OidcIssuerDiscoveryException('Unexpected issuer value found on discovery response'); } $discoveredSettings = []; @@ -160,13 +163,14 @@ protected function loadSettingsFromIssuerDiscovery(ClientInterface $httpClient): */ protected function filterKeys(array $keys): array { - return array_filter($keys, function(array $key) { + return array_filter($keys, function (array $key) { return $key['kty'] === 'RSA' && $key['use'] === 'sig' && $key['alg'] === 'RS256'; }); } /** * Return an array of jwks as PHP key=>value arrays. + * * @throws ClientExceptionInterface * @throws OidcIssuerDiscoveryException */ @@ -177,7 +181,7 @@ protected function loadKeysFromUri(string $uri, ClientInterface $httpClient): ar $result = json_decode($response->getBody()->getContents(), true); if (empty($result) || !is_array($result) || !isset($result['keys'])) { - throw new OidcIssuerDiscoveryException("Error reading keys from issuer jwks_uri"); + throw new OidcIssuerDiscoveryException('Error reading keys from issuer jwks_uri'); } return $result['keys']; @@ -193,6 +197,7 @@ public function arrayForProvider(): array foreach ($settingKeys as $setting) { $settings[$setting] = $this->$setting; } + return $settings; } -} \ No newline at end of file +} diff --git a/app/Auth/Access/Oidc/OidcService.php b/app/Auth/Access/Oidc/OidcService.php index d59d274e887..b8e017b4b13 100644 --- a/app/Auth/Access/Oidc/OidcService.php +++ b/app/Auth/Access/Oidc/OidcService.php @@ -1,5 +1,8 @@ -getProviderSettings(); $provider = $this->getProvider($settings); + return [ - 'url' => $provider->getAuthorizationUrl(), + 'url' => $provider->getAuthorizationUrl(), 'state' => $provider->getState(), ]; } @@ -56,6 +60,7 @@ public function login(): array * return the matching, or new if registration active, user matched to * the authorization server. * Returns null if not authenticated. + * * @throws Exception * @throws ClientExceptionInterface */ @@ -80,12 +85,12 @@ protected function getProviderSettings(): OidcProviderSettings { $config = $this->config(); $settings = new OidcProviderSettings([ - 'issuer' => $config['issuer'], - 'clientId' => $config['client_id'], - 'clientSecret' => $config['client_secret'], - 'redirectUri' => url('/oidc/callback'), + 'issuer' => $config['issuer'], + 'clientId' => $config['client_id'], + 'clientSecret' => $config['client_secret'], + 'redirectUri' => url('/oidc/callback'), 'authorizationEndpoint' => $config['authorization_endpoint'], - 'tokenEndpoint' => $config['token_endpoint'], + 'tokenEndpoint' => $config['token_endpoint'], ]); // Use keys if configured @@ -109,13 +114,13 @@ protected function getProviderSettings(): OidcProviderSettings protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider { return new OidcOAuthProvider($settings->arrayForProvider(), [ - 'httpClient' => $this->httpClient, + 'httpClient' => $this->httpClient, 'optionProvider' => new HttpBasicAuthOptionProvider(), ]); } /** - * Calculate the display name + * Calculate the display name. */ protected function getUserDisplayName(OidcIdToken $token, string $defaultValue): string { @@ -138,21 +143,24 @@ protected function getUserDisplayName(OidcIdToken $token, string $defaultValue): /** * Extract the details of a user from an ID token. + * * @return array{name: string, email: string, external_id: string} */ protected function getUserDetails(OidcIdToken $token): array { $id = $token->getClaim('sub'); + return [ 'external_id' => $id, - 'email' => $token->getClaim('email'), - 'name' => $this->getUserDisplayName($token, $id), + 'email' => $token->getClaim('email'), + 'name' => $this->getUserDisplayName($token, $id), ]; } /** * Processes a received access token for a user. Login the user when * they exist, optionally registering them automatically. + * * @throws OpenIdConnectException * @throws JsonDebugException * @throws UserRegistrationException @@ -189,7 +197,9 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc } $user = $this->registrationService->findOrRegister( - $userDetails['name'], $userDetails['email'], $userDetails['external_id'] + $userDetails['name'], + $userDetails['email'], + $userDetails['external_id'] ); if ($user === null) { @@ -197,6 +207,7 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc } $this->loginService->login($user, 'oidc'); + return $user; } diff --git a/app/Auth/Access/RegistrationService.php b/app/Auth/Access/RegistrationService.php index 48970bd2e4c..dcdb68bd5cd 100644 --- a/app/Auth/Access/RegistrationService.php +++ b/app/Auth/Access/RegistrationService.php @@ -54,6 +54,7 @@ protected function registrationAllowed(): bool /** * Attempt to find a user in the system otherwise register them as a new * user. For use with external auth systems since password is auto-generated. + * * @throws UserRegistrationException */ public function findOrRegister(string $name, string $email, string $externalId): User diff --git a/app/Auth/Access/Saml2Service.php b/app/Auth/Access/Saml2Service.php index 8e076f86ca3..6d3915c4d5b 100644 --- a/app/Auth/Access/Saml2Service.php +++ b/app/Auth/Access/Saml2Service.php @@ -29,10 +29,9 @@ class Saml2Service */ public function __construct( RegistrationService $registrationService, - LoginService $loginService, - GroupSyncService $groupSyncService - ) - { + LoginService $loginService, + GroupSyncService $groupSyncService + ) { $this->config = config('saml2'); $this->registrationService = $registrationService; $this->loginService = $loginService; @@ -51,7 +50,7 @@ public function login(): array return [ 'url' => $toolKit->login($returnRoute, [], false, false, true), - 'id' => $toolKit->getLastRequestID(), + 'id' => $toolKit->getLastRequestID(), ]; } @@ -200,7 +199,7 @@ protected function getToolkit(): Auth protected function loadOneloginServiceProviderDetails(): array { $spDetails = [ - 'entityId' => url('/saml2/metadata'), + 'entityId' => url('/saml2/metadata'), 'assertionConsumerService' => [ 'url' => url('/saml2/acs'), ], @@ -211,7 +210,7 @@ protected function loadOneloginServiceProviderDetails(): array return [ 'baseurl' => url('/saml2'), - 'sp' => $spDetails, + 'sp' => $spDetails, ]; } @@ -263,6 +262,7 @@ protected function getExternalId(array $samlAttributes, string $defaultValue) /** * Extract the details of a user from a SAML response. + * * @return array{external_id: string, name: string, email: string, saml_id: string} */ protected function getUserDetails(string $samlID, $samlAttributes): array @@ -275,9 +275,9 @@ protected function getUserDetails(string $samlID, $samlAttributes): array return [ 'external_id' => $externalId, - 'name' => $this->getUserDisplayName($samlAttributes, $externalId), - 'email' => $email, - 'saml_id' => $samlID, + 'name' => $this->getUserDisplayName($samlAttributes, $externalId), + 'email' => $email, + 'saml_id' => $samlID, ]; } @@ -344,8 +344,8 @@ public function processLoginCallback(string $samlID, array $samlAttributes): Use if ($this->config['dump_user_details']) { throw new JsonDebugException([ - 'id_from_idp' => $samlID, - 'attrs_from_idp' => $samlAttributes, + 'id_from_idp' => $samlID, + 'attrs_from_idp' => $samlAttributes, 'attrs_after_parsing' => $userDetails, ]); } @@ -359,7 +359,9 @@ public function processLoginCallback(string $samlID, array $samlAttributes): Use } $user = $this->registrationService->findOrRegister( - $userDetails['name'], $userDetails['email'], $userDetails['external_id'] + $userDetails['name'], + $userDetails['email'], + $userDetails['external_id'] ); if ($user === null) { diff --git a/app/Config/auth.php b/app/Config/auth.php index 69da69bf119..88c22e70aca 100644 --- a/app/Config/auth.php +++ b/app/Config/auth.php @@ -41,7 +41,7 @@ 'provider' => 'external', ], 'oidc' => [ - 'driver' => 'async-external-session', + 'driver' => 'async-external-session', 'provider' => 'external', ], 'api' => [ diff --git a/app/Config/oidc.php b/app/Config/oidc.php index 1b50d9d66c9..842ac8af6b8 100644 --- a/app/Config/oidc.php +++ b/app/Config/oidc.php @@ -31,5 +31,5 @@ // OAuth2 endpoints. 'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null), - 'token_endpoint' => env('OIDC_TOKEN_ENDPOINT', null), + 'token_endpoint' => env('OIDC_TOKEN_ENDPOINT', null), ]; diff --git a/app/Exceptions/OpenIdConnectException.php b/app/Exceptions/OpenIdConnectException.php index d585857320f..7bbc4bdaf57 100644 --- a/app/Exceptions/OpenIdConnectException.php +++ b/app/Exceptions/OpenIdConnectException.php @@ -1,6 +1,7 @@ -showErrorNotification(trans('errors.oidc_fail_authed', ['system' => config('oidc.name')])); + return redirect('/login'); } $this->oidcService->processAuthorizeResponse($request->query('code')); + return redirect()->intended(); } } diff --git a/app/Providers/AppServiceProvider.php b/app/Providers/AppServiceProvider.php index da41de6513a..34a3a290f0c 100644 --- a/app/Providers/AppServiceProvider.php +++ b/app/Providers/AppServiceProvider.php @@ -22,8 +22,8 @@ use Illuminate\Support\Facades\View; use Illuminate\Support\ServiceProvider; use Laravel\Socialite\Contracts\Factory as SocialiteFactory; -use Whoops\Handler\HandlerInterface; use Psr\Http\Client\ClientInterface as HttpClientInterface; +use Whoops\Handler\HandlerInterface; class AppServiceProvider extends ServiceProvider { @@ -85,7 +85,7 @@ public function register() return new CspService(); }); - $this->app->bind(HttpClientInterface::class, function($app) { + $this->app->bind(HttpClientInterface::class, function ($app) { return new Client([ 'timeout' => 3, ]); diff --git a/app/Providers/AuthServiceProvider.php b/app/Providers/AuthServiceProvider.php index bc7caa195ac..4a626e4fadd 100644 --- a/app/Providers/AuthServiceProvider.php +++ b/app/Providers/AuthServiceProvider.php @@ -4,8 +4,8 @@ use BookStack\Api\ApiTokenGuard; use BookStack\Auth\Access\ExternalBaseUserProvider; -use BookStack\Auth\Access\Guards\LdapSessionGuard; use BookStack\Auth\Access\Guards\AsyncExternalBaseSessionGuard; +use BookStack\Auth\Access\Guards\LdapSessionGuard; use BookStack\Auth\Access\LdapService; use BookStack\Auth\Access\LoginService; use BookStack\Auth\Access\RegistrationService; diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php index cf04080fcdd..6c806aa8ed4 100644 --- a/tests/Auth/OidcTest.php +++ b/tests/Auth/OidcTest.php @@ -1,7 +1,8 @@ -keyFilePath, OidcJwtHelper::publicPemKey()); config()->set([ - 'auth.method' => 'oidc', - 'auth.defaults.guard' => 'oidc', - 'oidc.name' => 'SingleSignOn-Testing', - 'oidc.display_name_claims' => ['name'], - 'oidc.client_id' => OidcJwtHelper::defaultClientId(), - 'oidc.client_secret' => 'testpass', - 'oidc.jwt_public_key' => $this->keyFilePath, - 'oidc.issuer' => OidcJwtHelper::defaultIssuer(), + 'auth.method' => 'oidc', + 'auth.defaults.guard' => 'oidc', + 'oidc.name' => 'SingleSignOn-Testing', + 'oidc.display_name_claims' => ['name'], + 'oidc.client_id' => OidcJwtHelper::defaultClientId(), + 'oidc.client_secret' => 'testpass', + 'oidc.jwt_public_key' => $this->keyFilePath, + 'oidc.issuer' => OidcJwtHelper::defaultIssuer(), 'oidc.authorization_endpoint' => 'https://oidc.local/auth', - 'oidc.token_endpoint' => 'https://oidc.local/token', - 'oidc.discover' => false, - 'oidc.dump_user_details' => false, + 'oidc.token_endpoint' => 'https://oidc.local/token', + 'oidc.discover' => false, + 'oidc.dump_user_details' => false, ]); } @@ -131,7 +132,7 @@ public function test_login_success_flow() $transactions = &$this->mockHttpClient([$this->getMockAuthorizationResponse([ 'email' => 'benny@example.com', - 'sub' => 'benny1010101' + 'sub' => 'benny1010101', ])]); // Callback from auth provider @@ -148,12 +149,11 @@ public function test_login_success_flow() $this->assertStringContainsString('code=SplxlOBeZQQYbYS6WxSbIA', $tokenRequest->getBody()); $this->assertStringContainsString('redirect_uri=' . urlencode(url('/oidc/callback')), $tokenRequest->getBody()); - $this->assertTrue(auth()->check()); $this->assertDatabaseHas('users', [ - 'email' => 'benny@example.com', + 'email' => 'benny@example.com', 'external_auth_id' => 'benny1010101', - 'email_confirmed' => false, + 'email_confirmed' => false, ]); $user = User::query()->where('email', '=', 'benny@example.com')->first(); @@ -176,15 +176,15 @@ public function test_dump_user_details_option_outputs_as_expected() $resp = $this->runLogin([ 'email' => 'benny@example.com', - 'sub' => 'benny505' + 'sub' => 'benny505', ]); $resp->assertStatus(200); $resp->assertJson([ 'email' => 'benny@example.com', - 'sub' => 'benny505', - "iss" => OidcJwtHelper::defaultIssuer(), - "aud" => OidcJwtHelper::defaultClientId(), + 'sub' => 'benny505', + 'iss' => OidcJwtHelper::defaultIssuer(), + 'aud' => OidcJwtHelper::defaultClientId(), ]); $this->assertFalse(auth()->check()); } @@ -193,7 +193,7 @@ public function test_auth_fails_if_no_email_exists_in_user_data() { $this->runLogin([ 'email' => '', - 'sub' => 'benny505' + 'sub' => 'benny505', ]); $this->assertSessionError('Could not find an email address, for this user, in the data provided by the external authentication system'); @@ -205,7 +205,7 @@ public function test_auth_fails_if_already_logged_in() $this->runLogin([ 'email' => 'benny@example.com', - 'sub' => 'benny505' + 'sub' => 'benny505', ]); $this->assertSessionError('Already logged in'); @@ -221,7 +221,7 @@ public function test_auth_login_as_existing_user() $this->runLogin([ 'email' => 'benny@example.com', - 'sub' => 'benny505' + 'sub' => 'benny505', ]); $this->assertTrue(auth()->check()); @@ -238,7 +238,7 @@ public function test_auth_login_as_existing_user_email_with_different_auth_id_fa $this->runLogin([ 'email' => $editor->email, - 'sub' => 'benny505' + 'sub' => 'benny505', ]); $this->assertSessionError('A user with the email ' . $editor->email . ' already exists but with different credentials.'); @@ -300,7 +300,7 @@ public function test_autodiscovery_calls_are_cached() $this->getAutoDiscoveryResponse(), $this->getJwksResponse(), $this->getAutoDiscoveryResponse([ - 'issuer' => 'https://auto.example.com' + 'issuer' => 'https://auto.example.com', ]), $this->getJwksResponse(), ]); @@ -321,11 +321,11 @@ public function test_autodiscovery_calls_are_cached() protected function withAutodiscovery() { config()->set([ - 'oidc.issuer' => OidcJwtHelper::defaultIssuer(), - 'oidc.discover' => true, + 'oidc.issuer' => OidcJwtHelper::defaultIssuer(), + 'oidc.discover' => true, 'oidc.authorization_endpoint' => null, - 'oidc.token_endpoint' => null, - 'oidc.jwt_public_key' => null, + 'oidc.token_endpoint' => null, + 'oidc.jwt_public_key' => null, ]); } @@ -341,41 +341,41 @@ protected function runLogin($claimOverrides = []): TestResponse protected function getAutoDiscoveryResponse($responseOverrides = []): Response { return new Response(200, [ - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Cache-Control' => 'no-cache, no-store', - 'Pragma' => 'no-cache' + 'Pragma' => 'no-cache', ], json_encode(array_merge([ - 'token_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/token', + 'token_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/token', 'authorization_endpoint' => OidcJwtHelper::defaultIssuer() . '/oidc/authorize', - 'jwks_uri' => OidcJwtHelper::defaultIssuer() . '/oidc/keys', - 'issuer' => OidcJwtHelper::defaultIssuer() + 'jwks_uri' => OidcJwtHelper::defaultIssuer() . '/oidc/keys', + 'issuer' => OidcJwtHelper::defaultIssuer(), ], $responseOverrides))); } protected function getJwksResponse(): Response { return new Response(200, [ - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Cache-Control' => 'no-cache, no-store', - 'Pragma' => 'no-cache' + 'Pragma' => 'no-cache', ], json_encode([ 'keys' => [ - OidcJwtHelper::publicJwkKeyArray() - ] + OidcJwtHelper::publicJwkKeyArray(), + ], ])); } protected function getMockAuthorizationResponse($claimOverrides = []): Response { return new Response(200, [ - 'Content-Type' => 'application/json', + 'Content-Type' => 'application/json', 'Cache-Control' => 'no-cache, no-store', - 'Pragma' => 'no-cache' + 'Pragma' => 'no-cache', ], json_encode([ 'access_token' => 'abc123', - 'token_type' => 'Bearer', - 'expires_in' => 3600, - 'id_token' => OidcJwtHelper::idToken($claimOverrides) + 'token_type' => 'Bearer', + 'expires_in' => 3600, + 'id_token' => OidcJwtHelper::idToken($claimOverrides), ])); } } diff --git a/tests/Helpers/OidcJwtHelper.php b/tests/Helpers/OidcJwtHelper.php index 0c44efb01a6..55a34d4dc96 100644 --- a/tests/Helpers/OidcJwtHelper.php +++ b/tests/Helpers/OidcJwtHelper.php @@ -12,31 +12,31 @@ class OidcJwtHelper { public static function defaultIssuer(): string { - return "https://auth.example.com"; + return 'https://auth.example.com'; } public static function defaultClientId(): string { - return "xxyyzz.aaa.bbccdd.123"; + return 'xxyyzz.aaa.bbccdd.123'; } public static function defaultPayload(): array { return [ - "sub" => "abc1234def", - "name" => "Barry Scott", - "email" => "bscott@example.com", - "ver" => 1, - "iss" => static::defaultIssuer(), - "aud" => static::defaultClientId(), - "iat" => time(), - "exp" => time() + 720, - "jti" => "ID.AaaBBBbbCCCcccDDddddddEEEeeeeee", - "amr" => ["pwd"], - "idp" => "fghfghgfh546456dfgdfg", - "preferred_username" => "xXBazzaXx", - "auth_time" => time(), - "at_hash" => "sT4jbsdSGy9w12pq3iNYDA", + 'sub' => 'abc1234def', + 'name' => 'Barry Scott', + 'email' => 'bscott@example.com', + 'ver' => 1, + 'iss' => static::defaultIssuer(), + 'aud' => static::defaultClientId(), + 'iat' => time(), + 'exp' => time() + 720, + 'jti' => 'ID.AaaBBBbbCCCcccDDddddddEEEeeeeee', + 'amr' => ['pwd'], + 'idp' => 'fghfghgfh546456dfgdfg', + 'preferred_username' => 'xXBazzaXx', + 'auth_time' => time(), + 'at_hash' => 'sT4jbsdSGy9w12pq3iNYDA', ]; } @@ -55,6 +55,7 @@ public static function idToken($payloadOverrides = [], $headerOverrides = []): s $privateKey = static::privateKeyInstance(); $signature = $privateKey->sign($top); + return $top . '.' . static::base64UrlEncode($signature); } @@ -75,7 +76,7 @@ public static function base64UrlEncode(string $decoded): string public static function publicPemKey(): string { - return "-----BEGIN PUBLIC KEY----- + return '-----BEGIN PUBLIC KEY----- MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAqo1OmfNKec5S2zQC4SP9 DrHuUR0VgCi6oqcGERz7zqO36hqk3A3R3aCgJkEjfnbnMuszRRKs45NbXoOp9pvm zXL16c93Obn7G8x8A3ao6yN5qKO5S5+CETqOZfKN/g75Xlz7VsC3igOhgsXnPx6i @@ -83,12 +84,12 @@ public static function publicPemKey(): string +zihMHCZUUvQu99P+o0MDR0lMUT+vPJ6SJeRfnoHexwt6bZFiNnsZIEL03bX4QNk WvsLta1+jNUee+8IPVhzCO8bvM86NzLaKUJ4k6NZ5IVrmdCFpFsjCWByOrDG8wdw 3wIDAQAB ------END PUBLIC KEY-----"; +-----END PUBLIC KEY-----'; } public static function privatePemKey(): string { - return "-----BEGIN PRIVATE KEY----- + return '-----BEGIN PRIVATE KEY----- MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQCqjU6Z80p5zlLb NALhI/0Ose5RHRWAKLqipwYRHPvOo7fqGqTcDdHdoKAmQSN+ducy6zNFEqzjk1te g6n2m+bNcvXpz3c5ufsbzHwDdqjrI3moo7lLn4IROo5l8o3+DvleXPtWwLeKA6GC @@ -115,7 +116,7 @@ public static function privatePemKey(): string efqXPwg2wAPYeiec49EbfnteQQKAkqNfJ9K69yE2naf6bw3/5mCBsq/cXeuaBMII ylysUIRBqt2J0kWm2yCpFWR7H+Ilhdx9A7ZLCqYVt8e+vjO/BOI3cQDe2VPOLPSl q/1PY4iJviGKddtmfClH3v4= ------END PRIVATE KEY-----"; +-----END PRIVATE KEY-----'; } public static function publicJwkKeyArray(): array @@ -125,8 +126,8 @@ public static function publicJwkKeyArray(): array 'alg' => 'RS256', 'kid' => '066e52af-8884-4926-801d-032a276f9f2a', 'use' => 'sig', - 'e' => 'AQAB', - 'n' => 'qo1OmfNKec5S2zQC4SP9DrHuUR0VgCi6oqcGERz7zqO36hqk3A3R3aCgJkEjfnbnMuszRRKs45NbXoOp9pvmzXL16c93Obn7G8x8A3ao6yN5qKO5S5-CETqOZfKN_g75Xlz7VsC3igOhgsXnPx6iiM6sbYbk0U_XpFaT84LXKI8VTIPUo7gTeZN1pTET__i9FlzAOzX-xfWBKdOqlEzl-zihMHCZUUvQu99P-o0MDR0lMUT-vPJ6SJeRfnoHexwt6bZFiNnsZIEL03bX4QNkWvsLta1-jNUee-8IPVhzCO8bvM86NzLaKUJ4k6NZ5IVrmdCFpFsjCWByOrDG8wdw3w', + 'e' => 'AQAB', + 'n' => 'qo1OmfNKec5S2zQC4SP9DrHuUR0VgCi6oqcGERz7zqO36hqk3A3R3aCgJkEjfnbnMuszRRKs45NbXoOp9pvmzXL16c93Obn7G8x8A3ao6yN5qKO5S5-CETqOZfKN_g75Xlz7VsC3igOhgsXnPx6iiM6sbYbk0U_XpFaT84LXKI8VTIPUo7gTeZN1pTET__i9FlzAOzX-xfWBKdOqlEzl-zihMHCZUUvQu99P-o0MDR0lMUT-vPJ6SJeRfnoHexwt6bZFiNnsZIEL03bX4QNkWvsLta1-jNUee-8IPVhzCO8bvM86NzLaKUJ4k6NZ5IVrmdCFpFsjCWByOrDG8wdw3w', ]; } -} \ No newline at end of file +} diff --git a/tests/SharedTestHelpers.php b/tests/SharedTestHelpers.php index 606a3cd9e20..04952d22345 100644 --- a/tests/SharedTestHelpers.php +++ b/tests/SharedTestHelpers.php @@ -252,6 +252,7 @@ protected function mockHttpFetch($returnData, int $times = 1) /** * Mock the http client used in BookStack. * Returns a reference to the container which holds all history of http transactions. + * * @link https://docs.guzzlephp.org/en/stable/testing.html#history-middleware */ protected function &mockHttpClient(array $responses = []): array @@ -262,6 +263,7 @@ protected function &mockHttpClient(array $responses = []): array $handlerStack = new HandlerStack($mock); $handlerStack->push($history); $this->app[ClientInterface::class] = new Client(['handler' => $handlerStack]); + return $container; } diff --git a/tests/Unit/OidcIdTokenTest.php b/tests/Unit/OidcIdTokenTest.php index b08d578b3b9..ad91eecd8b2 100644 --- a/tests/Unit/OidcIdTokenTest.php +++ b/tests/Unit/OidcIdTokenTest.php @@ -2,8 +2,8 @@ namespace Tests\Unit; -use BookStack\Auth\Access\Oidc\OidcInvalidTokenException; use BookStack\Auth\Access\Oidc\OidcIdToken; +use BookStack\Auth\Access\Oidc\OidcInvalidTokenException; use Tests\Helpers\OidcJwtHelper; use Tests\TestCase; @@ -12,7 +12,7 @@ class OidcIdTokenTest extends TestCase public function test_valid_token_passes_validation() { $token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), [ - OidcJwtHelper::publicJwkKeyArray() + OidcJwtHelper::publicJwkKeyArray(), ]); $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123')); @@ -54,6 +54,7 @@ public function test_token_structure_error_cases() foreach ($messagesAndTokenValues as [$message, $tokenValue]) { $token = new OidcIdToken($tokenValue, OidcJwtHelper::defaultIssuer(), []); $err = null; + try { $token->validate('abc'); } catch (\Exception $exception) { @@ -77,8 +78,8 @@ public function test_error_thrown_if_token_signature_not_validated_from_non_matc { $token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), [ array_merge(OidcJwtHelper::publicJwkKeyArray(), [ - 'n' => 'iqK-1QkICMf_cusNLpeNnN-bhT0-9WLBvzgwKLALRbrevhdi5ttrLHIQshaSL0DklzfyG2HWRmAnJ9Q7sweEjuRiiqRcSUZbYu8cIv2hLWYu7K_NH67D2WUjl0EnoHEuiVLsZhQe1CmdyLdx087j5nWkd64K49kXRSdxFQUlj8W3NeK3CjMEUdRQ3H4RZzJ4b7uuMiFA29S2ZhMNG20NPbkUVsFL-jiwTd10KSsPT8yBYipI9O7mWsUWt_8KZs1y_vpM_k3SyYihnWpssdzDm1uOZ8U3mzFr1xsLAO718GNUSXk6npSDzLl59HEqa6zs4O9awO2qnSHvcmyELNk31w' - ]) + 'n' => 'iqK-1QkICMf_cusNLpeNnN-bhT0-9WLBvzgwKLALRbrevhdi5ttrLHIQshaSL0DklzfyG2HWRmAnJ9Q7sweEjuRiiqRcSUZbYu8cIv2hLWYu7K_NH67D2WUjl0EnoHEuiVLsZhQe1CmdyLdx087j5nWkd64K49kXRSdxFQUlj8W3NeK3CjMEUdRQ3H4RZzJ4b7uuMiFA29S2ZhMNG20NPbkUVsFL-jiwTd10KSsPT8yBYipI9O7mWsUWt_8KZs1y_vpM_k3SyYihnWpssdzDm1uOZ8U3mzFr1xsLAO718GNUSXk6npSDzLl59HEqa6zs4O9awO2qnSHvcmyELNk31w', + ]), ]); $this->expectException(OidcInvalidTokenException::class); $this->expectExceptionMessage('Token signature could not be validated using the provided keys'); @@ -97,7 +98,7 @@ public function test_error_thrown_if_token_algorithm_is_not_rs256() { $token = new OidcIdToken(OidcJwtHelper::idToken([], ['alg' => 'HS256']), OidcJwtHelper::defaultIssuer(), []); $this->expectException(OidcInvalidTokenException::class); - $this->expectExceptionMessage("Only RS256 signature validation is supported. Token reports using HS256"); + $this->expectExceptionMessage('Only RS256 signature validation is supported. Token reports using HS256'); $token->validate('abc'); } @@ -134,10 +135,11 @@ public function test_token_claim_error_cases() foreach ($claimOverridesByErrorMessage as [$message, $overrides]) { $token = new OidcIdToken(OidcJwtHelper::idToken($overrides), OidcJwtHelper::defaultIssuer(), [ - OidcJwtHelper::publicJwkKeyArray() + OidcJwtHelper::publicJwkKeyArray(), ]); $err = null; + try { $token->validate('xxyyzz.aaa.bbccdd.123'); } catch (\Exception $exception) { @@ -155,10 +157,10 @@ public function test_keys_can_be_a_local_file_reference_to_pem_key() $testFilePath = 'file://' . stream_get_meta_data($file)['uri']; file_put_contents($testFilePath, OidcJwtHelper::publicPemKey()); $token = new OidcIdToken(OidcJwtHelper::idToken(), OidcJwtHelper::defaultIssuer(), [ - $testFilePath + $testFilePath, ]); $this->assertTrue($token->validate('xxyyzz.aaa.bbccdd.123')); unlink($testFilePath); } -} \ No newline at end of file +}