Merge pull request #69 from square/federman/user_cancelled

Differentiate between user cancel and deleted keychain item in VALSecureEnclaveValet
square · Jan 21, 2016 · fe4e27c · fe4e27c
2 parents 00f6979 + 7c87353
commit fe4e27c
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 25 deletions.
diff --git a/Valet.podspec b/Valet.podspec
@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name     = 'Valet'
-  s.version  = '2.1.0'
+  s.version  = '2.2.0'
   s.license  = 'Apache License, Version 2.0'
   s.summary  = 'Valet lets you securely store data in the iOS or OS X Keychain without knowing a thing about how the Keychain works. It\'s easy. We promise.'
   s.homepage = 'https://github.com/square/Valet'

diff --git a/Valet/VALSecureEnclaveValet.h b/Valet/VALSecureEnclaveValet.h
@@ -65,12 +65,24 @@ typedef NS_ENUM(NSUInteger, VALAccessControl) {
 /// Convenience method for retrieving data from the keychain with a user prompt.
 /// @param userPrompt The prompt displayed to the user in Apple's Touch ID and passcode entry UI.
 /// @return The object currently stored in the keychain for the provided key. Returns nil if no string exists in the keychain for the specified key, or if the keychain is inaccessible.
-- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
+
+/// Convenience method for retrieving data from the keychain with a user prompt.
+/// @param userPrompt The prompt displayed to the user in Apple's Touch ID and passcode entry UI.
+/// @param userCancelled A pointer to a BOOL which will be set to YES if the user cancels out of Touch ID or entering the device Passcode.
+/// @return The object currently stored in the keychain for the provided key. Returns nil if no string exists in the keychain for the specified key, or if the keychain is inaccessible.
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
+
+/// Convenience method for retrieving a string from the keychain with a user prompt.
+/// @param userPrompt The prompt displayed to the user in Apple's Touch ID and passcode entry UI.
+/// @return The string currently stored in the keychain for the provided key. Returns nil if no string exists in the keychain for the specified key, or if the keychain is inaccessible.
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
 
 /// Convenience method for retrieving a string from the keychain with a user prompt.
 /// @param userPrompt The prompt displayed to the user in Apple's Touch ID and passcode entry UI.
+/// @param userCancelled A pointer to a BOOL which will be set to YES if the user cancels out of Touch ID or entering the device Passcode.
 /// @return The string currently stored in the keychain for the provided key. Returns nil if no string exists in the keychain for the specified key, or if the keychain is inaccessible.
-- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
 
 /// This method is not supported on VALSecureEnclaveValet.
 - (nonnull NSSet *)allKeys NS_UNAVAILABLE;

diff --git a/Valet/VALSecureEnclaveValet.m b/Valet/VALSecureEnclaveValet.m
@@ -298,26 +298,60 @@ - (nullable NSError *)migrateObjectsMatchingQuery:(nonnull NSDictionary *)secIte
 
 #pragma mark - Public Methods
 
-- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
 {
-    return [self objectForKey:key options:@{ (__bridge id)kSecUseOperationPrompt : userPrompt }];
+    return [self objectForKey:key userPrompt:userPrompt userCancelled:NULL];
 }
 
-- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
 {
-    return [self stringForKey:key options:@{ (__bridge id)kSecUseOperationPrompt : userPrompt }];
+    OSStatus status = errSecSuccess;
+    NSData *const objectForKey = [self objectForKey:key options:[self _optionsDictionaryForUserPrompt:userPrompt] status:&status];
+    if (userCancelled != NULL) {
+        *userCancelled = (status == errSecUserCanceled);
+    }
+
+    return objectForKey;
+}
+
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
+{
+    return [self stringForKey:key userPrompt:userPrompt userCancelled:NULL];
+}
+
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
+{
+    OSStatus status = errSecSuccess;
+    NSString *const stringForKey = [self stringForKey:key options:[self _optionsDictionaryForUserPrompt:userPrompt] status:&status];
+    if (userCancelled != NULL) {
+        *userCancelled = (status == errSecUserCanceled);
+    }
+
+    return stringForKey;
 }
 
 #pragma mark - Protected Methods
 
-- (BOOL)setObject:(nonnull NSData *)value forKey:(nonnull NSString *)key options:(NSDictionary *)options;
+- (BOOL)setObject:(nonnull NSData *)value forKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
 {
     // Remove the key before trying to set it. This will prevent us from calling SecItemUpdate on an item stored on the Secure Enclave, which would cause iOS to prompt the user for authentication.
     [self removeObjectForKey:key];
 
     return [super setObject:value forKey:key options:options];
 }
 
+#pragma mark - Private Methods
+
+- (nullable NSDictionary *)_optionsDictionaryForUserPrompt:(nullable NSString *)userPrompt;
+{
+    if (userPrompt.length == 0) {
+        return nil;
+
+    } else {
+        return @{ (__bridge id)kSecUseOperationPrompt : userPrompt };
+    }
+}
+
 @end
 
 
@@ -373,12 +407,22 @@ - (nullable instancetype)initWithSharedAccessGroupIdentifier:(nonnull NSString *
     VALCheckCondition(NO, nil, @"VALSecureEnclaveValet unsupported on this SDK");
 }
 
-- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
+{
+    VALCheckCondition(NO, nil, @"VALSecureEnclaveValet unsupported on this SDK");
+}
+
+- (nullable NSData *)objectForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
+{
+    VALCheckCondition(NO, nil, @"VALSecureEnclaveValet unsupported on this SDK");
+}
+
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt;
 {
     VALCheckCondition(NO, nil, @"VALSecureEnclaveValet unsupported on this SDK");
 }
 
-- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nonnull NSString *)userPrompt;
+- (nullable NSString *)stringForKey:(nonnull NSString *)key userPrompt:(nullable NSString *)userPrompt userCancelled:(nullable inout BOOL *)userCancelled;
 {
     VALCheckCondition(NO, nil, @"VALSecureEnclaveValet unsupported on this SDK");
 }

