Merge pull request #204 from AzureAD/oldalton/merge_release_1.0.0

oldalton · web-flow · commit b991a98cb09c · 2018-08-20T19:10:52.000-07:00
Merge release 1.0.0 back to dev (cache optimizations and tombstone changes)
diff --git a/IdentityCore/src/cache/MSIDMacTokenCache.m b/IdentityCore/src/cache/MSIDMacTokenCache.m
@@ -296,6 +296,13 @@ - (void)addToItems:(nonnull NSMutableArray *)items
     if (item)
     {
         item = [item copy];
+
+        // Skip tombstones generated from previous versions of ADAL.
+        if ([item isTombstone])
+        {
+            return;
+        }
+
         [items addObject:item];
     }
 }
diff --git a/IdentityCore/src/cache/accessor/MSIDDefaultTokenCacheAccessor.m b/IdentityCore/src/cache/accessor/MSIDDefaultTokenCacheAccessor.m
@@ -76,10 +76,17 @@ - (BOOL)saveTokensWithConfiguration:(MSIDConfiguration *)configuration
 {
     MSID_LOG_VERBOSE(context, @"(Default accessor) Saving multi resource refresh token");
 
+    // Save access token
     BOOL result = [self saveAccessTokenWithConfiguration:configuration response:response context:context error:error];
 
     if (!result) return result;
 
+    // Save ID token
+    result = [self saveIDTokenWithConfiguration:configuration response:response context:context error:error];
+
+    if (!result) return result;
+
+    // Save SSO state (refresh token and account)
     return [self saveSSOStateWithConfiguration:configuration response:response context:context error:error];
 }
 
@@ -110,28 +117,11 @@ - (BOOL)saveSSOStateWithConfiguration:(MSIDConfiguration *)configuration
 
     MSID_LOG_VERBOSE(context, @"(Legacy accessor) Saving SSO state");
 
-    BOOL result = [self saveIDTokenWithConfiguration:configuration response:response context:context error:error];
-    result &= [self saveRefreshTokenWithConfiguration:configuration response:response context:context error:error];
-    result &= [self saveAccountWithConfiguration:configuration response:response context:context error:error];
+    BOOL result = [self saveRefreshTokenWithConfiguration:configuration response:response context:context error:error];
 
-    if (!result)
-    {
-        return NO;
-    }
+    if (!result) return NO;
 
-    for (id<MSIDCacheAccessor> accessor in _otherAccessors)
-    {
-        if (![accessor saveSSOStateWithConfiguration:configuration
-                                            response:response
-                                             context:context
-                                               error:error])
-        {
-            MSID_LOG_WARN(context, @"Failed to save SSO state in other accessor: %@", accessor.class);
-            MSID_LOG_WARN_PII(context, @"Failed to save SSO state in other accessor: %@, error %@", accessor.class, *error);
-        }
-    }
-
-    return YES;
+    return [self saveAccountWithConfiguration:configuration response:response context:context error:error];
 }
 
 - (MSIDRefreshToken *)getRefreshTokenWithAccount:(MSIDAccountIdentifier *)account
