From 44defce929745bde91abe5373ea75a0e32cd7e07 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Mon, 5 May 2025 15:26:15 +0600 Subject: [PATCH 1/8] fix: validate requested userId with SsoService user --- lib/Service/SSOService.php | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index a6cb4d23..fbdb5d14 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -20,6 +20,7 @@ class SSOService { private array $ssoConfig = []; private string $adminAccessToken; private string $currentUserId; + private string $currentUserName; private ICrypto $crypto; private IFactory $l10nFactory; private IUserManager $userManager; @@ -55,9 +56,10 @@ class SSOService { } public function migrateCredential(string $username, string $secret) : void { - if(empty($this->currentUserId)) { + if($this->isNotCurrentUser($username)) { $this->getUserId($username); } + $this->deleteCredentials($username); $decryptedSecret = $this->crypto->decrypt($secret); @@ -74,9 +76,10 @@ class SSOService { } public function deleteCredentials(string $username) : void { - if(empty($this->currentUserId)) { + if($this->isNotCurrentUser($username)) { $this->getUserId($username); } + $credentialIds = $this->getCredentialIds(); foreach ($credentialIds as $credentialId) { @@ -89,7 +92,7 @@ class SSOService { } public function logout(string $username) : void { - if(empty($this->currentUserId)) { + if($this->isNotCurrentUser($username)) { $this->getUserId($username); } @@ -166,6 +169,7 @@ class SSOService { throw new SSOAdminAPIException('Error: no user found for search with url: ' . $url); } $this->currentUserId = $users[0]['id']; + $this->currentUserName = $username; } private function getAdminAccessToken() : void { @@ -235,4 +239,14 @@ class SSOService { $answer = json_decode($answer, true); return $answer; } + + private function isNotCurrentUser(string $username): bool { + if (!empty($this->currentUserId) && !empty($this->currentUserName) && $username === $this->currentUserName) { + return false; + } + + $this->currentUserId = ''; + $this->currentUserName = ''; + return true; + } } -- GitLab From 8dae520dca6c55d18dd19ff31e41b72efbafbb6d Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 11:42:52 +0600 Subject: [PATCH 2/8] chore: bump to v10.0.1 - update to 10.0.1 (validate userId before making sso request: (https://gitlab.e.foundation/e/infra/backlog/-/issues/4053) - set min-nc-version to 28, to support current prod version --- appinfo/info.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index d4c3e5e2..3997e0ee 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -10,14 +10,14 @@ - 10.0.0 + 10.0.1 agpl Murena SAS EcloudAccounts tools https://gitlab.e.foundation/e/management/issues - + OCA\EcloudAccounts\Settings\DeleteShopAccountSetting -- GitLab From 3e0edbf6f535f0c8721786e1a6f3bc55bc8ebb15 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 16:59:05 +0600 Subject: [PATCH 3/8] fix: validate against KC returned username data --- lib/Service/SSOService.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index fbdb5d14..ee108a19 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -169,7 +169,10 @@ class SSOService { throw new SSOAdminAPIException('Error: no user found for search with url: ' . $url); } $this->currentUserId = $users[0]['id']; - $this->currentUserName = $username; + $this->currentUserName = $users[0]['username']; + if ($username !== $this->currentUserName) { + throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName ') from the Keycloak for ' . $username); + } } private function getAdminAccessToken() : void { -- GitLab From e15d2a49176c6521fb48e29b0fe83748294a2212 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 19:27:20 +0600 Subject: [PATCH 4/8] Apply 1 suggestion(s) to 1 file(s) Co-authored-by: Akhil --- lib/Service/SSOService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index ee108a19..c1059392 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -171,7 +171,7 @@ class SSOService { $this->currentUserId = $users[0]['id']; $this->currentUserName = $users[0]['username']; if ($username !== $this->currentUserName) { - throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName ') from the Keycloak for ' . $username); + throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName ') from SSO service for ' . $username); } } -- GitLab From 14488423c0161b6b56c7aa809780a146072dc9e8 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 19:34:24 +0600 Subject: [PATCH 5/8] chore: simplify isNotCurrentUser method implementation --- lib/Service/SSOService.php | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index c1059392..2ab5d265 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -244,12 +244,6 @@ class SSOService { } private function isNotCurrentUser(string $username): bool { - if (!empty($this->currentUserId) && !empty($this->currentUserName) && $username === $this->currentUserName) { - return false; - } - - $this->currentUserId = ''; - $this->currentUserName = ''; - return true; + return !(!empty($this->currentUserId) && !empty($this->currentUserName) && $username === $this->currentUserName); } } -- GitLab From ac95fe44a74d522ea3eb068b85139808637a0de6 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 20:40:06 +0600 Subject: [PATCH 6/8] fix: sanitize userName for SSO username cross-match We have some username which contains the domain part in it, some donot. To resolve this, we need to remove the domain part from the usernames itself. This can be done by this sanitization. --- lib/Service/SSOService.php | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index 2ab5d265..0010ae06 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -25,6 +25,9 @@ class SSOService { private IFactory $l10nFactory; private IUserManager $userManager; + private string $mainDomain; + private string $legacyDomain; + private const ADMIN_TOKEN_ENDPOINT = '/auth/realms/master/protocol/openid-connect/token'; private const USERS_ENDPOINT = '/users'; private const CREDENTIALS_ENDPOINT = '/users/{USER_ID}/credentials'; @@ -49,6 +52,9 @@ class SSOService { $this->logger = $logger; $this->l10nFactory = $l10nFactory; $this->userManager = $userManager; + + $this->mainDomain = $this->config->getSystemValue("main_domain"); + $this->legacyDomain = $this->config->getSystemValue("legacy_domain"); } public function shouldSync2FA() : bool { @@ -169,9 +175,10 @@ class SSOService { throw new SSOAdminAPIException('Error: no user found for search with url: ' . $url); } $this->currentUserId = $users[0]['id']; - $this->currentUserName = $users[0]['username']; + $this->currentUserName = $this->sanitizeUserName(users[0]['username']); + $username = $this->sanitizeUserName($username) if ($username !== $this->currentUserName) { - throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName ') from SSO service for ' . $username); + throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName . ') from SSO service for ' . $username); } } @@ -243,7 +250,19 @@ class SSOService { return $answer; } + private function sanitizeUserName(string $username): string { + $username = strtolower($username); + + if (str_contains($username, "@" . $this->mainDomain) || str_contains($username, "@" . $this->legacyDomain)) { + list($name, $domain) = explode("@", $username); + $username = $name; + } + + return $username; + } + private function isNotCurrentUser(string $username): bool { + $username = $this->sanitizeUserName($username); return !(!empty($this->currentUserId) && !empty($this->currentUserName) && $username === $this->currentUserName); } } -- GitLab From 264421723008592b034787ab49e87570aa412f00 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 20:46:13 +0600 Subject: [PATCH 7/8] fix: php-lint issue --- lib/Service/SSOService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index 0010ae06..d0842f58 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -176,7 +176,7 @@ class SSOService { } $this->currentUserId = $users[0]['id']; $this->currentUserName = $this->sanitizeUserName(users[0]['username']); - $username = $this->sanitizeUserName($username) + $username = $this->sanitizeUserName($username); if ($username !== $this->currentUserName) { throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName . ') from SSO service for ' . $username); } -- GitLab From 3feed4d65d8d1b115c66052d2bfced335d16977c Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 6 May 2025 22:03:14 +0600 Subject: [PATCH 8/8] fix: php-lint issue --- lib/Service/SSOService.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index d0842f58..05e591f5 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -175,7 +175,7 @@ class SSOService { throw new SSOAdminAPIException('Error: no user found for search with url: ' . $url); } $this->currentUserId = $users[0]['id']; - $this->currentUserName = $this->sanitizeUserName(users[0]['username']); + $this->currentUserName = $this->sanitizeUserName($users[0]['username']); $username = $this->sanitizeUserName($username); if ($username !== $this->currentUserName) { throw new SSOAdminAPIException('Error: retrieved wrong user info (' . $this->currentUserName . ') from SSO service for ' . $username); -- GitLab