-
-
Notifications
You must be signed in to change notification settings - Fork 107
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
Split authenticationValidityDurationSeconds between android and iOS #77
Changes from 3 commits
8624c6b
6e954ba
a2ec968
31f043a
9120b67
a498619
444e141
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,13 +80,45 @@ class AuthException implements Exception { | |
|
||
class StorageFileInitOptions { | ||
StorageFileInitOptions({ | ||
Duration? androidAuthenticationValidityDuration, | ||
Duration? iosTouchIDAuthenticationAllowableReuseDuration, | ||
this.iosTouchIDAuthenticationForceReuseContextDuration, | ||
this.authenticationValidityDurationSeconds = -1, | ||
this.authenticationRequired = true, | ||
this.androidBiometricOnly = true, | ||
}); | ||
|
||
}) : androidAuthenticationValidityDuration = | ||
androidAuthenticationValidityDuration ?? | ||
(authenticationValidityDurationSeconds <= 0 | ||
? null | ||
: Duration(seconds: authenticationValidityDurationSeconds)), | ||
iosTouchIDAuthenticationAllowableReuseDuration = | ||
iosTouchIDAuthenticationAllowableReuseDuration ?? | ||
(authenticationValidityDurationSeconds <= 0 | ||
? null | ||
: Duration(seconds: authenticationValidityDurationSeconds)); | ||
|
||
@Deprecated('use androidAuthenticationValidityDuration instead') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At various times this has affected iOS behaviour too so maybe: |
||
final int authenticationValidityDurationSeconds; | ||
|
||
/// see https://developer.android.com/reference/android/security/keystore/KeyGenParameterSpec.Builder#setUserAuthenticationParameters(int,%20int) | ||
final Duration? androidAuthenticationValidityDuration; | ||
|
||
/// see https://developer.apple.com/documentation/localauthentication/lacontext/1622329-touchidauthenticationallowablere | ||
/// > If the user unlocks the device using Touch ID within the specified time interval, then authentication for the receiver succeeds automatically, without prompting the user for Touch ID. This bypasses a scenario where the user unlocks the device and then is almost immediately prompted for another fingerprint. | ||
/// and https://developer.apple.com/documentation/localauthentication/accessing_keychain_items_with_face_id_or_touch_id | ||
/// > Note that this grace period applies specifically to device unlock with Touch ID, not keychain retrieval authentications | ||
/// | ||
/// If you want to avoid requiring authentication after a successful | ||
/// keychain retrieval see [iosTouchIDAuthenticationForceReuseContextDuration] | ||
final Duration? iosTouchIDAuthenticationAllowableReuseDuration; | ||
|
||
/// To prevent forcing the user to authenticate again after unlocking once | ||
/// we can reuse the `LAContext` object for the given amount of time. | ||
/// see https://github.com/authpass/biometric_storage/pull/73 | ||
/// This is pretty much undocumented behavior, but works similar to | ||
/// `androidAuthenticationValidityDuration`. | ||
final Duration? iosTouchIDAuthenticationForceReuseContextDuration; | ||
|
||
/// Whether an authentication is required. if this is | ||
/// false NO BIOMETRIC CHECK WILL BE PERFORMED! and the value | ||
/// will simply be save encrypted. (default: true) | ||
|
@@ -97,14 +129,20 @@ class StorageFileInitOptions { | |
/// On Android < 30 this will always be ignored. (always `true`) | ||
/// https://github.com/authpass/biometric_storage/issues/12#issuecomment-900358154 | ||
/// | ||
/// Also: this **must** be `true` if [authenticationValidityDurationSeconds] | ||
/// is `-1`. | ||
/// Also: this **must** be `true` if [androidAuthenticationValidityDuration] | ||
/// is null. | ||
/// https://github.com/authpass/biometric_storage/issues/12#issuecomment-902508609 | ||
final bool androidBiometricOnly; | ||
|
||
Map<String, dynamic> toJson() => <String, dynamic>{ | ||
'authenticationValidityDurationSeconds': | ||
authenticationValidityDurationSeconds, | ||
'androidAuthenticationValidityDurationSeconds': | ||
androidAuthenticationValidityDuration?.inSeconds, | ||
'iosTouchIDAuthenticationAllowableReuseDurationSeconds': | ||
iosTouchIDAuthenticationAllowableReuseDuration?.inSeconds, | ||
'iosTouchIDAuthenticationForceReuseContextDurationSeconds': | ||
iosTouchIDAuthenticationForceReuseContextDuration?.inSeconds, | ||
'authenticationRequired': authenticationRequired, | ||
'androidBiometricOnly': androidBiometricOnly, | ||
}; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,10 +14,12 @@ struct StorageMethodCall { | |
|
||
class InitOptions { | ||
init(params: [String: Any]) { | ||
authenticationValidityDurationSeconds = params["authenticationValidityDurationSeconds"] as? Int | ||
iosTouchIDAuthenticationAllowableReuseDuration = params["iosTouchIDAuthenticationAllowableReuseDurationSeconds"] as? Int | ||
iosTouchIDAuthenticationForceReuseContextDuration = params["iosTouchIDAuthenticationForceReuseContextDurationSeconds"] as? Int | ||
authenticationRequired = params["authenticationRequired"] as? Bool | ||
} | ||
let authenticationValidityDurationSeconds: Int! | ||
let iosTouchIDAuthenticationAllowableReuseDuration: Int? | ||
let iosTouchIDAuthenticationForceReuseContextDuration: Int? | ||
let authenticationRequired: Bool! | ||
} | ||
|
||
|
@@ -148,23 +150,41 @@ class BiometricStorageImpl { | |
} | ||
} | ||
|
||
typealias StoredContext = (context: LAContext, expireAt: Date) | ||
|
||
class BiometricStorageFile { | ||
private let name: String | ||
private let initOptions: InitOptions | ||
private var context: LAContext { get { | ||
let context = LAContext() | ||
if (initOptions.authenticationRequired) { | ||
if initOptions.authenticationValidityDurationSeconds > 0 { | ||
if #available(OSX 10.12, *) { | ||
context.touchIDAuthenticationAllowableReuseDuration = Double(initOptions.authenticationValidityDurationSeconds) | ||
private var _context: StoredContext? | ||
private var context: LAContext { | ||
get { | ||
if let context = _context { | ||
if context.expireAt.timeIntervalSinceNow < 0 { | ||
// already expired. | ||
_context = nil | ||
} else { | ||
// Fallback on earlier versions | ||
hpdebug("Pre OSX 10.12 no touchIDAuthenticationAllowableReuseDuration available. ignoring.") | ||
return context.context | ||
} | ||
} | ||
|
||
let context = LAContext() | ||
if (initOptions.authenticationRequired) { | ||
if let duration = initOptions.iosTouchIDAuthenticationAllowableReuseDuration { | ||
if #available(OSX 10.12, *) { | ||
context.touchIDAuthenticationAllowableReuseDuration = Double(duration) | ||
} else { | ||
// Fallback on earlier versions | ||
hpdebug("Pre OSX 10.12 no touchIDAuthenticationAllowableReuseDuration available. ignoring.") | ||
} | ||
} | ||
|
||
if let duration = initOptions.iosTouchIDAuthenticationForceReuseContextDuration { | ||
_context = (context: context, expireAt: Date(timeIntervalSinceNow: Double(duration))) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?) |
||
} | ||
} | ||
return context | ||
} | ||
return context | ||
} } | ||
} | ||
private let storageError: StorageError | ||
|
||
init(name: String, initOptions: InitOptions, storageError: @escaping StorageError) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
)?