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

Implement ADAL logging callback for NetCore #6166

Closed
MiYanni opened this issue May 9, 2018 · 1 comment
Closed

Implement ADAL logging callback for NetCore #6166

MiYanni opened this issue May 9, 2018 · 1 comment

Comments

@MiYanni
Copy link
Contributor

MiYanni commented May 9, 2018

Currently, there is a bug in the version of ADAL (3.19.2) we are using for NetCore.
AzureAD/azure-activedirectory-library-for-dotnet#1028

This version is resolved directly from Common.Authentication.Abstractions.Netcore via Microsoft.ClientRuntime.Azure.Authentication which uses ADAL 3.19.2. For now, to avoid this issue, we have disabled logging for ADAL via LoggerCallbackHandler.UseDefaultLogging = false; set in AzureSessionInitializer.cs in Common.Authentication.Netcore. This is simply a workaround so that logging information isn't written to the console directly.

For us to have proper ADAL logging, we need to implement a callback. Basically, we can add this callback method to the AdalSession and set it via LoggerCallbackHandler.LogCallback. More information can be found here:
https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/wiki/Logging-in-ADAL.Net

We have to do this even if we upgrade to 3.19.4 (current version). The reason is because .NET Core has no default output for logging. You can see that here:
https://github.com/AzureAD/azure-activedirectory-library-for-dotnet/blob/82565905ca0213e7963a275c40382a8560f9d141/adal/src/Microsoft.IdentityModel.Clients.ActiveDirectory/Platforms/netstandard1.1/AdalLogger.cs#L40

Note: We would keep LoggerCallbackHandler.UseDefaultLogging = false; for NetCore since we'd have this callback. Even if they implement a default logger down the line, we want it to use our callback anyway.

@markcowl
Copy link
Member

Closing in favor of MSAL update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants