Jennyf/ios broker fixes (#1614)

* bug fixes
add logging
fix teamId default
fix keychain w/legacy cache persistance

* combine iosTokenCacheAccessor and iOSlegacy stuff

* reverting changes

devApps/XFormsApp.iOS/Entitlements.plist

@@ -1,5 +1,10 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
 <plist version="1.0">
-<dict/>
+  <dict>
+    <key>keychain-access-groups</key>
+    <array>
+      <string>$(AppIdentifierPrefix)com.microsoft.adalcache</string>
+    </array>
+  </dict>
 </plist>

devApps/XFormsApp/AppConstants.cs

@@ -10,7 +10,7 @@ public static class AppConstants
         public const string UiAutomationTestClientId = "3c1e0e0d-b742-45ba-a35e-01c664e14b16";
         public const string MSIDLAB4ClientId = "4b0db8c2-9f26-4417-8bde-3f0e3656f8e0";
         public const string ManualTestClientId = "d3590ed6-52b3-4102-aeff-aad2292ab01c";
-        public const string BrokerClientId = "c663b6e3-d25b-4566-8b68-4858fc86e85d";
+        public const string BrokerClientId = "3a981c29-5df7-4656-a776-c473e132a0d4";
 
         //Resources
         public const string UiAutomationTestResource = "ae55a6cc-da5e-42f8-b75d-c37e41a1a0d9";

devApps/XFormsApp/SecondPage.cs

@@ -41,7 +41,7 @@ public class SecondPage : ContentPage
 
         public string User = "<User>";
         private string Tenant = "<Tenant>";
-        public const string AndroidBrokerRedirectURI = "msauth://com.microsoft.xformsdroid.adal/mJaAVvdXtcXy369xPWv2C7mV674=";
+        public const string AndroidBrokerRedirectURI = "msauth://com.microsoft.xformsdroid.adal/h9/XUqAd80F9odQHvfN02DYklMA=";
         public const string IOSBrokerRedirectURI = "adaliosapp://com.yourcompany.xformsapp";
         static string RedirectURI = "urn:ietf:wg:oauth:2.0:oob";
 

src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs

@@ -182,7 +182,6 @@ public Guid CorrelationId
         }
 
 #if iOS
-
         private string keychainSecurityGroup;
 
         /// <summary>

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenHandlerBase.cs

@@ -263,7 +263,7 @@ private async Task CheckAndAcquireTokenUsingBrokerAsync()
             else
             {
                 RequestContext.Logger.Verbose("Broker invocation is NOT required");
-                ResultEx = await this.SendTokenRequestAsync().ConfigureAwait(false);
+                ResultEx = await SendTokenRequestAsync().ConfigureAwait(false);
             }
         }
 

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/OAuth2/TokenResponse.cs

@@ -31,16 +31,12 @@
 using System.IO;
 using System.Net;
 using System.Runtime.Serialization;
-using System.Runtime.Serialization.Json;
-using System.Text;
 using Microsoft.Identity.Core;
 using Microsoft.Identity.Core.Cache;
 using Microsoft.Identity.Core.Helpers;
 using Microsoft.Identity.Core.Http;
 using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Helpers;
-using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Http;
 using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Instance;
-using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Platform;
 
 namespace Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.OAuth2
 {
@@ -59,6 +55,7 @@ internal static class TokenResponseClaim
         public const string Error = "error";
         public const string ErrorDescription = "error_description";
         public const string ErrorCodes = "error_codes";
+        public const string ErrorDomain = "error_domain";
         public const string Claims = "claims";
         public const string CloudInstanceHost = "cloud_instance_host_name";
         public const string Authority = "authority";
@@ -119,33 +116,47 @@ internal class TokenResponse
 
         internal static TokenResponse CreateFromBrokerResponse(IDictionary<string, string> responseDictionary)
         {
+            TokenResponse tokenResponse;
+
             if (responseDictionary.ContainsKey(TokenResponseClaim.ErrorDescription))
             {
-                return new TokenResponse
+                tokenResponse = new TokenResponse
                 {
-                    Error = responseDictionary[TokenResponseClaim.Error],
                     ErrorDescription = responseDictionary[TokenResponseClaim.ErrorDescription]
                 };
+                if (responseDictionary.ContainsKey(TokenResponseClaim.Error))
+                {
+                    tokenResponse.Error = responseDictionary[TokenResponseClaim.Error];
+                }
+                else
+                {
+                    // error_domain is a required field in a failed iOS broker response
+                    tokenResponse.Error = responseDictionary[TokenResponseClaim.ErrorDomain];
+                }
             }
 
-            return new TokenResponse
+            else
             {
-                Authority = responseDictionary.ContainsKey("authority")
+                tokenResponse = new TokenResponse
+                {
+                    Authority = responseDictionary.ContainsKey("authority")
                     ? Authenticator.EnsureUrlEndsWithForwardSlash(EncodingHelper.UrlDecode(responseDictionary["authority"]))
                     : null,
-                AccessToken = responseDictionary["access_token"],
-                RefreshToken = responseDictionary.ContainsKey("refresh_token")
+                    AccessToken = responseDictionary["access_token"],
+                    RefreshToken = responseDictionary.ContainsKey("refresh_token")
                     ? responseDictionary["refresh_token"]
                     : null,
-                IdTokenString = responseDictionary["id_token"],
-                TokenType = "Bearer",
-                CorrelationId = responseDictionary["correlation_id"],
-                Resource = responseDictionary["resource"],
-                ExpiresOn = long.Parse(responseDictionary["expires_on"].Split('.')[0], CultureInfo.CurrentCulture),
-                ClientInfo = responseDictionary.ContainsKey("client_info")
+                    IdTokenString = responseDictionary["id_token"],
+                    TokenType = "Bearer",
+                    CorrelationId = responseDictionary["correlation_id"],
+                    Resource = responseDictionary["resource"],
+                    ExpiresOn = long.Parse(responseDictionary["expires_on"].Split('.')[0], CultureInfo.CurrentCulture),
+                    ClientInfo = responseDictionary.ContainsKey("client_info")
                 ? responseDictionary["client_info"]
                 : null,
-            };
+                };
+            }
+            return tokenResponse;
         }
 
         public static TokenResponse CreateFromErrorResponse(IHttpWebResponse webResponse, ICoreLogger logger)

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs

