1
0
mirror of https://github.com/bitwarden/browser synced 2026-02-08 12:40:26 +00:00

key connector url required to be provided when migrating user

This commit is contained in:
Maciej Zieniuk
2025-04-08 18:20:12 +01:00
parent a10cd49e53
commit db0ee9bb71
7 changed files with 51 additions and 105 deletions

View File

@@ -130,7 +130,10 @@ describe("ConvertToKeyConnectorCommand", () => {
expect(response).not.toBeNull();
expect(response.success).toEqual(true);
expect(keyConnectorService.migrateUser).toHaveBeenCalledWith(userId);
expect(keyConnectorService.migrateUser).toHaveBeenCalledWith(
organization.keyConnectorUrl,
userId,
);
expect(environmentService.setEnvironment).toHaveBeenCalledWith(Region.SelfHosted, {
keyConnector: organization.keyConnectorUrl,
} as Urls);

View File

@@ -64,7 +64,7 @@ export class ConvertToKeyConnectorCommand {
if (answer.convert === "remove") {
try {
await this.keyConnectorService.migrateUser(this.userId);
await this.keyConnectorService.migrateUser(organization.keyConnectorUrl, this.userId);
} catch (e) {
await this.logout();
throw e;

View File

@@ -5,13 +5,13 @@ import { IdentityTokenResponse } from "../../../auth/models/response/identity-to
import { UserId } from "../../../types/guid";
export abstract class KeyConnectorService {
abstract setMasterKeyFromUrl(url: string, userId: UserId): Promise<void>;
abstract setMasterKeyFromUrl(keyConnectorUrl: string, userId: UserId): Promise<void>;
abstract getManagingOrganization(userId: UserId): Promise<Organization>;
abstract getUsesKeyConnector(userId: UserId): Promise<boolean>;
abstract migrateUser(userId: UserId): Promise<void>;
abstract migrateUser(keyConnectorUrl: string, userId: UserId): Promise<void>;
abstract convertNewSsoUserToKeyConnector(
tokenResponse: IdentityTokenResponse,

View File

@@ -45,6 +45,8 @@ describe("KeyConnectorService", () => {
key: "eO9nVlVl3I3sU6O+CyK0kEkpGtl/auT84Hig2WTXmZtDTqYtKpDvUPfjhgMOHf+KQzx++TVS2AOLYq856Caa7w==",
});
const keyConnectorUrl = "https://key-connector-url.com";
beforeEach(() => {
jest.clearAllMocks();
@@ -124,27 +126,9 @@ describe("KeyConnectorService", () => {
it("should return the managing organization with key connector enabled", async () => {
// Arrange
const orgs = [
organizationData(
true,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
),
organizationData(
false,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
),
organizationData(
true,
false,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
),
organizationData(true, true, keyConnectorUrl, OrganizationUserType.User, false),
organizationData(false, true, keyConnectorUrl, OrganizationUserType.User, false),
organizationData(true, false, keyConnectorUrl, OrganizationUserType.User, false),
organizationData(true, true, "https://other-url.com", OrganizationUserType.User, false),
];
organizationService.organizations$.mockReturnValue(of(orgs));
@@ -159,20 +143,8 @@ describe("KeyConnectorService", () => {
it("should return undefined if no managing organization with key connector enabled is found", async () => {
// Arrange
const orgs = [
organizationData(
true,
false,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
),
organizationData(
false,
false,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
),
organizationData(true, false, keyConnectorUrl, OrganizationUserType.User, false),
organizationData(false, false, keyConnectorUrl, OrganizationUserType.User, false),
];
organizationService.organizations$.mockReturnValue(of(orgs));
@@ -186,8 +158,8 @@ describe("KeyConnectorService", () => {
it("should return undefined if user is Owner or Admin", async () => {
// Arrange
const orgs = [
organizationData(true, true, "https://key-connector-url.com", 0, false),
organizationData(true, true, "https://key-connector-url.com", 1, false),
organizationData(true, true, keyConnectorUrl, 0, false),
organizationData(true, true, keyConnectorUrl, 1, false),
];
organizationService.organizations$.mockReturnValue(of(orgs));
@@ -201,20 +173,8 @@ describe("KeyConnectorService", () => {
it("should return undefined if user is a Provider", async () => {
// Arrange
const orgs = [
organizationData(
true,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
true,
),
organizationData(
false,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
true,
),
organizationData(true, true, keyConnectorUrl, OrganizationUserType.User, true),
organizationData(false, true, keyConnectorUrl, OrganizationUserType.User, true),
];
organizationService.organizations$.mockReturnValue(of(orgs));
@@ -229,7 +189,7 @@ describe("KeyConnectorService", () => {
describe("setMasterKeyFromUrl", () => {
it("should set the master key from the provided URL", async () => {
// Arrange
const url = "https://key-connector-url.com";
const url = keyConnectorUrl;
apiService.getMasterKeyFromKeyConnector.mockResolvedValue(mockMasterKeyResponse);
@@ -247,7 +207,7 @@ describe("KeyConnectorService", () => {
it("should handle errors thrown during the process", async () => {
// Arrange
const url = "https://key-connector-url.com";
const url = keyConnectorUrl;
const error = new Error("Failed to get master key");
apiService.getMasterKeyFromKeyConnector.mockRejectedValue(error);
@@ -267,27 +227,18 @@ describe("KeyConnectorService", () => {
describe("migrateUser", () => {
it("should migrate the user to the key connector", async () => {
// Arrange
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
);
const masterKey = getMockMasterKey();
masterPasswordService.masterKeySubject.next(masterKey);
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
jest.spyOn(apiService, "postUserKeyToKeyConnector").mockResolvedValue();
// Act
await keyConnectorService.migrateUser(mockUserId);
await keyConnectorService.migrateUser(keyConnectorUrl, mockUserId);
// Assert
expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
organization.keyConnectorUrl,
keyConnectorUrl,
keyConnectorRequest,
);
expect(apiService.postConvertToKeyConnector).toHaveBeenCalled();
@@ -295,32 +246,21 @@ describe("KeyConnectorService", () => {
it("should handle errors thrown during migration", async () => {
// Arrange
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
OrganizationUserType.User,
false,
);
const masterKey = getMockMasterKey();
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
const error = new Error("Failed to post user key to key connector");
organizationService.organizations$.mockReturnValue(of([organization]));
masterPasswordService.masterKeySubject.next(masterKey);
jest.spyOn(keyConnectorService, "getManagingOrganization").mockResolvedValue(organization);
const error = new Error("Failed to post user key to key connector");
jest.spyOn(apiService, "postUserKeyToKeyConnector").mockRejectedValue(error);
jest.spyOn(logService, "error");
try {
// Act
await keyConnectorService.migrateUser(mockUserId);
await keyConnectorService.migrateUser(keyConnectorUrl, mockUserId);
} catch {
// Assert
expect(logService.error).toHaveBeenCalledWith(error);
expect(keyConnectorService.getManagingOrganization).toHaveBeenCalled();
expect(apiService.postUserKeyToKeyConnector).toHaveBeenCalledWith(
organization.keyConnectorUrl,
keyConnectorUrl,
keyConnectorRequest,
);
}
@@ -332,7 +272,7 @@ describe("KeyConnectorService", () => {
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
keyConnectorUrl,
OrganizationUserType.User,
false,
);
@@ -360,7 +300,7 @@ describe("KeyConnectorService", () => {
const organization = organizationData(
true,
false,
"https://key-connector-url.com",
keyConnectorUrl,
OrganizationUserType.User,
false,
);
@@ -375,7 +315,7 @@ describe("KeyConnectorService", () => {
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
keyConnectorUrl,
OrganizationUserType.Admin,
false,
);
@@ -390,7 +330,7 @@ describe("KeyConnectorService", () => {
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
keyConnectorUrl,
OrganizationUserType.Owner,
false,
);
@@ -405,7 +345,7 @@ describe("KeyConnectorService", () => {
const organization = organizationData(
true,
true,
"https://key-connector-url.com",
keyConnectorUrl,
OrganizationUserType.User,
true,
);

View File

@@ -90,22 +90,12 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
);
}
async migrateUser(userId: UserId) {
const organization = await this.getManagingOrganization(userId);
if (organization == null) {
throw new Error(
"[Key Connector service] No key connector enabled organization found, aborting user migration to key connector.",
);
}
async migrateUser(keyConnectorUrl: string, userId: UserId) {
const masterKey = await firstValueFrom(this.masterPasswordService.masterKey$(userId));
const keyConnectorRequest = new KeyConnectorUserKeyRequest(masterKey.encKeyB64);
try {
await this.apiService.postUserKeyToKeyConnector(
organization.keyConnectorUrl,
keyConnectorRequest,
);
await this.apiService.postUserKeyToKeyConnector(keyConnectorUrl, keyConnectorRequest);
} catch (e) {
this.handleKeyConnectorError(e);
}
@@ -116,9 +106,9 @@ export class KeyConnectorService implements KeyConnectorServiceAbstraction {
}
// TODO: UserKey should be renamed to MasterKey and typed accordingly
async setMasterKeyFromUrl(url: string, userId: UserId) {
async setMasterKeyFromUrl(keyConnectorUrl: string, userId: UserId) {
try {
const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(url);
const masterKeyResponse = await this.apiService.getMasterKeyFromKeyConnector(keyConnectorUrl);
const keyArr = Utils.fromB64ToArray(masterKeyResponse.key);
const masterKey = new SymmetricCryptoKey(keyArr) as MasterKey;
await this.masterPasswordService.setMasterKey(masterKey, userId);

View File

@@ -22,6 +22,7 @@ describe("RemovePasswordComponent", () => {
const organization = {
id: "test-organization-id",
name: "test-organization-name",
keyConnectorUrl: "https://key-connector-url.com",
} as Organization;
const accountService = mockAccountServiceWith(userId);
@@ -121,7 +122,10 @@ describe("RemovePasswordComponent", () => {
await component.convert();
expect(component.continuing).toBe(true);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(userId);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(
organization.keyConnectorUrl,
userId,
);
expect(mockToastService.showToast).toHaveBeenCalledWith({
variant: "success",
message: "removed master password",
@@ -137,7 +141,10 @@ describe("RemovePasswordComponent", () => {
await component.convert();
expect(component.continuing).toBe(false);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(userId);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(
organization.keyConnectorUrl,
userId,
);
expect(mockToastService.showToast).toHaveBeenCalledWith({
variant: "error",
title: "error occurred",
@@ -161,7 +168,10 @@ describe("RemovePasswordComponent", () => {
await component.convert();
expect(component.continuing).toBe(false);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(userId);
expect(mockKeyConnectorService.migrateUser).toHaveBeenCalledWith(
organization.keyConnectorUrl,
userId,
);
expect(mockToastService.showToast).toHaveBeenCalledWith({
variant: "error",
title: "error occurred",

View File

@@ -65,7 +65,10 @@ export class RemovePasswordComponent implements OnInit {
this.continuing = true;
try {
await this.keyConnectorService.migrateUser(this.activeUserId);
await this.keyConnectorService.migrateUser(
this.organization.keyConnectorUrl,
this.activeUserId,
);
this.toastService.showToast({
variant: "success",