From 493788c9d06afd0a0069514d2b30132b5d0c7e25 Mon Sep 17 00:00:00 2001 From: cd-bitwarden <106776772+cd-bitwarden@users.noreply.github.com> Date: Fri, 5 Sep 2025 13:15:28 -0400 Subject: [PATCH] [SM-1564] Fixing issues (#16295) * Fixing issues * fixes --- .../guards/project-access.guard.spec.ts | 135 ++++++++++++++++++ .../guards/project-access.guard.ts | 33 +++++ .../dialog/project-dialog.component.ts | 5 +- .../projects/project.service.ts | 23 ++- .../project/project-secrets.component.html | 8 +- .../project/project-secrets.component.ts | 31 ++-- .../projects/project/project.component.html | 2 +- .../projects/project/project.component.ts | 5 +- .../projects/projects-routing.module.ts | 3 + .../shared/projects-list.component.html | 8 +- 10 files changed, 229 insertions(+), 24 deletions(-) create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.spec.ts create mode 100644 bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.ts diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.spec.ts b/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.spec.ts new file mode 100644 index 00000000000..f442c85f46d --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.spec.ts @@ -0,0 +1,135 @@ +import { Component } from "@angular/core"; +import { TestBed } from "@angular/core/testing"; +import { Router } from "@angular/router"; +import { RouterTestingModule } from "@angular/router/testing"; +import { MockProxy, mock } from "jest-mock-extended"; +import { of } from "rxjs"; + +import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; +import { Organization } from "@bitwarden/common/admin-console/models/domain/organization"; +import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { Utils } from "@bitwarden/common/platform/misc/utils"; +import { FakeAccountService, mockAccountServiceWith } from "@bitwarden/common/spec"; +import { UserId } from "@bitwarden/common/types/guid"; +import { ToastService } from "@bitwarden/components"; +import { RouterService } from "@bitwarden/web-vault/app/core"; + +import { ProjectView } from "../models/view/project.view"; +import { ProjectService } from "../projects/project.service"; + +import { projectAccessGuard } from "./project-access.guard"; + +@Component({ + template: "", + standalone: false, +}) +export class GuardedRouteTestComponent {} + +@Component({ + template: "", + standalone: false, +}) +export class RedirectTestComponent {} + +describe("Project Redirect Guard", () => { + let organizationService: MockProxy; + let routerService: MockProxy; + let projectServiceMock: MockProxy; + let i18nServiceMock: MockProxy; + let toastService: MockProxy; + let router: Router; + let accountService: FakeAccountService; + const userId = Utils.newGuid() as UserId; + + const smOrg1 = { id: "123", canAccessSecretsManager: true } as Organization; + const projectView = { + id: "123", + organizationId: "123", + name: "project-name", + creationDate: Date.now.toString(), + revisionDate: Date.now.toString(), + read: true, + write: true, + } as ProjectView; + + beforeEach(async () => { + organizationService = mock(); + routerService = mock(); + projectServiceMock = mock(); + i18nServiceMock = mock(); + toastService = mock(); + accountService = mockAccountServiceWith(userId); + + TestBed.configureTestingModule({ + imports: [ + RouterTestingModule.withRoutes([ + { + path: "sm/:organizationId/projects/:projectId", + component: GuardedRouteTestComponent, + canActivate: [projectAccessGuard], + }, + { + path: "sm", + component: RedirectTestComponent, + }, + { + path: "sm/:organizationId/projects", + component: RedirectTestComponent, + }, + ]), + ], + providers: [ + { provide: OrganizationService, useValue: organizationService }, + { provide: AccountService, useValue: accountService }, + { provide: RouterService, useValue: routerService }, + { provide: ProjectService, useValue: projectServiceMock }, + { provide: I18nService, useValue: i18nServiceMock }, + { provide: ToastService, useValue: toastService }, + ], + }); + + router = TestBed.inject(Router); + }); + + it("redirects to sm/{orgId}/projects/{projectId} if project exists", async () => { + // Arrange + organizationService.organizations$.mockReturnValue(of([smOrg1])); + projectServiceMock.getByProjectId.mockReturnValue(Promise.resolve(projectView)); + + // Act + await router.navigateByUrl("sm/123/projects/123"); + + // Assert + expect(router.url).toBe("/sm/123/projects/123"); + }); + + it("redirects to sm/projects if project does not exist", async () => { + // Arrange + organizationService.organizations$.mockReturnValue(of([smOrg1])); + + // Act + await router.navigateByUrl("sm/123/projects/124"); + + // Assert + expect(router.url).toBe("/sm/123/projects"); + }); + + it("redirects to sm/123/projects if exception occurs while looking for Project", async () => { + // Arrange + jest.spyOn(projectServiceMock, "getByProjectId").mockImplementation(() => { + throw new Error("Test error"); + }); + jest.spyOn(i18nServiceMock, "t").mockReturnValue("Project not found"); + + // Act + await router.navigateByUrl("sm/123/projects/123"); + // Assert + expect(toastService.showToast).toHaveBeenCalledWith({ + variant: "error", + title: null, + message: "Project not found", + }); + expect(router.url).toBe("/sm/123/projects"); + }); +}); diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.ts b/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.ts new file mode 100644 index 00000000000..b8c9640dc50 --- /dev/null +++ b/bitwarden_license/bit-web/src/app/secrets-manager/guards/project-access.guard.ts @@ -0,0 +1,33 @@ +// FIXME: Update this file to be type safe and remove this and next line +// @ts-strict-ignore +import { inject } from "@angular/core"; +import { ActivatedRouteSnapshot, CanActivateFn, createUrlTreeFromSnapshot } from "@angular/router"; + +import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; +import { ToastService } from "@bitwarden/components"; + +import { ProjectService } from "../projects/project.service"; + +/** + * Redirects to projects list if the user doesn't have access to project. + */ +export const projectAccessGuard: CanActivateFn = async (route: ActivatedRouteSnapshot) => { + const projectService = inject(ProjectService); + const toastService = inject(ToastService); + const i18nService = inject(I18nService); + + try { + const project = await projectService.getByProjectId(route.params.projectId, true); + if (project) { + return true; + } + } catch { + toastService.showToast({ + variant: "error", + title: null, + message: i18nService.t("notFound", i18nService.t("project")), + }); + return createUrlTreeFromSnapshot(route, ["/sm", route.params.organizationId, "projects"]); + } + return createUrlTreeFromSnapshot(route, ["/sm", route.params.organizationId, "projects"]); +}; diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts index ec420d653cc..819f2107fcf 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/dialog/project-dialog.component.ts @@ -59,7 +59,10 @@ export class ProjectDialogComponent implements OnInit { async loadData() { this.loading = true; - const project: ProjectView = await this.projectService.getByProjectId(this.data.projectId); + const project: ProjectView = await this.projectService.getByProjectId( + this.data.projectId, + true, + ); this.loading = false; this.formGroup.setValue({ name: project.name }); } diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project.service.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project.service.ts index fce1fc16d6c..5d350a684a5 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project.service.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project.service.ts @@ -27,6 +27,7 @@ import { ProjectResponse } from "./models/responses/project.response"; export class ProjectService { protected _project = new Subject(); project$ = this._project.asObservable(); + private projectCache = new Map>(); constructor( private keyService: KeyService, @@ -48,10 +49,24 @@ export class ProjectService { return await firstValueFrom(this.getOrganizationKey$(organizationId)); } - async getByProjectId(projectId: string): Promise { - const r = await this.apiService.send("GET", "/projects/" + projectId, null, true, true); - const projectResponse = new ProjectResponse(r); - return await this.createProjectView(projectResponse); + async getByProjectId(projectId: string, forceRequest: boolean): Promise { + if (forceRequest || !this.projectCache.has(projectId)) { + const request = this.apiService + .send("GET", `/projects/${projectId}`, null, true, true) + .then((r) => { + const projectResponse = new ProjectResponse(r); + return this.createProjectView(projectResponse); + }) + .catch((err) => { + // remove from cache if it failed, so future calls can retry + this.projectCache.delete(projectId); + throw err; + }); + + this.projectCache.set(projectId, request); + } + + return this.projectCache.get(projectId)!; } async getProjects(organizationId: string): Promise { diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.html index 7a2968cfb2c..d9919ef6bac 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project-secrets.component.html @@ -1,7 +1,7 @@ - - + +
; private organizationEnabled: boolean; - protected project = inject(ROUTER_OUTLET_DATA) as Signal; - readonly writeAccess = computed(() => this.project().write); - readonly projectExists = computed(() => !!this.project()); constructor( private route: ActivatedRoute, + private projectService: ProjectService, private secretService: SecretService, private dialogService: DialogService, private platformUtilsService: PlatformUtilsService, @@ -61,9 +67,18 @@ export class ProjectSecretsComponent implements OnInit { ) {} ngOnInit() { + const currentProjectEdited = this.projectService.project$.pipe( + filter((p) => p?.id === this.projectId), + startWith(null), + ); + + this.project$ = combineLatest([this.route.params, currentProjectEdited]).pipe( + switchMap(([params, _]) => this.projectService.getByProjectId(params.projectId, false)), + ); + this.secrets$ = this.secretService.secret$.pipe( startWith(null), - combineLatestWith(this.route.params), + combineLatestWith(this.route.params, currentProjectEdited), switchMap(async ([_, params]) => { this.organizationId = params.organizationId; this.projectId = params.projectId; diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.html index ef7c8ff1275..d44d7898cac 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.html @@ -36,4 +36,4 @@ {{ "editProject" | i18n }} - + diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts index 2d008dd219a..c79ebd733c0 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/project/project.component.ts @@ -67,9 +67,10 @@ export class ProjectComponent implements OnInit, OnDestroy { ); this.project$ = combineLatest([this.route.params, currentProjectEdited]).pipe( - switchMap(([params, _]) => this.projectService.getByProjectId(params.projectId)), + switchMap(([params, currentProj]) => + this.projectService.getByProjectId(params.projectId, currentProj != null), + ), ); - const projectId$ = this.route.params.pipe(map((p) => p.projectId)); const organization$ = this.route.params.pipe( concatMap((params) => diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts index 6078520989a..ab0f7f663e3 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts +++ b/bitwarden_license/bit-web/src/app/secrets-manager/projects/projects-routing.module.ts @@ -1,6 +1,8 @@ import { NgModule } from "@angular/core"; import { RouterModule, Routes } from "@angular/router"; +import { projectAccessGuard } from "../guards/project-access.guard"; + import { ProjectPeopleComponent } from "./project/project-people.component"; import { ProjectSecretsComponent } from "./project/project-secrets.component"; import { ProjectServiceAccountsComponent } from "./project/project-service-accounts.component"; @@ -24,6 +26,7 @@ const routes: Routes = [ { path: "secrets", component: ProjectSecretsComponent, + canActivate: [projectAccessGuard], }, { path: "people", diff --git a/bitwarden_license/bit-web/src/app/secrets-manager/shared/projects-list.component.html b/bitwarden_license/bit-web/src/app/secrets-manager/shared/projects-list.component.html index 9e31ff628fb..ef5df7aaf4c 100644 --- a/bitwarden_license/bit-web/src/app/secrets-manager/shared/projects-list.component.html +++ b/bitwarden_license/bit-web/src/app/secrets-manager/shared/projects-list.component.html @@ -110,10 +110,6 @@ {{ "editProject" | i18n }} - +