From 53c425172b2679ccb0b0f117f0b04d704cb96eaf Mon Sep 17 00:00:00 2001 From: Johannes Salas Schmidt Date: Thu, 4 Jun 2026 17:00:08 +0200 Subject: [PATCH] Bug 2045032 - logins: don't count update() as a password use Login's `update()` incremented `times_used` and set `time_last_used` on every edit. Only touch() should update those fields. --- CHANGELOG.md | 6 +++++ components/logins/src/db.rs | 49 +++++++++++++++++++++++++++++++--- components/logins/src/store.rs | 6 ++--- 3 files changed, 54 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60d90a35c7..59c43c29e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # v153.0 (In progress) +## 🔧 What's Fixed 🔧 + +### Logins + +- `update()` no longer bumps `times_used` or `time_last_used`, this is done only via `touch()`. Verified that both Firefox Android and iOS already track password use via `touch()` and call `update()` only for explicit edits. ([bug 2045032](https://bugzilla.mozilla.org/show_bug.cgi?id=2045032)) + ### Nimbus - Enrollment change events (visible to the UDL) now include the feature IDs of the features involved when possible. ([#7391](https://github.com/mozilla/application-services/pull/7391/)) diff --git a/components/logins/src/db.rs b/components/logins/src/db.rs index 0b29fe215c..19755289d3 100644 --- a/components/logins/src/db.rs +++ b/components/logins/src/db.rs @@ -552,6 +552,7 @@ impl LoginDb { fn update_existing_login(&self, login: &EncryptedLogin) -> Result<()> { // assumes the "local overlay" exists, so the guid must too. + let now_ms = util::system_time_ms_i64(SystemTime::now()); let sql = format!( "UPDATE loginsL SET local_modified = :now_millis, @@ -583,8 +584,7 @@ impl LoginDb { ":time_password_changed": login.meta.time_password_changed, ":sec_fields": login.sec_fields, ":guid": &login.meta.id, - // time_last_used has been set to now. - ":now_millis": login.meta.time_last_used, + ":now_millis": now_ms, }, )?; Ok(()) @@ -755,8 +755,9 @@ impl LoginDb { id: existing.id, time_created: existing.time_created, time_password_changed, - time_last_used: now_ms, - times_used: existing.times_used + 1, + // An edit is not a use (see bug 2045032) + time_last_used: existing.time_last_used, + times_used: existing.times_used, time_last_breach_alert_dismissed: None, }, fields: LoginFields { @@ -1956,6 +1957,46 @@ mod tests { assert_eq!(login2.meta.times_used, login.meta.times_used + 1); } + #[test] + fn test_update_does_not_count_as_use() { + // A plain update is not a password use. + // It must not bump `times_used` or `time_last_used`. Only `touch()` is + // allowed to do that. + ensure_initialized(); + let db = LoginDb::open_in_memory(); + let login = db + .add( + LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password1".into(), + ..Default::default() + }, + &*TEST_ENCDEC, + ) + .unwrap(); + // Make sure the "now" an update would use differs from the add time. + thread::sleep(time::Duration::from_millis(50)); + db.update( + &login.meta.id, + LoginEntry { + origin: "https://www.example.com".into(), + http_realm: Some("https://www.example.com".into()), + username: "user1".into(), + password: "password2".into(), + ..Default::default() + }, + &*TEST_ENCDEC, + ) + .unwrap(); + let updated = db.get_by_id(&login.meta.id).unwrap().unwrap(); + // An edit is not a use: times_used must stay unchanged. + assert_eq!(updated.meta.times_used, login.meta.times_used); + // An edit is not a use: time_last_used must stay unchanged. + assert_eq!(updated.meta.time_last_used, login.meta.time_last_used); + } + #[test] fn test_breach_alert_dismissal() { ensure_initialized(); diff --git a/components/logins/src/store.rs b/components/logins/src/store.rs index aa95bd1157..aa89246bf9 100644 --- a/components/logins/src/store.rs +++ b/components/logins/src/store.rs @@ -499,9 +499,9 @@ mod tests { assert!(b_after_update.time_created >= start_us); assert!(b_after_update.time_created <= now_us); assert!(b_after_update.time_password_changed >= now_us); - assert!(b_after_update.time_last_used >= now_us); - // Should be two even though we updated twice - assert_eq!(b_after_update.times_used, 2); + // An edit is not a use: usage stats are unchanged by update(). + assert_eq!(b_after_update.time_last_used, b_from_db.time_last_used); + assert_eq!(b_after_update.times_used, 1); } #[test]