1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-18322] Fix: Allow organization admins to upload attachments for items without direct access (#14361)

* Wire organization ID into AttachmentsV2Component for org-based ciphers

* Enhance AttachmentsV2Component to accept organization ID for improved handling of org-based ciphers

* Integrate organization ID into VaultComponent for AttachmentsV2Component to enhance org-based cipher handling

* Add unit tests for CipherAttachmentsComponent to validate attachment saving behavior for admins

- Introduced mocks for ApiService and OrganizationService in the test setup.
- Updated tests to check `saveAttachmentWithServer` calls with the correct parameters, including an `isAdmin` flag for admin API usage.

* Fix unit tests for AttachmentsV2Component by adding mocks for ApiService and OrganizationService

* Fix AttachmentsV2Component tests
This commit is contained in:
Rui Tomé
2025-04-29 12:42:02 +01:00
committed by GitHub
parent 67b1158bf0
commit 9cd08e8a9f
10 changed files with 122 additions and 21 deletions

View File

@@ -3,6 +3,8 @@ import { ComponentFixture, TestBed } from "@angular/core/testing";
import { By } from "@angular/platform-browser";
import { mock } from "jest-mock-extended";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -86,6 +88,14 @@ describe("CipherAttachmentsComponent", () => {
provide: AccountService,
useValue: accountService,
},
{
provide: ApiService,
useValue: mock<ApiService>(),
},
{
provide: OrganizationService,
useValue: mock<OrganizationService>(),
},
],
})
.overrideComponent(CipherAttachmentsComponent, {
@@ -234,7 +244,21 @@ describe("CipherAttachmentsComponent", () => {
it("calls `saveAttachmentWithServer`", async () => {
await component.submit();
expect(saveAttachmentWithServer).toHaveBeenCalledWith(cipherDomain, file, mockUserId);
expect(saveAttachmentWithServer).toHaveBeenCalledWith(
cipherDomain,
file,
mockUserId,
false,
);
});
it("calls `saveAttachmentWithServer` with isAdmin=true when using admin API", async () => {
// Set isAdmin to true to use admin API
Object.defineProperty(component, "isAdmin", { value: true });
await component.submit();
expect(saveAttachmentWithServer).toHaveBeenCalledWith(cipherDomain, file, mockUserId, true);
});
it("resets form and input values", async () => {

View File

@@ -24,12 +24,16 @@ import {
import { firstValueFrom } from "rxjs";
import { JslibModule } from "@bitwarden/angular/jslib.module";
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 { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { getUserId } from "@bitwarden/common/auth/services/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { CipherId, UserId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationId, UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherData } from "@bitwarden/common/vault/models/data/cipher.data";
import { Cipher } from "@bitwarden/common/vault/models/domain/cipher";
import { AttachmentView } from "@bitwarden/common/vault/models/view/attachment.view";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
@@ -82,6 +86,9 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
/** The `id` of the cipher in context */
@Input({ required: true }) cipherId: CipherId;
/** The organization ID if this cipher belongs to an organization */
@Input() organizationId?: OrganizationId;
/** An optional submit button, whose loading/disabled state will be tied to the form state. */
@Input() submitBtn?: ButtonComponent;
@@ -99,6 +106,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
private cipherDomain: Cipher;
private activeUserId: UserId;
private isAdmin = false;
private destroy$ = inject(DestroyRef);
constructor(
@@ -108,6 +116,8 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
private logService: LogService,
private toastService: ToastService,
private accountService: AccountService,
private apiService: ApiService,
private organizationService: OrganizationService,
) {
this.attachmentForm.statusChanges.pipe(takeUntilDestroyed()).subscribe((status) => {
if (!this.submitBtn) {
@@ -120,7 +130,8 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
async ngOnInit(): Promise<void> {
this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
this.cipherDomain = await this.cipherService.get(this.cipherId, this.activeUserId);
this.cipherDomain = await this.getCipher(this.cipherId);
this.cipher = await this.cipherDomain.decrypt(
await this.cipherService.getKeyForCipherKeyDecryption(this.cipherDomain, this.activeUserId),
);
@@ -190,6 +201,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
this.cipherDomain,
file,
this.activeUserId,
this.isAdmin,
);
// re-decrypt the cipher to update the attachments
@@ -223,4 +235,50 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
this.onRemoveSuccess.emit();
}
/**
* Gets a cipher using the appropriate method based on user permissions.
* If the user doesn't have direct access, but has organization admin access,
* it will retrieve the cipher using the admin endpoint.
*/
private async getCipher(id: CipherId): Promise<Cipher | null> {
if (id == null) {
return null;
}
// First try to get the cipher directly with user permissions
const localCipher = await this.cipherService.get(id, this.activeUserId);
// If we got the cipher or there's no organization context, return the result
if (localCipher != null || !this.organizationId) {
return localCipher;
}
// Get the organization to check admin permissions
const organization = await this.getOrganization();
// Only try the admin API if the user has admin permissions
if (organization != null && organization.canEditAllCiphers) {
this.isAdmin = true;
const cipherResponse = await this.apiService.getCipherAdmin(id);
const cipherData = new CipherData(cipherResponse);
return new Cipher(cipherData);
}
return null;
}
/**
* Gets the organization for the given organization ID
*/
private async getOrganization(): Promise<Organization | null> {
if (!this.organizationId) {
return null;
}
const organizations = await firstValueFrom(
this.organizationService.organizations$(this.activeUserId),
);
return organizations.find((o) => o.id === this.organizationId) || null;
}
}

View File

@@ -6,6 +6,7 @@
<app-cipher-attachments
*ngIf="cipherId"
[cipherId]="cipherId"
[organizationId]="organizationId"
[submitBtn]="submitBtn"
(onUploadSuccess)="uploadSuccessful()"
(onRemoveSuccess)="removalSuccessful()"

View File

@@ -2,10 +2,12 @@ import { ComponentFixture, TestBed } from "@angular/core/testing";
import { NoopAnimationsModule } from "@angular/platform-browser/animations";
import { mock } from "jest-mock-extended";
import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { CipherId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { DIALOG_DATA, DialogRef } from "@bitwarden/components";
@@ -20,8 +22,10 @@ describe("AttachmentsV2Component", () => {
let fixture: ComponentFixture<AttachmentsV2Component>;
const mockCipherId: CipherId = "cipher-id" as CipherId;
const mockOrganizationId: OrganizationId = "organization-id" as OrganizationId;
const mockParams: AttachmentsDialogParams = {
cipherId: mockCipherId,
organizationId: mockOrganizationId,
};
beforeEach(async () => {
@@ -34,6 +38,8 @@ describe("AttachmentsV2Component", () => {
{ provide: CipherService, useValue: mock<CipherService>() },
{ provide: LogService, useValue: mock<LogService>() },
{ provide: AccountService, useValue: mock<AccountService>() },
{ provide: ApiService, useValue: mock<ApiService>() },
{ provide: OrganizationService, useValue: mock<OrganizationService>() },
],
}).compileComponents();
@@ -42,9 +48,10 @@ describe("AttachmentsV2Component", () => {
fixture.detectChanges();
});
it("initializes without errors and with the correct cipherId", () => {
it("initializes without errors and with the correct cipherId and organizationId", () => {
expect(component).toBeTruthy();
expect(component.cipherId).toBe(mockParams.cipherId);
expect(component.organizationId).toBe(mockParams.organizationId);
});
it("closes the dialog with 'uploaded' result on uploadSuccessful", () => {

View File

@@ -3,7 +3,7 @@
import { CommonModule } from "@angular/common";
import { Component, Inject } from "@angular/core";
import { CipherId } from "@bitwarden/common/types/guid";
import { CipherId, OrganizationId } from "@bitwarden/common/types/guid";
import {
ButtonModule,
DialogModule,
@@ -17,6 +17,7 @@ import { CipherAttachmentsComponent } from "../../cipher-form/components/attachm
export interface AttachmentsDialogParams {
cipherId: CipherId;
organizationId?: OrganizationId;
}
/**
@@ -43,6 +44,7 @@ export interface AttachmentDialogCloseResult {
})
export class AttachmentsV2Component {
cipherId: CipherId;
organizationId?: OrganizationId;
attachmentFormId = CipherAttachmentsComponent.attachmentFormID;
/**
@@ -55,6 +57,7 @@ export class AttachmentsV2Component {
@Inject(DIALOG_DATA) public params: AttachmentsDialogParams,
) {
this.cipherId = params.cipherId;
this.organizationId = params.organizationId;
}
/**