diff --git a/Valet/VALValet.m b/Valet/VALValet.m
@@ -338,7 +338,7 @@ - (BOOL)setObject:(nonnull NSData *)value forKey:(nonnull NSString *)key;
 
 - (nullable NSData *)objectForKey:(nonnull NSString *)key;
 {
-    return [self objectForKey:key options:nil];
+    return [self objectForKey:key options:nil status:NULL];
 }
 
 - (BOOL)setString:(nonnull NSString *)string forKey:(nonnull NSString *)key;
@@ -348,7 +348,7 @@ - (BOOL)setString:(nonnull NSString *)string forKey:(nonnull NSString *)key;
 
 - (nullable NSString *)stringForKey:(nonnull NSString *)key;
 {
-    return [self stringForKey:key options:nil];
+    return [self stringForKey:key options:nil status:NULL];
 }
 
 - (BOOL)containsObjectForKey:(nonnull NSString *)key;
@@ -528,7 +528,7 @@ - (BOOL)setObject:(nonnull NSData *)value forKey:(nonnull NSString *)key options
     return (status == errSecSuccess);
 }
 
-- (nullable NSData *)objectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options status:(nullable inout OSStatus *)status;
 {
     VALCheckCondition(key.length > 0, nil, @"Can not retrieve value with empty key.");
     NSMutableDictionary *query = [self.baseQuery mutableCopy];
@@ -540,14 +540,17 @@ - (nullable NSData *)objectForKey:(nonnull NSString *)key options:(nullable NSDi
     }
 
     CFTypeRef outTypeRef = NULL;
-
-    OSStatus const status = VALAtomicSecItemCopyMatching((__bridge CFDictionaryRef)query, &outTypeRef);
-    if (status == errSecMissingEntitlement) {
+    OSStatus const copyMatchingStatus = VALAtomicSecItemCopyMatching((__bridge CFDictionaryRef)query, &outTypeRef);
+    if (copyMatchingStatus == errSecMissingEntitlement) {
         NSLog(@"A 'Missing Entitlements' error occurred. This is likely due to an Apple Keychain bug. As a workaround try running on a device that is not attached to a debugger.\n\nMore information: https://forums.developer.apple.com/thread/4743\n");
     }
 
+    if (status != NULL) {
+        *status = copyMatchingStatus;
+    }
+
     NSData *const value = (__bridge_transfer NSData *)outTypeRef;
-    return (status == errSecSuccess) ? value : nil;
+    return (copyMatchingStatus == errSecSuccess) ? value : nil;
 }
 
 - (BOOL)setString:(nonnull NSString *)string forKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
@@ -561,9 +564,9 @@ - (BOOL)setString:(nonnull NSString *)string forKey:(nonnull NSString *)key opti
     return NO;
 }
 
