Log MSAL for ConfidentialClient (Azure#23516)

christothes · web-flow · commit f34705ea92b0 · 2021-08-25T13:14:55.000-05:00
* Log MSAL for ConfidentialClient
diff --git a/sdk/identity/Azure.Identity/src/AuthorizationCodeCredential.cs b/sdk/identity/Azure.Identity/src/AuthorizationCodeCredential.cs
@@ -93,7 +93,15 @@ internal AuthorizationCodeCredential(string tenantId, string clientId, string cl
                 _ => null
             };
 
-            _client = client ?? new MsalConfidentialClient(_pipeline, tenantId, clientId, clientSecret, options as ITokenCacheOptions, null);
+            _client = client ??
+                      new MsalConfidentialClient(
+                          _pipeline,
+                          tenantId,
+                          clientId,
+                          clientSecret,
+                          options as ITokenCacheOptions,
+                          null,
+                          options?.IsLoggingPIIEnabled ?? false);
         }
 
         /// <summary>
diff --git a/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs b/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs
@@ -134,7 +134,16 @@ internal ClientCertificateCredential(string tenantId, string clientId, IX509Cert
 
             ClientCertificateCredentialOptions certCredOptions = (options as ClientCertificateCredentialOptions);
 
-            Client = client ?? new MsalConfidentialClient(_pipeline, tenantId, clientId, certificateProvider, certCredOptions?.SendCertificateChain ?? false, options as ITokenCacheOptions, certCredOptions?.RegionalAuthority);
+            Client = client ??
+                     new MsalConfidentialClient(
+                         _pipeline,
+                         tenantId,
+                         clientId,
+                         certificateProvider,
+                         certCredOptions?.SendCertificateChain ?? false,
+                         options as ITokenCacheOptions,
+                         certCredOptions?.RegionalAuthority,
+                         options?.IsLoggingPIIEnabled ?? false);
         }
 
         /// <summary>
diff --git a/sdk/identity/Azure.Identity/src/ClientSecretCredential.cs b/sdk/identity/Azure.Identity/src/ClientSecretCredential.cs
@@ -90,7 +90,15 @@ internal ClientSecretCredential(string tenantId, string clientId, string clientS
             ClientSecret = clientSecret;
             _allowMultiTenantAuthentication = options?.AllowMultiTenantAuthentication ?? false;
             _pipeline = pipeline ?? CredentialPipeline.GetInstance(options);
-            Client = client ?? new MsalConfidentialClient(_pipeline, tenantId, clientId, clientSecret, options as ITokenCacheOptions, (options as ClientSecretCredentialOptions)?.RegionalAuthority);
+            Client = client ??
+                     new MsalConfidentialClient(
+                         _pipeline,
+                         tenantId,
+                         clientId,
+                         clientSecret,
+                         options as ITokenCacheOptions,
+                         (options as ClientSecretCredentialOptions)?.RegionalAuthority,
+                         options?.IsLoggingPIIEnabled ?? false);
         }
 
         /// <summary>
diff --git a/sdk/identity/Azure.Identity/src/MsalClientBase.cs b/sdk/identity/Azure.Identity/src/MsalClientBase.cs
@@ -11,6 +11,7 @@ internal abstract class MsalClientBase<TClient>
         where TClient : IClientApplicationBase
     {
         private readonly AsyncLockWithValue<TClient> _clientAsyncLock;
+        internal protected bool LogPII { get; protected set; }
 
         /// <summary>
         /// For mocking purposes only.
diff --git a/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs b/sdk/identity/Azure.Identity/src/MsalConfidentialClient.cs
@@ -22,19 +22,21 @@ protected MsalConfidentialClient()
         {
         }
 
-        public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, string clientSecret, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority)
+        public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, string clientSecret, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority, bool logPii)
             : base(pipeline, tenantId, clientId, cacheOptions)
         {
             _clientSecret = clientSecret;
             RegionalAuthority = regionalAuthority;
+            LogPII = logPii;
         }
 
-        public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, ClientCertificateCredential.IX509Certificate2Provider certificateProvider, bool includeX5CClaimHeader, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority)
+        public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, ClientCertificateCredential.IX509Certificate2Provider certificateProvider, bool includeX5CClaimHeader, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority, bool logPii)
             : base(pipeline, tenantId, clientId, cacheOptions)
         {
             _includeX5CClaimHeader = includeX5CClaimHeader;
             _certificateProvider = certificateProvider;
             RegionalAuthority = regionalAuthority;
+            LogPII = logPii;
         }
 
         internal RegionalAuthority? RegionalAuthority { get; }
@@ -43,7 +45,8 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA
         {
             ConfidentialClientApplicationBuilder confClientBuilder = ConfidentialClientApplicationBuilder.Create(ClientId)
                 .WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, TenantId)
