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

Support gmsa when installing agent #44

Open
tristanbarcelon opened this issue Sep 14, 2023 · 7 comments
Open

Support gmsa when installing agent #44

tristanbarcelon opened this issue Sep 14, 2023 · 7 comments
Labels
enhancement New feature or request

Comments

@tristanbarcelon
Copy link
Contributor

tristanbarcelon commented Sep 14, 2023

@jwittner, Install-VSTSAgent function currently supports installation of agents as a Windows service when LogonCredential parameter is set. We would like to deploy it without needing to provide passwords. GMSAs provide the ability to run Windows services without having to provide/rotate passwords and agent now supports it since this update. I'd like to propose adding this feature to Install-VSTSAgent but before that, would prefer to discuss how to do so with maintainers.

@jwittner
Copy link
Member

jwittner commented Sep 18, 2023

Is it as straightforward as not passing --windowsLogonPassword when calling with a GMSA? We could check where we append the argument here for e.g. an empty string and not pass the argument. So users would create a credential with the user name and an empty password.

@jwittner jwittner added the enhancement New feature or request label Sep 18, 2023
@tristanbarcelon
Copy link
Contributor Author

tristanbarcelon commented Sep 19, 2023

@jwittner, I tried creating a PSCredential object with an empty string as password and convertto-securestring disallows it, resulting in a null $LogonCredential parameter.

image

Looking at the existing logic below, a non-null $LogonCredential parameter is utilized for detecting intent to run as service by specifying --windowsLogonAccount and --windowsLogonPassword.

image

This feels really icky, but what if we supply a new optional switch parameter $UseGMSA? This will work in conjunction with $LogonParameter and only use the UserName property of $LogonParameter when UseGMSA is also true or specified. Conversely, --windowsLogonPassword will only be specified if UseGMSA is NOT specified or false. Only this block will be modified. Unfortunately, securestring password of the account will still be required even if it is set to an invalid value like a guid string just to have a valid non-null $LogonCredential.

if ( $LogonCredential ) {
    if (($PSBoundParameters.ContainsKey('UseGMSA') -or $UseGMSA)
    {
        $configArgs += '--windowsLogonPassword', `
        [System.Net.NetworkCredential]::new($null, $LogonCredential.Password).Password
    }
}

An alternative to the introduction of $UseGMSA switch parameter would be a new and an optional $GMSA string parameter. If this is not null or empty, then we ignore the $LogonCredential parameter and set value of --windowsLogonAccount equal to value of $GMSA. It might warrant setting different ParameterSet combinations so specifying GMSA precludes the need to specify LogonParameter to make Install-VSTSAgent intuitive.

@jwittner
Copy link
Member

This Stack Overflow has a suggestion that worked for me.

$mycreds = New-Object System.Management.Automation.PSCredential("username", (New-Object System.Security.SecureString))

@tristanbarcelon
Copy link
Contributor Author

tristanbarcelon commented Sep 19, 2023

Thanks @jwittner. Doesn't look like we'll need any new parameter to use GMSA but simply a new documented way to create $LogonCredential object to signal intent to use GMSA when calling Install-VSTSAgent. Are you ok with a change like this?

if ($LogonCredential ) {
    $NetworkCredential = [System.Net.NetworkCredential]::new($null, $LogonCredential.Password)
    if (-not [string]::IsNullOrEmpty($NetworkCredential.Password))
    {
        $configArgs += '--windowsLogonPassword', $NetworkCredential.Password
    }
}

@jwittner
Copy link
Member

Pretty close, but we don't need to convert to test the length, and there's a handy GetNetworkCredential on PSCredential. Maybe this?

if ($LogonCredential -and $LogonCredential.Password.Length -gt 0) {
    $configArgs += '--windowsLogonPassword', $LogonCredential.GetNetworkCredential().Password
}

@tristanbarcelon
Copy link
Contributor Author

tristanbarcelon commented Sep 26, 2023

@jwittner, would you be ok if I combine checks involving $LogonCredential? For example, instead of making change as suggested above:

image

I place them closer together as shown below:

image

I'd like to add Write-Verbose statements so I could tell which way the agent is being configured and consolidating them in this block makes code easier to read.

@jwittner
Copy link
Member

jwittner commented Sep 27, 2023

That change would lose the print of the logon account, valuable information, in the What-If and -Verbose operations. The split is mainly to add the password after printing.

I like the verbose statements you're adding though - might still test the password where the account is added to configArgs and do that, but leave adding the password to the configArgs as a second test below the call to ShouldProcess, something akin to my suggestion above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants