From 4f33a7454ab1be6594a693a0f983c3725624bdc8 Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Tue, 9 Jul 2024 16:30:40 +0200 Subject: [PATCH 1/2] added logging to delete loop --- lib/User/UserAccountDeletionJob.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/User/UserAccountDeletionJob.php b/lib/User/UserAccountDeletionJob.php index 29311d1..e7af604 100644 --- a/lib/User/UserAccountDeletionJob.php +++ b/lib/User/UserAccountDeletionJob.php @@ -65,7 +65,9 @@ public function run($arguments) { foreach ($expiredUids as $uid) { try { $user = $this->userManager->get($uid); + $this->logger->info("Deleting " . $uid); $user->delete(); + $this->logger->info(\count($uid) . " deleted"); } catch (\Throwable $e) { $this->logger->logException($e, [ 'message' => $uid . ': Deletion failed with ' . $e->getMessage(), From b227fc3d958ed783ea0bb80909094b06eeb11ea1 Mon Sep 17 00:00:00 2001 From: Mauro Mura Date: Tue, 9 Jul 2024 16:43:33 +0200 Subject: [PATCH 2/2] fixed php cs --- lib/Rules/UserAccountRules.php | 91 ++++++++-------- lib/User/NmcUserService.php | 191 ++++++++++++++++----------------- 2 files changed, 138 insertions(+), 144 deletions(-) diff --git a/lib/Rules/UserAccountRules.php b/lib/Rules/UserAccountRules.php index 1998ebf..f1e9d18 100644 --- a/lib/Rules/UserAccountRules.php +++ b/lib/Rules/UserAccountRules.php @@ -21,24 +21,23 @@ class UserAccountRules { /** @var NmcUserService */ private $nmcUserService; - /** @var IUserManager */ - private $userManager; + /** @var IUserManager */ + private $userManager; - /** @var ProviderMapper */ - private $oidcProviderMapper; + /** @var ProviderMapper */ + private $oidcProviderMapper; public function __construct(IConfig $config, - ILogger $logger, - NmcUserService $nmcUserService, - IUserManager $userManager, - ProviderMapper $oidcProviderMapper) - { + ILogger $logger, + NmcUserService $nmcUserService, + IUserManager $userManager, + ProviderMapper $oidcProviderMapper) { $this->config = $config; $this->logger = $logger; $this->nmcUserService = $nmcUserService; - $this->userManager = $userManager; - $this->oidcProviderMapper = $oidcProviderMapper; - } + $this->userManager = $userManager; + $this->oidcProviderMapper = $oidcProviderMapper; + } /** * Check whether NextMagentaCloud product is booked on customer. @@ -108,20 +107,19 @@ public function withdrawDate($claims) : \DateTime { } } - /** - * @param string $displayname - * @param string $testParam - * @return bool - */ - public function isTestAccount(string $displayname, string $testParam = "-test", string $explodeParam = "@"): bool - { - $explode = (str_contains($explodeParam, $displayname)) ? explode($explodeParam, $displayname) : $displayname; - if (is_array($explode)) { - return str_contains($explode[0], $testParam); - } else { - return str_contains($explode, $testParam); - } - } + /** + * @param string $displayname + * @param string $testParam + * @return bool + */ + public function isTestAccount(string $displayname, string $testParam = "-test", string $explodeParam = "@"): bool { + $explode = (str_contains($explodeParam, $displayname)) ? explode($explodeParam, $displayname) : $displayname; + if (is_array($explode)) { + return str_contains($explode[0], $testParam); + } else { + return str_contains($explode, $testParam); + } + } /** * @@ -132,27 +130,26 @@ public function isTestAccount(string $displayname, string $testParam = "-test", * `sudo -u www-data php /var/www/nextcloud/occ config:app:set nmcprovisioning userratesurl --value "https://cloud.telekom-dienste.de/tarife"` */ public function deriveAccountState(string $uid, ?string $displayname, ?string $mainEmail, - string $quota, object $claims, bool $create = true, $providerName = 'Telekom'): array - { + string $quota, object $claims, bool $create = true, $providerName = 'Telekom'): array { $this->logger->info("PROV {$uid}: Check user existence"); - $this->logger->debug("Account change event: " . json_encode(get_object_vars($claims))); - $this->logger->debug("Provider {$uid}: " . $providerName); - $config = $this->config->getSystemValue('nmc_provisioning', [ - 'slup_test_account_check' => true, - 'slup_test_account_name' => '-test', - 'slup_test_account_explode' => '@' - ]); - if ($user = $this->nmcUserService->userExists($providerName, $uid, true)) { + $this->logger->debug("Account change event: " . json_encode(get_object_vars($claims))); + $this->logger->debug("Provider {$uid}: " . $providerName); + $config = $this->config->getSystemValue('nmc_provisioning', [ + 'slup_test_account_check' => true, + 'slup_test_account_name' => '-test', + 'slup_test_account_explode' => '@' + ]); + if ($user = $this->nmcUserService->userExists($providerName, $uid, true)) { $this->logger->info("PROV {$uid}: Modify existing"); - return $this->deriveExistingAccountState($user, $displayname, $mainEmail, $quota, $claims, $providerName); - } elseif ($create || $config['slup_test_account_check'] && - $this->isTestAccount($displayname,$config['slup_test_account_name'],$config['slup_test_account_explode'])) { + return $this->deriveExistingAccountState($user, $displayname, $mainEmail, $quota, $claims, $providerName); + } elseif ($create || $config['slup_test_account_check'] && + $this->isTestAccount($displayname, $config['slup_test_account_name'], $config['slup_test_account_explode'])) { $this->logger->info("PROV {$uid}: Create"); - return $this->deriveNewAccountState($uid, $displayname, $mainEmail, $quota, $claims, $providerName); - }else{ - $this->logger->info("PROV {$uid}: No create"); - return array('allowed' => true, 'reason' => 'No create - please login with the user', 'changed' => true); - } + return $this->deriveNewAccountState($uid, $displayname, $mainEmail, $quota, $claims, $providerName); + } else { + $this->logger->info("PROV {$uid}: No create"); + return array('allowed' => true, 'reason' => 'No create - please login with the user', 'changed' => true); + } } /** @@ -161,7 +158,7 @@ public function deriveAccountState(string $uid, ?string $displayname, ?string $m * In many negative cases, no user account is created at all. */ protected function deriveNewAccountState(string $uid, ?string $displayname, ?string $mainEmail, - string $quota, object $claims, $providerName) : array { + string $quota, object $claims, $providerName) : array { if (is_null($displayname)) { $this->logger->error("{$uid}: New user without displayName"); return array('allowed' => false, 'reason' => 'No displayname no new account', 'changed' => false); @@ -180,7 +177,7 @@ protected function deriveNewAccountState(string $uid, ?string $displayname, ?str return array('allowed' => false, 'reason' => 'No tariff no new account', 'changed' => false, 'redirect' => $withdrawUrl); } - $this->nmcUserService->create($providerName, $uid, $displayname, $mainEmail, $quota); + $this->nmcUserService->create($providerName, $uid, $displayname, $mainEmail, $quota); $this->logger->info("{$uid}: New user created"); return array('allowed' => true, 'reason' => 'Created', 'changed' => true); } @@ -212,7 +209,7 @@ public function deriveExistingAccountState(User|IUser $user, ?string $displaynam // update case // user is active and gets update - //TODO Check is update required???? + //TODO Check is update required???? $this->nmcUserService->update($user, $displayname, $mainEmail, $quota, true); return array('allowed' => true, 'reason' => 'Updated', 'changed' => true); } else { diff --git a/lib/User/NmcUserService.php b/lib/User/NmcUserService.php index 03e5220..b0334d3 100644 --- a/lib/User/NmcUserService.php +++ b/lib/User/NmcUserService.php @@ -133,9 +133,9 @@ public function userExists(string $provider, string $username, bool $returnUser try { $user = $this->findUser($provider, $username); - if ($returnUser){ - return $user; - } + if ($returnUser) { + return $user; + } return true; } catch (NotFoundException $eNotFound) { @@ -239,41 +239,39 @@ public function findAll(string $provider, string $pattern = null, ?int $limit = /** * Encapsulation */ - protected function createOidcUser(string $providerId, string $username, string $displayname) - { + protected function createOidcUser(string $providerId, string $username, string $displayname) { // old way with hashed names only: // return $this->oidcUserMapper->getOrCreate($providerId, $username); $userId = $this->computeUserId($providerId, $username); $user = new User(); $user->setUserId($userId); - $this->logger->debug("PROV displayname"); - $user->setDisplayName($displayname); + $this->logger->debug("PROV displayname"); + $user->setDisplayName($displayname); return $this->oidcUserMapper->insert($user); } - protected function createAccountUser(IUser $user, $email, string $quota, bool $enabled) - { - /*if ($altemail !== null) { - $this->logger->debug("PROV altemail"); - $userAccount = $this->accountManager->getAccount($user); - $userAccount->setProperty(IAccountManager::PROPERTY_ADDRESS, $altemail, - IAccountManager::SCOPE_PRIVATE, IAccountManager::VERIFIED); - $this->accountManager->updateAccount($userAccount); - }*/ - - if ($email !== null) { - $this->logger->debug("PROV email"); - $user->setEMailAddress($email); - } - - $this->logger->debug("PROV quota"); - $user->setQuota($quota); - $this->autoGroupMatch($user, $quota); - $this->logger->debug("PROV enable"); - $user->setEnabled($enabled); - /*$this->logger->debug("PROV migration flag"); - $this->setMigrationFlag($user->getUID(), $migrated);*/ - } + protected function createAccountUser(IUser $user, $email, string $quota, bool $enabled) { + /*if ($altemail !== null) { + $this->logger->debug("PROV altemail"); + $userAccount = $this->accountManager->getAccount($user); + $userAccount->setProperty(IAccountManager::PROPERTY_ADDRESS, $altemail, + IAccountManager::SCOPE_PRIVATE, IAccountManager::VERIFIED); + $this->accountManager->updateAccount($userAccount); + }*/ + + if ($email !== null) { + $this->logger->debug("PROV email"); + $user->setEMailAddress($email); + } + + $this->logger->debug("PROV quota"); + $user->setQuota($quota); + $this->autoGroupMatch($user, $quota); + $this->logger->debug("PROV enable"); + $user->setEnabled($enabled); + /*$this->logger->debug("PROV migration flag"); + $this->setMigrationFlag($user->getUID(), $migrated);*/ + } /** @@ -290,94 +288,93 @@ public function create(string $provider, throw new UserExistException("OpenID user " . $provider . ":" . $username . " already exists!"); }*/ - //Create oidc user + //Create oidc user $this->logger->debug("PROV create db user"); - $oidcUser = $this->createOidcUser($providerId, $username, $displayname); + $oidcUser = $this->createOidcUser($providerId, $username, $displayname); - //Create account user + //Create account user $this->logger->debug("PROV standard account"); $user = $this->userManager->get($oidcUser->getUserId()); $this->logger->info("UserID: ".$oidcUser->getUserId()); - $this->createAccountUser($user, $email, $quota, $enabled); - -/* try { - $this->logger->debug("PROV folder"); - $this->serverc->get('UserFolder')->create($user->getUID()); - $userFolder = $this->serverc->getUserFolder($user->getUID()); - \OC::$server->getLogger()->debug('nmcuser_oidc: User folder created "' . $user->getUID() . '", exists=' . ($this->serverc->getRootFolder()->nodeExists('/' . $user->getUID() . '/files') ? 'true' : 'false'), ['app' => 'debug_create']); - - // Write a temporary file to the user home to make sure it is properly setup - // FIXME: Remove once the issue with the missing user directory on concurrent webdav requests are sorted out - $file = $userFolder->newFile('.userCreateTemp'); - $file->delete(); - - // copy skeleton - \OC_Util::copySkeleton($user->getUID(), $userFolder); - } catch (NotPermittedException $ex) { - \OC::$server->getLogger()->logException($ex, ['app' => 'nmcuser_oidc']); - throw new ForbiddenException("Newly created user cannot init home folder. Reason:\n" . $ex->getMessage()); - }*/ + $this->createAccountUser($user, $email, $quota, $enabled); + + /* try { + $this->logger->debug("PROV folder"); + $this->serverc->get('UserFolder')->create($user->getUID()); + $userFolder = $this->serverc->getUserFolder($user->getUID()); + \OC::$server->getLogger()->debug('nmcuser_oidc: User folder created "' . $user->getUID() . '", exists=' . ($this->serverc->getRootFolder()->nodeExists('/' . $user->getUID() . '/files') ? 'true' : 'false'), ['app' => 'debug_create']); + + // Write a temporary file to the user home to make sure it is properly setup + // FIXME: Remove once the issue with the missing user directory on concurrent webdav requests are sorted out + $file = $userFolder->newFile('.userCreateTemp'); + $file->delete(); + + // copy skeleton + \OC_Util::copySkeleton($user->getUID(), $userFolder); + } catch (NotPermittedException $ex) { + \OC::$server->getLogger()->logException($ex, ['app' => 'nmcuser_oidc']); + throw new ForbiddenException("Newly created user cannot init home folder. Reason:\n" . $ex->getMessage()); + }*/ return [ 'id' => $oidcUser->getUserId() ]; } - protected function updateUserSettings(IUser $user, string|null $email, string|null $quota, bool $enabled){ - if (!empty($email) && - strtolower($email) !== strtolower($user->getEMailAddress())) { - $this->logger->debug("PROV email {$user->getUID()} old: {$user->getEMailAddress()} new: {$email}"); - $user->setEMailAddress($email); - } - - //Its enough to check the string value from quota, the group mapping is matching with exact quota string - if (!is_null($quota) && - $quota !== $user->getQuota()) { - $this->logger->debug("PROV quota {$user->getUID()} old: {$user->getQuota()} new: {$quota}"); - $user->setQuota($quota); - $this->autoGroupMatch($user, $quota); - } - - if ($enabled !== $user->isEnabled()) { - $user->setEnabled($enabled); - } - } - - protected function updateOidcUser(User $oidcUser, string|null $displayname) - { - //Check is displayname changed - if (!is_null($displayname) && - $displayname !== $oidcUser->getDisplayName()) { - $this->logger->debug("PROV displayname {$oidcUser->getUserId()} old: {$oidcUser->getDisplayName()} new: {$displayname}"); - $oidcUser->setDisplayName($displayname); - $this->oidcUserMapper->update($oidcUser); - } - } - - public function update(User|IUser $user, + protected function updateUserSettings(IUser $user, string|null $email, string|null $quota, bool $enabled) { + if (!empty($email) && + strtolower($email) !== strtolower($user->getEMailAddress())) { + $this->logger->debug("PROV email {$user->getUID()} old: {$user->getEMailAddress()} new: {$email}"); + $user->setEMailAddress($email); + } + + //Its enough to check the string value from quota, the group mapping is matching with exact quota string + if (!is_null($quota) && + $quota !== $user->getQuota()) { + $this->logger->debug("PROV quota {$user->getUID()} old: {$user->getQuota()} new: {$quota}"); + $user->setQuota($quota); + $this->autoGroupMatch($user, $quota); + } + + if ($enabled !== $user->isEnabled()) { + $user->setEnabled($enabled); + } + } + + protected function updateOidcUser(User $oidcUser, string|null $displayname) { + //Check is displayname changed + if (!is_null($displayname) && + $displayname !== $oidcUser->getDisplayName()) { + $this->logger->debug("PROV displayname {$oidcUser->getUserId()} old: {$oidcUser->getDisplayName()} new: {$displayname}"); + $oidcUser->setDisplayName($displayname); + $this->oidcUserMapper->update($oidcUser); + } + } + + public function update(User|IUser $user, $displayname = null, $email = null, $quota = null, bool $enabled = null) { $this->logger->debug("PROV standard account"); $oidcUser = $this->oidcUserMapper->getUser($user->getUID()); -// $userAccount = $this->accountManager->getAccount($user); + // $userAccount = $this->accountManager->getAccount($user); -/* if ($altemail !== null) { - $this->logger->debug("PROV altemail"); - $userAccount->setProperty(IAccountManager::PROPERTY_ADDRESS, $altemail, - IAccountManager::SCOPE_PRIVATE, IAccountManager::VERIFIED); - $this->accountManager->updateAccount($userAccount); - }*/ + /* if ($altemail !== null) { + $this->logger->debug("PROV altemail"); + $userAccount->setProperty(IAccountManager::PROPERTY_ADDRESS, $altemail, + IAccountManager::SCOPE_PRIVATE, IAccountManager::VERIFIED); + $this->accountManager->updateAccount($userAccount); + }*/ -/* if ($migrated !== null) { - $this->logger->debug("PROV migration flag"); - $this->setMigrationFlag($user->getUID(), $migrated); - }*/ + /* if ($migrated !== null) { + $this->logger->debug("PROV migration flag"); + $this->setMigrationFlag($user->getUID(), $migrated); + }*/ - $this->updateOidcUser($oidcUser, $displayname); + $this->updateOidcUser($oidcUser, $displayname); - $this->updateUserSettings($user, $email, $quota, $enabled); + $this->updateUserSettings($user, $email, $quota, $enabled); $this->logger->debug("PROV read state"); $userState = [