@@ -794,51 +784,60 @@ - (MSIDBaseToken *)getRefreshTokenByLegacyUserId:(NSString *)legacyUserId
                                          context:(id<MSIDRequestContext>)context
                                            error:(NSError **)error
 {
+    MSID_LOG_VERBOSE(context, @"(Default accessor) Looking for token with authority %@, clientId %@", authority, clientId);
+    MSID_LOG_VERBOSE_PII(context, @"(Default accessor) Looking for token with authority %@, clientId %@, legacy userId %@", authority, clientId, legacyUserId);
+
     MSIDTelemetryCacheEvent *event = [MSIDTelemetry startCacheEventWithName:MSID_TELEMETRY_EVENT_TOKEN_CACHE_LOOKUP context:context];
 
     NSArray<NSString *> *aliases = [_factory defaultCacheAliasesForEnvironment:authority.msidHostWithPortIfNecessary];
 
-    for (NSString *alias in aliases)
+    NSString *clientIdForQueries = clientId;
+
+    if (familyId)
     {
-        MSID_LOG_VERBOSE(context, @"(Default accessor) Looking for token with alias %@, clientId %@", alias, clientId);
-        MSID_LOG_VERBOSE_PII(context, @"(Default accessor) Looking for token with alias %@, clientId %@, legacy userId %@", alias, clientId, legacyUserId);
+        // If family ID is provided, we don't need to lookup by a specific client ID
+        clientIdForQueries = nil;
+    }
 
-        MSIDDefaultCredentialCacheQuery *idTokensQuery = [MSIDDefaultCredentialCacheQuery new];
-        idTokensQuery.environment = alias;
-        idTokensQuery.clientId = clientId;
-        idTokensQuery.credentialType = MSIDIDTokenType;
+    MSIDDefaultCredentialCacheQuery *idTokensQuery = [MSIDDefaultCredentialCacheQuery new];
+    idTokensQuery.environmentAliases = aliases;
+    idTokensQuery.clientId = clientIdForQueries;
+    idTokensQuery.credentialType = MSIDIDTokenType;
 
-        NSArray<MSIDCredentialCacheItem *> *matchedIdTokens = [_accountCredentialCache getCredentialsWithQuery:idTokensQuery
-                                                                                                  legacyUserId:legacyUserId
-                                                                                                       context:context
-                                                                                                         error:error];
+    NSArray<MSIDCredentialCacheItem *> *matchedIdTokens = [_accountCredentialCache getCredentialsWithQuery:idTokensQuery
+                                                                                              legacyUserId:legacyUserId
+                                                                                                   context:context
+                                                                                                     error:error];
 
-        if ([matchedIdTokens count])
-        {
-            NSString *homeAccountId = matchedIdTokens[0].homeAccountId;
+    if ([matchedIdTokens count])
+    {
+        MSIDCredentialCacheItem *matchedIdToken = matchedIdTokens[0];
+        NSString *homeAccountId = matchedIdToken.homeAccountId;
 
-            MSIDDefaultCredentialCacheQuery *rtQuery = [MSIDDefaultCredentialCacheQuery new];
-            rtQuery.homeAccountId = homeAccountId;
-            rtQuery.environment = alias;
-            rtQuery.clientId = clientId;
-            rtQuery.familyId = familyId;
-            rtQuery.credentialType = MSIDRefreshTokenType;
+        MSID_LOG_VERBOSE(context, @"(Default accessor] Found Match with environment %@, realm %@, client %@", matchedIdToken.environment, matchedIdToken.realm, matchedIdToken.clientId);
+        MSID_LOG_VERBOSE_PII(context, @"(Default accessor] Found Match with environment %@, realm %@, client %@, home account ID %@", matchedIdToken.environment, matchedIdToken.realm, matchedIdToken.clientId, matchedIdToken.homeAccountId);
 
-            NSArray<MSIDCredentialCacheItem *> *rtCacheItems = [_accountCredentialCache getCredentialsWithQuery:rtQuery
-                                                                                              legacyUserId:nil
-                                                                                                   context:context
-                                                                                                     error:error];
+        MSIDDefaultCredentialCacheQuery *rtQuery = [MSIDDefaultCredentialCacheQuery new];
+        rtQuery.homeAccountId = homeAccountId;
+        rtQuery.environmentAliases = aliases;
+        rtQuery.clientId = clientIdForQueries;
+        rtQuery.familyId = familyId;
+        rtQuery.credentialType = MSIDRefreshTokenType;
 
-            if ([rtCacheItems count])
-            {
-                MSID_LOG_VERBOSE(context, @"(Default accessor) Found %lu refresh tokens", (unsigned long)[rtCacheItems count]);
-                MSIDCredentialCacheItem *resultItem = rtCacheItems[0];
-                MSIDBaseToken *resultToken = [resultItem tokenWithType:MSIDRefreshTokenType];
-                resultToken.storageAuthority = resultToken.authority;
-                resultToken.authority = authority;
-                [MSIDTelemetry stopCacheEvent:event withItem:resultToken success:YES context:context];
-                return resultToken;
-            }
+        NSArray<MSIDCredentialCacheItem *> *rtCacheItems = [_accountCredentialCache getCredentialsWithQuery:rtQuery
+                                                                                               legacyUserId:nil
+                                                                                                    context:context
+                                                                                                      error:error];
+
+        if ([rtCacheItems count])
+        {
+            MSID_LOG_VERBOSE(context, @"(Default accessor) Found %lu refresh tokens", (unsigned long)[rtCacheItems count]);
+            MSIDCredentialCacheItem *resultItem = rtCacheItems[0];
+            MSIDBaseToken *resultToken = [resultItem tokenWithType:MSIDRefreshTokenType];
+            resultToken.storageAuthority = resultToken.authority;
+            resultToken.authority = authority;
+            [MSIDTelemetry stopCacheEvent:event withItem:resultToken success:YES context:context];
+            return resultToken;
         }
     }
 
diff --git a/IdentityCore/src/cache/ios/MSIDKeychainTokenCache.m b/IdentityCore/src/cache/ios/MSIDKeychainTokenCache.m
@@ -216,7 +216,7 @@ - (MSIDCredentialCacheItem *)tokenWithKey:(MSIDCacheKey *)key
         if (tokenItem)
         {
             // Delete tombstones generated from previous versions of ADAL.
-            if ([tokenItem.secret isEqualToString:@"<tombstone>"])
+            if ([tokenItem isTombstone])
             {
                 [self deleteTombstoneWithService:attrs[(id)kSecAttrService]
                                          account:attrs[(id)kSecAttrAccount]
diff --git a/IdentityCore/src/cache/token/MSIDCredentialCacheItem.h b/IdentityCore/src/cache/token/MSIDCredentialCacheItem.h
@@ -85,4 +85,6 @@
           targetMatching:(MSIDComparisonOptions)matchingOptions
         clientIdMatching:(MSIDComparisonOptions)clientIDMatchingOptions;
 
+- (BOOL)isTombstone;
+
 @end
diff --git a/IdentityCore/src/cache/token/MSIDCredentialCacheItem.m b/IdentityCore/src/cache/token/MSIDCredentialCacheItem.m
@@ -309,13 +309,20 @@ - (BOOL)matchesWithRealm:(nullable NSString *)realm
         return NO;
     }
 
+    if (!clientId && !familyId)
+    {
+        return YES;
+    }
+
     if (clientIDMatchingOptions == MSIDSuperSet)
     {
         if ((clientId && [self.clientId isEqualToString:clientId])
             || (familyId && [self.familyId isEqualToString:familyId]))
         {
             return YES;
         }
+
+        return NO;
     }
     else
     {
@@ -333,4 +340,9 @@ - (BOOL)matchesWithRealm:(nullable NSString *)realm
     return YES;
 }
 
+- (BOOL)isTombstone
+{
+    return [self.secret isEqualToString:@"<tombstone>"];
+}
+
 @end
diff --git a/IdentityCore/src/cache/token/MSIDLegacyTokenCacheItem.m b/IdentityCore/src/cache/token/MSIDLegacyTokenCacheItem.m
@@ -218,4 +218,9 @@ - (MSIDIdTokenClaims *)idTokenClaims
     return _idTokenClaims;
 }
 
+- (BOOL)isTombstone
+{
+    return self.refreshToken && [self.refreshToken isEqualToString:@"<tombstone>"];
+}
+
 @end
diff --git a/IdentityCore/src/oauth2/MSIDWebviewFactory.m b/IdentityCore/src/oauth2/MSIDWebviewFactory.m
@@ -133,7 +133,6 @@ - (NSURL *)startURLFromConfiguration:(MSIDWebviewConfiguration *)configuration r
     NSURLComponents *urlComponents = [NSURLComponents componentsWithURL:configuration.authorizationEndpoint resolvingAgainstBaseURL:NO];
     NSDictionary *parameters = [self authorizationParametersFromConfiguration:configuration requestState:state];
     
-    urlComponents.queryItems = [parameters urlQueryItemsArray];
     urlComponents.percentEncodedQuery = [parameters msidURLFormEncode];
     
     return urlComponents.URL;
diff --git a/IdentityCore/src/oauth2/aad_v2/MSIDAADV2IdTokenClaims.m b/IdentityCore/src/oauth2/aad_v2/MSIDAADV2IdTokenClaims.m
@@ -52,7 +52,16 @@ - (void)initDerivedProperties
     }
 
     _uniqueId = [MSIDHelpers normalizeUserId:uniqueId];
-    _userId = [MSIDHelpers normalizeUserId:self.preferredUsername];
+
+    // Set userId
+    NSString *userId = self.preferredUsername;
+
+    if ([NSString msidIsStringNilOrBlank:userId])
+    {
+        userId = self.subject;
+    }
+
+    _userId = [MSIDHelpers normalizeUserId:userId];
     _userIdDisplayable = YES;
 }
 
diff --git a/IdentityCore/tests/integration/MSIDDefaultAccessorSSOIntegrationTests.m b/IdentityCore/tests/integration/MSIDDefaultAccessorSSOIntegrationTests.m
@@ -201,7 +201,7 @@ - (void)testSaveTokensWithFactory_whenMultiResourceResponse_andNoOtherAccessors_
     XCTAssertEqualObjects(account.authority.absoluteString, @"https://login.microsoftonline.com/tenantId.onmicrosoft.com");
 }
 
