From 5bcf881b66dfae4429f1eaad68de2752aca6a049 Mon Sep 17 00:00:00 2001 From: Jenny Ferries Date: Thu, 6 Jun 2019 19:00:32 -0700 Subject: [PATCH 1/3] bug fixes add logging fix teamId default fix keychain w/legacy cache persistance --- devApps/XFormsApp.iOS/Entitlements.plist | 7 ++- devApps/XFormsApp/AppConstants.cs | 2 +- devApps/XFormsApp/SecondPage.cs | 2 +- .../Internal/Flows/AcquireTokenHandlerBase.cs | 2 +- .../Internal/OAuth2/TokenResponse.cs | 45 ++++++++++++------- .../Platforms/iOS/iOSBroker.cs | 10 ++++- .../iOS/iOSLegacyCachePersistance.cs | 28 ++++++------ .../Platforms/iOS/iOSTokenCacheAccessor.cs | 18 +++----- 8 files changed, 66 insertions(+), 48 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/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..acccc3b71 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs @@ -36,22 +36,18 @@ internal class iOSLegacyCachePersistence : ILegacyCachePersistence const string NAME = "ADAL.PCL.iOS"; private const string LocalSettingsContainerName = "ActiveDirectoryAuthenticationLibrary"; - private string keychainGroup; - - private string GetBundleId() - { - return NSBundle.MainBundle.BundleIdentifier; - } + private string keychainGroupLegacy; public void SetKeychainSecurityGroup(string keychainSecurityGroup) { - if (keychainSecurityGroup == null) + iOSTokenCacheAccessor iOSTokenCacheAccessor = new iOSTokenCacheAccessor(); + if (string.IsNullOrEmpty(keychainSecurityGroup)) { - keychainGroup = GetBundleId(); + keychainGroupLegacy = iOSTokenCacheAccessor.GetTeamId() + '.' + iOSTokenCacheAccessor.DefaultKeychainGroup; } else { - keychainGroup = keychainSecurityGroup; + keychainGroupLegacy = iOSTokenCacheAccessor.GetTeamId() + '.' + keychainSecurityGroup; } } @@ -71,9 +67,9 @@ byte[] ILegacyCachePersistence.LoadCache() Description = "Storage for cache" }; - if (keychainGroup != null) + if (keychainGroupLegacy != null) { - rec.AccessGroup = keychainGroup; + rec.AccessGroup = keychainGroupLegacy; } var match = SecKeyChain.QueryAsRecord(rec, out res); @@ -106,9 +102,9 @@ void ILegacyCachePersistence.WriteCache(byte[] serializedCache) Description = "Storage for cache" }; - if (keychainGroup != null) + if (keychainGroupLegacy != null) { - s.AccessGroup = keychainGroup; + s.AccessGroup = keychainGroupLegacy; } var err = SecKeyChain.Remove(s); @@ -127,6 +123,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 +135,4 @@ void ILegacyCachePersistence.WriteCache(byte[] serializedCache) } } } -} +} \ No newline at end of file diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs index 318471e00..5d7c9e4ce 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs @@ -59,12 +59,11 @@ enum CredentialAttrType private const bool _defaultSyncSetting = false; private const SecAccessible _defaultAccessiblityPolicy = SecAccessible.AfterFirstUnlockThisDeviceOnly; - private const string DefaultKeychainGroup = "com.microsoft.adalcache"; + internal const string DefaultKeychainGroup = "com.microsoft.adalcache"; // Identifier for the keychain item used to retrieve current team ID private const string TeamIdKey = "DotNetTeamIDHint"; private string keychainGroup; - private RequestContext _requestContext; private string GetBundleId() { @@ -73,9 +72,9 @@ private string GetBundleId() public void SetiOSKeychainSecurityGroup(string keychainSecurityGroup) { - if (keychainSecurityGroup == null) + if (string.IsNullOrEmpty(keychainSecurityGroup)) { - keychainGroup = GetBundleId(); + keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup; } else { @@ -96,7 +95,7 @@ public void SetKeychainSecurityGroup(string keychainSecurityGroup) } } - private string GetTeamId() + internal string GetTeamId() { var queryRecord = new SecRecord(SecKind.GenericPassword) { @@ -125,14 +124,9 @@ private string GetTeamId() public iOSTokenCacheAccessor() { - keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup; - } - - public iOSTokenCacheAccessor(RequestContext requestContext) : this() - { - _requestContext = requestContext; + SetiOSKeychainSecurityGroup(null); } - + public void SaveAccessToken(MsalAccessTokenCacheItem item) { var key = item.GetKey(); From 0edda19e28ef867fe6e41e0a5f79855b8eed9c14 Mon Sep 17 00:00:00 2001 From: Jenny Ferries Date: Fri, 7 Jun 2019 15:14:09 -0700 Subject: [PATCH 2/3] combine iosTokenCacheAccessor and iOSlegacy stuff --- .../AuthenticationContext.cs | 3 - .../iOS/iOSLegacyCachePersistance.cs | 138 ------------------ .../Platforms/iOS/iOSPlatformProxy.cs | 5 +- .../Platforms/iOS/iOSTokenCacheAccessor.cs | 128 +++++++++++++--- 4 files changed, 108 insertions(+), 166 deletions(-) delete mode 100644 src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs index 972b526b8..939c58b06 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; /// @@ -201,7 +200,6 @@ public string iOSKeychainSecurityGroup set { keychainSecurityGroup = value; - StorageDelegates.LegacyCachePersistence.SetKeychainSecurityGroup(value); TokenCache.TokenCacheAccessor.SetiOSKeychainSecurityGroup(value); } } @@ -223,7 +221,6 @@ public string KeychainSecurityGroup set { keychainSecurityGroup = value; - StorageDelegates.LegacyCachePersistence.SetKeychainSecurityGroup(value); TokenCache.TokenCacheAccessor.SetKeychainSecurityGroup(value); } } diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs deleted file mode 100644 index acccc3b71..000000000 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs +++ /dev/null @@ -1,138 +0,0 @@ -//---------------------------------------------------------------------- -// -// Copyright (c) Microsoft Corporation. -// All rights reserved. -// -// This code is licensed under the MIT License. -// -// Permission is hereby granted, free of charge, to any person obtaining a copy -// of this software and associated documentation files(the "Software"), to deal -// in the Software without restriction, including without limitation the rights -// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell -// copies of the Software, and to permit persons to whom the Software is -// furnished to do so, subject to the following conditions : -// -// The above copyright notice and this permission notice shall be included in -// all copies or substantial portions of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE -// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -// THE SOFTWARE. -// -//------------------------------------------------------------------------------ - -using Foundation; -using Security; -using System; - -namespace Microsoft.Identity.Core.Cache -{ - internal class iOSLegacyCachePersistence : ILegacyCachePersistence - { - const string NAME = "ADAL.PCL.iOS"; - private const string LocalSettingsContainerName = "ActiveDirectoryAuthenticationLibrary"; - - private string keychainGroupLegacy; - - public void SetKeychainSecurityGroup(string keychainSecurityGroup) - { - iOSTokenCacheAccessor iOSTokenCacheAccessor = new iOSTokenCacheAccessor(); - if (string.IsNullOrEmpty(keychainSecurityGroup)) - { - keychainGroupLegacy = iOSTokenCacheAccessor.GetTeamId() + '.' + iOSTokenCacheAccessor.DefaultKeychainGroup; - } - else - { - keychainGroupLegacy = iOSTokenCacheAccessor.GetTeamId() + '.' + keychainSecurityGroup; - } - } - - byte[] ILegacyCachePersistence.LoadCache() - { - try - { - SecStatusCode res; - var rec = new SecRecord(SecKind.GenericPassword) - { - Generic = NSData.FromString(LocalSettingsContainerName), - Accessible = SecAccessible.Always, - Service = NAME + " Service", - Account = NAME + " cache", - Label = NAME + " Label", - Comment = NAME + " Cache", - Description = "Storage for cache" - }; - - if (keychainGroupLegacy != null) - { - rec.AccessGroup = keychainGroupLegacy; - } - - var match = SecKeyChain.QueryAsRecord(rec, out res); - if (res == SecStatusCode.Success && match != null && match.ValueData != null) - { - return match.ValueData.ToArray(); - - } - } - catch (Exception ex) - { - CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to load adal cache: "); - // Ignore as the cache seems to be corrupt - } - return null; - } - - void ILegacyCachePersistence.WriteCache(byte[] serializedCache) - { - try - { - var s = new SecRecord(SecKind.GenericPassword) - { - Generic = NSData.FromString(LocalSettingsContainerName), - Accessible = SecAccessible.Always, - Service = NAME + " Service", - Account = NAME + " cache", - Label = NAME + " Label", - Comment = NAME + " Cache", - Description = "Storage for cache" - }; - - if (keychainGroupLegacy != null) - { - s.AccessGroup = keychainGroupLegacy; - } - - var err = SecKeyChain.Remove(s); - if (err != SecStatusCode.Success) - { - string msg = "Failed to remove adal cache record: "; - CoreLoggerBase.Default.WarningPii(msg + err, msg); - } - - if (serializedCache != null && serializedCache.Length > 0) - { - s.ValueData = NSData.FromArray(serializedCache); - err = SecKeyChain.Add(s); - if (err != SecStatusCode.Success) - { - 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) - { - CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to save adal cache: "); - } - } - } -} \ 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..c5f5479be 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 { @@ -127,12 +126,12 @@ public string GetDeviceId() public ILegacyCachePersistence CreateLegacyCachePersistence() { - return new iOSLegacyCachePersistence(); + return new iOSTokenCacheAccessor(); } public ITokenCacheAccessor CreateTokenCacheAccessor() { - return new iOSTokenCacheAccessor(); + return new iOSTokenCacheAccessor() as ITokenCacheAccessor; } /// diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs index 5d7c9e4ce..88023556c 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs @@ -36,9 +36,24 @@ namespace Microsoft.Identity.Core { - internal class iOSTokenCacheAccessor : ITokenCacheAccessor + internal class iOSTokenCacheAccessor : ITokenCacheAccessor, ILegacyCachePersistence { public const string CacheKeyDelimiter = "-"; + private const bool _defaultSyncSetting = false; + private const SecAccessible _defaultAccessiblityPolicy = SecAccessible.AfterFirstUnlockThisDeviceOnly; + + private const string DefaultKeychainGroup = "com.microsoft.adalcache"; + // Identifier for the keychain item used to retrieve current team ID + private const string TeamIdKey = "DotNetTeamIDHint"; + const string NAME = "ADAL.PCL.iOS"; + private const string LocalSettingsContainerName = "ActiveDirectoryAuthenticationLibrary"; + + private string keychainGroup; + + public iOSTokenCacheAccessor() + { + SetiOSKeychainSecurityGroup(null); + } static Dictionary AuthorityTypeToAttrType = new Dictionary() { @@ -56,20 +71,6 @@ enum CredentialAttrType Password = 2004 } - private const bool _defaultSyncSetting = false; - private const SecAccessible _defaultAccessiblityPolicy = SecAccessible.AfterFirstUnlockThisDeviceOnly; - - internal const string DefaultKeychainGroup = "com.microsoft.adalcache"; - // Identifier for the keychain item used to retrieve current team ID - private const string TeamIdKey = "DotNetTeamIDHint"; - - private string keychainGroup; - - private string GetBundleId() - { - return NSBundle.MainBundle.BundleIdentifier; - } - public void SetiOSKeychainSecurityGroup(string keychainSecurityGroup) { if (string.IsNullOrEmpty(keychainSecurityGroup)) @@ -94,8 +95,12 @@ public void SetKeychainSecurityGroup(string keychainSecurityGroup) keychainGroup = keychainSecurityGroup; } } + private string GetBundleId() + { + return NSBundle.MainBundle.BundleIdentifier; + } - internal string GetTeamId() + private string GetTeamId() { var queryRecord = new SecRecord(SecKind.GenericPassword) { @@ -122,11 +127,6 @@ internal string GetTeamId() CoreErrorMessages.CannotAccessPublisherKeyChain); } - public iOSTokenCacheAccessor() - { - SetiOSKeychainSecurityGroup(null); - } - public void SaveAccessToken(MsalAccessTokenCacheItem item) { var key = item.GetKey(); @@ -325,5 +325,89 @@ public void ClearAccessTokens() { throw new NotImplementedException(); } + + byte[] ILegacyCachePersistence.LoadCache() + { + try + { + SecStatusCode res; + var rec = new SecRecord(SecKind.GenericPassword) + { + Generic = NSData.FromString(LocalSettingsContainerName), + Accessible = SecAccessible.Always, + Service = NAME + " Service", + Account = NAME + " cache", + Label = NAME + " Label", + Comment = NAME + " Cache", + Description = "Storage for cache" + }; + + if (keychainGroup != null) + { + rec.AccessGroup = keychainGroup; + } + + var match = SecKeyChain.QueryAsRecord(rec, out res); + if (res == SecStatusCode.Success && match != null && match.ValueData != null) + { + return match.ValueData.ToArray(); + + } + } + catch (Exception ex) + { + CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to load adal cache: "); + // Ignore as the cache seems to be corrupt + } + return null; + } + + void ILegacyCachePersistence.WriteCache(byte[] serializedCache) + { + try + { + var s = new SecRecord(SecKind.GenericPassword) + { + Generic = NSData.FromString(LocalSettingsContainerName), + Accessible = SecAccessible.Always, + Service = NAME + " Service", + Account = NAME + " cache", + Label = NAME + " Label", + Comment = NAME + " Cache", + Description = "Storage for cache" + }; + + if (keychainGroup != null) + { + s.AccessGroup = keychainGroup; + } + + var err = SecKeyChain.Remove(s); + if (err != SecStatusCode.Success) + { + string msg = "Failed to remove adal cache record: "; + CoreLoggerBase.Default.WarningPii(msg + err, msg); + } + + if (serializedCache != null && serializedCache.Length > 0) + { + s.ValueData = NSData.FromArray(serializedCache); + err = SecKeyChain.Add(s); + if (err != SecStatusCode.Success) + { + 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) + { + CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to save adal cache: "); + } + } } -} +} \ No newline at end of file From b1bd7b5962cf7646a9ef2a55b68a891fd2644ebe Mon Sep 17 00:00:00 2001 From: Jenny Ferries Date: Mon, 10 Jun 2019 16:49:32 -0700 Subject: [PATCH 3/3] reverting changes --- .../AuthenticationContext.cs | 2 + .../iOS/iOSLegacyCachePersistance.cs | 142 ++++++++++++++++++ .../Platforms/iOS/iOSPlatformProxy.cs | 4 +- .../Platforms/iOS/iOSTokenCacheAccessor.cs | 132 ++++------------ 4 files changed, 173 insertions(+), 107 deletions(-) create mode 100644 src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs index 939c58b06..9771bf398 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/AuthenticationContext.cs @@ -200,6 +200,7 @@ public string iOSKeychainSecurityGroup set { keychainSecurityGroup = value; + StorageDelegates.LegacyCachePersistence.SetKeychainSecurityGroup(value); TokenCache.TokenCacheAccessor.SetiOSKeychainSecurityGroup(value); } } @@ -221,6 +222,7 @@ public string KeychainSecurityGroup set { keychainSecurityGroup = value; + StorageDelegates.LegacyCachePersistence.SetKeychainSecurityGroup(value); TokenCache.TokenCacheAccessor.SetKeychainSecurityGroup(value); } } diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs new file mode 100644 index 000000000..4f032e17c --- /dev/null +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSLegacyCachePersistance.cs @@ -0,0 +1,142 @@ +//---------------------------------------------------------------------- +// +// Copyright (c) Microsoft Corporation. +// All rights reserved. +// +// This code is licensed under the MIT License. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files(the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and / or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions : +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. +// +//------------------------------------------------------------------------------ + +using Foundation; +using Security; +using System; + +namespace Microsoft.Identity.Core.Cache +{ + internal class iOSLegacyCachePersistence : ILegacyCachePersistence + { + const string NAME = "ADAL.PCL.iOS"; + private const string LocalSettingsContainerName = "ActiveDirectoryAuthenticationLibrary"; + + private string keychainGroup; + + private string GetBundleId() + { + return NSBundle.MainBundle.BundleIdentifier; + } + + public void SetKeychainSecurityGroup(string keychainSecurityGroup) + { + if (keychainSecurityGroup == null) + { + keychainGroup = GetBundleId(); + } + else + { + keychainGroup = keychainSecurityGroup; + } + } + + byte[] ILegacyCachePersistence.LoadCache() + { + try + { + SecStatusCode res; + var rec = new SecRecord(SecKind.GenericPassword) + { + Generic = NSData.FromString(LocalSettingsContainerName), + Accessible = SecAccessible.Always, + Service = NAME + " Service", + Account = NAME + " cache", + Label = NAME + " Label", + Comment = NAME + " Cache", + Description = "Storage for cache" + }; + + if (keychainGroup != null) + { + rec.AccessGroup = keychainGroup; + } + + var match = SecKeyChain.QueryAsRecord(rec, out res); + if (res == SecStatusCode.Success && match != null && match.ValueData != null) + { + return match.ValueData.ToArray(); + + } + } + catch (Exception ex) + { + CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to load adal cache: "); + // Ignore as the cache seems to be corrupt + } + return null; + } + + void ILegacyCachePersistence.WriteCache(byte[] serializedCache) + { + try + { + var s = new SecRecord(SecKind.GenericPassword) + { + Generic = NSData.FromString(LocalSettingsContainerName), + Accessible = SecAccessible.Always, + Service = NAME + " Service", + Account = NAME + " cache", + Label = NAME + " Label", + Comment = NAME + " Cache", + Description = "Storage for cache" + }; + + if (keychainGroup != null) + { + s.AccessGroup = keychainGroup; + } + + var err = SecKeyChain.Remove(s); + if (err != SecStatusCode.Success) + { + string msg = "Failed to remove adal cache record: "; + CoreLoggerBase.Default.WarningPii(msg + err, msg); + } + + if (serializedCache != null && serializedCache.Length > 0) + { + s.ValueData = NSData.FromArray(serializedCache); + err = SecKeyChain.Add(s); + if (err != SecStatusCode.Success) + { + 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) + { + CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to save adal cache: "); + } + } + } +} \ 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 c5f5479be..2da8bf3bd 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSPlatformProxy.cs @@ -126,12 +126,12 @@ public string GetDeviceId() public ILegacyCachePersistence CreateLegacyCachePersistence() { - return new iOSTokenCacheAccessor(); + return new iOSLegacyCachePersistence(); } public ITokenCacheAccessor CreateTokenCacheAccessor() { - return new iOSTokenCacheAccessor() as ITokenCacheAccessor; + return new iOSTokenCacheAccessor(); } /// diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs index 88023556c..572f2119b 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSTokenCacheAccessor.cs @@ -36,46 +36,56 @@ namespace Microsoft.Identity.Core { - internal class iOSTokenCacheAccessor : ITokenCacheAccessor, ILegacyCachePersistence + internal class iOSTokenCacheAccessor : ITokenCacheAccessor { public const string CacheKeyDelimiter = "-"; + + static Dictionary AuthorityTypeToAttrType = new Dictionary() + { + {AuthorityType.AAD.ToString(), 1001}, + {AuthorityType.MSA.ToString(), 1002}, + {AuthorityType.MSSTS.ToString(), 1003}, + {AuthorityType.OTHER.ToString(), 1004}, + }; + + enum CredentialAttrType + { + AccessToken = 2001, + RefreshToken = 2002, + IdToken = 2003, + Password = 2004 + } + private const bool _defaultSyncSetting = false; private const SecAccessible _defaultAccessiblityPolicy = SecAccessible.AfterFirstUnlockThisDeviceOnly; private const string DefaultKeychainGroup = "com.microsoft.adalcache"; // Identifier for the keychain item used to retrieve current team ID private const string TeamIdKey = "DotNetTeamIDHint"; - const string NAME = "ADAL.PCL.iOS"; - private const string LocalSettingsContainerName = "ActiveDirectoryAuthenticationLibrary"; + private RequestContext _requestContext; private string keychainGroup; public iOSTokenCacheAccessor() { - SetiOSKeychainSecurityGroup(null); + keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup; } - static Dictionary AuthorityTypeToAttrType = new Dictionary() + public iOSTokenCacheAccessor(RequestContext requestContext) : this() { - {AuthorityType.AAD.ToString(), 1001}, - {AuthorityType.MSA.ToString(), 1002}, - {AuthorityType.MSSTS.ToString(), 1003}, - {AuthorityType.OTHER.ToString(), 1004}, - }; + _requestContext = requestContext; + } - enum CredentialAttrType + private string GetBundleId() { - AccessToken = 2001, - RefreshToken = 2002, - IdToken = 2003, - Password = 2004 + return NSBundle.MainBundle.BundleIdentifier; } public void SetiOSKeychainSecurityGroup(string keychainSecurityGroup) { - if (string.IsNullOrEmpty(keychainSecurityGroup)) + if (keychainSecurityGroup == null) { - keychainGroup = GetTeamId() + '.' + DefaultKeychainGroup; + keychainGroup = GetBundleId(); } else { @@ -95,10 +105,6 @@ public void SetKeychainSecurityGroup(string keychainSecurityGroup) keychainGroup = keychainSecurityGroup; } } - private string GetBundleId() - { - return NSBundle.MainBundle.BundleIdentifier; - } private string GetTeamId() { @@ -325,89 +331,5 @@ public void ClearAccessTokens() { throw new NotImplementedException(); } - - byte[] ILegacyCachePersistence.LoadCache() - { - try - { - SecStatusCode res; - var rec = new SecRecord(SecKind.GenericPassword) - { - Generic = NSData.FromString(LocalSettingsContainerName), - Accessible = SecAccessible.Always, - Service = NAME + " Service", - Account = NAME + " cache", - Label = NAME + " Label", - Comment = NAME + " Cache", - Description = "Storage for cache" - }; - - if (keychainGroup != null) - { - rec.AccessGroup = keychainGroup; - } - - var match = SecKeyChain.QueryAsRecord(rec, out res); - if (res == SecStatusCode.Success && match != null && match.ValueData != null) - { - return match.ValueData.ToArray(); - - } - } - catch (Exception ex) - { - CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to load adal cache: "); - // Ignore as the cache seems to be corrupt - } - return null; - } - - void ILegacyCachePersistence.WriteCache(byte[] serializedCache) - { - try - { - var s = new SecRecord(SecKind.GenericPassword) - { - Generic = NSData.FromString(LocalSettingsContainerName), - Accessible = SecAccessible.Always, - Service = NAME + " Service", - Account = NAME + " cache", - Label = NAME + " Label", - Comment = NAME + " Cache", - Description = "Storage for cache" - }; - - if (keychainGroup != null) - { - s.AccessGroup = keychainGroup; - } - - var err = SecKeyChain.Remove(s); - if (err != SecStatusCode.Success) - { - string msg = "Failed to remove adal cache record: "; - CoreLoggerBase.Default.WarningPii(msg + err, msg); - } - - if (serializedCache != null && serializedCache.Length > 0) - { - s.ValueData = NSData.FromArray(serializedCache); - err = SecKeyChain.Add(s); - if (err != SecStatusCode.Success) - { - 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) - { - CoreLoggerBase.Default.WarningPiiWithPrefix(ex, "Failed to save adal cache: "); - } - } } } \ No newline at end of file