Split authenticationValidityDurationSeconds between android and iOS #77
Conversation
…thenticationValidityDuration
…IDAuthenticationForceReuseContextDuration
|
|
There was a problem hiding this comment.
I think the option names are better now - it's probably still not what I would choose but that might be mainly down to a different opinion on how much abstraction this library should offer over the various native implementations so it's your opinion that matters.
I think the context expiry time needs adjustment but apart from that and those other minor suggestions this looks good to me.
I have also not had a chance to test it out yet and might not be able to find enough time to do that thoroughly for a little while but I'll try to at least build my project against the branch and test in some emulators in the next week or two.
| @@ -74,8 +77,8 @@ class BiometricStorageFile( | |||
| } | |||
|
|
|||
| private fun validateOptions() { | |||
| if (options.authenticationValidityDurationSeconds == -1 && !options.androidBiometricOnly) { | |||
| throw IllegalArgumentException("when authenticationValidityDurationSeconds is -1, androidBiometricOnly must be true") | |||
| if (options.androidAuthenticationValidityDuration == null && !options.androidBiometricOnly) { | |||
There was a problem hiding this comment.
Should we also validate that the user doesn't set androidAuthenticationValidityDuration to a negative number (especially since the similar old API gave special meaning to -1)?
lib/src/biometric_storage.dart
Outdated
| ? null | ||
| : Duration(seconds: authenticationValidityDurationSeconds)); | ||
|
|
||
| @Deprecated('use androidAuthenticationValidityDuration instead') |
There was a problem hiding this comment.
At various times this has affected iOS behaviour too so maybe:
'use androidAuthenticationValidityDuration, iosTouchIDAuthenticationAllowableReuseDuration or iosTouchIDAuthenticationForceReuseContextDuration instead'
| } | ||
|
|
||
| if let duration = initOptions.iosTouchIDAuthenticationForceReuseContextDuration { | ||
| _context = (context: context, expireAt: Date(timeIntervalSinceNow: Double(duration))) |
There was a problem hiding this comment.
The problem with assigning the expiry time when creating the context is that this will not reflect the actual time since successful keychain authentication occurred. That will be particularly noticeable any time a reasonably low (<30 seconds?) iosTouchIDAuthenticationForceReuseContextDuration is used and the user has to attempt authentication a number of times before the biometrics are confirmed.
authenticationValidityDurationSecondsandroidAuthenticationValidityDurationiosTouchIDAuthenticationAllowableReuseDurationiosTouchIDAuthenticationForceReuseContextDurationreplaces