-- (void)testSaveTokensWithFactory_whenMultiResourceResponse_savesTokensToBothAccessors
+- (void)testSaveTokensWithFactory_whenMultiResourceResponse_savesTokensOnlyToDefaultAccessor
 {
     NSString *idToken = [MSIDTestIdTokenUtil idTokenWithPreferredUsername:@"upn@test.com" subject:@"subject" givenName:@"Hello" familyName:@"World" name:@"Hello World" version:@"2.0" tid:@"tenantId.onmicrosoft.com"];
 
@@ -283,21 +283,10 @@ - (void)testSaveTokensWithFactory_whenMultiResourceResponse_savesTokensToBothAcc
     XCTAssertEqual([legacyAccessTokens count], 0);
 
     NSArray *legacyRefreshTokens = [self getAllLegacyRefreshTokens];
-    XCTAssertEqual([legacyRefreshTokens count], 1);
-
-    MSIDLegacyRefreshToken *legacyRefreshToken = legacyRefreshTokens[0];
-    XCTAssertEqualObjects(legacyRefreshToken.idToken, idToken);
-    XCTAssertEqualObjects(legacyRefreshToken.accountIdentifier.legacyAccountId, @"upn@test.com");
-    XCTAssertEqualObjects(legacyRefreshToken.refreshToken, @"refresh token");
-    XCTAssertNil(legacyRefreshToken.familyId);
-    XCTAssertEqual(legacyRefreshToken.credentialType, MSIDRefreshTokenType);
-    XCTAssertEqualObjects(legacyRefreshToken.authority.absoluteString, @"https://login.microsoftonline.com/common");
-    XCTAssertEqualObjects(legacyRefreshToken.clientId, @"test_client_id");
-    XCTAssertEqualObjects(legacyRefreshToken.accountIdentifier.homeAccountId, @"uid.utid");
-    XCTAssertEqualObjects(legacyRefreshToken.additionalServerInfo, [NSDictionary dictionary]);
+    XCTAssertEqual([legacyRefreshTokens count], 0);
 }
 
