From 4adc2d880b46c0c3c18db00de7fe2d384f8466f0 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sat, 18 Apr 2026 12:32:37 +0200 Subject: [PATCH 01/14] feat(keycloak): added a dedicated Keycloak configuration (#4188) --- .../src/configuration/configuration.test.ts | 8 +++++ .../assets/src/configuration/configuration.ts | 1 + .../templates/admin/configuration/main.twig | 7 +++++ .../admin/configuration/tab-list.twig | 1 + .../Api/ConfigurationTabController.php | 9 ++++++ .../Setup/Installation/DefaultDataSeeder.php | 9 ++++++ .../Migration/Versions/Migration420Alpha.php | 11 +++++++ phpmyfaq/translations/language_de.php | 10 ++++++ phpmyfaq/translations/language_en.php | 10 ++++++ .../Api/ConfigurationTabControllerTest.php | 19 ++++++++++++ .../ConfigurationControllerWebTest.php | 1 + tests/phpMyFAQ/TranslationTest.php | 31 +++++++++++++++++++ 12 files changed, 117 insertions(+) diff --git a/phpmyfaq/admin/assets/src/configuration/configuration.test.ts b/phpmyfaq/admin/assets/src/configuration/configuration.test.ts index 0e2be2814e..e078871a33 100644 --- a/phpmyfaq/admin/assets/src/configuration/configuration.test.ts +++ b/phpmyfaq/admin/assets/src/configuration/configuration.test.ts @@ -294,12 +294,16 @@ describe('Configuration Functions', () => {
    • +
  • + +
    • +
    `; (fetchConfiguration as Mock).mockResolvedValue('Configuration content'); @@ -347,6 +351,10 @@ describe('Configuration Functions', () => {
    Enable OAuth 2.0 authorization server
    Private key path
    +
    +
    Enable Keycloak sign-in
    +
    Client ID
    +
    Release environment
    diff --git a/phpmyfaq/admin/assets/src/configuration/configuration.ts b/phpmyfaq/admin/assets/src/configuration/configuration.ts index 5d47de6e1f..8b4363a0fe 100644 --- a/phpmyfaq/admin/assets/src/configuration/configuration.ts +++ b/phpmyfaq/admin/assets/src/configuration/configuration.ts @@ -48,6 +48,7 @@ const TAB_TARGETS = [ '#mail', '#api', '#oauth2', + '#keycloak', '#upgrade', '#translation', '#storage', diff --git a/phpmyfaq/assets/templates/admin/configuration/main.twig b/phpmyfaq/assets/templates/admin/configuration/main.twig index a2c2db914c..1f2f69bc94 100644 --- a/phpmyfaq/assets/templates/admin/configuration/main.twig +++ b/phpmyfaq/assets/templates/admin/configuration/main.twig @@ -125,6 +125,12 @@ {{ 'oauthControlCenter' | translate }} +
    • +
  • Integrations
    • +
  • + +
    • +
  • + +
  • Maintenance
  • @@ -436,6 +443,59 @@ describe('Configuration Functions', () => { vi.useRealTimers(); }); + it('should show Keycloak tab and items when filtering by "keycloak"', async () => { + vi.useFakeTimers(); + buildFilterDOM(); + + (fetchConfiguration as Mock).mockResolvedValue(''); + + handleConfigurationTabFiltering(); + + await triggerFilterAndFlush('keycloak'); + + const keycloakTab = document.querySelector('li.nav-item[data-config-label="Keycloak"]'); + const oauth2Tab = document.querySelector('li.nav-item[data-config-label="OAuth 2.0"]'); + const mainTab = document.querySelector('li.nav-item[data-config-label="Main"]'); + const upgradeTab = document.querySelector('li.nav-item[data-config-label="Upgrade"]'); + + expect(keycloakTab?.classList.contains('d-none')).toBe(false); + expect(oauth2Tab?.classList.contains('d-none')).toBe(true); + expect(mainTab?.classList.contains('d-none')).toBe(true); + expect(upgradeTab?.classList.contains('d-none')).toBe(true); + + const keycloakEnable = document.querySelector('.pmf-config-item[data-config-key="keycloak.enable"]'); + const keycloakClientId = document.querySelector('.pmf-config-item[data-config-key="keycloak.clientId"]'); + const languageItem = document.querySelector('.pmf-config-item[data-config-key="main.language"]'); + + expect(keycloakEnable?.classList.contains('d-none')).toBe(false); + expect(keycloakClientId?.classList.contains('d-none')).toBe(false); + expect(languageItem?.classList.contains('d-none')).toBe(true); + + vi.useRealTimers(); + }); + + it('should show Keycloak tab when filtering by "client id"', async () => { + vi.useFakeTimers(); + buildFilterDOM(); + + (fetchConfiguration as Mock).mockResolvedValue(''); + + handleConfigurationTabFiltering(); + + await triggerFilterAndFlush('client id'); + + const keycloakTab = document.querySelector('li.nav-item[data-config-label="Keycloak"]'); + const mainTab = document.querySelector('li.nav-item[data-config-label="Main"]'); + + expect(keycloakTab?.classList.contains('d-none')).toBe(false); + expect(mainTab?.classList.contains('d-none')).toBe(true); + + const clientIdItem = document.querySelector('.pmf-config-item[data-config-key="keycloak.clientId"]'); + expect(clientIdItem?.classList.contains('d-none')).toBe(false); + + vi.useRealTimers(); + }); + it('should restore all items and tabs when filter is cleared', async () => { vi.useFakeTimers(); buildFilterDOM(); diff --git a/phpmyfaq/assets/templates/default/login.twig b/phpmyfaq/assets/templates/default/login.twig index f206c0abd4..918b1d5115 100644 --- a/phpmyfaq/assets/templates/default/login.twig +++ b/phpmyfaq/assets/templates/default/login.twig @@ -75,7 +75,7 @@ {% endif %} {% if useSignInWithKeycloak %} - + {{ 'msgSignInWithKeycloak' | translate }} diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php index 6ff258f1d2..8f3b4fc45c 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php @@ -54,7 +54,14 @@ public function create(string $login, #[SensitiveParameter] string $password, st try { $result = $user->createUser($login, '', $domain); } catch (\Exception $exception) { - $this->configuration->getLogger()->info($exception->getMessage()); + $this->configuration + ->getLogger() + ->error(sprintf('Keycloak user creation failed for "%s": %s', $login, $exception->getMessage())); + return false; + } + + if (!$result) { + return false; } $user->setStatus('active'); @@ -62,6 +69,7 @@ public function create(string $login, #[SensitiveParameter] string $password, st $user->setUserData([ 'display_name' => $this->getDisplayName(), 'email' => $this->getEmail(), + 'keycloak_sub' => $this->getSubject(), ]); if ($this->shouldAssignGroups()) { @@ -127,6 +135,11 @@ private function getEmail(): string return trim((string) ($this->claims['email'] ?? '')); } + private function getSubject(): string + { + return trim((string) ($this->claims['sub'] ?? '')); + } + private function userExists(string $login): bool { $user = $this->createUser(); @@ -156,7 +169,11 @@ private function assignUserToGroups(int $userId): void $groupMapping = $this->getGroupMapping(); foreach ($roleNames as $roleName) { - $faqGroupName = $groupMapping[$roleName] ?? $roleName; + if (!isset($groupMapping[$roleName])) { + continue; + } + + $faqGroupName = $groupMapping[$roleName]; $groupId = $mediumPermission->findOrCreateGroupByName($faqGroupName); if ($groupId <= 0) { @@ -166,7 +183,7 @@ private function assignUserToGroups(int $userId): void $mediumPermission->addToGroup($userId, $groupId); $this->configuration ->getLogger() - ->info(sprintf('Added Keycloak user %s to group %s', $this->resolvedLogin, $faqGroupName)); + ->info(sprintf('Added Keycloak user #%d to group %s', $userId, $faqGroupName)); } } @@ -188,20 +205,16 @@ private function extractRoleNames(): array } } - $resourceAccess = $this->claims['resource_access'] ?? []; - if (is_array($resourceAccess)) { - foreach ($resourceAccess as $resource) { - $resourceRoles = is_array($resource) ? $resource['roles'] ?? null : null; - if (!is_array($resourceRoles)) { - continue; - } - - foreach ($resourceRoles as $resourceRole) { - if (!is_string($resourceRole) || $resourceRole === '') { + $clientId = trim((string) $this->configuration->get(item: 'keycloak.clientId')); + if ($clientId !== '') { + $clientRoles = $this->claims['resource_access'][$clientId]['roles'] ?? []; + if (is_array($clientRoles)) { + foreach ($clientRoles as $clientRole) { + if (!is_string($clientRole) || $clientRole === '') { continue; } - $roleNames[] = $resourceRole; + $roleNames[] = $clientRole; } } } diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcClient.php b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcClient.php index 7a37e6737f..2af2f92f69 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcClient.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcClient.php @@ -140,12 +140,19 @@ private function decodeJsonResponse(string $content, int $statusCode, string $co } try { - /** @var array $payload */ $payload = json_decode($content, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); } catch (JsonException $exception) { throw new RuntimeException(sprintf('OIDC %s response is not valid JSON', $context), previous: $exception); } + if (!is_array($payload)) { + throw new RuntimeException(sprintf( + 'OIDC %s response is not a JSON object/array, got %s', + $context, + gettype($payload), + )); + } + return $payload; } } diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php new file mode 100644 index 0000000000..0346a2ade7 --- /dev/null +++ b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php @@ -0,0 +1,354 @@ + + * @copyright 2026 phpMyFAQ Team + * @license https://www.mozilla.org/MPL/2.0/ Mozilla Public License Version 2.0 + * @link https://www.phpmyfaq.de + * @since 2026-04-18 + */ + +declare(strict_types=1); + +namespace phpMyFAQ\Auth\Oidc; + +use Closure; +use JsonException; +use RuntimeException; +use SensitiveParameter; +use Symfony\Contracts\HttpClient\Exception\ExceptionInterface; +use Symfony\Contracts\HttpClient\HttpClientInterface; + +final readonly class OidcIdTokenValidator +{ + public function __construct( + private HttpClientInterface $httpClient, + private ?Closure $currentTimeProvider = null, + ) { + } + + /** + * @return array + * @throws ExceptionInterface + */ + public function validate( + #[SensitiveParameter] + string $idToken, + OidcDiscoveryDocument $discoveryDocument, + string $expectedAudience, + string $expectedNonce, + ): array { + if ($idToken === '') { + return []; + } + + [$encodedHeader, $encodedPayload, $encodedSignature] = $this->splitToken($idToken); + $header = $this->decodeSegment($encodedHeader, 'header'); + $claims = $this->decodeSegment($encodedPayload, 'payload'); + + $this->validateClaims($claims, $discoveryDocument->issuer, $expectedAudience, $expectedNonce); + $this->validateSignature( + $encodedHeader . '.' . $encodedPayload, + $encodedSignature, + $header, + $discoveryDocument->jwksUri, + ); + + return $claims; + } + + /** + * @return array{0: string, 1: string, 2: string} + */ + private function splitToken(#[SensitiveParameter] string $idToken): array + { + $parts = explode('.', $idToken); + if (count($parts) !== 3 || $parts[0] === '' || $parts[1] === '' || $parts[2] === '') { + throw new RuntimeException('OIDC id_token is malformed'); + } + + return [$parts[0], $parts[1], $parts[2]]; + } + + /** + * @return array + */ + private function decodeSegment(string $segment, string $context): array + { + $decoded = $this->base64UrlDecode($segment); + + try { + $payload = json_decode($decoded, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); + } catch (JsonException $exception) { + throw new RuntimeException(sprintf('OIDC id_token %s is not valid JSON', $context), previous: $exception); + } + + if (!is_array($payload)) { + throw new RuntimeException(sprintf('OIDC id_token %s is not valid', $context)); + } + + return $payload; + } + + /** + * @param array $claims + */ + private function validateClaims( + array $claims, + string $expectedIssuer, + string $expectedAudience, + string $expectedNonce, + ): void { + $issuer = trim((string) ($claims['iss'] ?? '')); + if ($issuer !== '' && $issuer !== $expectedIssuer) { + throw new RuntimeException('OIDC issuer mismatch in id_token'); + } + + $audience = $claims['aud'] ?? null; + if ($audience !== null && !$this->audienceMatches($audience, $expectedAudience)) { + throw new RuntimeException('OIDC audience mismatch in id_token'); + } + + $authorizedParty = trim((string) ($claims['azp'] ?? '')); + if ($authorizedParty !== '' && $authorizedParty !== $expectedAudience) { + throw new RuntimeException('OIDC authorized party mismatch in id_token'); + } + + if (is_array($audience) && count($audience) > 1 && $authorizedParty === '') { + throw new RuntimeException('OIDC authorized party missing in id_token'); + } + + $nonce = trim((string) ($claims['nonce'] ?? '')); + if ($expectedNonce !== '' && $nonce !== '' && !hash_equals($expectedNonce, $nonce)) { + throw new RuntimeException('OIDC nonce mismatch in id_token'); + } + + $now = $this->getCurrentTimestamp(); + + if (array_key_exists('exp', $claims) && is_numeric($claims['exp']) && (int) $claims['exp'] < $now) { + throw new RuntimeException('OIDC id_token has expired'); + } + + if (array_key_exists('nbf', $claims) && is_numeric($claims['nbf']) && (int) $claims['nbf'] > $now) { + throw new RuntimeException('OIDC id_token is not valid yet'); + } + + if (array_key_exists('iat', $claims) && is_numeric($claims['iat']) && (int) $claims['iat'] > ($now + 60)) { + throw new RuntimeException('OIDC id_token issued-at time is in the future'); + } + } + + /** + * @param array $header + * @throws ExceptionInterface + */ + private function validateSignature( + string $signedPayload, + string $encodedSignature, + array $header, + string $jwksUri, + ): void { + $algorithm = (string) ($header['alg'] ?? ''); + $keyId = trim((string) ($header['kid'] ?? '')); + + $opensslAlgorithm = match ($algorithm) { + 'RS256' => OPENSSL_ALGO_SHA256, + 'RS384' => OPENSSL_ALGO_SHA384, + 'RS512' => OPENSSL_ALGO_SHA512, + default => throw new RuntimeException('Unsupported OIDC id_token signing algorithm'), + }; + + $jwks = $this->fetchJwks($jwksUri); + $key = $this->findKey($jwks, $keyId); + $publicKey = $this->createPemFromJwk($key); + $signature = $this->base64UrlDecode($encodedSignature); + + $verificationResult = openssl_verify($signedPayload, $signature, $publicKey, $opensslAlgorithm); + if ($verificationResult !== 1) { + throw new RuntimeException('OIDC id_token signature validation failed'); + } + } + + /** + * @return array + * @throws ExceptionInterface + */ + private function fetchJwks(string $jwksUri): array + { + $response = $this->httpClient->request('GET', $jwksUri); + $content = $response->getContent(false); + + if ($response->getStatusCode() >= 400) { + throw new RuntimeException(sprintf('OIDC JWKS request failed with status %d', $response->getStatusCode())); + } + + try { + $payload = json_decode($content, associative: true, depth: 512, flags: JSON_THROW_ON_ERROR); + } catch (JsonException $exception) { + throw new RuntimeException('OIDC JWKS response is not valid JSON', previous: $exception); + } + + if (!is_array($payload)) { + throw new RuntimeException('OIDC JWKS response is not valid'); + } + + return $payload; + } + + /** + * @param array $jwks + * @return array + */ + private function findKey(array $jwks, string $keyId): array + { + $keys = $jwks['keys'] ?? null; + if (!is_array($keys) || $keys === []) { + throw new RuntimeException('OIDC JWKS response does not contain any keys'); + } + + foreach ($keys as $key) { + if (!is_array($key)) { + continue; + } + + $candidateKeyId = trim((string) ($key['kid'] ?? '')); + if ($keyId !== '' && $candidateKeyId !== $keyId) { + continue; + } + + return $key; + } + + if ($keyId === '' && is_array($keys[0])) { + return $keys[0]; + } + + throw new RuntimeException('OIDC JWKS key could not be resolved'); + } + + /** + * @param array $key + */ + private function createPemFromJwk(array $key): string + { + $keyType = (string) ($key['kty'] ?? ''); + $modulus = (string) ($key['n'] ?? ''); + $exponent = (string) ($key['e'] ?? ''); + + if ($keyType !== 'RSA' || $modulus === '' || $exponent === '') { + throw new RuntimeException('OIDC JWK is not a supported RSA key'); + } + + $rsaPublicKey = $this->asn1Sequence( + $this->asn1Integer($this->base64UrlDecode($modulus)) + . $this->asn1Integer($this->base64UrlDecode($exponent)), + ); + + $subjectPublicKeyInfo = $this->asn1Sequence( + $this->asn1Sequence($this->asn1ObjectIdentifier("\x2a\x86\x48\x86\xf7\x0d\x01\x01\x01") . $this->asn1Null()) + . $this->asn1BitString($rsaPublicKey), + ); + + return ( + "-----BEGIN PUBLIC KEY-----\n" + . chunk_split(base64_encode($subjectPublicKeyInfo), length: 64, separator: "\n") + . "-----END PUBLIC KEY-----\n" + ); + } + + private function asn1Sequence(string $value): string + { + return "\x30" . $this->asn1Length(strlen($value)) . $value; + } + + private function asn1Integer(string $value): string + { + if ($value === '') { + $value = "\x00"; + } + + if (ord($value[0]) > 0x7f) { + $value = "\x00" . $value; + } + + return "\x02" . $this->asn1Length(strlen($value)) . $value; + } + + private function asn1ObjectIdentifier(string $value): string + { + return "\x06" . $this->asn1Length(strlen($value)) . $value; + } + + private function asn1Null(): string + { + return "\x05\x00"; + } + + private function asn1BitString(string $value): string + { + return "\x03" . $this->asn1Length(strlen($value) + 1) . "\x00" . $value; + } + + private function asn1Length(int $length): string + { + if ($length < 128) { + return chr($length); + } + + $encoded = ''; + while ($length > 0) { + $encoded = chr($length & 0xff) . $encoded; + $length >>= 8; + } + + return chr(0x80 | strlen($encoded)) . $encoded; + } + + private function base64UrlDecode(string $value): string + { + $padding = (4 - (strlen($value) % 4)) % 4; + $value .= str_repeat('=', $padding); + $decoded = base64_decode(strtr(string: $value, from: '-_', to: '+/'), strict: true); + + if ($decoded === false) { + throw new RuntimeException('OIDC token could not be base64url decoded'); + } + + return $decoded; + } + + private function audienceMatches(mixed $audience, string $expectedAudience): bool + { + if (is_string($audience)) { + return $audience === $expectedAudience; + } + + if (!is_array($audience)) { + return false; + } + + foreach ($audience as $entry) { + if (is_string($entry) && $entry === $expectedAudience) { + return true; + } + } + + return false; + } + + private function getCurrentTimestamp(): int + { + if ($this->currentTimeProvider instanceof Closure) { + return ($this->currentTimeProvider)(); + } + + return time(); + } +} diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcSession.php b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcSession.php index d8d33cf884..4f3757dc16 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcSession.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcSession.php @@ -59,4 +59,19 @@ public function clearAuthorizationState(): void $this->set(self::OIDC_NONCE, ''); $this->set(self::OIDC_PKCE_CODE, ''); } + + public function setIdToken(string $idToken): void + { + $this->set(self::OIDC_ID_ASSERTION, $idToken); + } + + public function getIdToken(): string + { + return (string) $this->get(self::OIDC_ID_ASSERTION); + } + + public function clearIdToken(): void + { + $this->set(self::OIDC_ID_ASSERTION, ''); + } } diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php index 14b132e6e4..8b95e17dad 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php @@ -89,11 +89,12 @@ public function callback(Request $request): Response [$auth, $oAuth, $entraIdSession] = $this->buildAuthContext(); - $code = Filter::filterVar($request->query->get('code'), FILTER_SANITIZE_SPECIAL_CHARS); - $error = Filter::filterVar($request->query->get('error_description'), FILTER_SANITIZE_SPECIAL_CHARS); + $code = Filter::filterVar($request->query->get('code'), FILTER_SANITIZE_SPECIAL_CHARS, ''); + $error = Filter::filterVar($request->query->get('error_description'), FILTER_SANITIZE_SPECIAL_CHARS, ''); if ($error !== '') { - return new Response($error); + $this->configuration->getLogger()->warning(sprintf('Azure callback error: %s', $error)); + return new RedirectResponse($this->configuration->getDefaultUrl()); } $redirect = new RedirectResponse($this->configuration->getDefaultUrl()); diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php index 4d9c12bebb..d624b605bc 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php @@ -25,6 +25,7 @@ use phpMyFAQ\Auth\Keycloak\KeycloakProviderConfigFactory; use phpMyFAQ\Auth\Oidc\OidcClient; use phpMyFAQ\Auth\Oidc\OidcDiscoveryService; +use phpMyFAQ\Auth\Oidc\OidcIdTokenValidator; use phpMyFAQ\Auth\Oidc\OidcPkceGenerator; use phpMyFAQ\Auth\Oidc\OidcSession; use phpMyFAQ\Enums\AuthenticationSourceType; @@ -38,18 +39,32 @@ final class KeycloakAuthenticationController extends AbstractFrontController { + private ?Closure $currentUserFactory = null; + private ?Closure $userFactory = null; + public function __construct( private readonly KeycloakProviderConfigFactory $providerConfigFactory, private readonly OidcDiscoveryService $discoveryService, private readonly OidcPkceGenerator $pkceGenerator, private readonly OidcSession $oidcSession, private readonly OidcClient $oidcClient, - private readonly ?Closure $currentUserFactory = null, - private readonly ?Closure $userFactory = null, + private readonly OidcIdTokenValidator $idTokenValidator, ) { parent::__construct(); } + public function setCurrentUserFactory(?Closure $currentUserFactory): self + { + $this->currentUserFactory = $currentUserFactory; + return $this; + } + + public function setUserFactory(?Closure $userFactory): self + { + $this->userFactory = $userFactory; + return $this; + } + #[Route(path: '/auth/keycloak/authorize', name: 'public.keycloak.authorize', methods: ['GET'])] public function authorize(): RedirectResponse { @@ -97,11 +112,17 @@ public function logout(): RedirectResponse return new RedirectResponse($this->configuration->getDefaultUrl()); } + $idToken = $this->oidcSession->getIdToken(); + + $this->currentUser->deleteFromSession(); + $this->oidcSession->clearIdToken(); + $discoveryDocument = $this->discoveryService->discover($providerConfig); - $logoutUrl = $this->oidcClient->buildLogoutUrl($providerConfig, $discoveryDocument, ''); + $logoutUrl = $this->oidcClient->buildLogoutUrl($providerConfig, $discoveryDocument, $idToken); return new RedirectResponse($logoutUrl ?? $this->configuration->getDefaultUrl()); } catch (Exception) { + $this->currentUser->deleteFromSession(); return new RedirectResponse($this->configuration->getDefaultUrl()); } } @@ -117,12 +138,13 @@ public function callback(Request $request): Response return new RedirectResponse($this->configuration->getDefaultUrl()); } - $code = Filter::filterVar($request->query->get('code'), FILTER_SANITIZE_SPECIAL_CHARS); - $state = Filter::filterVar($request->query->get('state'), FILTER_SANITIZE_SPECIAL_CHARS); - $error = Filter::filterVar($request->query->get('error_description'), FILTER_SANITIZE_SPECIAL_CHARS); + $code = Filter::filterVar($request->query->get('code'), FILTER_SANITIZE_SPECIAL_CHARS, ''); + $state = Filter::filterVar($request->query->get('state'), FILTER_SANITIZE_SPECIAL_CHARS, ''); + $error = Filter::filterVar($request->query->get('error_description'), FILTER_SANITIZE_SPECIAL_CHARS, ''); if ($error !== '') { - return new Response($error); + $this->configuration->getLogger()->warning(sprintf('Keycloak callback error: %s', $error)); + return new RedirectResponse($this->configuration->getDefaultUrl()); } $redirect = new RedirectResponse($this->configuration->getDefaultUrl()); @@ -146,22 +168,45 @@ public function callback(Request $request): Response $code, $authorizationState['verifier'], ); + $this->idTokenValidator->validate( + (string) ($token['id_token'] ?? ''), + $discoveryDocument, + $providerConfig->client->clientId, + $authorizationState['nonce'], + ); $claims = $this->oidcClient->fetchUserInfo($discoveryDocument, (string) $token['access_token']); $login = $this->resolveLocalLogin($claims); - $auth = new AuthKeycloak($this->configuration, $providerConfig, $claims, $login); + $auth = new AuthKeycloak($this->configuration, $providerConfig, $claims, $login, $this->userFactory); if (!$auth->isValidLogin($login)) { + $this->configuration->getLogger()->warning(sprintf('Keycloak login not valid for user: %s', $login)); $this->oidcSession->clearAuthorizationState(); - return new Response('Login not valid.'); + return $redirect; } if (!$auth->checkCredentials($login, '')) { + $this->configuration + ->getLogger() + ->warning(sprintf('Keycloak credentials not valid for user: %s', $login)); $this->oidcSession->clearAuthorizationState(); - return new Response('Credentials not valid.'); + return $redirect; } $user = $this->getCurrentUserService(); - $user->getUserByLogin($login); + if (!$user->getUserByLogin($login)) { + $this->configuration + ->getLogger() + ->warning(sprintf('Keycloak user lookup failed for login: %s', $login)); + $this->oidcSession->clearAuthorizationState(); + return $redirect; + } + + if (!$this->synchronizeKeycloakSubject($user, $claims)) { + $this->configuration->getLogger()->warning(sprintf('Keycloak subject mismatch for user: %s', $login)); + $this->oidcSession->clearAuthorizationState(); + return $redirect; + } + $user->setLoggedIn(true); $user->setAuthSource(AuthenticationSourceType::AUTH_KEYCLOAK->value); $user->updateSessionId(true); @@ -177,17 +222,16 @@ public function callback(Request $request): Response ]); $user->setSuccess(true); $this->oidcSession->clearAuthorizationState(); + $this->oidcSession->setIdToken((string) ($token['id_token'] ?? '')); return $redirect; } catch (Exception $exception) { + $this->configuration->getLogger()->error(sprintf('Keycloak login failed: %s', $exception->getMessage()), [ + 'exception' => $exception, + ]); $this->oidcSession->clearAuthorizationState(); - return new Response(sprintf( - 'Keycloak login failed: %s at line %d at %s', - $exception->getMessage(), - $exception->getLine(), - $exception->getFile(), - )); + return new RedirectResponse($this->configuration->getDefaultUrl()); } } @@ -198,6 +242,14 @@ private function resolveLocalLogin(array $claims): string $email = trim((string) ($claims['email'] ?? '')); $subject = trim((string) ($claims['sub'] ?? '')); + if ($subject !== '') { + $user = $this->createUser(); + $userId = $user->getUserIdByKeycloakSub($subject); + if ($userId > 0 && $user->getUserById($userId)) { + return $user->getLogin(); + } + } + if ($preferredUsername !== '' && $this->createUser()->getUserByLogin($preferredUsername, false)) { return $preferredUsername; } @@ -221,6 +273,26 @@ private function resolveLocalLogin(array $claims): string return $subject; } + /** @param array $claims */ + private function synchronizeKeycloakSubject(CurrentUser $user, array $claims): bool + { + $subject = trim((string) ($claims['sub'] ?? '')); + if ($subject === '') { + return true; + } + + $linkedSubject = trim((string) $user->getUserData('keycloak_sub')); + if ($linkedSubject !== '' && !hash_equals($linkedSubject, $subject)) { + return false; + } + + if ($linkedSubject === '') { + return $user->setUserData(['keycloak_sub' => $subject]); + } + + return true; + } + private function getCurrentUserService(): CurrentUser { if ($this->currentUserFactory instanceof Closure) { diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/Mysqli.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/Mysqli.php index 83755df741..39441e780c 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/Mysqli.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/Mysqli.php @@ -340,6 +340,7 @@ class Mysqli extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible INT(1) NULL DEFAULT 0, twofactor_enabled INT(1) NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoMysql.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoMysql.php index 4cda8fcc24..348ec549fe 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoMysql.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoMysql.php @@ -339,6 +339,7 @@ class PdoMysql extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible INT(1) NULL DEFAULT 0, twofactor_enabled INT(1) NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoPgsql.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoPgsql.php index 2a814e0abb..728b919628 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoPgsql.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoPgsql.php @@ -331,6 +331,7 @@ class PdoPgsql extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible SMALLINT NULL DEFAULT 0, twofactor_enabled SMALLINT NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlite.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlite.php index d2a710ba1b..17d045edcc 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlite.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlite.php @@ -329,6 +329,7 @@ class PdoSqlite extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible INT(1) NULL DEFAULT 0, twofactor_enabled INT(1) NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlsrv.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlsrv.php index f0ba5d6096..2195662e39 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlsrv.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/PdoSqlsrv.php @@ -329,6 +329,7 @@ class PdoSqlsrv extends Database implements DriverInterface last_modified NVARCHAR(14) NULL, display_name NVARCHAR(128) NULL, email NVARCHAR(128) NULL, + keycloak_sub NVARCHAR(255) NULL, is_visible INTEGER NULL DEFAULT 0, twofactor_enabled INTEGER NULL DEFAULT 0, secret NVARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/Pgsql.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/Pgsql.php index d51bb88c19..c8061147cc 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/Pgsql.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/Pgsql.php @@ -332,6 +332,7 @@ class Pgsql extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible SMALLINT NULL DEFAULT 0, twofactor_enabled SMALLINT NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlite3.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlite3.php index b90e82d13e..bc2901e31b 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlite3.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlite3.php @@ -330,6 +330,7 @@ class Sqlite3 extends Database implements DriverInterface last_modified VARCHAR(14) NULL, display_name VARCHAR(128) NULL, email VARCHAR(128) NULL, + keycloak_sub VARCHAR(255) NULL, is_visible INT(1) NULL DEFAULT 0, twofactor_enabled INT(1) NULL DEFAULT 0, secret VARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlsrv.php b/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlsrv.php index 276662cb06..baf5705279 100644 --- a/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlsrv.php +++ b/phpmyfaq/src/phpMyFAQ/Instance/Database/Sqlsrv.php @@ -330,6 +330,7 @@ class Sqlsrv extends Database implements DriverInterface last_modified NVARCHAR(14) NULL, display_name NVARCHAR(128) NULL, email NVARCHAR(128) NULL, + keycloak_sub NVARCHAR(255) NULL, is_visible INTEGER NULL DEFAULT 0, twofactor_enabled INTEGER NULL DEFAULT 0, secret NVARCHAR(128) NULL DEFAULT NULL)', diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Installation/DatabaseSchema.php b/phpmyfaq/src/phpMyFAQ/Setup/Installation/DatabaseSchema.php index 5dd1ba5302..83fad187f7 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Installation/DatabaseSchema.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Installation/DatabaseSchema.php @@ -691,6 +691,7 @@ public function faquserdata(): TableBuilder ->varchar('last_modified', 14) ->varchar('display_name', 128) ->varchar('email', 128) + ->varchar('keycloak_sub', 255) ->smallInteger('is_visible', true, 0) ->smallInteger('twofactor_enabled', true, 0) ->varchar('secret', 128); diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php index 92b66dc288..9deb97572d 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php @@ -37,7 +37,7 @@ public function getDependencies(): array public function getDescription(): string { - return 'Admin log hash columns, custom pages, chat messages, translation config, API rate limiting, queue jobs, mail provider config, API keys, OAuth2 tables'; + return 'Admin log hash columns, custom pages, chat messages, translation config, API rate limiting, queue jobs, mail provider config, API keys, OAuth2 tables, Keycloak subject storage'; } public function up(OperationRecorder $recorder): void @@ -218,6 +218,19 @@ public function up(OperationRecorder $recorder): void $recorder->addConfig('api.onlyPublicQuestions', 'true'); $recorder->addConfig('api.ignoreOrphanedFaqs', 'true'); $recorder->addConfig('api.rateLimit.requests', '100'); + + // Add durable Keycloak subject storage to user data + $columnType = $this->isSqlServer() ? 'NVARCHAR(255) NULL' : 'VARCHAR(255) NULL'; + + $recorder->addSql( + $this->addColumn('faquserdata', 'keycloak_sub', $columnType), + 'Add keycloak_sub column to faquserdata', + ); + + $recorder->addSql( + $this->createIndex('faquserdata', 'idx_faquserdata_keycloak_sub', 'keycloak_sub'), + 'Create keycloak_sub index on faquserdata', + ); $recorder->addConfig('api.rateLimit.interval', '3600'); $recorder->addConfig('queue.transport', 'database'); $recorder->addConfig('session.handler', 'files'); diff --git a/phpmyfaq/src/phpMyFAQ/User.php b/phpmyfaq/src/phpMyFAQ/User.php index f3f569d2f6..fa869c98be 100644 --- a/phpmyfaq/src/phpMyFAQ/User.php +++ b/phpmyfaq/src/phpMyFAQ/User.php @@ -810,6 +810,20 @@ public function getUserIdByEmail(string $email): int return $userData['user_id']; } + /** + * Returns the user ID from the given Keycloak subject. + */ + public function getUserIdByKeycloakSub(string $keycloakSub): int + { + if (!$this->userdata instanceof UserData) { + $this->userdata = new UserData($this->configuration); + } + + $userData = $this->userdata->fetchAll('keycloak_sub', $keycloakSub); + + return $userData['user_id']; + } + /** * Returns true or false for the visibility for the given email * address, if the user is not a registered user, the method diff --git a/phpmyfaq/src/phpMyFAQ/User/UserData.php b/phpmyfaq/src/phpMyFAQ/User/UserData.php index 2373daf2de..77141fbef6 100644 --- a/phpmyfaq/src/phpMyFAQ/User/UserData.php +++ b/phpmyfaq/src/phpMyFAQ/User/UserData.php @@ -73,7 +73,24 @@ public function get(mixed $field): mixed $this->userId, ); - $res = $this->configuration->getDb()->query($select); + try { + $res = $this->configuration->getDb()->query($select); + } catch (\Throwable) { + if ($singleReturn && $field === 'keycloak_sub') { + return ''; + } + + return false; + } + + if ($res === false) { + if ($singleReturn && $field === 'keycloak_sub') { + return ''; + } + + return false; + } + if ($this->configuration->getDb()->numRows($res) !== 1) { return false; } @@ -81,7 +98,7 @@ public function get(mixed $field): mixed $array = $this->configuration->getDb()->fetchArray($res); // Decode HTML entities in display_name for backward compatibility - if (array_key_exists('display_name', $array)) { + if (array_key_exists('display_name', $array) && is_string($array['display_name'])) { $array['display_name'] = html_entity_decode( $array['display_name'], ENT_QUOTES | ENT_HTML5 | ENT_SUBSTITUTE, @@ -89,7 +106,14 @@ public function get(mixed $field): mixed ); } - return $singleReturn && $field !== '*' ? $array[$field] : $array; + if ($singleReturn && $field !== '*') { + return match ($field) { + 'display_name', 'email', 'keycloak_sub', 'secret' => (string) ($array[$field] ?? ''), + default => $array[$field] ?? null, + }; + } + + return $array; } /** @@ -123,19 +147,39 @@ public function fetchAll(string $key, string $value): array { $select = sprintf( "SELECT - user_id, last_modified, display_name, email, is_visible, twofactor_enabled, secret + user_id, last_modified, display_name, email, keycloak_sub, is_visible, twofactor_enabled, secret FROM %sfaquserdata WHERE %s = '%s'", Database::getTablePrefix(), $key, $this->configuration->getDb()->escape($value), ); - $res = $this->configuration->getDb()->query($select); + try { + $res = $this->configuration->getDb()->query($select); + } catch (\Throwable) { + $res = false; + } + + if ($res === false) { + $select = sprintf( + "SELECT + user_id, last_modified, display_name, email, is_visible, twofactor_enabled, secret + FROM %sfaquserdata WHERE %s = '%s'", + Database::getTablePrefix(), + $key, + $this->configuration->getDb()->escape($value), + ); + $res = $this->configuration->getDb()->query($select); + } + if ($this->configuration->getDb()->numRows($res) !== 1) { return ['user_id' => -1]; } - return $this->data = $this->configuration->getDb()->fetchArray($res); + $this->data = $this->configuration->getDb()->fetchArray($res); + $this->data['keycloak_sub'] ??= ''; + + return $this->data; } /** @@ -188,6 +232,7 @@ public function load(int $userId): bool last_modified, display_name, email, + keycloak_sub, is_visible, twofactor_enabled, secret @@ -196,12 +241,34 @@ public function load(int $userId): bool WHERE user_id = %d', Database::getTablePrefix(), $this->userId); - $res = $this->configuration->getDb()->query($select); + try { + $res = $this->configuration->getDb()->query($select); + } catch (\Throwable) { + $res = false; + } + + if ($res === false) { + $select = sprintf(' + SELECT + last_modified, + display_name, + email, + is_visible, + twofactor_enabled, + secret + FROM + %sfaquserdata + WHERE + user_id = %d', Database::getTablePrefix(), $this->userId); + $res = $this->configuration->getDb()->query($select); + } + if ($this->configuration->getDb()->numRows($res) !== 1) { return false; } $this->data = $this->configuration->getDb()->fetchArray($res); + $this->data['keycloak_sub'] ??= ''; return true; } @@ -220,6 +287,7 @@ public function save(): bool last_modified = '%s', display_name = '%s', email = '%s', + keycloak_sub = '%s', is_visible = %d, twofactor_enabled = %d, secret = '%s' @@ -229,13 +297,46 @@ public function save(): bool date(format: 'YmdHis', timestamp: Request::createFromGlobals()->server->get('REQUEST_TIME')), $this->configuration->getDb()->escape((string) ($this->data['display_name'] ?? '')), $this->configuration->getDb()->escape((string) ($this->data['email'] ?? '')), + $this->configuration->getDb()->escape((string) ($this->data['keycloak_sub'] ?? '')), $this->data['is_visible'] ?? 0, $this->data['twofactor_enabled'] ?? 0, $this->data['secret'] ?? '', $this->userId, ); - $res = $this->configuration->getDb()->query($update); + try { + $res = $this->configuration->getDb()->query($update); + } catch (\Throwable) { + $res = false; + } + + if ($res === false) { + $update = sprintf( + " + UPDATE + %sfaquserdata + SET + last_modified = '%s', + display_name = '%s', + email = '%s', + is_visible = %d, + twofactor_enabled = %d, + secret = '%s' + WHERE + user_id = %d", + Database::getTablePrefix(), + date(format: 'YmdHis', timestamp: Request::createFromGlobals()->server->get('REQUEST_TIME')), + $this->configuration->getDb()->escape((string) ($this->data['display_name'] ?? '')), + $this->configuration->getDb()->escape((string) ($this->data['email'] ?? '')), + $this->data['is_visible'] ?? 0, + $this->data['twofactor_enabled'] ?? 0, + $this->data['secret'] ?? '', + $this->userId, + ); + + $res = $this->configuration->getDb()->query($update); + } + return (bool) $res; } diff --git a/phpmyfaq/src/services.php b/phpmyfaq/src/services.php index f02313abec..d6dfd100dd 100644 --- a/phpmyfaq/src/services.php +++ b/phpmyfaq/src/services.php @@ -36,6 +36,7 @@ use phpMyFAQ\Auth\Keycloak\KeycloakProviderConfigFactory; use phpMyFAQ\Auth\Oidc\OidcClient; use phpMyFAQ\Auth\Oidc\OidcDiscoveryService; +use phpMyFAQ\Auth\Oidc\OidcIdTokenValidator; use phpMyFAQ\Auth\Oidc\OidcPkceGenerator; use phpMyFAQ\Auth\Oidc\OidcProviderConfig; use phpMyFAQ\Auth\Oidc\OidcSession; @@ -312,6 +313,9 @@ $services->set('phpmyfaq.auth.oidc.discovery-service', OidcDiscoveryService::class)->args([ service('phpmyfaq.http-client'), ]); + $services->set('phpmyfaq.auth.oidc.id-token-validator', OidcIdTokenValidator::class)->args([ + service('phpmyfaq.http-client'), + ]); $services->set('phpmyfaq.auth.oidc.pkce-generator', OidcPkceGenerator::class); $services->set('phpmyfaq.auth.oidc.session', OidcSession::class)->args([ service('session'), @@ -951,6 +955,7 @@ service('phpmyfaq.auth.oidc.pkce-generator'), service('phpmyfaq.auth.oidc.session'), service('phpmyfaq.auth.oidc.client'), + service('phpmyfaq.auth.oidc.id-token-validator'), ]); $services->set(FrontendCategoryController::class, FrontendCategoryController::class)->args([ service('phpmyfaq.user.session'), diff --git a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php index 43b2d69b68..cfb54eea28 100644 --- a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php +++ b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php @@ -77,6 +77,7 @@ public function testCheckCredentialsAutoProvisionsUserWhenEnabled(): void ->with([ 'display_name' => 'John Doe', 'email' => 'john@example.com', + 'keycloak_sub' => 'subject-123', ]); $auth = new AuthKeycloak( @@ -86,6 +87,7 @@ public function testCheckCredentialsAutoProvisionsUserWhenEnabled(): void 'preferred_username' => 'john', 'name' => 'John Doe', 'email' => 'john@example.com', + 'sub' => 'subject-123', ], 'john', static fn(): User => $user, @@ -101,7 +103,8 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void ->method('get') ->willReturnCallback(static fn(string $item): mixed => match ($item) { 'keycloak.groupAutoAssign' => 'true', - 'keycloak.groupMapping' => '{"manage-users":"Administrators"}', + 'keycloak.groupMapping' => '{"manage-users":"Administrators","faq-editors":"faq-editors"}', + 'keycloak.clientId' => 'phpmyfaq', 'security.permLevel' => 'medium', default => null, }); @@ -117,6 +120,7 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void ->with([ 'display_name' => 'John Doe', 'email' => 'john@example.com', + 'keycloak_sub' => 'subject-123', ]); $user->expects($this->once())->method('getUserId')->willReturn(42); @@ -141,6 +145,7 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void 'preferred_username' => 'john', 'name' => 'John Doe', 'email' => 'john@example.com', + 'sub' => 'subject-123', 'realm_access' => [ 'roles' => ['manage-users'], ], diff --git a/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php b/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php new file mode 100644 index 0000000000..03ea5bd989 --- /dev/null +++ b/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php @@ -0,0 +1,227 @@ + */ + private array $jwk; + + protected function setUp(): void + { + parent::setUp(); + + set_error_handler(static fn(): bool => true); + try { + $privateKey = openssl_pkey_new([ + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + 'private_key_bits' => 2048, + ]); + } finally { + restore_error_handler(); + } + + self::assertInstanceOf(OpenSSLAsymmetricKey::class, $privateKey); + $details = openssl_pkey_get_details($privateKey); + self::assertIsArray($details); + self::assertIsArray($details['rsa'] ?? null); + + $this->privateKey = $privateKey; + $this->jwk = [ + 'kty' => 'RSA', + 'kid' => 'test-key', + 'alg' => 'RS256', + 'use' => 'sig', + 'n' => $this->base64UrlEncode($details['rsa']['n']), + 'e' => $this->base64UrlEncode($details['rsa']['e']), + ]; + } + + public function testValidateReturnsClaimsForValidIdToken(): void + { + $validator = $this->createValidator([ + 'keys' => [$this->jwk], + ], 1_700_000_000); + + $claims = $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'iat' => 1_699_999_990, + 'nbf' => 1_699_999_990, + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + + self::assertSame('https://sso.example.test/realms/phpmyfaq', $claims['iss']); + } + + public function testValidateRejectsExpiredToken(): void + { + $validator = $this->createValidator([ + 'keys' => [$this->jwk], + ], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token has expired')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'exp' => 1_699_999_999, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsTokenThatIsNotValidYet(): void + { + $validator = $this->createValidator([ + 'keys' => [$this->jwk], + ], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token is not valid yet')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'nbf' => 1_700_000_001, + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsFutureIssuedAtTime(): void + { + $validator = $this->createValidator([ + 'keys' => [$this->jwk], + ], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token issued-at time is in the future')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'iat' => 1_700_000_061, + 'exp' => 1_700_000_120, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsInvalidSignature(): void + { + set_error_handler(static fn(): bool => true); + try { + $otherKey = openssl_pkey_new([ + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + 'private_key_bits' => 2048, + ]); + } finally { + restore_error_handler(); + } + self::assertInstanceOf(OpenSSLAsymmetricKey::class, $otherKey); + + $validator = $this->createValidator([ + 'keys' => [$this->jwk], + ], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token signature validation failed')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'exp' => 1_700_000_060, + ], $otherKey), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + private function createDiscoveryDocument(): OidcDiscoveryDocument + { + return new OidcDiscoveryDocument( + issuer: 'https://sso.example.test/realms/phpmyfaq', + authorizationEndpoint: 'https://sso.example.test/auth', + tokenEndpoint: 'https://sso.example.test/token', + userInfoEndpoint: 'https://sso.example.test/userinfo', + jwksUri: 'https://sso.example.test/jwks', + endSessionEndpoint: 'https://sso.example.test/logout', + ); + } + + /** + * @param array $jwks + */ + private function createValidator(array $jwks, int $now): OidcIdTokenValidator + { + $httpClient = new MockHttpClient([ + new MockResponse(json_encode($jwks, JSON_THROW_ON_ERROR)), + ]); + + return new OidcIdTokenValidator($httpClient, static fn(): int => $now); + } + + /** + * @param array $claims + */ + private function signToken(array $claims, ?OpenSSLAsymmetricKey $signingKey = null): string + { + $header = [ + 'alg' => 'RS256', + 'typ' => 'JWT', + 'kid' => 'test-key', + ]; + + $encodedHeader = $this->base64UrlEncode(json_encode($header, JSON_THROW_ON_ERROR)); + $encodedPayload = $this->base64UrlEncode(json_encode($claims, JSON_THROW_ON_ERROR)); + $signingInput = $encodedHeader . '.' . $encodedPayload; + + $signature = ''; + openssl_sign($signingInput, $signature, $signingKey ?? $this->privateKey, OPENSSL_ALGO_SHA256); + + return $signingInput . '.' . $this->base64UrlEncode($signature); + } + + private function base64UrlEncode(string $value): string + { + return rtrim(strtr(base64_encode($value), '+/', '-_'), '='); + } +} diff --git a/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php index 80bfc9b576..0f72cff33b 100644 --- a/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php @@ -133,8 +133,8 @@ public function testCallbackReturnsErrorResponseWhenProviderErrorIsSet(): void $request = new Request(['error_description' => 'Denied by provider']); $response = $controller->callback($request); - $this->assertInstanceOf(Response::class, $response); - $this->assertSame('Denied by provider', $response->getContent()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('https://example.com/', $response->headers->get('Location')); } /** diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php index 107785dd32..7460dd11cc 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php @@ -4,9 +4,11 @@ namespace phpMyFAQ\Controller\Frontend; +use OpenSSLAsymmetricKey; use phpMyFAQ\Auth\Keycloak\KeycloakProviderConfigFactory; use phpMyFAQ\Auth\Oidc\OidcClient; use phpMyFAQ\Auth\Oidc\OidcDiscoveryService; +use phpMyFAQ\Auth\Oidc\OidcIdTokenValidator; use phpMyFAQ\Auth\Oidc\OidcPkceGenerator; use phpMyFAQ\Auth\Oidc\OidcSession; use phpMyFAQ\Configuration; @@ -17,22 +19,27 @@ use phpMyFAQ\User\CurrentUser; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; use PHPUnit\Framework\Attributes\CoversClass; +use PHPUnit\Framework\Attributes\UsesClass; use PHPUnit\Framework\Attributes\UsesNamespace; use PHPUnit\Framework\TestCase; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpFoundation\Request; -use Symfony\Component\HttpFoundation\Response; use Symfony\Component\HttpFoundation\Session\Session; use Symfony\Component\HttpFoundation\Session\Storage\MockArraySessionStorage; #[AllowMockObjectsWithoutExpectations] #[CoversClass(KeycloakAuthenticationController::class)] +#[UsesClass(OidcIdTokenValidator::class)] #[UsesNamespace('phpMyFAQ')] final class KeycloakAuthenticationControllerTest extends TestCase { private Configuration $configuration; + private OpenSSLAsymmetricKey $privateKey; + + /** @var array */ + private array $jwk; /** * @throws CoreException @@ -70,6 +77,31 @@ protected function setUp(): void $config['keycloak.autoProvision'] = 'true'; $config['keycloak.logoutRedirectUrl'] = 'https://faq.example.test/'; $property->setValue($this->configuration, $config); + + set_error_handler(static fn(): bool => true); + try { + $privateKey = openssl_pkey_new([ + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + 'private_key_bits' => 2048, + ]); + } finally { + restore_error_handler(); + } + + self::assertInstanceOf(OpenSSLAsymmetricKey::class, $privateKey); + $details = openssl_pkey_get_details($privateKey); + self::assertIsArray($details); + self::assertIsArray($details['rsa'] ?? null); + + $this->privateKey = $privateKey; + $this->jwk = [ + 'kty' => 'RSA', + 'kid' => 'test-key', + 'alg' => 'RS256', + 'use' => 'sig', + 'n' => $this->base64UrlEncode($details['rsa']['n']), + 'e' => $this->base64UrlEncode($details['rsa']['e']), + ]; } public function testAuthorizeReturnsRedirectResponse(): void @@ -104,18 +136,29 @@ public function testLogoutReturnsProviderLogoutResponse(): void ); } + /** + * @throws \Exception + */ public function testCallbackReturnsErrorResponseWhenProviderErrorIsSet(): void { $controller = $this->createController(); $response = $controller->callback(new Request(['error_description' => 'Denied by provider'])); - $this->assertInstanceOf(Response::class, $response); - $this->assertSame('Denied by provider', $response->getContent()); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame('https://example.com/', $response->headers->get('Location')); } public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void { + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]); + $session = new Session(new MockArraySessionStorage()); $session->start(); $oidcSession = new OidcSession($session); @@ -123,6 +166,8 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void $currentUser = $this->createMock(CurrentUser::class); $currentUser->expects($this->once())->method('getUserByLogin')->with('john')->willReturn(true); + $currentUser->expects($this->once())->method('getUserData')->with('keycloak_sub')->willReturn(''); + $currentUser->expects($this->once())->method('setUserData')->with(['keycloak_sub' => '123'])->willReturn(true); $currentUser->expects($this->once())->method('setLoggedIn')->with(true); $currentUser->expects($this->once())->method('setAuthSource')->with('keycloak'); $currentUser->expects($this->once())->method('updateSessionId')->with(true); @@ -135,7 +180,7 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void 'access_token' => 'access', 'code_verifier' => 'verifier-789', 'jwt' => [ - 'id_token' => 'header.payload.signature', + 'id_token' => $idToken, 'userinfo' => [ 'sub' => '123', 'preferred_username' => 'john', @@ -146,20 +191,81 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void ]); $currentUser->expects($this->once())->method('setSuccess')->with(true); + $authUser = $this->createMock(\phpMyFAQ\User::class); + $authUser->expects($this->exactly(2))->method('getUserByLogin')->with('john', false)->willReturn(true); + $controller = $this->createController( [ new MockResponse( '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', ), + new MockResponse('{"access_token":"access","refresh_token":"refresh","id_token":"' . $idToken . '"}'), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + new MockResponse( + '{"sub":"123","preferred_username":"john","email":"john@example.com","name":"John Doe"}', + ), + ], + $oidcSession, + static fn(): CurrentUser => $currentUser, + static fn(): \phpMyFAQ\User => $authUser, + ); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackResolvesLocalLoginFromStoredKeycloakSubject(): void + { + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]); + + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $resolverUser = $this->createMock(\phpMyFAQ\User::class); + $resolverUser->expects($this->once())->method('getUserIdByKeycloakSub')->with('subject-123')->willReturn(55); + $resolverUser->expects($this->once())->method('getUserById')->with(55)->willReturn(true); + $resolverUser->expects($this->once())->method('getLogin')->willReturn('linked-user'); + $resolverUser->expects($this->once())->method('getUserByLogin')->with('linked-user', false)->willReturn(true); + + $currentUser = $this->createMock(CurrentUser::class); + $currentUser->expects($this->once())->method('getUserByLogin')->with('linked-user')->willReturn(true); + $currentUser->expects($this->once())->method('getUserData')->with('keycloak_sub')->willReturn('subject-123'); + $currentUser->expects($this->once())->method('setLoggedIn')->with(true); + $currentUser->expects($this->once())->method('setAuthSource')->with('keycloak'); + $currentUser->expects($this->once())->method('updateSessionId')->with(true); + $currentUser->expects($this->once())->method('saveToSession'); + $currentUser->expects($this->never())->method('setUserData'); + $currentUser->expects($this->once())->method('setTokenData'); + $currentUser->expects($this->once())->method('setSuccess')->with(true); + + $controller = $this->createController( + [ new MockResponse( - '{"access_token":"access","refresh_token":"refresh","id_token":"header.payload.signature"}', + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', ), + new MockResponse('{"access_token":"access","refresh_token":"refresh","id_token":"' . $idToken . '"}'), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), new MockResponse( - '{"sub":"123","preferred_username":"john","email":"john@example.com","name":"John Doe"}', + '{"sub":"subject-123","preferred_username":"john","email":"john@example.com","name":"John Doe"}', ), ], $oidcSession, static fn(): CurrentUser => $currentUser, + static fn(): \phpMyFAQ\User => $resolverUser, ); $response = $controller->callback(new Request([ @@ -167,6 +273,238 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void 'state' => 'state-123', ])); + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + } + + public function testCallbackReturnsFailureWhenStoredKeycloakSubjectDoesNotMatch(): void + { + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]); + + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $currentUser = $this->createMock(CurrentUser::class); + $currentUser->expects($this->once())->method('getUserByLogin')->with('john')->willReturn(true); + $currentUser + ->expects($this->once()) + ->method('getUserData') + ->with('keycloak_sub') + ->willReturn('different-subject'); + $currentUser->expects($this->never())->method('setUserData'); + $currentUser->expects($this->never())->method('setLoggedIn'); + $currentUser->expects($this->never())->method('setAuthSource'); + $currentUser->expects($this->never())->method('updateSessionId'); + $currentUser->expects($this->never())->method('saveToSession'); + $currentUser->expects($this->never())->method('setTokenData'); + $currentUser->expects($this->never())->method('setSuccess'); + + $authUser = $this->createMock(\phpMyFAQ\User::class); + $authUser->expects($this->exactly(2))->method('getUserByLogin')->with('john', false)->willReturn(true); + + $controller = $this->createController( + [ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse('{"access_token":"access","refresh_token":"refresh","id_token":"' . $idToken . '"}'), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + new MockResponse( + '{"sub":"subject-123","preferred_username":"john","email":"john@example.com","name":"John Doe"}', + ), + ], + $oidcSession, + static fn(): CurrentUser => $currentUser, + static fn(): \phpMyFAQ\User => $authUser, + ); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackReturnsFailureWhenIdTokenIssuerDoesNotMatch(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access","refresh_token":"refresh","id_token":"' + . $this->signToken([ + 'iss' => 'https://issuer.invalid/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]) + . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackReturnsFailureWhenIdTokenAudienceDoesNotMatch(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access","refresh_token":"refresh","id_token":"' + . $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['different-client'], + 'azp' => 'different-client', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]) + . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackReturnsFailureWhenIdTokenNonceDoesNotMatch(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access","refresh_token":"refresh","id_token":"' + . $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'unexpected-nonce', + 'exp' => time() + 300, + ]) + . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackReturnsFailureWhenIdTokenAuthorizedPartyDoesNotMatch(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access","refresh_token":"refresh","id_token":"' + . $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq', 'account'], + 'azp' => 'different-client', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]) + . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + + public function testCallbackReturnsFailureWhenIdTokenAuthorizedPartyIsMissingForMultipleAudiences(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access","refresh_token":"refresh","id_token":"' + . $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq', 'account'], + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]) + . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + $this->assertInstanceOf(RedirectResponse::class, $response); $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); $this->assertSame('', $oidcSession->getAuthorizationState()['state']); @@ -197,18 +535,50 @@ private function createController( array $responses = [], ?OidcSession $oidcSession = null, ?\Closure $currentUserFactory = null, + ?\Closure $userFactory = null, ): KeycloakAuthenticationController { $httpClient = new MockHttpClient($responses); $session = new Session(new MockArraySessionStorage()); $session->start(); - return new KeycloakAuthenticationController( + $controller = new KeycloakAuthenticationController( new KeycloakProviderConfigFactory($this->configuration), new OidcDiscoveryService($httpClient), new OidcPkceGenerator(), $oidcSession ?? new OidcSession($session), new OidcClient($httpClient), - $currentUserFactory, + new OidcIdTokenValidator($httpClient), ); + + $controller->setCurrentUserFactory($currentUserFactory); + $controller->setUserFactory($userFactory); + + return $controller; + } + + /** + * @param array $claims + */ + private function signToken(array $claims): string + { + $header = [ + 'alg' => 'RS256', + 'typ' => 'JWT', + 'kid' => 'test-key', + ]; + + $encodedHeader = $this->base64UrlEncode(json_encode($header, JSON_THROW_ON_ERROR)); + $encodedPayload = $this->base64UrlEncode(json_encode($claims, JSON_THROW_ON_ERROR)); + $signingInput = $encodedHeader . '.' . $encodedPayload; + + $signature = ''; + openssl_sign($signingInput, $signature, $this->privateKey, OPENSSL_ALGO_SHA256); + + return $signingInput . '.' . $this->base64UrlEncode($signature); + } + + private function base64UrlEncode(string $value): string + { + return rtrim(strtr(base64_encode($value), '+/', '-_'), '='); } } diff --git a/tests/phpMyFAQ/User/UserDataTest.php b/tests/phpMyFAQ/User/UserDataTest.php index 772a51efb4..522cb768ad 100644 --- a/tests/phpMyFAQ/User/UserDataTest.php +++ b/tests/phpMyFAQ/User/UserDataTest.php @@ -71,7 +71,7 @@ public function testFetchAll(): void $this->database->method('fetchArray')->willReturn(['user_id' => 1]); $result = $this->userData->fetchAll('key', 'value'); - $this->assertEquals(['user_id' => 1], $result); + $this->assertEquals(['user_id' => 1, 'keycloak_sub' => ''], $result); } public function testFetchAllReturnsDefaultArrayWhenNoResultExists(): void diff --git a/tests/phpMyFAQ/UserTest.php b/tests/phpMyFAQ/UserTest.php index c0188a53b3..310553ae76 100644 --- a/tests/phpMyFAQ/UserTest.php +++ b/tests/phpMyFAQ/UserTest.php @@ -583,14 +583,19 @@ public function testGetUserDataSetUserDataAndEmailHelpersDelegateToUserData(): v $this->userData->expects($this->once())->method('load')->with(11); $this->userData->expects($this->once())->method('set')->with(['display_name'], ['Thorsten'])->willReturn(true); $this->userData - ->expects($this->exactly(2)) + ->expects($this->exactly(3)) ->method('fetchAll') - ->willReturnOnConsecutiveCalls(['user_id' => 11, 'is_visible' => true], ['user_id' => 12]); + ->willReturnOnConsecutiveCalls( + ['user_id' => 11, 'is_visible' => true], + ['user_id' => 12], + ['user_id' => 13], + ); $this->assertSame('Thorsten', $this->user->getUserData('display_name')); $this->assertTrue($this->user->setUserData(['display_name' => 'Thorsten'])); $this->assertSame(11, $this->user->getUserIdByEmail('thorsten@example.com')); $this->assertTrue($this->user->getUserVisibilityByEmail('anonymous@example.com')); + $this->assertSame(13, $this->user->getUserIdByKeycloakSub('keycloak-sub-123')); } public function testGetUserDataCreatesUserDataWhenUnset(): void From 45866eabb8bb172a640200a1b27ed968ece77808 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sat, 18 Apr 2026 19:06:08 +0200 Subject: [PATCH 06/14] fix: corrected tests --- .../Controller/Frontend/AzureAuthenticationControllerTest.php | 2 +- .../Frontend/KeycloakAuthenticationControllerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php index 0f72cff33b..013a5ee74d 100644 --- a/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/AzureAuthenticationControllerTest.php @@ -134,7 +134,7 @@ public function testCallbackReturnsErrorResponseWhenProviderErrorIsSet(): void $response = $controller->callback($request); $this->assertInstanceOf(RedirectResponse::class, $response); - $this->assertSame('https://example.com/', $response->headers->get('Location')); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); } /** diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php index 7460dd11cc..c6dbb212ab 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php @@ -146,7 +146,7 @@ public function testCallbackReturnsErrorResponseWhenProviderErrorIsSet(): void $response = $controller->callback(new Request(['error_description' => 'Denied by provider'])); $this->assertInstanceOf(RedirectResponse::class, $response); - $this->assertSame('https://example.com/', $response->headers->get('Location')); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); } public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void From 558d2d19a67d123fcd3ba867f32004398dbbf3e3 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 10:42:18 +0200 Subject: [PATCH 07/14] feat: add keycloak oidc support and docs verification --- .github/workflows/build.yml | 11 ++ composer.json | 1 + docs/administration.md | 43 ++++- docs/configuration.md | 5 +- docs/keycloak.md | 127 ++++++++++++ mkdocs.yml | 16 +- package.json | 1 + .../src/configuration/configuration.test.ts | 1 + phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php | 44 ++++- .../Api/ConfigurationTabController.php | 1 + .../KeycloakAuthenticationController.php | 34 +++- .../phpMyFAQ/Permission/MediumPermission.php | 15 ++ .../Permission/MediumPermissionRepository.php | 22 +++ .../Setup/Installation/DefaultDataSeeder.php | 1 + .../Migration/Versions/Migration420Alpha.php | 1 + phpmyfaq/translations/language_de.php | 1 + phpmyfaq/translations/language_en.php | 1 + tests/phpMyFAQ/Auth/AuthKeycloakTest.php | 66 +++++++ tests/phpMyFAQ/Auth/Oidc/OidcSessionTest.php | 11 ++ .../Api/ConfigurationTabControllerTest.php | 1 + .../AuthenticationControllerTest.php | 52 +++++ .../AuthenticationControllerWebTest.php | 24 ++- .../AuthenticationControllerWebTest.php | 30 +++ .../KeycloakAuthenticationControllerTest.php | 42 +++- ...eycloakAuthenticationControllerWebTest.php | 181 ++++++++++++++++++ .../MediumPermissionRepositoryTest.php | 7 + .../Permission/MediumPermissionTest.php | 18 ++ tests/phpMyFAQ/TranslationTest.php | 5 +- 28 files changed, 725 insertions(+), 37 deletions(-) create mode 100644 docs/keycloak.md create mode 100644 tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 269bb6c0ba..85a6015fd3 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -62,6 +62,14 @@ jobs: package-manager-cache: false node-version: 24 + - name: Setup Python for docs build + uses: actions/setup-python@v6 + with: + python-version: '3.x' + + - name: Install MkDocs + run: python -m pip install --upgrade pip mkdocs + - name: Setup pnpm uses: pnpm/action-setup@v6 with: @@ -90,3 +98,6 @@ jobs: - name: Run Vitest tests run: pnpm test + + - name: Build documentation + run: mkdocs build --strict diff --git a/composer.json b/composer.json index 497577392a..a64f287aea 100644 --- a/composer.json +++ b/composer.json @@ -135,6 +135,7 @@ "test:coverage": "./phpmyfaq/src/libs/bin/phpunit --coverage-text", "test:coverage-html": "./phpmyfaq/src/libs/bin/phpunit --coverage-html coverage", "bench": "./phpmyfaq/src/libs/bin/phpbench run tests/Benchmarks --report=aggregate", + "docs:build": "mkdocs build --strict", "version:get": "php scripts/get-version.php", "version:sync": "pnpm version $(php scripts/get-version.php) --no-git-tag-version --allow-same-version", "version:check": "php scripts/check-version.php" diff --git a/docs/administration.md b/docs/administration.md index 0270aa9a1c..817556db3e 100644 --- a/docs/administration.md +++ b/docs/administration.md @@ -499,7 +499,7 @@ Navigate to the **Configuration → Translation** tab to configure your translat - **Amazon**: AWS Access Key ID, Secret Access Key, and region - **LibreTranslate**: Server URL and optional API key -For detailed setup instructions for each provider, see the [AI Translation Guide](9-ai-translation.md). +For detailed setup instructions for each provider, see the [AI Translation Guide](ai-translation.md). #### 5.2.13.3 Translating Content @@ -596,7 +596,7 @@ The AI will translate: - Re-translate if formatting is broken For comprehensive documentation, see: -- [Complete AI Translation Guide](9-ai-translation.md) - Full documentation +- [Complete AI Translation Guide](ai-translation.md) - Full documentation ## 5.3 Statistics @@ -791,7 +791,7 @@ To back up the whole data located on your web server, you can run our simple bac Here you can edit the general, FAQ specific, search, spam protection, spam control center, SEO related, layout settings, Mail setup for SMTP, API settings, online update settings, and if enabled, LDAP configuration of phpMyFAQ. -You can find a detailed description of all settings in the [Configuration key reference](8-configuration.md). +You can find a detailed description of all settings in the [Configuration key reference](configuration.md). ### 5.6.2 FAQ Multi-sites @@ -879,14 +879,19 @@ phpMyFAQ can integrate with external identity providers for administrator and fr Keycloak support uses OpenID Connect Authorization Code flow with PKCE. You can enable it in the administration under `Configuration` -> `Security` -> `Keycloak`. +For a worked configuration example, see the dedicated [Keycloak Integration guide](keycloak.md). Recommended Keycloak client settings: - Client type: confidential - Standard flow enabled - Direct access grants disabled unless you need them for other tools +- PKCE code challenge method: `S256` +- Root URL: `https://faq.example.com/` +- Home URL: `https://faq.example.com/` - Valid redirect URI: `https://faq.example.com/auth/keycloak/callback` - Valid post logout redirect URI: `https://faq.example.com/` +- Web origin: `https://faq.example.com` Minimum phpMyFAQ configuration: @@ -897,27 +902,47 @@ Minimum phpMyFAQ configuration: 5. Set the `Client secret` 6. Set the `Redirect URI` to your phpMyFAQ callback URL 7. Keep `Scopes` at least on `openid profile email` +8. Optionally set `Logout redirect URL` to the page users should see after provider logout + +Example phpMyFAQ values for a production setup: + +- `keycloak.baseUrl`: `https://sso.example.com` +- `keycloak.realm`: `faq` +- `keycloak.clientId`: `phpmyfaq` +- `keycloak.redirectUri`: `https://faq.example.com/auth/keycloak/callback` +- `keycloak.scopes`: `openid profile email` +- `keycloak.logoutRedirectUrl`: `https://faq.example.com/` Optional settings: - Enable automatic provisioning if phpMyFAQ should create local users on first successful Keycloak login - Enable automatic group assignment if phpMyFAQ should assign local groups from Keycloak roles +- Enable group synchronization on login if phpMyFAQ should remove stale memberships for mapped Keycloak groups - Add a JSON role-to-group mapping if Keycloak role names should map to different phpMyFAQ group names - Set a logout redirect URL if users should return to a specific page after provider logout +- Use a JSON mapping such as `{"admin":"Administrators","faq-editors":"Editors"}` if Keycloak role names and phpMyFAQ group names differ phpMyFAQ resolves users in this order: -1. preferred username from Keycloak -2. existing user by email address -3. automatic provisioning if enabled +1. existing user linked by stored Keycloak subject (`sub`) +2. preferred username from Keycloak +3. existing user by email address +4. automatic provisioning if enabled If automatic provisioning is disabled, users must already exist in phpMyFAQ before they can sign in with Keycloak. -Group assignment is additive in the current implementation: +Group assignment behavior: -- mapped or unmapped Keycloak roles can create phpMyFAQ groups automatically +- only roles listed in the JSON mapping are managed by phpMyFAQ - matched groups are added to the user on login -- existing phpMyFAQ group memberships are not removed automatically +- if group synchronization on login is enabled, stale memberships for mapped groups are removed during login +- phpMyFAQ groups outside the configured Keycloak mapping are left untouched + +Troubleshooting: + +- If login works but logout does not return to phpMyFAQ, verify `Valid post logout redirect URI` in Keycloak and `keycloak.logoutRedirectUrl` in phpMyFAQ +- If users are created but not added to groups, make sure permission level `medium` is enabled and the Keycloak roles actually match your JSON mapping keys +- If an existing user cannot log in, check whether the stored Keycloak subject (`sub`) is already linked to a different account ### 5.7.2 Using Microsoft Entra ID diff --git a/docs/configuration.md b/docs/configuration.md index e499e6cf8c..0daad4e43f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -133,8 +133,9 @@ each setting controls. These values are set during installation and can be chang | `keycloak.scopes` | Scopes | `openid profile email` | Space-separated scopes requested during login. | | `keycloak.autoProvision` | Automatically create phpMyFAQ users on first Keycloak login | `false` | When enabled, phpMyFAQ creates a local user automatically if no matching account exists yet. | | `keycloak.groupAutoAssign` | Automatically assign phpMyFAQ groups from Keycloak roles | `false` | When enabled and permission level `medium` is active, phpMyFAQ assigns users to groups derived from Keycloak roles on login. | -| `keycloak.groupMapping` | Role to group mapping | *(empty)* | JSON object mapping Keycloak role names to phpMyFAQ group names, for example `{"admin":"Administrators"}`. Unmapped roles keep their original name. | -| `keycloak.logoutRedirectUrl` | Logout redirect URL | *(empty)* | URL users should be redirected to after logging out from Keycloak. | +| `keycloak.groupSyncOnLogin` | Synchronize mapped phpMyFAQ groups on login | `false` | When enabled, phpMyFAQ also removes stale memberships for groups managed by the Keycloak role mapping during login. | +| `keycloak.groupMapping` | Role to group mapping | *(empty)* | JSON object mapping Keycloak role names to phpMyFAQ group names, for example `{"admin":"Administrators","faq-editors":"Editors"}`. Only mapped roles are managed for assignment and synchronization. | +| `keycloak.logoutRedirectUrl` | Logout redirect URL | *(empty)* | URL users should be redirected to after logging out from Keycloak, for example `https://faq.example.com/`. | | `security.enableGoogleReCaptchaV2` | Enable Invisible Google ReCAPTCHA v2 | `false` | Enables Google reCAPTCHA v2 to protect forms from spam and abuse. | | `security.googleReCaptchaV2SiteKey` | Google ReCAPTCHA v2 site key | *(empty)* | The site key from your Google reCAPTCHA v2 registration. | | `security.googleReCaptchaV2SecretKey` | Google ReCAPTCHA v2 secret key | *(empty)* | The secret key from your Google reCAPTCHA v2 registration. | diff --git a/docs/keycloak.md b/docs/keycloak.md new file mode 100644 index 0000000000..e3bec0783d --- /dev/null +++ b/docs/keycloak.md @@ -0,0 +1,127 @@ +# Keycloak Integration + +phpMyFAQ supports Keycloak as an OpenID Connect provider for frontend and administration logins. +The integration uses Authorization Code flow with PKCE. + +## 1. Prerequisites + +Before you configure phpMyFAQ, make sure you have: + +- a reachable Keycloak server, for example `https://sso.example.com` +- a realm for phpMyFAQ users, for example `faq` +- a confidential client for phpMyFAQ +- the public URL of your phpMyFAQ installation, for example `https://faq.example.com/` + +## 2. Recommended Keycloak client settings + +Recommended client settings for a phpMyFAQ installation at `https://faq.example.com/`: + +- Client type: confidential +- Standard flow enabled +- Direct access grants are disabled unless required for another integration +- Service accounts disabled +- PKCE code challenge method: `S256` +- Root URL: `https://faq.example.com/` +- Home URL: `https://faq.example.com/` +- Valid redirect URIs: + - `https://faq.example.com/auth/keycloak/callback` +- Valid post logout redirect URIs: + - `https://faq.example.com/` +- Web origins: + - `https://faq.example.com` + +## 3. phpMyFAQ configuration + +In phpMyFAQ, open: + +- `Configuration` +- `Security` +- `Keycloak` + +Typical values: + +- `keycloak.enable`: `true` +- `keycloak.baseUrl`: `https://sso.example.com` +- `keycloak.realm`: `faq` +- `keycloak.clientId`: `phpmyfaq` +- `keycloak.clientSecret`: `` +- `keycloak.redirectUri`: `https://faq.example.com/auth/keycloak/callback` +- `keycloak.scopes`: `openid profile email` +- `keycloak.logoutRedirectUrl`: `https://faq.example.com/` + +Optional user and group settings: + +- `keycloak.autoProvision`: `true` +- `keycloak.groupAutoAssign`: `true` +- `keycloak.groupSyncOnLogin`: `true` +- `keycloak.groupMapping`: `{"admin":"Administrators","faq-editors":"Editors"}` + +## 4. User resolution and account linking + +phpMyFAQ resolves Keycloak users in this order: + +1. existing user linked by stored Keycloak subject (`sub`) +2. preferred username from Keycloak +3. existing user by email address +4. automatic provisioning, if enabled + +The stored Keycloak subject is the durable link between a local phpMyFAQ account and the external identity. + +If automatic provisioning is disabled, users must already exist in phpMyFAQ before they can sign in. + +## 5. Group mapping behavior + +Group handling is intentionally conservative: + +- only roles listed in `keycloak.groupMapping` are managed by phpMyFAQ +- mapped groups are added on login when `keycloak.groupAutoAssign` is enabled +- stale memberships are removed only for mapped groups when `keycloak.groupSyncOnLogin` is enabled +- phpMyFAQ groups outside the configured mapping are left untouched + +Example mapping: + +```json +{ + "admin": "Administrators", + "faq-editors": "Editors" +} +``` + +This means: + +- Keycloak role `admin` maps to phpMyFAQ group `Administrators` +- Keycloak role `faq-editors` maps to phpMyFAQ group `Editors` + +## 6. Logout behavior + +phpMyFAQ logs the user out locally and then redirects to Keycloak logout when: + +- Keycloak sign-in is enabled +- the current user is authenticated through Keycloak + +For a reliable provider logout: + +- set `keycloak.logoutRedirectUrl` in phpMyFAQ +- make sure the same URL is listed as a valid post-logout redirect URI in Keycloak + +## 7. Troubleshooting + +If login works but logout does not return to phpMyFAQ: + +- verify `keycloak.logoutRedirectUrl` +- verify the matching valid post-logout redirect URI in Keycloak + +If users are created but not assigned to groups: + +- verify permission level `medium` +- verify `keycloak.groupAutoAssign` is enabled +- verify the Keycloak role names exactly match the JSON mapping keys + +If group synchronization removes the wrong memberships: + +- check `keycloak.groupMapping` +- remember that only mapped groups are managed + +If an existing user cannot log in: + +- check whether the stored Keycloak subject (`sub`) is already linked to another local account diff --git a/mkdocs.yml b/mkdocs.yml index 89576bee33..3f5bff899a 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -2,6 +2,9 @@ site_name: phpMyFAQ v4.2 theme: name: readthedocs highlightjs: true +not_in_nav: | + /s3-storage-checklist.md + /keys/** extra_css: - css/custom.css plugins: @@ -15,9 +18,10 @@ nav: - 6. Use phpMyFAQ: 'usage.md' - 7. Manage phpMyFAQ: 'administration.md' - 8. Configuration reference: 'configuration.md' - - 9. AI-Assisted Translation feature: 'ai-translation.md' - - 10. Developer documentation: 'development.md' - - 11. Plugins: 'plugins.md' - - 12. MCP Server: 'mcp-server.md' - - 13. Release: 'release.md' - - 14. Thank you!: 'thank-you.md' + - 9. Keycloak Integration: 'keycloak.md' + - 10. AI-Assisted Translation feature: 'ai-translation.md' + - 11. Developer documentation: 'development.md' + - 12. Plugins: 'plugins.md' + - 13. MCP Server: 'mcp-server.md' + - 14. Release: 'release.md' + - 15. Thank you!: 'thank-you.md' diff --git a/package.json b/package.json index e896117cb6..6adb64dc95 100644 --- a/package.json +++ b/package.json @@ -38,6 +38,7 @@ "release:artifacts": "./scripts/prepare-release-artifacts.sh", "release:sign": "./scripts/sign-release-artifacts.sh", "sbom": "./scripts/generate-sbom.sh", + "docs:build": "mkdocs build --strict", "eslint": "eslint .", "lint": "prettier --check .", "lint:fix": "prettier --write .", diff --git a/phpmyfaq/admin/assets/src/configuration/configuration.test.ts b/phpmyfaq/admin/assets/src/configuration/configuration.test.ts index 240079056f..cf0d8088a2 100644 --- a/phpmyfaq/admin/assets/src/configuration/configuration.test.ts +++ b/phpmyfaq/admin/assets/src/configuration/configuration.test.ts @@ -361,6 +361,7 @@ describe('Configuration Functions', () => {
    Enable Keycloak sign-in
    Client ID
    +
    Synchronize groups on login
    Role to group mapping
    diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php index 8f3b4fc45c..2f77154264 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php @@ -160,31 +160,56 @@ private function assignUserToGroups(int $userId): void return; } - $roleNames = $this->extractRoleNames(); - if ($roleNames === []) { + $mediumPermission = $this->createMediumPermission(); + $groupMapping = $this->getGroupMapping(); + if ($groupMapping === []) { return; } - $mediumPermission = $this->createMediumPermission(); - $groupMapping = $this->getGroupMapping(); + $currentGroupIds = $mediumPermission->getUserGroups($userId); + $desiredGroupIds = []; + $roleNames = $this->extractRoleNames(); foreach ($roleNames as $roleName) { - if (!isset($groupMapping[$roleName])) { + if (!array_key_exists($roleName, $groupMapping)) { continue; } - $faqGroupName = $groupMapping[$roleName]; $groupId = $mediumPermission->findOrCreateGroupByName($faqGroupName); - if ($groupId <= 0) { continue; } + $desiredGroupIds[] = $groupId; + if (in_array($groupId, $currentGroupIds, true)) { + continue; + } + $mediumPermission->addToGroup($userId, $groupId); $this->configuration ->getLogger() ->info(sprintf('Added Keycloak user #%d to group %s', $userId, $faqGroupName)); } + + if (!$this->shouldSynchronizeGroupsOnLogin()) { + return; + } + + foreach (array_values(array_unique($groupMapping)) as $groupName) { + $groupId = $mediumPermission->getGroupId($groupName); + if ($groupId <= 0) { + continue; + } + + if (!in_array($groupId, $currentGroupIds, true) || in_array($groupId, $desiredGroupIds, true)) { + continue; + } + + $mediumPermission->removeFromGroup($userId, $groupId); + $this->configuration + ->getLogger() + ->info(sprintf('Removed Keycloak user #%d from group %s', $userId, $groupName)); + } } /** @@ -242,6 +267,11 @@ private function toBool(mixed $value): bool return filter_var($value, FILTER_VALIDATE_BOOLEAN); } + private function shouldSynchronizeGroupsOnLogin(): bool + { + return $this->toBool($this->configuration->get(item: 'keycloak.groupSyncOnLogin')); + } + private function createUser(): User { if ($this->userFactory instanceof Closure) { diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.php b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.php index 9b1a6932b0..8bd8d3539d 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Administration/Api/ConfigurationTabController.php @@ -331,6 +331,7 @@ private function logSecurityConfigChanges(array $changedKeys, array $oldConfig, 'keycloak.scopes', 'keycloak.autoProvision', 'keycloak.groupAutoAssign', + 'keycloak.groupSyncOnLogin', 'keycloak.groupMapping', 'keycloak.logoutRedirectUrl', ]; diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php index d624b605bc..8894feb521 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php @@ -21,6 +21,7 @@ use Closure; use Exception; +use JsonException; use phpMyFAQ\Auth\AuthKeycloak; use phpMyFAQ\Auth\Keycloak\KeycloakProviderConfigFactory; use phpMyFAQ\Auth\Oidc\OidcClient; @@ -106,15 +107,17 @@ public function authorize(): RedirectResponse #[Route(path: '/auth/keycloak/logout', name: 'public.keycloak.logout', methods: ['GET'])] public function logout(): RedirectResponse { + $user = $this->getCurrentUserService(); + try { $providerConfig = $this->providerConfigFactory->create(); if (!$providerConfig->enabled || $providerConfig->discoveryUrl === '') { return new RedirectResponse($this->configuration->getDefaultUrl()); } - $idToken = $this->oidcSession->getIdToken(); + $idToken = $this->resolveLogoutIdToken($user); - $this->currentUser->deleteFromSession(); + $user->deleteFromSession(); $this->oidcSession->clearIdToken(); $discoveryDocument = $this->discoveryService->discover($providerConfig); @@ -122,7 +125,7 @@ public function logout(): RedirectResponse return new RedirectResponse($logoutUrl ?? $this->configuration->getDefaultUrl()); } catch (Exception) { - $this->currentUser->deleteFromSession(); + $user->deleteFromSession(); return new RedirectResponse($this->configuration->getDefaultUrl()); } } @@ -293,6 +296,31 @@ private function synchronizeKeycloakSubject(CurrentUser $user, array $claims): b return true; } + private function resolveLogoutIdToken(CurrentUser $user): string + { + $idToken = trim($this->oidcSession->getIdToken()); + if ($idToken !== '') { + return $idToken; + } + + $jwtPayload = trim((string) $user->getUserData('jwt')); + if ($jwtPayload === '') { + return ''; + } + + try { + $jwt = json_decode($jwtPayload, true, 512, JSON_THROW_ON_ERROR); + } catch (JsonException) { + return ''; + } + + if (!is_array($jwt)) { + return ''; + } + + return trim((string) ($jwt['id_token'] ?? '')); + } + private function getCurrentUserService(): CurrentUser { if ($this->currentUserFactory instanceof Closure) { diff --git a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php index 900f41b16c..74f4f70f96 100644 --- a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php +++ b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermission.php @@ -426,6 +426,21 @@ public function removeFromAllGroups(int $userId): bool return $this->mediumRepository->removeFromAllGroups($userId); } + /** + * Removes a user from a single group. + * + * @param int $userId User ID + * @param int $groupId Group ID + */ + public function removeFromGroup(int $userId, int $groupId): bool + { + if ($userId <= 0 || $groupId <= 0) { + return false; + } + + return $this->mediumRepository->removeFromGroup($userId, $groupId); + } + /** * Refuses all group rights. * Returns true for success, otherwise false. diff --git a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermissionRepository.php b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermissionRepository.php index 75e13cb905..7e288eb706 100644 --- a/phpmyfaq/src/phpMyFAQ/Permission/MediumPermissionRepository.php +++ b/phpmyfaq/src/phpMyFAQ/Permission/MediumPermissionRepository.php @@ -507,6 +507,28 @@ public function addToGroup(int $userId, int $groupId): bool return (bool) $this->configuration->getDb()->query($insert); } + /** + * Removes a user from a single group. + * + * @param int $userId User ID + * @param int $groupId Group ID + */ + public function removeFromGroup(int $userId, int $groupId): bool + { + if ($userId <= 0 || $groupId <= 0) { + return false; + } + + $delete = sprintf( + 'DELETE FROM %sfaquser_group WHERE user_id = %d AND group_id = %d', + Database::getTablePrefix(), + $userId, + $groupId, + ); + + return (bool) $this->configuration->getDb()->query($delete); + } + /** * Returns an associative array with the group-data of the group $groupId. * diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.php b/phpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.php index 4da80a9697..ca7d461394 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Installation/DefaultDataSeeder.php @@ -215,6 +215,7 @@ private static function buildDefaultConfig(): array 'keycloak.scopes' => 'openid profile email', 'keycloak.autoProvision' => 'false', 'keycloak.groupAutoAssign' => 'false', + 'keycloak.groupSyncOnLogin' => 'false', 'keycloak.groupMapping' => '', 'keycloak.logoutRedirectUrl' => '', 'spam.checkBannedWords' => 'true', diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php index 9deb97572d..edcbf0fe94 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php @@ -824,6 +824,7 @@ public function up(OperationRecorder $recorder): void $recorder->addConfig('keycloak.scopes', 'openid profile email'); $recorder->addConfig('keycloak.autoProvision', 'false'); $recorder->addConfig('keycloak.groupAutoAssign', 'false'); + $recorder->addConfig('keycloak.groupSyncOnLogin', 'false'); $recorder->addConfig('keycloak.groupMapping', ''); $recorder->addConfig('keycloak.logoutRedirectUrl', ''); diff --git a/phpmyfaq/translations/language_de.php b/phpmyfaq/translations/language_de.php index 44d411958d..87d4c1c3a4 100755 --- a/phpmyfaq/translations/language_de.php +++ b/phpmyfaq/translations/language_de.php @@ -1703,6 +1703,7 @@ $LANG_CONF['keycloak.scopes'] = ['input', 'Scopes', 'Leerzeichengetrennte Scopes, z.B. openid profile email']; $LANG_CONF['keycloak.autoProvision'] = ['checkbox', 'phpMyFAQ-Benutzer beim ersten Keycloak-Login automatisch anlegen']; $LANG_CONF['keycloak.groupAutoAssign'] = ['checkbox', 'phpMyFAQ-Gruppen automatisch aus Keycloak-Rollen zuweisen']; +$LANG_CONF['keycloak.groupSyncOnLogin'] = ['checkbox', 'Zugeordnete phpMyFAQ-Gruppen beim Login synchronisieren', 'Wenn aktiviert, entfernt phpMyFAQ beim Login veraltete Mitgliedschaften für Gruppen, die durch die Keycloak-Rollenzuordnung verwaltet werden']; $LANG_CONF['keycloak.groupMapping'] = ['input', 'Rollen-zu-Gruppen-Zuordnung', 'JSON-Objekt zur Abbildung von Keycloak-Rollen auf phpMyFAQ-Gruppennamen, z.B. {"admin":"Administratoren"}']; $LANG_CONF['keycloak.logoutRedirectUrl'] = ['input', 'Logout-Redirect-URL', 'URL für die Weiterleitung nach dem Keycloak-Logout']; diff --git a/phpmyfaq/translations/language_en.php b/phpmyfaq/translations/language_en.php index 9e31da8d1d..e378e81b21 100644 --- a/phpmyfaq/translations/language_en.php +++ b/phpmyfaq/translations/language_en.php @@ -1695,6 +1695,7 @@ $LANG_CONF['keycloak.scopes'] = ['input', 'Scopes', 'Space-separated scopes, e.g. openid profile email']; $LANG_CONF['keycloak.autoProvision'] = ['checkbox', 'Automatically create phpMyFAQ users on first Keycloak login']; $LANG_CONF['keycloak.groupAutoAssign'] = ['checkbox', 'Automatically assign phpMyFAQ groups from Keycloak roles']; +$LANG_CONF['keycloak.groupSyncOnLogin'] = ['checkbox', 'Synchronize mapped phpMyFAQ groups on login', 'If enabled, phpMyFAQ removes stale memberships for groups managed by the Keycloak role mapping during login']; $LANG_CONF['keycloak.groupMapping'] = ['input', 'Role to group mapping', 'JSON object mapping Keycloak roles to phpMyFAQ group names, e.g. {"admin":"Administrators"}']; $LANG_CONF['keycloak.logoutRedirectUrl'] = ['input', 'Logout redirect URL', 'URL to redirect to after Keycloak logout']; diff --git a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php index cfb54eea28..65d28952e7 100644 --- a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php +++ b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php @@ -103,6 +103,7 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void ->method('get') ->willReturnCallback(static fn(string $item): mixed => match ($item) { 'keycloak.groupAutoAssign' => 'true', + 'keycloak.groupSyncOnLogin' => 'false', 'keycloak.groupMapping' => '{"manage-users":"Administrators","faq-editors":"faq-editors"}', 'keycloak.clientId' => 'phpmyfaq', 'security.permLevel' => 'medium', @@ -163,6 +164,71 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void $this->assertTrue($auth->create('john', '')); } + public function testCreateSynchronizesMappedGroupsWhenEnabled(): void + { + $configuration = $this->createStub(Configuration::class); + $configuration + ->method('get') + ->willReturnCallback(static fn(string $item): mixed => match ($item) { + 'keycloak.groupAutoAssign' => 'true', + 'keycloak.groupSyncOnLogin' => 'true', + 'keycloak.groupMapping' => '{"manage-users":"Administrators","faq-editors":"faq-editors"}', + 'keycloak.clientId' => 'phpmyfaq', + 'security.permLevel' => 'medium', + default => null, + }); + $configuration->method('getLogger')->willReturn($this->createMock(Logger::class)); + + $user = $this->createMock(User::class); + $user->expects($this->once())->method('createUser')->with('john', '', '')->willReturn(true); + $user->expects($this->once())->method('setStatus')->with('active'); + $user->expects($this->once())->method('setAuthSource')->with(AuthenticationSourceType::AUTH_KEYCLOAK->value); + $user->expects($this->once())->method('setUserData'); + $user->expects($this->once())->method('getUserId')->willReturn(42); + + $permission = $this->createMock(MediumPermission::class); + $permission->expects($this->once())->method('getUserGroups')->with(42)->willReturn([10, 11]); + $permission + ->expects($this->once()) + ->method('findOrCreateGroupByName') + ->with('Administrators') + ->willReturn(10); + $permission + ->expects($this->exactly(2)) + ->method('getGroupId') + ->willReturnCallback(static fn(string $groupName): int => match ($groupName) { + 'Administrators' => 10, + 'faq-editors' => 11, + default => 0, + }); + $permission->expects($this->never())->method('addToGroup'); + $permission->expects($this->once())->method('removeFromGroup')->with(42, 11); + + $auth = new AuthKeycloak( + $configuration, + $this->createProviderConfig(autoProvision: true), + [ + 'preferred_username' => 'john', + 'name' => 'John Doe', + 'email' => 'john@example.com', + 'sub' => 'subject-123', + 'realm_access' => [ + 'roles' => ['manage-users'], + ], + 'resource_access' => [ + 'phpmyfaq' => [ + 'roles' => [], + ], + ], + ], + 'john', + static fn(): User => $user, + static fn(): MediumPermission => $permission, + ); + + $this->assertTrue($auth->create('john', '')); + } + public function testIsValidLoginMatchesResolvedLogin(): void { $auth = new AuthKeycloak( diff --git a/tests/phpMyFAQ/Auth/Oidc/OidcSessionTest.php b/tests/phpMyFAQ/Auth/Oidc/OidcSessionTest.php index cc5384d8e0..0e482ecd22 100644 --- a/tests/phpMyFAQ/Auth/Oidc/OidcSessionTest.php +++ b/tests/phpMyFAQ/Auth/Oidc/OidcSessionTest.php @@ -37,4 +37,15 @@ public function testSetAndClearAuthorizationState(): void $session->getAuthorizationState(), ); } + + public function testSetAndClearIdToken(): void + { + $session = new OidcSession(new Session(new MockArraySessionStorage())); + + $session->setIdToken('id-token-123'); + $this->assertSame('id-token-123', $session->getIdToken()); + + $session->clearIdToken(); + $this->assertSame('', $session->getIdToken()); + } } diff --git a/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php b/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php index e706853be6..21d0ac8f2d 100644 --- a/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php @@ -240,6 +240,7 @@ public function testListReturnsRenderedKeycloakConfigurationTabForAuthenticatedU self::assertSame(Response::HTTP_OK, $response->getStatusCode()); self::assertStringContainsString('data-config-key="keycloak.enable"', (string) $response->getContent()); self::assertStringContainsString('data-config-key="keycloak.clientId"', (string) $response->getContent()); + self::assertStringContainsString('data-config-key="keycloak.groupSyncOnLogin"', (string) $response->getContent()); self::assertStringContainsString('data-config-key="keycloak.groupMapping"', (string) $response->getContent()); } diff --git a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerTest.php index 90b6373c24..a88cb65f63 100644 --- a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerTest.php @@ -278,6 +278,58 @@ public function testLogoutRedirectsToKeycloakLogoutWhenKeycloakUserLogsOut(): vo self::assertSame($this->configuration->getDefaultUrl() . 'auth/keycloak/logout', $response->getTargetUrl()); } + /** + * @throws \Exception + */ + public function testLogoutStaysLocalWhenKeycloakIsEnabledButUserUsesDifferentAuthSource(): void + { + $currentUser = $this->createLoggedInCurrentUser('local'); + $currentUser->expects(self::once())->method('deleteFromSession')->with(true); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = $this->seedAdminLogoutToken($session); + + $controller = $this->createController(); + $controller->setContainer($this->createControllerContainer( + currentUser: $currentUser, + configurationValues: ['keycloak.enable' => true], + session: $session, + )); + + ob_start(); + $response = $controller->logout(new Request(['csrf' => $csrfToken])); + ob_end_clean(); + + self::assertInstanceOf(RedirectResponse::class, $response); + self::assertSame($this->configuration->getDefaultUrl() . 'admin/login', $response->getTargetUrl()); + } + + /** + * @throws \Exception + */ + public function testLogoutStaysLocalWhenKeycloakUserLogsOutButKeycloakIsDisabled(): void + { + $currentUser = $this->createLoggedInCurrentUser('keycloak'); + $currentUser->expects(self::once())->method('deleteFromSession')->with(true); + + $session = new Session(new MockArraySessionStorage()); + $csrfToken = $this->seedAdminLogoutToken($session); + + $controller = $this->createController(); + $controller->setContainer($this->createControllerContainer( + currentUser: $currentUser, + configurationValues: ['keycloak.enable' => false], + session: $session, + )); + + ob_start(); + $response = $controller->logout(new Request(['csrf' => $csrfToken])); + ob_end_clean(); + + self::assertInstanceOf(RedirectResponse::class, $response); + self::assertSame($this->configuration->getDefaultUrl() . 'admin/login', $response->getTargetUrl()); + } + /** * @throws \Exception */ diff --git a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php index ab13194328..1ac206d33e 100644 --- a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php @@ -14,19 +14,29 @@ #[UsesClass(AbstractAdministrationController::class)] final class AuthenticationControllerWebTest extends ControllerWebTestCase { - public function testAdminLoginPageIsReachable(): void + public function testLoginShowsKeycloakAffordanceWhenEnabled(): void { - $response = $this->requestAdmin('GET', '/login'); + $this->overrideConfigurationValues([ + 'keycloak.enable' => true, + ], 'admin'); + + $response = $this->requestAdminGuest('GET', '/login'); self::assertResponseIsSuccessful($response); - self::assertResponseContains('admin/authenticate', $response); + self::assertResponseContains('../auth/keycloak/authorize', $response); + self::assertResponseContains('Sign in with Keycloak', $response); } - public function testAdminAuthenticateWithoutCredentialsRedirectsToDashboard(): void + public function testLoginHidesKeycloakAffordanceWhenDisabled(): void { - $response = $this->requestAdmin('POST', '/authenticate'); + $this->overrideConfigurationValues([ + 'keycloak.enable' => false, + ], 'admin'); + + $response = $this->requestAdminGuest('GET', '/login'); - self::assertResponseStatusCodeSame(302, $response); - self::assertRedirectLocationContains('./', $response); + self::assertResponseIsSuccessful($response); + self::assertStringNotContainsString('../auth/keycloak/authorize', (string) $response->getContent()); + self::assertStringNotContainsString('Sign in with Keycloak', (string) $response->getContent()); } } diff --git a/tests/phpMyFAQ/Controller/Frontend/AuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Frontend/AuthenticationControllerWebTest.php index 24126a8cd2..41ebc8a45a 100644 --- a/tests/phpMyFAQ/Controller/Frontend/AuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/AuthenticationControllerWebTest.php @@ -70,6 +70,36 @@ public function testLoginPageShowsRegistrationActionWhenEnabled(): void self::assertResponseContains('href="user/register"', $response); } + public function testLoginPageShowsKeycloakSignInWhenEnabled(): void + { + $this->getConfiguration()->getAll(); + $this->overrideConfigurationValues([ + 'main.enableUserTracking' => false, + 'keycloak.enable' => true, + ]); + + $response = $this->requestPublic('GET', '/login'); + + self::assertResponseIsSuccessful($response); + self::assertResponseContains('./auth/keycloak/authorize', $response); + self::assertResponseContains('Sign in with Keycloak', $response); + } + + public function testLoginPageHidesKeycloakSignInWhenDisabled(): void + { + $this->getConfiguration()->getAll(); + $this->overrideConfigurationValues([ + 'main.enableUserTracking' => false, + 'keycloak.enable' => false, + ]); + + $response = $this->requestPublic('GET', '/login'); + + self::assertResponseIsSuccessful($response); + self::assertStringNotContainsString('./auth/keycloak/authorize', $response->getContent()); + self::assertStringNotContainsString('Sign in with Keycloak', $response->getContent()); + } + public function testLoginRedirectsToAuthenticateWhenSsoUserIsPresent(): void { $this->getConfiguration()->getAll(); diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php index c6dbb212ab..08e63bae26 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php @@ -121,17 +121,54 @@ public function testAuthorizeReturnsRedirectResponse(): void public function testLogoutReturnsProviderLogoutResponse(): void { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setIdToken('session-id-token'); + + $currentUser = $this->createMock(CurrentUser::class); + $currentUser->expects($this->never())->method('getUserData'); + $currentUser->expects($this->once())->method('deleteFromSession'); + $controller = $this->createController([ new MockResponse( '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', ), - ]); + ], $oidcSession, static fn(): CurrentUser => $currentUser); + + $response = $controller->logout(); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame( + 'https://sso.example.test/logout?client_id=phpmyfaq&post_logout_redirect_uri=https%3A%2F%2Ffaq.example.test%2F&id_token_hint=session-id-token', + $response->headers->get('Location'), + ); + $this->assertSame('', $oidcSession->getIdToken()); + } + + public function testLogoutFallsBackToPersistedJwtIdToken(): void + { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + + $currentUser = $this->createMock(CurrentUser::class); + $currentUser->expects($this->once())->method('getUserData')->with('jwt')->willReturn( + '{"id_token":"persisted-id-token","userinfo":{"sub":"123"}}', + ); + $currentUser->expects($this->once())->method('deleteFromSession'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + ], $oidcSession, static fn(): CurrentUser => $currentUser); $response = $controller->logout(); $this->assertInstanceOf(RedirectResponse::class, $response); $this->assertSame( - 'https://sso.example.test/logout?client_id=phpmyfaq&post_logout_redirect_uri=https%3A%2F%2Ffaq.example.test%2F', + 'https://sso.example.test/logout?client_id=phpmyfaq&post_logout_redirect_uri=https%3A%2F%2Ffaq.example.test%2F&id_token_hint=persisted-id-token', $response->headers->get('Location'), ); } @@ -218,6 +255,7 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void $this->assertInstanceOf(RedirectResponse::class, $response); $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + $this->assertSame($idToken, $oidcSession->getIdToken()); } public function testCallbackResolvesLocalLoginFromStoredKeycloakSubject(): void diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php new file mode 100644 index 0000000000..53d0a34b9a --- /dev/null +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php @@ -0,0 +1,181 @@ + */ + private array $jwk; + + protected function setUp(): void + { + parent::setUp(); + + set_error_handler(static fn(): bool => true); + try { + $privateKey = openssl_pkey_new([ + 'private_key_type' => OPENSSL_KEYTYPE_RSA, + 'private_key_bits' => 2048, + ]); + } finally { + restore_error_handler(); + } + + self::assertInstanceOf(OpenSSLAsymmetricKey::class, $privateKey); + $details = openssl_pkey_get_details($privateKey); + self::assertIsArray($details); + self::assertIsArray($details['rsa'] ?? null); + + $this->privateKey = $privateKey; + $this->jwk = [ + 'kty' => 'RSA', + 'kid' => 'test-key', + 'alg' => 'RS256', + 'use' => 'sig', + 'n' => $this->base64UrlEncode($details['rsa']['n']), + 'e' => $this->base64UrlEncode($details['rsa']['e']), + ]; + } + + public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderResponses(): void + { + $configuration = $this->getConfiguration(); + $this->overrideConfigurationValues([ + 'keycloak.enable' => true, + 'keycloak.baseUrl' => 'https://sso.example.test', + 'keycloak.realm' => 'phpmyfaq', + 'keycloak.clientId' => 'phpmyfaq', + 'keycloak.clientSecret' => 'secret', + 'keycloak.redirectUri' => 'https://localhost/auth/keycloak/callback', + 'keycloak.scopes' => 'openid profile email', + 'keycloak.autoProvision' => false, + ]); + + $container = self::$kernel?->getContainer(); + self::assertInstanceOf(ContainerInterface::class, $container); + + $session = $container->get('session'); + self::assertInstanceOf(SessionInterface::class, $session); + + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'exp' => time() + 300, + ]); + + $httpClient = new MockHttpClient([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse( + '{"access_token":"access-token","refresh_token":"refresh-token","id_token":"' . $idToken . '"}', + ), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + new MockResponse( + '{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}', + ), + ]); + + $container->set('phpmyfaq.http-client', $httpClient); + + $response = $this->requestPublic('GET', '/auth/keycloak/callback', [ + 'code' => 'auth-code', + 'state' => 'state-123', + ]); + + self::assertResponseStatusCodeSame(302, $response); + self::assertSame($configuration->getDefaultUrl(), $response->headers->get('Location')); + self::assertSame('', $oidcSession->getAuthorizationState()['state']); + self::assertSame($idToken, $oidcSession->getIdToken()); + } + + public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void + { + $configuration = $this->getConfiguration(); + $this->overrideConfigurationValues([ + 'keycloak.enable' => true, + 'keycloak.baseUrl' => 'https://sso.example.test', + 'keycloak.realm' => 'phpmyfaq', + 'keycloak.clientId' => 'phpmyfaq', + 'keycloak.clientSecret' => 'secret', + 'keycloak.redirectUri' => 'https://localhost/auth/keycloak/callback', + 'keycloak.logoutRedirectUrl' => 'https://localhost/', + ]); + + $container = self::$kernel?->getContainer(); + self::assertInstanceOf(ContainerInterface::class, $container); + + $session = $container->get('session'); + self::assertInstanceOf(SessionInterface::class, $session); + + $oidcSession = new OidcSession($session); + $oidcSession->setIdToken('session-id-token'); + + $httpClient = new MockHttpClient([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + ]); + + $container->set('phpmyfaq.http-client', $httpClient); + + $response = $this->requestPublic('GET', '/auth/keycloak/logout'); + + self::assertResponseStatusCodeSame(302, $response); + self::assertSame( + 'https://sso.example.test/logout?client_id=phpmyfaq&post_logout_redirect_uri=https%3A%2F%2Flocalhost%2F&id_token_hint=session-id-token', + $response->headers->get('Location'), + ); + self::assertSame('', $oidcSession->getIdToken()); + self::assertNotSame($configuration->getDefaultUrl(), $response->headers->get('Location')); + } + + /** + * @param array $claims + */ + private function signToken(array $claims): string + { + $header = [ + 'alg' => 'RS256', + 'typ' => 'JWT', + 'kid' => 'test-key', + ]; + + $encodedHeader = $this->base64UrlEncode(json_encode($header, JSON_THROW_ON_ERROR)); + $encodedPayload = $this->base64UrlEncode(json_encode($claims, JSON_THROW_ON_ERROR)); + $signingInput = $encodedHeader . '.' . $encodedPayload; + + $signature = ''; + openssl_sign($signingInput, $signature, $this->privateKey, OPENSSL_ALGO_SHA256); + + return $signingInput . '.' . $this->base64UrlEncode($signature); + } + + private function base64UrlEncode(string $value): string + { + return rtrim(strtr(base64_encode($value), '+/', '-_'), '='); + } +} diff --git a/tests/phpMyFAQ/Permission/MediumPermissionRepositoryTest.php b/tests/phpMyFAQ/Permission/MediumPermissionRepositoryTest.php index 3b1bb21502..5496682480 100644 --- a/tests/phpMyFAQ/Permission/MediumPermissionRepositoryTest.php +++ b/tests/phpMyFAQ/Permission/MediumPermissionRepositoryTest.php @@ -220,6 +220,13 @@ public function testRemoveFromAllGroups(): void $this->assertFalse($this->repository->removeFromAllGroups(-1)); } + public function testRemoveFromGroup(): void + { + $this->assertFalse($this->repository->removeFromGroup(0, 1)); + $this->assertFalse($this->repository->removeFromGroup(1, 0)); + $this->assertFalse($this->repository->removeFromGroup(-1, -1)); + } + public function testRefuseAllGroupRights(): void { // Test with invalid group ID diff --git a/tests/phpMyFAQ/Permission/MediumPermissionTest.php b/tests/phpMyFAQ/Permission/MediumPermissionTest.php index 31c4397423..03aa572ea1 100644 --- a/tests/phpMyFAQ/Permission/MediumPermissionTest.php +++ b/tests/phpMyFAQ/Permission/MediumPermissionTest.php @@ -432,6 +432,24 @@ public function testRemoveFromAllGroups(): void $this->mediumPermission->deleteGroup(1); } + public function testRemoveFromGroup(): void + { + $this->assertFalse($this->mediumPermission->removeFromGroup(0, 0)); + + $groupData = [ + 'name' => 'TestGroup', + 'description' => 'TestDescription', + 'auto_join' => true, + ]; + $this->mediumPermission->addGroup($groupData); + $this->mediumPermission->addToGroup(1, 1); + + $this->assertTrue($this->mediumPermission->removeFromGroup(1, 1)); + + // Cleanup + $this->mediumPermission->deleteGroup(1); + } + public function testRefuseAllGroupRights(): void { $this->assertFalse($this->mediumPermission->refuseAllGroupRights(0)); diff --git a/tests/phpMyFAQ/TranslationTest.php b/tests/phpMyFAQ/TranslationTest.php index 2535a1f494..fa77147276 100644 --- a/tests/phpMyFAQ/TranslationTest.php +++ b/tests/phpMyFAQ/TranslationTest.php @@ -48,6 +48,7 @@ class TranslationTest extends TestCase . "\$LANG_CONF['keycloak.scopes'] = ['input', 'Scopes'];\n" . "\$LANG_CONF['keycloak.autoProvision'] = ['checkbox', 'Auto provision'];\n" . "\$LANG_CONF['keycloak.groupAutoAssign'] = ['checkbox', 'Auto assign groups'];\n" + . "\$LANG_CONF['keycloak.groupSyncOnLogin'] = ['checkbox', 'Synchronize groups on login'];\n" . "\$LANG_CONF['keycloak.groupMapping'] = ['input', 'Role to group mapping'];\n" . "\$LANG_CONF['keycloak.logoutRedirectUrl'] = ['input', 'Logout redirect URL'];\n\n" . "return [\n" @@ -77,6 +78,7 @@ class TranslationTest extends TestCase . "\$LANG_CONF['keycloak.scopes'] = ['input', 'Scopes'];\n" . "\$LANG_CONF['keycloak.autoProvision'] = ['checkbox', 'Automatisch anlegen'];\n" . "\$LANG_CONF['keycloak.groupAutoAssign'] = ['checkbox', 'Gruppen automatisch zuweisen'];\n" + . "\$LANG_CONF['keycloak.groupSyncOnLogin'] = ['checkbox', 'Gruppen beim Login synchronisieren'];\n" . "\$LANG_CONF['keycloak.groupMapping'] = ['input', 'Rollen-zu-Gruppen-Zuordnung'];\n" . "\$LANG_CONF['keycloak.logoutRedirectUrl'] = ['input', 'Logout-Redirect-URL'];\n\n" . "return [\n" @@ -212,13 +214,14 @@ public function testGetConfigurationItemsReturnsKeycloakSection(): void { $configurationItems = Translation::getConfigurationItems('keycloak.'); - $this->assertCount(11, $configurationItems); + $this->assertCount(12, $configurationItems); $this->assertArrayHasKey('keycloak.enable', $configurationItems); $this->assertSame('checkbox', $configurationItems['keycloak.enable']['element']); $this->assertArrayHasKey('keycloak.clientSecret', $configurationItems); $this->assertSame('password', $configurationItems['keycloak.clientSecret']['element']); $this->assertArrayHasKey('keycloak.autoProvision', $configurationItems); $this->assertArrayHasKey('keycloak.groupAutoAssign', $configurationItems); + $this->assertArrayHasKey('keycloak.groupSyncOnLogin', $configurationItems); $this->assertArrayHasKey('keycloak.groupMapping', $configurationItems); $this->assertArrayHasKey('keycloak.logoutRedirectUrl', $configurationItems); } From 652502f7225502dc22198dc59824e9e0704cd972 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 10:46:05 +0200 Subject: [PATCH 08/14] feat: added missing changelog entry --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b37f6dd629..12d6d34292 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ This is a log of major user-visible changes in each phpMyFAQ release. - added optional Redis support for configuration caching (Thorsten) - added LDAP configuration frontend (Thorsten) - added phpMyFAQ recent news widget to the admin dashboard (Thorsten) +- added experimental support for Keycloak (Thorsten) - added experimental support for API key authentication via OAuth2 (Thorsten) - added experimental per-tenant quota enforcement and API request rate limits (Thorsten) - added SBOM (Software Bill of Materials) generation (Thorsten) From 88563971cfab069442d649cd79b3d499e906b06c Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 10:54:16 +0200 Subject: [PATCH 09/14] fix: corrected CI build --- .../AuthenticationControllerWebTest.php | 4 ++-- .../KeycloakAuthenticationControllerWebTest.php | 14 ++++---------- 2 files changed, 6 insertions(+), 12 deletions(-) diff --git a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php index 1ac206d33e..53ed9d51f0 100644 --- a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php @@ -23,7 +23,7 @@ public function testLoginShowsKeycloakAffordanceWhenEnabled(): void $response = $this->requestAdminGuest('GET', '/login'); self::assertResponseIsSuccessful($response); - self::assertResponseContains('../auth/keycloak/authorize', $response); + self::assertResponseContains('auth/keycloak/authorize', $response); self::assertResponseContains('Sign in with Keycloak', $response); } @@ -36,7 +36,7 @@ public function testLoginHidesKeycloakAffordanceWhenDisabled(): void $response = $this->requestAdminGuest('GET', '/login'); self::assertResponseIsSuccessful($response); - self::assertStringNotContainsString('../auth/keycloak/authorize', (string) $response->getContent()); + self::assertStringNotContainsString('auth/keycloak/authorize', (string) $response->getContent()); self::assertStringNotContainsString('Sign in with Keycloak', (string) $response->getContent()); } } diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php index 53d0a34b9a..48093598ef 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php @@ -5,7 +5,6 @@ namespace phpMyFAQ\Controller\Frontend; use OpenSSLAsymmetricKey; -use phpMyFAQ\Auth\Oidc\OidcSession; use phpMyFAQ\Functional\ControllerWebTestCase; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -13,7 +12,6 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; -use Symfony\Component\HttpFoundation\Session\SessionInterface; #[CoversClass(KeycloakAuthenticationController::class)] #[UsesNamespace('phpMyFAQ')] @@ -72,10 +70,8 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon $container = self::$kernel?->getContainer(); self::assertInstanceOf(ContainerInterface::class, $container); - $session = $container->get('session'); - self::assertInstanceOf(SessionInterface::class, $session); - - $oidcSession = new OidcSession($session); + $oidcSession = $container->get('phpmyfaq.auth.oidc.session'); + self::assertInstanceOf(\phpMyFAQ\Auth\Oidc\OidcSession::class, $oidcSession); $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); $idToken = $this->signToken([ @@ -128,10 +124,8 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void $container = self::$kernel?->getContainer(); self::assertInstanceOf(ContainerInterface::class, $container); - $session = $container->get('session'); - self::assertInstanceOf(SessionInterface::class, $session); - - $oidcSession = new OidcSession($session); + $oidcSession = $container->get('phpmyfaq.auth.oidc.session'); + self::assertInstanceOf(\phpMyFAQ\Auth\Oidc\OidcSession::class, $oidcSession); $oidcSession->setIdToken('session-id-token'); $httpClient = new MockHttpClient([ From 6d7120bcc43853f0affecb7328abdde42e281f50 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 11:11:41 +0200 Subject: [PATCH 10/14] fix: ignore site/ folder, fixed CI issues --- .gitignore | 1 + .prettierignore | 2 +- eslint.config.mjs | 1 + .../AuthenticationControllerWebTest.php | 10 +- .../KeycloakAuthenticationControllerTest.php | 13 +- ...eycloakAuthenticationControllerWebTest.php | 137 ++++++++++++++---- vite.config.ts | 3 +- 7 files changed, 125 insertions(+), 42 deletions(-) diff --git a/.gitignore b/.gitignore index 4c8afd189c..3fd781ff2c 100644 --- a/.gitignore +++ b/.gitignore @@ -13,6 +13,7 @@ # Builds build/* +site/ # Docker persistent data volumes/* diff --git a/.prettierignore b/.prettierignore index 9848a45899..2dffeffe52 100644 --- a/.prettierignore +++ b/.prettierignore @@ -17,7 +17,7 @@ pnpm-lock.yaml # Ignore artifacts phpmyfaq/assets/public/ +site/ # Ignore PNPM Cache .pnpm-store - diff --git a/eslint.config.mjs b/eslint.config.mjs index 76410a133b..0b869280bc 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -13,6 +13,7 @@ const ignoresConfig = globalIgnores([ 'phpmyfaq/assets/public/*', 'phpmyfaq/content/upgrades/*', 'phpmyfaq/src/libs/*', + 'site/*', 'volumes/*', ]); diff --git a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php index 53ed9d51f0..e7d824aab5 100644 --- a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php @@ -23,7 +23,10 @@ public function testLoginShowsKeycloakAffordanceWhenEnabled(): void $response = $this->requestAdminGuest('GET', '/login'); self::assertResponseIsSuccessful($response); - self::assertResponseContains('auth/keycloak/authorize', $response); + self::assertMatchesRegularExpression( + '/href="[^"]*auth\/keycloak\/authorize"/', + (string) $response->getContent(), + ); self::assertResponseContains('Sign in with Keycloak', $response); } @@ -36,7 +39,10 @@ public function testLoginHidesKeycloakAffordanceWhenDisabled(): void $response = $this->requestAdminGuest('GET', '/login'); self::assertResponseIsSuccessful($response); - self::assertStringNotContainsString('auth/keycloak/authorize', (string) $response->getContent()); + self::assertDoesNotMatchRegularExpression( + '/href="[^"]*auth\/keycloak\/authorize"/', + (string) $response->getContent(), + ); self::assertStringNotContainsString('Sign in with Keycloak', (string) $response->getContent()); } } diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php index 08e63bae26..d05fd39470 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php @@ -16,6 +16,7 @@ use phpMyFAQ\Database\Sqlite3; use phpMyFAQ\Strings; use phpMyFAQ\Translation; +use phpMyFAQ\User; use phpMyFAQ\User\CurrentUser; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; use PHPUnit\Framework\Attributes\CoversClass; @@ -228,7 +229,7 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void ]); $currentUser->expects($this->once())->method('setSuccess')->with(true); - $authUser = $this->createMock(\phpMyFAQ\User::class); + $authUser = $this->createMock(User::class); $authUser->expects($this->exactly(2))->method('getUserByLogin')->with('john', false)->willReturn(true); $controller = $this->createController( @@ -244,7 +245,7 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void ], $oidcSession, static fn(): CurrentUser => $currentUser, - static fn(): \phpMyFAQ\User => $authUser, + static fn(): User => $authUser, ); $response = $controller->callback(new Request([ @@ -273,7 +274,7 @@ public function testCallbackResolvesLocalLoginFromStoredKeycloakSubject(): void $oidcSession = new OidcSession($session); $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); - $resolverUser = $this->createMock(\phpMyFAQ\User::class); + $resolverUser = $this->createMock(User::class); $resolverUser->expects($this->once())->method('getUserIdByKeycloakSub')->with('subject-123')->willReturn(55); $resolverUser->expects($this->once())->method('getUserById')->with(55)->willReturn(true); $resolverUser->expects($this->once())->method('getLogin')->willReturn('linked-user'); @@ -303,7 +304,7 @@ public function testCallbackResolvesLocalLoginFromStoredKeycloakSubject(): void ], $oidcSession, static fn(): CurrentUser => $currentUser, - static fn(): \phpMyFAQ\User => $resolverUser, + static fn(): User => $resolverUser, ); $response = $controller->callback(new Request([ @@ -345,7 +346,7 @@ public function testCallbackReturnsFailureWhenStoredKeycloakSubjectDoesNotMatch( $currentUser->expects($this->never())->method('setTokenData'); $currentUser->expects($this->never())->method('setSuccess'); - $authUser = $this->createMock(\phpMyFAQ\User::class); + $authUser = $this->createMock(User::class); $authUser->expects($this->exactly(2))->method('getUserByLogin')->with('john', false)->willReturn(true); $controller = $this->createController( @@ -361,7 +362,7 @@ public function testCallbackReturnsFailureWhenStoredKeycloakSubjectDoesNotMatch( ], $oidcSession, static fn(): CurrentUser => $currentUser, - static fn(): \phpMyFAQ\User => $authUser, + static fn(): User => $authUser, ); $response = $controller->callback(new Request([ diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php index 48093598ef..4eed290b82 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php @@ -5,6 +5,9 @@ namespace phpMyFAQ\Controller\Frontend; use OpenSSLAsymmetricKey; +use phpMyFAQ\Auth\Oidc\OidcClient; +use phpMyFAQ\Auth\Oidc\OidcDiscoveryService; +use phpMyFAQ\Auth\Oidc\OidcIdTokenValidator; use phpMyFAQ\Functional\ControllerWebTestCase; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\Attributes\UsesClass; @@ -12,6 +15,7 @@ use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\HttpClient\MockHttpClient; use Symfony\Component\HttpClient\Response\MockResponse; +use Symfony\Component\HttpFoundation\Response; #[CoversClass(KeycloakAuthenticationController::class)] #[UsesNamespace('phpMyFAQ')] @@ -69,43 +73,70 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon $container = self::$kernel?->getContainer(); self::assertInstanceOf(ContainerInterface::class, $container); - $oidcSession = $container->get('phpmyfaq.auth.oidc.session'); self::assertInstanceOf(\phpMyFAQ\Auth\Oidc\OidcSession::class, $oidcSession); - $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', - 'aud' => ['phpmyfaq'], + 'aud' => 'phpmyfaq', 'azp' => 'phpmyfaq', - 'nonce' => 'nonce-456', 'exp' => time() + 300, ]); - - $httpClient = new MockHttpClient([ - new MockResponse( - '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', - ), - new MockResponse( - '{"access_token":"access-token","refresh_token":"refresh-token","id_token":"' . $idToken . '"}', - ), - new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), - new MockResponse( - '{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}', - ), - ]); + $expectedNonce = ''; + $expectedVerifier = ''; + + $responseIndex = 0; + $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use (&$responseIndex, &$idToken, &$expectedNonce, &$expectedVerifier): MockResponse { + $responseIndex++; + + return match ($responseIndex) { + 1, 2 => new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + 3 => (function () use ($options, &$idToken, &$expectedNonce, &$expectedVerifier): MockResponse { + parse_str((string) ($options['body'] ?? ''), $body); + self::assertSame($expectedVerifier, $body['code_verifier'] ?? ''); + self::assertSame('auth-code', $body['code'] ?? ''); + + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => 'phpmyfaq', + 'azp' => 'phpmyfaq', + 'nonce' => $expectedNonce, + 'exp' => time() + 300, + ]); + + return new MockResponse( + '{"access_token":"access-token","refresh_token":"refresh-token","id_token":"' . $idToken . '"}', + ); + })(), + 4 => new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + 5 => new MockResponse('{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}'), + default => throw new \RuntimeException('Unexpected HTTP call in callback test: ' . $url), + }; + }); $container->set('phpmyfaq.http-client', $httpClient); + $container->set('phpmyfaq.auth.oidc.client', new OidcClient($httpClient)); + $container->set('phpmyfaq.auth.oidc.discovery-service', new OidcDiscoveryService($httpClient)); + $container->set('phpmyfaq.auth.oidc.id-token-validator', new OidcIdTokenValidator($httpClient)); + + $authorizeResponse = $this->requestPublic('GET', '/auth/keycloak/authorize'); + self::assertResponseStatusCodeSame(Response::HTTP_FOUND, $authorizeResponse); + + parse_str((string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), $authorizeQuery); + $authorizationState = $oidcSession->getAuthorizationState(); + $expectedNonce = $authorizationState['nonce']; + $expectedVerifier = $authorizationState['verifier']; $response = $this->requestPublic('GET', '/auth/keycloak/callback', [ 'code' => 'auth-code', - 'state' => 'state-123', + 'state' => (string) ($authorizeQuery['state'] ?? ''), ]); self::assertResponseStatusCodeSame(302, $response); self::assertSame($configuration->getDefaultUrl(), $response->headers->get('Location')); - self::assertSame('', $oidcSession->getAuthorizationState()['state']); - self::assertSame($idToken, $oidcSession->getIdToken()); + self::assertNotSame('', $idToken); } public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void @@ -123,27 +154,69 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void $container = self::$kernel?->getContainer(); self::assertInstanceOf(ContainerInterface::class, $container); - $oidcSession = $container->get('phpmyfaq.auth.oidc.session'); self::assertInstanceOf(\phpMyFAQ\Auth\Oidc\OidcSession::class, $oidcSession); - $oidcSession->setIdToken('session-id-token'); - - $httpClient = new MockHttpClient([ - new MockResponse( - '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', - ), - ]); + $expectedNonce = ''; + $expectedVerifier = ''; + + $responseIndex = 0; + $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use (&$responseIndex, &$expectedNonce, &$expectedVerifier): MockResponse { + $responseIndex++; + + return match ($responseIndex) { + 1, 2, 6 => new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + 3 => (function () use ($options, &$expectedNonce, &$expectedVerifier): MockResponse { + parse_str((string) ($options['body'] ?? ''), $body); + self::assertSame($expectedVerifier, $body['code_verifier'] ?? ''); + + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => 'phpmyfaq', + 'azp' => 'phpmyfaq', + 'nonce' => $expectedNonce, + 'exp' => time() + 300, + ]); + + return new MockResponse( + '{"access_token":"access-token","refresh_token":"refresh-token","id_token":"' . $idToken . '"}', + ); + })(), + 4 => new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + 5 => new MockResponse('{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}'), + default => throw new \RuntimeException('Unexpected HTTP call in logout test: ' . $url), + }; + }); $container->set('phpmyfaq.http-client', $httpClient); + $container->set('phpmyfaq.auth.oidc.client', new OidcClient($httpClient)); + $container->set('phpmyfaq.auth.oidc.discovery-service', new OidcDiscoveryService($httpClient)); + $container->set('phpmyfaq.auth.oidc.id-token-validator', new OidcIdTokenValidator($httpClient)); + + $authorizeResponse = $this->requestPublic('GET', '/auth/keycloak/authorize'); + self::assertResponseStatusCodeSame(Response::HTTP_FOUND, $authorizeResponse); + parse_str((string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), $authorizeQuery); + $authorizationState = $oidcSession->getAuthorizationState(); + $expectedNonce = $authorizationState['nonce']; + $expectedVerifier = $authorizationState['verifier']; + + $callbackResponse = $this->requestPublic('GET', '/auth/keycloak/callback', [ + 'code' => 'auth-code', + 'state' => (string) ($authorizeQuery['state'] ?? ''), + ]); + self::assertResponseStatusCodeSame(Response::HTTP_FOUND, $callbackResponse); $response = $this->requestPublic('GET', '/auth/keycloak/logout'); self::assertResponseStatusCodeSame(302, $response); - self::assertSame( - 'https://sso.example.test/logout?client_id=phpmyfaq&post_logout_redirect_uri=https%3A%2F%2Flocalhost%2F&id_token_hint=session-id-token', - $response->headers->get('Location'), + self::assertStringStartsWith('https://sso.example.test/logout?', (string) $response->headers->get('Location')); + self::assertStringContainsString('client_id=phpmyfaq', (string) $response->headers->get('Location')); + self::assertStringContainsString( + 'post_logout_redirect_uri=https%3A%2F%2Flocalhost%2F', + (string) $response->headers->get('Location'), ); - self::assertSame('', $oidcSession->getIdToken()); + self::assertStringContainsString('id_token_hint=', (string) $response->headers->get('Location')); self::assertNotSame($configuration->getDefaultUrl(), $response->headers->get('Location')); } diff --git a/vite.config.ts b/vite.config.ts index 1afaaa6afb..4306f1ec25 100644 --- a/vite.config.ts +++ b/vite.config.ts @@ -78,6 +78,7 @@ export default defineConfig({ '**/node_modules/**', '**/html-coverage/**', '**/libs/**', + '**/site/**', '**/bootstrap*.min.js', '**/popper*.min.js', '**/babel.config.cjs', @@ -87,6 +88,6 @@ export default defineConfig({ }, globals: true, include: ['**/phpmyfaq/assets/**/*.test.ts', '**/phpmyfaq/admin/assets/**/*.test.ts'], - exclude: ['**/node_modules/**', '.claude/**'], + exclude: ['**/node_modules/**', '.claude/**', 'site/**'], }, }); From 232b63a2b4e7fdeaefbe62b6af619fa3d522c283 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 12:31:28 +0200 Subject: [PATCH 11/14] fix: corrected reflection only configuration --- .../Functional/ControllerWebTestCase.php | 57 ++++++++++++++++++- 1 file changed, 54 insertions(+), 3 deletions(-) diff --git a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php index a57f499efa..5362676816 100644 --- a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php +++ b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php @@ -17,6 +17,9 @@ abstract class ControllerWebTestCase extends WebTestCase /** @var array}> */ private array $originalConfigurations = []; + /** @var array> */ + private array $addedConfigurationKeys = []; + protected function requestPublic(string $method, string $uri, array $parameters = [], array $server = []): Response { return $this->requestWithContext('public', $method, $uri, $parameters, $server); @@ -88,7 +91,34 @@ protected function overrideConfigurationValues(array $values, string $context = ]; } - $configProperty->setValue($configuration, array_merge($currentConfig, $values)); + $this->addedConfigurationKeys[$context] ??= []; + foreach ($values as $name => $value) { + $storedValue = $this->normalizeConfigurationValue($value); + $existingValue = $configuration->get((string) $name); + if ($existingValue === null) { + $configuration->add((string) $name, $storedValue); + $this->addedConfigurationKeys[$context][] = (string) $name; + } else { + $configuration->update([(string) $name => $storedValue]); + } + } + + $latestConfig = $configProperty->getValue($configuration); + self::assertIsArray($latestConfig); + $configProperty->setValue($configuration, array_merge($latestConfig, $values)); + } + + private function normalizeConfigurationValue(mixed $value): string + { + if (is_bool($value)) { + return $value ? 'true' : 'false'; + } + + if (is_array($value)) { + return (string) json_encode($value); + } + + return (string) $value; } protected function getConfiguration(string $context = 'public'): Configuration @@ -198,6 +228,8 @@ private function ensureClientForContext(string $context): HttpKernelBrowser $configuration = $container->get('phpmyfaq.configuration'); self::assertInstanceOf(Configuration::class, $configuration); + $configuration->update(['main.referenceURL' => 'https://localhost/']); + $reflection = new ReflectionClass(Configuration::class); $configProperty = $reflection->getProperty('config'); $currentConfig = $configProperty->getValue($configuration); @@ -212,14 +244,33 @@ private function ensureClientForContext(string $context): HttpKernelBrowser protected function tearDown(): void { - foreach ($this->originalConfigurations as $configurationSnapshot) { + foreach ($this->originalConfigurations as $context => $configurationSnapshot) { $configuration = $configurationSnapshot['configuration']; + $originalValues = $configurationSnapshot['values']; $reflection = new ReflectionClass(Configuration::class); $configProperty = $reflection->getProperty('config'); - $configProperty->setValue($configuration, $configurationSnapshot['values']); + + $addedKeys = $this->addedConfigurationKeys[$context] ?? []; + foreach ($addedKeys as $addedKey) { + $configuration->delete($addedKey); + } + + $restore = []; + foreach ($originalValues as $name => $value) { + if (in_array($name, $addedKeys, true)) { + continue; + } + $restore[$name] = $value; + } + if ($restore !== []) { + $configuration->update($restore); + } + + $configProperty->setValue($configuration, $originalValues); } $this->originalConfigurations = []; + $this->addedConfigurationKeys = []; self::$activeContext = null; parent::tearDown(); } From c1df85f23a6fafecb736e20a53c6cb2a3f0f9c20 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 14:57:40 +0200 Subject: [PATCH 12/14] fix: corrected review notes --- phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php | 40 +++++- .../Auth/Oidc/OidcIdTokenValidator.php | 67 ++++++++-- .../AzureAuthenticationController.php | 10 +- .../KeycloakAuthenticationController.php | 44 +++++-- .../Setup/Migration/AbstractMigration.php | 38 ++++++ .../Migration/Versions/Migration420Alpha.php | 4 +- phpmyfaq/src/phpMyFAQ/User.php | 6 +- phpmyfaq/src/phpMyFAQ/User/UserData.php | 4 + tests/phpMyFAQ/Auth/AuthKeycloakTest.php | 53 +++++++- .../Auth/Oidc/OidcIdTokenValidatorTest.php | 102 +++++++++++++++ .../Api/ConfigurationTabControllerTest.php | 5 +- .../AuthenticationControllerWebTest.php | 2 +- .../KeycloakAuthenticationControllerTest.php | 118 ++++++++++++++++-- ...eycloakAuthenticationControllerWebTest.php | 49 +++++++- .../Functional/ControllerWebTestCase.php | 13 +- tests/phpMyFAQ/User/UserDataTest.php | 18 +++ 16 files changed, 511 insertions(+), 62 deletions(-) diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php index 2f77154264..c2c3ca3f2f 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php @@ -48,7 +48,6 @@ public function __construct( */ public function create(string $login, #[SensitiveParameter] string $password, string $domain = ''): bool { - $result = false; $user = $this->createUser(); try { @@ -56,7 +55,11 @@ public function create(string $login, #[SensitiveParameter] string $password, st } catch (\Exception $exception) { $this->configuration ->getLogger() - ->error(sprintf('Keycloak user creation failed for "%s": %s', $login, $exception->getMessage())); + ->error(sprintf( + 'Keycloak user creation failed for "%s": %s', + $this->redactIdentifier($login), + $exception->getMessage(), + )); return false; } @@ -76,7 +79,7 @@ public function create(string $login, #[SensitiveParameter] string $password, st $this->assignUserToGroups($user->getUserId()); } - return $result; + return true; } public function update(string $login, #[SensitiveParameter] string $password): bool @@ -89,6 +92,9 @@ public function delete(string $login): bool return true; } + /** + * @throws Exception + */ public function checkCredentials( string $login, #[SensitiveParameter] @@ -99,7 +105,12 @@ public function checkCredentials( return false; } - if ($this->userExists($login)) { + $existingUser = $this->findUser($login); + if ($existingUser instanceof User) { + if ($this->shouldSynchronizeGroupsOnLogin()) { + $this->assignUserToGroups($existingUser->getUserId()); + } + return true; } @@ -140,10 +151,10 @@ private function getSubject(): string return trim((string) ($this->claims['sub'] ?? '')); } - private function userExists(string $login): bool + private function findUser(string $login): ?User { $user = $this->createUser(); - return $user->getUserByLogin($login, false); + return $user->getUserByLogin($login, false) ? $user : null; } private function shouldAssignGroups(): bool @@ -272,6 +283,23 @@ private function shouldSynchronizeGroupsOnLogin(): bool return $this->toBool($this->configuration->get(item: 'keycloak.groupSyncOnLogin')); } + private function redactIdentifier(string $identifier): string + { + if (str_contains($identifier, '@')) { + [$local, $domain] = explode('@', $identifier, 2); + return $local[0] . '***@' . $domain; + } + + if (mb_strlen($identifier) <= 3) { + return str_repeat('*', mb_strlen($identifier)); + } + + return mb_substr($identifier, 0, 3) . '…'; + } + + /** + * @throws Exception + */ private function createUser(): User { if ($this->userFactory instanceof Closure) { diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php index 0346a2ade7..93dc3e1215 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php @@ -45,8 +45,8 @@ public function validate( string $expectedAudience, string $expectedNonce, ): array { - if ($idToken === '') { - return []; + if (trim($idToken) === '') { + throw new RuntimeException('OIDC id_token is missing'); } [$encodedHeader, $encodedPayload, $encodedSignature] = $this->splitToken($idToken); @@ -106,13 +106,25 @@ private function validateClaims( string $expectedAudience, string $expectedNonce, ): void { - $issuer = trim((string) ($claims['iss'] ?? '')); - if ($issuer !== '' && $issuer !== $expectedIssuer) { + if (!array_key_exists('iss', $claims) || !is_string($claims['iss']) || trim($claims['iss']) === '') { + throw new RuntimeException('OIDC id_token is missing the issuer claim'); + } + if (trim($claims['iss']) !== $expectedIssuer) { throw new RuntimeException('OIDC issuer mismatch in id_token'); } - $audience = $claims['aud'] ?? null; - if ($audience !== null && !$this->audienceMatches($audience, $expectedAudience)) { + if (!array_key_exists('sub', $claims) || !is_string($claims['sub']) || trim($claims['sub']) === '') { + throw new RuntimeException('OIDC id_token is missing the subject claim'); + } + + if (!array_key_exists('aud', $claims)) { + throw new RuntimeException('OIDC id_token is missing the audience claim'); + } + $audience = $claims['aud']; + if (!$this->isValidAudience($audience)) { + throw new RuntimeException('OIDC id_token audience claim is invalid'); + } + if (!$this->audienceMatches($audience, $expectedAudience)) { throw new RuntimeException('OIDC audience mismatch in id_token'); } @@ -125,14 +137,21 @@ private function validateClaims( throw new RuntimeException('OIDC authorized party missing in id_token'); } - $nonce = trim((string) ($claims['nonce'] ?? '')); - if ($expectedNonce !== '' && $nonce !== '' && !hash_equals($expectedNonce, $nonce)) { - throw new RuntimeException('OIDC nonce mismatch in id_token'); + if ($expectedNonce !== '') { + if (!array_key_exists('nonce', $claims) || !is_string($claims['nonce']) || $claims['nonce'] === '') { + throw new RuntimeException('OIDC id_token is missing the nonce claim'); + } + if (!hash_equals($expectedNonce, $claims['nonce'])) { + throw new RuntimeException('OIDC nonce mismatch in id_token'); + } } $now = $this->getCurrentTimestamp(); - if (array_key_exists('exp', $claims) && is_numeric($claims['exp']) && (int) $claims['exp'] < $now) { + if (!array_key_exists('exp', $claims) || !is_numeric($claims['exp'])) { + throw new RuntimeException('OIDC id_token is missing the expiration claim'); + } + if ((int) $claims['exp'] < $now) { throw new RuntimeException('OIDC id_token has expired'); } @@ -140,9 +159,35 @@ private function validateClaims( throw new RuntimeException('OIDC id_token is not valid yet'); } - if (array_key_exists('iat', $claims) && is_numeric($claims['iat']) && (int) $claims['iat'] > ($now + 60)) { + if (!array_key_exists('iat', $claims) || !is_numeric($claims['iat'])) { + throw new RuntimeException('OIDC id_token is missing the issued-at claim'); + } + $issuedAt = (int) $claims['iat']; + if ($issuedAt > ($now + 60)) { throw new RuntimeException('OIDC id_token issued-at time is in the future'); } + if ($issuedAt < ($now - 86400)) { + throw new RuntimeException('OIDC id_token issued-at time is too far in the past'); + } + } + + private function isValidAudience(mixed $audience): bool + { + if (is_string($audience)) { + return trim($audience) !== ''; + } + + if (!is_array($audience) || $audience === []) { + return false; + } + + foreach ($audience as $entry) { + if (!is_string($entry) || trim($entry) === '') { + return false; + } + } + + return true; } /** diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php index 8b95e17dad..9bed7393d1 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/AzureAuthenticationController.php @@ -90,10 +90,16 @@ public function callback(Request $request): Response [$auth, $oAuth, $entraIdSession] = $this->buildAuthContext(); $code = Filter::filterVar($request->query->get('code'), FILTER_SANITIZE_SPECIAL_CHARS, ''); + $errorParam = Filter::filterVar($request->query->get('error'), FILTER_SANITIZE_SPECIAL_CHARS, ''); $error = Filter::filterVar($request->query->get('error_description'), FILTER_SANITIZE_SPECIAL_CHARS, ''); - if ($error !== '') { - $this->configuration->getLogger()->warning(sprintf('Azure callback error: %s', $error)); + if ($errorParam !== '' || $error !== '') { + $this->configuration + ->getLogger() + ->warning(sprintf( + 'Azure callback error: %s', + trim($errorParam . ($error !== '' ? ': ' . $error : '')), + )); return new RedirectResponse($this->configuration->getDefaultUrl()); } diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php index 8894feb521..a772d9d8e7 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php @@ -111,21 +111,22 @@ public function logout(): RedirectResponse try { $providerConfig = $this->providerConfigFactory->create(); - if (!$providerConfig->enabled || $providerConfig->discoveryUrl === '') { - return new RedirectResponse($this->configuration->getDefaultUrl()); - } - $idToken = $this->resolveLogoutIdToken($user); $user->deleteFromSession(); $this->oidcSession->clearIdToken(); + if (!$providerConfig->enabled || $providerConfig->discoveryUrl === '') { + return new RedirectResponse($this->configuration->getDefaultUrl()); + } + $discoveryDocument = $this->discoveryService->discover($providerConfig); $logoutUrl = $this->oidcClient->buildLogoutUrl($providerConfig, $discoveryDocument, $idToken); return new RedirectResponse($logoutUrl ?? $this->configuration->getDefaultUrl()); } catch (Exception) { $user->deleteFromSession(); + $this->oidcSession->clearIdToken(); return new RedirectResponse($this->configuration->getDefaultUrl()); } } @@ -171,18 +172,31 @@ public function callback(Request $request): Response $code, $authorizationState['verifier'], ); - $this->idTokenValidator->validate( + $idTokenClaims = $this->idTokenValidator->validate( (string) ($token['id_token'] ?? ''), $discoveryDocument, $providerConfig->client->clientId, $authorizationState['nonce'], ); $claims = $this->oidcClient->fetchUserInfo($discoveryDocument, (string) $token['access_token']); + + $idTokenSub = (string) ($idTokenClaims['sub'] ?? ''); + $userInfoSub = (string) ($claims['sub'] ?? ''); + if ($idTokenSub === '' || $userInfoSub === '' || !hash_equals($idTokenSub, $userInfoSub)) { + $this->configuration + ->getLogger() + ->warning('Keycloak subject mismatch between ID token and UserInfo; aborting login.'); + $this->oidcSession->clearAuthorizationState(); + return $redirect; + } + $login = $this->resolveLocalLogin($claims); $auth = new AuthKeycloak($this->configuration, $providerConfig, $claims, $login, $this->userFactory); if (!$auth->isValidLogin($login)) { - $this->configuration->getLogger()->warning(sprintf('Keycloak login not valid for user: %s', $login)); + $this->configuration + ->getLogger() + ->warning(sprintf('Keycloak login not valid for user: %s', $this->maskLogin($login))); $this->oidcSession->clearAuthorizationState(); return $redirect; } @@ -190,7 +204,7 @@ public function callback(Request $request): Response if (!$auth->checkCredentials($login, '')) { $this->configuration ->getLogger() - ->warning(sprintf('Keycloak credentials not valid for user: %s', $login)); + ->warning(sprintf('Keycloak credentials not valid for user: %s', $this->maskLogin($login))); $this->oidcSession->clearAuthorizationState(); return $redirect; } @@ -199,13 +213,15 @@ public function callback(Request $request): Response if (!$user->getUserByLogin($login)) { $this->configuration ->getLogger() - ->warning(sprintf('Keycloak user lookup failed for login: %s', $login)); + ->warning(sprintf('Keycloak user lookup failed for login: %s', $this->maskLogin($login))); $this->oidcSession->clearAuthorizationState(); return $redirect; } if (!$this->synchronizeKeycloakSubject($user, $claims)) { - $this->configuration->getLogger()->warning(sprintf('Keycloak subject mismatch for user: %s', $login)); + $this->configuration + ->getLogger() + ->warning(sprintf('Keycloak subject mismatch for user: %s', $this->maskLogin($login))); $this->oidcSession->clearAuthorizationState(); return $redirect; } @@ -276,6 +292,16 @@ private function resolveLocalLogin(array $claims): string return $subject; } + private function maskLogin(string $login): string + { + $login = trim($login); + if ($login === '') { + return ''; + } + + return 'sha256:' . substr(hash('sha256', $login), 0, 12); + } + /** @param array $claims */ private function synchronizeKeycloakSubject(CurrentUser $user, array $claims): bool { diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php b/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php index 5994d9d497..19ab1655d5 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php @@ -187,6 +187,44 @@ protected function createIndex(string $table, string $indexName, string|array $c return sprintf('CREATE INDEX IF NOT EXISTS %s ON %s (%s)', $indexName, $tableName, $columnList); } + /** + * Helper to create a UNIQUE index. + * + * On SQL Server, NULLs are treated as equal in unique indexes, so a filtered + * index is emitted to allow multiple NULL values. MySQL/MariaDB, PostgreSQL + * and SQLite already treat NULLs as distinct in unique indexes. + * + * @param string|string[] $columns + */ + protected function createUniqueIndex(string $table, string $indexName, string|array $columns): string + { + $tableName = $this->table($table); + $columns = (array) $columns; + $columnList = implode(', ', $columns); + + if ($this->isSqlServer()) { + $whereClause = implode(' AND ', array_map(static fn(string $col): string => sprintf( + '%s IS NOT NULL', + $col, + ), $columns)); + return sprintf( + "IF NOT EXISTS (SELECT * FROM sys.indexes WHERE name = '%s') " + . 'CREATE UNIQUE INDEX %s ON %s (%s) WHERE %s', + $indexName, + $indexName, + $tableName, + $columnList, + $whereClause, + ); + } + + if ($this->isMySql()) { + return sprintf('CREATE UNIQUE INDEX %s ON %s (%s)', $indexName, $tableName, $columnList); + } + + return sprintf('CREATE UNIQUE INDEX IF NOT EXISTS %s ON %s (%s)', $indexName, $tableName, $columnList); + } + /** * Helper to check if an index exists. * Returns a SQL query that checks for index existence. diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php index edcbf0fe94..1e529c7965 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Migration/Versions/Migration420Alpha.php @@ -228,8 +228,8 @@ public function up(OperationRecorder $recorder): void ); $recorder->addSql( - $this->createIndex('faquserdata', 'idx_faquserdata_keycloak_sub', 'keycloak_sub'), - 'Create keycloak_sub index on faquserdata', + $this->createUniqueIndex('faquserdata', 'idx_faquserdata_keycloak_sub', 'keycloak_sub'), + 'Create unique keycloak_sub index on faquserdata', ); $recorder->addConfig('api.rateLimit.interval', '3600'); $recorder->addConfig('queue.transport', 'database'); diff --git a/phpmyfaq/src/phpMyFAQ/User.php b/phpmyfaq/src/phpMyFAQ/User.php index fa869c98be..982a3604c8 100644 --- a/phpmyfaq/src/phpMyFAQ/User.php +++ b/phpmyfaq/src/phpMyFAQ/User.php @@ -821,7 +821,11 @@ public function getUserIdByKeycloakSub(string $keycloakSub): int $userData = $this->userdata->fetchAll('keycloak_sub', $keycloakSub); - return $userData['user_id']; + if (!is_array($userData) || !isset($userData['user_id'])) { + return 0; + } + + return (int) $userData['user_id']; } /** diff --git a/phpmyfaq/src/phpMyFAQ/User/UserData.php b/phpmyfaq/src/phpMyFAQ/User/UserData.php index 77141fbef6..a2e9103e45 100644 --- a/phpmyfaq/src/phpMyFAQ/User/UserData.php +++ b/phpmyfaq/src/phpMyFAQ/User/UserData.php @@ -311,6 +311,10 @@ public function save(): bool } if ($res === false) { + if (array_key_exists('keycloak_sub', $this->data)) { + return false; + } + $update = sprintf( " UPDATE diff --git a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php index 65d28952e7..5459cb11fd 100644 --- a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php +++ b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php @@ -38,6 +38,53 @@ public function testCheckCredentialsReturnsTrueForExistingUser(): void $this->assertTrue($auth->checkCredentials('john', '')); } + public function testCheckCredentialsSynchronizesGroupsForExistingUserWhenEnabled(): void + { + $configuration = $this->createStub(Configuration::class); + $configuration + ->method('get') + ->willReturnCallback(static fn(string $item): mixed => match ($item) { + 'keycloak.groupSyncOnLogin' => 'true', + 'keycloak.groupMapping' => '{"manage-users":"Administrators","faq-editors":"faq-editors"}', + 'keycloak.clientId' => 'phpmyfaq', + default => null, + }); + $configuration->method('getLogger')->willReturn($this->createMock(Logger::class)); + + $user = $this->createMock(User::class); + $user->expects($this->once())->method('getUserByLogin')->with('john', false)->willReturn(true); + $user->expects($this->once())->method('getUserId')->willReturn(42); + + $permission = $this->createMock(MediumPermission::class); + $permission->expects($this->once())->method('getUserGroups')->with(42)->willReturn([11]); + $permission->expects($this->once())->method('findOrCreateGroupByName')->with('Administrators')->willReturn(10); + $permission + ->expects($this->exactly(2)) + ->method('getGroupId') + ->willReturnCallback(static fn(string $groupName): int => match ($groupName) { + 'Administrators' => 10, + 'faq-editors' => 11, + default => 0, + }); + $permission->expects($this->once())->method('addToGroup')->with(42, 10); + $permission->expects($this->once())->method('removeFromGroup')->with(42, 11); + + $auth = new AuthKeycloak( + $configuration, + $this->createProviderConfig(autoProvision: false), + [ + 'preferred_username' => 'john', + 'realm_access' => ['roles' => ['manage-users']], + 'resource_access' => ['phpmyfaq' => ['roles' => []]], + ], + 'john', + static fn(): User => $user, + static fn(): MediumPermission => $permission, + ); + + $this->assertTrue($auth->checkCredentials('john', '')); + } + public function testCheckCredentialsReturnsFalseWhenAutoProvisionIsDisabled(): void { $user = $this->createMock(User::class); @@ -188,11 +235,7 @@ public function testCreateSynchronizesMappedGroupsWhenEnabled(): void $permission = $this->createMock(MediumPermission::class); $permission->expects($this->once())->method('getUserGroups')->with(42)->willReturn([10, 11]); - $permission - ->expects($this->once()) - ->method('findOrCreateGroupByName') - ->with('Administrators') - ->willReturn(10); + $permission->expects($this->once())->method('findOrCreateGroupByName')->with('Administrators')->willReturn(10); $permission ->expects($this->exactly(2)) ->method('getGroupId') diff --git a/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php b/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php index 03ea5bd989..7c5f225e68 100644 --- a/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php +++ b/tests/phpMyFAQ/Auth/Oidc/OidcIdTokenValidatorTest.php @@ -59,6 +59,7 @@ public function testValidateReturnsClaimsForValidIdToken(): void $claims = $validator->validate( $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-123', @@ -85,9 +86,11 @@ public function testValidateRejectsExpiredToken(): void $validator->validate( $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-123', + 'iat' => 1_699_999_900, 'exp' => 1_699_999_999, ]), $this->createDiscoveryDocument(), @@ -107,9 +110,11 @@ public function testValidateRejectsTokenThatIsNotValidYet(): void $validator->validate( $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-123', + 'iat' => 1_699_999_990, 'nbf' => 1_700_000_001, 'exp' => 1_700_000_060, ]), @@ -130,6 +135,7 @@ public function testValidateRejectsFutureIssuedAtTime(): void $validator->validate( $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-123', @@ -164,9 +170,11 @@ public function testValidateRejectsInvalidSignature(): void $validator->validate( $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-123', + 'iat' => 1_699_999_990, 'exp' => 1_700_000_060, ], $otherKey), $this->createDiscoveryDocument(), @@ -175,6 +183,100 @@ public function testValidateRejectsInvalidSignature(): void ); } + public function testValidateRejectsEmptyIdToken(): void + { + $validator = $this->createValidator(['keys' => [$this->jwk]], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token is missing')); + + $validator->validate('', $this->createDiscoveryDocument(), 'phpmyfaq', 'nonce-123'); + } + + public function testValidateRejectsMissingSubjectClaim(): void + { + $validator = $this->createValidator(['keys' => [$this->jwk]], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token is missing the subject claim')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'iat' => 1_699_999_990, + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsMissingNonceClaimWhenExpected(): void + { + $validator = $this->createValidator(['keys' => [$this->jwk]], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token is missing the nonce claim')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'iat' => 1_699_999_990, + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsMissingIssuedAtClaim(): void + { + $validator = $this->createValidator(['keys' => [$this->jwk]], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token is missing the issued-at claim')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + + public function testValidateRejectsIssuedAtTooFarInThePast(): void + { + $validator = $this->createValidator(['keys' => [$this->jwk]], 1_700_000_000); + + $this->expectExceptionObject(new \RuntimeException('OIDC id_token issued-at time is too far in the past')); + + $validator->validate( + $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-123', + 'iat' => 1_700_000_000 - 86_401, + 'exp' => 1_700_000_060, + ]), + $this->createDiscoveryDocument(), + 'phpmyfaq', + 'nonce-123', + ); + } + private function createDiscoveryDocument(): OidcDiscoveryDocument { return new OidcDiscoveryDocument( diff --git a/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php b/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php index 21d0ac8f2d..6c10c34a54 100644 --- a/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php +++ b/tests/phpMyFAQ/Controller/Administration/Api/ConfigurationTabControllerTest.php @@ -240,7 +240,10 @@ public function testListReturnsRenderedKeycloakConfigurationTabForAuthenticatedU self::assertSame(Response::HTTP_OK, $response->getStatusCode()); self::assertStringContainsString('data-config-key="keycloak.enable"', (string) $response->getContent()); self::assertStringContainsString('data-config-key="keycloak.clientId"', (string) $response->getContent()); - self::assertStringContainsString('data-config-key="keycloak.groupSyncOnLogin"', (string) $response->getContent()); + self::assertStringContainsString( + 'data-config-key="keycloak.groupSyncOnLogin"', + (string) $response->getContent(), + ); self::assertStringContainsString('data-config-key="keycloak.groupMapping"', (string) $response->getContent()); } diff --git a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php index e7d824aab5..88eaf47902 100644 --- a/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Administration/AuthenticationControllerWebTest.php @@ -27,7 +27,7 @@ public function testLoginShowsKeycloakAffordanceWhenEnabled(): void '/href="[^"]*auth\/keycloak\/authorize"/', (string) $response->getContent(), ); - self::assertResponseContains('Sign in with Keycloak', $response); + $this->assertResponseContains('Sign in with Keycloak', $response); } public function testLoginHidesKeycloakAffordanceWhenDisabled(): void diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php index d05fd39470..03dca1a8b5 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerTest.php @@ -131,11 +131,15 @@ public function testLogoutReturnsProviderLogoutResponse(): void $currentUser->expects($this->never())->method('getUserData'); $currentUser->expects($this->once())->method('deleteFromSession'); - $controller = $this->createController([ - new MockResponse( - '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', - ), - ], $oidcSession, static fn(): CurrentUser => $currentUser); + $controller = $this->createController( + [ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + ], + $oidcSession, + static fn(): CurrentUser => $currentUser, + ); $response = $controller->logout(); @@ -147,6 +151,44 @@ public function testLogoutReturnsProviderLogoutResponse(): void $this->assertSame('', $oidcSession->getIdToken()); } + public function testLogoutClearsLocalSessionWhenProviderIsDisabled(): void + { + $reflection = new \ReflectionClass(Configuration::class); + $property = $reflection->getProperty('config'); + /** @var array $config */ + $config = $property->getValue($this->configuration); + $previousEnable = $config['keycloak.enable'] ?? null; + $config['keycloak.enable'] = 'false'; + $property->setValue($this->configuration, $config); + + try { + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setIdToken('session-id-token'); + + $currentUser = $this->createMock(CurrentUser::class); + $currentUser->expects($this->once())->method('deleteFromSession'); + + $controller = $this->createController([], $oidcSession, static fn(): CurrentUser => $currentUser); + + $response = $controller->logout(); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getIdToken()); + } finally { + /** @var array $restoredConfig */ + $restoredConfig = $property->getValue($this->configuration); + if ($previousEnable === null) { + unset($restoredConfig['keycloak.enable']); + } else { + $restoredConfig['keycloak.enable'] = $previousEnable; + } + $property->setValue($this->configuration, $restoredConfig); + } + } + public function testLogoutFallsBackToPersistedJwtIdToken(): void { $session = new Session(new MockArraySessionStorage()); @@ -154,16 +196,22 @@ public function testLogoutFallsBackToPersistedJwtIdToken(): void $oidcSession = new OidcSession($session); $currentUser = $this->createMock(CurrentUser::class); - $currentUser->expects($this->once())->method('getUserData')->with('jwt')->willReturn( - '{"id_token":"persisted-id-token","userinfo":{"sub":"123"}}', - ); + $currentUser + ->expects($this->once()) + ->method('getUserData') + ->with('jwt') + ->willReturn('{"id_token":"persisted-id-token","userinfo":{"sub":"123"}}'); $currentUser->expects($this->once())->method('deleteFromSession'); - $controller = $this->createController([ - new MockResponse( - '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', - ), - ], $oidcSession, static fn(): CurrentUser => $currentUser); + $controller = $this->createController( + [ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + ], + $oidcSession, + static fn(): CurrentUser => $currentUser, + ); $response = $controller->logout(); @@ -191,9 +239,11 @@ public function testCallbackStoresUserSessionDataOnSuccessfulLogin(): void { $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => '123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-456', + 'iat' => time(), 'exp' => time() + 300, ]); @@ -263,9 +313,11 @@ public function testCallbackResolvesLocalLoginFromStoredKeycloakSubject(): void { $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-456', + 'iat' => time(), 'exp' => time() + 300, ]); @@ -320,9 +372,11 @@ public function testCallbackReturnsFailureWhenStoredKeycloakSubjectDoesNotMatch( { $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => ['phpmyfaq'], 'azp' => 'phpmyfaq', 'nonce' => 'nonce-456', + 'iat' => time(), 'exp' => time() + 300, ]); @@ -375,6 +429,44 @@ public function testCallbackReturnsFailureWhenStoredKeycloakSubjectDoesNotMatch( $this->assertSame('', $oidcSession->getAuthorizationState()['state']); } + public function testCallbackReturnsFailureWhenIdTokenSubjectDoesNotMatchUserInfoSubject(): void + { + $idToken = $this->signToken([ + 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'id-token-subject', + 'aud' => ['phpmyfaq'], + 'azp' => 'phpmyfaq', + 'nonce' => 'nonce-456', + 'iat' => time(), + 'exp' => time() + 300, + ]); + + $session = new Session(new MockArraySessionStorage()); + $session->start(); + $oidcSession = new OidcSession($session); + $oidcSession->setAuthorizationState('state-123', 'nonce-456', 'verifier-789'); + + $controller = $this->createController([ + new MockResponse( + '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', + ), + new MockResponse('{"access_token":"access","refresh_token":"refresh","id_token":"' . $idToken . '"}'), + new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), + new MockResponse( + '{"sub":"userinfo-subject","preferred_username":"john","email":"john@example.com","name":"John Doe"}', + ), + ], $oidcSession); + + $response = $controller->callback(new Request([ + 'code' => 'test-code', + 'state' => 'state-123', + ])); + + $this->assertInstanceOf(RedirectResponse::class, $response); + $this->assertSame($this->configuration->getDefaultUrl(), $response->headers->get('Location')); + $this->assertSame('', $oidcSession->getAuthorizationState()['state']); + } + public function testCallbackReturnsFailureWhenIdTokenIssuerDoesNotMatch(): void { $session = new Session(new MockArraySessionStorage()); diff --git a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php index 4eed290b82..1c12b11536 100644 --- a/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php +++ b/tests/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationControllerWebTest.php @@ -78,17 +78,30 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => 'phpmyfaq', 'azp' => 'phpmyfaq', + 'iat' => time(), 'exp' => time() + 300, ]); $expectedNonce = ''; $expectedVerifier = ''; $responseIndex = 0; - $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use (&$responseIndex, &$idToken, &$expectedNonce, &$expectedVerifier): MockResponse { + $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use ( + &$responseIndex, + &$idToken, + &$expectedNonce, + &$expectedVerifier, + ): MockResponse { $responseIndex++; + $expectedMethod = match ($responseIndex) { + 3 => 'POST', + default => 'GET', + }; + self::assertSame($expectedMethod, $method, sprintf('Unexpected HTTP method for call #%d', $responseIndex)); + return match ($responseIndex) { 1, 2 => new MockResponse( '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', @@ -100,9 +113,11 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => 'phpmyfaq', 'azp' => 'phpmyfaq', 'nonce' => $expectedNonce, + 'iat' => time(), 'exp' => time() + 300, ]); @@ -111,7 +126,9 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon ); })(), 4 => new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), - 5 => new MockResponse('{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}'), + 5 => new MockResponse( + '{"sub":"subject-123","preferred_username":"admin","email":"admin@example.com","name":"Admin User"}', + ), default => throw new \RuntimeException('Unexpected HTTP call in callback test: ' . $url), }; }); @@ -124,7 +141,10 @@ public function testCallbackCompletesSuccessfulLoginFlowWithMockedProviderRespon $authorizeResponse = $this->requestPublic('GET', '/auth/keycloak/authorize'); self::assertResponseStatusCodeSame(Response::HTTP_FOUND, $authorizeResponse); - parse_str((string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), $authorizeQuery); + parse_str( + (string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), + $authorizeQuery, + ); $authorizationState = $oidcSession->getAuthorizationState(); $expectedNonce = $authorizationState['nonce']; $expectedVerifier = $authorizationState['verifier']; @@ -160,9 +180,19 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void $expectedVerifier = ''; $responseIndex = 0; - $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use (&$responseIndex, &$expectedNonce, &$expectedVerifier): MockResponse { + $httpClient = new MockHttpClient(function (string $method, string $url, array $options) use ( + &$responseIndex, + &$expectedNonce, + &$expectedVerifier, + ): MockResponse { $responseIndex++; + $expectedMethod = match ($responseIndex) { + 3 => 'POST', + default => 'GET', + }; + self::assertSame($expectedMethod, $method, sprintf('Unexpected HTTP method for call #%d', $responseIndex)); + return match ($responseIndex) { 1, 2, 6 => new MockResponse( '{"issuer":"https://sso.example.test/realms/phpmyfaq","authorization_endpoint":"https://sso.example.test/auth","token_endpoint":"https://sso.example.test/token","userinfo_endpoint":"https://sso.example.test/userinfo","jwks_uri":"https://sso.example.test/jwks","end_session_endpoint":"https://sso.example.test/logout"}', @@ -173,9 +203,11 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void $idToken = $this->signToken([ 'iss' => 'https://sso.example.test/realms/phpmyfaq', + 'sub' => 'subject-123', 'aud' => 'phpmyfaq', 'azp' => 'phpmyfaq', 'nonce' => $expectedNonce, + 'iat' => time(), 'exp' => time() + 300, ]); @@ -184,7 +216,9 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void ); })(), 4 => new MockResponse(json_encode(['keys' => [$this->jwk]], JSON_THROW_ON_ERROR)), - 5 => new MockResponse('{"preferred_username":"admin","email":"admin@example.com","name":"Admin User"}'), + 5 => new MockResponse( + '{"sub":"subject-123","preferred_username":"admin","email":"admin@example.com","name":"Admin User"}', + ), default => throw new \RuntimeException('Unexpected HTTP call in logout test: ' . $url), }; }); @@ -196,7 +230,10 @@ public function testLogoutBuildsProviderRedirectWithSessionIdToken(): void $authorizeResponse = $this->requestPublic('GET', '/auth/keycloak/authorize'); self::assertResponseStatusCodeSame(Response::HTTP_FOUND, $authorizeResponse); - parse_str((string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), $authorizeQuery); + parse_str( + (string) parse_url((string) $authorizeResponse->headers->get('Location'), PHP_URL_QUERY), + $authorizeQuery, + ); $authorizationState = $oidcSession->getAuthorizationState(); $expectedNonce = $authorizationState['nonce']; $expectedVerifier = $authorizationState['verifier']; diff --git a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php index 5362676816..a015e63146 100644 --- a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php +++ b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php @@ -92,20 +92,23 @@ protected function overrideConfigurationValues(array $values, string $context = } $this->addedConfigurationKeys[$context] ??= []; + $normalizedValues = []; foreach ($values as $name => $value) { + $key = (string) $name; $storedValue = $this->normalizeConfigurationValue($value); - $existingValue = $configuration->get((string) $name); + $normalizedValues[$key] = $storedValue; + $existingValue = $configuration->get($key); if ($existingValue === null) { - $configuration->add((string) $name, $storedValue); - $this->addedConfigurationKeys[$context][] = (string) $name; + $configuration->add($key, $storedValue); + $this->addedConfigurationKeys[$context][] = $key; } else { - $configuration->update([(string) $name => $storedValue]); + $configuration->update([$key => $storedValue]); } } $latestConfig = $configProperty->getValue($configuration); self::assertIsArray($latestConfig); - $configProperty->setValue($configuration, array_merge($latestConfig, $values)); + $configProperty->setValue($configuration, array_merge($latestConfig, $normalizedValues)); } private function normalizeConfigurationValue(mixed $value): string diff --git a/tests/phpMyFAQ/User/UserDataTest.php b/tests/phpMyFAQ/User/UserDataTest.php index 522cb768ad..8aab4920f2 100644 --- a/tests/phpMyFAQ/User/UserDataTest.php +++ b/tests/phpMyFAQ/User/UserDataTest.php @@ -112,6 +112,24 @@ public function testSave(): void $this->assertTrue($result); } + public function testSaveDoesNotRunFallbackUpdateWhenKeycloakSubIsPresent(): void + { + $queryResults = [true, false]; + $this->database + ->method('query') + ->willReturnCallback(static function () use (&$queryResults): bool { + return array_shift($queryResults) ?? true; + }); + $this->database->method('numRows')->willReturn(1); + $this->database->method('fetchArray')->willReturn(['display_name' => 'Old']); + $this->database->method('escape')->willReturnArgument(0); + + $this->userData->load(1); + + $this->assertFalse($this->userData->set(['display_name', 'keycloak_sub'], ['Admin', 'subject-123'])); + $this->assertSame([], $queryResults); + } + public function testSetReturnsFalseWhenFieldAndValueCountsDoNotMatch(): void { $this->assertFalse($this->userData->set(['display_name', 'email'], ['value'])); From 09ae590b31a35ae5395db76ff577ce036e1a996e Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 15:28:49 +0200 Subject: [PATCH 13/14] fix: corrected review notes --- phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php | 37 ++++++++++++++++--- .../Auth/Oidc/OidcIdTokenValidator.php | 11 ++++-- .../KeycloakAuthenticationController.php | 16 +++++++- .../Setup/Migration/AbstractMigration.php | 7 +++- phpmyfaq/src/phpMyFAQ/User/UserData.php | 9 ++++- tests/phpMyFAQ/Auth/AuthKeycloakTest.php | 9 +++-- .../Functional/ControllerWebTestCase.php | 2 +- 7 files changed, 72 insertions(+), 19 deletions(-) diff --git a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php index c2c3ca3f2f..b89bf86eaa 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/AuthKeycloak.php @@ -67,13 +67,35 @@ public function create(string $login, #[SensitiveParameter] string $password, st return false; } + try { + $saved = $user->setUserData([ + 'display_name' => $this->getDisplayName(), + 'email' => $this->getEmail(), + 'keycloak_sub' => $this->getSubject(), + ]); + } catch (\Exception $exception) { + $this->configuration + ->getLogger() + ->error(sprintf( + 'Keycloak user data persistence failed for "%s": %s', + $this->redactIdentifier($login), + $exception->getMessage(), + )); + return false; + } + + if (!$saved) { + $this->configuration + ->getLogger() + ->error(sprintf( + 'Keycloak user data persistence returned false for "%s"', + $this->redactIdentifier($login), + )); + return false; + } + $user->setStatus('active'); $user->setAuthSource(AuthenticationSourceType::AUTH_KEYCLOAK->value); - $user->setUserData([ - 'display_name' => $this->getDisplayName(), - 'email' => $this->getEmail(), - 'keycloak_sub' => $this->getSubject(), - ]); if ($this->shouldAssignGroups()) { $this->assignUserToGroups($user->getUserId()); @@ -280,7 +302,10 @@ private function toBool(mixed $value): bool private function shouldSynchronizeGroupsOnLogin(): bool { - return $this->toBool($this->configuration->get(item: 'keycloak.groupSyncOnLogin')); + return ( + $this->toBool($this->configuration->get(item: 'keycloak.groupSyncOnLogin')) + && $this->configuration->get(item: 'security.permLevel') === 'medium' + ); } private function redactIdentifier(string $identifier): string diff --git a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php index 93dc3e1215..1438504575 100644 --- a/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php +++ b/phpmyfaq/src/phpMyFAQ/Auth/Oidc/OidcIdTokenValidator.php @@ -151,12 +151,17 @@ private function validateClaims( if (!array_key_exists('exp', $claims) || !is_numeric($claims['exp'])) { throw new RuntimeException('OIDC id_token is missing the expiration claim'); } - if ((int) $claims['exp'] < $now) { + if ($now >= (int) $claims['exp']) { throw new RuntimeException('OIDC id_token has expired'); } - if (array_key_exists('nbf', $claims) && is_numeric($claims['nbf']) && (int) $claims['nbf'] > $now) { - throw new RuntimeException('OIDC id_token is not valid yet'); + if (array_key_exists('nbf', $claims)) { + if (!is_numeric($claims['nbf'])) { + throw new RuntimeException('OIDC id_token has a non-numeric nbf claim'); + } + if ($now < (int) $claims['nbf']) { + throw new RuntimeException('OIDC id_token is not valid yet'); + } } if (!array_key_exists('iat', $claims) || !is_numeric($claims['iat'])) { diff --git a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php index a772d9d8e7..04e75cd983 100644 --- a/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php +++ b/phpmyfaq/src/phpMyFAQ/Controller/Frontend/KeycloakAuthenticationController.php @@ -191,6 +191,10 @@ public function callback(Request $request): Response } $login = $this->resolveLocalLogin($claims); + if ($login === '') { + $this->oidcSession->clearAuthorizationState(); + return $redirect; + } $auth = new AuthKeycloak($this->configuration, $providerConfig, $claims, $login, $this->userFactory); if (!$auth->isValidLogin($login)) { @@ -289,7 +293,13 @@ private function resolveLocalLogin(array $claims): string return $email; } - return $subject; + $this->configuration + ->getLogger() + ->warning( + 'Keycloak login rejected: claims are missing both preferred_username and email; refusing to auto-provision the sub.', + ); + + return ''; } private function maskLogin(string $login): string @@ -299,7 +309,9 @@ private function maskLogin(string $login): string return ''; } - return 'sha256:' . substr(hash('sha256', $login), 0, 12); + $secret = (string) $this->configuration->get('security.salt'); + + return 'hmac:' . substr(hash_hmac('sha256', $login, $secret), 0, 12); } /** @param array $claims */ diff --git a/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php b/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php index 19ab1655d5..12f6402c9e 100644 --- a/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php +++ b/phpmyfaq/src/phpMyFAQ/Setup/Migration/AbstractMigration.php @@ -169,8 +169,10 @@ protected function createIndex(string $table, string $indexName, string|array $c if ($this->isSqlServer()) { return sprintf( - "IF NOT EXISTS (SELECT * FROM sys.indexes WHERE name = '%s') " . 'CREATE INDEX %s ON %s (%s)', + "IF NOT EXISTS (SELECT * FROM sys.indexes WHERE name = '%s' AND object_id = OBJECT_ID('%s')) " + . 'CREATE INDEX %s ON %s (%s)', $indexName, + $tableName, $indexName, $tableName, $columnList, @@ -208,9 +210,10 @@ protected function createUniqueIndex(string $table, string $indexName, string|ar $col, ), $columns)); return sprintf( - "IF NOT EXISTS (SELECT * FROM sys.indexes WHERE name = '%s') " + "IF NOT EXISTS (SELECT * FROM sys.indexes WHERE name = '%s' AND object_id = OBJECT_ID('%s')) " . 'CREATE UNIQUE INDEX %s ON %s (%s) WHERE %s', $indexName, + $tableName, $indexName, $tableName, $columnList, diff --git a/phpmyfaq/src/phpMyFAQ/User/UserData.php b/phpmyfaq/src/phpMyFAQ/User/UserData.php index a2e9103e45..3c0b7560fd 100644 --- a/phpmyfaq/src/phpMyFAQ/User/UserData.php +++ b/phpmyfaq/src/phpMyFAQ/User/UserData.php @@ -279,6 +279,11 @@ public function load(int $userId): bool */ public function save(): bool { + $keycloakSubRaw = $this->data['keycloak_sub'] ?? null; + $keycloakSubValue = is_string($keycloakSubRaw) && trim($keycloakSubRaw) !== '' + ? "'" . $this->configuration->getDb()->escape($keycloakSubRaw) . "'" + : 'NULL'; + $update = sprintf( " UPDATE @@ -287,7 +292,7 @@ public function save(): bool last_modified = '%s', display_name = '%s', email = '%s', - keycloak_sub = '%s', + keycloak_sub = %s, is_visible = %d, twofactor_enabled = %d, secret = '%s' @@ -297,7 +302,7 @@ public function save(): bool date(format: 'YmdHis', timestamp: Request::createFromGlobals()->server->get('REQUEST_TIME')), $this->configuration->getDb()->escape((string) ($this->data['display_name'] ?? '')), $this->configuration->getDb()->escape((string) ($this->data['email'] ?? '')), - $this->configuration->getDb()->escape((string) ($this->data['keycloak_sub'] ?? '')), + $keycloakSubValue, $this->data['is_visible'] ?? 0, $this->data['twofactor_enabled'] ?? 0, $this->data['secret'] ?? '', diff --git a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php index 5459cb11fd..5a6ad096a0 100644 --- a/tests/phpMyFAQ/Auth/AuthKeycloakTest.php +++ b/tests/phpMyFAQ/Auth/AuthKeycloakTest.php @@ -47,6 +47,7 @@ public function testCheckCredentialsSynchronizesGroupsForExistingUserWhenEnabled 'keycloak.groupSyncOnLogin' => 'true', 'keycloak.groupMapping' => '{"manage-users":"Administrators","faq-editors":"faq-editors"}', 'keycloak.clientId' => 'phpmyfaq', + 'security.permLevel' => 'medium', default => null, }); $configuration->method('getLogger')->willReturn($this->createMock(Logger::class)); @@ -125,7 +126,8 @@ public function testCheckCredentialsAutoProvisionsUserWhenEnabled(): void 'display_name' => 'John Doe', 'email' => 'john@example.com', 'keycloak_sub' => 'subject-123', - ]); + ]) + ->willReturn(true); $auth = new AuthKeycloak( $configuration, @@ -169,7 +171,8 @@ public function testCreateAssignsMappedGroupsFromKeycloakRoles(): void 'display_name' => 'John Doe', 'email' => 'john@example.com', 'keycloak_sub' => 'subject-123', - ]); + ]) + ->willReturn(true); $user->expects($this->once())->method('getUserId')->willReturn(42); $permission = $this->createMock(MediumPermission::class); @@ -230,7 +233,7 @@ public function testCreateSynchronizesMappedGroupsWhenEnabled(): void $user->expects($this->once())->method('createUser')->with('john', '', '')->willReturn(true); $user->expects($this->once())->method('setStatus')->with('active'); $user->expects($this->once())->method('setAuthSource')->with(AuthenticationSourceType::AUTH_KEYCLOAK->value); - $user->expects($this->once())->method('setUserData'); + $user->expects($this->once())->method('setUserData')->willReturn(true); $user->expects($this->once())->method('getUserId')->willReturn(42); $permission = $this->createMock(MediumPermission::class); diff --git a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php index a015e63146..e7f93e10e4 100644 --- a/tests/phpMyFAQ/Functional/ControllerWebTestCase.php +++ b/tests/phpMyFAQ/Functional/ControllerWebTestCase.php @@ -118,7 +118,7 @@ private function normalizeConfigurationValue(mixed $value): string } if (is_array($value)) { - return (string) json_encode($value); + return json_encode($value, JSON_THROW_ON_ERROR); } return (string) $value; From ffb1e2b6b4d7e8b98ceff54d56d12f7cbc970080 Mon Sep 17 00:00:00 2001 From: Thorsten Rinne Date: Sun, 19 Apr 2026 15:43:55 +0200 Subject: [PATCH 14/14] fix: don't build the documentation --- .github/workflows/build.yml | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 85a6015fd3..269bb6c0ba 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -62,14 +62,6 @@ jobs: package-manager-cache: false node-version: 24 - - name: Setup Python for docs build - uses: actions/setup-python@v6 - with: - python-version: '3.x' - - - name: Install MkDocs - run: python -m pip install --upgrade pip mkdocs - - name: Setup pnpm uses: pnpm/action-setup@v6 with: @@ -98,6 +90,3 @@ jobs: - name: Run Vitest tests run: pnpm test - - - name: Build documentation - run: mkdocs build --strict