Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# v148.0 (In progress)

### Logins
- Add breach alert support, including a database migration to version 3,
new `Login` fields (`time_of_last_breach`, `time_last_breach_alert_dismissed`),
and new `LoginStore` APIs (`record_breach`, `reset_all_breaches`, `is_potentially_breached`, `record_breach_alert_dismissal`, `is_breach_alert_dismissed`). ([#7127](https://github.com/mozilla/application-services/pull/7127))

[Full Changelog](In progress)

### Ads Client
Expand Down
170 changes: 159 additions & 11 deletions components/logins/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -306,6 +306,74 @@ impl LoginDb {
Ok(())
}

pub fn record_breach(&self, id: &str, timestamp: i64) -> Result<()> {
let tx = self.unchecked_transaction()?;
self.ensure_local_overlay_exists(id)?;
self.mark_mirror_overridden(id)?;
self.execute_cached(
"UPDATE loginsL
SET timeOfLastBreach = :now_millis
WHERE guid = :guid
AND is_deleted = 0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the is_deleted = 0 part could be removed. If somehow a deleted login gets merged with a non-deleted login and the merge code decides to go with the non-deleted version, then maybe we'd want to merge the timeOfLastBreach value. That's a lot of "ifs" and "maybes". I can't think of a real scenario where that would happen, but I can't think of any downsides either. I trust your judgement on this one, just wanted to flag it.

named_params! {
":now_millis": timestamp,
":guid": id,
},
)?;
tx.commit()?;
Ok(())
}

pub fn is_potentially_breached(&self, id: &str) -> Result<bool> {
let is_potentially_breached: bool = self.db.query_row(
"SELECT EXISTS(SELECT 1 FROM loginsL WHERE guid = :guid AND timeOfLastBreach IS NOT NULL AND timeOfLastBreach > timePasswordChanged)",
named_params! { ":guid": id },
|row| row.get(0),
)?;
Ok(is_potentially_breached)
}

pub fn reset_all_breaches(&self) -> Result<()> {
let tx = self.unchecked_transaction()?;
self.execute_cached(
"UPDATE loginsL
SET timeOfLastBreach = NULL
WHERE timeOfLastBreach IS NOT NULL
AND is_deleted = 0",
[],
)?;
tx.commit()?;
Ok(())
}

pub fn is_breach_alert_dismissed(&self, id: &str) -> Result<bool> {
let is_breach_alert_dismissed: bool = self.db.query_row(
"SELECT EXISTS(SELECT 1 FROM loginsL WHERE guid = :guid AND timeOfLastBreach < timeLastBreachAlertDismissed)",
named_params! { ":guid": id },
|row| row.get(0),
)?;
Ok(is_breach_alert_dismissed)
}

pub fn record_breach_alert_dismissal(&self, id: &str) -> Result<()> {
let tx = self.unchecked_transaction()?;
self.ensure_local_overlay_exists(id)?;
self.mark_mirror_overridden(id)?;
let now_ms = util::system_time_ms_i64(SystemTime::now());
self.execute_cached(
"UPDATE loginsL
SET timeLastBreachAlertDismissed = :now_millis
WHERE guid = :guid
AND is_deleted = 0",
named_params! {
":now_millis": now_ms,
":guid": id,
},
)?;
tx.commit()?;
Ok(())
}

// The single place we insert new rows or update existing local rows.
// just the SQL - no validation or anything.
fn insert_new_login(&self, login: &EncryptedLogin) -> Result<()> {
Expand All @@ -322,6 +390,8 @@ impl LoginDb {
timeCreated,
timeLastUsed,
timePasswordChanged,
timeOfLastBreach,
timeLastBreachAlertDismissed,
local_modified,
is_deleted,
sync_status
Expand All @@ -337,6 +407,8 @@ impl LoginDb {
:time_created,
:time_last_used,
:time_password_changed,
:time_of_last_breach,
:time_last_breach_alert_dismissed,
:local_modified,
0, -- is_deleted
{new} -- sync_status
Expand All @@ -356,6 +428,8 @@ impl LoginDb {
":times_used": login.meta.times_used,
":time_last_used": login.meta.time_last_used,
":time_password_changed": login.meta.time_password_changed,
":time_of_last_breach": login.fields.time_of_last_breach,
":time_last_breach_alert_dismissed": login.fields.time_last_breach_alert_dismissed,
":local_modified": login.meta.time_created,
":sec_fields": login.sec_fields,
":guid": login.guid(),
Expand All @@ -368,18 +442,20 @@ impl LoginDb {
// assumes the "local overlay" exists, so the guid must too.
let sql = format!(
"UPDATE loginsL
SET local_modified = :now_millis,
timeLastUsed = :time_last_used,
timePasswordChanged = :time_password_changed,
httpRealm = :http_realm,
formActionOrigin = :form_action_origin,
usernameField = :username_field,
passwordField = :password_field,
timesUsed = :times_used,
secFields = :sec_fields,
origin = :origin,
SET local_modified = :now_millis,
timeLastUsed = :time_last_used,
timePasswordChanged = :time_password_changed,
timeOfLastBreach = :time_of_last_breach,
timeLastBreachAlertDismissed = :time_last_breach_alert_dismissed,
httpRealm = :http_realm,
formActionOrigin = :form_action_origin,
usernameField = :username_field,
passwordField = :password_field,
timesUsed = :times_used,
secFields = :sec_fields,
origin = :origin,
-- leave New records as they are, otherwise update them to `changed`
sync_status = max(sync_status, {changed})
sync_status = max(sync_status, {changed})
WHERE guid = :guid",
changed = SyncStatus::Changed as u8
);
Expand All @@ -397,6 +473,8 @@ impl LoginDb {
":time_password_changed": login.meta.time_password_changed,
":sec_fields": login.sec_fields,
":guid": &login.meta.id,
":time_of_last_breach": login.fields.time_of_last_breach,
":time_last_breach_alert_dismissed": login.fields.time_last_breach_alert_dismissed,
// time_last_used has been set to now.
":now_millis": login.meta.time_last_used,
},
Expand Down Expand Up @@ -459,6 +537,8 @@ impl LoginDb {
http_realm: new_entry.http_realm,
username_field: new_entry.username_field,
password_field: new_entry.password_field,
time_of_last_breach: None,
time_last_breach_alert_dismissed: None,
},
sec_fields,
};
Expand Down Expand Up @@ -573,6 +653,8 @@ impl LoginDb {
http_realm: entry.http_realm,
username_field: entry.username_field,
password_field: entry.password_field,
time_of_last_breach: None,
time_last_breach_alert_dismissed: None,
},
sec_fields,
};
Expand Down Expand Up @@ -1018,6 +1100,9 @@ pub mod test_utils {
timePasswordChanged,
timeCreated,

timeOfLastBreach,
timeLastBreachAlertDismissed,

guid
) VALUES (
:is_overridden,
Expand All @@ -1035,6 +1120,9 @@ pub mod test_utils {
:time_password_changed,
:time_created,

:time_of_last_breach,
:time_last_breach_alert_dismissed,

:guid
)";
let mut stmt = db.prepare_cached(sql)?;
Expand All @@ -1052,6 +1140,8 @@ pub mod test_utils {
":time_last_used": login.meta.time_last_used,
":time_password_changed": login.meta.time_password_changed,
":time_created": login.meta.time_created,
":time_of_last_breach": login.fields.time_of_last_breach,
":time_last_breach_alert_dismissed": login.fields.time_last_breach_alert_dismissed,
":guid": login.guid_str(),
})?;
Ok(())
Expand Down Expand Up @@ -1678,6 +1768,64 @@ mod tests {
assert_eq!(login2.meta.times_used, login.meta.times_used + 1);
}

#[test]
fn test_breach_alerts() {
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();
// initial state
assert!(login.fields.time_of_last_breach.is_none());
assert!(!db.is_potentially_breached(&login.meta.id).unwrap());
assert!(login.fields.time_last_breach_alert_dismissed.is_none());

// set
let now_ms = util::system_time_ms_i64(SystemTime::now());
db.record_breach(&login.meta.id, now_ms).unwrap();
assert!(db.is_potentially_breached(&login.meta.id).unwrap());
let login1 = db.get_by_id(&login.meta.id).unwrap().unwrap();
assert!(login1.fields.time_of_last_breach.is_some());

// dismiss
db.record_breach_alert_dismissal(&login.meta.id).unwrap();
let login2 = db.get_by_id(&login.meta.id).unwrap().unwrap();
assert!(login2.fields.time_last_breach_alert_dismissed.is_some());

// reset
db.reset_all_breaches().unwrap();
assert!(!db.is_potentially_breached(&login.meta.id).unwrap());
let login3 = db.get_by_id(&login.meta.id).unwrap().unwrap();
assert!(login3.fields.time_of_last_breach.is_none());

// set again
let now_ms = util::system_time_ms_i64(SystemTime::now());
db.record_breach(&login.meta.id, now_ms).unwrap();
assert!(db.is_potentially_breached(&login.meta.id).unwrap());

// now change password
db.update(
&login.meta.id.clone(),
LoginEntry {
password: "changed-password".into(),
..login.clone().decrypt(&*TEST_ENCDEC).unwrap().entry()
},
&*TEST_ENCDEC,
)
.unwrap();
// not breached anymore
assert!(!db.is_potentially_breached(&login.meta.id).unwrap());
}

#[test]
fn test_delete() {
ensure_initialized();
Expand Down
3 changes: 3 additions & 0 deletions components/logins/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,9 @@ pub enum Error {

#[error("Migration Error: {0}")]
MigrationError(String),

#[error("IncompatibleVersion: {0}")]
IncompatibleVersion(i64),
}

/// Error::InvalidLogin subtypes
Expand Down
18 changes: 18 additions & 0 deletions components/logins/src/login.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,8 @@ pub struct LoginFields {
pub http_realm: Option<String>,
pub username_field: String,
pub password_field: String,
pub time_of_last_breach: Option<i64>,
pub time_last_breach_alert_dismissed: Option<i64>,
}

/// LoginEntry fields that are stored encrypted
Expand Down Expand Up @@ -355,6 +357,9 @@ pub struct LoginEntryWithMeta {
}

/// A bulk insert result entry, returned by `add_many` and `add_many_with_records`
/// Please note that although the success case is much larger than the error case, this is
/// negligible in real life, as we expect a very small success/error ratio.
#[allow(clippy::large_enum_variant)]
pub enum BulkResultEntry {
Success { login: Login },
Error { message: String },
Expand Down Expand Up @@ -465,6 +470,10 @@ pub struct Login {
// secure fields
pub username: String,
pub password: String,

// breach alerts
pub time_of_last_breach: Option<i64>,
pub time_last_breach_alert_dismissed: Option<i64>,
}

impl Login {
Expand All @@ -484,6 +493,9 @@ impl Login {

username: sec_fields.username,
password: sec_fields.password,

time_of_last_breach: fields.time_last_breach_alert_dismissed,
time_last_breach_alert_dismissed: fields.time_last_breach_alert_dismissed,
}
}

Expand Down Expand Up @@ -525,6 +537,8 @@ impl Login {
http_realm: self.http_realm,
username_field: self.username_field,
password_field: self.password_field,
time_of_last_breach: self.time_last_breach_alert_dismissed,
time_last_breach_alert_dismissed: self.time_last_breach_alert_dismissed,
},
sec_fields,
})
Expand Down Expand Up @@ -581,6 +595,10 @@ impl EncryptedLogin {

username_field: string_or_default(row, "usernameField")?,
password_field: string_or_default(row, "passwordField")?,

time_of_last_breach: row.get::<_, Option<i64>>("timeOfLastBreach")?,
time_last_breach_alert_dismissed: row
.get::<_, Option<i64>>("timeLastBreachAlertDismissed")?,
},
sec_fields: row.get("secFields")?,
};
Expand Down
25 changes: 25 additions & 0 deletions components/logins/src/logins.udl
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,10 @@ dictionary Login {
// secure login fields
string password;
string username;

// breach alert fields
i64? time_of_last_breach;
i64? time_last_breach_alert_dismissed;
};

/// Metrics tracking deletion of logins that cannot be decrypted, see `delete_undecryptable_records_for_remote_replacement`
Expand Down Expand Up @@ -228,6 +232,27 @@ interface LoginStore {
[Throws=LoginsApiError]
void touch([ByRef] string id);

/// Determines whether a login’s password is potentially breached, based on the breach date and the time of the last password change.
[Throws=LoginsApiError]
boolean is_potentially_breached([ByRef] string id);

/// Stores a known breach date for a login.
/// In Firefox Desktop this is updated once per session from Remote Settings.
[Throws=LoginsApiError]
void record_breach([ByRef] string id, i64 timestamp);

/// Removes all recorded breaches for all logins (i.e. sets time_of_last_breach to null).
[Throws=LoginsApiError]
void reset_all_breaches();

/// Determines whether a breach alert has been dismissed, based on the breach date and the alert dismissal timestamp.
[Throws=LoginsApiError]
boolean is_breach_alert_dismissed([ByRef] string id);

/// Stores the time at which the user dismissed the breach alert for a login.
[Throws=LoginsApiError]
void record_breach_alert_dismissal([ByRef] string id);

[Throws=LoginsApiError]
boolean is_empty();

Expand Down
Loading