1
0
mirror of https://github.com/bitwarden/browser.git synced 2025-02-22 02:21:34 +01:00

[PM-14416] Risk Insights - Initial security task service (#12446)

* [PM-14416] Add initial SecurityTask models and enums

* [PM-14416] Add support for PATCH request method and 204 No Content response

* [PM-14416] Add initial task service abstraction

* [PM-14416] Add SecurityTask state/key definitions

* [PM-14416] Add DefaultTaskService implementation

* [PM-14416] Add DefaultTaskService tests

* [PM-14416] Add better null checking to new models

* [PM-14416] Improve null value filtering for task service
This commit is contained in:
Shane Melton 2025-01-21 09:50:50 -08:00 committed by GitHub
parent 1b9f546139
commit 5e1d5bad07
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
18 changed files with 574 additions and 5 deletions

View File

@ -1,9 +1,9 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import {
CollectionRequest,
CollectionAccessDetailsResponse,
CollectionDetailsResponse,
CollectionRequest,
CollectionResponse,
} from "@bitwarden/admin-console/common";
@ -136,7 +136,7 @@ import { OptionalCipherResponse } from "../vault/models/response/optional-cipher
*/
export abstract class ApiService {
send: (
method: "GET" | "POST" | "PUT" | "DELETE",
method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH",
path: string,
body: any,
authed: boolean,

View File

@ -187,3 +187,4 @@ export const NEW_DEVICE_VERIFICATION_NOTICE = new StateDefinition(
},
);
export const VAULT_APPEARANCE = new StateDefinition("vaultAppearance", "disk");
export const SECURITY_TASKS_DISK = new StateDefinition("securityTasks", "disk");

View File

@ -3,9 +3,9 @@
import { firstValueFrom } from "rxjs";
import {
CollectionRequest,
CollectionAccessDetailsResponse,
CollectionDetailsResponse,
CollectionRequest,
CollectionResponse,
} from "@bitwarden/admin-console/common";
import { LogoutReason } from "@bitwarden/auth/common";
@ -1829,7 +1829,7 @@ export class ApiService implements ApiServiceAbstraction {
}
async send(
method: "GET" | "POST" | "PUT" | "DELETE",
method: "GET" | "POST" | "PUT" | "DELETE" | "PATCH",
path: string,
body: any,
authed: boolean,
@ -1869,7 +1869,7 @@ export class ApiService implements ApiServiceAbstraction {
return responseJson;
} else if (hasResponse && response.status === 200 && responseIsCsv) {
return await response.text();
} else if (response.status !== 200) {
} else if (response.status !== 200 && response.status !== 204) {
const error = await this.handleError(response, false, authed);
return Promise.reject(error);
}

View File

@ -10,3 +10,4 @@ export type PolicyId = Opaque<string, "PolicyId">;
export type CipherId = Opaque<string, "CipherId">;
export type SendId = Opaque<string, "SendId">;
export type IndexedEntityId = Opaque<string, "IndexedEntityId">;
export type SecurityTaskId = Opaque<string, "SecurityTaskId">;

View File

@ -19,3 +19,5 @@ export { NewDeviceVerificationNoticePageTwoComponent } from "./components/new-de
export { DecryptionFailureDialogComponent } from "./components/decryption-failure-dialog/decryption-failure-dialog.component";
export * as VaultIcons from "./icons";
export * from "./tasks";

View File

@ -0,0 +1,45 @@
import { Observable } from "rxjs";
import { SecurityTaskId, UserId } from "@bitwarden/common/types/guid";
import { SecurityTask } from "@bitwarden/vault";
export abstract class TaskService {
/**
* Observable indicating if tasks are enabled for a given user.
*
* @remarks Internally, this checks the user's organization details to determine if tasks are enabled.
* @param userId
*/
abstract tasksEnabled$(userId: UserId): Observable<boolean>;
/**
* Observable of all tasks for a given user.
* @param userId
*/
abstract tasks$(userId: UserId): Observable<SecurityTask[]>;
/**
* Observable of pending tasks for a given user.
* @param userId
*/
abstract pendingTasks$(userId: UserId): Observable<SecurityTask[]>;
/**
* Retrieves tasks from the API for a given user and updates the local state.
* @param userId
*/
abstract refreshTasks(userId: UserId): Promise<void>;
/**
* Clears all the tasks from state for the given user.
* @param userId
*/
abstract clear(userId: UserId): Promise<void>;
/**
* Marks a task as complete in local state and updates the server.
* @param taskId - The ID of the task to mark as complete.
* @param userId - The user who is completing the task.
*/
abstract markAsComplete(taskId: SecurityTaskId, userId: UserId): Promise<void>;
}

View File

@ -0,0 +1,2 @@
export * from "./security-task-status.enum";
export * from "./security-task-type.enum";

View File

@ -0,0 +1,11 @@
export enum SecurityTaskStatus {
/**
* Default status for newly created tasks that have not been completed.
*/
Pending = 0,
/**
* Status when a task is considered complete and has no remaining actions
*/
Completed = 1,
}

View File

@ -0,0 +1,6 @@
export enum SecurityTaskType {
/**
* Task to update a cipher's password that was found to be at-risk by an administrator
*/
UpdateAtRiskCredential = 0,
}

View File

@ -0,0 +1,5 @@
export * from "./enums";
export * from "./models";
export * from "./abstractions/task.service";
export * from "./services/default-task.service";

View File

@ -0,0 +1 @@
export * from "./security-task";

View File

@ -0,0 +1,34 @@
import { Jsonify } from "type-fest";
import { CipherId, OrganizationId, SecurityTaskId } from "@bitwarden/common/types/guid";
import { SecurityTaskStatus, SecurityTaskType } from "../enums";
import { SecurityTaskResponse } from "./security-task.response";
export class SecurityTaskData {
id: SecurityTaskId;
organizationId: OrganizationId;
cipherId: CipherId | undefined;
type: SecurityTaskType;
status: SecurityTaskStatus;
creationDate: Date;
revisionDate: Date;
constructor(response: SecurityTaskResponse) {
this.id = response.id;
this.organizationId = response.organizationId;
this.cipherId = response.cipherId;
this.type = response.type;
this.status = response.status;
this.creationDate = response.creationDate;
this.revisionDate = response.revisionDate;
}
static fromJSON(obj: Jsonify<SecurityTaskData>) {
return Object.assign(new SecurityTaskData({} as SecurityTaskResponse), obj, {
creationDate: new Date(obj.creationDate),
revisionDate: new Date(obj.revisionDate),
});
}
}

View File

@ -0,0 +1,28 @@
import { BaseResponse } from "@bitwarden/common/models/response/base.response";
import { CipherId, OrganizationId, SecurityTaskId } from "@bitwarden/common/types/guid";
import { SecurityTaskStatus, SecurityTaskType } from "../enums";
export class SecurityTaskResponse extends BaseResponse {
id: SecurityTaskId;
organizationId: OrganizationId;
/**
* Optional cipherId for tasks that are related to a cipher.
*/
cipherId: CipherId | undefined;
type: SecurityTaskType;
status: SecurityTaskStatus;
creationDate: Date;
revisionDate: Date;
constructor(response: any) {
super(response);
this.id = this.getResponseProperty("Id");
this.organizationId = this.getResponseProperty("OrganizationId");
this.cipherId = this.getResponseProperty("CipherId") || undefined;
this.type = this.getResponseProperty("Type");
this.status = this.getResponseProperty("Status");
this.creationDate = this.getResponseProperty("CreationDate");
this.revisionDate = this.getResponseProperty("RevisionDate");
}
}

View File

@ -0,0 +1,28 @@
import { CipherId, OrganizationId, SecurityTaskId } from "@bitwarden/common/types/guid";
import { SecurityTaskStatus, SecurityTaskType } from "../enums";
import { SecurityTaskData } from "./security-task.data";
export class SecurityTask {
id: SecurityTaskId;
organizationId: OrganizationId;
/**
* Optional cipherId for tasks that are related to a cipher.
*/
cipherId: CipherId | undefined;
type: SecurityTaskType;
status: SecurityTaskStatus;
creationDate: Date;
revisionDate: Date;
constructor(obj: SecurityTaskData) {
this.id = obj.id;
this.organizationId = obj.organizationId;
this.cipherId = obj.cipherId;
this.type = obj.type;
this.status = obj.status;
this.creationDate = obj.creationDate;
this.revisionDate = obj.revisionDate;
}
}

View File

@ -0,0 +1,261 @@
import { TestBed } from "@angular/core/testing";
import { BehaviorSubject, firstValueFrom } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { StateProvider } from "@bitwarden/common/platform/state";
import { SecurityTaskId, UserId } from "@bitwarden/common/types/guid";
import { DefaultTaskService, SecurityTaskStatus } from "@bitwarden/vault";
import { FakeStateProvider, mockAccountServiceWith } from "../../../../common/spec";
import { SecurityTaskData } from "../models/security-task.data";
import { SecurityTaskResponse } from "../models/security-task.response";
import { SECURITY_TASKS } from "../state/security-task.state";
describe("Default task service", () => {
let fakeStateProvider: FakeStateProvider;
const mockApiSend = jest.fn();
const mockGetAllOrgs$ = jest.fn();
let testBed: TestBed;
beforeEach(async () => {
mockApiSend.mockClear();
mockGetAllOrgs$.mockClear();
fakeStateProvider = new FakeStateProvider(mockAccountServiceWith("user-id" as UserId));
testBed = TestBed.configureTestingModule({
imports: [],
providers: [
DefaultTaskService,
{
provide: StateProvider,
useValue: fakeStateProvider,
},
{
provide: ApiService,
useValue: {
send: mockApiSend,
},
},
{
provide: OrganizationService,
useValue: {
getAll$: mockGetAllOrgs$,
},
},
],
});
});
describe("tasksEnabled$", () => {
it("should emit true if any organization uses risk insights", async () => {
mockGetAllOrgs$.mockReturnValue(
new BehaviorSubject([
{
useRiskInsights: false,
},
{
useRiskInsights: true,
},
] as Organization[]),
);
const { tasksEnabled$ } = testBed.inject(DefaultTaskService);
const result = await firstValueFrom(tasksEnabled$("user-id" as UserId));
expect(result).toBe(true);
});
it("should emit false if no organization uses risk insights", async () => {
mockGetAllOrgs$.mockReturnValue(
new BehaviorSubject([
{
useRiskInsights: false,
},
{
useRiskInsights: false,
},
] as Organization[]),
);
const { tasksEnabled$ } = testBed.inject(DefaultTaskService);
const result = await firstValueFrom(tasksEnabled$("user-id" as UserId));
expect(result).toBe(false);
});
});
describe("tasks$", () => {
it("should fetch tasks from the API when the state is null", async () => {
mockApiSend.mockResolvedValue({
data: [
{
id: "task-id",
},
] as SecurityTaskResponse[],
});
fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, null);
const { tasks$ } = testBed.inject(DefaultTaskService);
const result = await firstValueFrom(tasks$("user-id" as UserId));
expect(result.length).toBe(1);
expect(mockApiSend).toHaveBeenCalledWith("GET", "/tasks", null, true, true);
});
it("should use the tasks from state when not null", async () => {
fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, [
{
id: "task-id" as SecurityTaskId,
} as SecurityTaskData,
]);
const { tasks$ } = testBed.inject(DefaultTaskService);
const result = await firstValueFrom(tasks$("user-id" as UserId));
expect(result.length).toBe(1);
expect(mockApiSend).not.toHaveBeenCalled();
});
it("should share the same observable for the same user", async () => {
const { tasks$ } = testBed.inject(DefaultTaskService);
const first = tasks$("user-id" as UserId);
const second = tasks$("user-id" as UserId);
expect(first).toBe(second);
});
});
describe("pendingTasks$", () => {
it("should filter tasks to only pending tasks", async () => {
fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, [
{
id: "completed-task-id" as SecurityTaskId,
status: SecurityTaskStatus.Completed,
},
{
id: "pending-task-id" as SecurityTaskId,
status: SecurityTaskStatus.Pending,
},
] as SecurityTaskData[]);
const { pendingTasks$ } = testBed.inject(DefaultTaskService);
const result = await firstValueFrom(pendingTasks$("user-id" as UserId));
expect(result.length).toBe(1);
expect(result[0].id).toBe("pending-task-id" as SecurityTaskId);
});
});
describe("refreshTasks()", () => {
it("should fetch tasks from the API", async () => {
mockApiSend.mockResolvedValue({
data: [
{
id: "task-id",
},
] as SecurityTaskResponse[],
});
const service = testBed.inject(DefaultTaskService);
await service.refreshTasks("user-id" as UserId);
expect(mockApiSend).toHaveBeenCalledWith("GET", "/tasks", null, true, true);
});
it("should update the local state with refreshed tasks", async () => {
mockApiSend.mockResolvedValue({
data: [
{
id: "task-id",
},
] as SecurityTaskResponse[],
});
const mock = fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, null);
const service = testBed.inject(DefaultTaskService);
await service.refreshTasks("user-id" as UserId);
expect(mock.nextMock).toHaveBeenCalledWith([
{
id: "task-id" as SecurityTaskId,
} as SecurityTaskData,
]);
});
});
describe("clear()", () => {
it("should clear the local state for the user", async () => {
const mock = fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, [
{
id: "task-id" as SecurityTaskId,
} as SecurityTaskData,
]);
const service = testBed.inject(DefaultTaskService);
await service.clear("user-id" as UserId);
expect(mock.nextMock).toHaveBeenCalledWith([]);
});
});
describe("markAsComplete()", () => {
it("should send an API request to mark the task as complete", async () => {
const service = testBed.inject(DefaultTaskService);
await service.markAsComplete("task-id" as SecurityTaskId, "user-id" as UserId);
expect(mockApiSend).toHaveBeenCalledWith(
"PATCH",
"/tasks/task-id/complete",
null,
true,
false,
);
});
it("should refresh all tasks for the user after marking the task as complete", async () => {
mockApiSend
.mockResolvedValueOnce(null) // Mark as complete
.mockResolvedValueOnce({
// Refresh tasks
data: [
{
id: "new-task-id",
},
] as SecurityTaskResponse[],
});
const mockState = fakeStateProvider.singleUser.mockFor("user-id" as UserId, SECURITY_TASKS, [
{
id: "old-task-id" as SecurityTaskId,
} as SecurityTaskData,
]);
const service = testBed.inject(DefaultTaskService);
await service.markAsComplete("task-id" as SecurityTaskId, "user-id" as UserId);
expect(mockApiSend).toHaveBeenCalledWith("GET", "/tasks", null, true, true);
expect(mockState.nextMock).toHaveBeenCalledWith([
{
id: "new-task-id",
} as SecurityTaskData,
]);
});
});
});

