From 368c461db346b3d3cdd00a6e137b33d7c2583fc6 Mon Sep 17 00:00:00 2001 From: jennyf19 Date: Wed, 9 Oct 2019 08:03:02 -0700 Subject: [PATCH] Fix issue w/iOS13 broker and nonce mismatch (#1667) * update msbuild sdk extras update ADAL broker error message update logging for nonce * add check for v3 broker * add return ok --- changelog.txt | 2 +- global.json | 2 +- .../Exceptions/AdalErrorMessage.cs | 2 +- .../Platforms/iOS/iOSBroker.cs | 26 ++++++++++++------- 4 files changed, 20 insertions(+), 12 deletions(-) diff --git a/changelog.txt b/changelog.txt index 7b03123c2..3471a75b6 100644 --- a/changelog.txt +++ b/changelog.txt @@ -1,7 +1,7 @@ Version 5.2.2 ============== Bug Fixes: -- **Ensures that ADAL.NET works fine with brokers on iOS 13**. On iOS 13, iOS, the broker, may or may not return the source application, which is used by ADAL.NET to verify the response is coming from broker. To maintain secure calls, MSAL.NET will now also create a nonce to send in the broker request and will verify the same nonce is returned in the broker response in the case of a missing source application. [Issue](https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/1357) +- **Ensures that ADAL.NET works with brokers on iOS 13**. On iOS 13, the iOS broker, may or may not return the source application, which will be used by ADAL.NET to verify that the response is coming from the iOS broker. To maintain secure calls, ADAL.NET will now also create a nonce to send in the broker request and will verify the same nonce is returned in the broker response in the case of a missing source application. [Issue](https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/issues/1642) Version 5.2.1 ============== diff --git a/global.json b/global.json index a75963ff5..a5b8bc95a 100644 --- a/global.json +++ b/global.json @@ -1,5 +1,5 @@ { "msbuild-sdks": { - "MSBuild.Sdk.Extras": "1.6.61" + "MSBuild.Sdk.Extras": "2.0.41" } } \ No newline at end of file diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs index 9e96362d0..d775845a3 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Exceptions/AdalErrorMessage.cs @@ -89,7 +89,7 @@ internal static class AdalErrorMessage public const string RedirectUriContainsFragment = "'redirectUri' must NOT include a fragment component"; public const string ServiceReturnedError = "Service returned error. Check InnerException for more details"; public const string BrokerReponseHashMismatch = "Unencrypted broker response hash did not match the expected hash"; - public const string BrokerNonceMismatch = "Broker response nonce does not match the request nonce sent by MSAL.NET." + + public const string BrokerNonceMismatch = "Broker response nonce does not match the request nonce sent by ADAL.NET." + "Please see https://aka.ms/adal-net-ios-13-broker for more details. "; public const string StsMetadataRequestFailed = diff --git a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs index 57155679a..f2607c9f9 100644 --- a/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs +++ b/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/iOS/iOSBroker.cs @@ -39,6 +39,7 @@ using Microsoft.Identity.Core.Cache; using Microsoft.Identity.Core.Helpers; using Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Broker; +using System.Globalization; namespace Microsoft.IdentityModel.Clients.ActiveDirectory.Internal.Platform { @@ -131,7 +132,6 @@ public async Task AcquireTokenUsingBrokerAsync(IDictionary r { response = new TokenResponse { - Error = AdalError.BrokerReponseHashMismatch, - ErrorDescription = AdalErrorMessage.BrokerReponseHashMismatch + Error = AdalError.BrokerNonceMismatch, + ErrorDescription = AdalErrorMessage.BrokerNonceMismatch }; } } @@ -237,15 +237,23 @@ private AdalResultWrapper ResultFromBrokerResponse(IDictionary r private bool ValidateBrokerResponseNonceWithRequestNonce(IDictionary brokerResponseDictionary) { - if (!string.IsNullOrEmpty(_brokerRequestNonce)) + if (_brokerV3Installed) { - string brokerResponseNonce = brokerResponseDictionary.ContainsKey(BrokerParameter.BrokerNonce) - ? brokerResponseDictionary[BrokerParameter.BrokerNonce] - : null; + string brokerResponseNonce = brokerResponseDictionary[BrokerParameter.BrokerNonce]; + bool ok = string.Equals(brokerResponseNonce, _brokerRequestNonce, StringComparison.InvariantCultureIgnoreCase); - return string.Equals(brokerResponseNonce, _brokerRequestNonce); + if (!ok) + { + _logger.Error( + string.Format( + CultureInfo.CurrentCulture, + "Nonce check failed! Broker response nonce is: {0}, \nBroker request nonce is: {1}", + brokerResponseNonce, + _brokerRequestNonce)); + } + return ok; } - return false; + return true; } public static void SetBrokerResponse(NSUrl responseUrl)