Skip to content

Commit

Permalink
Pm-10953/add-user-context-to-sync-replaces (#10627)
Browse files Browse the repository at this point in the history
* Require userId for setting masterKeyEncryptedUserKey

* Replace folders for specified user

* Require userId for collection replace

* Cipher Replace requires userId

* Require UserId to update equivalent domains

* Require userId for policy replace

* sync state updates between fake state for better testing

* Revert to public observable tests

Since they now sync, we can test single-user updates impacting active user observables

* Do not init fake states through sync

Do not sync initial null values, that might wipe out already existing data.

* Require userId for Send replace

* Include userId for organization replace

* Require userId for billing sync data

* Require user Id for key connector sync data

* Allow decode of token by userId

* Require userId for synced key connector updates

* Add userId to policy setting during organization invite accept

* Fix cli

* Handle null userId

---------

Co-authored-by: bnagawiecki <[email protected]>
  • Loading branch information
MGibson1 and bnagawiecki committed Aug 27, 2024
1 parent 866a624 commit 9459cda
Show file tree
Hide file tree
Showing 46 changed files with 665 additions and 483 deletions.
6 changes: 3 additions & 3 deletions apps/cli/src/auth/commands/login.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -342,7 +342,7 @@ export class LoginCommand {
}
}

return await this.handleSuccessResponse();
return await this.handleSuccessResponse(response);
} catch (e) {
return Response.error(e);
}
Expand All @@ -353,8 +353,8 @@ export class LoginCommand {
process.env.BW_SESSION = Utils.fromBufferToB64(key);
}

