search

AzureAD / microsoft-authentication-library-for-dotnet

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

IPublicClientApplication contains only one override of AcquireTokenByUsernamePasswordAsync whereas PublicClientApplication contains two #641

Closed jmprieur closed 6 years ago

jmprieur commented 6 years ago

Which Version of MSAL are you using ? MSAL 2.2.1-preview

Which platform has the issue? net45

What authentication flow has the issue?

Repro IPublicClientApplication contains only one override of AcquireTokenByUsernamePasswordAsync whereas PublicClientApplication contains two.

Possible Solution Add the missing override to the IPublicClientApplication interface

bgavrilMS commented 6 years ago

Will review all public api differences between the interface and the clients. Note that in this case, the 2nd override of AcquireTokenByUP is an obsolete one.

jmprieur commented 6 years ago

@bgavrilMS @henrik-me given that the missing override is an obsolete one, suggesting won't fix;

henrik-me commented 6 years ago

@jmprieur @bgavrilMS : why is it obsolete? We just added these. Would be great if you could specify which one was obsolete. If we agree it is obsolete then this bug could be used to go and flip that on the public client. Not convinced people using the API is actually using the interface thus they can still get to it. Removing wontfix as I believe we should at least mark the method as obsolete or something similar.

bgavrilMS commented 6 years ago

There is a single AcquireTokenByUP method that works, and it supports a SecureString password param. We have added a secondary AcquireTokenByUP method to MigrationAid.cs, taking in a string password. The reasoniong behing the second one is to help users migrate from ADAL.

This second method is Obsolete with a message along the lines of "please use SecureString instead of string for the password. We will not add this MigrationAid method to the interface, so closing this as Won't Fix.

Please reopen if you do not think we should have added the obsolete method to MigrationAid.