Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-10-08 10:44:56 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Backed out changeset ca712de42eca (bug 1829456) for causing xpcshell failures on test_password_engine.js. CLOSED TREE

Browse Source
This commit is contained in:
Cosmin Sabou 2023-05-24 10:31:22 +03:00
parent c7daca2e34
commit 18ddb256a5
11 changed files with 24 additions and 231 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

61
services/sync/modules/engines/passwords.sys.mjs
View File

@ -18,28 +18,6 @@ import { Svc, Utils } from "resource://services-sync/util.sys.mjs";
import { Async } from "resource://services-common/async.sys.mjs";
// These are valid fields the server could have for a logins record
// we mainly use this to detect if there are any unknownFields and
// store (but don't process) those fields to roundtrip them back
const VALID_LOGIN_FIELDS = [
"id",
"displayOrigin",
"formSubmitURL",
"formActionOrigin",
"httpRealm",
"hostname",
"origin",
"password",
"passwordField",
"timeCreated",
"timeLastUsed",
"timePasswordChanged",
"timesUsed",
"username",
"usernameField",
"unknownFields",
];
const SYNCABLE_LOGIN_FIELDS = [
// `nsILoginInfo` fields.
"hostname",
@ -224,26 +202,6 @@ PasswordStore.prototype = {
);
},
// Returns an stringified object of any fields not "known" by this client
// mainly used to to prevent data loss for other clients by roundtripping
// these fields without processing them
_processUnknownFields(record) {
let unknownFields = {};
let keys = Object.keys(record);
keys
.filter(key => !VALID_LOGIN_FIELDS.includes(key))
.forEach(key => {
unknownFields[key] = record[key];
});
// If we found some unknown fields, we stringify it to be able
// to properly encrypt it for roundtripping since we can't know if
// it contained sensitive fields or not
if (Object.keys(unknownFields).length) {
return JSON.stringify(unknownFields);
}
return null;
},
/**
* Return an instance of nsILoginInfo (and, implicitly, nsILoginMetaInfo).
*/
@ -290,12 +248,6 @@ PasswordStore.prototype = {
info.timePasswordChanged = record.timePasswordChanged;
}
// Check the record if there are any unknown fields from other clients
// that we want to roundtrip during sync to prevent data loss
let unknownFields = this._processUnknownFields(record.cleartext);
if (unknownFields) {
info.unknownFields = unknownFields;
}
return info;
},
@ -377,19 +329,6 @@ PasswordStore.prototype = {
record.timeCreated = login.timeCreated;
record.timePasswordChanged = login.timePasswordChanged;
// put the unknown fields back to the top-level record
// during upload
if (login.unknownFields) {
let unknownFields = JSON.parse(login.unknownFields);
if (unknownFields) {
Object.keys(unknownFields).forEach(key => {
// We have to manually add it to the cleartext since that's
// what gets processed during upload
record.cleartext[key] = unknownFields[key];
});
}
}
return record;
},

112
services/sync/tests/unit/test_password_engine.js
View File

