Upstream/bitwarden-browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser.git synced 2025-03-12 13:39:14 +01:00
Code Issues Projects Releases Wiki Activity

fix(LoginComp + LoginStrategies): [Auth/PM-18654] Refreshed UI - Desktop TDE JIT provisioned user creation errors with missing org SSO id (#13619)

Browse Source 
* PM-18654 - State Service & Login Strategy Refactor - move env seeding into login strategy so that new accounts always load w/ the correct environment

* PM-18654 - SSO Comp - just use user id from auth result

* PM-18654 - Config Service - (1) don't allow cascading calls to the renewConfig by using a private promise (2) Replace shareReplay with share configured with manual timer

* PM-18654 - LoginComponents - detail issue and possible fix

* PM-18654 - DesktopLoginV1Comp - use correct destroy hook

* PM-18654 - LoginComp - clean up no longer correct comment

* PM-18654 - New Device Verification Component - Remove unused PasswordLoginStrategy dependency

* PM-18654 - Browser Home Component - fix qParam logic

* PM-18654 - DefaultConfigService - revert changes as they aren't necessary to fix the bug.

* PM-18654 - DefaultConfigService - remove commented code

* PM-18654 - LoginStrategy - add comment

* PM-18654 - Fix login strat tests
This commit is contained in:
Jared Snider 2025-03-03 12:09:35 -05:00 committed by GitHub
parent b5b791f414
commit 92f027af5e
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 44 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
apps/browser/src/auth/popup/home.component.ts
View File

@ -81,8 +81,10 @@ export class HomeComponent implements OnInit, OnDestroy {
tap(async (flag) => {
// If the flag is turned ON, we must force a reload to ensure the correct UI is shown
if (flag) {
const qParams = await firstValueFrom(this.route.queryParams);
const uniqueQueryParams = {
...this.route.queryParams,
...qParams,
// adding a unique timestamp to the query params to force a reload
t: new Date().getTime().toString(),
};

9
apps/desktop/src/auth/login/login-v1.component.ts
View File

@ -3,7 +3,7 @@
import { Component, NgZone, OnDestroy, OnInit, ViewChild, ViewContainerRef } from "@angular/core";
import { FormBuilder } from "@angular/forms";
import { ActivatedRoute, Router } from "@angular/router";
import { Subject, takeUntil, tap } from "rxjs";
import { Subject, firstValueFrom, takeUntil, tap } from "rxjs";
import { LoginComponentV1 as BaseLoginComponent } from "@bitwarden/angular/auth/components/login-v1.component";
import { FormValidationErrorsService } from "@bitwarden/angular/platform/abstractions/form-validation-errors.service";
@ -143,10 +143,11 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe
.getFeatureFlag$(FeatureFlag.UnauthenticatedExtensionUIRefresh)
.pipe(
tap(async (flag) => {
// If the flag is turned ON, we must force a reload to ensure the correct UI is shown
if (flag) {
const qParams = await firstValueFrom(this.route.queryParams);
const uniqueQueryParams = {
...this.route.queryParams,
...qParams,
// adding a unique timestamp to the query params to force a reload
t: new Date().getTime().toString(),
};
@ -156,7 +157,7 @@ export class LoginComponentV1 extends BaseLoginComponent implements OnInit, OnDe
});
}
}),
takeUntil(this.destroy$),
takeUntil(this.componentDestroyed$),
)
.subscribe();
}

33
libs/angular/src/services/jslib-services.module.ts
View File

@ -45,8 +45,6 @@ import {
DefaultAuthRequestApiService,
DefaultLoginSuccessHandlerService,
LoginSuccessHandlerService,
PasswordLoginStrategy,
PasswordLoginStrategyData,
LoginApprovalComponentServiceAbstraction,
} from "@bitwarden/auth/common";
import { ApiService as ApiServiceAbstraction } from "@bitwarden/common/abstractions/api.service";
@ -1460,37 +1458,6 @@ const safeProviders: SafeProvider[] = [
useClass: DefaultLoginSuccessHandlerService,
deps: [SyncService, UserAsymmetricKeysRegenerationService],
}),
safeProvider({
provide: PasswordLoginStrategy,
useClass: PasswordLoginStrategy,
deps: [
PasswordLoginStrategyData,
PasswordStrengthServiceAbstraction,
PolicyServiceAbstraction,
LoginStrategyServiceAbstraction,
AccountServiceAbstraction,
InternalMasterPasswordServiceAbstraction,
KeyService,
EncryptService,
ApiServiceAbstraction,
TokenServiceAbstraction,
AppIdServiceAbstraction,
PlatformUtilsServiceAbstraction,
MessagingServiceAbstraction,
LogService,
StateServiceAbstraction,
TwoFactorServiceAbstraction,
InternalUserDecryptionOptionsServiceAbstraction,
BillingAccountProfileStateService,
VaultTimeoutSettingsService,
KdfConfigService,
],
}),
safeProvider({
provide: PasswordLoginStrategyData,
useClass: PasswordLoginStrategyData,
deps: [],
}),
safeProvider({
provide: TaskService,
useClass: DefaultTaskService,

3
libs/auth/src/angular/login/login.component.ts
View File

@ -161,8 +161,9 @@ export class LoginComponent implements OnInit, OnDestroy {
tap(async (flag) => {
// If the flag is turned OFF, we must force a reload to ensure the correct UI is shown
if (!flag) {
const qParams = await firstValueFrom(this.activatedRoute.queryParams);
const uniqueQueryParams = {
...this.activatedRoute.queryParams,
...qParams,
// adding a unique timestamp to the query params to force a reload
t: new Date().getTime().toString(), // Adding a unique timestamp as a query parameter
};

2
libs/auth/src/angular/new-device-verification/new-device-verification.component.ts
View File

@ -20,7 +20,6 @@ import {
import { LoginEmailServiceAbstraction } from "../../common/abstractions/login-email.service";
import { LoginStrategyServiceAbstraction } from "../../common/abstractions/login-strategy.service";
import { PasswordLoginStrategy } from "../../common/login-strategies/password-login.strategy";
/**
* Component for verifying a new device via a one-time password (OTP).
@ -58,7 +57,6 @@ export class NewDeviceVerificationComponent implements OnInit, OnDestroy {
constructor(
private router: Router,
private formBuilder: FormBuilder,
private passwordLoginStrategy: PasswordLoginStrategy,
private apiService: ApiService,
private loginStrategyService: LoginStrategyServiceAbstraction,
private logService: LogService,

8
libs/auth/src/angular/sso/sso.component.ts
View File

@ -427,7 +427,6 @@ export class SsoComponent implements OnInit {
);
this.formPromise = this.loginStrategyService.logIn(credentials);
const authResult = await this.formPromise;
if (authResult.requiresTwoFactor) {
return await this.handleTwoFactorRequired(orgSsoIdentifier);
}
@ -441,9 +440,10 @@ export class SsoComponent implements OnInit {
// - Browser SSO on extension open
// Note: you cannot set this in state before 2FA b/c there won't be an account in state.
// 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);
await this.ssoLoginService.setActiveUserOrganizationSsoIdentifier(
orgSsoIdentifier,
authResult.userId,
);
// must come after 2fa check since user decryption options aren't available if 2fa is required
const userDecryptionOpts = await firstValueFrom(

4
libs/auth/src/common/login-strategies/auth-request-login.strategy.spec.ts
View File

@ -14,6 +14,7 @@ import {
VaultTimeoutSettingsService,
} from "@bitwarden/common/key-management/vault-timeout";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -53,6 +54,7 @@ describe("AuthRequestLoginStrategy", () => {
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let kdfConfigService: MockProxy<KdfConfigService>;
let environmentService: MockProxy<EnvironmentService>;
const mockUserId = Utils.newGuid() as UserId;
let accountService: FakeAccountService;
@ -88,6 +90,7 @@ describe("AuthRequestLoginStrategy", () => {
billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
kdfConfigService = mock<KdfConfigService>();
environmentService = mock<EnvironmentService>();
accountService = mockAccountServiceWith(mockUserId);
masterPasswordService = new FakeMasterPasswordService();
@ -117,6 +120,7 @@ describe("AuthRequestLoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
tokenResponse = identityTokenResponseFactory();

6
libs/auth/src/common/login-strategies/login.strategy.spec.ts
View File

@ -25,6 +25,7 @@ import {
VaultTimeoutSettingsService,
} from "@bitwarden/common/key-management/vault-timeout";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -123,6 +124,7 @@ describe("LoginStrategy", () => {
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let kdfConfigService: MockProxy<KdfConfigService>;
let environmentService: MockProxy<EnvironmentService>;
let passwordLoginStrategy: PasswordLoginStrategy;
let credentials: PasswordLoginCredentials;
@ -147,6 +149,7 @@ describe("LoginStrategy", () => {
policyService = mock<PolicyService>();
passwordStrengthService = mock<PasswordStrengthService>();
billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
environmentService = mock<EnvironmentService>();
vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
@ -175,6 +178,7 @@ describe("LoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
credentials = new PasswordLoginCredentials(email, masterPassword);
});
@ -496,6 +500,7 @@ describe("LoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
apiService.postIdentityToken.mockResolvedValue(identityTokenResponseFactory());
@ -559,6 +564,7 @@ describe("LoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
const result = await passwordLoginStrategy.logIn(credentials);

6
libs/auth/src/common/login-strategies/login.strategy.ts
View File

@ -27,6 +27,7 @@ import {
} from "@bitwarden/common/key-management/vault-timeout";
import { KeysRequest } from "@bitwarden/common/models/request/keys.request";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -93,6 +94,7 @@ export abstract class LoginStrategy {
protected billingAccountProfileStateService: BillingAccountProfileStateService,
protected vaultTimeoutSettingsService: VaultTimeoutSettingsService,
protected KdfConfigService: KdfConfigService,
protected environmentService: EnvironmentService,
) {}
abstract exportCache(): CacheData;
@ -196,6 +198,10 @@ export abstract class LoginStrategy {
emailVerified: accountInformation.email_verified ?? false,
});
// User env must be seeded from currently set env before switching to the account
// to avoid any incorrect emissions of the global default env.
await this.environmentService.seedUserEnvironment(userId);
await this.accountService.switchAccount(userId);
await this.stateService.addAccount(

4
libs/auth/src/common/login-strategies/password-login.strategy.spec.ts
View File

@ -18,6 +18,7 @@ import {
VaultTimeoutSettingsService,
} from "@bitwarden/common/key-management/vault-timeout";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -80,6 +81,7 @@ describe("PasswordLoginStrategy", () => {
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let kdfConfigService: MockProxy<KdfConfigService>;
let environmentService: MockProxy<EnvironmentService>;
let passwordLoginStrategy: PasswordLoginStrategy;
let credentials: PasswordLoginCredentials;
@ -106,6 +108,7 @@ describe("PasswordLoginStrategy", () => {
billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
kdfConfigService = mock<KdfConfigService>();
environmentService = mock<EnvironmentService>();
appIdService.getAppId.mockResolvedValue(deviceId);
tokenService.decodeAccessToken.mockResolvedValue({
@ -144,6 +147,7 @@ describe("PasswordLoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
credentials = new PasswordLoginCredentials(email, masterPassword);
tokenResponse = identityTokenResponseFactory(masterPasswordPolicy);

4
libs/auth/src/common/login-strategies/sso-login.strategy.spec.ts
View File

@ -19,6 +19,7 @@ import {
} from "@bitwarden/common/key-management/vault-timeout";
import { ErrorResponse } from "@bitwarden/common/models/response/error.response";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
@ -63,6 +64,7 @@ describe("SsoLoginStrategy", () => {
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let kdfConfigService: MockProxy<KdfConfigService>;
let environmentService: MockProxy<EnvironmentService>;
let ssoLoginStrategy: SsoLoginStrategy;
let credentials: SsoLoginCredentials;
@ -98,6 +100,7 @@ describe("SsoLoginStrategy", () => {
billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
kdfConfigService = mock<KdfConfigService>();
environmentService = mock<EnvironmentService>();
tokenService.getTwoFactorToken.mockResolvedValue(null);
appIdService.getAppId.mockResolvedValue(deviceId);
@ -142,6 +145,7 @@ describe("SsoLoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
credentials = new SsoLoginCredentials(ssoCode, ssoCodeVerifier, ssoRedirectUrl, ssoOrgId);
});

2
libs/auth/src/common/login-strategies/user-api-login.strategy.spec.ts
View File

@ -97,7 +97,6 @@ describe("UserApiLoginStrategy", () => {
apiLogInStrategy = new UserApiLoginStrategy(
cache,
environmentService,
keyConnectorService,
accountService,
masterPasswordService,
@ -115,6 +114,7 @@ describe("UserApiLoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
credentials = new UserApiLoginCredentials(apiClientId, apiClientSecret);

2
libs/auth/src/common/login-strategies/user-api-login.strategy.ts
View File

@ -7,7 +7,6 @@ import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-con
import { UserApiTokenRequest } from "@bitwarden/common/auth/models/request/identity-token/user-api-token.request";
import { IdentityTokenResponse } from "@bitwarden/common/auth/models/response/identity-token.response";
import { VaultTimeoutAction } from "@bitwarden/common/key-management/vault-timeout";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { UserId } from "@bitwarden/common/types/guid";
import { UserApiLoginCredentials } from "../models/domain/login-credentials";
@ -31,7 +30,6 @@ export class UserApiLoginStrategy extends LoginStrategy {
constructor(
data: UserApiLoginStrategyData,
private environmentService: EnvironmentService,
private keyConnectorService: KeyConnectorService,
...sharedDeps: ConstructorParameters<typeof LoginStrategy>
) {

4
libs/auth/src/common/login-strategies/webauthn-login.strategy.spec.ts
View File

@ -16,6 +16,7 @@ import {
VaultTimeoutSettingsService,
} from "@bitwarden/common/key-management/vault-timeout";
import { AppIdService } from "@bitwarden/common/platform/abstractions/app-id.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { MessagingService } from "@bitwarden/common/platform/abstractions/messaging.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
@ -52,6 +53,7 @@ describe("WebAuthnLoginStrategy", () => {
let billingAccountProfileStateService: MockProxy<BillingAccountProfileStateService>;
let vaultTimeoutSettingsService: MockProxy<VaultTimeoutSettingsService>;
let kdfConfigService: MockProxy<KdfConfigService>;
let environmentService: MockProxy<EnvironmentService>;
let webAuthnLoginStrategy!: WebAuthnLoginStrategy;
@ -95,6 +97,7 @@ describe("WebAuthnLoginStrategy", () => {
billingAccountProfileStateService = mock<BillingAccountProfileStateService>();
vaultTimeoutSettingsService = mock<VaultTimeoutSettingsService>();
kdfConfigService = mock<KdfConfigService>();
environmentService = mock<EnvironmentService>();
tokenService.getTwoFactorToken.mockResolvedValue(null);
appIdService.getAppId.mockResolvedValue(deviceId);
@ -120,6 +123,7 @@ describe("WebAuthnLoginStrategy", () => {
billingAccountProfileStateService,
vaultTimeoutSettingsService,
kdfConfigService,
environmentService,
);
// Create credentials

2
libs/auth/src/common/services/login-strategies/login-strategy.service.ts
View File

@ -402,6 +402,7 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
this.billingAccountProfileStateService,
this.vaultTimeoutSettingsService,
this.kdfConfigService,
this.environmentService,
];
return source.pipe(
@ -430,7 +431,6 @@ export class LoginStrategyService implements LoginStrategyServiceAbstraction {
case AuthenticationType.UserApiKey:
return new UserApiLoginStrategy(
data?.userApiKey ?? new UserApiLoginStrategyData(),
this.environmentService,
this.keyConnectorService,
...sharedDeps,
);

1
libs/common/src/platform/services/state.service.ts
View File

@ -134,7 +134,6 @@ export class StateService<
}
async addAccount(account: TAccount) {
await this.environmentService.seedUserEnvironment(account.profile.userId as UserId);
await this.updateState(async (state) => {
state.accounts[account.profile.userId] = account;
return state;