1
0
mirror of https://github.com/bitwarden/browser synced 2026-01-21 11:53:34 +00:00

[PM-29523] Remove ts strict ignore in browser context menu clicked handler (#18264)

* early return if no cipher before switch case

* explicit null checks within switch cases for early returns

* lower cipher check and add to explicit checks

* add test cases for null values

* format spec file
This commit is contained in:
Daniel Riera
2026-01-15 12:09:55 -05:00
committed by GitHub
parent 9a22907e27
commit 535b958f9e
2 changed files with 116 additions and 3 deletions

View File

@@ -4,8 +4,10 @@ import { of } from "rxjs";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import {
AUTOFILL_ID,
COPY_IDENTIFIER_ID,
COPY_PASSWORD_ID,
COPY_USERNAME_ID,
COPY_VERIFICATION_CODE_ID,
@@ -85,6 +87,7 @@ describe("ContextMenuClickedHandler", () => {
accountService = mockAccountServiceWith(mockUserId as UserId);
totpService = mock();
eventCollectionService = mock();
userVerificationService = mock();
sut = new ContextMenuClickedHandler(
copyToClipboard,
@@ -102,6 +105,93 @@ describe("ContextMenuClickedHandler", () => {
afterEach(() => jest.resetAllMocks());
describe("run", () => {
beforeEach(() => {
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked);
userVerificationService.hasMasterPasswordAndMasterKeyHash.mockResolvedValue(false);
});
const runWithUrl = (data: chrome.contextMenus.OnClickData) =>
sut.run(data, { url: "https://test.com" } as any);
describe("early returns", () => {
it.each([
{
name: "tab id is missing",
data: createData(COPY_IDENTIFIER_ID),
tab: { url: "https://test.com" } as any,
expectNotCalled: () => expect(copyToClipboard).not.toHaveBeenCalled(),
},
{
name: "tab url is missing",
data: createData(`${COPY_USERNAME_ID}_${NOOP_COMMAND_SUFFIX}`, COPY_USERNAME_ID),
tab: {} as any,
expectNotCalled: () => {
expect(cipherService.getAllDecryptedForUrl).not.toHaveBeenCalled();
expect(copyToClipboard).not.toHaveBeenCalled();
},
},
])("returns early when $name", async ({ data, tab, expectNotCalled }) => {
await expect(sut.run(data, tab)).resolves.toBeUndefined();
expectNotCalled();
});
});
describe("missing cipher", () => {
it.each([
{
label: "AUTOFILL",
parentId: AUTOFILL_ID,
extra: () => expect(autofill).not.toHaveBeenCalled(),
},
{ label: "username", parentId: COPY_USERNAME_ID, extra: () => {} },
{ label: "password", parentId: COPY_PASSWORD_ID, extra: () => {} },
{
label: "totp",
parentId: COPY_VERIFICATION_CODE_ID,
extra: () => expect(totpService.getCode$).not.toHaveBeenCalled(),
},
])("breaks silently when cipher is missing for $label", async ({ parentId, extra }) => {
cipherService.getAllDecrypted.mockResolvedValue([]);
await expect(runWithUrl(createData(`${parentId}_1`, parentId))).resolves.toBeUndefined();
expect(copyToClipboard).not.toHaveBeenCalled();
extra();
});
});
describe("missing login properties", () => {
it.each([
{
label: "username",
parentId: COPY_USERNAME_ID,
unset: (c: CipherView): void => (c.login.username = undefined),
},
{
label: "password",
parentId: COPY_PASSWORD_ID,
unset: (c: CipherView): void => (c.login.password = undefined),
},
{
label: "totp",
parentId: COPY_VERIFICATION_CODE_ID,
unset: (c: CipherView): void => (c.login.totp = undefined),
isTotp: true,
},
])("breaks silently when $label property is missing", async ({ parentId, unset, isTotp }) => {
const cipher = createCipher();
unset(cipher);
cipherService.getAllDecrypted.mockResolvedValue([cipher]);
await expect(runWithUrl(createData(`${parentId}_1`, parentId))).resolves.toBeUndefined();
expect(copyToClipboard).not.toHaveBeenCalled();
if (isTotp) {
expect(totpService.getCode$).not.toHaveBeenCalled();
}
});
});
it("can generate password", async () => {
await sut.run(createData(GENERATE_PASSWORD_ID), { id: 5 } as any);

View File

@@ -1,5 +1,3 @@
// FIXME: Update this file to be type safe and remove this and next line
// @ts-strict-ignore
import { firstValueFrom } from "rxjs";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
@@ -72,6 +70,10 @@ export class ContextMenuClickedHandler {
await this.generatePasswordToClipboard(tab);
break;
case COPY_IDENTIFIER_ID:
if (!tab.id) {
return;
}
this.copyToClipboard({ text: await this.getIdentifier(tab, info), tab: tab });
break;
default:
@@ -120,6 +122,10 @@ export class ContextMenuClickedHandler {
if (isCreateCipherAction) {
// pass; defer to logic below
} else if (menuItemId === NOOP_COMMAND_SUFFIX) {
if (!tab.url) {
return;
}
const additionalCiphersToGet =
info.parentMenuItemId === AUTOFILL_IDENTITY_ID
? [CipherType.Identity]
@@ -158,6 +164,10 @@ export class ContextMenuClickedHandler {
break;
}
if (!cipher) {
break;
}
if (await this.isPasswordRepromptRequired(cipher)) {
await openVaultItemPasswordRepromptPopout(tab, {
cipherId: cipher.id,
@@ -176,6 +186,10 @@ export class ContextMenuClickedHandler {
break;
}
if (!cipher || !cipher.login?.username) {
break;
}
this.copyToClipboard({ text: cipher.login.username, tab: tab });
break;
case COPY_PASSWORD_ID:
@@ -184,6 +198,10 @@ export class ContextMenuClickedHandler {
break;
}
if (!cipher || !cipher.login?.password) {
break;
}
if (await this.isPasswordRepromptRequired(cipher)) {
await openVaultItemPasswordRepromptPopout(tab, {
cipherId: cipher.id,
@@ -205,6 +223,10 @@ export class ContextMenuClickedHandler {
break;
}
if (!cipher || !cipher.login?.totp) {
break;
}
if (await this.isPasswordRepromptRequired(cipher)) {
await openVaultItemPasswordRepromptPopout(tab, {
cipherId: cipher.id,
@@ -240,9 +262,10 @@ export class ContextMenuClickedHandler {
}
private async getIdentifier(tab: chrome.tabs.Tab, info: chrome.contextMenus.OnClickData) {
const tabId = tab.id!;
return new Promise<string>((resolve, reject) => {
BrowserApi.sendTabsMessage(
tab.id,
tabId,
{ command: "getClickedElement" },
{ frameId: info.frameId },
(identifier: string) => {