Add more details to error message. (#3935)

* Add more details to error message. * Update src/client/Microsoft.Identity.Client/MsalErrorMessage.cs Co-authored-by: Gladwin Johnson <[email protected]> * Update src/client/Microsoft.Identity.Client/MsalErrorMessage.cs Co-authored-by: Gladwin Johnson <[email protected]> * Update src/client/Microsoft.Identity.Client/MsalErrorMessage.cs Co-authored-by: Bogdan Gavril <[email protected]> * Add more logging and check for null response after parsing. * Add unit test * Address comments --------- Co-authored-by: Gladwin Johnson <[email protected]> Co-authored-by: Bogdan Gavril <[email protected]>
AzureAD · Feb 8, 2023 · 16b744a · 16b744a
1 parent 4f98cf1
commit 16b744a
Show file tree

Hide file tree

Showing 6 changed files with 89 additions and 8 deletions.
diff --git a/src/client/Microsoft.Identity.Client/LogMessages.cs b/src/client/Microsoft.Identity.Client/LogMessages.cs
@@ -36,6 +36,8 @@ internal static class LogMessages
         public const string UserCancelledAuthentication = "Authorization result status returned user cancelled authentication. ";
         public const string AuthorizationResultWasNotSuccessful = "Authorization result was not successful. See error message for more details. ";
 
+        public const string WsTrustRequestFailed = "Ws-Trust request failed. See error message for more details.";
+
         public static string ErrorReturnedInBrokerResponse(string error)
         {
             return string.Format(CultureInfo.InvariantCulture, "Error {0} returned in broker response. ", error);

diff --git a/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs b/src/client/Microsoft.Identity.Client/MsalErrorMessage.cs
@@ -197,6 +197,10 @@ public static string iOSBrokerKeySaveFailed(string keyChainResult)
         public const string FederatedServiceReturnedErrorTemplate = "Federated service at {0} returned error: {1} ";
         public const string ParsingWsTrustResponseFailedErrorTemplate = "Federated service at {0} parse error: Body {1} ";
         public const string UnknownUserType = "Unknown User Type";
+        public const string ParsingWsTrustResponseFailedDueToConfiguration = "There was an error parsing the WS-Trust response from the endpoint. " +
+            "\nThis may occur if there are issues with your ADFS configuration. See https://aka.ms/msal-net-iwa-troubleshooting for more details." +
+            "\nEnable logging to see more details. See https://aka.ms/msal-net-logging.";
+
 
         public const string InternalErrorCacheEmptyUsername =
             "Internal error - trying to remove an MSAL user with an empty username. Possible cache corruption. See https://aka.ms/adal_token_cache_serialization. ";

diff --git a/src/client/Microsoft.Identity.Client/WsTrust/CommonNonInteractiveHandler.cs b/src/client/Microsoft.Identity.Client/WsTrust/CommonNonInteractiveHandler.cs
@@ -133,12 +133,12 @@ internal async Task<WsTrustResponse> GetWsTrustResponseAsync(
                 _requestContext.Logger.Info($"Token of type '{wsTrustResponse.TokenType}' acquired from WS-Trust endpoint. ");
                 return wsTrustResponse;
             }
-            catch (Exception ex)
+            catch (Exception ex) when (ex is not MsalClientException)
             {
                 throw new MsalClientException(
                     MsalError.ParsingWsTrustResponseFailed,
-                    "There was an error parsing WS-Trust response from the endpoint. This may occur if there is an issue with your ADFS configuration." 
-                    + " See https://aka.ms/msal-net-iwa-troubleshooting for more details. Error Message: " + ex.Message,
+                    MsalErrorMessage.ParsingWsTrustResponseFailedDueToConfiguration +
+                    " Error Message: " + ex.Message,
                     ex);
             }
         }

diff --git a/src/client/Microsoft.Identity.Client/WsTrust/WsTrustWebRequestManager.cs b/src/client/Microsoft.Identity.Client/WsTrust/WsTrustWebRequestManager.cs
@@ -96,19 +96,22 @@ public async Task<WsTrustResponse> GetWsTrustResponseAsync(
                 headers, 
                 body, 
                 requestContext.Logger, 
-                cancellationToken: requestContext.UserCancellationToken).ConfigureAwait(false);
-
+                cancellationToken: requestContext.UserCancellationToken).ConfigureAwait(false);
+
             if (resp.StatusCode != System.Net.HttpStatusCode.OK)
             {
                 string errorMessage = null;
                 try
                 {
-                    errorMessage = WsTrustResponse.ReadErrorResponse(XDocument.Parse(resp.Body, LoadOptions.None), requestContext);
+                    errorMessage = WsTrustResponse.ReadErrorResponse(XDocument.Parse(resp.Body, LoadOptions.None), requestContext);
                 }
                 catch (System.Xml.XmlException)
                 {
                     errorMessage = resp.Body;
                 }
+
+                requestContext.Logger.ErrorPii(LogMessages.WsTrustRequestFailed + $"Status code: {resp.StatusCode} \nError message: {errorMessage}", 
+                    LogMessages.WsTrustRequestFailed + $"Status code: {resp.StatusCode}");
 
                 string message = string.Format(
                         CultureInfo.CurrentCulture,
@@ -123,8 +126,16 @@ public async Task<WsTrustResponse> GetWsTrustResponseAsync(
             }
 
             try
-            {
-                return WsTrustResponse.CreateFromResponse(resp.Body, wsTrustEndpoint.Version);
+            {
+                var wsTrustResponse = WsTrustResponse.CreateFromResponse(resp.Body, wsTrustEndpoint.Version);
+
+                if  (wsTrustResponse == null)
+                {
+                    requestContext.Logger.ErrorPii("Token not found in the ws trust response. See response for more details: \n" + resp.Body, "Token not found in WS-Trust response.");
+                    throw new MsalClientException(MsalError.ParsingWsTrustResponseFailed, MsalErrorMessage.ParsingWsTrustResponseFailedDueToConfiguration);
+                }
+
+                return wsTrustResponse;
             }
             catch (System.Xml.XmlException ex)
             {

diff --git a/tests/Microsoft.Identity.Test.Unit/CoreTests/WsTrustTests/WsTrustTests.cs b/tests/Microsoft.Identity.Test.Unit/CoreTests/WsTrustTests/WsTrustTests.cs
@@ -14,11 +14,14 @@
 using System.Globalization;
 using Microsoft.Identity.Client.Internal;
 using System.Linq;
+using Microsoft.Identity.Test.Common.Core.Helpers;
+using NSubstitute.Extensions;
 
 namespace Microsoft.Identity.Test.Unit.CoreTests.WsTrustTests
 {
     [TestClass]
     [DeploymentItem(@"Resources\WsTrustResponse13.xml")]
+    [DeploymentItem(@"Resources\WsTrustResponseNoToken.xml")]
     public class WsTrustTests : TestBase
     {
         [TestMethod]
@@ -129,5 +132,38 @@ public async Task WsTrustRequestParseErrorTestAsync()
                 }
             }
         }
+
+        [TestMethod]
+        [Description("WsTrustRequest encounters a response with no token from the wsTrust endpoint")]
+        public async Task WsTrustRequestTokenNotFoundInResponseTestAsync()
+        {
+            const string uri = "https://some/address/usernamemixed";
+
+            var endpoint = new WsTrustEndpoint(new Uri(uri), WsTrustVersion.WsTrust2005);
+
+            using (var harness = CreateTestHarness())
+            {
+                harness.HttpManager.AddMockHandler(
+                    new MockHttpMessageHandler()
+                    {
+                        ExpectedUrl = uri,
+                        ExpectedMethod = HttpMethod.Post,
+                        ResponseMessage = new HttpResponseMessage(HttpStatusCode.OK)
+                        {
+                            Content = new StringContent(
+                               File.ReadAllText(ResourceHelper.GetTestResourceRelativePath("WsTrustResponseNoToken.xml")))
+                        }
+                    });
+
+                var requestContext = new RequestContext(harness.ServiceBundle, Guid.NewGuid());
+
+                var message = endpoint.BuildTokenRequestMessageWindowsIntegratedAuth("urn:federation:SomeAudience");
+
+                var ex = await AssertException.TaskThrowsAsync<MsalClientException>(() =>
+                    harness.ServiceBundle.WsTrustWebRequestManager.GetWsTrustResponseAsync(endpoint, message, requestContext)).ConfigureAwait(false);
+
+                Assert.AreEqual(MsalError.ParsingWsTrustResponseFailed, ex.ErrorCode);
+            }
+        }
     }
 }
diff --git a/tests/Microsoft.Identity.Test.Unit/Resources/WsTrustResponseNoToken.xml b/tests/Microsoft.Identity.Test.Unit/Resources/WsTrustResponseNoToken.xml
@@ -0,0 +1,28 @@
+<s:Envelope xmlns:s="http://www.w3.org/2003/05/soap-envelope" xmlns:a="http://www.w3.org/2005/08/addressing" xmlns:u="http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-utility-1.0.xsd">
+  <s:Header>
+    <a:Action s:mustUnderstand="1">http://www.w3.org/2005/08/addressing/soap/fault</a:Action>
+    <a:RelatesTo>urn:uuid:a4444a44-aaaa-4444-4444-aaaaaaaaaa</a:RelatesTo>
+    <o:Security s:mustUnderstand="1" xmlns:o="http://docs.oasis-open.org/wss/2004/01/oasis-200401-wss-wssecurity-secext-1.0.xsd">
+      <u:Timestamp u:Id="_0">
+        <u:Created>2022-10-18T13:54:59.751Z</u:Created>
+        <u:Expires>2022-10-18T13:59:59.751Z</u:Expires>
+      </u:Timestamp>
+    </o:Security>
+  </s:Header>
+  <s:Body>
+    <s:Fault>
+      <s:Code>
+        <s:Value>s:Sender</s:Value>
+        <s:Subcode>
+          <s:Value xmlns:a="http://schemas.xmlsoap.org/ws/2005/02/trust">a:FailedAuthentication</s:Value>
+        </s:Subcode>
+      </s:Code>
+      <s:Reason>
+        <s:Text xml:lang="en-US">MSIS7068: Access denied.</s:Text>
+      </s:Reason>
+      <s:Detail>
+
+      </s:Detail>
+    </s:Fault>
+  </s:Body>
+</s:Envelope>