1
0
mirror of https://github.com/bitwarden/browser synced 2026-01-28 07:13:29 +00:00

add branching qualification logic for cipher notifications

This commit is contained in:
Jonathan Prusik
2026-01-06 13:01:07 -05:00
parent b690348b83
commit 37e49acab2
4 changed files with 193 additions and 24 deletions

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,6 +18,21 @@ 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 "Cipher" will be removed post-`useFullCipherTriggeringLogic`
* 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"]>;

View File

@@ -323,6 +323,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 +335,7 @@ describe("OverlayNotificationsBackground", () => {
"triggerChangedPasswordNotification",
);
notificationAddLoginSpy = jest.spyOn(notificationBackground, "triggerAddLoginNotification");
cipherNotificationSpy = jest.spyOn(notificationBackground, "triggerCipherNotification");
sendMockExtensionMessage(
{ command: "collectPageDetailsResponse", details: pageDetails },
@@ -456,6 +458,7 @@ describe("OverlayNotificationsBackground", () => {
const pageDetails = mock<AutofillPageDetails>({ fields: [mock<AutofillField>()] });
beforeEach(async () => {
overlayNotificationsBackground["useUndiscoveredCipherScenarioTriggeringLogic"] = false;
sendMockExtensionMessage(
{ command: "collectPageDetailsResponse", details: pageDetails },
sender,
@@ -519,6 +522,44 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationAddLoginSpy).toHaveBeenCalled();
});
it("with `useUndiscoveredCipherScenarioTriggeringLogic` on, waits for the tab's navigation to complete using the web navigation API before initializing the notification", async () => {
overlayNotificationsBackground["useUndiscoveredCipherScenarioTriggeringLogic"] = 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 +593,40 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationAddLoginSpy).toHaveBeenCalled();
});
it("with `useUndiscoveredCipherScenarioTriggeringLogic` on, initializes the notification immediately when the tab's navigation is complete", async () => {
overlayNotificationsBackground["useUndiscoveredCipherScenarioTriggeringLogic"] = 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 +676,57 @@ describe("OverlayNotificationsBackground", () => {
expect(notificationChangedPasswordSpy).toHaveBeenCalled();
});
it("with `useUndiscoveredCipherScenarioTriggeringLogic` on, triggers the notification on the beforeRequest listener when a post-submission redirection is encountered", async () => {
overlayNotificationsBackground["useUndiscoveredCipherScenarioTriggeringLogic"] = 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();
});
});
});

View File

@@ -4,7 +4,6 @@ import { CLEAR_NOTIFICATION_LOGIN_DATA_DURATION } from "@bitwarden/common/autofi
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,6 +33,8 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
collectPageDetailsResponse: ({ message, sender }) =>
this.handleCollectPageDetailsResponse(message, sender),
};
// @TODO update via feature-flag
private useUndiscoveredCipherScenarioTriggeringLogic = false;
constructor(
private logService: LogService,
@@ -281,7 +284,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
const shouldAttemptAddNotification = this.shouldAttemptNotification(
modifyLoginData,
NotificationTypes.Add,
NotificationScenarios.Add,
);
if (shouldAttemptAddNotification) {
@@ -290,7 +293,7 @@ export class OverlayNotificationsBackground implements OverlayNotificationsBackg
const shouldAttemptChangeNotification = this.shouldAttemptNotification(
modifyLoginData,
NotificationTypes.Change,
NotificationScenarios.Change,
);
if (shouldAttemptChangeNotification) {
@@ -445,29 +448,41 @@ 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 notificationCandidates = this.useUndiscoveredCipherScenarioTriggeringLogic
? [
{
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 +504,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 +527,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:

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;