View File

@ -0,0 +1,93 @@
import { Injectable } from "@angular/core";
import { map, switchMap } from "rxjs";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { ListResponse } from "@bitwarden/common/models/response/list.response";
import { StateProvider } from "@bitwarden/common/platform/state";
import { SecurityTaskId, UserId } from "@bitwarden/common/types/guid";
import { SecurityTask, SecurityTaskStatus, TaskService } from "@bitwarden/vault";
import { filterOutNullish, perUserCache$ } from "../../utils/observable-utilities";
import { SecurityTaskData } from "../models/security-task.data";
import { SecurityTaskResponse } from "../models/security-task.response";
import { SECURITY_TASKS } from "../state/security-task.state";
@Injectable()
export class DefaultTaskService implements TaskService {
constructor(
private stateProvider: StateProvider,
private apiService: ApiService,
private organizationService: OrganizationService,
) {}
tasksEnabled$ = perUserCache$((userId) => {
return this.organizationService
.getAll$(userId)
.pipe(map((orgs) => orgs.some((o) => o.useRiskInsights)));
});
tasks$ = perUserCache$((userId) => {
return this.taskState(userId).state$.pipe(
switchMap(async (tasks) => {
if (tasks == null) {
await this.fetchTasksFromApi(userId);
}
return tasks;
}),
filterOutNullish(),
map((tasks) => tasks.map((t) => new SecurityTask(t))),
);
});
pendingTasks$ = perUserCache$((userId) => {
return this.tasks$(userId).pipe(
map((tasks) => tasks.filter((t) => t.status === SecurityTaskStatus.Pending)),
);
});
async refreshTasks(userId: UserId): Promise<void> {
await this.fetchTasksFromApi(userId);
}
async clear(userId: UserId): Promise<void> {
await this.updateTaskState(userId, []);
}
async markAsComplete(taskId: SecurityTaskId, userId: UserId): Promise<void> {
await this.apiService.send("PATCH", `/tasks/${taskId}/complete`, null, true, false);
await this.refreshTasks(userId);
}
/**
* Fetches the tasks from the API and updates the local state
* @param userId
* @private
*/
private async fetchTasksFromApi(userId: UserId): Promise<void> {
const r = await this.apiService.send("GET", "/tasks", null, true, true);
const response = new ListResponse(r, SecurityTaskResponse);
const taskData = response.data.map((t) => new SecurityTaskData(t));
await this.updateTaskState(userId, taskData);
}
/**
* Returns the local state for the tasks
* @param userId
* @private
*/
private taskState(userId: UserId) {
return this.stateProvider.getUser(userId, SECURITY_TASKS);
}
/**
* Updates the local state with the provided tasks and returns the updated state
* @param userId
* @param tasks
* @private
*/
private updateTaskState(userId: UserId, tasks: SecurityTaskData[]): Promise<SecurityTaskData[]> {
return this.taskState(userId).update(() => tasks);
}
}

