search

AzureAD / microsoft-authentication-library-for-python

Microsoft Authentication Library (MSAL) for Python makes it easy to authenticate to Microsoft Entra ID. General docs are available here https://learn.microsoft.com/entra/msal/python/ Stable APIs are documented here https://msal-python.readthedocs.io. Questions can be asked on www.stackoverflow.com with tag "msal" + "python".
https://stackoverflow.com/questions/tagged/azure-ad-msal+python
Other
756 stars 191 forks source link

MSAL logging discrepancy #568

Closed pvaneck closed 10 months ago

pvaneck commented 1 year ago

For posterity, I wanted to open this issue since there has been discussion within our team regarding a recent feature request from the MSAL team where Azure Identity should allow extraction of all MSAL logs by enabling some PII boolean flag or increasing log verbosity: https://github.com/Azure/azure-sdk-for-python/issues/29406.

As far as I know, other MSAL language SDKs support toggling PII logging through some flag, but this SDK does not. From previous discussion with @rayluo, it was chosen to not enable PII logging here since it was felt that the info wasn't useful for troubleshooting.

Would appreciate some clarification on the following:

rayluo commented 1 year ago
  1. MSAL Python's code base does not emit logs with PII, so, there has been no need to support an enable_pii_log boolean flag.
  2. Nowadays, MSALs start to integrate with MsalRuntime (a.k.a. broker), and MsalRuntime has such an "enable PII log" behavior. So, MSAL Python can add an enable_pii_log boolean flag and wire it up with MsalRuntime. Therefore, we are labeling this issue as an "enhancement" now. We are currently fully booked until August. We will revisit this at that time.
  3. The lack of this PII flag does not block downstream partners such as Azure SDK from implementing its own EnableSupportLogging flag. For example, Azure Identity may choose to log the UPN right before calling into MSAL Python, when EnableSupportLogging is set.
  • Currently, is all logging control within MSAL Python based on setting the log level (logging.basicConfig(level=logging.DEBUG))?

Yes, and that logging level is a pure Python behavior, not an MSAL Python invention. In particular, even when enable_pii_log is supported in the future, there will be no defined relationship between log level and the PII-ness. Meaning, enabling PII flag alone does not guarantee full logs; they are by-definition two different dimensions; you may need to teach downstream (3rd-party?) apps to enable both pii and debug for your troubleshooting.

bgavrilMS commented 10 months ago

Marking this as a bug @rayluo, as it hinders supportability.

rayluo commented 10 months ago

Marking this as a bug @rayluo, as it hinders supportability.

OK I get the emphasis on supportability here. In the earlier message we promised to pick up this issue in Aug, so, it is about time. Let me do it before my leave.

This is still an enhancement, not a bug, though. All other subjective classifications aside, this change will be a new enable_pii_log parameter which - per definition in Semantic Versioning - MUST be shipped with a minor version bump, rather than a patch version bump. (BTW, I'll be happy to have a separate conversation on Semantic Versioning. Last time when I delivered a "backend API versioning" talk in a learning day, we did not focus on the client-side versioning specifically.)