Merge pull request #59 from square/federman/shared_keychain_fix

Shared keychain fix
square · Nov 9, 2015 · 85d91c7 · 85d91c7
2 parents d550e29 + d51300f
commit 85d91c7
Show file tree

Hide file tree

Showing 6 changed files with 42 additions and 13 deletions.
diff --git a/Valet.podspec b/Valet.podspec
@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name     = 'Valet'
-  s.version  = '2.0.5'
+  s.version  = '2.0.6'
   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
@@ -18,7 +18,7 @@
 //  limitations under the License.
 //
 
-#import "VALValet.h"
+#import <Valet/VALValet.h>
 
 
 /// Reads and writes keychain elements that are stored on the Secure Enclave (supported on iOS 8.0 or later) using accessibility attribute VALAccessibilityWhenPasscodeSetThisDeviceOnly. Accessing or modifying these items will require the user to confirm their presence via Touch ID or passcode entry. If no passcode is set on the device, the below methods will fail. Data is removed from the Secure Enclave when the user removes a passcode from the device. Use the userPrompt methods to display custom text to the user in Apple's Touch ID and passcode entry UI.

diff --git a/Valet/VALSynchronizableValet.h b/Valet/VALSynchronizableValet.h
@@ -18,7 +18,7 @@
 //  limitations under the License.
 //
 
-#import "VALValet.h"
+#import <Valet/VALValet.h>
 
 
 /// Reads and writes keychain elements that are synchronized with iCloud (supported on devices on iOS 7.0.3 and later). Accessibility must not be scoped to this device.

diff --git a/Valet/VALValet.m b/Valet/VALValet.m
@@ -18,7 +18,6 @@
 //  limitations under the License.
 //
 
-#import "VALValet.h"
 #import "VALValet_Protected.h"
 
 #import "ValetDefines.h"
@@ -316,6 +315,7 @@ - (nullable NSError *)migrateObjectsMatchingQuery:(nonnull NSDictionary *)secIte
     VALCheckCondition(secItemQuery[(__bridge id)kSecReturnAttributes] != (__bridge id)kCFBooleanFalse, [NSError errorWithDomain:VALMigrationErrorDomain code:VALMigrationErrorInvalidQuery userInfo:nil], @"Migration requires kSecReturnAttributes to be set to kCFBooleanTrue.");
     VALCheckCondition(secItemQuery[(__bridge id)kSecReturnRef] != (__bridge id)kCFBooleanTrue, [NSError errorWithDomain:VALMigrationErrorDomain code:VALMigrationErrorInvalidQuery userInfo:nil], @"kSecReturnRef is not supported in a migration query.");
     VALCheckCondition(secItemQuery[(__bridge id)kSecReturnPersistentRef] != (__bridge id)kCFBooleanFalse, [NSError errorWithDomain:VALMigrationErrorDomain code:VALMigrationErrorInvalidQuery userInfo:nil], @"Migration requires SecReturnPersistentRef to be set to kCFBooleanTrue.");
+    VALCheckCondition([secItemQuery[(__bridge id)kSecClass] isKindOfClass:[NSString class]], [NSError errorWithDomain:VALMigrationErrorDomain code:VALMigrationErrorInvalidQuery userInfo:nil], @"Migration requires a kSecClass to be set to a valid kSecClass string.");
 
     NSMutableDictionary *query = [secItemQuery mutableCopy];
     query[(__bridge id)kSecMatchLimit] = (__bridge id)kSecMatchLimitAll;
@@ -608,8 +608,9 @@ - (nonnull NSString *)secServiceIdentifier;
 - (nullable NSString *)_sharedAccessGroupPrefix;
 {
     NSDictionary *query = @{ (__bridge NSString *)kSecClass : (__bridge NSString *)kSecClassGenericPassword,
-                             (__bridge id)kSecAttrAccount : @"SharedAccessGroupPrefixPlaceholder",
-                             (__bridge id)kSecReturnAttributes : @YES };
+                             (__bridge id)kSecAttrAccount : @"SharedAccessGroupAlwaysAccessiblePrefixPlaceholder",
+                             (__bridge id)kSecReturnAttributes : @YES,
+                             (__bridge id)kSecAttrAccessible : (__bridge id)kSecAttrAccessibleAlwaysThisDeviceOnly };
 
     CFTypeRef outTypeRef = NULL;
     NSDictionary *queryResult = nil;

