From d46859a4037b5c6a1d47f438c6a47f89b4196cc2 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Thu, 24 Jul 2025 18:58:36 +0600 Subject: [PATCH 1/5] feat: for SSO, use already saved KC uid oidc_login should already saved the KC uid in the preference DB table for loggedIn user. So, we don't need to retrieve the uid for each SSO requests. issue: https://gitlab.e.foundation/e/infra/backlog/-/issues/4352 --- lib/Service/SSOService.php | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index 8436d8eb..0e19d946 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -64,7 +64,7 @@ class SSOService { public function migrateCredential(string $username, string $secret) : void { if($this->isNotCurrentUser($username)) { - $this->getUserId($username); + $this->setupUserId($username); } $this->deleteCredentials($username); @@ -84,7 +84,7 @@ class SSOService { public function deleteCredentials(string $username) : void { if($this->isNotCurrentUser($username)) { - $this->getUserId($username); + $this->setupUserId($username); } $credentialIds = $this->getCredentialIds(); @@ -100,7 +100,7 @@ class SSOService { public function logout(string $username) : void { if($this->isNotCurrentUser($username)) { - $this->getUserId($username); + $this->setupUserId($username); } $url = $this->ssoConfig['admin_rest_api_url'] . self::USERS_ENDPOINT . '/' . $this->currentUserId . '/logout'; @@ -159,7 +159,19 @@ class SSOService { return $credentialEntry; } - private function getUserId(string $username) : void { + private function setupUserId(string $username) { + $user = $this->userManager->get($username); + $savedOIDCUid = $this->config->getUserValue($user->getUID(), 'oidc_login', 'oidc_uid'); + + if ($savedOIDCUid !== null && trim($savedOIDCUid) !== '') { + $this->currentUserId = $savedOIDCUid; + return; + } + + $this->retriveUserId($username); + } + + private function retriveUserId(string $username) { $user = $this->userManager->get($username); if ($user === null) { throw new SSOAdminAPIException('Error: no user exists in cloud with username ' . $username); -- GitLab From f43390db3113a598e3e3c1ac81cac5e04d4f9c93 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 25 Jul 2025 13:05:08 +0600 Subject: [PATCH 2/5] feat: Implement 2FASyncRetryBGJob Sometimes because of network issue, syncing 2FA state change with SSO Provider failed. To mitigate this, we have implemented a background job which will try max 4 times to resync. issue: https://gitlab.e.foundation/e/infra/backlog/-/issues/4352 --- lib/Cron/TwoFactorStateChangeJob.php | 60 +++++++++++++++++++ .../TwoFactorStateChangedListener.php | 29 ++++----- lib/Service/SSOService.php | 17 +++++- 3 files changed, 91 insertions(+), 15 deletions(-) create mode 100644 lib/Cron/TwoFactorStateChangeJob.php diff --git a/lib/Cron/TwoFactorStateChangeJob.php b/lib/Cron/TwoFactorStateChangeJob.php new file mode 100644 index 00000000..5ace28cc --- /dev/null +++ b/lib/Cron/TwoFactorStateChangeJob.php @@ -0,0 +1,60 @@ +jobList = $jobList; + $this->ssoService = $ssoService; + $this->logger = $logger; + + $this->setAllowParallelRuns(true); + } + + protected function run($arguments) { + $enabled = $arguments[$this::ENABLED_KEY]; + $username = $arguments[$this::USERNAME_KEY]; + $tryCount = $arguments[$this::TRYCOUNT_KEY]; + + try { + $this->ssoService->handle2FAStateChange($enabled, $username); + } catch (Exception $e) { + if ($tryCount > 2) { + $this->logger->logException($e, ['app' => Application::APP_ID]); + return; + } + + $tryCount = $tryCount + 1; + $arguments[$this::TRYCOUNT_KEY] = $tryCount; + $this->jobList->scheduleAfter( + TwoFactorStateChangeJob::class, + $arguments, + 60 + ); + } + } +} diff --git a/lib/Listeners/TwoFactorStateChangedListener.php b/lib/Listeners/TwoFactorStateChangedListener.php index 32308841..0ae5e8e3 100644 --- a/lib/Listeners/TwoFactorStateChangedListener.php +++ b/lib/Listeners/TwoFactorStateChangedListener.php @@ -5,27 +5,27 @@ declare(strict_types=1); namespace OCA\EcloudAccounts\Listeners; use OCA\EcloudAccounts\AppInfo\Application; -use OCA\EcloudAccounts\Db\TwoFactorMapper; +use OCA\EcloudAccounts\Cron\TwoFactorStateChangeJob; use OCA\EcloudAccounts\Service\SSOService; use OCA\TwoFactorTOTP\Event\StateChanged; use OCP\App\IAppManager; +use OCP\BackgroundJob\IJobList; use OCP\EventDispatcher\Event; use OCP\EventDispatcher\IEventListener; use OCP\ILogger; class TwoFactorStateChangedListener implements IEventListener { private IAppManager $appManager; - private TwoFactorMapper $twoFactorMapper; private SSOService $ssoService; + private IJobList $jobList; private ILogger $logger; private const TWOFACTOR_APP_ID = 'twofactor_totp'; - - public function __construct(IAppManager $appManager, SSOService $ssoService, TwoFactorMapper $twoFactorMapper, ILogger $logger) { + public function __construct(IAppManager $appManager, IJobList $jobList, SSOService $ssoService, ILogger $logger) { $this->appManager = $appManager; $this->ssoService = $ssoService; - $this->twoFactorMapper = $twoFactorMapper; + $this->jobList = $jobList; $this->logger = $logger; } @@ -37,18 +37,19 @@ class TwoFactorStateChangedListener implements IEventListener { $user = $event->getUser(); $username = $user->getUID(); + $enabled = $event->isEnabled(); try { - // When state change event is fired by user disabling 2FA, delete existing 2FA credentials and return - // i.e. disable 2FA for user at SSO - if (!$event->isEnabled()) { - $this->ssoService->deleteCredentials($username); - return; - } - - $secret = $this->twoFactorMapper->getSecret($username); - $this->ssoService->migrateCredential($username, $secret); + $this->ssoService->handle2FAStateChange($enabled, $username); } catch (Exception $e) { $this->logger->logException($e, ['app' => Application::APP_ID]); + + // faced exception. Initiate BG job to retry again. + $arguments = [ + TwoFactorStateChangeJob::ENABLED_KEY => $event->isEnabled(), + TwoFactorStateChangeJob::USERNAME_KEY => $username, + TwoFactorStateChangeJob::TRYCOUNT_KEY => 0 + ]; + $this->jobList->add(TwoFactorStateChangeJob::class, $arguments); } } } diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index 0e19d946..baaa4026 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -4,6 +4,7 @@ namespace OCA\EcloudAccounts\Service; use OCA\EcloudAccounts\AppInfo\Application; +use OCA\EcloudAccounts\Db\TwoFactorMapper; use OCA\EcloudAccounts\Exception\SSOAdminAccessTokenException; use OCA\EcloudAccounts\Exception\SSOAdminAPIException; use OCP\IConfig; @@ -25,6 +26,7 @@ class SSOService { private ICrypto $crypto; private IFactory $l10nFactory; private IUserManager $userManager; + private TwoFactorMapper $twoFactorMapper; private string $mainDomain; private string $legacyDomain; @@ -33,7 +35,7 @@ class SSOService { private const USERS_ENDPOINT = '/users'; private const CREDENTIALS_ENDPOINT = '/users/{USER_ID}/credentials'; - public function __construct($appName, IConfig $config, CurlService $curlService, ICrypto $crypto, IFactory $l10nFactory, IUserManager $userManager, ILogger $logger) { + public function __construct($appName, IConfig $config, CurlService $curlService, ICrypto $crypto, IFactory $l10nFactory, IUserManager $userManager, TwoFactorMapper $twoFactorMapper, ILogger $logger) { $this->appName = $appName; $this->config = $config; @@ -53,6 +55,7 @@ class SSOService { $this->logger = $logger; $this->l10nFactory = $l10nFactory; $this->userManager = $userManager; + $this->twoFactorMapper = $twoFactorMapper; $this->mainDomain = $this->config->getSystemValue("main_domain"); $this->legacyDomain = $this->config->getSystemValue("legacy_domain"); @@ -109,6 +112,18 @@ class SSOService { $this->callSSOAPI($url, 'POST', [], 204); } + public function handle2FAStateChange(bool $enabled, string $username) { + // When state change event is fired by user disabling 2FA, delete existing 2FA credentials and return + // i.e. disable 2FA for user at SSO + if (!$enabled) { + $this->deleteCredentials($username); + return; + } + + $secret = $this->twoFactorMapper->getSecret($username); + $this->migrateCredential($username, $secret); + } + private function getCredentialIds() : array { $url = $this->ssoConfig['admin_rest_api_url'] . self::CREDENTIALS_ENDPOINT; $url = str_replace('{USER_ID}', $this->currentUserId, $url); -- GitLab From 05e860d793d1a4816e3aa78632babcee0b9395d5 Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 25 Jul 2025 15:17:01 +0600 Subject: [PATCH 3/5] chore: bump verion to 10.1.1 --- appinfo/info.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appinfo/info.xml b/appinfo/info.xml index e2145280..fede4817 100644 --- a/appinfo/info.xml +++ b/appinfo/info.xml @@ -10,7 +10,7 @@ - 10.1.0 + 10.1.1 agpl Murena SAS EcloudAccounts -- GitLab From b9e5b8379a8ad8b02243d10664843c60dd4dee6d Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Fri, 25 Jul 2025 18:15:55 +0600 Subject: [PATCH 4/5] chore: update lint changes based on review --- lib/Cron/TwoFactorStateChangeJob.php | 11 ++++++----- lib/Service/SSOService.php | 4 ++-- 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/Cron/TwoFactorStateChangeJob.php b/lib/Cron/TwoFactorStateChangeJob.php index 5ace28cc..dbdd32ae 100644 --- a/lib/Cron/TwoFactorStateChangeJob.php +++ b/lib/Cron/TwoFactorStateChangeJob.php @@ -20,6 +20,7 @@ class TwoFactorStateChangeJob extends QueuedJob { public const ENABLED_KEY = 'enabled'; public const USERNAME_KEY = 'username'; public const TRYCOUNT_KEY = 'tryCount'; + private const INTERVAL_INBETWEEN_JOB_IN_SEC = 60; private SSOService $ssoService; private IJobList $jobList; @@ -36,9 +37,9 @@ class TwoFactorStateChangeJob extends QueuedJob { } protected function run($arguments) { - $enabled = $arguments[$this::ENABLED_KEY]; - $username = $arguments[$this::USERNAME_KEY]; - $tryCount = $arguments[$this::TRYCOUNT_KEY]; + $enabled = $arguments[self::ENABLED_KEY]; + $username = $arguments[self::USERNAME_KEY]; + $tryCount = $arguments[self::TRYCOUNT_KEY]; try { $this->ssoService->handle2FAStateChange($enabled, $username); @@ -49,11 +50,11 @@ class TwoFactorStateChangeJob extends QueuedJob { } $tryCount = $tryCount + 1; - $arguments[$this::TRYCOUNT_KEY] = $tryCount; + $arguments[self::TRYCOUNT_KEY] = $tryCount; $this->jobList->scheduleAfter( TwoFactorStateChangeJob::class, $arguments, - 60 + self::INTERVAL_INBETWEEN_JOB_IN_SEC ); } } diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index baaa4026..59750c27 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -112,7 +112,7 @@ class SSOService { $this->callSSOAPI($url, 'POST', [], 204); } - public function handle2FAStateChange(bool $enabled, string $username) { + public function handle2FAStateChange(bool $enabled, string $username) : void { // When state change event is fired by user disabling 2FA, delete existing 2FA credentials and return // i.e. disable 2FA for user at SSO if (!$enabled) { @@ -174,7 +174,7 @@ class SSOService { return $credentialEntry; } - private function setupUserId(string $username) { + private function setupUserId(string $username) : void { $user = $this->userManager->get($username); $savedOIDCUid = $this->config->getUserValue($user->getUID(), 'oidc_login', 'oidc_uid'); -- GitLab From aa444a7e3124660cbbca487ed52145f99095539d Mon Sep 17 00:00:00 2001 From: Fahim Salam Chowdhury Date: Tue, 29 Jul 2025 12:56:47 +0600 Subject: [PATCH 5/5] chore: fix typo --- lib/Service/SSOService.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Service/SSOService.php b/lib/Service/SSOService.php index 59750c27..ee17ac45 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -183,10 +183,10 @@ class SSOService { return; } - $this->retriveUserId($username); + $this->retrieveUserId($username); } - private function retriveUserId(string $username) { + private function retrieveUserId(string $username) { $user = $this->userManager->get($username); if ($user === null) { throw new SSOAdminAPIException('Error: no user exists in cloud with username ' . $username); -- GitLab