@@ -73,6 +73,7 @@ public bool CanInvokeBroker
                     pp.CallerViewController.InvokeOnMainThread(() =>
                     {
                         res = UIApplication.SharedApplication.CanOpenUrl(new NSUrl("msauth://"));
+                        _logger.Info("iOS Broker can be invoked. ");
                     });
                 }
 
@@ -84,6 +85,8 @@ public async Task<AdalResultWrapper> AcquireTokenUsingBrokerAsync(IDictionary<st
         {
             if (brokerPayload.ContainsKey(BrokerParameter.SilentBrokerFlow))
             {
+                _logger.Info("iOS Broker payload contains silent flow key in payload. " +
+                    "Throwing AdalSilentTokenAcquisitionException() ");
                 throw new AdalSilentTokenAcquisitionException();
             }
 
@@ -122,7 +125,7 @@ public async Task<AdalResultWrapper> AcquireTokenUsingBrokerAsync(IDictionary<st
 
             else
             {
-                _logger.Info("Invoking the iOS broker");
+                _logger.Info("Invoking the iOS broker. ");
                 NSUrl url = new NSUrl("msauth://broker?" + brokerPayload.ToQueryParameter());
                 DispatchQueue.MainQueue.DispatchAsync(() => UIApplication.SharedApplication.OpenUrl(url));
             }
@@ -136,6 +139,8 @@ private AdalResultWrapper ProcessBrokerResponse()
         {
             string[] keyValuePairs = brokerResponse.Query.Split('&');
 
+            _logger.Info("Processing response from iOS Broker. ");
+
             IDictionary<string, string> responseDictionary = new Dictionary<string, string>();
             foreach (string pair in keyValuePairs)
             {
@@ -156,6 +161,7 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary<string, string> r
 
             if (responseDictionary.ContainsKey("error") || responseDictionary.ContainsKey("error_description"))
             {
+                _logger.Info("Broker response returned an error. ");
                 response = TokenResponse.CreateFromBrokerResponse(responseDictionary);
             }
             else
@@ -170,6 +176,7 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary<string, string> r
                 {
                     responseDictionary = EncodingHelper.ParseKeyValueList(decryptedResponse, '&', false, null);
                     response = TokenResponse.CreateFromBrokerResponse(responseDictionary);
+                    _logger.Info("Broker response successful. ");
                 }
                 else
                 {
@@ -178,6 +185,7 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary<string, string> r
                         Error = AdalError.BrokerReponseHashMismatch,
                         ErrorDescription = AdalErrorMessage.BrokerReponseHashMismatch
                     };
+                    _logger.InfoPii("Broker response hash mismatch: " + response.Error, "Broker response hash mismatch. ");
                 }
             }
 

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs

@@ -127,6 +127,10 @@ void ILegacyCachePersistence.WriteCache(byte[] serializedCache)
                         string msg = "Failed to save adal cache record: ";
                         CoreLoggerBase.Default.WarningPii(msg + err, msg);
                     }
+                    else
+                    {
+                        CoreLoggerBase.Default.Warning("Saved adal cache record. ");
+                    }
                 }
             }
             catch (Exception ex)
@@ -135,4 +139,4 @@ void ILegacyCachePersistence.WriteCache(byte[] serializedCache)
             }
         }
     }
-}
+}

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs

@@ -31,7 +31,6 @@
 using Microsoft.Identity.Core.Cache;
 using UIKit;
 using Foundation;
-using Microsoft.Identity.Core.Http;
 
 namespace Microsoft.Identity.Core
 {

src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs

@@ -62,9 +62,19 @@ enum CredentialAttrType
         private const string DefaultKeychainGroup = "com.microsoft.adalcache";
         // Identifier for the keychain item used to retrieve current team ID
         private const string TeamIdKey = "DotNetTeamIDHint";
+        private RequestContext _requestContext;
 
         private string keychainGroup;
-        private RequestContext _requestContext;
+
+        public iOSTokenCacheAccessor()
+        {
+            keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup;
+        }
+
+        public iOSTokenCacheAccessor(RequestContext requestContext) : this()
+        {
+            _requestContext = requestContext;
+        }
 
         private string GetBundleId()
         {
@@ -123,16 +133,6 @@ private string GetTeamId()
                 CoreErrorMessages.CannotAccessPublisherKeyChain);
         }
 
-        public iOSTokenCacheAccessor()
-        {
-            keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup;
-        }
-
-        public iOSTokenCacheAccessor(RequestContext requestContext) : this()
-        {
-            _requestContext = requestContext;
-        }
-
         public void SaveAccessToken(MsalAccessTokenCacheItem item)
         {
             var key = item.GetKey();
@@ -332,4 +332,4 @@ public void ClearAccessTokens()
             throw new NotImplementedException();
         }
     }
-}
+}