[Identity] Update SP credentials to use MSAL confidential client (#13168

) * Update SP credentials to use MSAL confidential client * updating api spec * fixing tests * work around null PrivateKey issue on net461 * fixing pwsh presteps * fixing PR feedback issues * fixing doc comment * adding ITokenCacheOptions to client credential options types
Azure · Jul 2, 2020 · 95f99d9 · 95f99d9
1 parent 370be2e
commit 95f99d9
Show file tree

Hide file tree

Showing 37 changed files with 4,075 additions and 244 deletions.
diff --git a/sdk/identity/Azure.Identity/Azure.Identity.sln b/sdk/identity/Azure.Identity/Azure.Identity.sln
@@ -12,7 +12,9 @@ EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{8F748B65-7779-4A71-B2EC-9BA8B9526AB0}"
 	ProjectSection(SolutionItems) = preProject
 		CHANGELOG.md = CHANGELOG.md
+		..\ci.yml = ..\ci.yml
 		README.md = README.md
+		..\tests.yml = ..\tests.yml
 	EndProjectSection
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Core.TestFramework", "..\..\core\Azure.Core.TestFramework\src\Azure.Core.TestFramework.csproj", "{DBE8A141-33F2-489A-A228-B2C919E10C03}"

diff --git a/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs b/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs
@@ -47,20 +47,33 @@ public partial class ClientCertificateCredential : Azure.Core.TokenCredential
     {
         protected ClientCertificateCredential() { }
         public ClientCertificateCredential(string tenantId, string clientId, System.Security.Cryptography.X509Certificates.X509Certificate2 clientCertificate) { }
+        public ClientCertificateCredential(string tenantId, string clientId, System.Security.Cryptography.X509Certificates.X509Certificate2 clientCertificate, Azure.Identity.ClientCertificateCredentialOptions options) { }
         public ClientCertificateCredential(string tenantId, string clientId, System.Security.Cryptography.X509Certificates.X509Certificate2 clientCertificate, Azure.Identity.TokenCredentialOptions options) { }
         public ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath) { }
+        public ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, Azure.Identity.ClientCertificateCredentialOptions options) { }
         public ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, Azure.Identity.TokenCredentialOptions options) { }
         public override Azure.Core.AccessToken GetToken(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
         public override System.Threading.Tasks.ValueTask<Azure.Core.AccessToken> GetTokenAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
     }
