AzureAD/azure-activedirectory-library-for-dotnet

AuthenticationParameters.CreateFromUnauthorizedResponseAsync() is async but doesn't need to

abatishchev opened this issue · 1 comments

Which Version of ADAL are you using ?

<PackageReference Include="Microsoft.IdentityModel.Clients.ActiveDirectory" Version="5.2.9" />

Which platform has the issue?

.NET Standard 2.1, .NET Core 3.1

What authentication flow has the issue?

  • Other?

Short description

AuthenticationParameters.CreateFromUnauthorizedResponse() is async because it calls async helper method but doesn't need to, the same outcome can be achieved without doing that.

Long description

Here's the code execution flow:

  1. await AuthenticationParameters.CreateFromUnauthorizedResponseAsync(responseMessage);
  2. await OAuthClient.CreateResponseAsync(responseMessage);
  3. AuthenticationParameters.CreateFromUnauthorizedResponseCommon()

So the method is marked async because it calls OAuthClient.CreateResponseAsync which is async to read the response content. Then it passes the resulting IHttpWebResponse to the private helper. The issue is that the said helper doesn't use the content so the whole thing can be omited.

Another issue is that there are no tests that would cover the method and/or help to prove the safety of the change.

Possible Solution

  1. Add sync overload CreateFromUnauthorizedResponse() which doesn't call the async helper
  2. Mark the async overload as [Obsolete]

ADAL is deprecated, so we are not taking new features or small fixes. Only critical / security fixes.
If you wish to contribute to MSAL, the team is happy to help.