1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-10 14:43:34 +01:00

[PM-14965] Fix password reprompt for context menu actions (#12213)

* [PM-14965] Add return value for copy-cipher-field.service

* [PM-14965] Cleanup loadAction handling in updated browser view item page

* [PM-14965] Fix unit tests

* [PM-14965] Clear copy mock
This commit is contained in:
Shane Melton 2024-12-03 09:37:44 -08:00 committed by GitHub
parent 9188a31b4a
commit 60e52dd2f2
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 185 additions and 23 deletions

View File

@ -5,7 +5,12 @@ import { Subject } from "rxjs";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AUTOFILL_ID } from "@bitwarden/common/autofill/constants";
import {
AUTOFILL_ID,
COPY_PASSWORD_ID,
COPY_USERNAME_ID,
COPY_VERIFICATION_CODE_ID,
} from "@bitwarden/common/autofill/constants";
import { EventType } from "@bitwarden/common/enums";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
@ -17,7 +22,10 @@ import { UserId } from "@bitwarden/common/types/guid";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
import { CopyCipherFieldService } from "@bitwarden/vault";
import { BrowserApi } from "../../../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils";
import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
import { VaultPopupAutofillService } from "./../../../services/vault-popup-autofill.service";
@ -34,17 +42,26 @@ describe("ViewV2Component", () => {
const params$ = new Subject();
const mockNavigate = jest.fn();
const collect = jest.fn().mockResolvedValue(null);
const doAutofill = jest.fn();
const doAutofill = jest.fn().mockResolvedValue(true);
const copy = jest.fn().mockResolvedValue(true);
const mockCipher = {
id: "122-333-444",
type: CipherType.Login,
orgId: "222-444-555",
login: {
username: "test-username",
password: "test-password",
totp: "123",
},
};
const mockVaultPopupAutofillService = {
doAutofill,
};
const mockCopyCipherFieldService = {
copy,
};
const mockUserId = Utils.newGuid() as UserId;
const accountService: FakeAccountService = mockAccountServiceWith(mockUserId);
@ -57,6 +74,7 @@ describe("ViewV2Component", () => {
mockNavigate.mockClear();
collect.mockClear();
doAutofill.mockClear();
copy.mockClear();
await TestBed.configureTestingModule({
imports: [ViewV2Component],
@ -91,6 +109,10 @@ describe("ViewV2Component", () => {
canDeleteCipher$: jest.fn().mockReturnValue(true),
},
},
{
provide: CopyCipherFieldService,
useValue: mockCopyCipherFieldService,
},
],
}).compileComponents();
@ -159,5 +181,46 @@ describe("ViewV2Component", () => {
expect(doAutofill).toHaveBeenCalledOnce();
}));
it('invokes `copy` when action="copy-username"', fakeAsync(() => {
params$.next({ action: COPY_USERNAME_ID });
flush(); // Resolve all promises
expect(copy).toHaveBeenCalledOnce();
}));
it('invokes `copy` when action="copy-password"', fakeAsync(() => {
params$.next({ action: COPY_PASSWORD_ID });
flush(); // Resolve all promises
expect(copy).toHaveBeenCalledOnce();
}));
it('invokes `copy` when action="copy-totp"', fakeAsync(() => {
params$.next({ action: COPY_VERIFICATION_CODE_ID });
flush(); // Resolve all promises
expect(copy).toHaveBeenCalledOnce();
}));
it("closes the popout after a load action", fakeAsync(() => {
jest.spyOn(BrowserPopupUtils, "inPopout").mockReturnValueOnce(true);
jest.spyOn(BrowserPopupUtils, "inSingleActionPopout").mockReturnValueOnce(true);
const closeSpy = jest.spyOn(BrowserPopupUtils, "closeSingleActionPopout");
const focusSpy = jest
.spyOn(BrowserApi, "focusTab")
.mockImplementation(() => Promise.resolve());
params$.next({ action: AUTOFILL_ID, senderTabId: 99 });
flush(); // Resolve all promises
expect(doAutofill).toHaveBeenCalledOnce();
expect(focusSpy).toHaveBeenCalledWith(99);
expect(closeSpy).toHaveBeenCalledOnce();
}));
});
});

View File

