Fix: TouchID prompt on second install #273
Conversation
| @@ -8,6 +8,7 @@ import UIKit | |||
|
|
|||
| private struct Constants { | |||
| static let BiometricsSecretsLabel = "com.schibsted.account.biometrics.secrets" | |||
| static let HasLoggedInBeforeSettingsKey = "hasLoggedInBefore" | |||
There was a problem hiding this comment.
Maybe follow the pattern: "com.schibsted.account.hasLoggedInBefore"
| @@ -8,6 +8,7 @@ import UIKit | |||
|
|
|||
| private struct Constants { | |||
There was a problem hiding this comment.
add: private init() { } to avoid possibility to create an instance of Constants
| @@ -241,6 +242,11 @@ extension PasswordCoordinator { | |||
| // Fallback to passsword login | |||
| return nil | |||
| } | |||
|
|
|||
| let hasLoggedInBefore = Settings.value(forKey: Constants.HasLoggedInBeforeSettingsKey) as? Bool | |||
There was a problem hiding this comment.
guard let hasLoggedInBefore = Settings.value(forKey: Constants.HasLoggedInBeforeSettingsKey) as? Bool else { return nil }
There was a problem hiding this comment.
You can also join with the previous guard
guard #available(iOS 11.3, *) else {
There was a problem hiding this comment.
Wouldn't this prevent users from reusing password after app upgrade?
| @@ -253,6 +258,7 @@ extension PasswordCoordinator { | |||
| if status == noErr, let qresult = queryResult as? Data, let password = String(data: qresult as Data, encoding: .utf8) { | |||
| return password | |||
| } else { | |||
| Settings.setValue(false, forKey: Constants.HasLoggedInBeforeSettingsKey) | |||
There was a problem hiding this comment.
Is it necessary to set it to false here? A missing value for the key Constants.HasLoggedInBeforeSettingsKey is treated the same as it having the value false in all cases, right?
There was a problem hiding this comment.
True, but I prefer to be explicit here.
|
Since Apple keeps the data stored in the keychain even if the app is uninstalled, could we just keep using it without prompting for consent again on re-install? |
If merged this will fix this issue #272