mirror of
https://github.com/bitwarden/browser
synced 2025-12-14 15:23:33 +00:00
[PM-19838] Untrust devices that cannot be rotated (#14165)
* Untrust devices that cannot be rotated * Add tests and only send request on more than 0 failed devices * Address feedback
This commit is contained in:
@@ -38,4 +38,10 @@ export abstract class DevicesApiServiceAbstraction {
|
|||||||
* @param deviceId - The device ID
|
* @param deviceId - The device ID
|
||||||
*/
|
*/
|
||||||
deactivateDevice: (deviceId: string) => Promise<void>;
|
deactivateDevice: (deviceId: string) => Promise<void>;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Removes trust from a list of devices
|
||||||
|
* @param deviceIds - The device IDs to be untrusted
|
||||||
|
*/
|
||||||
|
untrustDevices: (deviceIds: string[]) => Promise<void>;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -0,0 +1,3 @@
|
|||||||
|
export class UntrustDevicesRequestModel {
|
||||||
|
constructor(public devices: string[]) {}
|
||||||
|
}
|
||||||
@@ -89,6 +89,24 @@ describe("DevicesApiServiceImplementation", () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe("untrustDevices", () => {
|
||||||
|
it("calls api with correct parameters", async () => {
|
||||||
|
const deviceIds = ["device1", "device2"];
|
||||||
|
apiService.send.mockResolvedValue(true);
|
||||||
|
|
||||||
|
await devicesApiService.untrustDevices(deviceIds);
|
||||||
|
expect(apiService.send).toHaveBeenCalledWith(
|
||||||
|
"POST",
|
||||||
|
"/devices/untrust",
|
||||||
|
{
|
||||||
|
devices: deviceIds,
|
||||||
|
},
|
||||||
|
true,
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("error handling", () => {
|
describe("error handling", () => {
|
||||||
it("propagates api errors", async () => {
|
it("propagates api errors", async () => {
|
||||||
const error = new Error("API Error");
|
const error = new Error("API Error");
|
||||||
|
|||||||
@@ -5,6 +5,7 @@ import { ListResponse } from "../../models/response/list.response";
|
|||||||
import { Utils } from "../../platform/misc/utils";
|
import { Utils } from "../../platform/misc/utils";
|
||||||
import { DeviceResponse } from "../abstractions/devices/responses/device.response";
|
import { DeviceResponse } from "../abstractions/devices/responses/device.response";
|
||||||
import { DevicesApiServiceAbstraction } from "../abstractions/devices-api.service.abstraction";
|
import { DevicesApiServiceAbstraction } from "../abstractions/devices-api.service.abstraction";
|
||||||
|
import { UntrustDevicesRequestModel } from "../models/request/untrust-devices.request";
|
||||||
import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request";
|
import { UpdateDevicesTrustRequest } from "../models/request/update-devices-trust.request";
|
||||||
import { ProtectedDeviceResponse } from "../models/response/protected-device.response";
|
import { ProtectedDeviceResponse } from "../models/response/protected-device.response";
|
||||||
|
|
||||||
@@ -117,4 +118,14 @@ export class DevicesApiServiceImplementation implements DevicesApiServiceAbstrac
|
|||||||
async deactivateDevice(deviceId: string): Promise<void> {
|
async deactivateDevice(deviceId: string): Promise<void> {
|
||||||
await this.apiService.send("POST", `/devices/${deviceId}/deactivate`, null, true, false);
|
await this.apiService.send("POST", `/devices/${deviceId}/deactivate`, null, true, false);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async untrustDevices(deviceIds: string[]): Promise<void> {
|
||||||
|
await this.apiService.send(
|
||||||
|
"POST",
|
||||||
|
"/devices/untrust",
|
||||||
|
new UntrustDevicesRequestModel(deviceIds),
|
||||||
|
true,
|
||||||
|
false,
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -204,7 +204,8 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const devices = await this.devicesApiService.getDevices();
|
const devices = await this.devicesApiService.getDevices();
|
||||||
return await Promise.all(
|
const devicesToUntrust: string[] = [];
|
||||||
|
const rotatedData = await Promise.all(
|
||||||
devices.data
|
devices.data
|
||||||
.filter((device) => device.isTrusted)
|
.filter((device) => device.isTrusted)
|
||||||
.map(async (device) => {
|
.map(async (device) => {
|
||||||
@@ -213,6 +214,13 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
|||||||
deviceWithKeys.encryptedPublicKey,
|
deviceWithKeys.encryptedPublicKey,
|
||||||
oldUserKey,
|
oldUserKey,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if (!publicKey) {
|
||||||
|
// Device was trusted but encryption is broken. This should be untrusted
|
||||||
|
devicesToUntrust.push(device.id);
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey);
|
const newEncryptedPublicKey = await this.encryptService.encrypt(publicKey, newUserKey);
|
||||||
const newEncryptedUserKey = await this.encryptService.rsaEncrypt(
|
const newEncryptedUserKey = await this.encryptService.rsaEncrypt(
|
||||||
newUserKey.key,
|
newUserKey.key,
|
||||||
@@ -229,8 +237,14 @@ export class DeviceTrustService implements DeviceTrustServiceAbstraction {
|
|||||||
request.encryptedUserKey = newRotateableKeySet.encryptedUserKey.encryptedString;
|
request.encryptedUserKey = newRotateableKeySet.encryptedUserKey.encryptedString;
|
||||||
request.deviceId = device.id;
|
request.deviceId = device.id;
|
||||||
return request;
|
return request;
|
||||||
}),
|
})
|
||||||
|
.filter((otherDeviceKeysUpdateRequest) => otherDeviceKeysUpdateRequest != null),
|
||||||
);
|
);
|
||||||
|
if (rotatedData.length > 0) {
|
||||||
|
this.logService.info("[Device trust rotation] Distrusting devices that failed to decrypt.");
|
||||||
|
await this.devicesApiService.untrustDevices(devicesToUntrust);
|
||||||
|
}
|
||||||
|
return rotatedData;
|
||||||
}
|
}
|
||||||
|
|
||||||
async rotateDevicesTrust(
|
async rotateDevicesTrust(
|
||||||
|
|||||||
@@ -679,6 +679,52 @@ describe("deviceTrustService", () => {
|
|||||||
).rejects.toThrow("New user key is required. Cannot get rotated data.");
|
).rejects.toThrow("New user key is required. Cannot get rotated data.");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("untrusts devices that failed to decrypt", async () => {
|
||||||
|
const deviceResponse = {
|
||||||
|
id: "id",
|
||||||
|
userId: "",
|
||||||
|
name: "",
|
||||||
|
identifier: "",
|
||||||
|
type: DeviceType.Android,
|
||||||
|
creationDate: "",
|
||||||
|
revisionDate: "",
|
||||||
|
isTrusted: true,
|
||||||
|
};
|
||||||
|
devicesApiService.getDevices.mockResolvedValue(
|
||||||
|
new ListResponse(
|
||||||
|
{
|
||||||
|
data: [deviceResponse],
|
||||||
|
},
|
||||||
|
DeviceResponse,
|
||||||
|
),
|
||||||
|
);
|
||||||
|
encryptService.decryptToBytes.mockResolvedValue(null);
|
||||||
|
encryptService.encrypt.mockResolvedValue(new EncString("test_encrypted_data"));
|
||||||
|
encryptService.rsaEncrypt.mockResolvedValue(new EncString("test_encrypted_data"));
|
||||||
|
|
||||||
|
const protectedDeviceResponse = new ProtectedDeviceResponse({
|
||||||
|
id: "id",
|
||||||
|
creationDate: "",
|
||||||
|
identifier: "test_device_identifier",
|
||||||
|
name: "Firefox",
|
||||||
|
type: DeviceType.FirefoxBrowser,
|
||||||
|
encryptedPublicKey: "",
|
||||||
|
encryptedUserKey: "",
|
||||||
|
});
|
||||||
|
devicesApiService.getDeviceKeys.mockResolvedValue(protectedDeviceResponse);
|
||||||
|
|
||||||
|
const fakeOldUserKeyData = new Uint8Array(64);
|
||||||
|
fakeOldUserKeyData.fill(5, 0, 1);
|
||||||
|
fakeOldUserKey = new SymmetricCryptoKey(fakeOldUserKeyData) as UserKey;
|
||||||
|
const fakeNewUserKeyData = new Uint8Array(64);
|
||||||
|
fakeNewUserKeyData.fill(1, 0, 1);
|
||||||
|
fakeNewUserKey = new SymmetricCryptoKey(fakeNewUserKeyData) as UserKey;
|
||||||
|
|
||||||
|
await deviceTrustService.getRotatedData(fakeOldUserKey, fakeNewUserKey, userId);
|
||||||
|
|
||||||
|
expect(devicesApiService.untrustDevices).toHaveBeenCalledWith(["id"]);
|
||||||
|
});
|
||||||
|
|
||||||
it("returns the expected data when all required parameters are provided", async () => {
|
it("returns the expected data when all required parameters are provided", async () => {
|
||||||
const deviceResponse = {
|
const deviceResponse = {
|
||||||
id: "",
|
id: "",
|
||||||
|
|||||||
Reference in New Issue
Block a user