Upgraded to latest pkg broken saving cache

[azuread]

Upgraded to latest pkg broken saving cache

[jmprieur]

Can you please tell us which version of the library you used to use with .NET Core 3.0, and which do you use with .NET Core 3.1. Do you have a full repro project?

[ivberg]

Apologies - the versions were included in the original issue submission, but were invisible since it looks like GitHub markup was hiding the version since there were in XML tags. I removed the surrounding tags so you can see the before/after versions

[bgavrilMS]

Looking at this now.

[bgavrilMS]

Was not able to reproduce this. I tried the exact versions you mentioned. AcquireTokenSilent always works for me. What is the exact exception that you get from AcquireTokenSilent ?

Is this happening for some users only? Do you have a reliable repro? Feel free to ping me and we can try to debug.

[ivberg]

Debugged with @bgavrilMS - thx!

So it looks there were multiple issues here:

1. There was a build file copy issue related to new pkgs with different build dates on dlls that now had different build order/date than before. Our project uses DPAPI System.Security.Cryptography.ProtectedData.dll which this lib uses to save/protect creds on Windows. It seems normally that this "stub" (21KB) dll copied to $(OutDir) (on Windows) has a NotImplementedException for DPAPI but somehow magically redirects to the full dll (33kB) under runtimes\win\lib\netstandard2.0. That is in the standard case so it just works.
   But our exe uses AssemblyLoadContext to load potentially different versions of the same named dll, and apparently our implementation made it so this magic doesn't work and the stub with NotImplementedException would be thrown and used instead. We had a workaround which is to copy the real dll in it's place. This workaround broke with the pkg upgrade because now the stub version was newer than the real .dll (xcopy /D) so although the dll copy code was the same, the runtime still broke.

This coupled with (2) below made this silently break w/o any exceptions/warnings. Build Fix To Always copy the .dll even if the one it's replacing is newer
// removed XML tags so GitHub shows the XML :(
FROM:
<Target Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="CALL XCOPY /dickherfy $(ProjectDir)$(OutDir)runtimes\win\lib\netstandard2.0\System.Security.Cryptography.ProtectedData.dll $(ProjectDir)$(OutDir)" />
<Exec Command="CALL XCOPY /dickherfy $(ProjectDir)$(OutDir)runtimes\win\lib\netcoreapp2.1\System.Security.Principal.Windows.dll $(ProjectDir)$(OutDir)" />
</Target>

TO:
<Target Name="PostBuild" AfterTargets="PostBuildEvent">
<Exec Command="CALL XCOPY /ickherfy $(ProjectDir)$(OutDir)runtimes\win\lib\netstandard2.0\System.Security.Cryptography.ProtectedData.dll $(ProjectDir)$(OutDir)" />
<Exec Command="CALL XCOPY /ickherfy $(ProjectDir)$(OutDir)runtimes\win\lib\netcoreapp2.1\System.Security.Principal.Windows.dll $(ProjectDir)$(OutDir)" />
</Target>

2. As mentioned above MsalCacheHelper doesn't throw exceptions when trying to use or create saved tokens, thus this silently broken /w no good exceptions stating why. It was suggested we use this API call which does help say what the exact issue is and is helpful - msalCacheHelper.VerifyPersistence(); // If there is an issue with DPAPI System.Security.Cryptography.ProtectedData.dll this will catch it where-as lib use above will silently fail and swallow exceptions

[bgavrilMS]

@jmprieur @henrik-me - the main problem here is that the cache ext does not throw exceptions, because MSAL can just work with in-memory cache. This is making investigations into issues much more complex, as developers just see that AcquireTokenSilent always fails, but have no idea why.

I think we should take a small breaking change here and make this token cache impl throw exceptions instead of swallowing them. We can have existing customer configure a flag to opt-out of exceptions instead.

[henrik-me]

@bgavrilMS not sure we can can make such a breaking change, we can do opt in, and do that in our sample code. Perhaps another job for the analyzer/best practice tool. We can also highlight the importance of using ValidatePersistence() ?

[jmprieur]

Which exception do we throw, @bgavrilMS? the one related to the IOs?

[bgavrilMS]

In this particular case, @jmprieur , the problem is on Windows with DPAPI, as the DPAPI.dll used by the program was throwing a NotImplementedException.

[bgavrilMS]

For logging, VS automatically adds a trace listener to its output window

[bgavrilMS]

I spent some time figuring out logging in this library and documented it here