-- (void)testSaveTokensWithFactory_whenMultiResourceFOCIResponse_savesTokensToBothAccessors
+- (void)testSaveTokensWithFactory_whenMultiResourceFOCIResponse_savesTokensOnlyToDefaultAccessor
 {
     NSString *idToken = [MSIDTestIdTokenUtil idTokenWithPreferredUsername:@"upn@test.com" subject:@"subject" givenName:@"Hello" familyName:@"World" name:@"Hello World" version:@"2.0" tid:@"tenantId.onmicrosoft.com"];
 
@@ -322,16 +311,7 @@ - (void)testSaveTokensWithFactory_whenMultiResourceFOCIResponse_savesTokensToBot
     XCTAssertEqual([legacyAccessTokens count], 0);
 
     NSArray *legacyRefreshTokens = [self getAllLegacyRefreshTokens];
-    XCTAssertEqual([legacyRefreshTokens count], 2);
-    XCTAssertNotEqualObjects(legacyRefreshTokens[0], legacyRefreshTokens[1]);
-
-    MSIDLegacyRefreshToken *refreshToken1 = legacyRefreshTokens[0];
-    MSIDLegacyRefreshToken *refreshToken2 = legacyRefreshTokens[1];
-
-    XCTAssertEqualObjects(refreshToken1.familyId, @"2");
-    XCTAssertEqualObjects(refreshToken2.familyId, @"2");
-    XCTAssertEqualObjects(refreshToken1.clientId, @"test_client_id");
-    XCTAssertEqualObjects(refreshToken2.clientId, @"foci-2");
+    XCTAssertEqual([legacyRefreshTokens count], 0);
 
     NSArray *defaultAccessTokens = [self getAllAccessTokens];
     XCTAssertEqual([defaultAccessTokens count], 1);
@@ -361,23 +341,23 @@ - (void)testSaveTokensWithFactory_whenMultiResourceFOCIResponse_savesTokensToBot
                                                                 context:nil
                                                                   error:&error];
 
