Microsoft-authentication-library-for-dotnet: [Bug] IPublicClientApplication is not mockable

[dfederm]

Which Version of MSAL are you using ?
Latest

Platform
Any

What authentication flow has the issue?
N/A

Is this a new or existing app?
N/A

Repro
IPublicClientApplication is not mockable. Specifically the AcquireTokenByIntegratedWindowsAuth method returns a AcquireTokenByIntegratedWindowsAuthParameterBuilder object, but that class only has internal constructors. This makes any class using that method untestable.

Expected behavior
IPublicClientApplication is mockble

Actual behavior
IPublicClientApplication is not mockable

Possible Solution
Either make the return values constructable, or interface them out.

Additional context/ Logs / Screenshots
N/A

[jmprieur]

Duplicate of https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/184. https://github.com/AzureAD/microsoft-authentication-library-for-dotnet/issues/679 provides some ideas of work arounds

[dfederm]

@jmprieur #184 is closed from 2017. Would you prefer I re-open it or use this issue? The interface as it currently exists cannot be unit tested.

[bgavrilMS]

Due to the use of the builder pattern, mocking the interface is practically impossible. We've tried. Even if the interface was mockable, mocking the builder pattern is hellish to say the least. So we decided to keep the builder pattern for the flexibility it provides in terms of the API.

I suggest you wrap all the MSAL calls into your own object, add an interface to that and mock it. It will be faster that way. You don't need to wrap AuthenticationResult - it has a constructor that can be used in test scenarios.

[dfederm]

@bgavrilMS This issue keeps coming up so being able to test their code is clearly a scenario developers want.

Wrapping in our own interface with concrete is a workable solution but it's not one that scales. Imagine needing to do this for every 3rd party package one consumes. That's just a nightmare of extra interface/class pairs purely because the underlying library doesn't want to expose an API in a testable way.

[bgavrilMS]

Reopening and adding for review to next major version as it would constitute a breaking change.

[bgavrilMS]

I want to be transparent in that we do not for-see a major version of MSAL in the near future. Wrapping MSAL logic is the only way to test your stuff today.

[bgavrilMS]

Given that we'd have to modify the main APIs significantly, we are refusing this.