Skip to content

Commit

Permalink
[PM-5559] Implement User Notification Settings state provider (#8032)
Browse files Browse the repository at this point in the history 
* create user notification settings state provider

* replace state service get/set disableAddLoginNotification and disableChangedPasswordNotification with user notification settings service equivalents

* migrate disableAddLoginNotification and disableChangedPasswordNotification global settings to user notification settings state provider

* add content script messaging the background for enableChangedPasswordPrompt setting

* Implementing feedback to provide on PR

* Implementing feedback to provide on PR

* PR suggestions cleanup

---------

Co-authored-by: Cesar Gonzalez <cgonzalez@bitwarden.com>
  • Loading branch information
@jprusik
jprusik and Cesar Gonzalez authored Mar 4, 2024
1 parent d87a8f9 commit 4ba2717
Show file tree
Hide file tree
Showing 18 changed files with 409 additions and 138 deletions.
2 changes: 2 additions & 0 deletions apps/browser/src/autofill/background/abstractions/notification.background.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,8 @@ type NotificationBackgroundExtensionMessageHandlers = {
bgReopenUnlockPopout: ({ sender }: BackgroundSenderParam) => Promise<void>;
checkNotificationQueue: ({ sender }: BackgroundSenderParam) => Promise<void>;
collectPageDetailsResponse: ({ message }: BackgroundMessageParam) => Promise<void>;
bgGetEnableChangedPasswordPrompt: () => Promise<boolean>;
bgGetEnableAddedLoginPrompt: () => Promise<boolean>;
getWebVaultUrlForNotification: () => string;
};

Expand Down
50 changes: 27 additions & 23 deletions apps/browser/src/autofill/background/notification.background.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { firstValueFrom } from "rxjs";
import { PolicyService } from "@bitwarden/common/admin-console/services/policy/policy.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { AuthService } from "@bitwarden/common/auth/services/auth.service";
import { UserNotificationSettingsService } from "@bitwarden/common/autofill/services/user-notification-settings.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { EnvironmentService } from "@bitwarden/common/platform/services/environment.service";
import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
Expand Down Expand Up @@ -45,6 +46,7 @@ describe("NotificationBackground", () => {
const policyService = mock<PolicyService>();
const folderService = mock<FolderService>();
const stateService = mock<BrowserStateService>();
const userNotificationSettingsService = mock<UserNotificationSettingsService>();
const environmentService = mock<EnvironmentService>();
const logService = mock<LogService>();

Expand All @@ -56,6 +58,7 @@ describe("NotificationBackground", () => {
policyService,
folderService,
stateService,
userNotificationSettingsService,
environmentService,
logService,
);
Expand Down Expand Up @@ -235,8 +238,8 @@ describe("NotificationBackground", () => {
let tab: chrome.tabs.Tab;
let sender: chrome.runtime.MessageSender;
let getAuthStatusSpy: jest.SpyInstance;
let getDisableAddLoginNotificationSpy: jest.SpyInstance;
let getDisableChangedPasswordNotificationSpy: jest.SpyInstance;
let getEnableAddedLoginPromptSpy: jest.SpyInstance;
let getEnableChangedPasswordPromptSpy: jest.SpyInstance;
let pushAddLoginToQueueSpy: jest.SpyInstance;
let pushChangePasswordToQueueSpy: jest.SpyInstance;
let getAllDecryptedForUrlSpy: jest.SpyInstance;
Expand All @@ -245,13 +248,13 @@ describe("NotificationBackground", () => {
tab = createChromeTabMock();
sender = mock<chrome.runtime.MessageSender>({ tab });
getAuthStatusSpy = jest.spyOn(authService, "getAuthStatus");
getDisableAddLoginNotificationSpy = jest.spyOn(
stateService,
"getDisableAddLoginNotification",
getEnableAddedLoginPromptSpy = jest.spyOn(
notificationBackground as any,
"getEnableAddedLoginPrompt",
);
getDisableChangedPasswordNotificationSpy = jest.spyOn(
stateService,
"getDisableChangedPasswordNotification",
getEnableChangedPasswordPromptSpy = jest.spyOn(
notificationBackground as any,
"getEnableChangedPasswordPrompt",
);
pushAddLoginToQueueSpy = jest.spyOn(notificationBackground as any, "pushAddLoginToQueue");
pushChangePasswordToQueueSpy = jest.spyOn(
Expand All @@ -272,7 +275,7 @@ describe("NotificationBackground", () => {
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).not.toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
});

Expand All @@ -287,7 +290,7 @@ describe("NotificationBackground", () => {
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).not.toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).not.toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
});

Expand All @@ -297,13 +300,13 @@ describe("NotificationBackground", () => {
login: { username: "test", password: "password", url: "https://example.com" },
};
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(true);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(false);

sendExtensionRuntimeMessage(message, sender);
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled();
expect(getAllDecryptedForUrlSpy).not.toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled();
Expand All @@ -315,14 +318,14 @@ describe("NotificationBackground", () => {
login: { username: "test", password: "password", url: "https://example.com" },
};
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(true);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(false);
getAllDecryptedForUrlSpy.mockResolvedValueOnce([]);

sendExtensionRuntimeMessage(message, sender);
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled();
expect(getAllDecryptedForUrlSpy).toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled();
Expand All @@ -334,8 +337,8 @@ describe("NotificationBackground", () => {
login: { username: "test", password: "password", url: "https://example.com" },
};
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(false);
getDisableChangedPasswordNotificationSpy.mockReturnValueOnce(true);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(true);
getEnableChangedPasswordPromptSpy.mockReturnValueOnce(false);
getAllDecryptedForUrlSpy.mockResolvedValueOnce([
mock<CipherView>({ login: { username: "test", password: "oldPassword" } }),
]);
Expand All @@ -344,9 +347,9 @@ describe("NotificationBackground", () => {
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled();
expect(getAllDecryptedForUrlSpy).toHaveBeenCalled();
expect(getDisableChangedPasswordNotificationSpy).toHaveBeenCalled();
expect(getEnableChangedPasswordPromptSpy).toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled();
});
Expand All @@ -357,7 +360,7 @@ describe("NotificationBackground", () => {
login: { username: "test", password: "password", url: "https://example.com" },
};
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(false);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(true);
getAllDecryptedForUrlSpy.mockResolvedValueOnce([
mock<CipherView>({ login: { username: "test", password: "password" } }),
]);
Expand All @@ -366,7 +369,7 @@ describe("NotificationBackground", () => {
await flushPromises();

expect(getAuthStatusSpy).toHaveBeenCalled();
expect(getDisableAddLoginNotificationSpy).toHaveBeenCalled();
expect(getEnableAddedLoginPromptSpy).toHaveBeenCalled();
expect(getAllDecryptedForUrlSpy).toHaveBeenCalled();
expect(pushAddLoginToQueueSpy).not.toHaveBeenCalled();
expect(pushChangePasswordToQueueSpy).not.toHaveBeenCalled();
Expand All @@ -376,7 +379,7 @@ describe("NotificationBackground", () => {
const login = { username: "test", password: "password", url: "https://example.com" };
const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login };
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Locked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(false);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(true);

sendExtensionRuntimeMessage(message, sender);
await flushPromises();
Expand All @@ -393,7 +396,7 @@ describe("NotificationBackground", () => {
} as any;
const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login };
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(false);
getEnableAddedLoginPromptSpy.mockReturnValueOnce(true);
getAllDecryptedForUrlSpy.mockResolvedValueOnce([
mock<CipherView>({ login: { username: "anotherTestUsername", password: "password" } }),
]);
Expand All @@ -409,7 +412,8 @@ describe("NotificationBackground", () => {
const login = { username: "tEsT", password: "password", url: "https://example.com" };
const message: NotificationBackgroundExtensionMessage = { command: "bgAddLogin", login };
getAuthStatusSpy.mockResolvedValueOnce(AuthenticationStatus.Unlocked);
getDisableAddLoginNotificationSpy.mockReturnValueOnce(false);
getEnableAddedLoginPromptSpy.mockResolvedValueOnce(true);
getEnableChangedPasswordPromptSpy.mockResolvedValueOnce(true);
getAllDecryptedForUrlSpy.mockResolvedValueOnce([
mock<CipherView>({
id: "cipher-id",
Expand Down
30 changes: 25 additions & 5 deletions apps/browser/src/autofill/background/notification.background.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { PolicyService } from "@bitwarden/common/admin-console/abstractions/poli
import { PolicyType } from "@bitwarden/common/admin-console/enums";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service";
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { Utils } from "@bitwarden/common/platform/misc/utils";
Expand Down Expand Up @@ -57,6 +58,8 @@ export default class NotificationBackground {
bgUnlockPopoutOpened: ({ message, sender }) => this.unlockVault(message, sender.tab),
checkNotificationQueue: ({ sender }) => this.checkNotificationQueue(sender.tab),
bgReopenUnlockPopout: ({ sender }) => this.openUnlockPopout(sender.tab),
bgGetEnableChangedPasswordPrompt: () => this.getEnableChangedPasswordPrompt(),
bgGetEnableAddedLoginPrompt: () => this.getEnableAddedLoginPrompt(),
getWebVaultUrlForNotification: () => this.getWebVaultUrl(),
};

Expand All @@ -67,6 +70,7 @@ export default class NotificationBackground {
private policyService: PolicyService,
private folderService: FolderService,
private stateService: BrowserStateService,
private userNotificationSettingsService: UserNotificationSettingsServiceAbstraction,
private environmentService: EnvironmentService,
private logService: LogService,
) {}
Expand All @@ -81,6 +85,20 @@ export default class NotificationBackground {
this.cleanupNotificationQueue();
}

/**
* Gets the enableChangedPasswordPrompt setting from the user notification settings service.
*/
async getEnableChangedPasswordPrompt(): Promise<boolean> {
return await firstValueFrom(this.userNotificationSettingsService.enableChangedPasswordPrompt$);
}

/**
* Gets the enableAddedLoginPrompt setting from the user notification settings service.
*/
async getEnableAddedLoginPrompt(): Promise<boolean> {
return await firstValueFrom(this.userNotificationSettingsService.enableAddedLoginPrompt$);
}

/**
* Checks the notification queue for any messages that need to be sent to the
* specified tab. If no tab is specified, the current tab will be used.
Expand Down Expand Up @@ -194,9 +212,10 @@ export default class NotificationBackground {
return;
}

const disabledAddLogin = await this.stateService.getDisableAddLoginNotification();
const addLoginIsEnabled = await this.getEnableAddedLoginPrompt();

if (authStatus === AuthenticationStatus.Locked) {
if (!disabledAddLogin) {
if (addLoginIsEnabled) {
await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab, true);
}

Expand All @@ -207,14 +226,15 @@ export default class NotificationBackground {
const usernameMatches = ciphers.filter(
(c) => c.login.username != null && c.login.username.toLowerCase() === normalizedUsername,
);
if (!disabledAddLogin && usernameMatches.length === 0) {
if (addLoginIsEnabled && usernameMatches.length === 0) {
await this.pushAddLoginToQueue(loginDomain, loginInfo, sender.tab);
return;
}

const disabledChangePassword = await this.stateService.getDisableChangedPasswordNotification();
const changePasswordIsEnabled = await this.getEnableChangedPasswordPrompt();

if (
!disabledChangePassword &&
changePasswordIsEnabled &&
usernameMatches.length === 1 &&
usernameMatches[0].login.password !== loginInfo.password
) {
Expand Down
25 changes: 25 additions & 0 deletions ...r/src/autofill/background/service_factories/user-notification-settings-service.factory.ts
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import { UserNotificationSettingsService } from "@bitwarden/common/autofill/services/user-notification-settings.service";

import {
CachedServices,
factory,
FactoryOptions,
} from "../../../platform/background/service-factories/factory-options";
import {
stateProviderFactory,
StateProviderInitOptions,
} from "../../../platform/background/service-factories/state-provider.factory";

export type UserNotificationSettingsServiceInitOptions = FactoryOptions & StateProviderInitOptions;

export function userNotificationSettingsServiceFactory(
cache: { userNotificationSettingsService?: UserNotificationSettingsService } & CachedServices,
opts: UserNotificationSettingsServiceInitOptions,
): Promise<UserNotificationSettingsService> {
return factory(
cache,
"userNotificationSettingsService",
opts,
async () => new UserNotificationSettingsService(await stateProviderFactory(cache, opts)),
);
}
Loading

0 comments on commit 4ba2717

Please sign in to comment.