commit 8e18c77bc4ae2d1c02e2e3198acbd01c5072084b
parent f8cb5f5687a1fe95b88bd8e2840737c3c483ddf4
Author: Duncan McIntosh <dmcintosh@mozilla.com>
Date: Mon, 22 Dec 2025 15:21:42 +0000
Bug 1996249 - Use enableEncryption itself to change the backup password instead of having to disable first. r=kpatenio
Differential Revision: https://phabricator.services.mozilla.com/D272078
Diffstat:
9 files changed, 95 insertions(+), 131 deletions(-)
diff --git a/browser/components/DesktopActorRegistry.sys.mjs b/browser/components/DesktopActorRegistry.sys.mjs
@@ -249,7 +249,6 @@ let JSWINDOWACTORS = {
"BackupUI:RestoreFromBackupChooseFile": { wantUntrusted: true },
"BackupUI:EnableEncryption": { wantUntrusted: true },
"BackupUI:DisableEncryption": { wantUntrusted: true },
- "BackupUI:RerunEncryption": { wantUntrusted: true },
"BackupUI:ShowBackupLocation": { wantUntrusted: true },
"BackupUI:EditBackupLocation": { wantUntrusted: true },
"BackupUI:SetEmbeddedComponentPersistentData": { wantUntrusted: true },
diff --git a/browser/components/backup/BackupService.sys.mjs b/browser/components/backup/BackupService.sys.mjs
@@ -3914,8 +3914,7 @@ export class BackupService extends EventTarget {
/**
* Enables encryption for backups, allowing sensitive data to be backed up.
- * Throws if encryption is already enabled. After enabling encryption, that
- * state is written to disk.
+ * After enabling encryption, the state is written to disk.
*
* @throws Exception
* @param {string} password
@@ -3927,14 +3926,6 @@ export class BackupService extends EventTarget {
*/
async enableEncryption(password, profilePath = PathUtils.profileDir) {
lazy.logConsole.debug("Enabling encryption.");
- let encState = await this.loadEncryptionState(profilePath);
- if (encState) {
- throw new BackupError(
- "Encryption is already enabled.",
- ERRORS.ENCRYPTION_ALREADY_ENABLED
- );
- }
-
if (!password) {
throw new BackupError(
"Cannot supply a blank password.",
@@ -3949,8 +3940,8 @@ export class BackupService extends EventTarget {
);
}
- ({ instance: encState } =
- await lazy.ArchiveEncryptionState.initialize(password));
+ let { instance: encState } =
+ await lazy.ArchiveEncryptionState.initialize(password);
if (!encState) {
throw new BackupError(
"Failed to construct ArchiveEncryptionState",
@@ -3974,7 +3965,7 @@ export class BackupService extends EventTarget {
}
/**
- * Disables encryption of backups. Throws is encryption is already disabled.
+ * Disables encryption of backups.
*
* @throws Exception
* @param {string} [profilePath=PathUtils.profileDir]
@@ -3984,22 +3975,11 @@ export class BackupService extends EventTarget {
*/
async disableEncryption(profilePath = PathUtils.profileDir) {
lazy.logConsole.debug("Disabling encryption.");
- let encState = await this.loadEncryptionState(profilePath);
- if (!encState) {
- throw new BackupError(
- "Encryption is already disabled.",
- ERRORS.ENCRYPTION_ALREADY_DISABLED
- );
- }
-
let encStateFile = PathUtils.join(
profilePath,
BackupService.PROFILE_FOLDER_NAME,
BackupService.ARCHIVE_ENCRYPTION_STATE_FILE
);
- // It'd be pretty strange, but not impossible, for something else to have
- // gotten rid of the encryption state file at this point. We'll ignore it
- // if that's the case.
await IOUtils.remove(encStateFile, {
ignoreAbsent: true,
retryReadonly: true,
diff --git a/browser/components/backup/actors/BackupUIChild.sys.mjs b/browser/components/backup/actors/BackupUIChild.sys.mjs
@@ -125,15 +125,6 @@ export class BackupUIChild extends JSWindowActorChild {
} else {
target.disableEncryptionErrorCode = result.errorCode;
}
- } else if (event.type == "BackupUI:RerunEncryption") {
- const target = event.target;
-
- const result = await this.sendQuery("RerunEncryption", event.detail);
- if (result.success) {
- target.close();
- } else {
- target.rerunEncryptionErrorCode = result.errorCode;
- }
} else if (event.type == "BackupUI:ShowBackupLocation") {
this.sendAsyncMessage("ShowBackupLocation");
} else if (event.type == "BackupUI:EditBackupLocation") {
diff --git a/browser/components/backup/actors/BackupUIParent.sys.mjs b/browser/components/backup/actors/BackupUIParent.sys.mjs
@@ -225,8 +225,13 @@ export class BackupUIParent extends JSWindowActorParent {
return { success: true };
} else if (message.name == "EnableEncryption") {
try {
+ let wasEncrypted = this.#bs.state.encryptionEnabled;
await this.#bs.enableEncryption(message.data.password);
- Glean.browserBackup.passwordAdded.record();
+ if (wasEncrypted) {
+ Glean.browserBackup.passwordChanged.record();
+ } else {
+ Glean.browserBackup.passwordAdded.record();
+ }
} catch (e) {
lazy.logConsole.error(`Failed to enable encryption`, e);
return { success: false, errorCode: e.cause || lazy.ERRORS.UNKNOWN };
@@ -243,19 +248,6 @@ export class BackupUIParent extends JSWindowActorParent {
}
return await this.#triggerCreateBackup({ reason: "encryption" });
- } else if (message.name == "RerunEncryption") {
- try {
- let { password } = message.data;
-
- await this.#bs.disableEncryption();
- await this.#bs.enableEncryption(password);
- Glean.browserBackup.passwordChanged.record();
- } catch (e) {
- lazy.logConsole.error(`Failed to rerun encryption`, e);
- return { success: false, errorCode: e.cause || lazy.ERRORS.UNKNOWN };
- }
-
- return await this.#triggerCreateBackup({ reason: "encryption" });
} else if (message.name == "ShowBackupLocation") {
this.#bs.showBackupLocation();
} else if (message.name == "EditBackupLocation") {
diff --git a/browser/components/backup/content/enable-backup-encryption.mjs b/browser/components/backup/content/enable-backup-encryption.mjs
@@ -61,7 +61,6 @@ export default class EnableBackupEncryption extends MozLitElement {
// managed by BackupUIChild
enableEncryptionErrorCode: { type: Number },
- rerunEncryptionErrorCode: { type: Number },
};
static get queries() {
@@ -83,7 +82,6 @@ export default class EnableBackupEncryption extends MozLitElement {
this._inputPassValue = "";
this._passwordsMatch = false;
this.enableEncryptionErrorCode = 0;
- this.rerunEncryptionErrorCode = 0;
}
connectedCallback() {
@@ -121,28 +119,14 @@ export default class EnableBackupEncryption extends MozLitElement {
}
handleConfirm() {
- switch (this.type) {
- case VALID_TYPES.SET_PASSWORD:
- this.dispatchEvent(
- new CustomEvent("BackupUI:EnableEncryption", {
- bubbles: true,
- detail: {
- password: this._inputPassValue,
- },
- })
- );
- break;
- case VALID_TYPES.CHANGE_PASSWORD:
- this.dispatchEvent(
- new CustomEvent("BackupUI:RerunEncryption", {
- bubbles: true,
- detail: {
- password: this._inputPassValue,
- },
- })
- );
- break;
- }
+ this.dispatchEvent(
+ new CustomEvent("BackupUI:EnableEncryption", {
+ bubbles: true,
+ detail: {
+ password: this._inputPassValue,
+ },
+ })
+ );
}
descriptionTemplate() {
@@ -184,9 +168,7 @@ export default class EnableBackupEncryption extends MozLitElement {
}
errorTemplate() {
- let messageId = this.enableEncryptionErrorCode
- ? getErrorL10nId(this.enableEncryptionErrorCode)
- : getErrorL10nId(this.rerunEncryptionErrorCode);
+ let messageId = getErrorL10nId(this.enableEncryptionErrorCode);
return html`
<moz-message-bar
id="enable-backup-encryption-error"
@@ -218,9 +200,7 @@ export default class EnableBackupEncryption extends MozLitElement {
>
</password-validation-inputs>
- ${this.enableEncryptionErrorCode || this.rerunEncryptionErrorCode
- ? this.errorTemplate()
- : null}
+ ${this.enableEncryptionErrorCode ? this.errorTemplate() : null}
</div>
${this.buttonGroupTemplate()}
</div>
diff --git a/browser/components/backup/content/enable-backup-encryption.stories.mjs b/browser/components/backup/content/enable-backup-encryption.stories.mjs
@@ -29,24 +29,14 @@ export default {
mapping: SELECTABLE_ERRORS,
control: { type: "select" },
},
- rerunEncryptionErrorCode: {
- options: Object.keys(SELECTABLE_ERRORS),
- mapping: SELECTABLE_ERRORS,
- control: { type: "select" },
- },
},
};
-const Template = ({
- type,
- enableEncryptionErrorCode,
- rerunEncryptionErrorCode,
-}) => html`
+const Template = ({ type, enableEncryptionErrorCode }) => html`
<moz-card style="width: 23.94rem; position: relative;">
<enable-backup-encryption
type=${type}
.enableEncryptionErrorCode=${enableEncryptionErrorCode}
- .rerunEncryptionErrorCode=${rerunEncryptionErrorCode}
></enable-backup-encryption>
</moz-card>
`;
@@ -66,9 +56,3 @@ SetPasswordError.args = {
type: "set-password",
enableEncryptionErrorCode: ERRORS.INVALID_PASSWORD,
};
-
-export const ChangePasswordError = Template.bind({});
-ChangePasswordError.args = {
- type: "change-password",
- rerunEncryptionErrorCode: ERRORS.INVALID_PASSWORD,
-};
diff --git a/browser/components/backup/tests/browser/browser_settings_enable_backup_encryption.js b/browser/components/backup/tests/browser/browser_settings_enable_backup_encryption.js
@@ -155,9 +155,6 @@ add_task(
let enableEncryptionStub = sandbox
.stub(BackupService.prototype, "enableEncryption")
.resolves(true);
- let disableEncryptionStub = sandbox
- .stub(BackupService.prototype, "disableEncryption")
- .resolves(true);
let createBackupStub = sandbox
.stub(BackupService.prototype, "createBackup")
.resolves(true);
@@ -228,18 +225,20 @@ add_task(
await settings.updateComplete;
confirmButton = settings.enableBackupEncryptionEl.confirmButtonEl;
+ let initialState = BackupService.get().state;
+ sandbox.stub(BackupService.get(), "state").get(() => ({
+ ...initialState,
+ encryptionEnabled: true,
+ }));
+
let promise = BrowserTestUtils.waitForEvent(
window,
- "BackupUI:RerunEncryption"
+ "BackupUI:EnableEncryption"
);
confirmButton.click();
await promise;
Assert.ok(
- disableEncryptionStub.calledOnce,
- "BackupService was called to disable encryption first before registering the changed password"
- );
- Assert.ok(
enableEncryptionStub.calledOnceWith(MOCK_PASSWORD),
"BackupService was called to re-run encryption with changed password"
);
diff --git a/browser/components/backup/tests/chrome/test_enable_backup_encryption.html b/browser/components/backup/tests/chrome/test_enable_backup_encryption.html
@@ -87,7 +87,7 @@
ok(!confirmButton.disabled, "Confirm button should no longer be disabled");
let content = document.getElementById("content");
- let encryptionPromise = BrowserTestUtils.waitForEvent(content, "BackupUI:RerunEncryption");
+ let encryptionPromise = BrowserTestUtils.waitForEvent(content, "BackupUI:EnableEncryption");
confirmButton.click()
diff --git a/browser/components/backup/tests/xpcshell/test_BackupService_enable_disable_encryption.js b/browser/components/backup/tests/xpcshell/test_BackupService_enable_disable_encryption.js
@@ -4,6 +4,54 @@ https://creativecommons.org/publicdomain/zero/1.0/ */
"use strict";
const TEST_PASSWORD = "This is some test password.";
+const TEST_PASSWORD_ALT = "mozilla.org";
+
+/**
+ * Creates a backup with this backup service and checks whether it can be
+ * decrypted with the given password. This ensures that encryption is
+ * enabled/disabled as needed, and that the backup service will actually use
+ * the given password if applicable.
+ *
+ * @param {BackupService} bs
+ * The BackupService to use.
+ * @param {string} profilePath
+ * The path to the current profile.
+ * @param {string?} password
+ * The expected password, or null if it should not be encrypted.
+ * @returns {boolean}
+ * True if the password matched, false if something went wrong.
+ */
+async function backupServiceUsesPassword(bs, profilePath, password = null) {
+ try {
+ const backup = await bs.createBackup({ profilePath });
+ let dest = await IOUtils.createUniqueFile(
+ PathUtils.parent(backup.archivePath),
+ "extracted-" + PathUtils.filename(backup.archivePath)
+ );
+
+ let { isEncrypted } = await bs.sampleArchive(backup.archivePath);
+ let shouldBeEncrypted = password != null;
+ Assert.equal(
+ isEncrypted,
+ shouldBeEncrypted,
+ `Archive is ${shouldBeEncrypted ? "" : "not "}encrypted`
+ );
+
+ // This should throw if the password is incorrect.
+ await bs.extractCompressedSnapshotFromArchive(
+ backup.archivePath,
+ dest,
+ password
+ );
+
+ await IOUtils.remove(backup.archivePath);
+ await IOUtils.remove(dest);
+ return true;
+ } catch (e) {
+ console.error(e);
+ return false;
+ }
+}
add_setup(async () => {
// Much of this setup is copied from toolkit/profile/xpcshell/head.js. It is
@@ -117,27 +165,6 @@ add_task(async function test_disabled_encryption() {
});
/**
- * Tests that encryption cannot be disabled if it's already disabled.
- */
-add_task(async function test_already_disabled_encryption() {
- let bs = new BackupService();
-
- Assert.ok(
- !bs.state.encryptionEnabled,
- "State should indicate that encryption is disabled."
- );
-
- let encState = await bs.loadEncryptionState();
- Assert.ok(!encState, "Should not find an ArchiveEncryptionState.");
-
- await Assert.rejects(
- bs.disableEncryption(),
- /already disabled/,
- "It should not be possible to disable encryption if it's already disabled"
- );
-});
-
-/**
* Tests that if encryption is enabled from a non-enabled state, that an
* ArchiveEncryptionState is created, and state is written to the profile
* directory. Also tests that this allows BackupResource's with
@@ -235,9 +262,10 @@ add_task(async function test_enable_encryption() {
});
/**
- * Tests that encryption cannot be enabled if it's already enabled.
+ * Tests that enabling encryption while it is already enabled changes the
+ * password.
*/
-add_task(async function test_already_enabled_encryption() {
+add_task(async function test_change_encryption_password() {
let bs = new BackupService();
let testProfilePath = await IOUtils.createUniqueDirectory(
@@ -250,11 +278,18 @@ add_task(async function test_already_enabled_encryption() {
let encState = await bs.loadEncryptionState(testProfilePath);
Assert.ok(encState, "ArchiveEncryptionState is available.");
+ Assert.ok(
+ await backupServiceUsesPassword(bs, testProfilePath, TEST_PASSWORD),
+ "BackupService is using TEST_PASSWORD"
+ );
- await Assert.rejects(
- bs.enableEncryption(TEST_PASSWORD, testProfilePath),
- /already enabled/,
- "It should not be possible to enable encryption if it's already enabled"
+ // Change the password.
+ await bs.enableEncryption(TEST_PASSWORD_ALT, testProfilePath);
+ encState = await bs.loadEncryptionState(testProfilePath);
+ Assert.ok(encState, "ArchiveEncryptionState is still available.");
+ Assert.ok(
+ await backupServiceUsesPassword(bs, testProfilePath, TEST_PASSWORD_ALT),
+ "BackupService is using TEST_PASSWORD_ALT"
);
await IOUtils.remove(testProfilePath, { recursive: true });
@@ -313,6 +348,10 @@ add_task(async function test_disabling_encryption() {
)),
"Encryption state file should have been removed."
);
+ Assert.ok(
+ await backupServiceUsesPassword(bs, testProfilePath, null),
+ "BackupService should not use encryption."
+ );
await IOUtils.remove(testProfilePath, { recursive: true });
});
