Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug] MSAL.NET throws a MsalClientException and not a MsalServiceException for the reset password exception from AAD B2C #1276

Closed
jennyf19 opened this issue Jul 19, 2019 · 8 comments
Assignees
Labels
B2C bug regression Behavior that worked in a previous release that no longer works in a newer release
Milestone

Comments

@jennyf19
Copy link
Collaborator

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

@henrik-me henrik-me added this to the 4.3 milestone Jul 19, 2019
@bgavrilMS
Copy link
Member

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
Copy link
Member

Workaround: catch MsalException instead.

@jennyf19
Copy link
Collaborator Author

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

@henrik-me
Copy link
Contributor

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.

@henrik-me henrik-me added bug regression Behavior that worked in a previous release that no longer works in a newer release and removed regression? labels Jul 23, 2019
@jennyf19
Copy link
Collaborator Author

@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
Copy link
Contributor

@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
Copy link
Contributor

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

@jennyf19
Copy link
Collaborator Author

jennyf19 commented Aug 8, 2019

Included in 4.3.0 release

@jennyf19 jennyf19 closed this as completed Aug 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B2C bug regression Behavior that worked in a previous release that no longer works in a newer release
Projects
None yet
Development

No branches or pull requests

4 participants