[PM-3589] Context Menu No Longer Shows Autofill Ciphers (#6085)

* [PM-3589] Context Menu No Longer Shows Autofill Ciphers

* [PM-3589] Ensuring that passwordless users can also access ciphers that require reprompt

* [PM-3589] Fixing jest test

* [PM-3589] Fixing issue where context menu autofill does not allow filling when passwordless setup is in place
This commit is contained in:
Cesar Gonzalez 2023-08-30 10:18:20 -05:00 committed by GitHub
parent 9288367bc8
commit b444eed0b5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 32 additions and 81 deletions

View File

@ -1,7 +1,6 @@
import { mock, MockProxy } from "jest-mock-extended";
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 { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
@ -14,7 +13,6 @@ describe("CipherContextMenuHandler", () => {
let mainContextMenuHandler: MockProxy<MainContextMenuHandler>;
let authService: MockProxy<AuthService>;
let cipherService: MockProxy<CipherService>;
let userVerificationService: MockProxy<UserVerificationService>;
let sut: CipherContextMenuHandler;
@ -22,17 +20,10 @@ describe("CipherContextMenuHandler", () => {
mainContextMenuHandler = mock();
authService = mock();
cipherService = mock();
userVerificationService = mock();
userVerificationService.hasMasterPassword.mockResolvedValue(true);
jest.spyOn(MainContextMenuHandler, "removeAll").mockResolvedValue();
sut = new CipherContextMenuHandler(
mainContextMenuHandler,
authService,
cipherService,
userVerificationService
);
sut = new CipherContextMenuHandler(mainContextMenuHandler, authService, cipherService);
});
afterEach(() => jest.resetAllMocks());
@ -78,7 +69,7 @@ describe("CipherContextMenuHandler", () => {
expect(mainContextMenuHandler.noLogins).toHaveBeenCalledTimes(1);
});
it("only adds valid ciphers", async () => {
it("only adds login ciphers including ciphers that require reprompt", async () => {
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked);
mainContextMenuHandler.init.mockResolvedValue(true);
@ -90,47 +81,6 @@ describe("CipherContextMenuHandler", () => {
name: "Test Cipher",
login: { username: "Test Username" },
};
cipherService.getAllDecryptedForUrl.mockResolvedValue([
null, // invalid cipher
undefined, // invalid cipher
{ type: CipherType.Card }, // invalid cipher
{ type: CipherType.Login, reprompt: CipherRepromptType.Password }, // invalid cipher
realCipher, // valid cipher
] as any[]);
await sut.update("https://test.com");
expect(cipherService.getAllDecryptedForUrl).toHaveBeenCalledTimes(1);
expect(cipherService.getAllDecryptedForUrl).toHaveBeenCalledWith("https://test.com");
expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledTimes(2);
expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledWith(
"Test Cipher (Test Username)",
"5",
"https://test.com",
realCipher
);
});
it("adds ciphers with master password reprompt if the user does not have a master password", async () => {
authService.getAuthStatus.mockResolvedValue(AuthenticationStatus.Unlocked);
// User does not have a master password, or has one but hasn't logged in with it (key connector user or TDE user)
userVerificationService.hasMasterPasswordAndMasterKeyHash.mockResolvedValue(false);
mainContextMenuHandler.init.mockResolvedValue(true);
const realCipher = {
id: "5",
type: CipherType.Login,
reprompt: CipherRepromptType.None,
name: "Test Cipher",
login: { username: "Test Username" },
};
const repromptCipher = {
id: "6",
type: CipherType.Login,
@ -143,8 +93,8 @@ describe("CipherContextMenuHandler", () => {
null, // invalid cipher
undefined, // invalid cipher
{ type: CipherType.Card }, // invalid cipher
repromptCipher, // valid cipher
realCipher, // valid cipher
repromptCipher,
] as any[]);
await sut.update("https://test.com");
@ -153,7 +103,6 @@ describe("CipherContextMenuHandler", () => {
expect(cipherService.getAllDecryptedForUrl).toHaveBeenCalledWith("https://test.com");
// Should call this twice, once for each valid cipher
expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledTimes(2);
expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledWith(

View File

@ -1,5 +1,4 @@
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 { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
import { Utils } from "@bitwarden/common/platform/misc/utils";
@ -12,7 +11,6 @@ import {
authServiceFactory,
AuthServiceInitOptions,
} from "../../auth/background/service-factories/auth-service.factory";
import { userVerificationServiceFactory } from "../../auth/background/service-factories/user-verification-service.factory";
import { Account } from "../../models/account";
import { CachedServices } from "../../platform/background/service-factories/factory-options";
import { BrowserApi } from "../../platform/browser/browser-api";
@ -39,8 +37,7 @@ export class CipherContextMenuHandler {
constructor(
private mainContextMenuHandler: MainContextMenuHandler,
private authService: AuthService,
private cipherService: CipherService,
private userVerificationService: UserVerificationService
private cipherService: CipherService
) {}
static async create(cachedServices: CachedServices) {
@ -79,8 +76,7 @@ export class CipherContextMenuHandler {
return new CipherContextMenuHandler(
await MainContextMenuHandler.mv3Create(cachedServices),
await authServiceFactory(cachedServices, serviceOptions),
await cipherServiceFactory(cachedServices, serviceOptions),
await userVerificationServiceFactory(cachedServices, serviceOptions)
await cipherServiceFactory(cachedServices, serviceOptions)
);
}
@ -180,11 +176,7 @@ export class CipherContextMenuHandler {
}
private async updateForCipher(url: string, cipher: CipherView) {
if (
cipher == null ||
cipher.type !== CipherType.Login ||
(await this.userVerificationService.hasMasterPasswordAndMasterKeyHash())
) {
if (cipher == null || cipher.type !== CipherType.Login) {
return;
}

View File

@ -3,6 +3,7 @@ import { mock, MockProxy } from "jest-mock-extended";
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { TotpService } from "@bitwarden/common/abstractions/totp.service";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
import { CipherRepromptType } from "@bitwarden/common/vault/enums/cipher-reprompt-type";
import { CipherType } from "@bitwarden/common/vault/enums/cipher-type";
@ -63,6 +64,7 @@ describe("ContextMenuClickedHandler", () => {
let cipherService: MockProxy<CipherService>;
let totpService: MockProxy<TotpService>;
let eventCollectionService: MockProxy<EventCollectionService>;
let userVerificationService: MockProxy<UserVerificationService>;
let sut: ContextMenuClickedHandler;
@ -82,7 +84,8 @@ describe("ContextMenuClickedHandler", () => {
authService,
cipherService,
totpService,
eventCollectionService
eventCollectionService,
userVerificationService
);
});

View File

@ -1,6 +1,7 @@
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
import { TotpService } from "@bitwarden/common/abstractions/totp.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 { EventType } from "@bitwarden/common/enums";
import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
@ -14,6 +15,7 @@ import {
AuthServiceInitOptions,
} from "../../auth/background/service-factories/auth-service.factory";
import { totpServiceFactory } from "../../auth/background/service-factories/totp-service.factory";
import { userVerificationServiceFactory } from "../../auth/background/service-factories/user-verification-service.factory";
import LockedVaultPendingNotificationsItem from "../../background/models/lockedVaultPendingNotificationsItem";
import { eventCollectionServiceFactory } from "../../background/service-factories/event-collection-service.factory";
import { Account } from "../../models/account";
@ -56,7 +58,8 @@ export class ContextMenuClickedHandler {
private authService: AuthService,
private cipherService: CipherService,
private totpService: TotpService,
private eventCollectionService: EventCollectionService
private eventCollectionService: EventCollectionService,
private userVerificationService: UserVerificationService
) {}
static async mv3Create(cachedServices: CachedServices) {
@ -109,7 +112,8 @@ export class ContextMenuClickedHandler {
await authServiceFactory(cachedServices, serviceOptions),
await cipherServiceFactory(cachedServices, serviceOptions),
await totpServiceFactory(cachedServices, serviceOptions),
await eventCollectionServiceFactory(cachedServices, serviceOptions)
await eventCollectionServiceFactory(cachedServices, serviceOptions),
await userVerificationServiceFactory(cachedServices, serviceOptions)
);
}
@ -204,7 +208,7 @@ export class ContextMenuClickedHandler {
return;
}
if (cipher.reprompt !== CipherRepromptType.None) {
if (await this.isPasswordRepromptRequired(cipher)) {
await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
cipherId: cipher.id,
action: AUTOFILL_ID,
@ -218,7 +222,7 @@ export class ContextMenuClickedHandler {
this.copyToClipboard({ text: cipher.login.username, tab: tab });
break;
case COPY_PASSWORD_ID:
if (cipher.reprompt !== CipherRepromptType.None) {
if (await this.isPasswordRepromptRequired(cipher)) {
await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
cipherId: cipher.id,
action: COPY_PASSWORD_ID,
@ -230,7 +234,7 @@ export class ContextMenuClickedHandler {
break;
case COPY_VERIFICATIONCODE_ID:
if (cipher.reprompt !== CipherRepromptType.None) {
if (await this.isPasswordRepromptRequired(cipher)) {
await BrowserApi.tabSendMessageData(tab, "passwordReprompt", {
cipherId: cipher.id,
action: COPY_VERIFICATIONCODE_ID,
@ -246,6 +250,13 @@ export class ContextMenuClickedHandler {
}
}
private async isPasswordRepromptRequired(cipher: CipherView): Promise<boolean> {
return (
cipher.reprompt === CipherRepromptType.Password &&
(await this.userVerificationService.hasMasterPasswordAndMasterKeyHash())
);
}
private async getIdentifier(tab: chrome.tabs.Tab, info: chrome.contextMenus.OnClickData) {
return new Promise<string>((resolve, reject) => {
BrowserApi.sendTabsMessage(

View File

@ -635,7 +635,8 @@ export default class MainBackground {
this.authService,
this.cipherService,
this.totpService,
this.eventCollectionService
this.eventCollectionService,
this.userVerificationService
);
this.contextMenusBackground = new ContextMenusBackground(contextMenuClickedHandler);
@ -670,8 +671,7 @@ export default class MainBackground {
this.cipherContextMenuHandler = new CipherContextMenuHandler(
this.mainContextMenuHandler,
this.authService,
this.cipherService,
this.userVerificationService
this.cipherService
);
}
}

View File

@ -170,8 +170,8 @@ export class ViewComponent extends BaseViewComponent {
switch (this.loadAction) {
case AUTOFILL_ID:
this.fillCipher();
return;
await this.fillCipher();
break;
case COPY_USERNAME_ID:
await this.copy(this.cipher.login.username, "username", "Username");
break;
@ -186,7 +186,7 @@ export class ViewComponent extends BaseViewComponent {
}
if (this.inPopout && this.loadAction) {
this.close();
setTimeout(() => this.close(), 1000);
}
}
@ -238,10 +238,6 @@ export class ViewComponent extends BaseViewComponent {
const didAutofill = await this.doAutofill();
if (didAutofill) {
this.platformUtilsService.showToast("success", null, this.i18nService.t("autoFillSuccess"));
if (this.inPopout) {
this.close();
}
}
}