Skip to content

Commit b650d8a

Browse files
trwalketrwalke
trwalke
and
trwalke
authored
Added better error message for OIDC error (#5433)
* Added better error message for OIDC error * Refactoring * Addressing feedback * Resolving test issue --------- Co-authored-by: trwalke <[email protected]>
1 parent 3377be3 commit b650d8a

File tree

8 files changed

+104
-12
lines changed

8 files changed

+104
-12
lines changed

‎src/client/Microsoft.Identity.Client/Internal/Constants.cs‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ internal static class Constants
2828

2929
public const string CcsRoutingHintHeader = "x-anchormailbox";
3030
public const string AadThrottledErrorCode = "AADSTS50196";
31+
public const string AadAccountTypeAndResourceIncompatibleErrorCode = "AADSTS500207";
32+
public const string AadMissingScopeErrorCode = "AADSTS900144";
33+
3134
//Represents 5 minutes in Unit time stamp
3235
public const int DefaultJitterRangeInSeconds = 300;
3336
public static readonly TimeSpan AccessTokenExpirationBuffer = TimeSpan.FromMinutes(5);

‎src/client/Microsoft.Identity.Client/MsalErrorMessage.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -444,5 +444,6 @@ public static string InvalidTokenProviderResponseValue(string invalidValueName)
444444
public const string RegionRequiredForMtlsPopMessage = "Regional auto-detect failed. mTLS Proof-of-Possession requires a region to be specified, as there is no global endpoint for mTLS. See https://aka.ms/msal-net-pop for details.";
445445
public const string ForceRefreshAndTokenHasNotCompatible = "Cannot specify ForceRefresh and AccessTokenSha256ToRefresh in the same request.";
446446
public const string RequestTimeOut = "Request to the endpoint timed out.";
447+
public const string MalformedOidcAuthorityFormat = "Possible cause: When using Entra External ID, you didn't append /v2.0, for example {0}/v2.0\"";
447448
}
448449
}

‎src/client/Microsoft.Identity.Client/MsalServiceExceptionFactory.cs‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
using System;
55
using System.Collections.Generic;
6+
using System.Globalization;
67
using System.Net;
78
using System.Text;
89
using Microsoft.Identity.Client.Http;
@@ -41,7 +42,7 @@ internal static MsalServiceException FromHttpResponse(
4142
}
4243
else
4344
{
44-
errorMessageToUse = errorMessage;
45+
errorMessageToUse = errorMessage;
4546
}
4647

4748
if (oAuth2Response.Claims == null)
@@ -64,6 +65,13 @@ internal static MsalServiceException FromHttpResponse(
6465
innerException);
6566
}
6667

68+
var authorityInfo = context?.ServiceBundle.Config.Authority.AuthorityInfo;
69+
70+
if (IsOidcAuthorityError(authorityInfo, oAuth2Response?.ErrorDescription))
71+
{
72+
errorMessage += " " + string.Format(CultureInfo.InvariantCulture, MsalErrorMessage.MalformedOidcAuthorityFormat, $"{authorityInfo.CanonicalAuthority}");
73+
}
74+
6775
ex ??= new MsalServiceException(errorCode, GetErrorMessage(errorMessage, httpResponse, context), innerException);
6876

6977
SetHttpExceptionData(ex, httpResponse);
@@ -108,6 +116,16 @@ private static bool IsThrottled(OAuth2ResponseBase oAuth2Response)
108116
oAuth2Response.ErrorDescription.StartsWith(Constants.AadThrottledErrorCode);
109117
}
110118

119+
private static bool IsOidcAuthorityError(AuthorityInfo authortyInfo, string ErrorDescription)
120+
{
121+
return authortyInfo is not null &&
122+
authortyInfo.AuthorityType == AuthorityType.Generic && // Generic Oidc authority
123+
!authortyInfo.CanonicalAuthority!.AbsoluteUri.EndsWith("/v2.0") && // Does not end with /v2.0
124+
ErrorDescription != null &&
125+
(ErrorDescription.StartsWith(Constants.AadAccountTypeAndResourceIncompatibleErrorCode) || // Certain error codes are returned
126+
ErrorDescription.StartsWith(Constants.AadMissingScopeErrorCode));
127+
}
128+
111129
internal static MsalServiceException FromBrokerResponse(
112130
MsalTokenResponse msalTokenResponse,
113131
string errorMessage)

