Microsoft-authentication-library-for-dotnet: [Bug] MSAL.NET throws a MsalClientException and not a MsalServiceException for the reset password exception from AAD B2C

[jennyf19]

Which Version of MSAL are you using ?
First noticed in 4.1, but not sure which version introduced the change in behavior

Platform
mobile, desktop, web app

Repro

example, include this code:

catch (MsalServiceException ex)
          {
                try
                {
                          if (ex.Message.Contains("AADB2C90118"))
                          {
                                   authResult = await (app as 
                                         publicClientApplication).AcquireTokenInteractive(App.ApiScopes)
                                        .WithParentActivityOrWindow(new WindowInteropHelper(this).Handle)
                                        .WithAccount(GetAccountByPolicy(accounts, App.PolicySignUpSignIn))
                                        .WithPrompt(Prompt.SelectAccount)
                                        .WithB2CAuthority(App.AuthorityResetPassword)
                                        .ExecuteAsync();
}

Can be tried w/B2C desktop sample.

Expected behavior
MSAL.NET should throw a MsalServiceException with the following error message:
AADB2C90118: The user has forgotten their password.

Actual behavior
MSAL.NET throws a MsalClientException instead

[bgavrilMS]

So what is happening is:

• Upon clicking on that URI, the server returns an url containg "ErrorCode=access_denied" and "ErrorDescription = AADB2C90118: The user has forgotten their password"
• This translates to an AuthorizationResult with Status = ProtocolError
• Based on this AuthorizationResult, we throw an MsalClient exception (logic here)

Note sure when such a regression would have occured, since the logic that throws exceptions has not been touched since we split ADAL from MSAL.

I agree that we should look at these exceptions again - this is indeed a server exception.

[bgavrilMS]

Workaround: catch MsalException instead.

[jennyf19]

@bgavrilMS Thanks for investigating. I'll update the sample

[henrik-me]

Absolutely a regression. Assuming when this is fixed we will have improved test coverage for the exceptions. Aside: The sample is doing something it really shouldn't do thus there should probably also be an update to the sample.

[jennyf19]

@henrik-me @jmprieur ...so here's the commit where the change happened, i think, because the changed happened between 3.08 and 4.0 release, and this commit changes from catching a client exception to a service one. I haven't had any customers report this as an issue...do we want to move it back to a service exception or leave it as is? We can discuss tomorrow.

[henrik-me]

@jennyf19 @jmprieur : I suppose we should consider fixing this as it's a regression betwen 3 and 4. 4.3 seems like a good vehicle for this change imo.

[jmprieur]

yes agree. even if this is a behavioral breaking change.

[jennyf19]

Included in 4.3.0 release