@ -10,7 +10,13 @@ import { JslibModule } from "@bitwarden/angular/jslib.module";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { Organization } from "@bitwarden/common/admin-console/models/domain/organization";
import { AccountService } from "@bitwarden/common/auth/abstractions/account.service";
import { AUTOFILL_ID, SHOW_AUTOFILL_BUTTON } from "@bitwarden/common/autofill/constants";
import {
AUTOFILL_ID,
COPY_PASSWORD_ID,
COPY_USERNAME_ID,
COPY_VERIFICATION_CODE_ID,
SHOW_AUTOFILL_BUTTON,
} from "@bitwarden/common/autofill/constants";
import { EventType } from "@bitwarden/common/enums";
import { I18nService } from "@bitwarden/common/platform/abstractions/i18n.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
@ -18,7 +24,6 @@ import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.servi
import { ViewPasswordHistoryService } from "@bitwarden/common/vault/abstractions/view-password-history.service";
import { CipherType } from "@bitwarden/common/vault/enums";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { FolderView } from "@bitwarden/common/vault/models/view/folder.view";
import { CipherAuthorizationService } from "@bitwarden/common/vault/services/cipher-authorization.service";
import {
AsyncActionsModule,
@ -28,19 +33,34 @@ import {
SearchModule,
ToastService,
} from "@bitwarden/components";
import { CopyCipherFieldService } from "@bitwarden/vault";
import { PremiumUpgradePromptService } from "../../../../../../../../libs/common/src/vault/abstractions/premium-upgrade-prompt.service";
import { CipherViewComponent } from "../../../../../../../../libs/vault/src/cipher-view";
import { BrowserApi } from "../../../../../platform/browser/browser-api";
import BrowserPopupUtils from "../../../../../platform/popup/browser-popup-utils";
import { PopOutComponent } from "../../../../../platform/popup/components/pop-out.component";
import { PopupRouterCacheService } from "../../../../../platform/popup/view-cache/popup-router-cache.service";
import { BrowserPremiumUpgradePromptService } from "../../../services/browser-premium-upgrade-prompt.service";
import { BrowserViewPasswordHistoryService } from "../../../services/browser-view-password-history.service";
import { closeViewVaultItemPopout, VaultPopoutType } from "../../../utils/vault-popout-window";
import { PopupFooterComponent } from "./../../../../../platform/popup/layout/popup-footer.component";
import { PopupHeaderComponent } from "./../../../../../platform/popup/layout/popup-header.component";
import { PopupPageComponent } from "./../../../../../platform/popup/layout/popup-page.component";
import { VaultPopupAutofillService } from "./../../../services/vault-popup-autofill.service";
/**
* The types of actions that can be triggered when loading the view vault item popout via the
* extension ContextMenu. See context-menu-clicked-handler.ts for more information.
*/
type LoadAction =
| typeof AUTOFILL_ID
| typeof SHOW_AUTOFILL_BUTTON
| typeof COPY_USERNAME_ID
| typeof COPY_PASSWORD_ID
| typeof COPY_VERIFICATION_CODE_ID;
@Component({
selector: "app-view-v2",
templateUrl: "view-v2.component.html",
@ -68,10 +88,10 @@ export class ViewV2Component {
headerText: string;
cipher: CipherView;
organization$: Observable<Organization>;
folder$: Observable<FolderView>;
canDeleteCipher$: Observable<boolean>;
collections$: Observable<CollectionView[]>;
loadAction: typeof AUTOFILL_ID | typeof SHOW_AUTOFILL_BUTTON;
loadAction: LoadAction;
senderTabId?: number;
constructor(
private route: ActivatedRoute,
@ -86,6 +106,7 @@ export class ViewV2Component {
private eventCollectionService: EventCollectionService,
private popupRouterCacheService: PopupRouterCacheService,
protected cipherAuthorizationService: CipherAuthorizationService,
private copyCipherFieldService: CopyCipherFieldService,
) {
this.subscribeToParams();
}
@ -95,13 +116,15 @@ export class ViewV2Component {
.pipe(
switchMap(async (params): Promise<CipherView> => {
this.loadAction = params.action;
this.senderTabId = params.senderTabId ? parseInt(params.senderTabId, 10) : undefined;
return await this.getCipherData(params.cipherId);
}),
switchMap(async (cipher) => {
this.cipher = cipher;
this.headerText = this.setHeader(cipher.type);
if (this.loadAction === AUTOFILL_ID) {
await this.vaultPopupAutofillService.doAutofill(this.cipher);
if (this.loadAction) {
await this._handleLoadAction(this.loadAction, this.senderTabId);
}
this.canDeleteCipher$ = this.cipherAuthorizationService.canDeleteCipher$(cipher);
@ -211,4 +234,65 @@ export class ViewV2Component {
protected showFooter(): boolean {
return this.cipher && (!this.cipher.isDeleted || (this.cipher.isDeleted && this.cipher.edit));
}
/**
* Handles the load action for the view vault item popout. These actions are typically triggered
* via the extension context menu. It is necessary to render the view for items that have password
* reprompt enabled.
* @param loadAction
* @param senderTabId
* @private
*/
private async _handleLoadAction(loadAction: LoadAction, senderTabId?: number): Promise<void> {
let actionSuccess = false;
// Both vaultPopupAutofillService and copyCipherFieldService will perform password re-prompting internally.
switch (loadAction) {
case "show-autofill-button":
// This action simply shows the cipher view, no need to do anything.
return;
case "autofill":
actionSuccess = await this.vaultPopupAutofillService.doAutofill(this.cipher, false);
break;
case "copy-username":
actionSuccess = await this.copyCipherFieldService.copy(
this.cipher.login.username,
"username",
this.cipher,
);
break;
case "copy-password":
actionSuccess = await this.copyCipherFieldService.copy(
this.cipher.login.password,
"password",
this.cipher,
);
break;
case "copy-totp":
actionSuccess = await this.copyCipherFieldService.copy(
this.cipher.login.totp,
"totp",
this.cipher,
);
break;
}
if (BrowserPopupUtils.inPopout(window)) {
setTimeout(
async () => {
if (
BrowserPopupUtils.inSingleActionPopout(window, VaultPopoutType.viewVaultItem) &&
senderTabId
) {
await BrowserApi.focusTab(senderTabId);
await closeViewVaultItemPopout(`${VaultPopoutType.viewVaultItem}_${this.cipher.id}`);
} else {
await this.popupRouterCacheService.back();
}
},
actionSuccess ? 1000 : 0,
);
}
}
}

View File

@ -58,18 +58,21 @@ describe("CopyCipherFieldService", () => {
it("should return early when valueToCopy is null", async () => {
valueToCopy = null;
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeFalsy();
expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled();
});
it("should copy value to clipboard", async () => {
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith(valueToCopy);
});
it("should show a success toast on copy", async () => {
i18nService.t.mockReturnValueOnce("Username").mockReturnValueOnce("Username copied");
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(toastService.showToast).toHaveBeenCalledWith({
variant: "success",
message: "Username copied",
@ -87,26 +90,30 @@ describe("CopyCipherFieldService", () => {
it("should show password prompt when actionType requires it", async () => {
passwordRepromptService.showPasswordPrompt.mockResolvedValue(true);
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(passwordRepromptService.showPasswordPrompt).toHaveBeenCalled();
});
it("should skip password prompt when cipher.reprompt is 'None'", async () => {
cipher.reprompt = CipherRepromptType.None;
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(passwordRepromptService.showPasswordPrompt).not.toHaveBeenCalled();
expect(platformUtilsService.copyToClipboard).toHaveBeenCalled();
});
it("should skip password prompt when skipReprompt is true", async () => {
skipReprompt = true;
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(passwordRepromptService.showPasswordPrompt).not.toHaveBeenCalled();
});
it("should return early when password prompt is not confirmed", async () => {
passwordRepromptService.showPasswordPrompt.mockResolvedValue(false);
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeFalsy();
expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled();
});
});
@ -123,7 +130,8 @@ describe("CopyCipherFieldService", () => {
it("should get TOTP code when allowed from premium", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(true);
totpService.getCode.mockResolvedValue("123456");
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(totpService.getCode).toHaveBeenCalledWith(valueToCopy);
expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith("123456");
});
@ -131,21 +139,24 @@ describe("CopyCipherFieldService", () => {
it("should get TOTP code when allowed from organization", async () => {
cipher.organizationUseTotp = true;
totpService.getCode.mockResolvedValue("123456");
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeTruthy();
expect(totpService.getCode).toHaveBeenCalledWith(valueToCopy);
expect(platformUtilsService.copyToClipboard).toHaveBeenCalledWith("123456");
});
it("should return early when the user is not allowed to use TOTP", async () => {
billingAccountProfileStateService.hasPremiumFromAnySource$ = of(false);
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeFalsy();
expect(totpService.getCode).not.toHaveBeenCalled();
expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled();
});
it("should return early when TOTP is not set", async () => {
cipher.login.totp = null;
await service.copy(valueToCopy, actionType, cipher, skipReprompt);
const result = await service.copy(valueToCopy, actionType, cipher, skipReprompt);
expect(result).toBeFalsy();
expect(totpService.getCode).not.toHaveBeenCalled();
expect(platformUtilsService.copyToClipboard).not.toHaveBeenCalled();
});

View File

@ -95,13 +95,15 @@ export class CopyCipherFieldService {
* @param actionType The type of field being copied.
* @param cipher The cipher containing the field to copy.
* @param skipReprompt Whether to skip password re-prompting.
*
* @returns Whether the field was copied successfully.
*/
async copy(
valueToCopy: string,
actionType: CopyAction,
cipher: CipherView,
skipReprompt: boolean = false,
) {
): Promise<boolean> {
const action = CopyActions[actionType];
if (
!skipReprompt &&
@ -109,16 +111,16 @@ export class CopyCipherFieldService {
action.protected &&
!(await this.passwordRepromptService.showPasswordPrompt())
) {
return;
return false;
}
if (valueToCopy == null) {
return;
return false;
}
if (actionType === "totp") {
if (!(await this.totpAllowed(cipher))) {
return;
return false;
}
valueToCopy = await this.totpService.getCode(valueToCopy);
}
@ -138,6 +140,8 @@ export class CopyCipherFieldService {
cipher.organizationId,
);
}
return true;
}
/**