‎src/client/Microsoft.Identity.Client/OAuth2/OAuth2Client.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ private static void ThrowServerException(HttpResponse response, RequestContext r
259259
MsalServiceException exceptionToThrow;
260260
try
261261
{
262-
exceptionToThrow = ExtractErrorsFromTheResponse(response, ref shouldLogAsError);
262+
exceptionToThrow = ExtractErrorsFromTheResponse(response, ref shouldLogAsError, requestContext);
263263
}
264264
catch (JsonException) // in the rare case we get an error response we cannot deserialize
265265
{
@@ -309,7 +309,7 @@ private static void ThrowServerException(HttpResponse response, RequestContext r
309309
throw exceptionToThrow;
310310
}
311311

312-
private static MsalServiceException ExtractErrorsFromTheResponse(HttpResponse response, ref bool shouldLogAsError)
312+
private static MsalServiceException ExtractErrorsFromTheResponse(HttpResponse response, ref bool shouldLogAsError, RequestContext context = null)
313313
{
314314
// In cases where the end-point is not found (404) response.body will be empty.
315315
if (string.IsNullOrWhiteSpace(response.Body))
@@ -352,7 +352,8 @@ private static MsalServiceException ExtractErrorsFromTheResponse(HttpResponse re
352352
return MsalServiceExceptionFactory.FromHttpResponse(
353353
msalTokenResponse.Error,
354354
msalTokenResponse.ErrorDescription,
355-
response);
355+
response,
356+
context: context);
356357
}
357358

358359
private Uri AddExtraQueryParams(Uri endPoint)

‎tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHelpers.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,9 +260,10 @@ public static HttpResponseMessage CreateFailureTokenResponseMessage(
260260
string error,
261261
string subError = null,
262262
string correlationId = null,
263-
HttpStatusCode? customStatusCode = null)
263+
HttpStatusCode? customStatusCode = null,
264+
string errorCode = "AADSTS00000")
264265
{
265-
string message = "{\"error\":\"" + error + "\",\"error_description\":\"AADSTS00000: Error for test." +
266+
string message = "{\"error\":\"" + error + "\",\"error_description\":\"" + errorCode + ": Error for test." +
266267
"Trace ID: f7ec686c-9196-4220-a754-cd9197de44e9Correlation ID: " +
267268
"04bb0cae-580b-49ac-9a10-b6c3316b1eaaTimestamp: 2015-09-16 07:24:55Z\"," +
268269
"\"error_codes\":[70002,70008],\"timestamp\":\"2015-09-16 07:24:55Z\"," +

‎tests/Microsoft.Identity.Test.Common/Core/Mocks/MockHttpManagerExtensions.cs‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -87,15 +87,18 @@ public static MockHttpMessageHandler AddFailureTokenEndpointResponse(
8787
this MockHttpManager httpManager,
8888
string error,
8989
string authority = TestConstants.AuthorityCommonTenant,
90-
string correlationId = null)
90+
string correlationId = null,
91+
string AadErrorCode = "AADSTS00000",
92+
string expectedUrl = null)
9193
{
9294
var handler = new MockHttpMessageHandler()
9395
{
94-
ExpectedUrl = authority + "oauth2/v2.0/token",
96+
ExpectedUrl = expectedUrl != null? expectedUrl : $"{authority}oauth2/v2.0/token",
9597
ExpectedMethod = HttpMethod.Post,
9698
ResponseMessage = MockHelpers.CreateFailureTokenResponseMessage(
97-
error,
98-
correlationId: correlationId)
99+
error,
100+
correlationId: correlationId,
101+
errorCode: AadErrorCode)
99102
};
100103
httpManager.AddMockHandler(handler);
101104
return handler;

‎tests/Microsoft.Identity.Test.Common/TestConstants.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,8 @@ public static HashSet<string> s_scope
122122
public const string CiamAuthorityMainFormat = "https://tenant.ciamlogin.com/";
123123
public const string CiamAuthorityWithFriendlyName = "https://tenant.ciamlogin.com/tenant.onmicrosoft.com";
124124
public const string CiamAuthorityWithGuid = "https://tenant.ciamlogin.com/aaaaaaab-aaaa-aaaa-cccc-aaaaaaaaaaaa";
125+
public const string CiamCUDAuthority = "https://login.msidlabsciam.com/aaaaaaab-aaaa-aaaa-cccc-aaaaaaaaaaaa/v2.0";
126+
public const string CiamCUDAuthorityMalformed = "https://login.msidlabsciam.com/aaaaaaab-aaaa-aaaa-cccc-aaaaaaaaaaaa";
125127

126128
public const string B2CLoginGlobal = ".b2clogin.com";
127129
public const string B2CLoginUSGov = ".b2clogin.us";
@@ -540,6 +542,14 @@ public static MsalTokenResponse CreateMsalTokenResponseWithTokenSource()
540542
],
541543
""backchannel_user_code_parameter_supported"":true
542544
}";
545+
public static string GetOidcResponse(string authority = null)
546+
{
547+
if (string.IsNullOrEmpty(authority))
548+
{
549+
return GenericOidcResponse;
550+
}
551+
return GenericOidcResponse.Replace("https://demo.duendesoftware.com", authority.TrimEnd('/'));
552+
}
543553

