diff --git a/libs/angular/src/auth/components/sso.component.ts b/libs/angular/src/auth/components/sso.component.ts index d0fc2140f06..5f5e53d8efe 100644 --- a/libs/angular/src/auth/components/sso.component.ts +++ b/libs/angular/src/auth/components/sso.component.ts @@ -1,7 +1,6 @@ // FIXME: Update this file to be type safe and remove this and next line // @ts-strict-ignore import { Directive, OnInit } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { ActivatedRoute, NavigationExtras, Router } from "@angular/router"; import { firstValueFrom } from "rxjs"; import { first } from "rxjs/operators"; @@ -28,7 +27,6 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { UserId } from "@bitwarden/common/types/guid"; import { ToastService } from "@bitwarden/components"; import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy"; @@ -57,7 +55,6 @@ export class SsoComponent implements OnInit { protected redirectUri: string; protected state: string; protected codeChallenge: string; - protected activeUserId: UserId; constructor( protected ssoLoginService: SsoLoginServiceAbstraction, @@ -77,11 +74,7 @@ export class SsoComponent implements OnInit { protected masterPasswordService: InternalMasterPasswordServiceAbstraction, protected accountService: AccountService, protected toastService: ToastService, - ) { - this.accountService.activeAccount$.pipe(takeUntilDestroyed()).subscribe((account) => { - this.activeUserId = account?.id; - }); - } + ) {} async ngOnInit() { // eslint-disable-next-line rxjs/no-async-subscribe @@ -233,10 +226,8 @@ export class SsoComponent implements OnInit { // - TDE login decryption options component // - Browser SSO on extension open // Note: you cannot set this in state before 2FA b/c there won't be an account in state. - await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier( - orgSsoIdentifier, - this.activeUserId, - ); + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(orgSsoIdentifier, userId); // Users enrolled in admin acct recovery can be forced to set a new password after // having the admin set a temp password for them (affects TDE & standard users) diff --git a/libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts b/libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts index 6afee461c42..3e59e4a29b9 100644 --- a/libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts +++ b/libs/angular/src/auth/components/two-factor-auth/two-factor-auth.component.ts @@ -2,7 +2,6 @@ // @ts-strict-ignore import { CommonModule } from "@angular/common"; import { Component, Inject, OnInit, ViewChild } from "@angular/core"; -import { takeUntilDestroyed } from "@angular/core/rxjs-interop"; import { FormBuilder, ReactiveFormsModule, Validators } from "@angular/forms"; import { ActivatedRoute, NavigationExtras, Router, RouterLink } from "@angular/router"; import { Subject, takeUntil, lastValueFrom, first, firstValueFrom } from "rxjs"; @@ -32,7 +31,6 @@ import { EnvironmentService } from "@bitwarden/common/platform/abstractions/envi import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; -import { UserId } from "@bitwarden/common/types/guid"; import { AsyncActionsModule, ButtonModule, @@ -128,7 +126,6 @@ export class TwoFactorAuthComponent extends CaptchaProtectedComponent implements protected changePasswordRoute = "set-password"; protected forcePasswordResetRoute = "update-temp-password"; protected successRoute = "vault"; - protected activeUserId: UserId; constructor( protected loginStrategyService: LoginStrategyServiceAbstraction, @@ -151,10 +148,6 @@ export class TwoFactorAuthComponent extends CaptchaProtectedComponent implements protected toastService: ToastService, ) { super(environmentService, i18nService, platformUtilsService, toastService); - - this.accountService.activeAccount$.pipe(takeUntilDestroyed()).subscribe((account) => { - this.activeUserId = account?.id; - }); } async ngOnInit() { @@ -269,10 +262,8 @@ export class TwoFactorAuthComponent extends CaptchaProtectedComponent implements // Save off the OrgSsoIdentifier for use in the TDE flows // - TDE login decryption options component // - Browser SSO on extension open - await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier( - this.orgIdentifier, - this.activeUserId, - ); + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(this.orgIdentifier, userId); this.loginEmailService.clearValues(); // note: this flow affects both TDE & standard users diff --git a/libs/angular/src/auth/components/two-factor.component.ts b/libs/angular/src/auth/components/two-factor.component.ts index 49af9d057f7..e43797332ec 100644 --- a/libs/angular/src/auth/components/two-factor.component.ts +++ b/libs/angular/src/auth/components/two-factor.component.ts @@ -35,7 +35,6 @@ import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.servic import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { StateService } from "@bitwarden/common/platform/abstractions/state.service"; -import { UserId } from "@bitwarden/common/types/guid"; import { ToastService } from "@bitwarden/components"; import { CaptchaProtectedComponent } from "./captcha-protected.component"; @@ -74,8 +73,6 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI protected successRoute = "vault"; protected twoFactorTimeoutRoute = "authentication-timeout"; - protected activeUserId: UserId; - get isDuoProvider(): boolean { return ( this.selectedProviderType === TwoFactorProviderType.Duo || @@ -108,10 +105,6 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI this.webAuthnSupported = this.platformUtilsService.supportsWebAuthn(win); - this.accountService.activeAccount$.pipe(takeUntilDestroyed()).subscribe((account) => { - this.activeUserId = account?.id; - }); - // Add subscription to authenticationSessionTimeout$ and navigate to twoFactorTimeoutRoute if expired this.loginStrategyService.authenticationSessionTimeout$ .pipe(takeUntilDestroyed()) @@ -295,10 +288,8 @@ export class TwoFactorComponent extends CaptchaProtectedComponent implements OnI // Save off the OrgSsoIdentifier for use in the TDE flows // - TDE login decryption options component // - Browser SSO on extension open - await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier( - this.orgIdentifier, - this.activeUserId, - ); + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(this.orgIdentifier, userId); this.loginEmailService.clearValues(); // note: this flow affects both TDE & standard users diff --git a/libs/auth/src/angular/sso/sso.component.ts b/libs/auth/src/angular/sso/sso.component.ts index b4373bfe96e..41cbc77e792 100644 --- a/libs/auth/src/angular/sso/sso.component.ts +++ b/libs/auth/src/angular/sso/sso.component.ts @@ -36,7 +36,6 @@ import { LogService } from "@bitwarden/common/platform/abstractions/log.service" import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service"; import { ValidationService } from "@bitwarden/common/platform/abstractions/validation.service"; import { Utils } from "@bitwarden/common/platform/misc/utils"; -import { UserId } from "@bitwarden/common/types/guid"; import { AsyncActionsModule, ButtonModule, @@ -90,7 +89,6 @@ export class SsoComponent implements OnInit { protected state: string | undefined; protected codeChallenge: string | undefined; protected clientId: SsoClientType | undefined; - protected activeUserId: UserId | undefined; formPromise: Promise | undefined; initiateSsoFormPromise: Promise | undefined; @@ -132,8 +130,6 @@ export class SsoComponent implements OnInit { } async ngOnInit() { - this.activeUserId = (await firstValueFrom(this.accountService.activeAccount$))?.id; - const qParams: QueryParams = await firstValueFrom(this.route.queryParams); // This if statement will pass on the second portion of the SSO flow @@ -388,10 +384,10 @@ export class SsoComponent implements OnInit { // - TDE login decryption options component // - Browser SSO on extension open // Note: you cannot set this in state before 2FA b/c there won't be an account in state. - await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier( - orgSsoIdentifier, - this.activeUserId, - ); + + // Grabbing the active user id right before making the state set to ensure it exists. + const userId = (await firstValueFrom(this.accountService.activeAccount$))?.id; + await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(orgSsoIdentifier, userId); // Users enrolled in admin acct recovery can be forced to set a new password after // having the admin set a temp password for them (affects TDE & standard users) diff --git a/libs/common/src/auth/abstractions/sso-login.service.abstraction.ts b/libs/common/src/auth/abstractions/sso-login.service.abstraction.ts index bf64dcafd69..3dbaf429edb 100644 --- a/libs/common/src/auth/abstractions/sso-login.service.abstraction.ts +++ b/libs/common/src/auth/abstractions/sso-login.service.abstraction.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { UserId } from "@bitwarden/common/types/guid"; export abstract class SsoLoginServiceAbstraction { @@ -13,7 +11,7 @@ export abstract class SsoLoginServiceAbstraction { * @see https://datatracker.ietf.org/doc/html/rfc7636 * @returns The code verifier used for SSO. */ - getCodeVerifier: () => Promise; + abstract getCodeVerifier: () => Promise; /** * Sets the code verifier used for SSO. * @@ -23,7 +21,7 @@ export abstract class SsoLoginServiceAbstraction { * and verify it matches the one sent in the request for the `authorization_code`. * @see https://datatracker.ietf.org/doc/html/rfc7636 */ - setCodeVerifier: (codeVerifier: string) => Promise; + abstract setCodeVerifier: (codeVerifier: string) => Promise; /** * Gets the value of the SSO state. * @@ -33,7 +31,7 @@ export abstract class SsoLoginServiceAbstraction { * @see https://datatracker.ietf.org/doc/html/rfc6749#section-4.1 * @returns The SSO state. */ - getSsoState: () => Promise; + abstract getSsoState: () => Promise; /** * Sets the value of the SSO state. * @@ -42,7 +40,7 @@ export abstract class SsoLoginServiceAbstraction { * returns the `state` in the callback and the client verifies that the value returned matches the value sent. * @see https://datatracker.ietf.org/doc/html/rfc6749#section-4.1 */ - setSsoState: (ssoState: string) => Promise; + abstract setSsoState: (ssoState: string) => Promise; /** * Gets the value of the user's organization sso identifier. * @@ -50,20 +48,20 @@ export abstract class SsoLoginServiceAbstraction { * Do not use this value outside of the SSO login flow. * @returns The user's organization identifier. */ - getOrganizationSsoIdentifier: () => Promise; + abstract getOrganizationSsoIdentifier: () => Promise; /** * Sets the value of the user's organization sso identifier. * * This should only be used during the SSO flow to identify the organization that the user is attempting to log in to. * Do not use this value outside of the SSO login flow. */ - setOrganizationSsoIdentifier: (organizationIdentifier: string) => Promise; + abstract setOrganizationSsoIdentifier: (organizationIdentifier: string) => Promise; /** * Gets the user's email. * Note: This should only be used during the SSO flow to identify the user that is attempting to log in. * @returns The user's email. */ - getSsoEmail: () => Promise; + abstract getSsoEmail: () => Promise; /** * Sets the user's email. * Note: This should only be used during the SSO flow to identify the user that is attempting to log in. @@ -71,20 +69,20 @@ export abstract class SsoLoginServiceAbstraction { * @returns A promise that resolves when the email has been set. * */ - setSsoEmail: (email: string) => Promise; + abstract setSsoEmail: (email: string) => Promise; /** * Gets the value of the active user's organization sso identifier. * * This should only be used post successful SSO login once the user is initialized. * @param userId The user id for retrieving the org identifier state. */ - getActiveUserOrganizationSsoIdentifier: (userId: UserId) => Promise; + abstract getActiveUserOrganizationSsoIdentifier: (userId: UserId) => Promise; /** * Sets the value of the active user's organization sso identifier. * * This should only be used post successful SSO login once the user is initialized. */ - setActiveUserOrganizationSsoIdentifier: ( + abstract setActiveUserOrganizationSsoIdentifier: ( organizationIdentifier: string, userId: UserId | undefined, ) => Promise; diff --git a/libs/common/src/auth/services/sso-login.service.spec.ts b/libs/common/src/auth/services/sso-login.service.spec.ts index 9cf49a07834..6764755e6ca 100644 --- a/libs/common/src/auth/services/sso-login.service.spec.ts +++ b/libs/common/src/auth/services/sso-login.service.spec.ts @@ -87,7 +87,7 @@ describe("SSOLoginService ", () => { const orgIdentifier = "test-active-org-identifier"; await sut.setActiveUserOrganizationSsoIdentifier(orgIdentifier, undefined); - expect(mockLogService.warning).toHaveBeenCalledWith( + expect(mockLogService.error).toHaveBeenCalledWith( "Tried to set a user organization sso identifier with an undefined user id.", ); }); diff --git a/libs/common/src/auth/services/sso-login.service.ts b/libs/common/src/auth/services/sso-login.service.ts index c73be3630be..b77e31dc79a 100644 --- a/libs/common/src/auth/services/sso-login.service.ts +++ b/libs/common/src/auth/services/sso-login.service.ts @@ -1,5 +1,3 @@ -// FIXME: Update this file to be type safe and remove this and next line -// @ts-strict-ignore import { firstValueFrom } from "rxjs"; import { LogService } from "@bitwarden/common/platform/abstractions/log.service"; @@ -107,7 +105,7 @@ export class SsoLoginService implements SsoLoginServiceAbstraction { await this.ssoEmailState.update((_) => email); } - getActiveUserOrganizationSsoIdentifier(userId: UserId): Promise { + getActiveUserOrganizationSsoIdentifier(userId: UserId): Promise { return firstValueFrom(this.userOrgSsoIdentifierState(userId).state$); } @@ -116,7 +114,7 @@ export class SsoLoginService implements SsoLoginServiceAbstraction { userId: UserId | undefined, ): Promise { if (userId === undefined) { - this.logService.warning( + this.logService.error( "Tried to set a user organization sso identifier with an undefined user id.", ); return;