From 610aec0d87469b3a66741bcb7a7f5e8b052d1105 Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Mon, 11 Nov 2024 18:52:31 +0100 Subject: [PATCH 01/14] Event based provisioning changes --- lib/Controller/LoginController.php | 33 +- lib/Event/UserAccountChangeEvent.php | 87 ++++ lib/Event/UserAccountChangeResult.php | 74 +++ lib/Service/ProvisioningDeniedException.php | 69 +++ lib/Service/ProvisioningEventService.php | 161 ++++++ .../unit/MagentaCloud/OpenidTokenTestCase.php | 141 ++++++ .../ProvisioningEventServiceTest.php | 461 ++++++++++++++++++ tests/unit/MagentaCloud/RegistrationsTest.php | 33 ++ 8 files changed, 1053 insertions(+), 6 deletions(-) create mode 100644 lib/Event/UserAccountChangeEvent.php create mode 100644 lib/Event/UserAccountChangeResult.php create mode 100644 lib/Service/ProvisioningDeniedException.php create mode 100644 lib/Service/ProvisioningEventService.php create mode 100644 tests/unit/MagentaCloud/OpenidTokenTestCase.php create mode 100644 tests/unit/MagentaCloud/ProvisioningEventServiceTest.php create mode 100644 tests/unit/MagentaCloud/RegistrationsTest.php diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 970ef16e..dd4a2b4b 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -23,6 +23,7 @@ use OCA\UserOIDC\Service\DiscoveryService; use OCA\UserOIDC\Service\LdapService; use OCA\UserOIDC\Service\ProviderService; +use OCA\UserOIDC\Service\ProvisioningDeniedException; use OCA\UserOIDC\Service\ProvisioningService; use OCA\UserOIDC\Vendor\Firebase\JWT\JWT; use OCP\AppFramework\Db\DoesNotExistException; @@ -466,15 +467,35 @@ public function code(string $state = '', string $code = '', string $scope = '', } if ($autoProvisionAllowed) { - $softAutoProvisionAllowed = (!isset($oidcSystemConfig['soft_auto_provision']) || $oidcSystemConfig['soft_auto_provision']); - if (!$softAutoProvisionAllowed && $userFromOtherBackend !== null) { + // $softAutoProvisionAllowed = (!isset($oidcSystemConfig['soft_auto_provision']) || $oidcSystemConfig['soft_auto_provision']); + // if (!$softAutoProvisionAllowed && $userFromOtherBackend !== null) { // if soft auto-provisioning is disabled, // we refuse login for a user that already exists in another backend - $message = $this->l10n->t('User conflict'); - return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); + // $message = $this->l10n->t('User conflict'); + // return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); + // } + + // TODO: (proposal) refactor all provisioning strategies into event handlers + $user = null; + + try { + $user = $this->provisioningService->provisionUser($userId, $providerId, $idTokenPayload, $userFromOtherBackend); + } catch (ProvisioningDeniedException $denied) { + // TODO MagentaCLOUD should upstream the exception handling + $redirectUrl = $denied->getRedirectUrl(); + if ($redirectUrl === null) { + $message = $this->l10n->t('Failed to provision user'); + return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => $denied->getMessage()]); + } else { + // error response is a redirect, e.g. to a booking site + // so that you can immediately get the registration page + return new RedirectResponse($redirectUrl); + } } + // use potential user from other backend, create it in our backend if it does not exist - $user = $this->provisioningService->provisionUser($userId, $providerId, $idTokenPayload, $userFromOtherBackend); + // $user = $this->provisioningService->provisionUser($userId, $providerId, $idTokenPayload, $userFromOtherBackend); + // no default exception handling to pass on unittest assertion failures } else { // when auto provision is disabled, we assume the user has been created by another user backend (or manually) $user = $userFromOtherBackend; @@ -714,7 +735,7 @@ public function backChannelLogout(string $providerIdentifier, string $logout_tok * @return JSONResponse */ private function getBackchannelLogoutErrorResponse( - string $error, string $description, array $throttleMetadata = [], + string $error, string $description, array $throttleMetadata = [], ?bool $throttle = null, ): JSONResponse { $this->logger->debug('Backchannel logout error. ' . $error . ' ; ' . $description); return new JSONResponse( diff --git a/lib/Event/UserAccountChangeEvent.php b/lib/Event/UserAccountChangeEvent.php new file mode 100644 index 00000000..c6b698aa --- /dev/null +++ b/lib/Event/UserAccountChangeEvent.php @@ -0,0 +1,87 @@ + + * + * @license GNU AGPL version 3 or any later version + * + */ + +declare(strict_types=1); + +namespace OCA\UserOIDC\Event; + +use OCP\EventDispatcher\Event; + +/** + * Event to provide custom mapping logic based on the OIDC token data + * In order to avoid further processing the event propagation should be stopped + * in the listener after processing as the value might get overwritten afterwards + * by other listeners through $event->stopPropagation(); + */ +class UserAccountChangeEvent extends Event { + private $uid; + private $displayname; + private $mainEmail; + private $quota; + private $claims; + private $result; + + + public function __construct(string $uid, ?string $displayname, ?string $mainEmail, ?string $quota, object $claims, bool $accessAllowed = false) { + parent::__construct(); + $this->uid = $uid; + $this->displayname = $displayname; + $this->mainEmail = $mainEmail; + $this->quota = $quota; + $this->claims = $claims; + $this->result = new UserAccountChangeResult($accessAllowed, 'default'); + } + + /** + * @return get event username (uid) + */ + public function getUid(): string { + return $this->uid; + } + + /** + * @return get event displayname + */ + public function getDisplayName(): ?string { + return $this->displayname; + } + + /** + * @return get event main email + */ + public function getMainEmail(): ?string { + return $this->mainEmail; + } + + /** + * @return get event quota + */ + public function getQuota(): ?string { + return $this->quota; + } + + /** + * @return array the array of claim values associated with the event + */ + public function getClaims(): object { + return $this->claims; + } + + /** + * @return value for the logged in user attribute + */ + public function getResult(): UserAccountChangeResult { + return $this->result; + } + + public function setResult(bool $accessAllowed, string $reason = '', ?string $redirectUrl = null) : void { + $this->result = new UserAccountChangeResult($accessAllowed, $reason, $redirectUrl); + } +} diff --git a/lib/Event/UserAccountChangeResult.php b/lib/Event/UserAccountChangeResult.php new file mode 100644 index 00000000..660e78f9 --- /dev/null +++ b/lib/Event/UserAccountChangeResult.php @@ -0,0 +1,74 @@ + + * + * @license GNU AGPL version 3 or any later version + * + */ + +declare(strict_types=1); + +namespace OCA\UserOIDC\Event; + +/** + * Event to provide custom mapping logic based on the OIDC token data + * In order to avoid further processing the event propagation should be stopped + * in the listener after processing as the value might get overwritten afterwards + * by other listeners through $event->stopPropagation(); + */ +class UserAccountChangeResult { + + /** @var bool */ + private $accessAllowed; + /** @var string */ + private $reason; + /** @var string */ + private $redirectUrl; + + public function __construct(bool $accessAllowed, string $reason = '', ?string $redirectUrl = null) { + $this->accessAllowed = $accessAllowed; + $this->redirectUrl = $redirectUrl; + $this->reason = $reason; + } + + /** + * @return value for the logged in user attribute + */ + public function isAccessAllowed(): bool { + return $this->accessAllowed; + } + + public function setAccessAllowed(bool $accessAllowed): void { + $this->accessAllowed = $accessAllowed; + } + + /** + * @return get optional alternate redirect address + */ + public function getRedirectUrl(): ?string { + return $this->redirectUrl; + } + + /** + * @return set optional alternate redirect address + */ + public function setRedirectUrl(?string $redirectUrl): void { + $this->redirectUrl = $redirectUrl; + } + + /** + * @return get decision reason + */ + public function getReason(): string { + return $this->reason; + } + + /** + * @return set decision reason + */ + public function setReason(string $reason): void { + $this->reason = $reason; + } +} diff --git a/lib/Service/ProvisioningDeniedException.php b/lib/Service/ProvisioningDeniedException.php new file mode 100644 index 00000000..58c2fb9b --- /dev/null +++ b/lib/Service/ProvisioningDeniedException.php @@ -0,0 +1,69 @@ + + * + * @license AGPL-3.0 + * + * This code is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License, version 3, + * as published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License, version 3, + * along with this program. If not, see + * + */ + +namespace OCA\UserOIDC\Service; + +/** + * Exception if the precondition of the config update method isn't met + * @since 1.4.0 + */ +class ProvisioningDeniedException extends \Exception { + private $redirectUrl; + + /** + * Exception constructor including an option redirect url. + * + * @param string $message The error message. It will be not revealed to the + * the user (unless the hint is empty) and thus + * should be not translated. + * @param string $hint A useful message that is presented to the end + * user. It should be translated, but must not + * contain sensitive data. + * @param int $code Set default to 403 (Forbidden) + * @param \Exception|null $previous + */ + public function __construct(string $message, ?string $redirectUrl = null, int $code = 403, \Exception $previous = null) { + parent::__construct($message, $code, $previous); + $this->redirectUrl = $redirectUrl; + } + + /** + * Read optional failure redirect if available + * @return string|null + */ + public function getRedirectUrl(): ?string { + return $this->redirectUrl; + } + + /** + * Include redirect in string serialisation. + * + * @return string + */ + public function __toString(): string { + $redirect = $this->redirectUrl ?? ''; + return __CLASS__ . ": [{$this->code}]: {$this->message} ({$redirect})\n"; + } +} diff --git a/lib/Service/ProvisioningEventService.php b/lib/Service/ProvisioningEventService.php new file mode 100644 index 00000000..465f8d24 --- /dev/null +++ b/lib/Service/ProvisioningEventService.php @@ -0,0 +1,161 @@ + + * + * @author Bernd Rederlechner + * + * @license GNU AGPL version 3 or any later version + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +declare(strict_types=1); + +namespace OCA\UserOIDC\Service; + +use OCA\UserOIDC\Db\UserMapper; +use OCA\UserOIDC\Event\AttributeMappedEvent; +use OCA\UserOIDC\Event\UserAccountChangeEvent; +use OCP\DB\Exception; +use OCP\EventDispatcher\IEventDispatcher; +use OCP\IGroupManager; +use OCP\ILogger; +use OCP\IUser; +use OCP\IUserManager; +use Psr\Container\ContainerExceptionInterface; +use Psr\Container\NotFoundExceptionInterface; + +// FIXME there should be an interface for both variations +class ProvisioningEventService extends ProvisioningService { + + /** @var IEventDispatcher */ + private $eventDispatcher; + + /** @var ILogger */ + private $logger; + + /** @var IUserManager */ + private $userManager; + + /** @var ProviderService */ + private $providerService; + + public function __construct( + LocalIdService $idService, + ProviderService $providerService, + UserMapper $userMapper, + IUserManager $userManager, + IGroupManager $groupManager, + IEventDispatcher $eventDispatcher, + ILogger $logger + ) { + parent::__construct($idService, + $providerService, + $userMapper, + $userManager, + $groupManager, + $eventDispatcher, + $logger); + $this->eventDispatcher = $eventDispatcher; + $this->logger = $logger; + $this->userManager = $userManager; + $this->providerService = $providerService; + } + + protected function mapDispatchUID(int $providerid, object $payload, string $tokenUserId) { + $uidAttribute = $this->providerService->getSetting($providerid, ProviderService::SETTING_MAPPING_UID, 'sub'); + $mappedUserId = $payload->{$uidAttribute} ?? $tokenUserId; + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_UID, $payload, $mappedUserId); + $this->eventDispatcher->dispatchTyped($event); + return $event->getValue(); + } + + protected function mapDispatchDisplayname(int $providerid, object $payload) { + $displaynameAttribute = $this->providerService->getSetting($providerid, ProviderService::SETTING_MAPPING_DISPLAYNAME, 'displayname'); + $mappedDisplayName = $payload->{$displaynameAttribute} ?? null; + + if (isset($mappedDisplayName)) { + $limitedDisplayName = mb_substr($mappedDisplayName, 0, 255); + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_DISPLAYNAME, $payload, $limitedDisplayName); + } else { + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_DISPLAYNAME, $payload); + } + $this->eventDispatcher->dispatchTyped($event); + return $event->getValue(); + } + + protected function mapDispatchEmail(int $providerid, object $payload) { + $emailAttribute = $this->providerService->getSetting($providerid, ProviderService::SETTING_MAPPING_EMAIL, 'email'); + $mappedEmail = $payload->{$emailAttribute} ?? null; + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_EMAIL, $payload, $mappedEmail); + $this->eventDispatcher->dispatchTyped($event); + return $event->getValue(); + } + + protected function mapDispatchQuota(int $providerid, object $payload) { + $quotaAttribute = $this->providerService->getSetting($providerid, ProviderService::SETTING_MAPPING_QUOTA, 'quota'); + $mappedQuota = $payload->{$quotaAttribute} ?? null; + $event = new AttributeMappedEvent(ProviderService::SETTING_MAPPING_QUOTA, $payload, $mappedQuota); + $this->eventDispatcher->dispatchTyped($event); + return $event->getValue(); + } + + protected function dispatchUserAccountUpdate(string $uid, ?string $displayName, ?string $email, ?string $quota, object $payload) { + $event = new UserAccountChangeEvent($uid, $displayName, $email, $quota, $payload); + $this->eventDispatcher->dispatchTyped($event); + return $event->getResult(); + } + + /** + * Trigger a provisioning via event system. + * This allows to flexibly implement complex provisioning strategies - + * even in a separate app. + * + * On error, the provisioning logic can deliver failure reasons and + * even a redirect to a different endpoint. + * + * @param string $tokenUserId + * @param int $providerId + * @param object $idTokenPayload + * @return IUser|null + * @throws Exception + * @throws ContainerExceptionInterface + * @throws NotFoundExceptionInterface + * @throws ProvisioningDeniedException + */ + public function provisionUser(string $tokenUserId, int $providerId, object $idTokenPayload, ?IUser $existingLocalUser = null): ?IUser { + try { + // for multiple reasons, it is better to take the uid directly from a token field + //$uid = $this->mapDispatchUID($providerId, $idTokenPayload, $tokenUserId); + $uid = $tokenUserId; + $displayname = $this->mapDispatchDisplayname($providerId, $idTokenPayload); + $email = $this->mapDispatchEmail($providerId, $idTokenPayload); + $quota = $this->mapDispatchQuota($providerId, $idTokenPayload); + } catch (AttributeValueException $eAttribute) { + $this->logger->info("{$uid}: user rejected by OpenId web authorization, reason: " . $eAttribute->getMessage()); + throw new ProvisioningDeniedException($eAttribute->getMessage()); + } + + $userReaction = $this->dispatchUserAccountUpdate($uid, $displayname, $email, $quota, $idTokenPayload); + if ($userReaction->isAccessAllowed()) { + $this->logger->info("{$uid}: account accepted, reason: " . $userReaction->getReason()); + $user = $this->userManager->get($uid); + return $user; + } else { + $this->logger->info("{$uid}: account rejected, reason: " . $userReaction->getReason()); + throw new ProvisioningDeniedException($userReaction->getReason(), $userReaction->getRedirectUrl()); + } + } +} diff --git a/tests/unit/MagentaCloud/OpenidTokenTestCase.php b/tests/unit/MagentaCloud/OpenidTokenTestCase.php new file mode 100644 index 00000000..cbaa9847 --- /dev/null +++ b/tests/unit/MagentaCloud/OpenidTokenTestCase.php @@ -0,0 +1,141 @@ +realOidClaims; + } + + public function getOidClientId() { + return "USER_NC_OPENID_TEST"; + } + + public function getOidNonce() { + return "CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K"; + } + + public function getOidClientSecret() { + return \Base64Url\Base64Url::encode('JQ17C99A-DAF8-4E27-FBW4-GV23B043C993'); + } + + public function getOidServerKey() { + return \Base64Url\Base64Url::encode('JQ17DAF8-C99A-4E27-FBW4-GV23B043C993'); + } + + public function getOidPrivateServerKey() { + return [ + "p" => "9US9kD6Q8nicR1se1U_iRI9x1iK0__HF7E9yhqrza9DHldC2h7PLuR7y9bITAUtcBmVvqEQlVUXRZPMrNUpLFI9hTdZXAACRqYBYGHg7Mvyzq-2JXhEE5CFDy9wSCPunc8bRq4TsY0ocSXugXKGjx-t1uO3fkF1UgNgNMjdzSPM", + "kty" => "RSA", + "q" => "85auJF6W3c91EebGpjMX-g_U0fLBMgO2oxBsldus9x2diRd3wVvUnrTg5fQctODdr4if8dBCPDdLxBUKul4MXULC_nCkGkDjORdESb7j8amGnOvxnaVcQT6C5yHivAawa4R8NchR7n23VrQWO8fHhQBYUHTTy01G3A8D6dznCC8", + "d" => "tP-lT4FJBKrhhBUk7J1fR0638jVjn46yIfSaB5l_JlqNItmRJtbz3QWopy4oDfvrY_ccZIYG9tLvJH-3LHtuEddwxFsL-9MSUx5qxWB4sKpKA6EpxWNR5EFnFKxR_B2P2yFYiRDdbBh8h9pNaOuNjZU5iitAGvSOfW4X5hyJyu9t9zsEX9O6stEtP3yK5sx-bt7osGDMIguFBMcPVHbYw_Pl7-aNPuQ4ioxVXa3JlO6tTcDrcyMy7d3CWuGACj3juEnO-1n8E_OSR9sMp1k_L7i-qQ3OnLCOx07HeTWklCvNxz7U9qLcQXGcfpdWmhWZt6MO3SIXV4f6Md0U836v0Q", + "e" => "AQAB", + "use" => "sig", + "kid" => "0123456789", + "qi" => "T3-NLCpVoITdS6PB9XYCsXsfhQSiE_4eTQnZf_Zya5hSd0xZDrrwNiXL8Dzy3YLjsZAFC0U6wAeC2wTBJ8c-6VxdP34J0sGj2I_TNhFFArksLy9ZaRbskCxKAPLipEFi8b1H2-aaRFRLs6BQJbfesQ5mcX2kB5AItAX3R6tcc0A", + "dp" => "ExUtFor3phXiOt8JEBmuBh2PAtUidgNuncs0ouusEshkrvBVM0u23wlcZ-dZ-TDO0SSVQmdC7FaJSyxsQTItk0TwkijKDhL9Qk3dDNJV8MqehBLwLCRw1_sKllLiCFbkGWrvp0OpTLRYbRM0T-C3qHdWanP_f_DzAS9OH4kW7Cc", + "alg" => "RS256", + "dq" => "xr3XAWeHkhw0uVFgHLQtSOJn0pBM3qC2_95jqfVc7xZjtTnHhKSHGqIbqKL-VPnvBcvkK-iuUfEPyUEdyqb3UZQqAm0nByCQA8Ge_shXtJGLejbroKMNXVJCfZBhLOYMRP0IVt1FM9-wmXY_ebDrcfGxHJvlPcekG-HIYKPSgBM", + "n" => "6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ" + ]; + } + + + public function getOidPublicServerKey() { + return \OCA\UserOIDC\Vendor\Firebase\JWT\JWK::parseKeySet([ "keys" => [[ + "kty" => "RSA", + "e" => "AQAB", + "use" => "sig", + "kid" => "0123456789", + "alg" => "RS256", + "n" => "6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ" + ]]]); + } + + public function getOidTestCode() { + return "66844608"; + } + + public function getOidTestState() { + return "4VSL5T274MJEMLZI1810HUFDA07CEPXZ"; + } + + public function setUp(): void { + parent::setUp(); + + $this->app = new App(Application::APP_ID); + $this->realOidClaims = array( + "sub" => "jgyros", + "urn:custom.com:displayname" => "Jonny G", + "urn:custom.com:email" => "jonny.gyros@x.y", + "urn:custom.com:mainEmail" => "jonny.gyuris@x.y.de", + "iss" => "https:\/\/accounts.login00.custom.de", + "urn:custom.com:feat1" => "0", + "urn:custom.com:uid" => "081500000001234", + "urn:custom.com:feat2" => "1", + "urn:custom.com:ext2" => "0", + "urn:custom.com:feat3" => "1", + "acr" => "urn:custom:names:idm:THO:1.0:ac:classes:passid:00", + "urn:custom.com:feat4" => "0", + "urn:custom.com:ext4" => "0", + "auth_time" => time(), + "exp" => time() + 7200, + 'iat' => time(), + "urn:custom.com:session_token" => "ad0fff71-e013-11ec-9e17-39677d2c891c", + "nonce" => "CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K", + "aud" => array("USER_NC_OPENID_TEST") ); + } + + protected function createSignToken(array $claims) : string { + // The algorithm manager with the HS256 algorithm. + $algorithmManager = new AlgorithmManager([ + new RS256(), + ]); + + // use a different key for an invalid signature + $jwk = new JWK($this->getOidPrivateServerKey()); + $jwsBuilder = new JWSBuilder($algorithmManager); + $jws = $jwsBuilder->create() // We want to create a new JWS + ->withPayload(json_encode($claims)) // We set the payload + ->addSignature($jwk, ['alg' => 'RS256', "kid" => "0123456789"]) // We add a signature with a simple protected header + ->build(); + + $serializer = new CompactSerializer(); + return $serializer->serialize($jws, 0); + } +} diff --git a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php new file mode 100644 index 00000000..553faaab --- /dev/null +++ b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php @@ -0,0 +1,461 @@ + + * + * @license GNU AGPL version 3 or any later version + * + */ + +declare(strict_types=1); + +use OCA\UserOIDC\Controller\LoginController; +use OCA\UserOIDC\Service\DiscoveryService; +use OCA\UserOIDC\Service\ProviderService; +use OCA\UserOIDC\Service\LdapService; +use OCA\UserOIDC\Service\LocalIdService; +use OCA\UserOIDC\Service\ProvisioningEventService; +use OCA\UserOIDC\AppInfo\Application; +use OCA\UserOIDC\Db\ProviderMapper; +use OCA\UserOIDC\Db\Provider; +use OCA\UserOIDC\Db\UserMapper; +use OCA\UserOIDC\Db\SessionMapper; +use OCP\EventDispatcher\Event; +use OCP\EventDispatcher\IEventDispatcher; +use OCA\UserOIDC\Event\UserAccountChangeEvent; +use OCA\UserOIDC\Event\AttributeMappedEvent; +use OCP\Http\Client\IClientService; +use OCP\Http\Client\IClient; +use OCP\Http\Client\IResponse; +use OCP\AppFramework\Utility\ITimeFactory; +use OC\AppFramework\Bootstrap\Coordinator; +use OC\Authentication\Token\IProvider; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\TemplateResponse; +use OCP\IGroupManager; +use OCP\IConfig; +use OCP\IL10N; +use OCP\IUser; +use OCP\IDBConnection; +use OCP\ICacheFactory; +use Psr\Log\LoggerInterface; +use OCP\ILogger; // deprecated! +use OCP\IRequest; +use OCP\ISession; +use OCP\IURLGenerator; +use OCP\IUserManager; +use OCP\IUserSession; +use OCP\Security\ISecureRandom; +use OC\Security\Crypto; + +use OCP\AppFramework\App; + +use PHPUnit\Framework\MockObject\MockObject; +use OCA\UserOIDC\BaseTest\OpenidTokenTestCase; + +class ProvisioningEventServiceTest extends OpenidTokenTestCase { + /** + * Set up needed system and app configurations + */ + protected function getConfigSetup() :MockObject { + $config = $this->getMockForAbstractClass(IConfig::class); + + $config->expects($this->any()) + ->method("getSystemValue") + ->with($this->logicalOr($this->equalTo('user_oidc'), $this->equalTo('secret'))) + ->willReturn($this->returnCallback(function ($key, $default) { + if ($key == 'user_oidc') { + return [ + 'auto_provisioning' => true, + ]; + } elseif ($key == 'secret') { + return "Streng_geheim"; + } + })); + return $config; + } + + /** + * Prepare a proper session as if the handshake with an + * OpenID authenticator entity has already been done. + */ + protected function getOidSessionSetup() :MockObject { + $session = $this->getMockForAbstractClass(ISession::class); + + $session->expects($this->any()) + ->method('get') + ->willReturn($this->returnCallback(function ($key) { + $values = [ + 'oidc.state' => $this->getOidTestState(), + 'oidc.providerid' => $this->getProviderId(), + 'oidc.nonce' => $this->getOidNonce(), + 'oidc.redirect' => 'https://welcome.to.magenta' + ]; + + return $values[$key] ? $values[$key] : "some_" . $key; + })); + $this->sessionMapper = $this->getMockBuilder(SessionMapper::class) + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) + ->getMock(); + $this->sessionMapper->expects($this->any()) + ->method('createSession'); + + return $session; + } + + /** + * Prepare a proper session as if the handshake with an + * OpenID authenticator entity has already been done. + */ + protected function getProviderSetup() :MockObject { + $provider = $this->getMockBuilder(Provider::class) + ->addMethods(['getClientId', 'getClientSecret']) + ->getMock(); + $provider->expects($this->any()) + ->method('getClientId') + ->willReturn($this->getOidClientId()); + $provider->expects($this->once()) + ->method('getClientSecret') + ->willReturn($this->crypto->encrypt($this->getOidClientSecret())); + $this->providerMapper->expects($this->once()) + ->method('getProvider') + ->with($this->equalTo($this->getProviderId())) + ->willReturn($provider); + + return $provider; + } + + + /** + * Prepare a proper mapping configuration for the provider + */ + protected function getProviderServiceSetup() :MockObject { + $providerService = $this->getMockBuilder(ProviderService::class) + ->setConstructorArgs([ $this->config, $this->providerMapper]) + ->getMock(); + $providerService->expects($this->any()) + ->method('getSetting') + ->with($this->equalTo($this->getProviderId()), $this->logicalOr( + $this->equalTo(ProviderService::SETTING_MAPPING_UID), + $this->equalTo(ProviderService::SETTING_MAPPING_DISPLAYNAME), + $this->equalTo(ProviderService::SETTING_MAPPING_QUOTA), + $this->equalTo(ProviderService::SETTING_MAPPING_EMAIL), + $this->anything())) + ->will($this->returnCallback(function ($providerid, $key, $default):string { + $values = [ + ProviderService::SETTING_MAPPING_UID => 'sub', + ProviderService::SETTING_MAPPING_DISPLAYNAME => 'urn:custom.com:displayname', + ProviderService::SETTING_MAPPING_QUOTA => 'urn:custom.com:f556', + ProviderService::SETTING_MAPPING_EMAIL => 'urn:custom.com:mainEmail' + ]; + return $values[$key]; + })); + return $providerService; + } + + /** + * Prepare a proper session as if the handshake with an + * OpenID authenticator entity has already been done. + */ + protected function getUserManagerSetup() :MockObject { + $userManager = $this->getMockForAbstractClass(IUserManager::class); + $this->user = $this->getMockForAbstractClass(IUser::class); + $this->user->expects($this->any()) + ->method("canChangeAvatar") + ->willReturn(false); + + return $userManager; + } + + + /** + * This is the standard execution sequence until provisoning + * is triggered in LoginController, set up with an artificial + * yet valid OpenID token. + */ + public function setUp(): void { + parent::setUp(); + + $this->app = new App(Application::APP_ID); + $this->config = $this->getConfigSetup(); + $this->crypto = $this->getMockBuilder(Crypto::class) + ->setConstructorArgs([ $this->config ]) + ->getMock(); + + $this->request = $this->getMockForAbstractClass(IRequest::class); + $this->request->expects($this->once()) + ->method('getServerProtocol') + ->willReturn('https'); + $this->providerMapper = $this->getMockBuilder(ProviderMapper::class) + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) + ->getMock(); + $this->provider = $this->getProviderSetup(); + $this->providerService = $this->getProviderServiceSetup(); + $this->localIdService = $this->getMockBuilder(LocalIdService::class) + ->setConstructorArgs([ $this->providerService, + $this->providerMapper]) + ->getMock(); + $this->userMapper = $this->getMockBuilder(UserMapper::class) + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class), + $this->localIdService ]) + ->getMock(); + $this->discoveryService = $this->getMockBuilder(DiscoveryService::class) + ->setConstructorArgs([ $this->app->getContainer()->get(LoggerInterface::class), + $this->getMockForAbstractClass(IClientService::class), + $this->providerService, + $this->app->getContainer()->get(ICacheFactory::class) ]) + ->getMock(); + $this->discoveryService->expects($this->once()) + ->method('obtainDiscovery') + ->willReturn(array( 'token_endpoint' => 'https://whatever.to.discover/token', + 'issuer' => 'https:\/\/accounts.login00.custom.de' )); + $this->discoveryService->expects($this->once()) + ->method('obtainJWK') + ->willReturn($this->getOidPublicServerKey()); + $this->session = $this->getOidSessionSetup(); + $this->client = $this->getMockForAbstractClass(IClient::class); + $this->response = $this->getMockForAbstractClass(IResponse::class); + //$this->usersession = $this->getMockForAbstractClass(IUserSession::class); + $this->usersession = $this->getMockBuilder(IUserSession::class) + ->disableOriginalConstructor() + ->onlyMethods(['setUser', 'login', 'logout', 'getUser', 'isLoggedIn', + 'getImpersonatingUserID', 'setImpersonatingUserID']) + ->addMethods(['completeLogin', 'createSessionToken', 'createRememberMeToken']) + ->getMock(); + $this->usermanager = $this->getUserManagerSetup(); + $this->groupmanager = $this->getMockForAbstractClass(IGroupManager::class); + $this->dispatcher = $this->app->getContainer()->get(IEventDispatcher::class); + + $this->provisioningService = new ProvisioningEventService( + $this->app->getContainer()->get(LocalIdService::class), + $this->providerService, + $this->userMapper, + $this->usermanager, + $this->groupmanager, + $this->dispatcher, + $this->app->getContainer()->get(ILogger::class)); + // here is where the token magic comes in + $this->token = array( 'id_token' => + $this->createSignToken($this->getRealOidClaims(), + $this->getOidServerKey())); + $this->tokenResponse = $this->getMockForAbstractClass(IResponse::class); + $this->tokenResponse->expects($this->once()) + ->method("getBody") + ->willReturn(json_encode($this->token)); + + // mock token retrieval + $this->client = $this->getMockForAbstractClass(IClient::class); + $this->client->expects($this->once()) + ->method("post") + ->with($this->equalTo('https://whatever.to.discover/token'), $this->arrayHasKey('body')) + ->willReturn($this->tokenResponse); + $this->clientService = $this->getMockForAbstractClass(IClientService::class); + $this->clientService->expects($this->once()) + ->method("newClient") + ->willReturn($this->client); + $this->registrationContext = + $this->app->getContainer()->get(Coordinator::class)->getRegistrationContext(); + $this->loginController = new LoginController($this->request, + $this->providerMapper, + $this->providerService, + $this->discoveryService, + $this->app->getContainer()->get(LdapService::class), + $this->app->getContainer()->get(ISecureRandom::class), + $this->session, + $this->clientService, + $this->app->getContainer()->get(IUrlGenerator::class), + $this->usersession, + $this->usermanager, + $this->app->getContainer()->get(ITimeFactory::class), + $this->dispatcher, + $this->config, + $this->app->getContainer()->get(IProvider::class), + $this->sessionMapper, + $this->provisioningService, + $this->app->getContainer()->get(IL10N::class), + $this->app->getContainer()->get(ILogger::class), + $this->crypto); + + $this->attributeListener = null; + $this->accountListener = null; + } + + /** + * Seems like the event dispatcher requires explicit unregistering + */ + public function tearDown(): void { + parent::tearDown(); + if ($this->accountListener != null) { + $this->dispatcher->removeListener(UserAccountChangeEvent::class, $this->accountListener); + } + if ($this->attributeListener != null) { + $this->dispatcher->removeListener(AttributeMappedEvent::class, $this->attributeListener); + } + } + + protected function mockAssertLoginSuccess() { + $this->usermanager->expects($this->once()) + ->method('get') + ->willReturn($this->user); + $this->session->expects($this->exactly(2)) + ->method("set") + ->withConsecutive([$this->equalTo('oidc.id_token'), $this->anything()], + [$this->equalTo('last-password-confirm'), $this->anything()]); + $this->usersession->expects($this->once()) + ->method("setUser") + ->with($this->equalTo($this->user)); + $this->usersession->expects($this->once()) + ->method("completeLogin") + ->with($this->anything(), $this->anything()); + $this->usersession->expects($this->once()) + ->method("createSessionToken"); + $this->usersession->expects($this->once()) + ->method("createRememberMeToken"); + } + + protected function assertLoginRedirect($result) { + $this->assertInstanceOf(RedirectResponse::class, + $result, "LoginController->code() did not end with success redirect: Status: " . + strval($result->getStatus() . ' ' . json_encode($result->getThrottleMetadata()))); + } + + protected function assertLogin403($result) { + $this->assertInstanceOf(TemplateResponse::class, + $result, "LoginController->code() did not end with 403 Forbidden: Actual status: " . + strval($result->getStatus() . ' ' . json_encode($result->getThrottleMetadata()))); + } + + /** + * Test with the default mapping, no mapping by attribute events + * provisioning with successful result. + */ + public function testNoMap_AccessOk() { + $this->mockAssertLoginSuccess(); + $this->accountListener = function (Event $event) :void { + $this->assertInstanceOf(UserAccountChangeEvent::class, $event); + $this->assertEquals('jgyros', $event->getUid()); + $this->assertEquals('Jonny G', $event->getDisplayname()); + $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); + $this->assertNull($event->getQuota()); + $event->setResult(true, 'ok', null); + }; + + $this->dispatcher->addListener(UserAccountChangeEvent::class, $this->accountListener); + $result = $this->loginController->code($this->getOidTestState(), $this->getOidTestCode(), ''); + + $this->assertLoginRedirect($result); + $this->assertEquals('https://welcome.to.magenta', $result->getRedirectURL()); + } + + /** + * For multiple reasons, uid should com directly from a token + * field, usually sub. Thus, uid is not remapped by event, even + * if you try with a listener. + */ + public function testUidNoMapEvent_AccessOk() { + $this->mockAssertLoginSuccess(); + $this->attributeListener = function (Event $event): void { + if ($event instanceof AttributeMappedEvent && + $event->getAttribute() == ProviderService::SETTING_MAPPING_UID) { + $this->fail('UID event mapping not supported'); + } + }; + $this->accountListener = function (Event $event) :void { + $this->assertInstanceOf(UserAccountChangeEvent::class, $event); + $this->assertEquals('jgyros', $event->getUid()); + $this->assertEquals('Jonny G', $event->getDisplayname()); + $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); + $this->assertNull($event->getQuota()); + $event->setResult(true, 'ok', "https://welcome.to.darkside"); + }; + + $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); + $this->dispatcher->addListener(UserAccountChangeEvent::class, $this->accountListener); + $result = $this->loginController->code($this->getOidTestState(), $this->getOidTestCode(), ''); + + $this->assertLoginRedirect($result); + $this->assertEquals('https://welcome.to.magenta', $result->getRedirectURL()); + } + + + + /** + * Test displayname set by event scheduling and negative result + */ + public function testDisplaynameMapEvent_NOk_NoRedirect() { + $this->attributeListener = function (Event $event): void { + if ($event instanceof AttributeMappedEvent && + $event->getAttribute() == ProviderService::SETTING_MAPPING_DISPLAYNAME) { + $event->setValue("Lisa, Mona"); + } + }; + $this->accountListener = function (Event $event) :void { + $this->assertInstanceOf(UserAccountChangeEvent::class, $event); + $this->assertEquals('jgyros', $event->getUid()); + $this->assertEquals('Lisa, Mona', $event->getDisplayname()); + $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); + $this->assertNull($event->getQuota()); + $event->setResult(false, 'not an original', null); + }; + $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); + $this->dispatcher->addListener(UserAccountChangeEvent::class, $this->accountListener); + $result = $this->loginController->code($this->getOidTestState(), $this->getOidTestCode(), ''); + + $this->assertLogin403($result); + } + + public function testMainEmailMap_Nok_Redirect() { + $this->attributeListener = function (Event $event): void { + if ($event instanceof AttributeMappedEvent && + $event->getAttribute() == ProviderService::SETTING_MAPPING_EMAIL) { + //$defaultUID = $event->getValue(); + $event->setValue("mona.lisa@louvre.fr"); + } + }; + $this->accountListener = function (Event $event) :void { + $this->assertInstanceOf(UserAccountChangeEvent::class, $event); + $this->assertEquals('jgyros', $event->getUid()); + $this->assertEquals('Jonny G', $event->getDisplayname()); + $this->assertEquals('mona.lisa@louvre.fr', $event->getMainEmail()); + $this->assertNull($event->getQuota()); + $event->setResult(false, 'under restoration', 'https://welcome.to.louvre'); + }; + $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); + $this->dispatcher->addListener(UserAccountChangeEvent::class, $this->accountListener); + $result = $this->loginController->code($this->getOidTestState(), $this->getOidTestCode(), ''); + + $this->assertLoginRedirect($result); + $this->assertEquals('https://welcome.to.louvre', $result->getRedirectURL()); + } + + public function testDisplaynameUidQuotaMapped_AccessOK() { + $this->mockAssertLoginSuccess(); + $this->attributeListener = function (Event $event): void { + if ($event instanceof AttributeMappedEvent) { + if ($event->getAttribute() == ProviderService::SETTING_MAPPING_UID) { + $this->fail('UID event mapping not supported'); + } elseif ($event->getAttribute() == ProviderService::SETTING_MAPPING_DISPLAYNAME) { + $event->setValue("Lisa, Mona"); + } elseif ($event->getAttribute() == ProviderService::SETTING_MAPPING_QUOTA) { + $event->setValue("5 TB"); + } + } + }; + $this->accountListener = function (Event $event) :void { + $this->assertInstanceOf(UserAccountChangeEvent::class, $event); + $this->assertEquals('jgyros', $event->getUid()); + $this->assertEquals('Lisa, Mona', $event->getDisplayname()); + $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); + $this->assertEquals('5 TB', $event->getQuota()); + $event->setResult(true, 'ok', "https://welcome.to.louvre"); + }; + + $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); + $this->dispatcher->addListener(UserAccountChangeEvent::class, $this->accountListener); + $result = $this->loginController->code($this->getOidTestState(), $this->getOidTestCode(), ''); + + $this->assertLoginRedirect($result); + $this->assertEquals('https://welcome.to.magenta', $result->getRedirectURL()); + } +} diff --git a/tests/unit/MagentaCloud/RegistrationsTest.php b/tests/unit/MagentaCloud/RegistrationsTest.php new file mode 100644 index 00000000..5a70f556 --- /dev/null +++ b/tests/unit/MagentaCloud/RegistrationsTest.php @@ -0,0 +1,33 @@ + + * + * @license GNU AGPL version 3 or any later version + * + */ + +declare(strict_types=1); + +use OCA\UserOIDC\AppInfo\Application; +use OCA\UserOIDC\Service\ProvisioningService; +use OCA\UserOIDC\Service\ProvisioningEventService; +use OC\AppFramework\Bootstrap\Coordinator; + +use PHPUnit\Framework\TestCase; + +class RegistrationsTest extends TestCase { + public function setUp() :void { + parent::setUp(); + + $this->app = new Application(); + $coordinator = \OC::$server->get(Coordinator::class); + $this->app->register($coordinator->getRegistrationContext()->for('user_oidc')); + } + + public function testRegistration() :void { + $provisioningService = $this->app->getContainer()->get(ProvisioningService::class); + $this->assertInstanceOf(ProvisioningEventService::class, $provisioningService); + } +} From 8a22d776b1bfb51b1894e2ad72dfe3b0e9362d5a Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Thu, 14 Nov 2024 10:15:45 +0100 Subject: [PATCH 02/14] fixed faulty constructor --- lib/Service/ProvisioningEventService.php | 41 ++++++++++++++++-------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/lib/Service/ProvisioningEventService.php b/lib/Service/ProvisioningEventService.php index 465f8d24..46855eaf 100644 --- a/lib/Service/ProvisioningEventService.php +++ b/lib/Service/ProvisioningEventService.php @@ -28,14 +28,18 @@ use OCA\UserOIDC\Db\UserMapper; use OCA\UserOIDC\Event\AttributeMappedEvent; use OCA\UserOIDC\Event\UserAccountChangeEvent; +use OCP\Accounts\IAccountManager; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; +use OCP\Http\Client\IClientService; +use OCP\IAvatarManager; +use OCP\IConfig; use OCP\IGroupManager; -use OCP\ILogger; use OCP\IUser; use OCP\IUserManager; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; +use Psr\Log\LoggerInterface; // FIXME there should be an interface for both variations class ProvisioningEventService extends ProvisioningService { @@ -43,7 +47,7 @@ class ProvisioningEventService extends ProvisioningService { /** @var IEventDispatcher */ private $eventDispatcher; - /** @var ILogger */ + /** @var LoggerInterface */ private $logger; /** @var IUserManager */ @@ -53,21 +57,30 @@ class ProvisioningEventService extends ProvisioningService { private $providerService; public function __construct( - LocalIdService $idService, - ProviderService $providerService, - UserMapper $userMapper, - IUserManager $userManager, - IGroupManager $groupManager, + LocalIdService $idService, + ProviderService $providerService, + UserMapper $userMapper, + IUserManager $userManager, + IGroupManager $groupManager, IEventDispatcher $eventDispatcher, - ILogger $logger + LoggerInterface $logger, + IAccountManager $accountManager, + IClientService $clientService, + IAvatarManager $avatarManager, + IConfig $config, ) { parent::__construct($idService, - $providerService, - $userMapper, - $userManager, - $groupManager, - $eventDispatcher, - $logger); + $providerService, + $userMapper, + $userManager, + $groupManager, + $eventDispatcher, + $logger, + $accountManager, + $clientService, + $avatarManager, + $config, + ); $this->eventDispatcher = $eventDispatcher; $this->logger = $logger; $this->userManager = $userManager; From f7eb69e3c2d1950aee86968f668dd003372aad96 Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Thu, 14 Nov 2024 10:20:47 +0100 Subject: [PATCH 03/14] fixed code style --- lib/Controller/LoginController.php | 8 +- lib/Service/ProvisioningDeniedException.php | 16 +- .../unit/MagentaCloud/OpenidTokenTestCase.php | 104 +++---- .../ProvisioningEventServiceTest.php | 276 +++++++++--------- tests/unit/MagentaCloud/RegistrationsTest.php | 4 +- 5 files changed, 204 insertions(+), 204 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index dd4a2b4b..491b733e 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -469,10 +469,10 @@ public function code(string $state = '', string $code = '', string $scope = '', if ($autoProvisionAllowed) { // $softAutoProvisionAllowed = (!isset($oidcSystemConfig['soft_auto_provision']) || $oidcSystemConfig['soft_auto_provision']); // if (!$softAutoProvisionAllowed && $userFromOtherBackend !== null) { - // if soft auto-provisioning is disabled, - // we refuse login for a user that already exists in another backend - // $message = $this->l10n->t('User conflict'); - // return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); + // if soft auto-provisioning is disabled, + // we refuse login for a user that already exists in another backend + // $message = $this->l10n->t('User conflict'); + // return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); // } // TODO: (proposal) refactor all provisioning strategies into event handlers diff --git a/lib/Service/ProvisioningDeniedException.php b/lib/Service/ProvisioningDeniedException.php index 58c2fb9b..9f317170 100644 --- a/lib/Service/ProvisioningDeniedException.php +++ b/lib/Service/ProvisioningDeniedException.php @@ -35,16 +35,16 @@ class ProvisioningDeniedException extends \Exception { /** * Exception constructor including an option redirect url. * - * @param string $message The error message. It will be not revealed to the - * the user (unless the hint is empty) and thus - * should be not translated. - * @param string $hint A useful message that is presented to the end - * user. It should be translated, but must not - * contain sensitive data. - * @param int $code Set default to 403 (Forbidden) + * @param string $message The error message. It will be not revealed to the + * the user (unless the hint is empty) and thus + * should be not translated. + * @param string $hint A useful message that is presented to the end + * user. It should be translated, but must not + * contain sensitive data. + * @param int $code Set default to 403 (Forbidden) * @param \Exception|null $previous */ - public function __construct(string $message, ?string $redirectUrl = null, int $code = 403, \Exception $previous = null) { + public function __construct(string $message, ?string $redirectUrl = null, int $code = 403, ?\Exception $previous = null) { parent::__construct($message, $code, $previous); $this->redirectUrl = $redirectUrl; } diff --git a/tests/unit/MagentaCloud/OpenidTokenTestCase.php b/tests/unit/MagentaCloud/OpenidTokenTestCase.php index cbaa9847..c0955093 100644 --- a/tests/unit/MagentaCloud/OpenidTokenTestCase.php +++ b/tests/unit/MagentaCloud/OpenidTokenTestCase.php @@ -4,20 +4,20 @@ namespace OCA\UserOIDC\BaseTest; -use OCP\AppFramework\App; - -use PHPUnit\Framework\TestCase; - -use OCA\UserOIDC\AppInfo\Application; - use Jose\Component\Core\AlgorithmManager; + use Jose\Component\Core\JWK; use Jose\Component\Signature\Algorithm\HS256; + use Jose\Component\Signature\Algorithm\RS256; -use Jose\Component\Signature\JWSBuilder; use Jose\Component\Signature\JWS; + +use Jose\Component\Signature\JWSBuilder; use Jose\Component\Signature\Serializer\CompactSerializer; +use OCA\UserOIDC\AppInfo\Application; +use OCP\AppFramework\App; +use PHPUnit\Framework\TestCase; /** * This test must be run with --stderr, e.g. @@ -43,11 +43,11 @@ public function getRealOidClaims() : array { } public function getOidClientId() { - return "USER_NC_OPENID_TEST"; + return 'USER_NC_OPENID_TEST'; } public function getOidNonce() { - return "CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K"; + return 'CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K'; } public function getOidClientSecret() { @@ -60,65 +60,65 @@ public function getOidServerKey() { public function getOidPrivateServerKey() { return [ - "p" => "9US9kD6Q8nicR1se1U_iRI9x1iK0__HF7E9yhqrza9DHldC2h7PLuR7y9bITAUtcBmVvqEQlVUXRZPMrNUpLFI9hTdZXAACRqYBYGHg7Mvyzq-2JXhEE5CFDy9wSCPunc8bRq4TsY0ocSXugXKGjx-t1uO3fkF1UgNgNMjdzSPM", - "kty" => "RSA", - "q" => "85auJF6W3c91EebGpjMX-g_U0fLBMgO2oxBsldus9x2diRd3wVvUnrTg5fQctODdr4if8dBCPDdLxBUKul4MXULC_nCkGkDjORdESb7j8amGnOvxnaVcQT6C5yHivAawa4R8NchR7n23VrQWO8fHhQBYUHTTy01G3A8D6dznCC8", - "d" => "tP-lT4FJBKrhhBUk7J1fR0638jVjn46yIfSaB5l_JlqNItmRJtbz3QWopy4oDfvrY_ccZIYG9tLvJH-3LHtuEddwxFsL-9MSUx5qxWB4sKpKA6EpxWNR5EFnFKxR_B2P2yFYiRDdbBh8h9pNaOuNjZU5iitAGvSOfW4X5hyJyu9t9zsEX9O6stEtP3yK5sx-bt7osGDMIguFBMcPVHbYw_Pl7-aNPuQ4ioxVXa3JlO6tTcDrcyMy7d3CWuGACj3juEnO-1n8E_OSR9sMp1k_L7i-qQ3OnLCOx07HeTWklCvNxz7U9qLcQXGcfpdWmhWZt6MO3SIXV4f6Md0U836v0Q", - "e" => "AQAB", - "use" => "sig", - "kid" => "0123456789", - "qi" => "T3-NLCpVoITdS6PB9XYCsXsfhQSiE_4eTQnZf_Zya5hSd0xZDrrwNiXL8Dzy3YLjsZAFC0U6wAeC2wTBJ8c-6VxdP34J0sGj2I_TNhFFArksLy9ZaRbskCxKAPLipEFi8b1H2-aaRFRLs6BQJbfesQ5mcX2kB5AItAX3R6tcc0A", - "dp" => "ExUtFor3phXiOt8JEBmuBh2PAtUidgNuncs0ouusEshkrvBVM0u23wlcZ-dZ-TDO0SSVQmdC7FaJSyxsQTItk0TwkijKDhL9Qk3dDNJV8MqehBLwLCRw1_sKllLiCFbkGWrvp0OpTLRYbRM0T-C3qHdWanP_f_DzAS9OH4kW7Cc", - "alg" => "RS256", - "dq" => "xr3XAWeHkhw0uVFgHLQtSOJn0pBM3qC2_95jqfVc7xZjtTnHhKSHGqIbqKL-VPnvBcvkK-iuUfEPyUEdyqb3UZQqAm0nByCQA8Ge_shXtJGLejbroKMNXVJCfZBhLOYMRP0IVt1FM9-wmXY_ebDrcfGxHJvlPcekG-HIYKPSgBM", - "n" => "6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ" + 'p' => '9US9kD6Q8nicR1se1U_iRI9x1iK0__HF7E9yhqrza9DHldC2h7PLuR7y9bITAUtcBmVvqEQlVUXRZPMrNUpLFI9hTdZXAACRqYBYGHg7Mvyzq-2JXhEE5CFDy9wSCPunc8bRq4TsY0ocSXugXKGjx-t1uO3fkF1UgNgNMjdzSPM', + 'kty' => 'RSA', + 'q' => '85auJF6W3c91EebGpjMX-g_U0fLBMgO2oxBsldus9x2diRd3wVvUnrTg5fQctODdr4if8dBCPDdLxBUKul4MXULC_nCkGkDjORdESb7j8amGnOvxnaVcQT6C5yHivAawa4R8NchR7n23VrQWO8fHhQBYUHTTy01G3A8D6dznCC8', + 'd' => 'tP-lT4FJBKrhhBUk7J1fR0638jVjn46yIfSaB5l_JlqNItmRJtbz3QWopy4oDfvrY_ccZIYG9tLvJH-3LHtuEddwxFsL-9MSUx5qxWB4sKpKA6EpxWNR5EFnFKxR_B2P2yFYiRDdbBh8h9pNaOuNjZU5iitAGvSOfW4X5hyJyu9t9zsEX9O6stEtP3yK5sx-bt7osGDMIguFBMcPVHbYw_Pl7-aNPuQ4ioxVXa3JlO6tTcDrcyMy7d3CWuGACj3juEnO-1n8E_OSR9sMp1k_L7i-qQ3OnLCOx07HeTWklCvNxz7U9qLcQXGcfpdWmhWZt6MO3SIXV4f6Md0U836v0Q', + 'e' => 'AQAB', + 'use' => 'sig', + 'kid' => '0123456789', + 'qi' => 'T3-NLCpVoITdS6PB9XYCsXsfhQSiE_4eTQnZf_Zya5hSd0xZDrrwNiXL8Dzy3YLjsZAFC0U6wAeC2wTBJ8c-6VxdP34J0sGj2I_TNhFFArksLy9ZaRbskCxKAPLipEFi8b1H2-aaRFRLs6BQJbfesQ5mcX2kB5AItAX3R6tcc0A', + 'dp' => 'ExUtFor3phXiOt8JEBmuBh2PAtUidgNuncs0ouusEshkrvBVM0u23wlcZ-dZ-TDO0SSVQmdC7FaJSyxsQTItk0TwkijKDhL9Qk3dDNJV8MqehBLwLCRw1_sKllLiCFbkGWrvp0OpTLRYbRM0T-C3qHdWanP_f_DzAS9OH4kW7Cc', + 'alg' => 'RS256', + 'dq' => 'xr3XAWeHkhw0uVFgHLQtSOJn0pBM3qC2_95jqfVc7xZjtTnHhKSHGqIbqKL-VPnvBcvkK-iuUfEPyUEdyqb3UZQqAm0nByCQA8Ge_shXtJGLejbroKMNXVJCfZBhLOYMRP0IVt1FM9-wmXY_ebDrcfGxHJvlPcekG-HIYKPSgBM', + 'n' => '6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ' ]; } public function getOidPublicServerKey() { - return \OCA\UserOIDC\Vendor\Firebase\JWT\JWK::parseKeySet([ "keys" => [[ - "kty" => "RSA", - "e" => "AQAB", - "use" => "sig", - "kid" => "0123456789", - "alg" => "RS256", - "n" => "6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ" + return \OCA\UserOIDC\Vendor\Firebase\JWT\JWK::parseKeySet([ 'keys' => [[ + 'kty' => 'RSA', + 'e' => 'AQAB', + 'use' => 'sig', + 'kid' => '0123456789', + 'alg' => 'RS256', + 'n' => '6WCdDo8KuksEFaFlzvmsaoYhfOoMt5XgnX98dx-F1OUz53SG0lQlFt-xkwra5B4GZ-13lki0qCa2CjA1aLa9kEvDdYhz_0Uc5qOy5haDj8Jn547s6gFyaLzJ0RN5i5eKeDMHcjeEC0_NjiB2UNUFJJ61b2nXIlUvp_vBfKCv4A-8C3mLSbCKJQhX84QRDgt_Abz0MXj_ga72Ka2cwVLo4OFQAK5m57Qfu9ZvseMcgoinyhIQ18b98SkWinn3DM0W1KXLkWLk0S3XEMxLV1M7-9RLo4fgEGOpX1xmmM6KbsC5SxXvRUO7tjU-o35fcewDwXYHnRbxqhRkEFfWb7b8nQ' ]]]); } public function getOidTestCode() { - return "66844608"; + return '66844608'; } public function getOidTestState() { - return "4VSL5T274MJEMLZI1810HUFDA07CEPXZ"; + return '4VSL5T274MJEMLZI1810HUFDA07CEPXZ'; } public function setUp(): void { parent::setUp(); $this->app = new App(Application::APP_ID); - $this->realOidClaims = array( - "sub" => "jgyros", - "urn:custom.com:displayname" => "Jonny G", - "urn:custom.com:email" => "jonny.gyros@x.y", - "urn:custom.com:mainEmail" => "jonny.gyuris@x.y.de", - "iss" => "https:\/\/accounts.login00.custom.de", - "urn:custom.com:feat1" => "0", - "urn:custom.com:uid" => "081500000001234", - "urn:custom.com:feat2" => "1", - "urn:custom.com:ext2" => "0", - "urn:custom.com:feat3" => "1", - "acr" => "urn:custom:names:idm:THO:1.0:ac:classes:passid:00", - "urn:custom.com:feat4" => "0", - "urn:custom.com:ext4" => "0", - "auth_time" => time(), - "exp" => time() + 7200, + $this->realOidClaims = [ + 'sub' => 'jgyros', + 'urn:custom.com:displayname' => 'Jonny G', + 'urn:custom.com:email' => 'jonny.gyros@x.y', + 'urn:custom.com:mainEmail' => 'jonny.gyuris@x.y.de', + 'iss' => "https:\/\/accounts.login00.custom.de", + 'urn:custom.com:feat1' => '0', + 'urn:custom.com:uid' => '081500000001234', + 'urn:custom.com:feat2' => '1', + 'urn:custom.com:ext2' => '0', + 'urn:custom.com:feat3' => '1', + 'acr' => 'urn:custom:names:idm:THO:1.0:ac:classes:passid:00', + 'urn:custom.com:feat4' => '0', + 'urn:custom.com:ext4' => '0', + 'auth_time' => time(), + 'exp' => time() + 7200, 'iat' => time(), - "urn:custom.com:session_token" => "ad0fff71-e013-11ec-9e17-39677d2c891c", - "nonce" => "CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K", - "aud" => array("USER_NC_OPENID_TEST") ); + 'urn:custom.com:session_token' => 'ad0fff71-e013-11ec-9e17-39677d2c891c', + 'nonce' => 'CVMI8I3JZPALSL5DIM6I1PDP8SDSEN4K', + 'aud' => ['USER_NC_OPENID_TEST'] ]; } protected function createSignToken(array $claims) : string { @@ -131,9 +131,9 @@ protected function createSignToken(array $claims) : string { $jwk = new JWK($this->getOidPrivateServerKey()); $jwsBuilder = new JWSBuilder($algorithmManager); $jws = $jwsBuilder->create() // We want to create a new JWS - ->withPayload(json_encode($claims)) // We set the payload - ->addSignature($jwk, ['alg' => 'RS256', "kid" => "0123456789"]) // We add a signature with a simple protected header - ->build(); + ->withPayload(json_encode($claims)) // We set the payload + ->addSignature($jwk, ['alg' => 'RS256', 'kid' => '0123456789']) // We add a signature with a simple protected header + ->build(); $serializer = new CompactSerializer(); return $serializer->serialize($jws, 0); diff --git a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php index 553faaab..fa0c5f3d 100644 --- a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php +++ b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php @@ -10,49 +10,49 @@ declare(strict_types=1); +use OC\AppFramework\Bootstrap\Coordinator; +use OC\Authentication\Token\IProvider; +use OC\Security\Crypto; +use OCA\UserOIDC\AppInfo\Application; +use OCA\UserOIDC\BaseTest\OpenidTokenTestCase; use OCA\UserOIDC\Controller\LoginController; +use OCA\UserOIDC\Db\Provider; +use OCA\UserOIDC\Db\ProviderMapper; +use OCA\UserOIDC\Db\SessionMapper; +use OCA\UserOIDC\Db\UserMapper; +use OCA\UserOIDC\Event\AttributeMappedEvent; +use OCA\UserOIDC\Event\UserAccountChangeEvent; use OCA\UserOIDC\Service\DiscoveryService; -use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\Service\LdapService; use OCA\UserOIDC\Service\LocalIdService; +use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\Service\ProvisioningEventService; -use OCA\UserOIDC\AppInfo\Application; -use OCA\UserOIDC\Db\ProviderMapper; -use OCA\UserOIDC\Db\Provider; -use OCA\UserOIDC\Db\UserMapper; -use OCA\UserOIDC\Db\SessionMapper; +use OCP\AppFramework\App; +use OCP\AppFramework\Http\RedirectResponse; +use OCP\AppFramework\Http\TemplateResponse; +use OCP\AppFramework\Utility\ITimeFactory; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventDispatcher; -use OCA\UserOIDC\Event\UserAccountChangeEvent; -use OCA\UserOIDC\Event\AttributeMappedEvent; -use OCP\Http\Client\IClientService; use OCP\Http\Client\IClient; +use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; -use OCP\AppFramework\Utility\ITimeFactory; -use OC\AppFramework\Bootstrap\Coordinator; -use OC\Authentication\Token\IProvider; -use OCP\AppFramework\Http\RedirectResponse; -use OCP\AppFramework\Http\TemplateResponse; -use OCP\IGroupManager; +use OCP\ICacheFactory; use OCP\IConfig; -use OCP\IL10N; -use OCP\IUser; use OCP\IDBConnection; -use OCP\ICacheFactory; -use Psr\Log\LoggerInterface; -use OCP\ILogger; // deprecated! +use OCP\IGroupManager; +use OCP\IL10N; // deprecated! +use OCP\ILogger; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; +use OCP\IUser; use OCP\IUserManager; use OCP\IUserSession; -use OCP\Security\ISecureRandom; -use OC\Security\Crypto; -use OCP\AppFramework\App; +use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; -use OCA\UserOIDC\BaseTest\OpenidTokenTestCase; +use Psr\Log\LoggerInterface; class ProvisioningEventServiceTest extends OpenidTokenTestCase { /** @@ -62,7 +62,7 @@ protected function getConfigSetup() :MockObject { $config = $this->getMockForAbstractClass(IConfig::class); $config->expects($this->any()) - ->method("getSystemValue") + ->method('getSystemValue') ->with($this->logicalOr($this->equalTo('user_oidc'), $this->equalTo('secret'))) ->willReturn($this->returnCallback(function ($key, $default) { if ($key == 'user_oidc') { @@ -70,7 +70,7 @@ protected function getConfigSetup() :MockObject { 'auto_provisioning' => true, ]; } elseif ($key == 'secret') { - return "Streng_geheim"; + return 'Streng_geheim'; } })); return $config; @@ -93,13 +93,13 @@ protected function getOidSessionSetup() :MockObject { 'oidc.redirect' => 'https://welcome.to.magenta' ]; - return $values[$key] ? $values[$key] : "some_" . $key; + return $values[$key] ? $values[$key] : 'some_' . $key; })); $this->sessionMapper = $this->getMockBuilder(SessionMapper::class) - ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) - ->getMock(); + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) + ->getMock(); $this->sessionMapper->expects($this->any()) - ->method('createSession'); + ->method('createSession'); return $session; } @@ -113,15 +113,15 @@ protected function getProviderSetup() :MockObject { ->addMethods(['getClientId', 'getClientSecret']) ->getMock(); $provider->expects($this->any()) - ->method('getClientId') - ->willReturn($this->getOidClientId()); + ->method('getClientId') + ->willReturn($this->getOidClientId()); $provider->expects($this->once()) - ->method('getClientSecret') - ->willReturn($this->crypto->encrypt($this->getOidClientSecret())); + ->method('getClientSecret') + ->willReturn($this->crypto->encrypt($this->getOidClientSecret())); $this->providerMapper->expects($this->once()) - ->method('getProvider') - ->with($this->equalTo($this->getProviderId())) - ->willReturn($provider); + ->method('getProvider') + ->with($this->equalTo($this->getProviderId())) + ->willReturn($provider); return $provider; } @@ -132,25 +132,25 @@ protected function getProviderSetup() :MockObject { */ protected function getProviderServiceSetup() :MockObject { $providerService = $this->getMockBuilder(ProviderService::class) - ->setConstructorArgs([ $this->config, $this->providerMapper]) - ->getMock(); + ->setConstructorArgs([ $this->config, $this->providerMapper]) + ->getMock(); $providerService->expects($this->any()) - ->method('getSetting') - ->with($this->equalTo($this->getProviderId()), $this->logicalOr( - $this->equalTo(ProviderService::SETTING_MAPPING_UID), - $this->equalTo(ProviderService::SETTING_MAPPING_DISPLAYNAME), - $this->equalTo(ProviderService::SETTING_MAPPING_QUOTA), - $this->equalTo(ProviderService::SETTING_MAPPING_EMAIL), - $this->anything())) - ->will($this->returnCallback(function ($providerid, $key, $default):string { - $values = [ - ProviderService::SETTING_MAPPING_UID => 'sub', - ProviderService::SETTING_MAPPING_DISPLAYNAME => 'urn:custom.com:displayname', - ProviderService::SETTING_MAPPING_QUOTA => 'urn:custom.com:f556', - ProviderService::SETTING_MAPPING_EMAIL => 'urn:custom.com:mainEmail' - ]; - return $values[$key]; - })); + ->method('getSetting') + ->with($this->equalTo($this->getProviderId()), $this->logicalOr( + $this->equalTo(ProviderService::SETTING_MAPPING_UID), + $this->equalTo(ProviderService::SETTING_MAPPING_DISPLAYNAME), + $this->equalTo(ProviderService::SETTING_MAPPING_QUOTA), + $this->equalTo(ProviderService::SETTING_MAPPING_EMAIL), + $this->anything())) + ->will($this->returnCallback(function ($providerid, $key, $default):string { + $values = [ + ProviderService::SETTING_MAPPING_UID => 'sub', + ProviderService::SETTING_MAPPING_DISPLAYNAME => 'urn:custom.com:displayname', + ProviderService::SETTING_MAPPING_QUOTA => 'urn:custom.com:f556', + ProviderService::SETTING_MAPPING_EMAIL => 'urn:custom.com:mainEmail' + ]; + return $values[$key]; + })); return $providerService; } @@ -162,8 +162,8 @@ protected function getUserManagerSetup() :MockObject { $userManager = $this->getMockForAbstractClass(IUserManager::class); $this->user = $this->getMockForAbstractClass(IUser::class); $this->user->expects($this->any()) - ->method("canChangeAvatar") - ->willReturn(false); + ->method('canChangeAvatar') + ->willReturn(false); return $userManager; } @@ -180,102 +180,102 @@ public function setUp(): void { $this->app = new App(Application::APP_ID); $this->config = $this->getConfigSetup(); $this->crypto = $this->getMockBuilder(Crypto::class) - ->setConstructorArgs([ $this->config ]) - ->getMock(); + ->setConstructorArgs([ $this->config ]) + ->getMock(); $this->request = $this->getMockForAbstractClass(IRequest::class); $this->request->expects($this->once()) - ->method('getServerProtocol') - ->willReturn('https'); + ->method('getServerProtocol') + ->willReturn('https'); $this->providerMapper = $this->getMockBuilder(ProviderMapper::class) - ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) - ->getMock(); + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class) ]) + ->getMock(); $this->provider = $this->getProviderSetup(); $this->providerService = $this->getProviderServiceSetup(); $this->localIdService = $this->getMockBuilder(LocalIdService::class) - ->setConstructorArgs([ $this->providerService, - $this->providerMapper]) - ->getMock(); + ->setConstructorArgs([ $this->providerService, + $this->providerMapper]) + ->getMock(); $this->userMapper = $this->getMockBuilder(UserMapper::class) - ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class), - $this->localIdService ]) - ->getMock(); + ->setConstructorArgs([ $this->getMockForAbstractClass(IDBConnection::class), + $this->localIdService ]) + ->getMock(); $this->discoveryService = $this->getMockBuilder(DiscoveryService::class) - ->setConstructorArgs([ $this->app->getContainer()->get(LoggerInterface::class), - $this->getMockForAbstractClass(IClientService::class), - $this->providerService, - $this->app->getContainer()->get(ICacheFactory::class) ]) - ->getMock(); + ->setConstructorArgs([ $this->app->getContainer()->get(LoggerInterface::class), + $this->getMockForAbstractClass(IClientService::class), + $this->providerService, + $this->app->getContainer()->get(ICacheFactory::class) ]) + ->getMock(); $this->discoveryService->expects($this->once()) - ->method('obtainDiscovery') - ->willReturn(array( 'token_endpoint' => 'https://whatever.to.discover/token', - 'issuer' => 'https:\/\/accounts.login00.custom.de' )); + ->method('obtainDiscovery') + ->willReturn([ 'token_endpoint' => 'https://whatever.to.discover/token', + 'issuer' => 'https:\/\/accounts.login00.custom.de' ]); $this->discoveryService->expects($this->once()) - ->method('obtainJWK') - ->willReturn($this->getOidPublicServerKey()); + ->method('obtainJWK') + ->willReturn($this->getOidPublicServerKey()); $this->session = $this->getOidSessionSetup(); $this->client = $this->getMockForAbstractClass(IClient::class); $this->response = $this->getMockForAbstractClass(IResponse::class); //$this->usersession = $this->getMockForAbstractClass(IUserSession::class); $this->usersession = $this->getMockBuilder(IUserSession::class) - ->disableOriginalConstructor() - ->onlyMethods(['setUser', 'login', 'logout', 'getUser', 'isLoggedIn', - 'getImpersonatingUserID', 'setImpersonatingUserID']) - ->addMethods(['completeLogin', 'createSessionToken', 'createRememberMeToken']) - ->getMock(); + ->disableOriginalConstructor() + ->onlyMethods(['setUser', 'login', 'logout', 'getUser', 'isLoggedIn', + 'getImpersonatingUserID', 'setImpersonatingUserID']) + ->addMethods(['completeLogin', 'createSessionToken', 'createRememberMeToken']) + ->getMock(); $this->usermanager = $this->getUserManagerSetup(); $this->groupmanager = $this->getMockForAbstractClass(IGroupManager::class); $this->dispatcher = $this->app->getContainer()->get(IEventDispatcher::class); $this->provisioningService = new ProvisioningEventService( - $this->app->getContainer()->get(LocalIdService::class), - $this->providerService, - $this->userMapper, - $this->usermanager, - $this->groupmanager, - $this->dispatcher, - $this->app->getContainer()->get(ILogger::class)); + $this->app->getContainer()->get(LocalIdService::class), + $this->providerService, + $this->userMapper, + $this->usermanager, + $this->groupmanager, + $this->dispatcher, + $this->app->getContainer()->get(ILogger::class)); // here is where the token magic comes in - $this->token = array( 'id_token' => + $this->token = [ 'id_token' => $this->createSignToken($this->getRealOidClaims(), - $this->getOidServerKey())); + $this->getOidServerKey())]; $this->tokenResponse = $this->getMockForAbstractClass(IResponse::class); $this->tokenResponse->expects($this->once()) - ->method("getBody") - ->willReturn(json_encode($this->token)); + ->method('getBody') + ->willReturn(json_encode($this->token)); // mock token retrieval $this->client = $this->getMockForAbstractClass(IClient::class); $this->client->expects($this->once()) - ->method("post") - ->with($this->equalTo('https://whatever.to.discover/token'), $this->arrayHasKey('body')) - ->willReturn($this->tokenResponse); + ->method('post') + ->with($this->equalTo('https://whatever.to.discover/token'), $this->arrayHasKey('body')) + ->willReturn($this->tokenResponse); $this->clientService = $this->getMockForAbstractClass(IClientService::class); $this->clientService->expects($this->once()) - ->method("newClient") - ->willReturn($this->client); + ->method('newClient') + ->willReturn($this->client); $this->registrationContext = $this->app->getContainer()->get(Coordinator::class)->getRegistrationContext(); $this->loginController = new LoginController($this->request, - $this->providerMapper, - $this->providerService, - $this->discoveryService, - $this->app->getContainer()->get(LdapService::class), - $this->app->getContainer()->get(ISecureRandom::class), - $this->session, - $this->clientService, - $this->app->getContainer()->get(IUrlGenerator::class), - $this->usersession, - $this->usermanager, - $this->app->getContainer()->get(ITimeFactory::class), - $this->dispatcher, - $this->config, - $this->app->getContainer()->get(IProvider::class), - $this->sessionMapper, - $this->provisioningService, - $this->app->getContainer()->get(IL10N::class), - $this->app->getContainer()->get(ILogger::class), - $this->crypto); + $this->providerMapper, + $this->providerService, + $this->discoveryService, + $this->app->getContainer()->get(LdapService::class), + $this->app->getContainer()->get(ISecureRandom::class), + $this->session, + $this->clientService, + $this->app->getContainer()->get(IUrlGenerator::class), + $this->usersession, + $this->usermanager, + $this->app->getContainer()->get(ITimeFactory::class), + $this->dispatcher, + $this->config, + $this->app->getContainer()->get(IProvider::class), + $this->sessionMapper, + $this->provisioningService, + $this->app->getContainer()->get(IL10N::class), + $this->app->getContainer()->get(ILogger::class), + $this->crypto); $this->attributeListener = null; $this->accountListener = null; @@ -296,33 +296,33 @@ public function tearDown(): void { protected function mockAssertLoginSuccess() { $this->usermanager->expects($this->once()) - ->method('get') - ->willReturn($this->user); + ->method('get') + ->willReturn($this->user); $this->session->expects($this->exactly(2)) - ->method("set") - ->withConsecutive([$this->equalTo('oidc.id_token'), $this->anything()], - [$this->equalTo('last-password-confirm'), $this->anything()]); + ->method('set') + ->withConsecutive([$this->equalTo('oidc.id_token'), $this->anything()], + [$this->equalTo('last-password-confirm'), $this->anything()]); $this->usersession->expects($this->once()) - ->method("setUser") - ->with($this->equalTo($this->user)); + ->method('setUser') + ->with($this->equalTo($this->user)); $this->usersession->expects($this->once()) - ->method("completeLogin") - ->with($this->anything(), $this->anything()); + ->method('completeLogin') + ->with($this->anything(), $this->anything()); $this->usersession->expects($this->once()) - ->method("createSessionToken"); + ->method('createSessionToken'); $this->usersession->expects($this->once()) - ->method("createRememberMeToken"); + ->method('createRememberMeToken'); } protected function assertLoginRedirect($result) { $this->assertInstanceOf(RedirectResponse::class, - $result, "LoginController->code() did not end with success redirect: Status: " . + $result, 'LoginController->code() did not end with success redirect: Status: ' . strval($result->getStatus() . ' ' . json_encode($result->getThrottleMetadata()))); } protected function assertLogin403($result) { $this->assertInstanceOf(TemplateResponse::class, - $result, "LoginController->code() did not end with 403 Forbidden: Actual status: " . + $result, 'LoginController->code() did not end with 403 Forbidden: Actual status: ' . strval($result->getStatus() . ' ' . json_encode($result->getThrottleMetadata()))); } @@ -367,7 +367,7 @@ public function testUidNoMapEvent_AccessOk() { $this->assertEquals('Jonny G', $event->getDisplayname()); $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); $this->assertNull($event->getQuota()); - $event->setResult(true, 'ok', "https://welcome.to.darkside"); + $event->setResult(true, 'ok', 'https://welcome.to.darkside'); }; $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); @@ -387,7 +387,7 @@ public function testDisplaynameMapEvent_NOk_NoRedirect() { $this->attributeListener = function (Event $event): void { if ($event instanceof AttributeMappedEvent && $event->getAttribute() == ProviderService::SETTING_MAPPING_DISPLAYNAME) { - $event->setValue("Lisa, Mona"); + $event->setValue('Lisa, Mona'); } }; $this->accountListener = function (Event $event) :void { @@ -410,7 +410,7 @@ public function testMainEmailMap_Nok_Redirect() { if ($event instanceof AttributeMappedEvent && $event->getAttribute() == ProviderService::SETTING_MAPPING_EMAIL) { //$defaultUID = $event->getValue(); - $event->setValue("mona.lisa@louvre.fr"); + $event->setValue('mona.lisa@louvre.fr'); } }; $this->accountListener = function (Event $event) :void { @@ -436,9 +436,9 @@ public function testDisplaynameUidQuotaMapped_AccessOK() { if ($event->getAttribute() == ProviderService::SETTING_MAPPING_UID) { $this->fail('UID event mapping not supported'); } elseif ($event->getAttribute() == ProviderService::SETTING_MAPPING_DISPLAYNAME) { - $event->setValue("Lisa, Mona"); + $event->setValue('Lisa, Mona'); } elseif ($event->getAttribute() == ProviderService::SETTING_MAPPING_QUOTA) { - $event->setValue("5 TB"); + $event->setValue('5 TB'); } } }; @@ -448,7 +448,7 @@ public function testDisplaynameUidQuotaMapped_AccessOK() { $this->assertEquals('Lisa, Mona', $event->getDisplayname()); $this->assertEquals('jonny.gyuris@x.y.de', $event->getMainEmail()); $this->assertEquals('5 TB', $event->getQuota()); - $event->setResult(true, 'ok', "https://welcome.to.louvre"); + $event->setResult(true, 'ok', 'https://welcome.to.louvre'); }; $this->dispatcher->addListener(AttributeMappedEvent::class, $this->attributeListener); diff --git a/tests/unit/MagentaCloud/RegistrationsTest.php b/tests/unit/MagentaCloud/RegistrationsTest.php index 5a70f556..eb714da9 100644 --- a/tests/unit/MagentaCloud/RegistrationsTest.php +++ b/tests/unit/MagentaCloud/RegistrationsTest.php @@ -10,10 +10,10 @@ declare(strict_types=1); +use OC\AppFramework\Bootstrap\Coordinator; use OCA\UserOIDC\AppInfo\Application; -use OCA\UserOIDC\Service\ProvisioningService; use OCA\UserOIDC\Service\ProvisioningEventService; -use OC\AppFramework\Bootstrap\Coordinator; +use OCA\UserOIDC\Service\ProvisioningService; use PHPUnit\Framework\TestCase; From b9d35e296c561e67f368d4f3b902f2f54da998bb Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 14 Jan 2025 11:35:08 +0100 Subject: [PATCH 04/14] Fix slow queries in scenarios where we do not need a search --- lib/Controller/LoginController.php | 13 +++++++++---- lib/Service/LdapService.php | 10 ++++++++++ lib/Service/ProvisioningService.php | 11 +++++++++++ 3 files changed, 30 insertions(+), 4 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 491b733e..6ac6ff17 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -456,11 +456,16 @@ public function code(string $state = '', string $code = '', string $scope = '', } $autoProvisionAllowed = (!isset($oidcSystemConfig['auto_provision']) || $oidcSystemConfig['auto_provision']); + $softAutoProvisionAllowed = (!isset($oidcSystemConfig['soft_auto_provision']) || $oidcSystemConfig['soft_auto_provision']); + + $shouldDoUserLookup = !$autoProvisionAllowed || ($softAutoProvisionAllowed && !$this->provisioningService->hasOidcUserProvisitioned($userId)); + if ($shouldDoUserLookup && $this->ldapService->isLDAPEnabled()) { + // in case user is provisioned by user_ldap, userManager->search() triggers an ldap search which syncs the results + // so new users will be directly available even if they were not synced before this login attempt + $this->userManager->search($userId, 1, 0); + $this->ldapService->syncUser($userId); + } - // in case user is provisioned by user_ldap, userManager->search() triggers an ldap search which syncs the results - // so new users will be directly available even if they were not synced before this login attempt - $this->userManager->search($userId); - $this->ldapService->syncUser($userId); $userFromOtherBackend = $this->userManager->get($userId); if ($userFromOtherBackend !== null && $this->ldapService->isLdapDeletedUser($userFromOtherBackend)) { $userFromOtherBackend = null; diff --git a/lib/Service/LdapService.php b/lib/Service/LdapService.php index 53cf80fa..f56fa12f 100644 --- a/lib/Service/LdapService.php +++ b/lib/Service/LdapService.php @@ -8,6 +8,7 @@ namespace OCA\UserOIDC\Service; +use OCP\App\IAppManager; use OCP\AppFramework\QueryException; use OCP\IUser; use Psr\Log\LoggerInterface; @@ -16,9 +17,14 @@ class LdapService { public function __construct( private LoggerInterface $logger, + private IAppManager $appManager, ) { } + public function isLDAPEnabled(): bool { + return $this->appManager->isEnabledForUser('user_ldap'); + } + /** * @param IUser $user * @return bool @@ -26,6 +32,10 @@ public function __construct( * @throws \Psr\Container\NotFoundExceptionInterface */ public function isLdapDeletedUser(IUser $user): bool { + if ($this->isLDAPEnabled()) { + return false; + } + $className = $user->getBackendClassName(); if ($className !== 'LDAP') { return false; diff --git a/lib/Service/ProvisioningService.php b/lib/Service/ProvisioningService.php index 02122a1d..e06d07bd 100644 --- a/lib/Service/ProvisioningService.php +++ b/lib/Service/ProvisioningService.php @@ -10,6 +10,8 @@ use OCA\UserOIDC\Db\UserMapper; use OCA\UserOIDC\Event\AttributeMappedEvent; use OCP\Accounts\IAccountManager; +use OCP\AppFramework\Db\DoesNotExistException; +use OCP\AppFramework\Db\MultipleObjectsReturnedException; use OCP\DB\Exception; use OCP\EventDispatcher\IEventDispatcher; use OCP\Http\Client\IClientService; @@ -40,6 +42,15 @@ public function __construct( ) { } + public function hasOidcUserProvisitioned(string $userId): bool { + try { + $this->userMapper->getUser($userId); + return true; + } catch (DoesNotExistException|MultipleObjectsReturnedException) { + } + return false; + } + /** * @param string $tokenUserId * @param int $providerId From 6e40d0ee1d0d3e7d30b9c4a96e0b8e409de1fe23 Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 14 Jan 2025 11:59:11 +0100 Subject: [PATCH 05/14] fix provisioning test --- lib/Db/UserMapper.php | 50 +++++++++++++++++++ .../ProvisioningEventServiceTest.php | 35 ++++++++++--- 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index fe209967..9bc257cf 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -13,6 +13,7 @@ use OCP\AppFramework\Db\QBMapper; use OCP\Cache\CappedMemoryCache; use OCP\IDBConnection; +use Psr\Log\LoggerInterface; /** * @extends QBMapper @@ -20,13 +21,16 @@ class UserMapper extends QBMapper { private CappedMemoryCache $userCache; + private LoggerInterface $logger; public function __construct( IDBConnection $db, + LoggerInterface $logger, private LocalIdService $idService, ) { parent::__construct($db, 'user_oidc', User::class); $this->userCache = new CappedMemoryCache(); + $this->logger = $logger; } /** @@ -57,6 +61,29 @@ public function getUser(string $uid): User { public function find(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); + $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); + $stack = []; + + foreach ($backtrace as $index => $trace) { + $class = $trace['class'] ?? ''; + $type = $trace['type'] ?? ''; + $function = $trace['function'] ?? ''; + $file = $trace['file'] ?? 'unknown file'; + $line = $trace['line'] ?? 'unknown line'; + + $stack[] = sprintf( + "#%d %s%s%s() called at [%s:%s]", + $index, + $class, + $type, + $function, + $file, + $line + ); + } + + $this->logger->debug("Find user by string: " . $search . " -- Call Stack:\n" . implode("\n", $stack)); + $qb->select('user_id', 'display_name') ->from($this->getTableName(), 'u') ->leftJoin('u', 'preferences', 'p', $qb->expr()->andX( @@ -77,6 +104,29 @@ public function find(string $search, $limit = null, $offset = null): array { public function findDisplayNames(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); + $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); + $stack = []; + + foreach ($backtrace as $index => $trace) { + $class = $trace['class'] ?? ''; + $type = $trace['type'] ?? ''; + $function = $trace['function'] ?? ''; + $file = $trace['file'] ?? 'unknown file'; + $line = $trace['line'] ?? 'unknown line'; + + $stack[] = sprintf( + "#%d %s%s%s() called at [%s:%s]", + $index, + $class, + $type, + $function, + $file, + $line + ); + } + + $this->logger->debug("Find user display names by string: " . $search . " -- Call Stack:\n" . implode("\n", $stack)); + $qb->select('user_id', 'display_name') ->from($this->getTableName(), 'u') ->leftJoin('u', 'preferences', 'p', $qb->expr()->andX( diff --git a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php index fa0c5f3d..827fbced 100644 --- a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php +++ b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php @@ -27,6 +27,7 @@ use OCA\UserOIDC\Service\LocalIdService; use OCA\UserOIDC\Service\ProviderService; use OCA\UserOIDC\Service\ProvisioningEventService; +use OCP\Accounts\IAccountManager; use OCP\AppFramework\App; use OCP\AppFramework\Http\RedirectResponse; use OCP\AppFramework\Http\TemplateResponse; @@ -36,6 +37,7 @@ use OCP\Http\Client\IClient; use OCP\Http\Client\IClientService; use OCP\Http\Client\IResponse; +use OCP\IAvatarManager; use OCP\ICacheFactory; use OCP\IConfig; use OCP\IDBConnection; @@ -49,6 +51,7 @@ use OCP\IUserManager; use OCP\IUserSession; + use OCP\Security\ISecureRandom; use PHPUnit\Framework\MockObject\MockObject; @@ -218,11 +221,24 @@ public function setUp(): void { $this->response = $this->getMockForAbstractClass(IResponse::class); //$this->usersession = $this->getMockForAbstractClass(IUserSession::class); $this->usersession = $this->getMockBuilder(IUserSession::class) - ->disableOriginalConstructor() - ->onlyMethods(['setUser', 'login', 'logout', 'getUser', 'isLoggedIn', - 'getImpersonatingUserID', 'setImpersonatingUserID']) - ->addMethods(['completeLogin', 'createSessionToken', 'createRememberMeToken']) - ->getMock(); + ->disableOriginalConstructor() + ->onlyMethods([ + 'setUser', + 'login', + 'logout', + 'getUser', + 'isLoggedIn', + 'getImpersonatingUserID', + 'setImpersonatingUserID', + 'setVolatileActiveUser' // Diese Methode hinzufügen, falls sie gebraucht wird. + ]) + ->addMethods([ + 'completeLogin', + 'createSessionToken', + 'createRememberMeToken' + ]) + ->getMock(); + $this->usermanager = $this->getUserManagerSetup(); $this->groupmanager = $this->getMockForAbstractClass(IGroupManager::class); $this->dispatcher = $this->app->getContainer()->get(IEventDispatcher::class); @@ -234,7 +250,11 @@ public function setUp(): void { $this->usermanager, $this->groupmanager, $this->dispatcher, - $this->app->getContainer()->get(ILogger::class)); + $this->app->getContainer()->get(LoggerInterface::class), + $this->app->getContainer()->get(IAccountManager::class), + $this->app->getContainer()->get(IClientService::class), + $this->app->getContainer()->get(IAvatarManager::class), + $this->app->getContainer()->get(IConfig::class)); // here is where the token magic comes in $this->token = [ 'id_token' => $this->createSignToken($this->getRealOidClaims(), @@ -274,7 +294,7 @@ public function setUp(): void { $this->sessionMapper, $this->provisioningService, $this->app->getContainer()->get(IL10N::class), - $this->app->getContainer()->get(ILogger::class), + $this->app->getContainer()->get(LoggerInterface::class), $this->crypto); $this->attributeListener = null; @@ -413,6 +433,7 @@ public function testMainEmailMap_Nok_Redirect() { $event->setValue('mona.lisa@louvre.fr'); } }; + $this->accountListener = function (Event $event) :void { $this->assertInstanceOf(UserAccountChangeEvent::class, $event); $this->assertEquals('jgyros', $event->getUid()); From 75da15864882acc67b559c81ef7ea7916b41858c Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 14 Jan 2025 17:14:20 +0100 Subject: [PATCH 06/14] remove token store and check --- lib/AppInfo/Application.php | 1 - lib/Controller/LoginController.php | 12 ++++++------ 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/AppInfo/Application.php b/lib/AppInfo/Application.php index 736b605e..454eaeb2 100644 --- a/lib/AppInfo/Application.php +++ b/lib/AppInfo/Application.php @@ -60,7 +60,6 @@ public function register(IRegistrationContext $context): void { public function boot(IBootContext $context): void { $context->injectFn(\Closure::fromCallable([$this->backend, 'injectSession'])); - $context->injectFn(\Closure::fromCallable([$this, 'checkLoginToken'])); /** @var IUserSession $userSession */ $userSession = $this->getContainer()->get(IUserSession::class); if ($userSession->isLoggedIn()) { diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 91e3dd3d..e0dd0fa3 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -534,12 +534,12 @@ public function code(string $state = '', string $code = '', string $scope = '', } // store all token information for potential token exchange requests - $tokenData = array_merge( - $data, - ['provider_id' => $providerId], - ); - $this->tokenService->storeToken($tokenData); - $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); + // $tokenData = array_merge( + // $data, + // ['provider_id' => $providerId], + // ); + // $this->tokenService->storeToken($tokenData); + // $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); // Set last password confirm to the future as we don't have passwords to confirm against with SSO $this->session->set('last-password-confirm', strtotime('+4 year', time())); From d947b0ddd6069769ba6c6aa4d2b10ae6bf853493 Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 15 Jan 2025 10:57:22 +0100 Subject: [PATCH 07/14] remove logging --- lib/Db/UserMapper.php | 46 ------------------------------------------- 1 file changed, 46 deletions(-) diff --git a/lib/Db/UserMapper.php b/lib/Db/UserMapper.php index c4a8cd21..3377694b 100644 --- a/lib/Db/UserMapper.php +++ b/lib/Db/UserMapper.php @@ -63,29 +63,6 @@ public function getUser(string $uid): User { public function find(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); - $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); - $stack = []; - - foreach ($backtrace as $index => $trace) { - $class = $trace['class'] ?? ''; - $type = $trace['type'] ?? ''; - $function = $trace['function'] ?? ''; - $file = $trace['file'] ?? 'unknown file'; - $line = $trace['line'] ?? 'unknown line'; - - $stack[] = sprintf( - "#%d %s%s%s() called at [%s:%s]", - $index, - $class, - $type, - $function, - $file, - $line - ); - } - - $this->logger->debug("Find user by string: " . $search . " -- Call Stack:\n" . implode("\n", $stack)); - $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); $matchEmails = !isset($oidcSystemConfig['user_search_match_emails']) || $oidcSystemConfig['user_search_match_emails'] === true; if ($matchEmails) { @@ -118,29 +95,6 @@ public function find(string $search, $limit = null, $offset = null): array { public function findDisplayNames(string $search, $limit = null, $offset = null): array { $qb = $this->db->getQueryBuilder(); - $backtrace = debug_backtrace(DEBUG_BACKTRACE_IGNORE_ARGS); - $stack = []; - - foreach ($backtrace as $index => $trace) { - $class = $trace['class'] ?? ''; - $type = $trace['type'] ?? ''; - $function = $trace['function'] ?? ''; - $file = $trace['file'] ?? 'unknown file'; - $line = $trace['line'] ?? 'unknown line'; - - $stack[] = sprintf( - "#%d %s%s%s() called at [%s:%s]", - $index, - $class, - $type, - $function, - $file, - $line - ); - } - - $this->logger->debug("Find user display names by string: " . $search . " -- Call Stack:\n" . implode("\n", $stack)); - $oidcSystemConfig = $this->config->getSystemValue('user_oidc', []); $matchEmails = !isset($oidcSystemConfig['user_search_match_emails']) || $oidcSystemConfig['user_search_match_emails'] === true; if ($matchEmails) { From adedcc5bfb341e9d178ad0e0604300935c51f9a7 Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 4 Feb 2025 08:38:12 +0100 Subject: [PATCH 08/14] fix errors --- lib/Controller/LoginController.php | 12 +++---- .../ProvisioningEventServiceTest.php | 35 +++++++++---------- 2 files changed, 23 insertions(+), 24 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index c49840c8..3c3d2637 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -542,12 +542,12 @@ public function code(string $state = '', string $code = '', string $scope = '', // $tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true); // if ($tokenExchangeEnabled) { - // store all token information for potential token exchange requests - // $tokenData = array_merge( - // $data, - // ['provider_id' => $providerId], - // ); - // $this->tokenService->storeToken($tokenData); + // store all token information for potential token exchange requests + // $tokenData = array_merge( + // $data, + // ['provider_id' => $providerId], + // ); + // $this->tokenService->storeToken($tokenData); // } // $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); diff --git a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php index 827fbced..85ac4d2a 100644 --- a/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php +++ b/tests/unit/MagentaCloud/ProvisioningEventServiceTest.php @@ -43,7 +43,6 @@ use OCP\IDBConnection; use OCP\IGroupManager; use OCP\IL10N; // deprecated! -use OCP\ILogger; use OCP\IRequest; use OCP\ISession; use OCP\IURLGenerator; @@ -221,23 +220,23 @@ public function setUp(): void { $this->response = $this->getMockForAbstractClass(IResponse::class); //$this->usersession = $this->getMockForAbstractClass(IUserSession::class); $this->usersession = $this->getMockBuilder(IUserSession::class) - ->disableOriginalConstructor() - ->onlyMethods([ - 'setUser', - 'login', - 'logout', - 'getUser', - 'isLoggedIn', - 'getImpersonatingUserID', - 'setImpersonatingUserID', - 'setVolatileActiveUser' // Diese Methode hinzufügen, falls sie gebraucht wird. - ]) - ->addMethods([ - 'completeLogin', - 'createSessionToken', - 'createRememberMeToken' - ]) - ->getMock(); + ->disableOriginalConstructor() + ->onlyMethods([ + 'setUser', + 'login', + 'logout', + 'getUser', + 'isLoggedIn', + 'getImpersonatingUserID', + 'setImpersonatingUserID', + 'setVolatileActiveUser' // Diese Methode hinzufügen, falls sie gebraucht wird. + ]) + ->addMethods([ + 'completeLogin', + 'createSessionToken', + 'createRememberMeToken' + ]) + ->getMock(); $this->usermanager = $this->getUserManagerSetup(); $this->groupmanager = $this->getMockForAbstractClass(IGroupManager::class); From 217b6474406f48d16851df3cd5f5b5e482989b50 Mon Sep 17 00:00:00 2001 From: memurats Date: Tue, 4 Feb 2025 10:51:04 +0100 Subject: [PATCH 09/14] fix error --- lib/Controller/LoginController.php | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 3c3d2637..f2cf6ad1 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -540,17 +540,6 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->eventDispatcher->dispatchTyped(new UserLoggedInEvent($user, $user->getUID(), null, false)); } - // $tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true); - // if ($tokenExchangeEnabled) { - // store all token information for potential token exchange requests - // $tokenData = array_merge( - // $data, - // ['provider_id' => $providerId], - // ); - // $this->tokenService->storeToken($tokenData); - // } - // $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); - // Set last password confirm to the future as we don't have passwords to confirm against with SSO $this->session->set('last-password-confirm', strtotime('+4 year', time())); From 3ddcfa9fb8eaae115e057939b52d2cbb5d032496 Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 5 Feb 2025 11:36:59 +0100 Subject: [PATCH 10/14] fix error --- lib/Controller/LoginController.php | 33 ++++++++++++++++++------------ 1 file changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index f2cf6ad1..1ac75c16 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -487,21 +487,14 @@ public function code(string $state = '', string $code = '', string $scope = '', } if ($autoProvisionAllowed) { - // $softAutoProvisionAllowed = (!isset($oidcSystemConfig['soft_auto_provision']) || $oidcSystemConfig['soft_auto_provision']); - // if (!$softAutoProvisionAllowed && $userFromOtherBackend !== null) { - // if soft auto-provisioning is disabled, - // we refuse login for a user that already exists in another backend - // $message = $this->l10n->t('User conflict'); - // return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); - // } - // TODO: (proposal) refactor all provisioning strategies into event handlers $user = null; try { + // use potential user from other backend, create it in our backend if it does not exist $user = $this->provisioningService->provisionUser($userId, $providerId, $idTokenPayload, $userFromOtherBackend); } catch (ProvisioningDeniedException $denied) { - // TODO MagentaCLOUD should upstream the exception handling + // TODO: MagentaCLOUD should upstream the exception handling $redirectUrl = $denied->getRedirectUrl(); if ($redirectUrl === null) { $message = $this->l10n->t('Failed to provision user'); @@ -513,9 +506,12 @@ public function code(string $state = '', string $code = '', string $scope = '', } } - // use potential user from other backend, create it in our backend if it does not exist - // $user = $this->provisioningService->provisionUser($userId, $providerId, $idTokenPayload, $userFromOtherBackend); - // no default exception handling to pass on unittest assertion failures + if (!$softAutoProvisionAllowed && $userFromOtherBackend !== null && $user === null) { + // if soft auto-provisioning is disabled, + // we refuse login for a user that already exists in another backend + $message = $this->l10n->t('User conflict'); + return $this->build403TemplateResponse($message, Http::STATUS_BAD_REQUEST, ['reason' => 'non-soft auto provision, user conflict'], false); + } } else { // when auto provision is disabled, we assume the user has been created by another user backend (or manually) $user = $userFromOtherBackend; @@ -540,6 +536,17 @@ public function code(string $state = '', string $code = '', string $scope = '', $this->eventDispatcher->dispatchTyped(new UserLoggedInEvent($user, $user->getUID(), null, false)); } + $tokenExchangeEnabled = (isset($oidcSystemConfig['token_exchange']) && $oidcSystemConfig['token_exchange'] === true); + if ($tokenExchangeEnabled) { + // store all token information for potential token exchange requests + $tokenData = array_merge( + $data, + ['provider_id' => $providerId], + ); + $this->tokenService->storeToken($tokenData); + } + $this->config->setUserValue($user->getUID(), Application::APP_ID, 'had_token_once', '1'); + // Set last password confirm to the future as we don't have passwords to confirm against with SSO $this->session->set('last-password-confirm', strtotime('+4 year', time())); @@ -796,4 +803,4 @@ private function toCodeChallenge(string $data): string { $s = str_replace('/', '_', $s); // 63rd char of encoding return $s; } -} +} \ No newline at end of file From 869d9b699f81f74cf558791216fb9f203c15d6ba Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 12 Feb 2025 10:00:23 +0100 Subject: [PATCH 11/14] fixed error --- lib/Service/ProvisioningEventService.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/Service/ProvisioningEventService.php b/lib/Service/ProvisioningEventService.php index 46855eaf..a4a9d857 100644 --- a/lib/Service/ProvisioningEventService.php +++ b/lib/Service/ProvisioningEventService.php @@ -35,6 +35,7 @@ use OCP\IAvatarManager; use OCP\IConfig; use OCP\IGroupManager; +use OCP\ISession; use OCP\IUser; use OCP\IUserManager; use Psr\Container\ContainerExceptionInterface; @@ -68,6 +69,7 @@ public function __construct( IClientService $clientService, IAvatarManager $avatarManager, IConfig $config, + ISession $session, ) { parent::__construct($idService, $providerService, @@ -80,6 +82,7 @@ public function __construct( $clientService, $avatarManager, $config, + $session, ); $this->eventDispatcher = $eventDispatcher; $this->logger = $logger; From 6bcb17c7df046bd79eb8f17583849e3a9d2c5b90 Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 12 Feb 2025 10:04:42 +0100 Subject: [PATCH 12/14] fixed coding style --- lib/Controller/LoginController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index 1ac75c16..ce28e161 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -803,4 +803,4 @@ private function toCodeChallenge(string $data): string { $s = str_replace('/', '_', $s); // 63rd char of encoding return $s; } -} \ No newline at end of file +} From c57ff656821d67f0d077413109f2f7419799eedd Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 29 Oct 2025 12:03:13 +0100 Subject: [PATCH 13/14] fixed event based user provisioning --- lib/Service/ProvisioningEventService.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/Service/ProvisioningEventService.php b/lib/Service/ProvisioningEventService.php index a4a9d857..d544bd60 100644 --- a/lib/Service/ProvisioningEventService.php +++ b/lib/Service/ProvisioningEventService.php @@ -145,30 +145,33 @@ protected function dispatchUserAccountUpdate(string $uid, ?string $displayName, * @param string $tokenUserId * @param int $providerId * @param object $idTokenPayload - * @return IUser|null + * @param IUser|null $existingLocalUser + * @return array{user: ?IUser, userData: array} * @throws Exception * @throws ContainerExceptionInterface * @throws NotFoundExceptionInterface * @throws ProvisioningDeniedException */ - public function provisionUser(string $tokenUserId, int $providerId, object $idTokenPayload, ?IUser $existingLocalUser = null): ?IUser { + public function provisionUser(string $tokenUserId, int $providerId, object $idTokenPayload, ?IUser $existingLocalUser = null): array { try { - // for multiple reasons, it is better to take the uid directly from a token field - //$uid = $this->mapDispatchUID($providerId, $idTokenPayload, $tokenUserId); $uid = $tokenUserId; $displayname = $this->mapDispatchDisplayname($providerId, $idTokenPayload); $email = $this->mapDispatchEmail($providerId, $idTokenPayload); $quota = $this->mapDispatchQuota($providerId, $idTokenPayload); } catch (AttributeValueException $eAttribute) { - $this->logger->info("{$uid}: user rejected by OpenId web authorization, reason: " . $eAttribute->getMessage()); + $this->logger->info("{$tokenUserId}: user rejected by OpenId web authorization, reason: " . $eAttribute->getMessage()); throw new ProvisioningDeniedException($eAttribute->getMessage()); } $userReaction = $this->dispatchUserAccountUpdate($uid, $displayname, $email, $quota, $idTokenPayload); + if ($userReaction->isAccessAllowed()) { $this->logger->info("{$uid}: account accepted, reason: " . $userReaction->getReason()); $user = $this->userManager->get($uid); - return $user; + return [ + 'user' => $user, + 'userData' => get_object_vars($idTokenPayload), // optional, analog zu ProvisioningService + ]; } else { $this->logger->info("{$uid}: account rejected, reason: " . $userReaction->getReason()); throw new ProvisioningDeniedException($userReaction->getReason(), $userReaction->getRedirectUrl()); From 181666d709c6f18a33eceff3324b4767bcd376f0 Mon Sep 17 00:00:00 2001 From: memurats Date: Wed, 29 Oct 2025 12:16:23 +0100 Subject: [PATCH 14/14] fixed codestyle --- lib/Controller/LoginController.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Controller/LoginController.php b/lib/Controller/LoginController.php index e6aac039..409b22f5 100644 --- a/lib/Controller/LoginController.php +++ b/lib/Controller/LoginController.php @@ -910,7 +910,7 @@ private function getBackchannelLogoutErrorResponse( string $error, string $description, array $throttleMetadata = [], - ?bool $throttle = null, + ?bool $throttle = null, ): JSONResponse { $this->logger->debug('Backchannel logout error. ' . $error . ' ; ' . $description); return new JSONResponse(