-    XCTAssertEqual([clientAccounts count], 1);
+    XCTAssertEqual([clientAccounts count], 0);
 
     NSArray *familyAccounts = [_otherAccessor allAccountsForEnvironment:@"login.microsoftonline.com"
                                                                clientId:nil
                                                                familyId:@"2"
                                                                 context:nil
                                                                   error:&error];
 
-    XCTAssertEqual([familyAccounts count], 1);
+    XCTAssertEqual([familyAccounts count], 0);
 
     NSArray *allAccounts = [_otherAccessor allAccountsForEnvironment:@"login.microsoftonline.com"
                                                             clientId:@"test_client_id"
                                                             familyId:@"2"
                                                              context:nil
                                                                error:&error];
 
-    XCTAssertEqual([allAccounts count], 1);
+    XCTAssertEqual([allAccounts count], 0);
 }
 
 
@@ -1759,6 +1739,18 @@ - (void)testClearCacheForAccount_whenAccountProvided_shouldRemoveTokens
                      familyId:nil
                      accessor:_defaultAccessor];
 
+    [self saveResponseWithUPN:@"upn@test.com"
+                     clientId:@"test_client_id"
+                    authority:@"https://login.windows.net/common"
+               responseScopes:@"user.sing2 user.dance"
+                  inputScopes:@"user.sing2 user.dance"
+                          uid:@"uid"
+                         utid:@"utid"
+                  accessToken:@"access token 2"
+                 refreshToken:@"refresh token"
+                     familyId:nil
+                     accessor:_otherAccessor];
+
     NSError *error = nil;
     NSArray *accounts = [_defaultAccessor allAccountsForEnvironment:@"login.windows.net" clientId:@"test_client_id" familyId:nil context:nil error:&error];
 
