From ea994d76522df18acbafcb6039a9ba8c6a52a99f Mon Sep 17 00:00:00 2001 From: Akhil Date: Thu, 29 May 2025 21:50:29 +0530 Subject: [PATCH 1/9] Only migrate TOTP secret to SSO if a secret is actually enabled --- lib/Db/TwoFactorMapper.php | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/lib/Db/TwoFactorMapper.php b/lib/Db/TwoFactorMapper.php index 5b9739ab..eeecccf4 100644 --- a/lib/Db/TwoFactorMapper.php +++ b/lib/Db/TwoFactorMapper.php @@ -2,21 +2,28 @@ namespace OCA\EcloudAccounts\Db; +use OCA\TwoFactorTOTP\Provider\TotpProvider; use OCP\IDBConnection; +use OCP\IUser; +use OCP\IUserManager; class TwoFactorMapper { private IDBConnection $conn; private const TOTP_SECRET_TABLE = 'twofactor_totp_secrets'; + private TotpProvider $totpProvider; + private IUserManager $userManager; - public function __construct(IDBConnection $conn) { + public function __construct(IDBConnection $conn, TotpProvider $totpProvider, IUserManager $userManager) { $this->conn = $conn; + $this->totpProvider = $totpProvider; + $this->userManager = $userManager; } public function getEntries(array $usernames = []) : array { $entries = []; $qb = $this->conn->getQueryBuilder(); - $qb->select('user_id', 'secret') + $qb->select('user_id', 'secret', 'stat') ->from(self::TOTP_SECRET_TABLE); if (!empty($usernames)) { @@ -26,8 +33,19 @@ class TwoFactorMapper { $result = $qb->execute(); while ($row = $result->fetch()) { + $username = (string) $row['user_id']; + $user = $this->userManager->get($username); + + if (!$user instanceof IUser) { + continue; + } + + if (!$this->totpProvider->isTwoFactorAuthenticated($user)) { + continue; + } + $entry = [ - 'username' => (string) $row['user_id'], + 'username' => $username, 'secret' => (string) $row['secret'] ]; $entries[] = $entry; -- GitLab From 384a12272221cfc657ee5b2a5e4b4a32dd70fa05 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 17:10:01 +0530 Subject: [PATCH 2/9] Query only for entries from 2FA table that are enabled --- lib/Command/Migrate2FASecrets.php | 2 +- lib/Db/TwoFactorMapper.php | 25 +++++++------------------ 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/lib/Command/Migrate2FASecrets.php b/lib/Command/Migrate2FASecrets.php index 69683768..512beab5 100644 --- a/lib/Command/Migrate2FASecrets.php +++ b/lib/Command/Migrate2FASecrets.php @@ -57,7 +57,7 @@ class Migrate2FASecrets extends Command { * @return void */ private function migrateUsers(array $usernames = []) : void { - $entries = $this->twoFactorMapper->getEntries($usernames); + $entries = $this->twoFactorMapper->getEnabledUsers($usernames); foreach ($entries as $entry) { try { $this->ssoService->migrateCredential($entry['username'], $entry['secret']); diff --git a/lib/Db/TwoFactorMapper.php b/lib/Db/TwoFactorMapper.php index eeecccf4..6488cac0 100644 --- a/lib/Db/TwoFactorMapper.php +++ b/lib/Db/TwoFactorMapper.php @@ -3,46 +3,35 @@ namespace OCA\EcloudAccounts\Db; use OCA\TwoFactorTOTP\Provider\TotpProvider; +use OCA\TwoFactorTOTP\Service\ITotp; use OCP\IDBConnection; -use OCP\IUser; -use OCP\IUserManager; class TwoFactorMapper { private IDBConnection $conn; private const TOTP_SECRET_TABLE = 'twofactor_totp_secrets'; private TotpProvider $totpProvider; - private IUserManager $userManager; - public function __construct(IDBConnection $conn, TotpProvider $totpProvider, IUserManager $userManager) { + public function __construct(IDBConnection $conn, TotpProvider $totpProvider) { $this->conn = $conn; $this->totpProvider = $totpProvider; - $this->userManager = $userManager; } - public function getEntries(array $usernames = []) : array { + public function getEnabledUsers(array $usernames = []) : array { $entries = []; $qb = $this->conn->getQueryBuilder(); - $qb->select('user_id', 'secret', 'stat') - ->from(self::TOTP_SECRET_TABLE); + $qb->select('user_id', 'secret') + ->from(self::TOTP_SECRET_TABLE) + ->where($qb->expr()->eq($qb->expr()->castColumn('state', IQueryBuilder::PARAM_INT), ITotp::STATE_ENABLED)); if (!empty($usernames)) { - $qb->where('user_id IN (:usernames)') + $qb->andWhere('user_id IN (:usernames)') ->setParameter('usernames', implode(',', $usernames)); } $result = $qb->execute(); while ($row = $result->fetch()) { $username = (string) $row['user_id']; - $user = $this->userManager->get($username); - - if (!$user instanceof IUser) { - continue; - } - - if (!$this->totpProvider->isTwoFactorAuthenticated($user)) { - continue; - } $entry = [ 'username' => $username, -- GitLab From 8a516be8abd419ff29faf96a041683d627be67b1 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 17:30:16 +0530 Subject: [PATCH 3/9] Output usernames in migrate command --- lib/Command/Migrate2FASecrets.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Command/Migrate2FASecrets.php b/lib/Command/Migrate2FASecrets.php index 512beab5..800f3324 100644 --- a/lib/Command/Migrate2FASecrets.php +++ b/lib/Command/Migrate2FASecrets.php @@ -60,9 +60,10 @@ class Migrate2FASecrets extends Command { $entries = $this->twoFactorMapper->getEnabledUsers($usernames); foreach ($entries as $entry) { try { + $this->commandOutput->writeln('Migrating 2FA credential for user: ' . $entry['username']); $this->ssoService->migrateCredential($entry['username'], $entry['secret']); } catch (\Exception $e) { - $this->commandOutput->writeln('Error inserting entry for user ' . $entry['username'] . ' message: ' . $e->getMessage()); + $this->commandOutput->writeln('Error migrating 2FA credential for user ' . $entry['username'] . ' message: ' . $e->getMessage()); continue; } } -- GitLab From e59c9fff557254848e8d0f7859378444a303248a Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 17:33:29 +0530 Subject: [PATCH 4/9] Remove double-check of credential subType --- 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 61882dc2..31263245 100644 --- a/lib/Service/SSOService.php +++ b/lib/Service/SSOService.php @@ -124,8 +124,8 @@ class SSOService { return false; } $credentialData = json_decode($credential['credentialData'], true); - if (!isset($credentialData['subType']) || !isset($credentialData['subType']) - || $credentialData['subType'] !== 'totp' || $credentialData['secretEncoding'] !== 'BASE32') { + if (!isset($credentialData['subType']) || $credentialData['subType'] !== 'totp' + || $credentialData['secretEncoding'] !== 'BASE32') { return false; } return true; -- GitLab From 00179c23ef1e6d62143388c59e5a4720ee0aa568 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 17:34:40 +0530 Subject: [PATCH 5/9] Remove TotpProvider import --- lib/Db/TwoFactorMapper.php | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/Db/TwoFactorMapper.php b/lib/Db/TwoFactorMapper.php index 6488cac0..9e33fc0e 100644 --- a/lib/Db/TwoFactorMapper.php +++ b/lib/Db/TwoFactorMapper.php @@ -2,19 +2,16 @@ namespace OCA\EcloudAccounts\Db; -use OCA\TwoFactorTOTP\Provider\TotpProvider; use OCA\TwoFactorTOTP\Service\ITotp; use OCP\IDBConnection; class TwoFactorMapper { private IDBConnection $conn; private const TOTP_SECRET_TABLE = 'twofactor_totp_secrets'; - private TotpProvider $totpProvider; - public function __construct(IDBConnection $conn, TotpProvider $totpProvider) { + public function __construct(IDBConnection $conn) { $this->conn = $conn; - $this->totpProvider = $totpProvider; } public function getEnabledUsers(array $usernames = []) : array { -- GitLab From 8aa11af1210c620594c99ef28b813d96f6c9d1a0 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 17:37:01 +0530 Subject: [PATCH 6/9] Allow staging deployment temporarily --- .gitlab-ci.yml | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 08326169..e7e07f85 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,3 +37,20 @@ build-vendor: artifacts: paths: - dist/ + +deploy:staging: + extends: .deploy:nextcloud-app + rules: + - if: $CI_COMMIT_BRANCH == "main" + when: manual + - if: $CI_COMMIT_BRANCH == "murena-main" + when: manual + - if: $CI_COMMIT_BRANCH == "production" + when: manual + - if: $CI_COMMIT_BRANCH == "dev/fix-totp-migration" + when: manual + - if: $CI_COMMIT_TAG + when: manual + environment: + name: staging/01 + url: $ENV_URL -- GitLab From f88723e8b6be7dcf4478274492fe80adc747b0e4 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 18:13:45 +0530 Subject: [PATCH 7/9] use a named parameter in query builder expression --- lib/Command/Migrate2FASecrets.php | 3 ++- lib/Db/TwoFactorMapper.php | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/Command/Migrate2FASecrets.php b/lib/Command/Migrate2FASecrets.php index 800f3324..bbe344a1 100644 --- a/lib/Command/Migrate2FASecrets.php +++ b/lib/Command/Migrate2FASecrets.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace OCA\EcloudAccounts\Command; +use OCA\EcloudAccounts\AppInfo\Application; use OCA\EcloudAccounts\Db\TwoFactorMapper; use OCA\EcloudAccounts\Service\SSOService; use Symfony\Component\Console\Command\Command; @@ -24,7 +25,7 @@ class Migrate2FASecrets extends Command { protected function configure(): void { $this - ->setName('ecloud-accounts:migrate-2fa-secrets') + ->setName(Application::APP_ID . ':migrate-2fa-secrets') ->setDescription('Migrates 2FA secrets to SSO database') ->addOption( 'users', diff --git a/lib/Db/TwoFactorMapper.php b/lib/Db/TwoFactorMapper.php index 9e33fc0e..e5eefce0 100644 --- a/lib/Db/TwoFactorMapper.php +++ b/lib/Db/TwoFactorMapper.php @@ -19,7 +19,11 @@ class TwoFactorMapper { $qb = $this->conn->getQueryBuilder(); $qb->select('user_id', 'secret') ->from(self::TOTP_SECRET_TABLE) - ->where($qb->expr()->eq($qb->expr()->castColumn('state', IQueryBuilder::PARAM_INT), ITotp::STATE_ENABLED)); + ->where( + $qb->expr()->eq( + 'state', $qb->createNamedParameter(ITotp::STATE_ENABLED) + ) + ); if (!empty($usernames)) { $qb->andWhere('user_id IN (:usernames)') -- GitLab From b7d526b245dcb00f9028dc55393e742861ea4afe Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 18:33:27 +0530 Subject: [PATCH 8/9] Fix query to fetch secrets given a list of users --- lib/Db/TwoFactorMapper.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/Db/TwoFactorMapper.php b/lib/Db/TwoFactorMapper.php index e5eefce0..1503de29 100644 --- a/lib/Db/TwoFactorMapper.php +++ b/lib/Db/TwoFactorMapper.php @@ -3,6 +3,7 @@ namespace OCA\EcloudAccounts\Db; use OCA\TwoFactorTOTP\Service\ITotp; +use OCP\DB\QueryBuilder\IQueryBuilder; use OCP\IDBConnection; class TwoFactorMapper { @@ -27,7 +28,7 @@ class TwoFactorMapper { if (!empty($usernames)) { $qb->andWhere('user_id IN (:usernames)') - ->setParameter('usernames', implode(',', $usernames)); + ->setParameter('usernames', $usernames, IQueryBuilder::PARAM_STR_ARRAY); } $result = $qb->execute(); -- GitLab From 49c68b49dc68df2ac1d05ad782796ddbebdb33d7 Mon Sep 17 00:00:00 2001 From: Akhil Date: Wed, 4 Jun 2025 18:54:29 +0530 Subject: [PATCH 9/9] Remove temporary CI changes to deploy to staging --- .gitlab-ci.yml | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index e7e07f85..08326169 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -37,20 +37,3 @@ build-vendor: artifacts: paths: - dist/ - -deploy:staging: - extends: .deploy:nextcloud-app - rules: - - if: $CI_COMMIT_BRANCH == "main" - when: manual - - if: $CI_COMMIT_BRANCH == "murena-main" - when: manual - - if: $CI_COMMIT_BRANCH == "production" - when: manual - - if: $CI_COMMIT_BRANCH == "dev/fix-totp-migration" - when: manual - - if: $CI_COMMIT_TAG - when: manual - environment: - name: staging/01 - url: $ENV_URL -- GitLab