1
0
mirror of https://github.com/bitwarden/browser.git synced 2024-12-22 16:29:09 +01:00

[PM-12548] Fido2 scripts should not load when user is logged out (#11444)

* [PM-12548] Fido2 scripts should not load when user is logged out

* [PM-12548] Fido2 scripts should not load when user is logged out
This commit is contained in:
Cesar Gonzalez 2024-10-08 16:02:49 -05:00 committed by GitHub
parent fdfbe66513
commit a5c1a5a42f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
9 changed files with 82 additions and 50 deletions

View File

@ -1484,9 +1484,7 @@ export class OverlayBackground implements OverlayBackgroundInterface {
} }
/** /**
* Gets the user's authentication status from the auth service. If the user's authentication * Gets the user's authentication status from the auth service.
* status has changed, the inline menu button's authentication status will be updated
* and the inline menu list's ciphers will be updated.
*/ */
private async getAuthStatus() { private async getAuthStatus() {
return await firstValueFrom(this.authService.activeAccountStatus$); return await firstValueFrom(this.authService.activeAccountStatus$);

View File

@ -45,7 +45,6 @@ type Fido2BackgroundExtensionMessageHandlers = {
interface Fido2Background { interface Fido2Background {
init(): void; init(): void;
injectFido2ContentScriptsInAllTabs(): Promise<void>;
} }
export { export {

View File

@ -1,6 +1,8 @@
import { mock, MockProxy } from "jest-mock-extended"; import { mock, MockProxy } from "jest-mock-extended";
import { BehaviorSubject } from "rxjs"; import { BehaviorSubject } from "rxjs";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { Fido2ActiveRequestManager } from "@bitwarden/common/platform/abstractions/fido2/fido2-active-request-manager.abstraction"; import { Fido2ActiveRequestManager } from "@bitwarden/common/platform/abstractions/fido2/fido2-active-request-manager.abstraction";
import { import {
@ -59,6 +61,8 @@ describe("Fido2Background", () => {
let scriptInjectorServiceMock!: MockProxy<BrowserScriptInjectorService>; let scriptInjectorServiceMock!: MockProxy<BrowserScriptInjectorService>;
let configServiceMock!: MockProxy<ConfigService>; let configServiceMock!: MockProxy<ConfigService>;
let enablePasskeysMock$!: BehaviorSubject<boolean>; let enablePasskeysMock$!: BehaviorSubject<boolean>;
let activeAccountStatusMock$: BehaviorSubject<AuthenticationStatus>;
let authServiceMock!: MockProxy<AuthService>;
let fido2Background!: Fido2Background; let fido2Background!: Fido2Background;
beforeEach(() => { beforeEach(() => {
@ -81,6 +85,9 @@ describe("Fido2Background", () => {
vaultSettingsService.enablePasskeys$ = enablePasskeysMock$; vaultSettingsService.enablePasskeys$ = enablePasskeysMock$;
fido2ActiveRequestManager = mock<Fido2ActiveRequestManager>(); fido2ActiveRequestManager = mock<Fido2ActiveRequestManager>();
fido2ClientService.isFido2FeatureEnabled.mockResolvedValue(true); fido2ClientService.isFido2FeatureEnabled.mockResolvedValue(true);
activeAccountStatusMock$ = new BehaviorSubject(AuthenticationStatus.Unlocked);
authServiceMock = mock<AuthService>();
authServiceMock.activeAccountStatus$ = activeAccountStatusMock$;
fido2Background = new Fido2Background( fido2Background = new Fido2Background(
logService, logService,
fido2ActiveRequestManager, fido2ActiveRequestManager,
@ -88,6 +95,7 @@ describe("Fido2Background", () => {
vaultSettingsService, vaultSettingsService,
scriptInjectorServiceMock, scriptInjectorServiceMock,
configServiceMock, configServiceMock,
authServiceMock,
); );
fido2Background["abortManager"] = abortManagerMock; fido2Background["abortManager"] = abortManagerMock;
abortManagerMock.runWithAbortController.mockImplementation((_requestId, runner) => abortManagerMock.runWithAbortController.mockImplementation((_requestId, runner) =>
@ -101,55 +109,31 @@ describe("Fido2Background", () => {
jest.clearAllMocks(); jest.clearAllMocks();
}); });
describe("injectFido2ContentScriptsInAllTabs", () => { describe("handleAuthStatusUpdate", () => {
it("does not inject any FIDO2 content scripts when no tabs have a secure url protocol", async () => { let updateContentScriptRegistrationSpy: jest.SpyInstance;
const insecureTab = mock<chrome.tabs.Tab>({ id: 789, url: "http://example.com" });
tabsQuerySpy.mockResolvedValueOnce([insecureTab]);
await fido2Background.injectFido2ContentScriptsInAllTabs(); beforeEach(() => {
updateContentScriptRegistrationSpy = jest
expect(scriptInjectorServiceMock.inject).not.toHaveBeenCalled(); .spyOn(fido2Background as any, "updateContentScriptRegistration")
.mockImplementation();
}); });
it("only injects the FIDO2 content script into tabs that contain a secure url protocol", async () => { it("skips triggering the passkeys settings update if the user is logged out", async () => {
const secondTabMock = mock<chrome.tabs.Tab>({ id: 456, url: "https://example.com" }); activeAccountStatusMock$.next(AuthenticationStatus.LoggedOut);
const insecureTab = mock<chrome.tabs.Tab>({ id: 789, url: "http://example.com" });
const noUrlTab = mock<chrome.tabs.Tab>({ id: 101, url: undefined });
tabsQuerySpy.mockResolvedValueOnce([tabMock, secondTabMock, insecureTab, noUrlTab]);
await fido2Background.injectFido2ContentScriptsInAllTabs(); fido2Background.init();
await flushPromises(); await flushPromises();
expect(scriptInjectorServiceMock.inject).toHaveBeenCalledWith({ expect(updateContentScriptRegistrationSpy).not.toHaveBeenCalled();
tabId: tabMock.id,
injectDetails: contentScriptDetails,
});
expect(scriptInjectorServiceMock.inject).toHaveBeenCalledWith({
tabId: secondTabMock.id,
injectDetails: contentScriptDetails,
});
expect(scriptInjectorServiceMock.inject).not.toHaveBeenCalledWith({
tabId: insecureTab.id,
injectDetails: contentScriptDetails,
});
expect(scriptInjectorServiceMock.inject).not.toHaveBeenCalledWith({
tabId: noUrlTab.id,
injectDetails: contentScriptDetails,
});
}); });
it("injects the `page-script.js` content script into the provided tab", async () => { it("triggers the passkeys setting update if the user is logged in", async () => {
tabsQuerySpy.mockResolvedValueOnce([tabMock]); activeAccountStatusMock$.next(AuthenticationStatus.Unlocked);
await fido2Background.injectFido2ContentScriptsInAllTabs(); fido2Background.init();
await flushPromises(); await flushPromises();
expect(scriptInjectorServiceMock.inject).toHaveBeenCalledWith({ expect(updateContentScriptRegistrationSpy).toHaveBeenCalled();
tabId: tabMock.id,
injectDetails: sharedScriptInjectionDetails,
mv2Details: { file: Fido2ContentScript.PageScriptAppend },
mv3Details: { file: Fido2ContentScript.PageScript, world: "MAIN" },
});
}); });
}); });
@ -157,6 +141,7 @@ describe("Fido2Background", () => {
let portMock!: MockProxy<chrome.runtime.Port>; let portMock!: MockProxy<chrome.runtime.Port>;
beforeEach(() => { beforeEach(() => {
jest.spyOn(fido2Background as any, "handleAuthStatusUpdate").mockImplementation();
fido2Background.init(); fido2Background.init();
jest.spyOn(BrowserApi, "registerContentScriptsMv2"); jest.spyOn(BrowserApi, "registerContentScriptsMv2");
jest.spyOn(BrowserApi, "registerContentScriptsMv3"); jest.spyOn(BrowserApi, "registerContentScriptsMv3");
@ -168,6 +153,15 @@ describe("Fido2Background", () => {
tabsQuerySpy.mockResolvedValue([tabMock]); tabsQuerySpy.mockResolvedValue([tabMock]);
}); });
it("skips handling the passkey update if the user is logged out", async () => {
activeAccountStatusMock$.next(AuthenticationStatus.LoggedOut);
enablePasskeysMock$.next(true);
expect(portMock.disconnect).not.toHaveBeenCalled();
expect(scriptInjectorServiceMock.inject).not.toHaveBeenCalled();
});
it("does not destroy and re-inject the content scripts when triggering `handleEnablePasskeysUpdate` with an undefined currentEnablePasskeysSetting property", async () => { it("does not destroy and re-inject the content scripts when triggering `handleEnablePasskeysUpdate` with an undefined currentEnablePasskeysSetting property", async () => {
await flushPromises(); await flushPromises();
@ -421,6 +415,7 @@ describe("Fido2Background", () => {
let portMock!: MockProxy<chrome.runtime.Port>; let portMock!: MockProxy<chrome.runtime.Port>;
beforeEach(() => { beforeEach(() => {
jest.spyOn(fido2Background as any, "handleAuthStatusUpdate").mockImplementation();
fido2Background.init(); fido2Background.init();
portMock = createPortSpyMock(Fido2PortName.InjectedScript); portMock = createPortSpyMock(Fido2PortName.InjectedScript);
triggerRuntimeOnConnectEvent(portMock); triggerRuntimeOnConnectEvent(portMock);

View File

@ -1,6 +1,8 @@
import { firstValueFrom, startWith } from "rxjs"; import { firstValueFrom, startWith, Subscription } from "rxjs";
import { pairwise } from "rxjs/operators"; import { pairwise } from "rxjs/operators";
import { AuthService } from "@bitwarden/common/auth/abstractions/auth.service";
import { AuthenticationStatus } from "@bitwarden/common/auth/enums/authentication-status";
import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum"; import { FeatureFlag } from "@bitwarden/common/enums/feature-flag.enum";
import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service"; import { ConfigService } from "@bitwarden/common/platform/abstractions/config/config.service";
import { Fido2ActiveRequestManager } from "@bitwarden/common/platform/abstractions/fido2/fido2-active-request-manager.abstraction"; import { Fido2ActiveRequestManager } from "@bitwarden/common/platform/abstractions/fido2/fido2-active-request-manager.abstraction";
@ -29,6 +31,7 @@ import {
} from "./abstractions/fido2.background"; } from "./abstractions/fido2.background";
export class Fido2Background implements Fido2BackgroundInterface { export class Fido2Background implements Fido2BackgroundInterface {
private currentAuthStatus$: Subscription;
private abortManager = new AbortManager(); private abortManager = new AbortManager();
private fido2ContentScriptPortsSet = new Set<chrome.runtime.Port>(); private fido2ContentScriptPortsSet = new Set<chrome.runtime.Port>();
private registeredContentScripts: browser.contentScripts.RegisteredContentScript; private registeredContentScripts: browser.contentScripts.RegisteredContentScript;
@ -55,6 +58,7 @@ export class Fido2Background implements Fido2BackgroundInterface {
private vaultSettingsService: VaultSettingsService, private vaultSettingsService: VaultSettingsService,
private scriptInjectorService: ScriptInjectorService, private scriptInjectorService: ScriptInjectorService,
private configService: ConfigService, private configService: ConfigService,
private authService: AuthService,
) {} ) {}
/** /**
@ -68,12 +72,32 @@ export class Fido2Background implements Fido2BackgroundInterface {
this.vaultSettingsService.enablePasskeys$ this.vaultSettingsService.enablePasskeys$
.pipe(startWith(undefined), pairwise()) .pipe(startWith(undefined), pairwise())
.subscribe(([previous, current]) => this.handleEnablePasskeysUpdate(previous, current)); .subscribe(([previous, current]) => this.handleEnablePasskeysUpdate(previous, current));
this.currentAuthStatus$ = this.authService.activeAccountStatus$
.pipe(startWith(undefined), pairwise())
.subscribe(([_previous, current]) => this.handleAuthStatusUpdate(current));
}
/**
* Handles initializing the FIDO2 content scripts based on the current
* authentication status. We only want to inject the FIDO2 content scripts
* if the user is logged in.
*
* @param authStatus - The current authentication status.
*/
private async handleAuthStatusUpdate(authStatus: AuthenticationStatus) {
if (authStatus === AuthenticationStatus.LoggedOut) {
return;
}
const enablePasskeys = await this.isPasskeySettingEnabled();
await this.handleEnablePasskeysUpdate(enablePasskeys, enablePasskeys);
this.currentAuthStatus$.unsubscribe();
} }
/** /**
* Injects the FIDO2 content and page script into all existing browser tabs. * Injects the FIDO2 content and page script into all existing browser tabs.
*/ */
async injectFido2ContentScriptsInAllTabs() { private async injectFido2ContentScriptsInAllTabs() {
const tabs = await BrowserApi.tabsQuery({}); const tabs = await BrowserApi.tabsQuery({});
for (let index = 0; index < tabs.length; index++) { for (let index = 0; index < tabs.length; index++) {
@ -85,6 +109,13 @@ export class Fido2Background implements Fido2BackgroundInterface {
} }
} }
/**
* Gets the user's authentication status from the auth service.
*/
private async getAuthStatus() {
return await firstValueFrom(this.authService.activeAccountStatus$);
}
/** /**
* Handles reacting to the enablePasskeys setting being updated. If the setting * Handles reacting to the enablePasskeys setting being updated. If the setting
* is enabled, the FIDO2 content scripts are injected into all tabs. If the setting * is enabled, the FIDO2 content scripts are injected into all tabs. If the setting
@ -98,13 +129,17 @@ export class Fido2Background implements Fido2BackgroundInterface {
previousEnablePasskeysSetting: boolean, previousEnablePasskeysSetting: boolean,
enablePasskeys: boolean, enablePasskeys: boolean,
) { ) {
this.fido2ActiveRequestManager.removeAllActiveRequests(); if ((await this.getAuthStatus()) === AuthenticationStatus.LoggedOut) {
await this.updateContentScriptRegistration(); return;
}
if (previousEnablePasskeysSetting === undefined) { if (previousEnablePasskeysSetting === undefined) {
return; return;
} }
this.fido2ActiveRequestManager.removeAllActiveRequests();
await this.updateContentScriptRegistration();
this.destroyLoadedFido2ContentScripts(); this.destroyLoadedFido2ContentScripts();
if (enablePasskeys) { if (enablePasskeys) {
void this.injectFido2ContentScriptsInAllTabs(); void this.injectFido2ContentScriptsInAllTabs();

View File

@ -9,6 +9,7 @@
const script = globalContext.document.createElement("script"); const script = globalContext.document.createElement("script");
script.src = chrome.runtime.getURL("content/fido2-page-script.js"); script.src = chrome.runtime.getURL("content/fido2-page-script.js");
script.async = false;
const scriptInsertionPoint = const scriptInsertionPoint =
globalContext.document.head || globalContext.document.documentElement; globalContext.document.head || globalContext.document.documentElement;

View File

@ -9,6 +9,7 @@
const script = globalContext.document.createElement("script"); const script = globalContext.document.createElement("script");
script.src = chrome.runtime.getURL("content/fido2-page-script.js"); script.src = chrome.runtime.getURL("content/fido2-page-script.js");
script.async = false;
// We are ensuring that the script injection is delayed in the event that we are loading // We are ensuring that the script injection is delayed in the event that we are loading
// within an iframe element. This prevents an issue with web mail clients that load content // within an iframe element. This prevents an issue with web mail clients that load content

View File

@ -4,6 +4,12 @@ import { MessageType } from "./messaging/message";
import { Messenger } from "./messaging/messenger"; import { Messenger } from "./messaging/messenger";
(function (globalContext) { (function (globalContext) {
if (globalContext.document.currentScript) {
globalContext.document.currentScript.parentNode.removeChild(
globalContext.document.currentScript,
);
}
const shouldExecuteContentScript = const shouldExecuteContentScript =
globalContext.document.contentType === "text/html" && globalContext.document.contentType === "text/html" &&
(globalContext.document.location.protocol === "https:" || (globalContext.document.location.protocol === "https:" ||

View File

@ -1103,6 +1103,7 @@ export default class MainBackground {
this.vaultSettingsService, this.vaultSettingsService,
this.scriptInjectorService, this.scriptInjectorService,
this.configService, this.configService,
this.authService,
); );
const lockService = new DefaultLockService(this.accountService, this.vaultTimeoutService); const lockService = new DefaultLockService(this.accountService, this.vaultTimeoutService);
@ -1118,7 +1119,6 @@ export default class MainBackground {
this.messagingService, this.messagingService,
this.logService, this.logService,
this.configService, this.configService,
this.fido2Background,
messageListener, messageListener,
this.accountService, this.accountService,
lockService, lockService,

View File

@ -21,7 +21,6 @@ import {
openTwoFactorAuthPopout, openTwoFactorAuthPopout,
} from "../auth/popup/utils/auth-popout-window"; } from "../auth/popup/utils/auth-popout-window";
import { LockedVaultPendingNotificationsData } from "../autofill/background/abstractions/notification.background"; import { LockedVaultPendingNotificationsData } from "../autofill/background/abstractions/notification.background";
import { Fido2Background } from "../autofill/fido2/background/abstractions/fido2.background";
import { AutofillService } from "../autofill/services/abstractions/autofill.service"; import { AutofillService } from "../autofill/services/abstractions/autofill.service";
import { BrowserApi } from "../platform/browser/browser-api"; import { BrowserApi } from "../platform/browser/browser-api";
import { BrowserEnvironmentService } from "../platform/services/browser-environment.service"; import { BrowserEnvironmentService } from "../platform/services/browser-environment.service";
@ -46,7 +45,6 @@ export default class RuntimeBackground {
private messagingService: MessagingService, private messagingService: MessagingService,
private logService: LogService, private logService: LogService,
private configService: ConfigService, private configService: ConfigService,
private fido2Background: Fido2Background,
private messageListener: MessageListener, private messageListener: MessageListener,
private accountService: AccountService, private accountService: AccountService,
private readonly lockService: LockService, private readonly lockService: LockService,
@ -365,7 +363,6 @@ export default class RuntimeBackground {
private async checkOnInstalled() { private async checkOnInstalled() {
setTimeout(async () => { setTimeout(async () => {
void this.fido2Background.injectFido2ContentScriptsInAllTabs();
void this.autofillService.loadAutofillScriptsOnInstall(); void this.autofillService.loadAutofillScriptsOnInstall();
if (this.onInstalledReason != null) { if (this.onInstalledReason != null) {