View File

@ -0,0 +1,14 @@
import { Jsonify } from "type-fest";
import { SECURITY_TASKS_DISK, UserKeyDefinition } from "@bitwarden/common/platform/state";
import { SecurityTaskData } from "../models/security-task.data";
export const SECURITY_TASKS = UserKeyDefinition.array<SecurityTaskData>(
SECURITY_TASKS_DISK,
"securityTasks",
{
deserializer: (task: Jsonify<SecurityTaskData>) => SecurityTaskData.fromJSON(task),
clearOn: ["logout", "lock"],
},
);

View File

@ -0,0 +1,37 @@
import { filter, Observable, OperatorFunction, shareReplay } from "rxjs";
import { UserId } from "@bitwarden/common/types/guid";
/**
* Builds an observable once per userId and caches it for future requests.
* The built observables are shared among subscribers with a replay buffer size of 1.
* @param create - A function that creates an observable for a given userId.
*/
export function perUserCache$<TValue>(
create: (userId: UserId) => Observable<TValue>,
): (userId: UserId) => Observable<TValue> {
const cache = new Map<UserId, Observable<TValue>>();
return (userId: UserId) => {
let observable = cache.get(userId);
if (!observable) {
observable = create(userId).pipe(shareReplay({ bufferSize: 1, refCount: false }));
cache.set(userId, observable);
}
return observable;
};
}
/**
* Strongly typed observable operator that filters out null/undefined values and adjusts the return type to
* be non-nullable.
*
* @example
* ```ts
* const source$ = of(1, null, 2, undefined, 3);
* source$.pipe(filterOutNullish()).subscribe(console.log);
* // Output: 1, 2, 3
* ```
*/
export function filterOutNullish<T>(): OperatorFunction<T | undefined | null, T> {
return filter((v): v is T => v != null);
}