Merge pull request #787 from AzureAD/oldalton/fix_external_account_matching

oldalton · web-flow · commit 8576bc8b5ed5 · 2019-11-26T16:52:23.000-08:00
Fix external account matching when account doesn't have an identifier
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,7 @@
+## [1.0.4] - 2019-11-26
+### Fixed
+- Fixed external account matching when identifier is not present (#787)
+
 ## [1.0.3] - 2019-11-15
 ### Added
 - Added default implementation for ADAL legacy persistence
diff --git a/MSAL.podspec b/MSAL.podspec
@@ -1,6 +1,6 @@
 Pod::Spec.new do |s|
   s.name         = "MSAL"
-  s.version      = "1.0.3"
+  s.version      = "1.0.4"
   s.summary      = "Microsoft Authentication Library (MSAL) Preview for iOS"
 
   s.description  = <<-DESC
diff --git a/MSAL/resources/ios/Info.plist b/MSAL/resources/ios/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>1.0.3</string>
+	<string>1.0.4</string>
 	<key>CFBundleVersion</key>
 	<string>$(CURRENT_PROJECT_VERSION)</string>
 	<key>NSPrincipalClass</key>
diff --git a/MSAL/resources/mac/Info.plist b/MSAL/resources/mac/Info.plist
@@ -15,7 +15,7 @@
 	<key>CFBundlePackageType</key>
 	<string>FMWK</string>
 	<key>CFBundleShortVersionString</key>
-	<string>1.0.3</string>
+	<string>1.0.4</string>
 	<key>CFBundleVersion</key>
 	<string>$(CURRENT_PROJECT_VERSION)</string>
 	<key>NSHumanReadableCopyright</key>
diff --git a/MSAL/src/MSALPublicClientApplication.m b/MSAL/src/MSALPublicClientApplication.m
@@ -357,6 +357,8 @@ - (MSALAccount *)accountForHomeAccountId:(NSString *)homeAccountId
 - (MSALAccount *)accountForIdentifier:(NSString *)identifier
                                 error:(NSError **)error
 {
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Querying MSAL account for identifier %@", MSID_PII_LOG_TRACKABLE(identifier));
+    
     MSALAccountsProvider *request = [[MSALAccountsProvider alloc] initWithTokenCache:self.tokenCache
                                                                             clientId:self.internalConfig.clientId
                                                              externalAccountProvider:self.externalAccountHandler];
@@ -368,12 +370,16 @@ - (MSALAccount *)accountForIdentifier:(NSString *)identifier
     
     if (error) *error = [MSALErrorConverter msalErrorFromMsidError:msidError];
     
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Found MSAL account with identifier %@, username %@", MSID_PII_LOG_TRACKABLE(account.identifier), MSID_PII_LOG_EMAIL(account.username));
+    
     return account;
 }
 
 - (NSArray<MSALAccount *> *)accountsForParameters:(MSALAccountEnumerationParameters *)parameters
                                             error:(NSError **)error
 {
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Querying MSAL accounts with parameters (identifier=%@, tenantProfileId=%@, username=%@, return only signed in accounts %d)", MSID_PII_LOG_MASKABLE(parameters.identifier), MSID_PII_LOG_MASKABLE(parameters.tenantProfileIdentifier), MSID_PII_LOG_EMAIL(parameters.username), parameters.returnOnlySignedInAccounts);
+    
     MSALAccountsProvider *request = [[MSALAccountsProvider alloc] initWithTokenCache:self.tokenCache
                                                                             clientId:self.internalConfig.clientId
                                                              externalAccountProvider:self.externalAccountHandler];
@@ -382,12 +388,16 @@ - (MSALAccount *)accountForIdentifier:(NSString *)identifier
     
     if (error) *error = [MSALErrorConverter msalErrorFromMsidError:msidError];
     
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Found MSAL accounts with count %ld", (long)accounts.count);
+    
     return accounts;
 }
 
 - (MSALAccount *)accountForUsername:(NSString *)username
                               error:(NSError * __autoreleasing *)error
 {
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Querying MSAL account for username %@", MSID_PII_LOG_EMAIL(username));
+    
     MSALAccountsProvider *request = [[MSALAccountsProvider alloc] initWithTokenCache:self.tokenCache
                                                                             clientId:self.internalConfig.clientId
                                                              externalAccountProvider:self.externalAccountHandler];
@@ -396,6 +406,8 @@ - (MSALAccount *)accountForUsername:(NSString *)username
     MSALAccount *account = [request accountForParameters:parameters error:&msidError];
 
     if (error) *error = [MSALErrorConverter msalErrorFromMsidError:msidError];
+    
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Found MSAL account with identifier %@, username %@", MSID_PII_LOG_TRACKABLE(account.identifier), MSID_PII_LOG_EMAIL(account.username));
 
     return account;
 }
diff --git a/MSAL/src/MSAL_Internal.h b/MSAL/src/MSAL_Internal.h
@@ -27,7 +27,7 @@
 
 #define MSAL_VER_HIGH       1
 #define MSAL_VER_LOW        0
-#define MSAL_VER_PATCH      3
+#define MSAL_VER_PATCH      4
 
 #define STR_HELPER(x) #x
 #define STR(x) STR_HELPER(x)
diff --git a/MSAL/src/configuration/external/ios/MSALLegacySharedADALAccount.m b/MSAL/src/configuration/external/ios/MSALLegacySharedADALAccount.m
@@ -51,9 +51,7 @@ - (instancetype)initWithJSONDictionary:(NSDictionary *)jsonDictionary error:(NSE
     self = [super initWithJSONDictionary:jsonDictionary error:error];
     
     if (self)
-    {
-        MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Creating external account from ADAL account");
-        
+    {        
         if (![_accountType isEqualToString:kADALAccountType])
         {
             MSID_LOG_WITH_CTX(MSIDLogLevelError, nil, @"Failed to create ADAL account. Wrong account type %@ provided", _accountType);
@@ -134,17 +132,17 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters
     
     if (parameters.identifier)
     {
-        matchResult &= ([self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);
+        matchResult &= (self.identifier && [self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);
     }
     
     if (parameters.username)
     {
-        matchResult &= ([self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);
+        matchResult &= (self.username && [self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);
     }
     
     if (parameters.tenantProfileIdentifier)
     {
-        matchResult &= ([self.objectId caseInsensitiveCompare:parameters.tenantProfileIdentifier] == NSOrderedSame);
+        matchResult &= (self.objectId && [self.objectId caseInsensitiveCompare:parameters.tenantProfileIdentifier] == NSOrderedSame);
     }
     
     return matchResult &= [super matchesParameters:parameters];
diff --git a/MSAL/src/configuration/external/ios/MSALLegacySharedAccount.m b/MSAL/src/configuration/external/ios/MSALLegacySharedAccount.m
@@ -118,6 +118,7 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters
         NSString *appIdentifier = [[NSBundle mainBundle] bundleIdentifier];
         NSString *signinStatus = [self.signinStatusDictionary msidStringObjectForKey:appIdentifier];
         
+        MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Requested to only returned signed in accounts. Current sign in status for the app is %@", signinStatus);
         return [signinStatus isEqualToString:@"SignedIn"];
     }
     else if (![self.signinStatusDictionary count])
diff --git a/MSAL/src/configuration/external/ios/MSALLegacySharedAccountsProvider.m b/MSAL/src/configuration/external/ios/MSALLegacySharedAccountsProvider.m
@@ -90,7 +90,7 @@ - (instancetype)initWithSharedKeychainAccessGroup:(NSString *)sharedGroup
 - (nullable NSArray<id<MSALAccount>> *)accountsWithParametersImpl:(MSALAccountEnumerationParameters *)parameters
                                                             error:(NSError * _Nullable * _Nullable)error
 {
-    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Reading accounts with parameters %@", MSID_PII_LOG_MASKABLE(parameters));
+    MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Reading accounts with parameters (identifier=%@, tenantProfileId=%@, username=%@, return only signed in accounts %d)", MSID_PII_LOG_MASKABLE(parameters.identifier), MSID_PII_LOG_MASKABLE(parameters.tenantProfileIdentifier), MSID_PII_LOG_EMAIL(parameters.username), parameters.returnOnlySignedInAccounts);
     
     NSMutableSet *allAccounts = [NSMutableSet new];
     NSTimeInterval lastWrite = [[NSDate distantPast] timeIntervalSince1970];
diff --git a/MSAL/src/configuration/external/ios/MSALLegacySharedMSAAccount.m b/MSAL/src/configuration/external/ios/MSALLegacySharedMSAAccount.m
@@ -110,12 +110,12 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters
     
     if (parameters.identifier)
     {
-        matchResult &= ([self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);
+        matchResult &= (self.identifier && [self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);
     }
     
     if (parameters.username)
     {
-        matchResult &= ([self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);
+        matchResult &= (self.username && [self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);
     }
     
     if (parameters.tenantProfileIdentifier)
diff --git a/MSAL/test/unit/ios/external-cache/MSALLegacySharedADALAccountTests.m b/MSAL/test/unit/ios/external-cache/MSALLegacySharedADALAccountTests.m
@@ -393,6 +393,47 @@ - (void)testMatchesWithParameters_whenIdentifierNonNil_andIdentifierDifferent_sh
     XCTAssertFalse(result);
 }
 
+- (void)testMatchesWithParameters_whenRequestedIdentifierNonNil_andIdentifierInCacheNil_shouldReturnNO
+{
+    NSDictionary *jsonDictionary = @{@"authEndpointUrl": @"https://login.windows.net/tid/oauth2/authorize",
+                                     @"id": [NSUUID UUID].UUIDString,
+                                     @"environment": @"PROD",
+                                     @"oid": @"oid",
+                                     @"originAppId": @"com.myapp.app",
+                                     @"tenantDisplayName": @"",
+                                     @"type": @"ADAL",
+                                     @"displayName": @"myDisplayName.contoso.user",
+                                     @"tenantId": @"tid",
+                                     @"username": @"user@contoso.com"};
+    
+    MSALLegacySharedADALAccount *account = [[MSALLegacySharedADALAccount alloc] initWithJSONDictionary:jsonDictionary error:nil];
+    
+    MSALAccountEnumerationParameters *params = [[MSALAccountEnumerationParameters alloc] initWithIdentifier:@"oid.tid"];
+    params.returnOnlySignedInAccounts = NO;
+    BOOL result = [account matchesParameters:params];
+    XCTAssertFalse(result);
+}
+
+- (void)testMatchesWithParameters_whenRequestedUsernameNonNil_andUsernameInCacheNil_shouldReturnNO
+{
+    NSDictionary *jsonDictionary = @{@"authEndpointUrl": @"https://login.windows.net/common/oauth2/authorize",
+                                     @"id": [NSUUID UUID].UUIDString,
+                                     @"environment": @"PROD",
+                                     @"oid": @"oid",
+                                     @"originAppId": @"com.myapp.app",
+                                     @"tenantDisplayName": @"",
+                                     @"type": @"ADAL",
+                                     @"displayName": @"myDisplayName.contoso.user",
+                                     @"tenantId": @"tid"};
+    
+    MSALLegacySharedADALAccount *account = [[MSALLegacySharedADALAccount alloc] initWithJSONDictionary:jsonDictionary error:nil];
+    
+    MSALAccountEnumerationParameters *params = [[MSALAccountEnumerationParameters alloc] initWithIdentifier:nil username:@"user@consoto.com"];
+    params.returnOnlySignedInAccounts = NO;
+    BOOL result = [account matchesParameters:params];
+    XCTAssertFalse(result);
+}
+
 #pragma mark - Update
 
 - (void)testUpdateWithMSALAccount_whenUsernameAndSigninStatusChanged_shouldUpdateUsername

Original file line number	Diff line number	Diff line change
`@@ -51,9 +51,7 @@ - (instancetype)initWithJSONDictionary:(NSDictionary *)jsonDictionary error:(NSE`
`51`	`51`	`self = [super initWithJSONDictionary:jsonDictionary error:error];`
`52`	`52`
`53`	`53`	`if (self)`
`54`		`- {`
`55`		`- MSID_LOG_WITH_CTX(MSIDLogLevelInfo, nil, @"Creating external account from ADAL account");`
`56`		`-`
	`54`	`+ {`
`57`	`55`	`if (![_accountType isEqualToString:kADALAccountType])`
`58`	`56`	`{`
`59`	`57`	`MSID_LOG_WITH_CTX(MSIDLogLevelError, nil, @"Failed to create ADAL account. Wrong account type %@ provided", _accountType);`
`@@ -134,17 +132,17 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters`
`134`	`132`
`135`	`133`	`if (parameters.identifier)`
`136`	`134`	`{`
`137`		`- matchResult &= ([self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);`
	`135`	`+ matchResult &= (self.identifier && [self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);`
`138`	`136`	`}`
`139`	`137`
`140`	`138`	`if (parameters.username)`
`141`	`139`	`{`
`142`		`- matchResult &= ([self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);`
	`140`	`+ matchResult &= (self.username && [self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);`
`143`	`141`	`}`
`144`	`142`
`145`	`143`	`if (parameters.tenantProfileIdentifier)`
`146`	`144`	`{`
`147`		`- matchResult &= ([self.objectId caseInsensitiveCompare:parameters.tenantProfileIdentifier] == NSOrderedSame);`
	`145`	`+ matchResult &= (self.objectId && [self.objectId caseInsensitiveCompare:parameters.tenantProfileIdentifier] == NSOrderedSame);`
`148`	`146`	`}`
`149`	`147`
`150`	`148`	`return matchResult &= [super matchesParameters:parameters];`
Original file line number	Diff line number	Diff line change
`@@ -118,6 +118,7 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters`
`118`	`118`	`NSString *appIdentifier = [[NSBundle mainBundle] bundleIdentifier];`
`119`	`119`	`NSString *signinStatus = [self.signinStatusDictionary msidStringObjectForKey:appIdentifier];`
`120`	`120`
	`121`	`+ MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Requested to only returned signed in accounts. Current sign in status for the app is %@", signinStatus);`
`121`	`122`	`return [signinStatus isEqualToString:@"SignedIn"];`
`122`	`123`	`}`
`123`	`124`	`else if (![self.signinStatusDictionary count])`
Original file line number	Diff line number	Diff line change
`@@ -90,7 +90,7 @@ - (instancetype)initWithSharedKeychainAccessGroup:(NSString *)sharedGroup`
`90`	`90`	`- (nullable NSArray<id<MSALAccount>> )accountsWithParametersImpl:(MSALAccountEnumerationParameters )parameters`
`91`	`91`	`error:(NSError * _Nullable * _Nullable)error`
`92`	`92`	`{`
`93`		`- MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Reading accounts with parameters %@", MSID_PII_LOG_MASKABLE(parameters));`
	`93`	`+ MSID_LOG_WITH_CTX_PII(MSIDLogLevelInfo, nil, @"Reading accounts with parameters (identifier=%@, tenantProfileId=%@, username=%@, return only signed in accounts %d)", MSID_PII_LOG_MASKABLE(parameters.identifier), MSID_PII_LOG_MASKABLE(parameters.tenantProfileIdentifier), MSID_PII_LOG_EMAIL(parameters.username), parameters.returnOnlySignedInAccounts);`
`94`	`94`
`95`	`95`	`NSMutableSet *allAccounts = [NSMutableSet new];`
`96`	`96`	`NSTimeInterval lastWrite = [[NSDate distantPast] timeIntervalSince1970];`
Original file line number	Diff line number	Diff line change
`@@ -110,12 +110,12 @@ - (BOOL)matchesParameters:(MSALAccountEnumerationParameters *)parameters`
`110`	`110`
`111`	`111`	`if (parameters.identifier)`
`112`	`112`	`{`
`113`		`- matchResult &= ([self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);`
	`113`	`+ matchResult &= (self.identifier && [self.identifier caseInsensitiveCompare:parameters.identifier] == NSOrderedSame);`
`114`	`114`	`}`
`115`	`115`
`116`	`116`	`if (parameters.username)`
`117`	`117`	`{`
`118`		`- matchResult &= ([self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);`
	`118`	`+ matchResult &= (self.username && [self.username caseInsensitiveCompare:parameters.username] == NSOrderedSame);`
`119`	`119`	`}`
`120`	`120`
`121`	`121`	`if (parameters.tenantProfileIdentifier)`