AzureAD / microsoft-authentication-library-for-dotnet

Microsoft Authentication Library (MSAL) for .NET
https://aka.ms/msal-net
MIT License
1.37k stars 338 forks source link

[Feature Request] Update MSAL exception when server returns 500 to include request URI #4412

Open gladjohn opened 9 months ago

gladjohn commented 9 months ago

MSAL client type

Confidential

Problem Statement

When failure happens while fetching a MEX document during a federated auth scenario we get a service_not_available exeption, the error misleads to thinking it was from AAD for a 429.

for e.g. in the case of an ADFS issue we see this error

ERROR,"Token refresh failed. Encountered exception MSAL.Desktop.4.50.0.0.MsalServiceException: ErrorCode: service_not_available Microsoft.Identity.Client.MsalServiceException: Service is unavailable to process the request at Microsoft.Identity.Client.Http.HttpManager.

but also in the stack trace we see this, meaning failure to get the MEX

Microsoft.Identity.Client.WsTrust.WsTrustWebRequestManager.<GetMexDocumentAsync>d__2.MoveNext()

MSAL does not bubble up this error.

One possible solution is to add the request URI in the MSAL exception when server returns 500.

Proposed solution

No response

Alternatives

No response

bgavrilMS commented 9 months ago

Let's take this as a confidential client fix.

bgavrilMS commented 2 months ago

Pls ensure to log just the base URL (host + path) as non-PII. Query paramters are PII.