From 0290dc12285a93ac00f0d0fcfa36f267d0058403 Mon Sep 17 00:00:00 2001 From: jennyf19 Date: Tue, 11 Jun 2019 07:25:39 -0700 Subject: [PATCH] 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 | 7 ++- devApps/XFormsApp/AppConstants.cs | 2 +- devApps/XFormsApp/SecondPage.cs | 2 +- .../AuthenticationContext.cs | 1 - .../Internal/Flows/AcquireTokenHandlerBase.cs | 2 +- .../Internal/OAuth2/TokenResponse.cs | 45 ++++++++++++------- .../Platforms/iOS/iOSBroker.cs | 10 ++++- .../iOS/iOSLegacyCachePersistance.cs | 6 ++- .../Platforms/iOS/iOSPlatformProxy.cs | 1 - .../Platforms/iOS/iOSTokenCacheAccessor.cs | 24 +++++----- 10 files changed, 63 insertions(+), 37 deletions(-) diff --git a/devApps/XFormsApp.iOS/Entitlements.plist b/devApps/XFormsApp.iOS/Entitlements.plist index 0c67376eb..6a8e4f5cd 100644 --- a/devApps/XFormsApp.iOS/Entitlements.plist +++ b/devApps/XFormsApp.iOS/Entitlements.plist @@ -1,5 +1,10 @@ - + + keychain-access-groups + + $(AppIdentifierPrefix)com.microsoft.adalcache + + diff --git a/devApps/XFormsApp/AppConstants.cs b/devApps/XFormsApp/AppConstants.cs index 35aff4334..577e96096 100644 --- a/devApps/XFormsApp/AppConstants.cs +++ b/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"; diff --git a/devApps/XFormsApp/SecondPage.cs b/devApps/XFormsApp/SecondPage.cs index 7349e7967..e121d36eb 100644 --- a/devApps/XFormsApp/SecondPage.cs +++ b/devApps/XFormsApp/SecondPage.cs @@ -41,7 +41,7 @@ public class SecondPage : ContentPage public string User = ""; private string 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"; diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs index 972b526b8..9771bf398 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs @@ -182,7 +182,6 @@ public Guid CorrelationId } #if iOS - private string keychainSecurityGroup; /// diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenHandlerBase.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenHandlerBase.cs index eaef88b9d..3afc03439 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/Flows/AcquireTokenHandlerBase.cs +++ b/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); } } diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/OAuth2/TokenResponse.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/OAuth2/TokenResponse.cs index 51d745e62..2c2374772 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Internal/OAuth2/TokenResponse.cs +++ b/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 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) diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs index 1c411c696..3003308b4 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs +++ b/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 AcquireTokenUsingBrokerAsync(IDictionary AcquireTokenUsingBrokerAsync(IDictionary UIApplication.SharedApplication.OpenUrl(url)); } @@ -136,6 +139,8 @@ private AdalResultWrapper ProcessBrokerResponse() { string[] keyValuePairs = brokerResponse.Query.Split('&'); + _logger.Info("Processing response from iOS Broker. "); + IDictionary responseDictionary = new Dictionary(); foreach (string pair in keyValuePairs) { @@ -156,6 +161,7 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary 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 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 r Error = AdalError.BrokerReponseHashMismatch, ErrorDescription = AdalErrorMessage.BrokerReponseHashMismatch }; + _logger.InfoPii("Broker response hash mismatch: " + response.Error, "Broker response hash mismatch. "); } } diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs index 4b84699aa..4f032e17c 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs +++ b/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) } } } -} +} \ No newline at end of file diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs index c0754493e..2da8bf3bd 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs +++ b/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 { diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs index 318471e00..572f2119b 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs +++ b/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(); } } -} +} \ No newline at end of file