diff --git a/IdentityCore/tests/integration/MSIDDefaultTokenCacheIntegrationTests.m b/IdentityCore/tests/integration/MSIDDefaultTokenCacheIntegrationTests.m
@@ -559,11 +559,10 @@ - (void)testGetTokenWithType_whenTypeRefreshAccountWithUtidAndUidProvided_should
 
 - (void)testGetTokenWithType_whenTypeRefreshAccountWithLegacyIDProvided_shouldReturnToken
 {
-    [_cacheAccessor saveSSOStateWithConfiguration:[MSIDTestConfiguration v2DefaultConfiguration]
-                                         response:[MSIDTestTokenResponse v2DefaultTokenResponse]
-                                          context:nil
-                                            error:nil];
-
+    [_cacheAccessor saveTokensWithConfiguration:[MSIDTestConfiguration v2DefaultConfiguration]
+                                       response:[MSIDTestTokenResponse v2DefaultTokenResponse]
+                                        context:nil
+                                          error:nil];
 
     MSIDAccountIdentifier *account = [[MSIDAccountIdentifier alloc] initWithLegacyAccountId:DEFAULT_TEST_ID_TOKEN_USERNAME
                                                                               homeAccountId:nil];
diff --git a/IdentityCore/tests/integration/MSIDLegacyAccessorSSOIntegrationTests.m b/IdentityCore/tests/integration/MSIDLegacyAccessorSSOIntegrationTests.m
diff --git a/IdentityCore/tests/integration/mac/MSIDMacTokenCacheIntegrationTests.m b/IdentityCore/tests/integration/mac/MSIDMacTokenCacheIntegrationTests.m

Original file line number	Diff line number	Diff line change
`@@ -296,6 +296,13 @@ - (void)addToItems:(nonnull NSMutableArray *)items`
`296`	`296`	`if (item)`
`297`	`297`	`{`
`298`	`298`	`item = [item copy];`
	`299`	`+`
	`300`	`+ // Skip tombstones generated from previous versions of ADAL.`
	`301`	`+ if ([item isTombstone])`
	`302`	`+ {`
	`303`	`+ return;`
	`304`	`+ }`
	`305`	`+`
`299`	`306`	`[items addObject:item];`
`300`	`307`	`}`
`301`	`308`	`}`
Original file line number	Diff line number	Diff line change
`@@ -216,7 +216,7 @@ - (MSIDCredentialCacheItem )tokenWithKey:(MSIDCacheKey )key`
`216`	`216`	`if (tokenItem)`
`217`	`217`	`{`
`218`	`218`	`// Delete tombstones generated from previous versions of ADAL.`
`219`		`- if ([tokenItem.secret isEqualToString:@"<tombstone>"])`
	`219`	`+ if ([tokenItem isTombstone])`
`220`	`220`	`{`
`221`	`221`	`[self deleteTombstoneWithService:attrs[(id)kSecAttrService]`
`222`	`222`	`account:attrs[(id)kSecAttrAccount]`
Original file line number	Diff line number	Diff line change
`@@ -309,13 +309,20 @@ - (BOOL)matchesWithRealm:(nullable NSString *)realm`
`309`	`309`	`return NO;`
`310`	`310`	`}`
`311`	`311`
	`312`	`+ if (!clientId && !familyId)`
	`313`	`+ {`
	`314`	`+ return YES;`
	`315`	`+ }`
	`316`	`+`
`312`	`317`	`if (clientIDMatchingOptions == MSIDSuperSet)`
`313`	`318`	`{`
`314`	`319`	`if ((clientId && [self.clientId isEqualToString:clientId])`
`315`	`320`	`\|\| (familyId && [self.familyId isEqualToString:familyId]))`
`316`	`321`	`{`
`317`	`322`	`return YES;`
`318`	`323`	`}`
	`324`	`+`
	`325`	`+ return NO;`
`319`	`326`	`}`
`320`	`327`	`else`
`321`	`328`	`{`
`@@ -333,4 +340,9 @@ - (BOOL)matchesWithRealm:(nullable NSString *)realm`
`333`	`340`	`return YES;`
`334`	`341`	`}`
`335`	`342`
	`343`	`+- (BOOL)isTombstone`
	`344`	`+{`
	`345`	`+ return [self.secret isEqualToString:@"<tombstone>"];`
	`346`	`+}`
	`347`	`+`
`336`	`348`	`@end`
Original file line number	Diff line number	Diff line change
`@@ -218,4 +218,9 @@ - (MSIDIdTokenClaims *)idTokenClaims`
`218`	`218`	`return _idTokenClaims;`
`219`	`219`	`}`
`220`	`220`
	`221`	`+- (BOOL)isTombstone`
	`222`	`+{`
	`223`	`+ return self.refreshToken && [self.refreshToken isEqualToString:@"<tombstone>"];`
	`224`	`+}`
	`225`	`+`
`221`	`226`	`@end`