From c2b103dc18c3498615690429d58a7e5309985e50 Mon Sep 17 00:00:00 2001 From: Maciej Zieniuk Date: Wed, 19 Mar 2025 22:48:00 +0000 Subject: [PATCH] organization observable race condition in key-connector --- .../abstractions/key-connector.service.ts | 2 +- .../services/key-connector.service.ts | 24 +++++++++++-------- .../src/platform/sync/default-sync.service.ts | 13 ++++++---- 3 files changed, 23 insertions(+), 16 deletions(-) diff --git a/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts b/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts index 17b2b4d91c9..e7f6d54b070 100644 --- a/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/abstractions/key-connector.service.ts @@ -11,7 +11,7 @@ export abstract class KeyConnectorService { abstract migrateUser(userId: UserId): Promise; - abstract userNeedsMigration(userId: UserId): Promise; + abstract userNeedsMigration(userId: UserId, organizations: Organization[]): Promise; abstract convertNewSsoUserToKeyConnector( tokenResponse: IdentityTokenResponse, diff --git a/libs/common/src/key-management/key-connector/services/key-connector.service.ts b/libs/common/src/key-management/key-connector/services/key-connector.service.ts index 5c21a4b618c..8871369dc62 100644 --- a/libs/common/src/key-management/key-connector/services/key-connector.service.ts +++ b/libs/common/src/key-management/key-connector/services/key-connector.service.ts @@ -82,9 +82,9 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { return firstValueFrom(this.stateProvider.getUserState$(USES_KEY_CONNECTOR, userId)); } - async userNeedsMigration(userId: UserId) { + async userNeedsMigration(userId: UserId, organizations: Organization[]) { const loggedInUsingSso = await this.tokenService.getIsExternal(userId); - const requiredByOrganization = (await this.getManagingOrganization(userId)) != null; + const requiredByOrganization = this.findManagingOrganization(organizations) != null; const userIsNotUsingKeyConnector = !(await this.getUsesKeyConnector(userId)); return loggedInUsingSso && requiredByOrganization && userIsNotUsingKeyConnector; @@ -120,14 +120,8 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { } async getManagingOrganization(userId: UserId): Promise { - const orgs = await firstValueFrom(this.organizationService.organizations$(userId)); - return orgs.find( - (o) => - o.keyConnectorEnabled && - o.type !== OrganizationUserType.Admin && - o.type !== OrganizationUserType.Owner && - !o.isProviderUser, - ); + const organizations = await firstValueFrom(this.organizationService.organizations$(userId)); + return this.findManagingOrganization(organizations); } async convertNewSsoUserToKeyConnector( @@ -203,4 +197,14 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction { } throw new Error("Key Connector error"); } + + private findManagingOrganization(organizations: Organization[]) { + return organizations.find( + (o) => + o.keyConnectorEnabled && + o.type !== OrganizationUserType.Admin && + o.type !== OrganizationUserType.Owner && + !o.isProviderUser, + ); + } } diff --git a/libs/common/src/platform/sync/default-sync.service.ts b/libs/common/src/platform/sync/default-sync.service.ts index 656267d864c..dd4d7b82b49 100644 --- a/libs/common/src/platform/sync/default-sync.service.ts +++ b/libs/common/src/platform/sync/default-sync.service.ts @@ -7,6 +7,7 @@ import { CollectionData, CollectionDetailsResponse, } from "@bitwarden/admin-console/common"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; import { KeyService } from "@bitwarden/key-management"; // FIXME: remove `src` and fix import @@ -213,15 +214,13 @@ export class DefaultSyncService extends CoreSyncService { await this.providerService.save(providers, response.id); - await this.syncProfileOrganizations(response, response.id); + const organizations = await this.syncProfileOrganizations(response, response.id); - if (await this.keyConnectorService.userNeedsMigration(response.id)) { + if (await this.keyConnectorService.userNeedsMigration(response.id, organizations)) { await this.keyConnectorService.setConvertAccountRequired(true, response.id); this.messageSender.send("convertAccountToKeyConnector"); } else { - // FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling. - // eslint-disable-next-line @typescript-eslint/no-floating-promises - this.keyConnectorService.removeConvertAccountRequired(response.id); + await this.keyConnectorService.removeConvertAccountRequired(response.id); } } @@ -293,6 +292,10 @@ export class DefaultSyncService extends CoreSyncService { }); await this.organizationService.replace(organizations, userId); + + return Object.values(organizations).map( + (organizationData) => new Organization(organizationData), + ); } private async syncFolders(response: FolderResponse[], userId: UserId) {