-                .WithHttpClientFactory(new HttpPipelineClientFactory(Pipeline.HttpPipeline));
+                .WithHttpClientFactory(new HttpPipelineClientFactory(Pipeline.HttpPipeline))
+                .WithLogging(AzureIdentityEventSource.Singleton.LogMsal, enablePiiLogging: LogPII);
 
             if (_clientSecret != null)
             {
diff --git a/sdk/identity/Azure.Identity/src/MsalPublicClient.cs b/sdk/identity/Azure.Identity/src/MsalPublicClient.cs
@@ -14,7 +14,6 @@ namespace Azure.Identity
     internal class MsalPublicClient : MsalClientBase<IPublicClientApplication>
     {
         internal string RedirectUrl { get; }
-        internal bool LogPII { get; }
 
         protected MsalPublicClient()
         { }
diff --git a/sdk/identity/Azure.Identity/tests/samples/TokenCacheSnippets.cs b/sdk/identity/Azure.Identity/tests/samples/TokenCacheSnippets.cs
@@ -12,9 +12,7 @@ namespace Azure.Identity.Samples
 {
     public class TokenCacheSnippets
     {
-        #region Snippet:Identity_TokenCache_CustomPersistence_Usage_TokenCachePath
         private const string TOKEN_CACHE_PATH = "./tokencache.bin";
-        #endregion
 
         public void Identity_TokenCache_PersistentDefault()
         {

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ internal abstract class MsalClientBase<TClient>`
`11`	`11`	`where TClient : IClientApplicationBase`
`12`	`12`	`{`
`13`	`13`	`private readonly AsyncLockWithValue<TClient> _clientAsyncLock;`
	`14`	`+ internal protected bool LogPII { get; protected set; }`
`14`	`15`
`15`	`16`	`/// <summary>`
`16`	`17`	`/// For mocking purposes only.`
Original file line number	Diff line number	Diff line change
`@@ -22,19 +22,21 @@ protected MsalConfidentialClient()`
`22`	`22`	`{`
`23`	`23`	`}`
`24`	`24`
`25`		`- public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, string clientSecret, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority)`
	`25`	`+ public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, string clientSecret, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority, bool logPii)`
`26`	`26`	`: base(pipeline, tenantId, clientId, cacheOptions)`
`27`	`27`	`{`
`28`	`28`	`_clientSecret = clientSecret;`
`29`	`29`	`RegionalAuthority = regionalAuthority;`
	`30`	`+ LogPII = logPii;`
`30`	`31`	`}`
`31`	`32`
`32`		`- public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, ClientCertificateCredential.IX509Certificate2Provider certificateProvider, bool includeX5CClaimHeader, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority)`
	`33`	`+ public MsalConfidentialClient(CredentialPipeline pipeline, string tenantId, string clientId, ClientCertificateCredential.IX509Certificate2Provider certificateProvider, bool includeX5CClaimHeader, ITokenCacheOptions cacheOptions, RegionalAuthority? regionalAuthority, bool logPii)`
`33`	`34`	`: base(pipeline, tenantId, clientId, cacheOptions)`
`34`	`35`	`{`
`35`	`36`	`_includeX5CClaimHeader = includeX5CClaimHeader;`
`36`	`37`	`_certificateProvider = certificateProvider;`
`37`	`38`	`RegionalAuthority = regionalAuthority;`
	`39`	`+ LogPII = logPii;`
`38`	`40`	`}`
`39`	`41`
`40`	`42`	`internal RegionalAuthority? RegionalAuthority { get; }`
`@@ -43,7 +45,8 @@ protected override async ValueTask<IConfidentialClientApplication> CreateClientA`
`43`	`45`	`{`
`44`	`46`	`ConfidentialClientApplicationBuilder confClientBuilder = ConfidentialClientApplicationBuilder.Create(ClientId)`
`45`	`47`	`.WithAuthority(Pipeline.AuthorityHost.AbsoluteUri, TenantId)`
`46`		`- .WithHttpClientFactory(new HttpPipelineClientFactory(Pipeline.HttpPipeline));`
	`48`	`+ .WithHttpClientFactory(new HttpPipelineClientFactory(Pipeline.HttpPipeline))`
	`49`	`+ .WithLogging(AzureIdentityEventSource.Singleton.LogMsal, enablePiiLogging: LogPII);`
`47`	`50`
`48`	`51`	`if (_clientSecret != null)`
`49`	`52`	`{`
Original file line number	Diff line number	Diff line change
`@@ -14,7 +14,6 @@ namespace Azure.Identity`
`14`	`14`	`internal class MsalPublicClient : MsalClientBase<IPublicClientApplication>`
`15`	`15`	`{`
`16`	`16`	`internal string RedirectUrl { get; }`
`17`		`- internal bool LogPII { get; }`
`18`	`17`
`19`	`18`	`protected MsalPublicClient()`
`20`	`19`	`{ }`
Original file line number	Diff line number	Diff line change
`@@ -12,9 +12,7 @@ namespace Azure.Identity.Samples`
`12`	`12`	`{`
`13`	`13`	`public class TokenCacheSnippets`
`14`	`14`	`{`
`15`		`- #region Snippet:Identity_TokenCache_CustomPersistence_Usage_TokenCachePath`
`16`	`15`	`private const string TOKEN_CACHE_PATH = "./tokencache.bin";`
`17`		`- #endregion`
`18`	`16`
`19`	`17`	`public void Identity_TokenCache_PersistentDefault()`
`20`	`18`	`{`