Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorporate BreachAlertsManager in to LoginsListViewController #6934

Merged
merged 20 commits into from
Jul 16, 2020
Merged
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
59 changes: 41 additions & 18 deletions Client/Frontend/Login Management/BreachAlertsManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import Storage // or whichever module has the LoginsRecord class
import Shared // or whichever module has the Maybe class

/// Breach structure decoded from JSON
struct BreachRecord: Codable, Equatable {
struct BreachRecord: Codable, Equatable, Hashable {
var name: String
var title: String
var domain: String
Expand Down Expand Up @@ -49,13 +49,22 @@ final public class BreachAlertsManager {
}

self.breaches = decoded
self.breaches.append(BreachRecord(
name: "MockBreach",
title: "A Mock BreachRecord",
domain: "abreached.com",
breachDate: "1970-01-02",
description: "A mock BreachRecord for testing purposes."
))
completion(Maybe(success: self.breaches))
}
}

/// Compares a list of logins to a list of breaches and returns breached logins.
/// - Parameters:
/// - logins: a list of logins to compare breaches to
/// - Returns:
/// - an array of LoginRecords of breaches in the original list.
func findUserBreaches(_ logins: [LoginRecord]) -> Maybe<[LoginRecord]> {
var result: [LoginRecord] = []

Expand All @@ -65,27 +74,41 @@ final public class BreachAlertsManager {
return Maybe(failure: BreachAlertsError(description: "cannot compare to an empty list of logins"))
}

// TODO: optimize this loop
for login in logins {
for breach in self.breaches {
// host check
let loginHostURL = URL(string: login.hostname)
if loginHostURL?.baseDomain == breach.domain {
print("compareToBreaches(): breach: \(breach.domain)")

// date check
let pwLastChanged = Date(timeIntervalSince1970: TimeInterval(login.timePasswordChanged))

let dateFormatter = DateFormatter()
dateFormatter.dateFormat = "yyyy-MM-dd"
if let breachDate = dateFormatter.date(from: breach.breachDate), pwLastChanged < breachDate {
print("compareToBreaches(): ⚠️ password exposed ⚠️: \(breach.breachDate)")
result.append(login)
}
let loginsDictionary = loginsByHostname(logins)
for breach in self.breaches {
guard let potentialBreaches = loginsDictionary[breach.domain] else {
continue
}
for item in potentialBreaches {
let pwLastChanged = TimeInterval(item.timePasswordChanged/1000)
let dateFormatter = DateFormatter()
dateFormatter.dateFormat = "yyyy-MM-dd"
guard let breachDate = dateFormatter.date(from: breach.breachDate)?.timeIntervalSince1970, pwLastChanged < breachDate else {
vphong marked this conversation as resolved.
Show resolved Hide resolved
continue
}
print("compareToBreaches(): ⚠️ password exposed ⚠️: \(breach.breachDate)")
result.append(item)
}
}
print("compareToBreaches(): fin")
return Maybe(success: result)
}

func loginsByHostname(_ logins: [LoginRecord]) -> [String: [LoginRecord]] {
var result = [String: [LoginRecord]]()
for login in logins {
let base = baseDomainForLogin(login)
if !result.keys.contains(base) {
result[base] = [login]
} else {
result[base]?.append(login)
}
vphong marked this conversation as resolved.
Show resolved Hide resolved
}
return result
}

private func baseDomainForLogin(_ login: LoginRecord) -> String {
guard let result = login.hostname.asURL?.baseDomain else { return login.hostname }
return result
}
}
13 changes: 10 additions & 3 deletions Client/Frontend/Login Management/LoginDataSource.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,10 @@ class LoginDataSource: NSObject, UITableViewDataSource {
@objc func tableView(_ tableView: UITableView, cellForRowAt indexPath: IndexPath) -> UITableViewCell {
let cell = ThemedTableViewCell(style: .subtitle, reuseIdentifier: CellReuseIdentifier)

// Need to override the default background multi-select color to support theming
cell.multipleSelectionBackgroundView = UIView()
cell.applyTheme()

if indexPath.section == LoginsSettingsSection {
let hideSettings = viewModel.searchController?.isActive ?? false || tableView.isEditing
let setting = indexPath.row == 0 ? boolSettings.0 : boolSettings.1
Expand All @@ -66,10 +70,13 @@ class LoginDataSource: NSObject, UITableViewDataSource {
cell.detailTextColor = UIColor.theme.tableView.rowDetailText
cell.detailTextLabel?.text = login.username
cell.accessoryType = .disclosureIndicator
if let breaches = viewModel.userBreaches {
if breaches.contains(login) {
cell.textLabel?.textColor = UIColor.systemRed
viewModel.setBreachIndexPath(indexPath: indexPath)
}
}
}
// Need to override the default background multi-select color to support theming
cell.multipleSelectionBackgroundView = UIView()
cell.applyTheme()
return cell
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -433,6 +433,12 @@ extension LoginListViewController: SearchInputViewDelegate {
// MARK: - LoginViewModelDelegate
extension LoginListViewController: LoginViewModelDelegate {

func breachPathDidUpdate() {
DispatchQueue.main.async {
self.tableView.reloadRows(at: self.viewModel.breachIndexPath, with: .right)
}
}

func loginSectionsDidUpdate() {
loadingView.isHidden = true
tableView.reloadData()
Expand Down
28 changes: 23 additions & 5 deletions Client/Frontend/Login Management/LoginListViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,13 @@ final class LoginListViewModel {
}
}
fileprivate let helper = LoginListDataSourceHelper()
private(set) var breachAlertsManager = BreachAlertsManager()
private(set) var userBreaches: [LoginRecord]?
private(set) var breachIndexPath = [IndexPath]() {
didSet {
delegate?.breachPathDidUpdate()
}
vphong marked this conversation as resolved.
Show resolved Hide resolved
}

init(profile: Profile, searchController: UISearchController) {
self.profile = profile
Expand All @@ -32,17 +39,23 @@ final class LoginListViewModel {

func loadLogins(_ query: String? = nil, loginDataSource: LoginDataSource) {
// Fill in an in-flight query and re-query
self.activeLoginQuery?.fillIfUnfilled(Maybe(success: []))
self.activeLoginQuery = self.queryLogins(query ?? "")
self.activeLoginQuery! >>== self.setLogins
activeLoginQuery?.fillIfUnfilled(Maybe(success: []))
activeLoginQuery = queryLogins(query ?? "")
breachAlertsManager.loadBreaches { [weak self] _ in
guard let self = self, let logins = self.activeLoginQuery?.value.successValue else { return }
self.userBreaches = self.breachAlertsManager.findUserBreaches(logins).successValue
}
activeLoginQuery! >>== self.setLogins
}

/// Searches SQLite database for logins that match query.
/// Wraps the SQLiteLogins method to allow us to cancel it from our end.
func queryLogins(_ query: String) -> Deferred<Maybe<[LoginRecord]>> {
let deferred = Deferred<Maybe<[LoginRecord]>>()
profile.logins.searchLoginsWithQuery(query) >>== { logins in
deferred.fillIfUnfilled(Maybe(success: logins.asArray()))
var log = logins.asArray()
log.append(LoginRecord(fromJSONDict: ["hostname" : "http://abreached.com", "timePasswordChanged": 46800000]))
deferred.fillIfUnfilled(Maybe(success: log))
succeed()
}
return deferred
Expand Down Expand Up @@ -101,6 +114,10 @@ final class LoginListViewModel {
}
}

func setBreachIndexPath(indexPath: IndexPath) {
self.breachIndexPath = [indexPath]
}

// MARK: - UX Constants
struct LoginListUX {
static let RowHeight: CGFloat = 58
Expand All @@ -114,10 +131,11 @@ final class LoginListViewModel {
// MARK: - LoginDataSourceViewModelDelegate
protocol LoginViewModelDelegate: AnyObject {
func loginSectionsDidUpdate()
func breachPathDidUpdate()
}

extension LoginRecord: Equatable {
public static func == (lhs: LoginRecord, rhs: LoginRecord) -> Bool {
return lhs.id == rhs.id
return lhs.id == rhs.id && lhs.hostname == rhs.hostname && lhs.credentials == rhs.credentials
}
}
29 changes: 23 additions & 6 deletions ClientTests/BreachAlertsTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ let mockRecord = BreachRecord(
breachDate: "1970-01-02",
description: "A mock BreachRecord for testing purposes."
)
// remove for official release
let amockRecord = BreachRecord(
name: "MockBreach",
title: "A Mock BreachRecord",
domain: "abreached.com",
breachDate: "1970-01-02",
description: "A mock BreachRecord for testing purposes."
)

class MockBreachAlertsClient: BreachAlertsClientProtocol {
func fetchData(endpoint: BreachAlertsClient.Endpoint, completion: @escaping (Maybe<Data>) -> Void) {
Expand All @@ -26,12 +34,12 @@ class MockBreachAlertsClient: BreachAlertsClientProtocol {
}

class BreachAlertsTests: XCTestCase {
var breachAlertsManager: BreachAlertsManager?
var breachAlertsManager: BreachAlertsManager!
let unbreachedLogin = [
LoginRecord(fromJSONDict: ["hostname" : "http://unbreached.com", "timePasswordChanged": 1590784648189])
LoginRecord(fromJSONDict: ["hostname" : "http://unbreached.com", "timePasswordChanged": 1594411049000])
]
let breachedLogin = [
LoginRecord(fromJSONDict: ["hostname" : "http://breached.com", "timePasswordChanged": 1])
LoginRecord(fromJSONDict: ["hostname" : "http://breached.com", "timePasswordChanged": 46800000])
]

override func setUp() {
Expand All @@ -43,7 +51,7 @@ class BreachAlertsTests: XCTestCase {
XCTAssertTrue(maybeBreaches.isSuccess)
XCTAssertNotNil(maybeBreaches.successValue)
if let breaches = maybeBreaches.successValue {
XCTAssertEqual([mockRecord], breaches)
XCTAssertEqual([mockRecord, amockRecord], breaches)
}
}
}
Expand All @@ -66,15 +74,24 @@ class BreachAlertsTests: XCTestCase {
XCTAssertNotNil(noBreachesOpt)
if let noBreaches = noBreachesOpt {
XCTAssertTrue(noBreaches.isSuccess)
XCTAssertEqual(noBreaches.successValue?.count, 0)
XCTAssertEqual(noBreaches.successValue, Optional([]))
}

let breachedOpt = self.breachAlertsManager?.findUserBreaches(self.breachedLogin)
XCTAssertNotNil(breachedOpt)
if let breached = breachedOpt {
XCTAssertTrue(breached.isSuccess)
XCTAssertEqual(breached.successValue?.count, 1)
XCTAssertEqual(breached.successValue, self.breachedLogin)
}
}
}

func testLoginsByHostname() {
let unbreached = ["unbreached.com": self.unbreachedLogin]
var result = breachAlertsManager.loginsByHostname(self.unbreachedLogin)
XCTAssertEqual(result, unbreached)
let breached = ["breached.com": self.breachedLogin]
result = breachAlertsManager.loginsByHostname(self.breachedLogin)
XCTAssertEqual(result, breached)
}
}
8 changes: 4 additions & 4 deletions ClientTests/LoginsListViewModelTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -45,19 +45,19 @@ class LoginsListViewModelTests: XCTestCase {

let emptyQueryResult = self.viewModel.queryLogins("")
XCTAssertTrue(emptyQueryResult.value.isSuccess)
XCTAssertEqual(emptyQueryResult.value.successValue?.count, 10)
XCTAssertEqual(emptyQueryResult.value.successValue?.count, 11)

let exampleQueryResult = self.viewModel.queryLogins("example")
XCTAssertTrue(exampleQueryResult.value.isSuccess)
XCTAssertEqual(exampleQueryResult.value.successValue?.count, 10)
XCTAssertEqual(exampleQueryResult.value.successValue?.count, 11)

let threeQueryResult = self.viewModel.queryLogins("3")
XCTAssertTrue(threeQueryResult.value.isSuccess)
XCTAssertEqual(threeQueryResult.value.successValue?.count, 1)
XCTAssertEqual(threeQueryResult.value.successValue?.count, 2)

let zQueryResult = self.viewModel.queryLogins("yxz")
XCTAssertTrue(zQueryResult.value.isSuccess)
XCTAssertEqual(zQueryResult.value.successValue?.count, 0)
XCTAssertEqual(zQueryResult.value.successValue?.count, 1)
}

func testIsDuringSearchControllerDismiss() {
Expand Down