Merge pull request #1175 from AzureAD/jason/fixReturnValueOfRemoveItem

Fix return value of removeItem
AzureAD · Mar 17, 2018 · 3245d3d · 3245d3d
2 parents 6c22d69 + e499bf5
commit 3245d3d
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 13 deletions.
diff --git a/ADAL/src/cache/ios/ADKeychainTokenCache+Internal.h b/ADAL/src/cache/ios/ADKeychainTokenCache+Internal.h
@@ -28,6 +28,7 @@
 
 @interface ADKeychainTokenCache (Internal) <ADTokenCacheDataSource>
 
+// Return YES if there is an error in status
 + (BOOL)checkStatus:(OSStatus)status
           operation:(NSString *)operation
       correlationId:(NSUUID *)correlationId

diff --git a/ADAL/src/cache/ios/ADKeychainTokenCache.m b/ADAL/src/cache/ios/ADKeychainTokenCache.m
@@ -395,7 +395,7 @@ - (BOOL)removeItem:(nonnull ADTokenCacheItem *)item
     //if item does not exist in cache or does not contain a refresh token, deletion is enough and should return.
     if (deleteStatus != errSecSuccess || [NSString adIsStringNilOrBlank:item.refreshToken])
     {
-        return [ADKeychainTokenCache checkStatus:deleteStatus operation:@"delete" correlationId:nil error:error];
+        return ![ADKeychainTokenCache checkStatus:deleteStatus operation:@"delete" correlationId:nil error:error];
     }
 
     return [self saveWipeTokenData:error];

diff --git a/ADAL/tests/unit/ios/ADKeychainTokenCacheTests.m b/ADAL/tests/unit/ios/ADKeychainTokenCacheTests.m
@@ -193,12 +193,12 @@ - (void)testItemDelete
 
     //remove item1.
     //Since item1 does not contain refresh token, it should be deleted from cache.
-    [mStore removeItem:item1 error:&error];
+    XCTAssertTrue([mStore removeItem:item1 error:&error]);
     ADAssertNoError;
     XCTAssertEqual([self count], 1);
 
     //remove item2.
-    [mStore removeItem:item2 error:&error];
+    XCTAssertTrue([mStore removeItem:item2 error:&error]);
     ADAssertNoError;
     XCTAssertEqual([self count], 0);
 }
@@ -211,10 +211,10 @@ - (void)testRemoveAllForClientId
     XCTAssertNotNil([mStore allItems:&error]);
     ADAssertNoError;
 
-    //add three items with the same client ID and one with a different client ID
-    ADTokenCacheItem* item1 = [self adCreateCacheItem:@"eric@contoso.com"];
+    //add some items (ATs and RTs) with the same client ID and one with a different client ID
+    ADTokenCacheItem* item1 = [self adCreateATCacheItem:TEST_RESOURCE userId:@"eric@contoso.com"];
     [mStore addOrUpdateItem:item1 correlationId:nil error:&error];
-    ADTokenCacheItem* item2 = [self adCreateCacheItem:@"stan@contoso.com"];
+    ADTokenCacheItem* item2 = [self adCreateMRRTCacheItem:@"stan@contoso.com"];
     [mStore addOrUpdateItem:item2 correlationId:nil error:&error];
     ADTokenCacheItem* item3 = [self adCreateCacheItem:@"jack@contoso.com"];
     [mStore addOrUpdateItem:item3 correlationId:nil error:&error];
@@ -240,29 +240,32 @@ - (void)testRemoveAllForUserIdAndClientId
     XCTAssertNotNil([mStore allItems:&error]);
     ADAssertNoError;
 
-    //add two items with the same client ID and same user ID but differnet resource
+    //add three items (ATs and RTs) with the same client ID and same user ID but differnet resource
     ADTokenCacheItem* item1 = [self adCreateCacheItem:@"eric@contoso.com"];
     [item1 setResource:@"resource 1"];
     [mStore addOrUpdateItem:item1 correlationId:nil error:&error];
     ADTokenCacheItem* item2 = [self adCreateCacheItem:@"eric@contoso.com"];
     [item2 setResource:@"resource 2"];
     [mStore addOrUpdateItem:item2 correlationId:nil error:&error];
-    //add another two more items
-    ADTokenCacheItem* item3 = [self adCreateCacheItem:@"jack@contoso.com"];
+    ADTokenCacheItem* item3 = [self adCreateATCacheItem:@"resource 3" userId:@"eric@contoso.com"];
     [mStore addOrUpdateItem:item3 correlationId:nil error:&error];
-    ADTokenCacheItem* item4 = [self adCreateCacheItem:@"rose@contoso.com"];
+
+    //add another two more items
+    ADTokenCacheItem* item4 = [self adCreateCacheItem:@"jack@contoso.com"];
     [mStore addOrUpdateItem:item4 correlationId:nil error:&error];
+    ADTokenCacheItem* item5 = [self adCreateCacheItem:@"rose@contoso.com"];
+    [mStore addOrUpdateItem:item5 correlationId:nil error:&error];
     ADAssertNoError;
-    XCTAssertEqual([self count], 4);
+    XCTAssertEqual([self count], 5);
 
     //remove items with user ID as @"eric@contoso.com" and client ID as TEST_CLIENT_ID
     [mStore removeAllForUserId:@"eric@contoso.com" clientId:TEST_CLIENT_ID error:&error];
     ADAssertNoError;
     XCTAssertEqual([self count], 2);
 
-    //only item3 and item4 are left in cache 
-    [self verifyCacheContainsItem:item3];
+    //only item4 and item5 are left in cache
     [self verifyCacheContainsItem:item4];
+    [self verifyCacheContainsItem:item5];
 }
 
 - (BOOL)wipeTokenDataExist