@ -239,9 +239,8 @@ add_task(async function test_password_dupe() {
let collection = server.user("foo").collection("passwords");
let guid1 = Utils.makeGUID();
let rec1 = new LoginRec("passwords", guid1);
let guid2 = Utils.makeGUID();
let cleartext = {
let details = {
formSubmitURL: "https://www.example.com",
hostname: "https://www.example.com",
httpRealm: null,
@ -249,19 +248,18 @@ add_task(async function test_password_dupe() {
password: "bar",
usernameField: "username-field",
passwordField: "password-field",
timeCreated: Math.round(Date.now()),
timePasswordChanged: Math.round(Date.now()),
timeCreated: Date.now(),
timePasswordChanged: Date.now(),
};
rec1.cleartext = cleartext;
_("Create remote record with same details and guid1");
collection.insert(guid1, encryptPayload(rec1.cleartext));
collection.insertRecord(Object.assign({}, details, { id: guid1 }));
_("Create remote record with guid2");
collection.insert(guid2, encryptPayload(cleartext));
collection.insertRecord(Object.assign({}, details, { id: guid2 }));
_("Create local record with same details and guid1");
await engine._store.create(rec1);
await engine._store.create(Object.assign({}, details, { id: guid1 }));
try {
_("Perform sync");
@ -400,8 +398,7 @@ add_task(async function test_new_null_password_sync() {
await SyncTestingInfrastructure(server);
let guid1 = Utils.makeGUID();
let rec1 = new LoginRec("passwords", guid1);
rec1.cleartext = {
let details = {
formSubmitURL: "https://www.example.com",
hostname: "https://www.example.com",
httpRealm: null,
@ -415,7 +412,7 @@ add_task(async function test_new_null_password_sync() {
try {
_("Create local login with null password");
await engine._store.create(rec1);
await engine._store.create(Object.assign({}, details, { id: guid1 }));
_("Perform sync");
await sync_engine_and_validate_telem(engine, false);
@ -440,8 +437,7 @@ add_task(async function test_new_undefined_password_sync() {
await SyncTestingInfrastructure(server);
let guid1 = Utils.makeGUID();
let rec1 = new LoginRec("passwords", guid1);
rec1.cleartext = {
let details = {
formSubmitURL: "https://www.example.com",
hostname: "https://www.example.com",
httpRealm: null,
@ -455,7 +451,7 @@ add_task(async function test_new_undefined_password_sync() {
try {
_("Create local login with undefined password");
await engine._store.create(rec1);
await engine._store.create(Object.assign({}, details, { id: guid1 }));
_("Perform sync");
await sync_engine_and_validate_telem(engine, false);
@ -497,91 +493,3 @@ add_task(async function test_sync_password_validation() {
await cleanup(engine, server);
}
});
add_task(async function test_roundtrip_unknown_fields() {
_(
"Testing that unknown fields from other clients get roundtripped back to server"
);
let engine = Service.engineManager.get("passwords");
let server = await serverForFoo(engine);
await SyncTestingInfrastructure(server);
let collection = server.user("foo").collection("passwords");
enableValidationPrefs();
_("Add login with older password change time to replace during first sync");
let oldLogin;
{
let login = new LoginInfo(
"https://mozilla.com",
"",
null,
"us3r",
"0ldpa55",
"",
""
);
Services.logins.addLogin(login);
let props = new PropertyBag();
let localPasswordChangeTime = Math.round(
Date.now() - 1 * 60 * 60 * 24 * 1000
);
props.setProperty("timePasswordChanged", localPasswordChangeTime);
Services.logins.modifyLogin(login, props);
let logins = Services.logins.findLogins("https://mozilla.com", "", "");
equal(logins.length, 1, "Should find old login in login manager");
oldLogin = logins[0].QueryInterface(Ci.nsILoginMetaInfo);
equal(oldLogin.timePasswordChanged, localPasswordChangeTime);
let rec = new LoginRec("passwords", oldLogin.guid);
rec.hostname = oldLogin.origin;
rec.formSubmitURL = oldLogin.formActionOrigin;
rec.httpRealm = oldLogin.httpRealm;
rec.username = oldLogin.username;
// Change the password and bump the password change time to ensure we prefer
// the remote one during reconciliation.
rec.password = "n3wpa55";
rec.usernameField = oldLogin.usernameField;
rec.passwordField = oldLogin.usernameField;
rec.timeCreated = oldLogin.timeCreated;
rec.timePasswordChanged = Math.round(Date.now());
// pretend other clients have some snazzy new fields
// we don't quite understand yet
rec.cleartext.someStrField = "I am a str";
rec.cleartext.someObjField = { newField: "I am a new field" };
collection.insert(oldLogin.guid, encryptPayload(rec.cleartext));
}
await engine._tracker.stop();
try {
await sync_engine_and_validate_telem(engine, false);
let logins = Services.logins.findLogins("https://mozilla.com", "", "");
equal(
logins[0].password,
"n3wpa55",
"Should update local password for older login"
);
let expectedUnknowns = JSON.stringify({
someStrField: "I am a str",
someObjField: { newField: "I am a new field" },
});
// Check that the local record has all unknown fields properly
// stringified
equal(logins[0].unknownFields, expectedUnknowns);
// Check that the server has the unknown fields unfurled and on the
// top-level record
let serverRec = collection.cleartext(oldLogin.guid);
equal(serverRec.someStrField, "I am a str");
equal(serverRec.someObjField.newField, "I am a new field");
} finally {
await cleanup(engine, server);
}
});

22
services/sync/tests/unit/test_password_store.js
View File

@ -1,7 +1,7 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
const { LoginRec } = ChromeUtils.importESModule(
ChromeUtils.importESModule(
"resource://services-sync/engines/passwords.sys.mjs"
);
const { Service } = ChromeUtils.importESModule(
@ -75,8 +75,8 @@ async function changePassword(
recordIsUpdated
) {
const BOGUS_GUID = "zzzzzz" + hostname;
let record = new LoginRec("passwords", BOGUS_GUID);
record.cleartext = {
let record = {
id: BOGUS_GUID,
hostname,
formSubmitURL: hostname,
@ -315,9 +315,7 @@ async function test_LoginRec_toString(store, recordData) {
add_task(async function run_test() {
const BOGUS_GUID_A = "zzzzzzzzzzzz";
const BOGUS_GUID_B = "yyyyyyyyyyyy";
let recordA = new LoginRec("passwords", BOGUS_GUID_A);
let recordB = new LoginRec("passwords", BOGUS_GUID_B);
recordA.cleartext = {
let recordA = {
id: BOGUS_GUID_A,
hostname: "http://foo.bar.com",
formSubmitURL: "http://foo.bar.com",
@ -327,7 +325,7 @@ add_task(async function run_test() {
usernameField: "username",
passwordField: "password",
};
recordB.cleartext = {
let recordB = {
id: BOGUS_GUID_B,
hostname: "http://foo.baz.com",
formSubmitURL: "http://foo.baz.com",
@ -335,7 +333,6 @@ add_task(async function run_test() {
password: "smith",
usernameField: "username",
passwordField: "password",
unknownStr: "an unknown string from another field",
};
let engine = Service.engineManager.get("passwords");
@ -368,15 +365,6 @@ add_task(async function run_test() {
Assert.equal(goodLogins.length, 1);
Assert.equal(badLogins.length, 0);
// applyIncoming should've put any unknown fields from the server
// into a catch-all unknownFields field
Assert.equal(
goodLogins[0].unknownFields,
JSON.stringify({
unknownStr: "an unknown string from another field",
})
);
Assert.ok(!!(await store.getAllIDs())[BOGUS_GUID_B]);
Assert.ok(!(await store.getAllIDs())[BOGUS_GUID_A]);

8
services/sync/tests/unit/test_password_tracker.js
View File

@ -1,7 +1,7 @@
/* Any copyright is dedicated to the Public Domain.
http://creativecommons.org/publicdomain/zero/1.0/ */
const { PasswordEngine, LoginRec } = ChromeUtils.importESModule(
const { PasswordEngine } = ChromeUtils.importESModule(
"resource://services-sync/engines/passwords.sys.mjs"
);
const { Service } = ChromeUtils.importESModule(
@ -28,8 +28,7 @@ add_task(async function test_tracking() {
async function createPassword() {
_("RECORD NUM: " + recordNum);
let record = new LoginRec("passwords", "GUID" + recordNum);
record.cleartext = {
let record = {
id: "GUID" + recordNum,
hostname: "http://foo.bar.com",
formSubmitURL: "http://foo.bar.com",
@ -116,8 +115,7 @@ add_task(async function test_removeAllLogins() {
async function createPassword() {
_("RECORD NUM: " + recordNum);
let record = new LoginRec("passwords", "GUID" + recordNum);
record.cleartext = {
let record = {
id: "GUID" + recordNum,
hostname: "http://foo.bar.com",
formSubmitURL: "http://foo.bar.com",

2
toolkit/components/passwordmgr/LoginHelper.sys.mjs
View File

@ -888,7 +888,6 @@ export const LoginHelper = {
aNewLoginData.usernameField,
aNewLoginData.passwordField
);
newLogin.unknownFields = aNewLoginData.unknownFields;
newLogin.QueryInterface(Ci.nsILoginMetaInfo);
// Automatically update metainfo when password is changed.
@ -920,7 +919,6 @@ export const LoginHelper = {
case "password":
case "usernameField":
case "passwordField":
case "unknownFields":
// nsILoginMetaInfo (fall through)
case "guid":
case "timeCreated":

4
toolkit/components/passwordmgr/LoginInfo.sys.mjs
View File

@ -25,7 +25,6 @@ nsLoginInfo.prototype = {
password: null,
usernameField: null,
passwordField: null,
unknownFields: null,
get displayOrigin() {
let displayOrigin = this.origin;
@ -121,9 +120,6 @@ nsLoginInfo.prototype = {
clone.timePasswordChanged = this.timePasswordChanged;
clone.timesUsed = this.timesUsed;
// Unknown fields from other clients
clone.unknownFields = this.unknownFields;
return clone;
},

7
toolkit/components/passwordmgr/LoginManager.sys.mjs
View File

@ -339,15 +339,12 @@ LoginManager.prototype = {
const crypto = Cc["@mozilla.org/login-manager/crypto/SDR;1"].getService(
Ci.nsILoginManagerCrypto
);
const plaintexts = [login.username, login.password, login.unknownFields];
const [username, password, unknownFields] = await crypto.encryptMany(
plaintexts
);
const plaintexts = [login.username, login.password];
const [username, password] = await crypto.encryptMany(plaintexts);
const { username: plaintextUsername, password: plaintextPassword } = login;
login.username = username;
login.password = password;
login.unknownFields = unknownFields;
lazy.log.debug("Adding login");
return this._storage.addLogin(

2
toolkit/components/passwordmgr/LoginStore.sys.mjs
View File

@ -25,8 +25,6 @@
* "timeLastUsed": 1262304000000,
* "timePasswordChanged": 1262476800000,
* "timesUsed": 1
* // only present if other clients had fields we didn't know about
* "encryptedUnknownFields: "...",
* },
* {
* "id": 4,

6
toolkit/components/passwordmgr/nsILoginInfo.idl
View File

@ -94,12 +94,6 @@ interface nsILoginInfo : nsISupports {
*/
attribute AString passwordField;
/**
* Unknown fields this client doesn't know about but will be roundtripped
* for other clients to prevent data loss
*/
attribute AString unknownFields;
/**
* Initialize a newly created nsLoginInfo object.
*

30
toolkit/components/passwordmgr/storage-json.sys.mjs
View File

@ -176,13 +176,8 @@ export class LoginManagerStorage_json {
// Throws if there are bogus values.
lazy.LoginHelper.checkLoginValues(login);
let [encUsername, encPassword, encType, encUnknownFields] = preEncrypted
? [
login.username,
login.password,
this._crypto.defaultEncType,
login.unknownFields,
]
let [encUsername, encPassword, encType] = preEncrypted
? [login.username, login.password, this._crypto.defaultEncType]
: this._encryptLogin(login);
// Reset the username and password to keep the same guarantees for preEncrypted
@ -248,7 +243,6 @@ export class LoginManagerStorage_json {
timeLastUsed: loginClone.timeLastUsed,
timePasswordChanged: loginClone.timePasswordChanged,
timesUsed: loginClone.timesUsed,
encryptedUnknownFields: encUnknownFields,
});
this._store.saveSoon();
@ -312,8 +306,7 @@ export class LoginManagerStorage_json {
}
// Get the encrypted value of the username and password.
let [encUsername, encPassword, encType, encUnknownFields] =
this._encryptLogin(newLogin);
let [encUsername, encPassword, encType] = this._encryptLogin(newLogin);
for (let loginItem of this._store.data.logins) {
if (loginItem.id == idToModify) {
@ -330,7 +323,6 @@ export class LoginManagerStorage_json {
loginItem.timeLastUsed = newLogin.timeLastUsed;
loginItem.timePasswordChanged = newLogin.timePasswordChanged;
loginItem.timesUsed = newLogin.timesUsed;
loginItem.encryptedUnknownFields = encUnknownFields;
this._store.saveSoon();
break;
}
@ -625,9 +617,6 @@ export class LoginManagerStorage_json {
login.timeLastUsed = loginItem.timeLastUsed;
login.timePasswordChanged = loginItem.timePasswordChanged;
login.timesUsed = loginItem.timesUsed;
// Any unknown fields along for the ride
login.unknownFields = loginItem.encryptedUnknownFields;
foundLogins.push(login);
foundIds.push(loginItem.id);
}
@ -824,16 +813,9 @@ export class LoginManagerStorage_json {
_encryptLogin(login) {
let encUsername = this._crypto.encrypt(login.username);
let encPassword = this._crypto.encrypt(login.password);
// Unknown fields should be encrypted since we can't know whether new fields
// from other clients will contain sensitive data or not
let encUnknownFields = null;
if (login.unknownFields) {
encUnknownFields = this._crypto.encrypt(login.unknownFields);
}
let encType = this._crypto.defaultEncType;
return [encUsername, encPassword, encType, encUnknownFields];
return [encUsername, encPassword, encType];
}
/**
@ -854,10 +836,6 @@ export class LoginManagerStorage_json {
try {
login.username = this._crypto.decrypt(login.username);
login.password = this._crypto.decrypt(login.password);
// Verify unknownFields actually has a value
if (login.unknownFields) {
login.unknownFields = this._crypto.decrypt(login.unknownFields);
}
} catch (e) {
// If decryption failed (corrupt entry?), just skip it.
// Rethrow other errors (like canceling entry of a primary pw)

1
toolkit/components/passwordmgr/test/unit/test_module_LoginExport.js
View File

@ -101,7 +101,6 @@ add_task(async function test_no_new_properties_to_export() {
"usernameField",
"password",
"passwordField",
"unknownFields",
"init",
"equals",
"matches",