[PM-7807][PM-7617] [PM-6185] Firefox private mode out of experimentation (#8921)

* Remove getbgService for crypto service

* Remove special authentication for state service

* Use synced memory storage

popup contexts use foreground, background contexts use background. Simple

* Remove private mode warnings
This commit is contained in:
Matt Gibson 2024-05-01 07:59:30 -04:00 committed by GitHub
parent b4631b0dd1
commit 7e9ab6a15b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
13 changed files with 53 additions and 106 deletions

View File

@ -1120,9 +1120,6 @@
"commandLockVaultDesc": {
"message": "Lock the vault"
},
"privateModeWarning": {
"message": "Private mode support is experimental and some features are limited."
},
"customFields": {
"message": "Custom fields"
},

View File

@ -89,7 +89,6 @@
<p class="text-center" *ngIf="!fido2Data.isFido2Session">
<button type="button" appStopClick (click)="logOut()">{{ "logOut" | i18n }}</button>
</p>
<app-private-mode-warning></app-private-mode-warning>
<app-callout *ngIf="biometricError" type="error">{{ biometricError }}</app-callout>
<p class="text-center text-muted" *ngIf="pendingBiometric">
<i class="bwi bwi-spinner bwi-spin" aria-hidden="true"></i> {{ "awaitDesktop" | i18n }}

View File

@ -57,7 +57,6 @@
</button>
</div>
</div>
<app-private-mode-warning></app-private-mode-warning>
<div class="content login-buttons">
<button type="submit" class="btn primary block" [disabled]="form.loading">
<span [hidden]="form.loading"

View File

@ -109,7 +109,6 @@ import { EncryptServiceImplementation } from "@bitwarden/common/platform/service
import { MultithreadEncryptServiceImplementation } from "@bitwarden/common/platform/services/cryptography/multithread-encrypt.service.implementation";
import { FileUploadService } from "@bitwarden/common/platform/services/file-upload/file-upload.service";
import { KeyGenerationService } from "@bitwarden/common/platform/services/key-generation.service";
import { MemoryStorageService } from "@bitwarden/common/platform/services/memory-storage.service";
import { MigrationBuilderService } from "@bitwarden/common/platform/services/migration-builder.service";
import { MigrationRunner } from "@bitwarden/common/platform/services/migration-runner";
import { SystemService } from "@bitwarden/common/platform/services/system.service";
@ -356,10 +355,7 @@ export default class MainBackground {
private isSafari: boolean;
private nativeMessagingBackground: NativeMessagingBackground;
constructor(
public isPrivateMode: boolean = false,
public popupOnlyContext: boolean = false,
) {
constructor(public popupOnlyContext: boolean = false) {
// Services
const lockedCallback = async (userId?: string) => {
if (this.notificationsService != null) {
@ -443,10 +439,14 @@ export default class MainBackground {
this.secureStorageService = this.storageService; // secure storage is not supported in browsers, so we use local storage and warn users when it is used
this.memoryStorageForStateProviders = BrowserApi.isManifestVersion(3)
? new BrowserMemoryStorageService() // mv3 stores to storage.session
: new BackgroundMemoryStorageService(); // mv2 stores to memory
: popupOnlyContext
? new ForegroundMemoryStorageService()
: new BackgroundMemoryStorageService(); // mv2 stores to memory
this.memoryStorageService = BrowserApi.isManifestVersion(3)
? this.memoryStorageForStateProviders // manifest v3 can reuse the same storage. They are split for v2 due to lacking a good sync mechanism, which isn't true for v3
: new MemoryStorageService();
: popupOnlyContext
? new ForegroundMemoryStorageService()
: new BackgroundMemoryStorageService();
this.largeObjectMemoryStorageForStateProviders = BrowserApi.isManifestVersion(3)
? mv3MemoryStorageCreator() // mv3 stores to local-backed session storage
: this.memoryStorageForStateProviders; // mv2 stores to the same location
@ -1109,27 +1109,9 @@ export default class MainBackground {
await this.idleBackground.init();
this.webRequestBackground?.startListening();
if (this.platformUtilsService.isFirefox() && !this.isPrivateMode) {
// Set Private Mode windows to the default icon - they do not share state with the background page
const privateWindows = await BrowserApi.getPrivateModeWindows();
privateWindows.forEach(async (win) => {
await new UpdateBadge(self).setBadgeIcon("", win.id);
});
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
BrowserApi.onWindowCreated(async (win) => {
if (win.incognito) {
await new UpdateBadge(self).setBadgeIcon("", win.id);
}
});
}
return new Promise<void>((resolve) => {
setTimeout(async () => {
if (!this.isPrivateMode) {
await this.refreshBadge();
}
await this.refreshBadge();
await this.fullSync(true);
setTimeout(() => this.notificationsService.init(), 2500);
resolve();

View File

@ -204,10 +204,6 @@ export class BrowserApi {
chrome.tabs.sendMessage<TabMessage, T>(tabId, message, options, responseCallback);
}
static async getPrivateModeWindows(): Promise<browser.windows.Window[]> {
return (await browser.windows.getAll()).filter((win) => win.incognito);
}
static async onWindowCreated(callback: (win: chrome.windows.Window) => any) {
// FIXME: Make sure that is does not cause a memory leak in Safari or use BrowserApi.AddListener
// and test that it doesn't break.

View File

@ -138,28 +138,6 @@ describe("BrowserPopupUtils", () => {
});
});
describe("inPrivateMode", () => {
it("returns false if the background requires initialization", () => {
jest.spyOn(BrowserPopupUtils, "backgroundInitializationRequired").mockReturnValue(false);
expect(BrowserPopupUtils.inPrivateMode()).toBe(false);
});
it("returns false if the manifest version is for version 3", () => {
jest.spyOn(BrowserPopupUtils, "backgroundInitializationRequired").mockReturnValue(true);
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(3);
expect(BrowserPopupUtils.inPrivateMode()).toBe(false);
});
it("returns true if the background does not require initalization and the manifest version is version 2", () => {
jest.spyOn(BrowserPopupUtils, "backgroundInitializationRequired").mockReturnValue(true);
jest.spyOn(BrowserApi, "manifestVersion", "get").mockReturnValue(2);
expect(BrowserPopupUtils.inPrivateMode()).toBe(true);
});
});
describe("openPopout", () => {
beforeEach(() => {
jest.spyOn(BrowserApi, "getWindow").mockResolvedValueOnce({

View File

@ -89,13 +89,6 @@ class BrowserPopupUtils {
return !BrowserApi.getBackgroundPage();
}
/**
* Identifies if the popup is loading in private mode.
*/
static inPrivateMode() {
return BrowserPopupUtils.backgroundInitializationRequired() && !BrowserApi.isManifestVersion(3);
}
/**
* Opens a popout window of any extension page. If the popout window is already open, it will be focused.
*

View File

@ -62,15 +62,6 @@ export class DefaultBrowserStateService
await super.addAccount(account);
}
async getIsAuthenticated(options?: StorageOptions): Promise<boolean> {
// Firefox Private Mode can clash with non-Private Mode because they both read from the same onDiskOptions
// Check that there is an account in memory before considering the user authenticated
return (
(await super.getIsAuthenticated(options)) &&
(await this.getAccount(await this.defaultInMemoryOptions())) != null
);
}
// Overriding the base class to prevent deleting the cache on save. We register a storage listener
// to delete the cache in the constructor above.
protected override async saveAccountToDisk(

View File

@ -70,7 +70,6 @@ import { FolderAddEditComponent } from "../vault/popup/settings/folder-add-edit.
import { AppRoutingModule } from "./app-routing.module";
import { AppComponent } from "./app.component";
import { PopOutComponent } from "./components/pop-out.component";
import { PrivateModeWarningComponent } from "./components/private-mode-warning.component";
import { UserVerificationComponent } from "./components/user-verification.component";
import { ServicesModule } from "./services/services.module";
import { ExcludedDomainsComponent } from "./settings/excluded-domains.component";
@ -150,7 +149,6 @@ import "../platform/popup/locales";
PasswordHistoryComponent,
PopOutComponent,
PremiumComponent,
PrivateModeWarningComponent,
RegisterComponent,
SendAddEditComponent,
SendGroupingsComponent,

View File

@ -1,6 +0,0 @@
<app-callout class="app-private-mode-warning" type="warning" *ngIf="showWarning">
{{ "privateModeWarning" | i18n }}
<a href="https://bitwarden.com/help/article/private-mode/" target="_blank" rel="noreferrer">{{
"learnMore" | i18n
}}</a>
</app-callout>

View File

@ -1,15 +0,0 @@
import { Component, OnInit } from "@angular/core";
import BrowserPopupUtils from "../../platform/popup/browser-popup-utils";
@Component({
selector: "app-private-mode-warning",
templateUrl: "private-mode-warning.component.html",
})
export class PrivateModeWarningComponent implements OnInit {
showWarning = false;
ngOnInit() {
this.showWarning = BrowserPopupUtils.inPrivateMode();
}
}

View File

@ -111,11 +111,6 @@ app-home {
}
}
.app-private-mode-warning {
display: block;
padding-top: 1rem;
}
body.body-sm,
body.body-xs {
app-home {

View File

@ -28,7 +28,9 @@ import { AccountService as AccountServiceAbstraction } from "@bitwarden/common/a
import { AuthService as AuthServiceAbstraction } from "@bitwarden/common/auth/abstractions/auth.service";
import { DeviceTrustServiceAbstraction } from "@bitwarden/common/auth/abstractions/device-trust.service.abstraction";
import { DevicesServiceAbstraction } from "@bitwarden/common/auth/abstractions/devices/devices.service.abstraction";
import { KdfConfigService } from "@bitwarden/common/auth/abstractions/kdf-config.service";
import { KeyConnectorService } from "@bitwarden/common/auth/abstractions/key-connector.service";
import { InternalMasterPasswordServiceAbstraction } from "@bitwarden/common/auth/abstractions/master-password.service.abstraction";
import { SsoLoginServiceAbstraction } from "@bitwarden/common/auth/abstractions/sso-login.service.abstraction";
import { TokenService } from "@bitwarden/common/auth/abstractions/token.service";
import { UserVerificationService } from "@bitwarden/common/auth/abstractions/user-verification/user-verification.service.abstraction";
@ -53,6 +55,7 @@ import { EncryptService } from "@bitwarden/common/platform/abstractions/encrypt.
import { EnvironmentService } from "@bitwarden/common/platform/abstractions/environment.service";
import { FileDownloadService } from "@bitwarden/common/platform/abstractions/file-download/file-download.service";
import { I18nService as I18nServiceAbstraction } from "@bitwarden/common/platform/abstractions/i18n.service";
import { KeyGenerationService } from "@bitwarden/common/platform/abstractions/key-generation.service";
import { LogService } from "@bitwarden/common/platform/abstractions/log.service";
import { PlatformUtilsService } from "@bitwarden/common/platform/abstractions/platform-utils.service";
import { StateService as BaseStateServiceAbstraction } from "@bitwarden/common/platform/abstractions/state.service";
@ -61,6 +64,7 @@ import {
AbstractStorageService,
ObservableStorageService,
} from "@bitwarden/common/platform/abstractions/storage.service";
import { BiometricStateService } from "@bitwarden/common/platform/biometrics/biometric-state.service";
import { StateFactory } from "@bitwarden/common/platform/factories/state-factory";
import { Message, MessageListener, MessageSender } from "@bitwarden/common/platform/messaging";
// eslint-disable-next-line no-restricted-imports -- Used for dependency injection
@ -100,6 +104,7 @@ import BrowserPopupUtils from "../../platform/popup/browser-popup-utils";
import { BrowserFileDownloadService } from "../../platform/popup/services/browser-file-download.service";
import { BrowserStateService as StateServiceAbstraction } from "../../platform/services/abstractions/browser-state.service";
import { ScriptInjectorService } from "../../platform/services/abstractions/script-injector.service";
import { BrowserCryptoService } from "../../platform/services/browser-crypto.service";
import { BrowserEnvironmentService } from "../../platform/services/browser-environment.service";
import BrowserLocalStorageService from "../../platform/services/browser-local-storage.service";
import { BrowserScriptInjectorService } from "../../platform/services/browser-script-injector.service";
@ -125,13 +130,12 @@ const OBSERVABLE_LARGE_OBJECT_MEMORY_STORAGE = new SafeInjectionToken<
>("OBSERVABLE_LARGE_OBJECT_MEMORY_STORAGE");
const needsBackgroundInit = BrowserPopupUtils.backgroundInitializationRequired();
const isPrivateMode = BrowserPopupUtils.inPrivateMode();
const mainBackground: MainBackground = needsBackgroundInit
? createLocalBgService()
: BrowserApi.getBackgroundPage().bitwardenMain;
function createLocalBgService() {
const localBgService = new MainBackground(isPrivateMode, true);
const localBgService = new MainBackground(true);
// FIXME: Verify that this floating promise is intentional. If it is, add an explanatory comment and ensure there is proper error handling.
// eslint-disable-next-line @typescript-eslint/no-floating-promises
localBgService.bootstrap();
@ -220,12 +224,48 @@ const safeProviders: SafeProvider[] = [
}),
safeProvider({
provide: CryptoService,
useFactory: (encryptService: EncryptService) => {
const cryptoService = getBgService<CryptoService>("cryptoService")();
useFactory: (
masterPasswordService: InternalMasterPasswordServiceAbstraction,
keyGenerationService: KeyGenerationService,
cryptoFunctionService: CryptoFunctionService,
encryptService: EncryptService,
platformUtilsService: PlatformUtilsService,
logService: LogService,
stateService: StateServiceAbstraction,
accountService: AccountServiceAbstraction,
stateProvider: StateProvider,
biometricStateService: BiometricStateService,
kdfConfigService: KdfConfigService,
) => {
const cryptoService = new BrowserCryptoService(
masterPasswordService,
keyGenerationService,
cryptoFunctionService,
encryptService,
platformUtilsService,
logService,
stateService,
accountService,
stateProvider,
biometricStateService,
kdfConfigService,
);
new ContainerService(cryptoService, encryptService).attachToGlobal(self);
return cryptoService;
},
deps: [EncryptService],
deps: [
InternalMasterPasswordServiceAbstraction,
KeyGenerationService,
CryptoFunctionService,
EncryptService,
PlatformUtilsService,
LogService,
StateServiceAbstraction,
AccountServiceAbstraction,
StateProvider,
BiometricStateService,
KdfConfigService,
],
}),
safeProvider({
provide: TotpServiceAbstraction,