Address review comments

Azure · Apr 13, 2021 · fe82e1b · fe82e1b
1 parent 1611de4
commit fe82e1b
Show file tree

Hide file tree

Showing 7 changed files with 15 additions and 19 deletions.
diff --git a/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs b/src/Accounts/Accounts/Account/ConnectAzureRmAccount.cs
@@ -185,7 +185,7 @@ public class ConnectAzureRmAccountCommand : AzureContextModificationCmdlet, IMod
         [Parameter(Mandatory = false, HelpMessage = "Overwrite the existing context with the same name, if any.")]
         public SwitchParameter Force { get; set; }
 
-        [Parameter(ParameterSetName = ServicePrincipalCertificateParameterSet, Mandatory = false, HelpMessage = "Present to use subject name issuer authentication.")]
+        [Parameter(ParameterSetName = ServicePrincipalCertificateParameterSet, Mandatory = false, HelpMessage = "Specifies if the x5c claim (public key of the certificate) should be sent to the STS to achieve easy certificate rollover in Azure AD.")]
         public SwitchParameter SendCertificateChain { get; set; }
 
         protected override IAzureContext DefaultContext
@@ -289,13 +289,15 @@ public override void ExecuteCmdlet()
                     azureAccount.SetProperty(AzureAccount.Property.KeyVaultAccessToken, KeyVaultAccessToken);
                     break;
                 case ServicePrincipalCertificateParameterSet:
-                case ServicePrincipalParameterSet:
-                    azureAccount.Type = AzureAccount.AccountType.ServicePrincipal;
-                    if (SendCertificateChain.IsPresent)
+                    if (SendCertificateChain)
                     {
-                        azureAccount.SetProperty("SendCertificateChain", Boolean.TrueString);
+                        azureAccount.SetProperty("SendCertificateChain", SendCertificateChain.ToString());
+                        WriteDebug("SendCertificateChain is set.");
                     }
-
+                    azureAccount.Type = AzureAccount.AccountType.ServicePrincipal;
+                    break;
+                case ServicePrincipalParameterSet:
+                    azureAccount.Type = AzureAccount.AccountType.ServicePrincipal;  
                     break;
                 case ManagedServiceParameterSet:
                     azureAccount.Type = AzureAccount.AccountType.ManagedService;

diff --git a/src/Accounts/Accounts/Az.Accounts.psd1 b/src/Accounts/Accounts/Az.Accounts.psd1
@@ -143,11 +143,7 @@ PrivateData = @{
         # IconUri = ''
 
         # ReleaseNotes of this module
-<<<<<<< HEAD
         ReleaseNotes = '* Fallback to first valid context if current default context key is ''Default'' which is invalid'
-=======
-        ReleaseNotes = '* Supported subject name issuer authentication.'
->>>>>>> 193beb0daf... Update help markdown file
 
         # Prerelease string of this module
         # Prerelease = ''

diff --git a/src/Accounts/Accounts/ChangeLog.md b/src/Accounts/Accounts/ChangeLog.md
@@ -19,6 +19,7 @@
 -->
 
 ## Upcoming Release
+* Supported subject name issuer authentication.
 
 ## Version 2.2.8
 * Fallback to first valid context if current default context key is "Default" which is invalid
@@ -34,9 +35,6 @@
 * Tracked CloudError code in exception
 * Raised 'ContextCleared' event when `Clear-AzContext` was executed
 
-## Version 100.0.0
-* Supported subject name issuer authentication.
-
 ## Version 2.2.4
 * Shown correct client request id on debug message [#13745]
 * Added common Azure PowerShell exception type

diff --git a/src/Accounts/Accounts/help/Connect-AzAccount.md b/src/Accounts/Accounts/help/Connect-AzAccount.md
@@ -501,7 +501,7 @@ Accept wildcard characters: False
 ```
 
 ### -SendCertificateChain
-Present to use subject name issuer authentication.
+Specifies if the x5c claim (public key of the certificate) should be sent to the STS to achieve easy certificate rollover in Azure AD.
 
 ```yaml
 Type: System.Management.Automation.SwitchParameter

diff --git a/src/Accounts/Authentication/Authentication/Parameters/AuthenticationParameters.cs b/src/Accounts/Authentication/Authentication/Parameters/AuthenticationParameters.cs
@@ -28,8 +28,6 @@ public abstract class AuthenticationParameters
 
         public string ResourceId { get; set; }
 
-        public bool? SendCertificateChain { get; set; } = null;
-
         public AuthenticationParameters(
             PowerShellTokenCacheProvider tokenCacheProvider,
             IAzureEnvironment environment,
@@ -43,7 +41,6 @@ public AuthenticationParameters(
             TokenCache = tokenCache;
             TenantId = tenantId;
             ResourceId = resourceId;
-            SendCertificateChain = sendCertificateChain;
         }
     }
 }
diff --git a/src/Accounts/Authentication/Authentication/Parameters/ServicePrincipalParameters.cs b/src/Accounts/Authentication/Authentication/Parameters/ServicePrincipalParameters.cs
@@ -26,6 +26,8 @@ public class ServicePrincipalParameters : AuthenticationParameters
 
         public SecureString Secret { get; set; }
 
+        public bool? SendCertificateChain { get; set; } = null;
+
         public ServicePrincipalParameters(
             PowerShellTokenCacheProvider tokenCacheProvider,
             IAzureEnvironment environment,
@@ -35,11 +37,12 @@ public ServicePrincipalParameters(
             string applicationId,
             string thumbprint,
             SecureString secret,
-            bool? sendCertificateChain) : base(tokenCacheProvider, environment, tokenCache, tenantId, resourceId, sendCertificateChain)
+            bool? sendCertificateChain) : base(tokenCacheProvider, environment, tokenCache, tenantId, resourceId)
         {
             ApplicationId = applicationId;
             Thumbprint = thumbprint;
             Secret = secret;
+            SendCertificateChain = sendCertificateChain;
         }
     }
 }
diff --git a/src/Accounts/Authenticators/ServicePrincipalAuthenticator.cs b/src/Accounts/Authenticators/ServicePrincipalAuthenticator.cs
@@ -50,7 +50,7 @@ public override Task<IAccessToken> Authenticate(AuthenticationParameters paramet
             var options = new ClientCertificateCredentialOptions()
             {
                 AuthorityHost = new Uri(authority),
-                IncludeX5CCliamHeader = parameters.SendCertificateChain ?? default(bool)
+                IncludeX5CCliamHeader = spParameters.SendCertificateChain ?? default(bool)
             };
 
             if (!string.IsNullOrEmpty(spParameters.Thumbprint))