544554
public static MsalTokenResponse CreateAadTestTokenResponse()
545555
{

‎tests/Microsoft.Identity.Test.Unit/CoreTests/InstanceTests/GenericAuthorityTests.cs‎

Lines changed: 57 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Diagnostics;
77
using System.Globalization;
88
using System.Linq;
9+
using System.Net;
910
using System.Net.Http;
1011
using System.Threading;
1112
using System.Threading.Tasks;
@@ -333,6 +334,60 @@ public async Task BadOidcResponse_ThrowsException_Async(string badOidcResponseTy
333334
}
334335
}
335336

337+
[TestMethod]
338+
public async Task Oidc_Malformed_Failure_Async()
339+
{
340+
using (var httpManager = new MockHttpManager())
341+
{
342+
string authority = TestConstants.CiamCUDAuthorityMalformed;
343+
IConfidentialClientApplication app = ConfidentialClientApplicationBuilder
344+
.Create(TestConstants.ClientId)
345+
.WithHttpManager(httpManager)
346+
.WithOidcAuthority(authority)
347+
.WithClientSecret(TestConstants.ClientSecret)
348+
.Build();
349+
350+
httpManager.AddMockHandler(
351+
CreateOidcHttpHandler($"{authority}/{Constants.WellKnownOpenIdConfigurationPath}", authority));
352+
353+
httpManager.AddFailureTokenEndpointResponse(
354+
error: "error",
355+
AadErrorCode: Constants.AadAccountTypeAndResourceIncompatibleErrorCode,
356+
expectedUrl: $"{TestConstants.CiamCUDAuthorityMalformed}/connect/token");
357+
358+
Assert.AreEqual(authority, app.Authority);
359+
var confidentailClientApp = (ConfidentialClientApplication)app;
360+
Assert.AreEqual(AuthorityType.Generic, confidentailClientApp.AuthorityInfo.AuthorityType);
361+
362+
var ex = await AssertException.TaskThrowsAsync<MsalServiceException>(() =>
363+
app.AcquireTokenForClient(new[] { "api" })
364+
.ExecuteAsync())
365+
.ConfigureAwait(false);
366+
367+
Assert.IsTrue(ex.Message.Contains(
368+
string.Format(
369+
CultureInfo.InvariantCulture,
370+
MsalErrorMessage.MalformedOidcAuthorityFormat,
371+
TestConstants.CiamCUDAuthorityMalformed)));
372+
373+
httpManager.AddFailureTokenEndpointResponse(
374+
error: "error",
375+
AadErrorCode: Constants.AadMissingScopeErrorCode,
376+
expectedUrl: $"{TestConstants.CiamCUDAuthorityMalformed}/connect/token");
377+
378+
ex = await AssertException.TaskThrowsAsync<MsalServiceException>(() =>
379+
app.AcquireTokenForClient(new[] { "api" })
380+
.ExecuteAsync())
381+
.ConfigureAwait(false);
382+
383+
Assert.IsTrue(ex.Message.Contains(
384+
string.Format(
385+
CultureInfo.InvariantCulture,
386+
MsalErrorMessage.MalformedOidcAuthorityFormat,
387+
TestConstants.CiamCUDAuthorityMalformed)));
388+
}
389+
}
390+
336391
[TestMethod]
337392
public async Task OidcIssuerValidation_AcceptsDifferentPath_Async()
338393
{
@@ -479,13 +534,13 @@ private static string CreateIdToken()
479534
return string.Format(CultureInfo.InvariantCulture, "someheader.{0}.somesignature", Base64UrlHelpers.Encode(id));
480535
}
481536

482-
private static MockHttpMessageHandler CreateOidcHttpHandler(string oidcEndpoint)
537+
private static MockHttpMessageHandler CreateOidcHttpHandler(string oidcEndpoint, string authority = null)
483538
{
484539
return new MockHttpMessageHandler()
485540
{
486541
ExpectedMethod = HttpMethod.Get,
487542
ExpectedUrl = oidcEndpoint,
488-
ResponseMessage = MockHelpers.CreateSuccessResponseMessage(TestConstants.GenericOidcResponse)
543+
ResponseMessage = MockHelpers.CreateSuccessResponseMessage(TestConstants.GetOidcResponse(authority))
489544
};
490545
}
491546
}

0 commit comments

Comments
 (0)