-- (nullable NSString *)stringForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
+- (nullable NSString *)stringForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options  status:(nullable inout OSStatus *)status;
 {
-    NSData *const stringData = [self objectForKey:key options:options];
+    NSData *const stringData = [self objectForKey:key options:options status:status];
     if (stringData.length > 0) {
         return [[NSString alloc] initWithBytes:stringData.bytes length:stringData.length encoding:NSUTF8StringEncoding];
     }

diff --git a/Valet/VALValet_Protected.h b/Valet/VALValet_Protected.h
@@ -40,9 +40,9 @@ extern NSString * __nonnull VALStringForAccessibility(VALAccessibility accessibi
 @property (nonnull, copy, readonly) NSDictionary *baseQuery;
 
 - (BOOL)setObject:(nonnull NSData *)value forKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
-- (nullable NSData *)objectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
+- (nullable NSData *)objectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options status:(nullable inout OSStatus *)status;
 - (BOOL)setString:(nonnull NSString *)string forKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
-- (nullable NSString *)stringForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
+- (nullable NSString *)stringForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options status:(nullable inout OSStatus *)status;
 - (OSStatus)containsObjectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;
 - (nonnull NSSet *)allKeysWithOptions:(nullable NSDictionary *)options;
 - (BOOL)removeObjectForKey:(nonnull NSString *)key options:(nullable NSDictionary *)options;

diff --git a/ValetTouchIDTest/ValetSecureElementTestViewController.m b/ValetTouchIDTest/ValetSecureElementTestViewController.m
@@ -48,21 +48,26 @@ - (void)viewDidLoad;
 
 - (IBAction)setOrUpdateItem:(id)sender;
 {
-    BOOL setOrUpdatedItem = [self.secureEnclaveValet setString:[NSString stringWithFormat:@"I am here! %@", [[NSUUID new] UUIDString]] forKey:self.username];
+    BOOL const setOrUpdatedItem = [self.secureEnclaveValet setString:[NSString stringWithFormat:@"I am here! %@", [[NSUUID new] UUIDString]] forKey:self.username];
 
     self.textView.text = [self.textView.text stringByAppendingFormat:@"\n%s %@", __PRETTY_FUNCTION__, (setOrUpdatedItem ? @"Success" : @"Failure")];
 }
 
 - (IBAction)getItem:(id)sender;
 {
-    NSString *password = [self.secureEnclaveValet stringForKey:self.username userPrompt:@"Use TouchID to retreive password"];
+    BOOL userCancelled = NO;
+    NSString *const password = [self.secureEnclaveValet stringForKey:self.username userPrompt:@"Use TouchID to retrieve password" userCancelled:&userCancelled];
 
-    self.textView.text = [self.textView.text stringByAppendingFormat:@"\n%s %@", __PRETTY_FUNCTION__, password];
+    if (userCancelled) {
+        self.textView.text = [self.textView.text stringByAppendingFormat:@"\n%s user cancelled TouchID", __PRETTY_FUNCTION__];
+    } else {
+        self.textView.text = [self.textView.text stringByAppendingFormat:@"\n%s %@", __PRETTY_FUNCTION__, password];
+    }
 }
 
 - (IBAction)removeItem:(id)sender;
 {
-    BOOL removedItem = [self.secureEnclaveValet removeObjectForKey:self.username];
+    BOOL const removedItem = [self.secureEnclaveValet removeObjectForKey:self.username];
 
     self.textView.text = [self.textView.text stringByAppendingFormat:@"\n%s %@", __PRETTY_FUNCTION__, (removedItem ? @"Success" : @"Failure")];
 }