bitwarden/browser
1
0
Fork 0
mirror of https://github.com/bitwarden/browser synced 2026-02-12 06:23:38 +00:00
Code Issues Releases Wiki Activity

[PM-29236] Refactor of post-submit notification triggering logic (#18395)

Browse Source 
* refactor triggerChangedPasswordNotification logic

* improve triggerChangedPasswordNotification and test coverage to handle scenarios more comprehensively

* restore triggerChangedPasswordNotification logic and move new logic and testing to triggerCipherNotification

* add branching qualification logic for cipher notifications

* add and implement undetermined-cipher-scenario-logic feature flag

* add optional chaining to username comparison of existing login ciphers

* cleanup

* update tests

* prefer explicit length comparisons
This commit is contained in:
Jonathan Prusik
2026-02-02 15:01:24 -05:00
committed by GitHub
parent 47edae1c21
commit 5a397fb44e
7 changed files with 2153 additions and 44 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
apps/browser/src/autofill/background/abstractions/overlay-notifications.background.ts
View File

@@ -2,6 +2,7 @@ import { CipherView } from "@bitwarden/common/vault/models/view/cipher.view";
import { SecurityTask } from "@bitwarden/common/vault/tasks";
import AutofillPageDetails from "../../models/autofill-page-details";
import { NotificationTypes } from "../../notification/abstractions/notification-bar";
export type NotificationTypeData = {
isVaultLocked?: boolean;
@@ -17,10 +18,26 @@ export type LoginSecurityTaskInfo = {
uri: ModifyLoginCipherFormData["uri"];
};
/**
* Distinguished from `NotificationTypes` in that this represents the
* pre-resolved notification scenario, vs the notification component
* (e.g. "Add" and "Change" will be removed
* post-`useUndeterminedCipherScenarioTriggeringLogic` migration)
*/
export const NotificationScenarios = {
...NotificationTypes,
/** represents scenarios handling saving new and updated ciphers after form submit */
Cipher: "cipher",
} as const;
export type NotificationScenario =
(typeof NotificationScenarios)[keyof typeof NotificationScenarios];
export type WebsiteOriginsWithFields = Map<chrome.tabs.Tab["id"], Set<string>>;
export type ActiveFormSubmissionRequests = Set<chrome.webRequest.WebRequestDetails["requestId"]>;
/** This type represents an expectation of nullish values being represented as empty strings */
export type ModifyLoginCipherFormData = {
uri: string;
username: string;

1513
apps/browser/src/autofill/background/notification.background.spec.ts
View File

File diff suppressed because it is too large Load Diff

457
apps/browser/src/autofill/background/notification.background.ts
View File

@@ -22,6 +22,7 @@ import {
import { DomainSettingsService } from "@bitwarden/common/autofill/services/domain-settings.service";
import { UserNotificationSettingsServiceAbstraction } from "@bitwarden/common/autofill/services/user-notification-settings.service";
import { ProductTierType } from "@bitwarden/common/billing/enums/product-tier-type.enum";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { NeverDomains } from "@bitwarden/common/models/domain/domain-service";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { ServerConfig } from "@bitwarden/common/platform/abstractions/config/server-config";
@@ -79,6 +80,30 @@ import {
} from "./abstractions/overlay-notifications.background";
import { OverlayBackgroundExtensionMessage } from "./abstractions/overlay.background";
const inputScenarios = {
usernamePasswordNewPassword: "usernamePasswordNewPassword",
usernameNewPassword: "usernameNewPassword",
usernamePassword: "usernamePassword",
username: "username",
passwordNewPassword: "passwordNewPassword",
newPassword: "newPassword",
password: "password",
} as const;
type InputScenarioKey = keyof typeof inputScenarios;
type InputScenario = (typeof inputScenarios)[InputScenarioKey];
type CiphersByInputMatchCategory = {
allFieldMatches: CipherView["id"][];
newPasswordOnlyMatches: CipherView["id"][];
noFieldMatches: CipherView["id"][];
passwordNewPasswordMatches: CipherView["id"][];
passwordOnlyMatches: CipherView["id"][];
usernameNewPasswordMatches: CipherView["id"][];
usernameOnlyMatches: CipherView["id"][];
usernamePasswordMatches: CipherView["id"][];
};
export default class NotificationBackground {
private openUnlockPopout = openUnlockPopout;
private openAddEditVaultItemPopout = openAddEditVaultItemPopout;
@@ -152,6 +177,10 @@ export default class NotificationBackground {
this.cleanupNotificationQueue();
}
useUndeterminedCipherScenarioTriggeringLogic$ = this.configService.getFeatureFlag$(
FeatureFlag.UseUndeterminedCipherScenarioTriggeringLogic,
);
/**
* Gets the enableChangedPasswordPrompt setting from the user notification settings service.
*/
@@ -292,7 +321,7 @@ export default class NotificationBackground {
type: CipherType.Login,
reprompt,
favorite,
...(organizationCategories.length ? { organizationCategories } : {}),
...(organizationCategories.length > 0 ? { organizationCategories } : {}),
icon: buildCipherIcon(iconsServerUrl, view, showFavicons),
login: login && { username: login.username },
};
@@ -309,7 +338,7 @@ export default class NotificationBackground {
activeUserId: UserId,
): Promise<LoginSecurityTaskInfo | null> {
const tasks: SecurityTask[] = await this.getSecurityTasks(activeUserId);
if (!tasks?.length) {
if (!(tasks?.length > 0)) {
return null;
}
@@ -317,7 +346,7 @@ export default class NotificationBackground {
modifyLoginData.uri,
activeUserId,
);
if (!urlCiphers?.length) {
if (!(urlCiphers?.length > 0)) {
return null;
}
@@ -596,6 +625,216 @@ export default class NotificationBackground {
await this.checkNotificationQueue(tab);
}
/**
* Receives filled form values and determines if a notification should be
* triggered, and if so, what kind and with what data.
*
* If an update scenario is identified, a change password message is added to the
* notification queue, prompting the user to update a stored login that has changed.
*
* A new cipher notification is triggered in other defined scenarios
* with the user's form input.
*
* Returns `true` or `false` to indicate if such a notification was
* triggered or not.
*
* For the purposes of this function, form field inputs should be assumed to be
* qualified accurately.
*/
async triggerCipherNotification(
data: ModifyLoginCipherFormData,
tab: chrome.tabs.Tab,
): Promise<boolean> {
const usernameFieldValue: string | null = data.username || null;
const currentPasswordFieldValue = data.password || null;
const newPasswordFieldValue = data.newPassword || null;
// If no values were entered, exit early
if (!usernameFieldValue && !currentPasswordFieldValue && !newPasswordFieldValue) {
return false;
}
// If the entered data doesn't have an associated URI, exit early
const loginDomain = Utils.getDomain(data.uri);
if (loginDomain === null) {
return false;
}
// If no cipher add/update notifications are enabled, we can exit early
const changePasswordNotificationIsEnabled = await this.getEnableChangedPasswordPrompt();
const newLoginNotificationIsEnabled = await this.getEnableAddedLoginPrompt();
if (!changePasswordNotificationIsEnabled && !newLoginNotificationIsEnabled) {
return false;
}
// If there is no account logged in (as opposed to only being locked), exit early
const authStatus = await this.getAuthStatus();
if (authStatus === AuthenticationStatus.LoggedOut) {
return false;
}
// If there is no active user, exit early
const activeUserId = await firstValueFrom(
this.accountService.activeAccount$.pipe(getOptionalUserId),
);
if (activeUserId === null) {
return false;
}
const normalizedUsername: string = usernameFieldValue ? usernameFieldValue.toLowerCase() : "";
const currentPasswordFieldHasValue =
typeof currentPasswordFieldValue === "string" && currentPasswordFieldValue.length > 0;
const newPasswordFieldHasValue =
typeof newPasswordFieldValue === "string" && newPasswordFieldValue.length > 0;
const usernameFieldHasValue =
typeof usernameFieldValue === "string" && usernameFieldValue.length > 0;
// If the current and new password inputs both have values and those values
// match, return early, since no change was made
if (
currentPasswordFieldHasValue &&
newPasswordFieldHasValue &&
currentPasswordFieldValue === newPasswordFieldValue
) {
return false;
}
/*
* We only show the unlock notification if a new password field was filled, since
* it's very likely to blindly represent an updated cipher value whereas other
* scenarios below require the vault to be unlocked in order to determine
* if an update has been made.
*/
if (authStatus === AuthenticationStatus.Locked) {
if (!newPasswordFieldHasValue) {
return false;
}
// This needs to be the call that includes the full form data
await this.pushChangePasswordToQueue(null, loginDomain, newPasswordFieldValue, tab, true);
return true;
}
const ciphersForURL: CipherView[] = await this.cipherService.getAllDecryptedForUrl(
data.uri,
activeUserId,
);
// Reducer structured to avoid subsequent array iterations
const ciphersByInputMatchCategory = ciphersForURL.reduce(
(acc, { id, login }) => {
const usernameInputMatchesCipher =
usernameFieldHasValue && login.username?.toLowerCase() === normalizedUsername;
const passwordInputMatchesCipher =
currentPasswordFieldHasValue && login.password === currentPasswordFieldValue;
const newPasswordInputMatchesCipher =
newPasswordFieldHasValue && login.password === newPasswordFieldValue;
if (
!newPasswordInputMatchesCipher &&
!usernameInputMatchesCipher &&
!passwordInputMatchesCipher
) {
return { ...acc, noFieldMatches: [...acc.noFieldMatches, id] };
} else if (
newPasswordInputMatchesCipher &&
usernameInputMatchesCipher &&
passwordInputMatchesCipher
) {
// Note: this case should be unreachable due to the early exit comparing
// the password input values against each other, but leaving this bit here
// as a defense against future changes to the pre-match checks.
return { ...acc, allFieldMatches: [...acc.allFieldMatches, id] };
} else if (
newPasswordInputMatchesCipher &&
!usernameInputMatchesCipher &&
!passwordInputMatchesCipher
) {
return { ...acc, newPasswordOnlyMatches: [...acc.newPasswordOnlyMatches, id] };
} else if (
passwordInputMatchesCipher &&
!usernameInputMatchesCipher &&
!newPasswordInputMatchesCipher
) {
return { ...acc, passwordOnlyMatches: [...acc.passwordOnlyMatches, id] };
} else if (
passwordInputMatchesCipher &&
newPasswordInputMatchesCipher &&
!usernameInputMatchesCipher
) {
// Note: this case should be unreachable due to the early exit comparing
// the password input values against each other, but leaving this bit here
// as a defense against future changes to the pre-match checks.
return { ...acc, passwordNewPasswordMatches: [...acc.passwordNewPasswordMatches, id] };
} else if (
usernameInputMatchesCipher &&
!passwordInputMatchesCipher &&
!newPasswordInputMatchesCipher
) {
return { ...acc, usernameOnlyMatches: [...acc.usernameOnlyMatches, id] };
} else if (
usernameInputMatchesCipher &&
passwordInputMatchesCipher &&
!newPasswordInputMatchesCipher
) {
return { ...acc, usernamePasswordMatches: [...acc.usernamePasswordMatches, id] };
} else if (
usernameInputMatchesCipher &&
newPasswordInputMatchesCipher &&
!passwordInputMatchesCipher
) {
return { ...acc, usernameNewPasswordMatches: [...acc.usernameNewPasswordMatches, id] };
}
return acc;
},
{
allFieldMatches: [],
newPasswordOnlyMatches: [],
noFieldMatches: [],
passwordNewPasswordMatches: [],
passwordOnlyMatches: [],
usernameNewPasswordMatches: [],
usernameOnlyMatches: [],
usernamePasswordMatches: [],
},
);
// Handle different field fill combinations and determine the input scenario
const inputScenariosByKey = {
upn: inputScenarios.usernamePasswordNewPassword,
un: inputScenarios.usernameNewPassword,
up: inputScenarios.usernamePassword,
u: inputScenarios.username,
pn: inputScenarios.passwordNewPassword,
n: inputScenarios.newPassword,
p: inputScenarios.password,
} as const;
type InputScenarioKeys = keyof typeof inputScenariosByKey;
const key = ((usernameFieldHasValue ? "u" : "") +
(currentPasswordFieldHasValue ? "p" : "") +
(newPasswordFieldHasValue ? "n" : "")) as InputScenarioKeys;
const inputScenario = key in inputScenariosByKey ? inputScenariosByKey[key] : null;
if (inputScenario) {
return await this.handleInputMatchScenario({
ciphersByInputMatchCategory,
ciphersForURL,
loginDomain,
tab,
data,
inputScenario,
changePasswordNotificationIsEnabled,
newLoginNotificationIsEnabled,
});
}
return false;
}
/**
* Adds a change password message to the notification queue, prompting the user
* to update the password for a login that has changed.
@@ -668,13 +907,14 @@ export default class NotificationBackground {
if (
ciphers.length > 0 &&
currentPasswordFieldValue?.length &&
(currentPasswordFieldValue?.length || 0) > 0 &&
// Only use current password for change if no new password present.
!newPasswordFieldValue
) {
const currentPasswordMatchesAnExistingValue = ciphers.some(
(cipher) =>
cipher.login?.password?.length && cipher.login.password === currentPasswordFieldValue,
(cipher.login?.password?.length || 0) > 0 &&
cipher.login.password === currentPasswordFieldValue,
);
// The password entered matched a stored cipher value with
@@ -710,6 +950,212 @@ export default class NotificationBackground {
return false;
}
private async handleInputMatchScenario({
inputScenario,
ciphersByInputMatchCategory,
ciphersForURL,
loginDomain,
tab,
data,
changePasswordNotificationIsEnabled,
newLoginNotificationIsEnabled,
}: {
ciphersByInputMatchCategory: CiphersByInputMatchCategory;
ciphersForURL: CipherView[];
loginDomain: string;
tab: chrome.tabs.Tab;
data: ModifyLoginCipherFormData;
inputScenario: InputScenario;
changePasswordNotificationIsEnabled: boolean;
newLoginNotificationIsEnabled: boolean;
}): Promise<boolean> {
const {
newPasswordOnlyMatches,
noFieldMatches,
passwordOnlyMatches,
usernameNewPasswordMatches,
usernameOnlyMatches,
usernamePasswordMatches,
} = ciphersByInputMatchCategory;
// IMPORTANT! The order of statements matters here; later evaluations
// depend on the assumptions of the early exits in preceding logic
// If no ciphers match any filled input values
// (Note, this block may uniquely exit early since this match scenario
// involves all ciphers, making it mutually exclusive from any other scenario)
if (noFieldMatches.length === ciphersForURL.length) {
// trigger a new cipher notification in these input scenarios
if (
(
[
inputScenarios.usernamePasswordNewPassword,
inputScenarios.usernameNewPassword,
inputScenarios.usernamePassword,
inputScenarios.username,
] as InputScenario[]
).includes(inputScenario) &&
newLoginNotificationIsEnabled
) {
await this.pushAddLoginToQueue(
loginDomain,
{ username: data.username, url: data.uri, password: data.newPassword || data.password },
tab,
);
return true;
}
// Trigger an update cipher notification with all URI ciphers
// in these input scenarios
if (
([inputScenarios.password, inputScenarios.newPassword] as InputScenario[]).includes(
inputScenario,
) &&
changePasswordNotificationIsEnabled
) {
await this.pushChangePasswordToQueue(
ciphersForURL.map((c) => c.id),
loginDomain,
// @TODO handle empty strings / incomplete data structure
data.newPassword || data.password,
tab,
);
return true;
}
return false;
}
// If ciphers match entered username and new password values
if (usernameNewPasswordMatches.length > 0) {
// Early exit in these scenarios as they represent "no change"
if (
(
[
inputScenarios.usernamePasswordNewPassword,
inputScenarios.usernameNewPassword,
] as InputScenario[]
).includes(inputScenario)
) {
return false;
}
}
// If ciphers match entered username and password values
if (usernamePasswordMatches.length > 0) {
// and username, password, and new password values were entered
if (
inputScenario === inputScenarios.usernamePasswordNewPassword &&
changePasswordNotificationIsEnabled
) {
await this.pushChangePasswordToQueue(
usernamePasswordMatches,
loginDomain,
// @TODO handle empty strings / incomplete data structure
data.newPassword || data.password,
tab,
);
return true;
}
if (inputScenario === inputScenarios.usernamePassword) {
return false;
}
}
// If ciphers match entered username value (only)
if (usernameOnlyMatches.length > 0) {
if (
(
[
inputScenarios.usernamePasswordNewPassword,
inputScenarios.usernameNewPassword,
inputScenarios.usernamePassword,
] as InputScenario[]
).includes(inputScenario) &&
changePasswordNotificationIsEnabled
) {
await this.pushChangePasswordToQueue(
usernameOnlyMatches,
loginDomain,
// @TODO handle empty strings / incomplete data structure
data.newPassword || data.password,
tab,
);
return true;
}
// Early exit in this scenario as it represents "no change"
if (inputScenario === inputScenarios.username) {
return false;
}
}
// If ciphers match entered new password value (only)
if (newPasswordOnlyMatches.length > 0) {
// Early exit in these scenarios
if (
(
[
inputScenarios.usernameNewPassword, // unclear user expectation
inputScenarios.password, // likely nothing to change
inputScenarios.newPassword, // nothing to change
] as InputScenario[]
).includes(inputScenario)
) {
return false;
}
// and username, password, and new password values were entered
if (
inputScenario === inputScenarios.usernamePasswordNewPassword &&
newLoginNotificationIsEnabled
) {
await this.pushAddLoginToQueue(
loginDomain,
{ username: data.username, url: data.uri, password: data.newPassword || data.password },
tab,
);
return true;
}
}
// If ciphers match entered password value (only)
if (passwordOnlyMatches.length > 0) {
if (
(
[
inputScenarios.usernamePasswordNewPassword,
inputScenarios.usernamePassword,
inputScenarios.passwordNewPassword,
] as InputScenario[]
).includes(inputScenario) &&
changePasswordNotificationIsEnabled
) {
await this.pushChangePasswordToQueue(
passwordOnlyMatches,
loginDomain,
// @TODO handle empty strings / incomplete data structure
data.newPassword || data.password,
tab,
);
return true;
}
// Early exit in this scenario as it represents "no change"
if (inputScenario === inputScenarios.password) {
return false;
}
}
return false;
}
/**
* Sends the page details to the notification bar. Will query all
* forms with a password field and pass them to the notification bar.
@@ -730,6 +1176,7 @@ export default class NotificationBackground {
});
}
// @TODO this needs the whole input record, and not just newPassword
private async pushChangePasswordToQueue(
cipherIds: CipherView["id"][],
loginDomain: string,

128
apps/browser/src/autofill/background/overlay-notifications.background.spec.ts
View File

@@ -1,4 +1,5 @@
import { mock, MockProxy } from "jest-mock-extended";
import { of } from "rxjs";
import { CLEAR_NOTIFICATION_LOGIN_DATA_DURATION } from "@bitwarden/common/autofill/constants";
import { ServerConfig } from "@bitwarden/common/platform/abstractions/config/server-config";
@@ -32,6 +33,7 @@ describe("OverlayNotificationsBackground", () => {
jest.useFakeTimers();
logService = mock<LogService>();
notificationBackground = mock<NotificationBackground>();
notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$ = of(false);
getEnableChangedPasswordPromptSpy = jest
.spyOn(notificationBackground, "getEnableChangedPasswordPrompt")
.mockResolvedValue(true);
@@ -323,6 +325,7 @@ describe("OverlayNotificationsBackground", () => {
const pageDetails = mock<AutofillPageDetails>({ fields: [mock<AutofillField>()] });
let notificationChangedPasswordSpy: jest.SpyInstance;
let notificationAddLoginSpy: jest.SpyInstance;
let cipherNotificationSpy: jest.SpyInstance;
beforeEach(async () => {
sender = mock<chrome.runtime.MessageSender>({
@@ -334,6 +337,7 @@ describe("OverlayNotificationsBackground", () => {
"triggerChangedPasswordNotification",
);
notificationAddLoginSpy = jest.spyOn(notificationBackground, "triggerAddLoginNotification");
cipherNotificationSpy = jest.spyOn(notificationBackground, "triggerCipherNotification");
sendMockExtensionMessage(
{ command: "collectPageDetailsResponse", details: pageDetails },
@@ -456,6 +460,7 @@ describe("OverlayNotificationsBackground", () => {
const pageDetails = mock<AutofillPageDetails>({ fields: [mock<AutofillField>()] });
beforeEach(async () => {
notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$ = of(false);
sendMockExtensionMessage(
{ command: "collectPageDetailsResponse", details: pageDetails },
sender,
@@ -519,6 +524,44 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationAddLoginSpy).toHaveBeenCalled();
});
it("with `useUndeterminedCipherScenarioTriggeringLogic` on, waits for the tab's navigation to complete using the web navigation API before initializing the notification", async () => {
notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$ = of(true);
chrome.tabs.get = jest.fn().mockImplementationOnce((tabId, callback) => {
callback(
mock<chrome.tabs.Tab>({
status: "loading",
url: sender.url,
}),
);
});
triggerWebRequestOnCompletedEvent(
mock<chrome.webRequest.OnCompletedDetails>({
url: sender.url,
tabId: sender.tab.id,
requestId,
}),
);
await flushPromises();
chrome.tabs.get = jest.fn().mockImplementationOnce((tabId, callback) => {
callback(
mock<chrome.tabs.Tab>({
status: "complete",
url: sender.url,
}),
);
});
triggerWebNavigationOnCompletedEvent(
mock<chrome.webNavigation.WebNavigationFramedCallbackDetails>({
tabId: sender.tab.id,
url: sender.url,
}),
);
await flushPromises();
expect(cipherNotificationSpy).toHaveBeenCalled();
});
it("initializes the notification immediately when the tab's navigation is complete", async () => {
sendMockExtensionMessage(
{
@@ -552,6 +595,40 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationAddLoginSpy).toHaveBeenCalled();
});
it("with `useUndeterminedCipherScenarioTriggeringLogic` on, initializes the notification immediately when the tab's navigation is complete", async () => {
notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$ = of(true);
sendMockExtensionMessage(
{
command: "formFieldSubmitted",
uri: "example.com",
username: "username",
password: "password",
newPassword: "newPassword",
},
sender,
);
await flushPromises();
chrome.tabs.get = jest.fn().mockImplementationOnce((tabId, callback) => {
callback(
mock<chrome.tabs.Tab>({
status: "complete",
url: sender.url,
}),
);
});
triggerWebRequestOnCompletedEvent(
mock<chrome.webRequest.OnCompletedDetails>({
url: sender.url,
tabId: sender.tab.id,
requestId,
}),
);
await flushPromises();
expect(cipherNotificationSpy).toHaveBeenCalled();
});
it("triggers the notification on the beforeRequest listener when a post-submission redirection is encountered", async () => {
sender.tab = mock<chrome.tabs.Tab>({ id: 4 });
sendMockExtensionMessage(
@@ -601,6 +678,57 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationChangedPasswordSpy).toHaveBeenCalled();
});
it("with `useUndeterminedCipherScenarioTriggeringLogic` on, triggers the notification on the beforeRequest listener when a post-submission redirection is encountered", async () => {
notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$ = of(true);
sender.tab = mock<chrome.tabs.Tab>({ id: 4 });
sendMockExtensionMessage(
{ command: "collectPageDetailsResponse", details: pageDetails },
sender,
);
await flushPromises();
sendMockExtensionMessage(
{
command: "formFieldSubmitted",
uri: "example.com",
username: "",
password: "password",
newPassword: "newPassword",
},
sender,
);
await flushPromises();
chrome.tabs.get = jest.fn().mockImplementation((tabId, callback) => {
callback(
mock<chrome.tabs.Tab>({
status: "complete",
url: sender.url,
}),
);
});
triggerWebRequestOnBeforeRequestEvent(
mock<chrome.webRequest.WebRequestDetails>({
url: sender.url,
tabId: sender.tab.id,
method: "POST",
requestId,
}),
);
await flushPromises();
triggerWebRequestOnBeforeRequestEvent(
mock<chrome.webRequest.WebRequestDetails>({
url: "https://example.com/redirect",
tabId: sender.tab.id,
method: "GET",
requestId,
}),
);
await flushPromises();
expect(cipherNotificationSpy).toHaveBeenCalled();
});
});
});

76
apps/browser/src/autofill/background/overlay-notifications.background.ts
View File

@@ -1,10 +1,9 @@
import { Subject, switchMap, timer } from "rxjs";
import { firstValueFrom, Subject, switchMap, timer } from "rxjs";
import { CLEAR_NOTIFICATION_LOGIN_DATA_DURATION } from "@bitwarden/common/autofill/constants";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { BrowserApi } from "../../platform/browser/browser-api";
import { NotificationType, NotificationTypes } from "../notification/abstractions/notification-bar";
import { generateDomainMatchPatterns, isInvalidResponseStatusCode } from "../utils";
import {
@@ -14,6 +13,8 @@ import {
OverlayNotificationsBackground as OverlayNotificationsBackgroundInterface,
OverlayNotificationsExtensionMessage,
OverlayNotificationsExtensionMessageHandlers,
NotificationScenarios,
NotificationScenario,
WebsiteOriginsWithFields,
} from "./abstractions/overlay-notifications.background";
import NotificationBackground from "./notification.background";
@@ -32,7 +33,6 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
collectPageDetailsResponse: ({ message, sender }) =>
this.handleCollectPageDetailsResponse(message, sender),
};
constructor(
private logService: LogService,
private notificationBackground: NotificationBackground,
@@ -281,7 +281,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
const shouldAttemptAddNotification = this.shouldAttemptNotification(
modifyLoginData,
NotificationTypes.Add,
NotificationScenarios.Add,
);
if (shouldAttemptAddNotification) {
@@ -290,7 +290,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
const shouldAttemptChangeNotification = this.shouldAttemptNotification(
modifyLoginData,
NotificationTypes.Change,
NotificationScenarios.Change,
);
if (shouldAttemptChangeNotification) {
@@ -445,29 +445,45 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
requestId: chrome.webRequest.WebRequestDetails["requestId"],
modifyLoginData: ModifyLoginCipherFormData,
tab: chrome.tabs.Tab,
config: { skippable: NotificationType[] } = { skippable: [] },
config: { skippable: NotificationScenario[] } = { skippable: [] },
) => {
const notificationCandidates = [
{
type: NotificationTypes.Change,
trigger: this.notificationBackground.triggerChangedPasswordNotification,
},
{
type: NotificationTypes.Add,
trigger: this.notificationBackground.triggerAddLoginNotification,
},
{
type: NotificationTypes.AtRiskPassword,
trigger: this.notificationBackground.triggerAtRiskPasswordNotification,
},
].filter(
const useUndeterminedCipherScenarioTriggeringLogic = await firstValueFrom(
this.notificationBackground.useUndeterminedCipherScenarioTriggeringLogic$,
);
const notificationCandidates = useUndeterminedCipherScenarioTriggeringLogic
? [
{
type: NotificationScenarios.Cipher,
trigger: this.notificationBackground.triggerCipherNotification,
},
{
type: NotificationScenarios.AtRiskPassword,
trigger: this.notificationBackground.triggerAtRiskPasswordNotification,
},
]
: [
{
type: NotificationScenarios.Change,
trigger: this.notificationBackground.triggerChangedPasswordNotification,
},
{
type: NotificationScenarios.Add,
trigger: this.notificationBackground.triggerAddLoginNotification,
},
{
type: NotificationScenarios.AtRiskPassword,
trigger: this.notificationBackground.triggerAtRiskPasswordNotification,
},
];
const filteredNotificationCandidates = notificationCandidates.filter(
(candidate) =>
this.shouldAttemptNotification(modifyLoginData, candidate.type) ||
config.skippable.includes(candidate.type),
);
const results: string[] = [];
for (const { trigger, type } of notificationCandidates) {
for (const { trigger, type } of filteredNotificationCandidates) {
const success = await trigger.bind(this.notificationBackground)(modifyLoginData, tab);
if (success) {
results.push(`Success: ${type}`);
@@ -489,8 +505,16 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
*/
private shouldAttemptNotification = (
modifyLoginData: ModifyLoginCipherFormData,
notificationType: NotificationType,
notificationType: NotificationScenario,
): boolean => {
if (notificationType === NotificationScenarios.Cipher) {
// The logic after this block pre-qualifies some cipher add/update scenarios
// prematurely (where matching against vault contents is required) and should be
// skipped for this case (these same checks are performed early in the
// notification triggering logic).
return true;
}
// Intentionally not stripping whitespace characters here as they
// represent user entry.
const usernameFieldHasValue = !!(modifyLoginData?.username || "").length;
@@ -504,15 +528,15 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
// `Add` case included because all forms with cached usernames (from previous
// visits) will appear to be "password only" and otherwise trigger the new login
// save notification.
case NotificationTypes.Add:
case NotificationScenarios.Add:
// Can be values for nonstored login or account creation
return usernameFieldHasValue && (passwordFieldHasValue || newPasswordFieldHasValue);
case NotificationTypes.Change:
case NotificationScenarios.Change:
// Can be login with nonstored login changes or account password update
return canBeUserLogin || canBePasswordUpdate;
case NotificationTypes.AtRiskPassword:
case NotificationScenarios.AtRiskPassword:
return !newPasswordFieldHasValue;
case NotificationTypes.Unlock:
case NotificationScenarios.Unlock:
// Unlock notifications are handled separately and do not require form data
return false;
default:

4
apps/browser/src/autofill/notification/abstractions/notification-bar.ts
View File

@@ -8,9 +8,13 @@ import {
} from "../../../autofill/content/components/common-types";
const NotificationTypes = {
/** represents scenarios handling saving new ciphers after form submit */
Add: "add",
/** represents scenarios handling saving updated ciphers after form submit */
Change: "change",
/** represents scenarios where user has interacted with an unlock action prompt or action otherwise requiring unlock as a prerequisite */
Unlock: "unlock",
/** represents scenarios where the user has security tasks after updating ciphers */
AtRiskPassword: "at-risk-password",
} as const;

2
libs/common/src/enums/feature-flag.enum.ts
View File

@@ -22,6 +22,7 @@ export enum FeatureFlag {
SafariAccountSwitching = "pm-5594-safari-account-switching",
/* Autofill */
UseUndeterminedCipherScenarioTriggeringLogic = "undetermined-cipher-scenario-logic",
MacOsNativeCredentialSync = "macos-native-credential-sync",
WindowsDesktopAutotype = "windows-desktop-autotype",
WindowsDesktopAutotypeGA = "windows-desktop-autotype-ga",
@@ -110,6 +111,7 @@ export const DefaultFeatureFlagValue = {
[FeatureFlag.MembersComponentRefactor]: FALSE,
/* Autofill */
[FeatureFlag.UseUndeterminedCipherScenarioTriggeringLogic]: FALSE,
[FeatureFlag.MacOsNativeCredentialSync]: FALSE,
[FeatureFlag.WindowsDesktopAutotype]: FALSE,
[FeatureFlag.WindowsDesktopAutotypeGA]: FALSE,