private async handleSuccessResponse(): Promise<Response> {
const usesKeyConnector = await this.keyConnectorService.getUsesKeyConnector();
private async handleSuccessResponse(response: AuthResult): Promise<Response> {
const usesKeyConnector = await this.keyConnectorService.getUsesKeyConnector(response.userId);

if (
(this.options.sso != null || this.options.apikey != null) &&
Expand Down
1 change: 1 addition & 0 deletions apps/cli/src/auth/commands/unlock.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ export class UnlockCommand {

if (await this.keyConnectorService.getConvertAccountRequired()) {
const convertToKeyConnectorCommand = new ConvertToKeyConnectorCommand(
userId,
this.keyConnectorService,
this.environmentService,
this.syncService,
Expand Down
18 changes: 14 additions & 4 deletions apps/cli/src/base-program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,20 +116,30 @@ export abstract class BaseProgram {
}
}

/**
* Exist if no user is authenticated
* @returns the userId of the active account
*/
protected async exitIfNotAuthed() {
const authed = await this.serviceContainer.stateService.getIsAuthenticated();
const fail = () => this.processResponse(Response.error("You are not logged in."), true);
const userId = (await firstValueFrom(this.serviceContainer.accountService.activeAccount$))?.id;
if (!userId) {
fail();
}
const authed = await this.serviceContainer.stateService.getIsAuthenticated({ userId });
if (!authed) {
this.processResponse(Response.error("You are not logged in."), true);
fail();
}
return userId;
}

protected async exitIfLocked() {
await this.exitIfNotAuthed();
const userId = await this.exitIfNotAuthed();
if (await this.serviceContainer.cryptoService.hasUserKey()) {
return;
} else if (process.env.BW_NOINTERACTION !== "true") {
// must unlock
if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector()) {
if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector(userId)) {
const response = Response.error(
"Your vault is locked. You must unlock your vault using your session key.\n" +
"If you do not have your session key, you can get a new one by logging out and logging in again.",
Expand Down
4 changes: 3 additions & 1 deletion apps/cli/src/commands/convert-to-key-connector.command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,15 @@ import {
EnvironmentService,
Region,
} from "@bitwarden/common/platform/abstractions/environment.service";
import { UserId } from "@bitwarden/common/types/guid";
import { SyncService } from "@bitwarden/common/vault/abstractions/sync/sync.service.abstraction";

import { Response } from "../models/response";
import { MessageResponse } from "../models/response/message.response";

export class ConvertToKeyConnectorCommand {
constructor(
private readonly userId: UserId,
private keyConnectorService: KeyConnectorService,
private environmentService: EnvironmentService,
private syncService: SyncService,
Expand Down Expand Up @@ -68,7 +70,7 @@ export class ConvertToKeyConnectorCommand {
}

await this.keyConnectorService.removeConvertAccountRequired();
await this.keyConnectorService.setUsesKeyConnector(true);
await this.keyConnectorService.setUsesKeyConnector(true, this.userId);

// Update environment URL - required for api key login
const env = await firstValueFrom(this.environmentService.environment$);
Expand Down
4 changes: 2 additions & 2 deletions apps/cli/src/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,9 +206,9 @@ export class Program extends BaseProgram {
writeLn("", true);
})
.action(async (cmd) => {
await this.exitIfNotAuthed();
const userId = await this.exitIfNotAuthed();

if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector()) {
if (await this.serviceContainer.keyConnectorService.getUsesKeyConnector(userId)) {
const logoutCommand = new LogoutCommand(
this.serviceContainer.authService,
this.serviceContainer.i18nService,
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/app/auth/login/login.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ 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 { PasswordStrengthServiceAbstraction } from "@bitwarden/common/tools/password-strength";
import { UserId } from "@bitwarden/common/types/guid";
import { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";

import { flagEnabled } from "../../../utils/flags";
Expand Down Expand Up @@ -129,7 +130,7 @@ export class LoginComponent extends BaseLoginComponent implements OnInit {
}
}

async goAfterLogIn() {
async goAfterLogIn(userId: UserId) {
const masterPassword = this.formGroup.value.masterPassword;

// Check master password against policy
Expand All @@ -150,7 +151,7 @@ export class LoginComponent extends BaseLoginComponent implements OnInit {
) {
const policiesData: { [id: string]: PolicyData } = {};
this.policies.map((p) => (policiesData[p.id] = PolicyData.fromPolicy(p)));
await this.policyService.replace(policiesData);
await this.policyService.replace(policiesData, userId);
await this.router.navigate(["update-password"]);
return;
}
Expand Down
5 changes: 3 additions & 2 deletions libs/angular/src/auth/components/login.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ 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 { PasswordGenerationServiceAbstraction } from "@bitwarden/generator-legacy";

import {
Expand All @@ -39,7 +40,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit,
showPassword = false;
formPromise: Promise<AuthResult>;
onSuccessfulLogin: () => Promise<any>;
onSuccessfulLoginNavigate: () => Promise<any>;
onSuccessfulLoginNavigate: (userId: UserId) => Promise<any>;
onSuccessfulLoginTwoFactorNavigate: () => Promise<any>;
onSuccessfulLoginForceResetNavigate: () => Promise<any>;
showLoginWithDevice: boolean;
Expand Down Expand Up @@ -185,7 +186,7 @@ export class LoginComponent extends CaptchaProtectedComponent implements OnInit,
if (this.onSuccessfulLoginNavigate != null) {
// 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.onSuccessfulLoginNavigate();
this.onSuccessfulLoginNavigate(response.userId);
} else {
this.loginEmailService.clearValues();
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,10 @@ describe("AuthRequestLoginStrategy", () => {
decMasterKeyHash,
mockUserId,
);
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
mockUserId,
);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(userKey, mockUserId);
expect(deviceTrustService.trustDeviceIfRequired).toHaveBeenCalled();
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId);
Expand All @@ -183,7 +186,10 @@ describe("AuthRequestLoginStrategy", () => {
expect(masterPasswordService.mock.setMasterKeyHash).not.toHaveBeenCalled();

// setMasterKeyEncryptedUserKey, setUserKey, and setPrivateKey should still be called
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
mockUserId,
);
expect(cryptoService.setUserKey).toHaveBeenCalledWith(decUserKey, mockUserId);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, mockUserId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ export class AuthRequestLoginStrategy extends LoginStrategy {
const authRequestCredentials = this.cache.value.authRequestCredentials;
// User now may or may not have a master password
// but set the master key encrypted user key if it exists regardless
await this.cryptoService.setMasterKeyEncryptedUserKey(response.key);
await this.cryptoService.setMasterKeyEncryptedUserKey(response.key, userId);

if (authRequestCredentials.decryptedUserKey) {
await this.cryptoService.setUserKey(authRequestCredentials.decryptedUserKey, userId);
Expand Down
6 changes: 5 additions & 1 deletion libs/auth/src/common/login-strategies/login.strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,11 @@ export abstract class LoginStrategy {
),
);

await this.billingAccountProfileStateService.setHasPremium(accountInformation.premium, false);
await this.billingAccountProfileStateService.setHasPremium(
accountInformation.premium,
false,
userId,
);
return userId;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,10 @@ describe("UserApiLoginStrategy", () => {

await apiLogInStrategy.logIn(credentials);

expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(tokenResponse.key);
expect(cryptoService.setMasterKeyEncryptedUserKey).toHaveBeenCalledWith(
tokenResponse.key,
userId,
);
expect(cryptoService.setPrivateKey).toHaveBeenCalledWith(tokenResponse.privateKey, userId);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ export class UserApiLoginStrategy extends LoginStrategy {
response: IdentityTokenResponse,
userId: UserId,
): Promise<void> {
await this.cryptoService.setMasterKeyEncryptedUserKey(response.key);
await this.cryptoService.setMasterKeyEncryptedUserKey(response.key, userId);

if (response.apiUseKeyConnector) {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
Expand Down
Loading

0 comments on commit 9459cda

Please sign in to comment.