+    public partial class ClientCertificateCredentialOptions : Azure.Identity.TokenCredentialOptions
+    {
+        public ClientCertificateCredentialOptions() { }
+        public bool EnablePersistentCache { get { throw null; } set { } }
+    }
     public partial class ClientSecretCredential : Azure.Core.TokenCredential
     {
         protected ClientSecretCredential() { }
         public ClientSecretCredential(string tenantId, string clientId, string clientSecret) { }
+        public ClientSecretCredential(string tenantId, string clientId, string clientSecret, Azure.Identity.ClientSecretCredentialOptions options) { }
         public ClientSecretCredential(string tenantId, string clientId, string clientSecret, Azure.Identity.TokenCredentialOptions options) { }
         public override Azure.Core.AccessToken GetToken(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
         public override System.Threading.Tasks.ValueTask<Azure.Core.AccessToken> GetTokenAsync(Azure.Core.TokenRequestContext requestContext, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) { throw null; }
     }
+    public partial class ClientSecretCredentialOptions : Azure.Identity.TokenCredentialOptions
+    {
+        public ClientSecretCredentialOptions() { }
+        public bool EnablePersistentCache { get { throw null; } set { } }
+    }
     public partial class CredentialUnavailableException : Azure.Identity.AuthenticationFailedException
     {
         public CredentialUnavailableException(string message) : base (default(string)) { }

diff --git a/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs b/sdk/identity/Azure.Identity/src/ClientCertificateCredential.cs
@@ -4,6 +4,7 @@
 
 using Azure.Core;
 using Azure.Core.Pipeline;
+using Microsoft.Identity.Client;
 using System;
 using System.Collections.Generic;
 using System.IO;
@@ -39,7 +40,7 @@ public class ClientCertificateCredential : TokenCredential
         /// </summary>
         internal IX509Certificate2Provider ClientCertificateProvider { get; }
 
-        private readonly AadIdentityClient _client;
+        private readonly MsalConfidentialClient _client;
         private readonly CredentialPipeline _pipeline;
 
         /// <summary>
@@ -68,7 +69,24 @@ public ClientCertificateCredential(string tenantId, string clientId, string clie
         /// <param name="clientCertificatePath">The path to a file which contains both the client certificate and private key.</param>
         /// <param name="options">Options that allow to configure the management of the requests sent to the Azure Active Directory service.</param>
         public ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, TokenCredentialOptions options)
-            : this(tenantId, clientId, clientCertificatePath, CredentialPipeline.GetInstance(options))
+            : this(new MsalConfidentialClientOptions(
+                tenantId: tenantId ?? throw new ArgumentNullException(nameof(tenantId)),
+                clientId: clientId ?? throw new ArgumentNullException(nameof(clientId)),
+                certificateProvider: new X509Certificate2FromFileProvider(clientCertificatePath ?? throw new ArgumentNullException(nameof(clientCertificatePath))),
+                options: options))
+
+        {
+        }
+
+        /// <summary>
+        /// Creates an instance of the ClientCertificateCredential with the details needed to authenticate against Azure Active Directory with the specified certificate.
+        /// </summary>
+        /// <param name="tenantId">The Azure Active Directory tenant (directory) Id of the service principal.</param>
+        /// <param name="clientId">The client (application) ID of the service principal</param>
+        /// <param name="clientCertificatePath">The path to a file which contains both the client certificate and private key.</param>
+        /// <param name="options">Options that allow to configure the management of the requests sent to the Azure Active Directory service.</param>
+        public ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, ClientCertificateCredentialOptions options)
+            : this(tenantId, clientId, clientCertificatePath, (TokenCredentialOptions)options)
 
         {
         }
@@ -92,38 +110,57 @@ public ClientCertificateCredential(string tenantId, string clientId, X509Certifi
         /// <param name="clientCertificate">The authentication X509 Certificate of the service principal</param>
         /// <param name="options">Options that allow to configure the management of the requests sent to the Azure Active Directory service.</param>
         public ClientCertificateCredential(string tenantId, string clientId, X509Certificate2 clientCertificate, TokenCredentialOptions options)
-            : this(tenantId, clientId, clientCertificate, CredentialPipeline.GetInstance(options))
+            : this(new MsalConfidentialClientOptions(
+                tenantId: tenantId ?? throw new ArgumentNullException(nameof(tenantId)),
+                clientId: clientId ?? throw new ArgumentNullException(nameof(clientId)),
+                certificateProvider: new X509Certificate2FromObjectProvider(clientCertificate ?? throw new ArgumentNullException(nameof(clientCertificate))),
+                options: options))
 
         {
         }
 
-        internal ClientCertificateCredential(string tenantId, string clientId, X509Certificate2 clientCertificate, CredentialPipeline pipeline)
-            : this(tenantId, clientId, clientCertificate, pipeline, new AadIdentityClient(pipeline))
+        /// <summary>
+        /// Creates an instance of the ClientCertificateCredential with the details needed to authenticate against Azure Active Directory with the specified certificate.
+        /// </summary>
+        /// <param name="tenantId">The Azure Active Directory tenant (directory) Id of the service principal.</param>
+        /// <param name="clientId">The client (application) ID of the service principal</param>
+        /// <param name="clientCertificate">The authentication X509 Certificate of the service principal</param>
+        /// <param name="options">Options that allow to configure the management of the requests sent to the Azure Active Directory service.</param>
+        public ClientCertificateCredential(string tenantId, string clientId, X509Certificate2 clientCertificate, ClientCertificateCredentialOptions options)
+            : this(tenantId, clientId, clientCertificate, (TokenCredentialOptions)options)
         {
         }
 
-        internal ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, CredentialPipeline pipeline)
-            : this(tenantId, clientId, clientCertificatePath, pipeline, new AadIdentityClient(pipeline))
+        internal ClientCertificateCredential(MsalConfidentialClientOptions clientOptions)
         {
+            TenantId = clientOptions.TenantId;
+
+            ClientId = clientOptions.ClientId;
+
+            ClientCertificateProvider = clientOptions.CertificateProvider;
+
+            _pipeline = clientOptions.Pipeline;
+
+            _client = new MsalConfidentialClient(clientOptions);
         }
 
-        internal ClientCertificateCredential(string tenantId, string clientId, X509Certificate2 clientCertificate, CredentialPipeline pipeline, AadIdentityClient client)
-            : this(tenantId, clientId, new X509Certificate2FromObjectProvider(clientCertificate ?? throw new ArgumentNullException(nameof(clientCertificate))), pipeline, client)
+        internal ClientCertificateCredential(string tenantId, string clientId, string certificatePath, CredentialPipeline pipeline, MsalConfidentialClient client)
+            : this(tenantId, clientId, new X509Certificate2FromFileProvider(certificatePath), pipeline, client)
         {
         }
 
-        internal ClientCertificateCredential(string tenantId, string clientId, string clientCertificatePath, CredentialPipeline pipeline, AadIdentityClient client)
-            : this(tenantId, clientId, new X509Certificate2FromFileProvider(clientCertificatePath ?? throw new ArgumentNullException(nameof(clientCertificatePath))), pipeline, client)
+        internal ClientCertificateCredential(string tenantId, string clientId, X509Certificate2 certificate, CredentialPipeline pipeline, MsalConfidentialClient client)
+            : this(tenantId, clientId, new X509Certificate2FromObjectProvider(certificate), pipeline, client)
         {
         }
 
-        internal ClientCertificateCredential(string tenantId, string clientId, IX509Certificate2Provider clientCertificateProvider, CredentialPipeline pipeline, AadIdentityClient client)
+        internal ClientCertificateCredential(string tenantId, string clientId, IX509Certificate2Provider certificateProvider, CredentialPipeline pipeline, MsalConfidentialClient client)
         {
-            TenantId = tenantId ?? throw new ArgumentNullException(nameof(tenantId));
+            TenantId = tenantId;
 
-            ClientId = clientId ?? throw new ArgumentNullException(nameof(clientId));
+            ClientId = clientId;
 
-            ClientCertificateProvider = clientCertificateProvider;
+            ClientCertificateProvider = certificateProvider;
 
             _pipeline = pipeline;
 
@@ -142,8 +179,9 @@ public override AccessToken GetToken(TokenRequestContext requestContext, Cancell
 
             try
             {
-                X509Certificate2 cert = ClientCertificateProvider.GetCertificateAsync(false, cancellationToken).EnsureCompleted();
-                return scope.Succeeded(_client.Authenticate(TenantId, ClientId, cert, requestContext.Scopes, cancellationToken));
+                AuthenticationResult result = _client.AcquireTokenForClientAsync(requestContext.Scopes, false, cancellationToken).EnsureCompleted();
+
+                return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
             }
             catch (Exception e)
             {
@@ -163,8 +201,9 @@ public override async ValueTask<AccessToken> GetTokenAsync(TokenRequestContext r
 
             try
             {
-                X509Certificate2 cert = await ClientCertificateProvider.GetCertificateAsync(true, cancellationToken).ConfigureAwait(false);
-                return scope.Succeeded(await _client.AuthenticateAsync(TenantId, ClientId, cert, requestContext.Scopes, cancellationToken).ConfigureAwait(false));
+                AuthenticationResult result = await _client.AcquireTokenForClientAsync(requestContext.Scopes, true, cancellationToken).ConfigureAwait(false);
+
+                return scope.Succeeded(new AccessToken(result.AccessToken, result.ExpiresOn));
             }
             catch (Exception e)
             {
@@ -353,6 +392,15 @@ private async ValueTask<X509Certificate2> LoadCertificateFromPemFileAsync(bool a
                     X509Certificate2 certWithoutPrivateKey = new X509Certificate2(Convert.FromBase64String(certificateMatch.Groups[3].Value));
                     Certificate = (X509Certificate2)copyWithPrivateKeyMethodInfo.Invoke(null, new object[] { certWithoutPrivateKey, privateKey });
 
+                    // On desktop NetFX it appears the PrivateKey property is not initialized after calling CopyWithPrivateKey
+                    // this leads to an issue when using the MSAL ConfidentialClient which uses the PrivateKey property to get the
+                    // signing key vs. the extension method GetRsaPrivateKey which we were previously using when signing the claim ourselves.
+                    // Because of this we need to set PrivateKey to the instance we created to deserialize the private key
+                    if (Certificate.PrivateKey == null)
+                    {
+                        Certificate.PrivateKey = privateKey;
+                    }
+
                     return Certificate;
                 }
                 catch (Exception e) when (!(e is OperationCanceledException))

diff --git a/sdk/identity/Azure.Identity/src/ClientCertificateCredentialOptions.cs b/sdk/identity/Azure.Identity/src/ClientCertificateCredentialOptions.cs
@@ -0,0 +1,17 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+namespace Azure.Identity
+{
+    /// <summary>
+    /// Options used to configure the <see cref="ClientCertificateCredential"/>.
+    /// </summary>
+    public class ClientCertificateCredentialOptions : TokenCredentialOptions, ITokenCacheOptions
+    {
+
+        /// <summary>
+        /// If set to true the credential will store tokens in a cache persisted to the machine, protected to the current user, which can be shared by other credentials and processes.
+        /// </summary>
+        public bool EnablePersistentCache { get; set; }
+    }
+}