Skip to content
This repository has been archived by the owner on Oct 6, 2022. It is now read-only.

Fix: TouchID prompt on second install #273

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix: TouchID prompt on second install #273

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d081efc
Fix: TouchID prompt on second install while it supposed to ask for pe…
Aug 1, 2019
fb7b038
Fix reviw comments
Aug 1, 2019
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
12 changes: 9 additions & 3 deletions Source/UI/Coordinators/PasswordCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import UIKit

private struct Constants {
Copy link
Collaborator

Choose a reason for hiding this comment

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

add: private init() { } to avoid possibility to create an instance of Constants

static let BiometricsSecretsLabel = "com.schibsted.account.biometrics.secrets"
static let HasLoggedInBeforeSettingsKey = "hasLoggedInBefore"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe follow the pattern: "com.schibsted.account.hasLoggedInBefore"

}

class PasswordCoordinator: AuthenticationCoordinator, RouteHandler {
Expand Down Expand Up @@ -241,6 +242,11 @@ extension PasswordCoordinator {
// Fallback to passsword login
return nil
}

let hasLoggedInBefore = Settings.value(forKey: Constants.HasLoggedInBeforeSettingsKey) as? Bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

guard let hasLoggedInBefore = Settings.value(forKey: Constants.HasLoggedInBeforeSettingsKey) as? Bool else { return nil }

Copy link
Collaborator

Choose a reason for hiding this comment

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

You can also join with the previous guard
guard #available(iOS 11.3, *) else {

Copy link
Collaborator

Choose a reason for hiding this comment

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

Wouldn't this prevent users from reusing password after app upgrade?

if hasLoggedInBefore == nil {
return nil
}
var query = [String: Any]()
query[kSecClass as String] = kSecClassGenericPassword
query[kSecReturnData as String] = kCFBooleanTrue
Expand Down Expand Up @@ -308,14 +314,14 @@ extension PasswordCoordinator {
dictionary[kSecValueData as String] = password.data(using: .utf8)! as CFData
dictionary[kSecAttrAccessControl as String] = accessControl

let hasLoggedInBeforeSettingsKey = "hasLoggedInBefore"
if let hasLoggedInBefore = Settings.value(forKey: hasLoggedInBeforeSettingsKey) as? Bool, hasLoggedInBefore {

if let hasLoggedInBefore = Settings.value(forKey: Constants.HasLoggedInBeforeSettingsKey) as? Bool, hasLoggedInBefore {
if self.configuration.useBiometrics {
SecItemAdd(dictionary as CFDictionary, nil)
}
completion()
} else {
Settings.setValue(true, forKey: hasLoggedInBeforeSettingsKey)
Settings.setValue(true, forKey: Constants.HasLoggedInBeforeSettingsKey)
if self.biometryType == .touchID {
let viewModel = PasswordViewModel(
identifier: identifier,
Expand Down