mirror of
https://github.com/bitwarden/browser.git
synced 2025-01-07 19:07:45 +01:00
[PM-3101] Fix autofill items not working for users without a master password (#5885)
* Add service factories for user verification services * Update autofill service to check for existence of master password for autofill * Update the context menu to check for existence of master password for autofill * context menu test fixes
This commit is contained in:
parent
7977776b1c
commit
81320a0d6d
@ -0,0 +1,29 @@
|
||||
import { UserVerificationApiServiceAbstraction } from "@bitwarden/common/auth/abstractions/user-verification/user-verification-api.service.abstraction";
|
||||
import { UserVerificationApiService } from "@bitwarden/common/auth/services/user-verification/user-verification-api.service";
|
||||
|
||||
import {
|
||||
ApiServiceInitOptions,
|
||||
apiServiceFactory,
|
||||
} from "../../../platform/background/service-factories/api-service.factory";
|
||||
import {
|
||||
FactoryOptions,
|
||||
CachedServices,
|
||||
factory,
|
||||
} from "../../../platform/background/service-factories/factory-options";
|
||||
|
||||
type UserVerificationApiServiceFactoryOptions = FactoryOptions;
|
||||
|
||||
export type UserVerificationApiServiceInitOptions = UserVerificationApiServiceFactoryOptions &
|
||||
ApiServiceInitOptions;
|
||||
|
||||
export function userVerificationApiServiceFactory(
|
||||
cache: { userVerificationApiService?: UserVerificationApiServiceAbstraction } & CachedServices,
|
||||
opts: UserVerificationApiServiceInitOptions
|
||||
): Promise<UserVerificationApiServiceAbstraction> {
|
||||
return factory(
|
||||
cache,
|
||||
"userVerificationApiService",
|
||||
opts,
|
||||
async () => new UserVerificationApiService(await apiServiceFactory(cache, opts))
|
||||
);
|
||||
}
|
@ -0,0 +1,51 @@
|
||||
import { UserVerificationService as AbstractUserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
|
||||
import { UserVerificationService } from "@bitwarden/common/auth/services/user-verification/user-verification.service";
|
||||
|
||||
import {
|
||||
CryptoServiceInitOptions,
|
||||
cryptoServiceFactory,
|
||||
} from "../../../platform/background/service-factories/crypto-service.factory";
|
||||
import {
|
||||
FactoryOptions,
|
||||
CachedServices,
|
||||
factory,
|
||||
} from "../../../platform/background/service-factories/factory-options";
|
||||
import {
|
||||
I18nServiceInitOptions,
|
||||
i18nServiceFactory,
|
||||
} from "../../../platform/background/service-factories/i18n-service.factory";
|
||||
import {
|
||||
StateServiceInitOptions,
|
||||
stateServiceFactory,
|
||||
} from "../../../platform/background/service-factories/state-service.factory";
|
||||
|
||||
import {
|
||||
UserVerificationApiServiceInitOptions,
|
||||
userVerificationApiServiceFactory,
|
||||
} from "./user-verification-api-service.factory";
|
||||
|
||||
type UserVerificationServiceFactoryOptions = FactoryOptions;
|
||||
|
||||
export type UserVerificationServiceInitOptions = UserVerificationServiceFactoryOptions &
|
||||
StateServiceInitOptions &
|
||||
CryptoServiceInitOptions &
|
||||
I18nServiceInitOptions &
|
||||
UserVerificationApiServiceInitOptions;
|
||||
|
||||
export function userVerificationServiceFactory(
|
||||
cache: { userVerificationService?: AbstractUserVerificationService } & CachedServices,
|
||||
opts: UserVerificationServiceInitOptions
|
||||
): Promise<AbstractUserVerificationService> {
|
||||
return factory(
|
||||
cache,
|
||||
"userVerificationService",
|
||||
opts,
|
||||
async () =>
|
||||
new UserVerificationService(
|
||||
await stateServiceFactory(cache, opts),
|
||||
await cryptoServiceFactory(cache, opts),
|
||||
await i18nServiceFactory(cache, opts),
|
||||
await userVerificationApiServiceFactory(cache, opts)
|
||||
)
|
||||
);
|
||||
}
|
@ -2,6 +2,10 @@ import {
|
||||
TotpServiceInitOptions,
|
||||
totpServiceFactory,
|
||||
} from "../../../auth/background/service-factories/totp-service.factory";
|
||||
import {
|
||||
UserVerificationServiceInitOptions,
|
||||
userVerificationServiceFactory,
|
||||
} from "../../../auth/background/service-factories/user-verification-service.factory";
|
||||
import {
|
||||
EventCollectionServiceInitOptions,
|
||||
eventCollectionServiceFactory,
|
||||
@ -38,7 +42,8 @@ export type AutoFillServiceInitOptions = AutoFillServiceOptions &
|
||||
TotpServiceInitOptions &
|
||||
EventCollectionServiceInitOptions &
|
||||
LogServiceInitOptions &
|
||||
SettingsServiceInitOptions;
|
||||
SettingsServiceInitOptions &
|
||||
UserVerificationServiceInitOptions;
|
||||
|
||||
export function autofillServiceFactory(
|
||||
cache: { autofillService?: AbstractAutoFillService } & CachedServices,
|
||||
@ -55,7 +60,8 @@ export function autofillServiceFactory(
|
||||
await totpServiceFactory(cache, opts),
|
||||
await eventCollectionServiceFactory(cache, opts),
|
||||
await logServiceFactory(cache, opts),
|
||||
await settingsServiceFactory(cache, opts)
|
||||
await settingsServiceFactory(cache, opts),
|
||||
await userVerificationServiceFactory(cache, opts)
|
||||
)
|
||||
);
|
||||
}
|
||||
|
@ -1,6 +1,7 @@
|
||||
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";
|
||||
@ -13,6 +14,7 @@ describe("CipherContextMenuHandler", () => {
|
||||
let mainContextMenuHandler: MockProxy<MainContextMenuHandler>;
|
||||
let authService: MockProxy<AuthService>;
|
||||
let cipherService: MockProxy<CipherService>;
|
||||
let userVerificationService: MockProxy<UserVerificationService>;
|
||||
|
||||
let sut: CipherContextMenuHandler;
|
||||
|
||||
@ -20,10 +22,17 @@ 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);
|
||||
sut = new CipherContextMenuHandler(
|
||||
mainContextMenuHandler,
|
||||
authService,
|
||||
cipherService,
|
||||
userVerificationService
|
||||
);
|
||||
});
|
||||
|
||||
afterEach(() => jest.resetAllMocks());
|
||||
@ -83,11 +92,11 @@ describe("CipherContextMenuHandler", () => {
|
||||
};
|
||||
|
||||
cipherService.getAllDecryptedForUrl.mockResolvedValue([
|
||||
null,
|
||||
undefined,
|
||||
{ type: CipherType.Card },
|
||||
{ type: CipherType.Login, reprompt: CipherRepromptType.Password },
|
||||
realCipher,
|
||||
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");
|
||||
@ -105,5 +114,61 @@ describe("CipherContextMenuHandler", () => {
|
||||
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 (key connector user or TDE user)
|
||||
userVerificationService.hasMasterPassword.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,
|
||||
reprompt: CipherRepromptType.Password,
|
||||
name: "Test Reprompt Cipher",
|
||||
login: { username: "Test Username" },
|
||||
};
|
||||
|
||||
cipherService.getAllDecryptedForUrl.mockResolvedValue([
|
||||
null, // invalid cipher
|
||||
undefined, // invalid cipher
|
||||
{ type: CipherType.Card }, // invalid cipher
|
||||
repromptCipher, // valid cipher
|
||||
realCipher, // valid cipher
|
||||
] as any[]);
|
||||
|
||||
await sut.update("https://test.com");
|
||||
|
||||
expect(cipherService.getAllDecryptedForUrl).toHaveBeenCalledTimes(1);
|
||||
|
||||
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(
|
||||
"Test Cipher (Test Username)",
|
||||
"5",
|
||||
"https://test.com",
|
||||
realCipher
|
||||
);
|
||||
|
||||
expect(mainContextMenuHandler.loadOptions).toHaveBeenCalledWith(
|
||||
"Test Reprompt Cipher (Test Username)",
|
||||
"6",
|
||||
"https://test.com",
|
||||
repromptCipher
|
||||
);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
@ -1,4 +1,5 @@
|
||||
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,6 +13,7 @@ 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";
|
||||
@ -38,7 +40,8 @@ export class CipherContextMenuHandler {
|
||||
constructor(
|
||||
private mainContextMenuHandler: MainContextMenuHandler,
|
||||
private authService: AuthService,
|
||||
private cipherService: CipherService
|
||||
private cipherService: CipherService,
|
||||
private userVerificationService: UserVerificationService
|
||||
) {}
|
||||
|
||||
static async create(cachedServices: CachedServices) {
|
||||
@ -77,7 +80,8 @@ export class CipherContextMenuHandler {
|
||||
return new CipherContextMenuHandler(
|
||||
await MainContextMenuHandler.mv3Create(cachedServices),
|
||||
await authServiceFactory(cachedServices, serviceOptions),
|
||||
await cipherServiceFactory(cachedServices, serviceOptions)
|
||||
await cipherServiceFactory(cachedServices, serviceOptions),
|
||||
await userVerificationServiceFactory(cachedServices, serviceOptions)
|
||||
);
|
||||
}
|
||||
|
||||
@ -174,7 +178,8 @@ export class CipherContextMenuHandler {
|
||||
if (
|
||||
cipher == null ||
|
||||
cipher.type !== CipherType.Login ||
|
||||
cipher.reprompt !== CipherRepromptType.None
|
||||
(cipher.reprompt !== CipherRepromptType.None &&
|
||||
(await this.userVerificationService.hasMasterPassword()))
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
@ -1,6 +1,7 @@
|
||||
import { EventCollectionService } from "@bitwarden/common/abstractions/event/event-collection.service";
|
||||
import { SettingsService } from "@bitwarden/common/abstractions/settings.service";
|
||||
import { TotpService } from "@bitwarden/common/abstractions/totp.service";
|
||||
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
|
||||
import { EventType, FieldType, UriMatchType } from "@bitwarden/common/enums";
|
||||
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
|
||||
import { CipherService } from "@bitwarden/common/vault/abstractions/cipher.service";
|
||||
@ -45,7 +46,8 @@ export default class AutofillService implements AutofillServiceInterface {
|
||||
private totpService: TotpService,
|
||||
private eventCollectionService: EventCollectionService,
|
||||
private logService: LogService,
|
||||
private settingsService: SettingsService
|
||||
private settingsService: SettingsService,
|
||||
private userVerificationService: UserVerificationService
|
||||
) {}
|
||||
|
||||
getFormsWithPasswordFields(pageDetails: AutofillPageDetails): FormData[] {
|
||||
@ -234,7 +236,11 @@ export default class AutofillService implements AutofillServiceInterface {
|
||||
}
|
||||
}
|
||||
|
||||
if (cipher == null || cipher.reprompt !== CipherRepromptType.None) {
|
||||
if (
|
||||
cipher == null ||
|
||||
(cipher.reprompt !== CipherRepromptType.None &&
|
||||
(await this.userVerificationService.hasMasterPassword()))
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
|
@ -495,13 +495,24 @@ export default class MainBackground {
|
||||
this.eventUploadService
|
||||
);
|
||||
this.totpService = new TotpService(this.cryptoFunctionService, this.logService);
|
||||
|
||||
this.userVerificationApiService = new UserVerificationApiService(this.apiService);
|
||||
|
||||
this.userVerificationService = new UserVerificationService(
|
||||
this.stateService,
|
||||
this.cryptoService,
|
||||
this.i18nService,
|
||||
this.userVerificationApiService
|
||||
);
|
||||
|
||||
this.autofillService = new AutofillService(
|
||||
this.cipherService,
|
||||
this.stateService,
|
||||
this.totpService,
|
||||
this.eventCollectionService,
|
||||
this.logService,
|
||||
this.settingsService
|
||||
this.settingsService,
|
||||
this.userVerificationService
|
||||
);
|
||||
this.auditService = new AuditService(this.cryptoFunctionService, this.apiService);
|
||||
this.exportService = new VaultExportService(
|
||||
@ -523,16 +534,6 @@ export default class MainBackground {
|
||||
this.authService,
|
||||
this.messagingService
|
||||
);
|
||||
|
||||
this.userVerificationApiService = new UserVerificationApiService(this.apiService);
|
||||
|
||||
this.userVerificationService = new UserVerificationService(
|
||||
this.stateService,
|
||||
this.cryptoService,
|
||||
this.i18nService,
|
||||
this.userVerificationApiService
|
||||
);
|
||||
|
||||
this.configService = new ConfigService(
|
||||
this.stateService,
|
||||
this.configApiService,
|
||||
@ -660,7 +661,8 @@ export default class MainBackground {
|
||||
this.cipherContextMenuHandler = new CipherContextMenuHandler(
|
||||
this.mainContextMenuHandler,
|
||||
this.authService,
|
||||
this.cipherService
|
||||
this.cipherService,
|
||||
this.userVerificationService
|
||||
);
|
||||
}
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user