1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-11 22:03:36 +00:00

[PM-19383] admins unable to download attachments (#14652)

* add admin support for downloading attachments

* fix delete and upload

* fix delete admin to return a response

* fix upload

* add missing param

* use getCipherAdmin

* fix cli

* fix QA bugs

* null check

* fix test

* cleanup

* add null check

* change comment
This commit is contained in:
Brandon Treston
2025-05-08 09:49:15 -04:00
committed by GitHub
parent 5943a00afd
commit f9edf048e3
7 changed files with 28 additions and 14 deletions

View File

@@ -853,6 +853,7 @@ export class VaultComponent implements OnInit, OnDestroy {
const dialogRef = AttachmentsV2Component.open(this.dialogService, { const dialogRef = AttachmentsV2Component.open(this.dialogService, {
cipherId: cipher.id as CipherId, cipherId: cipher.id as CipherId,
organizationId: cipher.organizationId as OrganizationId, organizationId: cipher.organizationId as OrganizationId,
admin: true,
}); });
const result = await firstValueFrom(dialogRef.closed); const result = await firstValueFrom(dialogRef.closed);

View File

@@ -10,14 +10,14 @@
<ng-container slot="end"> <ng-container slot="end">
<bit-item-action> <bit-item-action>
<app-download-attachment <app-download-attachment
[admin]="!!organizationId" [admin]="admin && organization?.canEditAllCiphers"
[cipher]="cipher" [cipher]="cipher"
[attachment]="attachment" [attachment]="attachment"
></app-download-attachment> ></app-download-attachment>
</bit-item-action> </bit-item-action>
<bit-item-action> <bit-item-action>
<app-delete-attachment <app-delete-attachment
[admin]="!!organizationId" [admin]="admin && organization?.canEditAllCiphers"
[cipherId]="cipher.id" [cipherId]="cipher.id"
[attachment]="attachment" [attachment]="attachment"
(onDeletionSuccess)="removeAttachment(attachment)" (onDeletionSuccess)="removeAttachment(attachment)"

View File

@@ -5,6 +5,8 @@ import { mock } from "jest-mock-extended";
import { ApiService } from "@bitwarden/common/abstractions/api.service"; import { ApiService } from "@bitwarden/common/abstractions/api.service";
import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction"; import { OrganizationService } from "@bitwarden/common/admin-console/abstractions/organization/organization.service.abstraction";
import { OrganizationUserType } from "@bitwarden/common/admin-console/enums";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service"; import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service"; import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@@ -53,6 +55,10 @@ describe("CipherAttachmentsComponent", () => {
decrypt: () => cipherView, decrypt: () => cipherView,
}; };
const organization = new Organization();
organization.type = OrganizationUserType.Admin;
organization.allowAdminAccessToAllCollectionItems = true;
const cipherServiceGet = jest.fn().mockResolvedValue(cipherDomain); const cipherServiceGet = jest.fn().mockResolvedValue(cipherDomain);
const saveAttachmentWithServer = jest.fn().mockResolvedValue(cipherDomain); const saveAttachmentWithServer = jest.fn().mockResolvedValue(cipherDomain);
@@ -70,6 +76,7 @@ describe("CipherAttachmentsComponent", () => {
{ {
provide: CipherService, provide: CipherService,
useValue: { useValue: {
organization,
get: cipherServiceGet, get: cipherServiceGet,
saveAttachmentWithServer, saveAttachmentWithServer,
getKeyForCipherKeyDecryption: () => Promise.resolve(null), getKeyForCipherKeyDecryption: () => Promise.resolve(null),
@@ -240,9 +247,11 @@ describe("CipherAttachmentsComponent", () => {
beforeEach(() => { beforeEach(() => {
component.attachmentForm.controls.file.setValue(file); component.attachmentForm.controls.file.setValue(file);
component.organization = organization;
}); });
it("calls `saveAttachmentWithServer`", async () => { it("calls `saveAttachmentWithServer` with admin=false when admin permission is false for organization", async () => {
component.organization.allowAdminAccessToAllCollectionItems = false;
await component.submit(); await component.submit();
expect(saveAttachmentWithServer).toHaveBeenCalledWith( expect(saveAttachmentWithServer).toHaveBeenCalledWith(
@@ -253,10 +262,8 @@ describe("CipherAttachmentsComponent", () => {
); );
}); });
it("calls `saveAttachmentWithServer` with isAdmin=true when using admin API", async () => { it("calls `saveAttachmentWithServer` with admin=true when using admin API", async () => {
// Set isAdmin to true to use admin API component.organization.allowAdminAccessToAllCollectionItems = true;
Object.defineProperty(component, "isAdmin", { value: true });
await component.submit(); await component.submit();
expect(saveAttachmentWithServer).toHaveBeenCalledWith(cipherDomain, file, mockUserId, true); expect(saveAttachmentWithServer).toHaveBeenCalledWith(cipherDomain, file, mockUserId, true);

View File

@@ -89,6 +89,9 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
/** The organization ID if this cipher belongs to an organization */ /** The organization ID if this cipher belongs to an organization */
@Input() organizationId?: OrganizationId; @Input() organizationId?: OrganizationId;
/** Denotes if the action is occurring from within the admin console */
@Input() admin: boolean = false;
/** An optional submit button, whose loading/disabled state will be tied to the form state. */ /** An optional submit button, whose loading/disabled state will be tied to the form state. */
@Input() submitBtn?: ButtonComponent; @Input() submitBtn?: ButtonComponent;
@@ -98,6 +101,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
/** Emits after a file has been successfully removed */ /** Emits after a file has been successfully removed */
@Output() onRemoveSuccess = new EventEmitter<void>(); @Output() onRemoveSuccess = new EventEmitter<void>();
organization: Organization;
cipher: CipherView; cipher: CipherView;
attachmentForm: CipherAttachmentForm = this.formBuilder.group({ attachmentForm: CipherAttachmentForm = this.formBuilder.group({
@@ -106,7 +110,6 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
private cipherDomain: Cipher; private cipherDomain: Cipher;
private activeUserId: UserId; private activeUserId: UserId;
private isAdmin = false;
private destroy$ = inject(DestroyRef); private destroy$ = inject(DestroyRef);
constructor( constructor(
@@ -130,6 +133,8 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
async ngOnInit(): Promise<void> { async ngOnInit(): Promise<void> {
this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId)); this.activeUserId = await firstValueFrom(this.accountService.activeAccount$.pipe(getUserId));
// Get the organization to check admin permissions
this.organization = await this.getOrganization();
this.cipherDomain = await this.getCipher(this.cipherId); this.cipherDomain = await this.getCipher(this.cipherId);
this.cipher = await this.cipherDomain.decrypt( this.cipher = await this.cipherDomain.decrypt(
@@ -201,7 +206,7 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
this.cipherDomain, this.cipherDomain,
file, file,
this.activeUserId, this.activeUserId,
this.isAdmin, this.organization?.canEditAllCiphers,
); );
// re-decrypt the cipher to update the attachments // re-decrypt the cipher to update the attachments
@@ -254,11 +259,8 @@ export class CipherAttachmentsComponent implements OnInit, AfterViewInit {
return localCipher; 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 // Only try the admin API if the user has admin permissions
if (organization != null && organization.canEditAllCiphers) { if (this.organization != null && this.organization.canEditAllCiphers) {
this.isAdmin = true;
const cipherResponse = await this.apiService.getCipherAdmin(id); const cipherResponse = await this.apiService.getCipherAdmin(id);
const cipherData = new CipherData(cipherResponse); const cipherData = new CipherData(cipherResponse);
return new Cipher(cipherData); return new Cipher(cipherData);

View File

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

View File

@@ -17,6 +17,7 @@ import { CipherAttachmentsComponent } from "../../cipher-form/components/attachm
export interface AttachmentsDialogParams { export interface AttachmentsDialogParams {
cipherId: CipherId; cipherId: CipherId;
admin?: boolean;
organizationId?: OrganizationId; organizationId?: OrganizationId;
} }
@@ -44,6 +45,7 @@ export interface AttachmentDialogCloseResult {
}) })
export class AttachmentsV2Component { export class AttachmentsV2Component {
cipherId: CipherId; cipherId: CipherId;
admin: boolean = false;
organizationId?: OrganizationId; organizationId?: OrganizationId;
attachmentFormId = CipherAttachmentsComponent.attachmentFormID; attachmentFormId = CipherAttachmentsComponent.attachmentFormID;
@@ -58,6 +60,7 @@ export class AttachmentsV2Component {
) { ) {
this.cipherId = params.cipherId; this.cipherId = params.cipherId;
this.organizationId = params.organizationId; this.organizationId = params.organizationId;
this.admin = params.admin ?? false;
} }
/** /**

View File

@@ -39,7 +39,7 @@ export class DownloadAttachmentComponent {
// Required for fetching attachment data when viewed from cipher via emergency access // Required for fetching attachment data when viewed from cipher via emergency access
@Input() emergencyAccessId?: EmergencyAccessId; @Input() emergencyAccessId?: EmergencyAccessId;
/** When accessing from the admin console, we will want to call the admin endpoint */ /** When owners/admins can mange all items and when accessing from the admin console, use the admin endpoint */
@Input() admin?: boolean = false; @Input() admin?: boolean = false;
/** The organization key if the cipher is associated with one */ /** The organization key if the cipher is associated with one */