diff --git a/Valet/VALValet_Protected.h b/Valet/VALValet_Protected.h
@@ -18,7 +18,7 @@
 //  limitations under the License.
 //
 
-#import "VALValet.h"
+#import <Valet/VALValet.h>
 
 
 NS_ASSUME_NONNULL_BEGIN

diff --git a/ValetTests/ValetTests.m b/ValetTests/ValetTests.m
@@ -557,20 +557,48 @@ - (void)test_migrateObjectsMatchingQueryRemoveOnCompletion_failsIfNoItemsFoundMa
     XCTAssertEqual([self.valet migrateObjectsMatchingQuery:queryWithNoMatches removeOnCompletion:YES].code, VALMigrationErrorNoItemsToMigrateFound);
 }
 
+- (void)test_migrateObjectsMatchingQueryRemoveOnCompletion_failsIfQueryInputHasNoClass;
+{
+    [self.valet setString:self.string forKey:self.key];
+
+    // Test that it succeeds before we test to see if it fails.
+    XCTAssertNil([self.testingValet migrateObjectsMatchingQuery:self.valet.baseQuery removeOnCompletion:NO]);
+
+    NSMutableDictionary *baseQueryNoClass = [self.valet.baseQuery mutableCopy];
+    [baseQueryNoClass removeObjectForKey:(__bridge id)kSecClass];
+    XCTAssertEqual([self.testingValet migrateObjectsMatchingQuery:baseQueryNoClass removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+}
+
 - (void)test_migrateObjectsMatchingQueryRemoveOnCompletion_failsOnBadQueryInput;
 {
     XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{} removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
     XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{} removeOnCompletion:YES].code, VALMigrationErrorInvalidQuery);
 
-    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{ (__bridge id)kSecMatchLimit : (__bridge id)kSecMatchLimitOne } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
-    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{ (__bridge id)kSecReturnData : (__bridge id)kCFBooleanTrue } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
-    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{ (__bridge id)kSecReturnAttributes : (__bridge id)kCFBooleanFalse } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
-    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{ (__bridge id)kSecReturnRef : (__bridge id)kCFBooleanTrue } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
-    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:@{ (__bridge id)kSecReturnPersistentRef : (__bridge id)kCFBooleanFalse } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+    NSDictionary *invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                                    (__bridge id)kSecMatchLimit : (__bridge id)kSecMatchLimitOne };
+    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+
+    invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                      (__bridge id)kSecReturnData : (__bridge id)kCFBooleanTrue };
+    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+
+    invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                      (__bridge id)kSecReturnAttributes : (__bridge id)kCFBooleanFalse };
+    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+
+    invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                      (__bridge id)kSecReturnRef : (__bridge id)kCFBooleanTrue };
+    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+
+    invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                      (__bridge id)kSecReturnPersistentRef : (__bridge id)kCFBooleanFalse };
+    XCTAssertEqual([self.valet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
 
 #if VAL_IOS_8_OR_LATER
     if ([VALSecureEnclaveValet supportsSecureEnclaveKeychainItems]) {
-        XCTAssertEqual([self.secureEnclaveValet migrateObjectsMatchingQuery:@{ (__bridge id)kSecUseOperationPrompt : @"Migration Prompt" } removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
+        invalidQuery = @{ (__bridge id)kSecClass : (__bridge id)kSecClassGenericPassword,
+                          (__bridge id)kSecUseOperationPrompt : @"Migration Prompt" };
+        XCTAssertEqual([self.secureEnclaveValet migrateObjectsMatchingQuery:invalidQuery removeOnCompletion:NO].code, VALMigrationErrorInvalidQuery);
     }
 #endif
 }