ADDomainController: Support for supplying DelegatedAdministratorAccountName

[Borgquite]

Pull Request (PR) description

When setting up a read-only domain controller, it is possible to supply a user or group which will gain local administrative privileges to the RODC. The specified user or members of the specified group can perform operations on the RODC with privileges equivalent to the computer's Administrators group. They aren't members of the Domain Admins or domain built-in Administrators groups.

https://learn.microsoft.com/en-us/windows-server/identity/ad-ds/deploy/rodc/install-a-windows-server-2012-active-directory-read-only-domain-controller--rodc---level-200-#delegation-of-rodc-installation-and-administration

This can be selected during initial setup via the DelegatedAdministratorAccountName parameter, but also updated later using the ManagedBy attribute on the computer account in Active Directory. This pull request adds support for configuring this via PowerShell DSC using the ADDomainController resource.

https://devblogs.microsoft.com/scripting/weekend-scripter-use-powershell-to-delegate-administrator-of-rodcs/

This Pull Request (PR) fixes the following issues

None

Task list

• Added an entry to the change log under the Unreleased section of the file CHANGELOG.md.
  Entry should say what was changed and how that affects users (if applicable), and
  reference the issue being resolved (if applicable).
• Resource documentation added/updated in README.md.
• Resource parameter descriptions added/updated in README.md, schema.mof and comment-based
  help.
• Comment-based help added/updated.
• Localization strings added/updated in all localization files as appropriate.
• Examples appropriately added/updated.
• Unit tests added/updated. See DSC Community Testing Guidelines.
• Integration tests added/updated (where possible). See DSC Community Testing Guidelines.
• New/changed code adheres to DSC Community Style Guidelines.

---

This change is 

[johlju]

Reviewed 6 of 7 files at r1, all commit messages.
Reviewable status: 6 of 7 files reviewed, 3 unresolved discussions (waiting on @Borgquite)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

            {
                $domainControllerManagedByObject = $domainControllerComputerObject.ManagedBy | Get-ADObject -Properties SamAccountName -Credential $Credential
                if ($domainControllerManagedByObject.SamAccountName)

Should we only return SamAccountName here? It seems a bit less precise - what if the user passes DOMAIN\Group? Is it possible to pass distinguish name (DN) in the parameter so we always know the expected format that is passed so that we can return the correct value here?

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 277 at r1 (raw file):

        [Parameter()]
        [System.String]
        $DelegatedAdministratorAccountName,

I suggest we throw an error if this parameter it used without also passing ReadOnlyReplica (for both Test- and Set-TargetResource).

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 791 at r1 (raw file):

    # If this is a read-only domain controller, check the delegated administrator
    if ($PSBoundParameters.ContainsKey('DelegatedAdministratorAccountName') -and $existingResource.DelegatedAdministratorAccountName -ne $DelegatedAdministratorAccountName -and $existingResource.ReadOnlyReplica)

Can we move this evaluation into its own if-block prior to evaluating if ManagedBy match? No need to check anything unless it is a Read-only replica, and it makes this line a bit easier to read.

Code quote:

-and $existingResource.ReadOnlyReplica

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 98%. Comparing base (bdde66f) to head (7b33fc0).
Report is 1 commits behind head on main.

Additional details and impacted files

@@         Coverage Diff         @@
##           main   #709   +/-   ##
===================================
  Coverage    98%    98%           
===================================
  Files        25     25           
  Lines      3475   3512   +37     
===================================
+ Hits       3406   3443   +37     
  Misses       69     69           

Files	Coverage Δ
...FT_ADDomainController/MSFT_ADDomainController.psm1	100% <100%> (ø)

[Borgquite]

Reviewable status: 5 of 9 files reviewed, 3 unresolved discussions (waiting on @johlju)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Should we only return SamAccountName here? It seems a bit less precise - what if the user passes DOMAIN\Group? Is it possible to pass distinguish name (DN) in the parameter so we always know the expected format that is passed so that we can return the correct value here?

Difficult to know the exact formats that DelegatedAdministratorAccountName supports as running Install-ADDSDomainController to test is quite time consuming, and the documentation doesn't specify, but presuming that the parameter in Add-ADDSReadOnlyDomainControllerAccount is the same, according to my own testing on Server 2022 Standard:

SUPPORTED:
x samAccountName (for users & groups)
x DOMAIN\samAccountName (for users & groups)
x UserPrincipalName (users only- groups don't have this attribute)

NOT SUPPORTED (returns the error below):
x DistinguishedName
x ObjectGUID
x ObjectSid
'The validation of the account '' that was specified for the delegation of RODC administration failed. The error was: No mapping between account names and security IDs was done.'

I reckon DOMAIN\samAccountName is the way to go.

https://learn.microsoft.com/en-us/powershell/module/addsdeployment/install-addsdomaincontroller?view=windowsserver2022-ps
https://learn.microsoft.com/en-us/powershell/module/addsdeployment/add-addsreadonlydomaincontrolleraccount?view=windowsserver2022-ps

[Borgquite]

Reviewable status: 2 of 9 files reviewed, 3 unresolved discussions (waiting on @johlju)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 277 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

I suggest we throw an error if this parameter it used without also passing ReadOnlyReplica (for both Test- and Set-TargetResource).

OK - I'll aalso apply the same code to AllowPasswordReplicationAccountName and DenyPasswordReplicationAccountName as they also are RODC-specific.

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 791 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Can we move this evaluation into its own if-block prior to evaluating if ManagedBy match? No need to check anything unless it is a Read-only replica, and it makes this line a bit easier to read.

Done.

[Borgquite]

Reviewable status: 1 of 9 files reviewed, 3 unresolved discussions (waiting on @johlju)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

Previously, Borgquite (Chris Hill) wrote…

Difficult to know the exact formats that DelegatedAdministratorAccountName supports as running Install-ADDSDomainController to test is quite time consuming, and the documentation doesn't specify, but presuming that the parameter in Add-ADDSReadOnlyDomainControllerAccount is the same, according to my own testing on Server 2022 Standard:

SUPPORTED:
x samAccountName (for users & groups)
x DOMAIN\samAccountName (for users & groups)
x UserPrincipalName (users only- groups don't have this attribute)

NOT SUPPORTED (returns the error below):
x DistinguishedName
x ObjectGUID
x ObjectSid
'The validation of the account '' that was specified for the delegation of RODC administration failed. The error was: No mapping between account names and security IDs was done.'

I reckon DOMAIN\samAccountName is the way to go.

https://learn.microsoft.com/en-us/powershell/module/addsdeployment/install-addsdomaincontroller?view=windowsserver2022-ps
https://learn.microsoft.com/en-us/powershell/module/addsdeployment/add-addsreadonlydomaincontrolleraccount?view=windowsserver2022-ps

Appreciate some direction here. As far as I can see there are two ways to convert a DistinguishedName into an NTName:

One is using Get-ADObject - but this takes two steps with Get-ADObject and Resolve-SamAccountName, and I'll need to fiddle with the stubs under ActiveDirectory_2019.psm1 to get the unit tests working

There's a more direct way using the COM object NameTranslate - this is probably quicker with less moving parts, and might be more resilient for e.g. domain trust situations, but not sure how to mock it.

Advice?

Code snippet (i):

$distinguishedName = "CN=whatever,DC=contoso,DC=com"
$distinguishedName | Get-ADObject -Properties objectSid
$NTName = Resolve-SamAccountName -ObjectSid $domainControllerManagedByObject.objectSid

Code snippet (ii):

$distinguishedName = "CN=whatever,DC=contoso,DC=com"

$NameTranslate = New-Object -ComObject "NameTranslate"
[System.__ComObject].InvokeMember("Init", "InvokeMethod", $null, $NameTranslate, (3, $null))
[System.__ComObject].InvokeMember("Set", "InvokeMethod", $null, $NameTranslate, (1, $distinguishedname))
$NTName = [System.__ComObject].InvokeMember("Get", "InvokeMethod", $null, $NameTranslate, 3)

[Borgquite]

@johlju I hope this is all good to go now!

[johlju]

Will get back to as soon as I have time. It's on the todo list. 🙂

[Borgquite]

Hey @johlju, wondering if you're able to carve out some time for this yet? Appreciate how busy things can be! :)

[Borgquite]

@johlju Sorry to pester you as appreciate you've got a lot on - keen to try this in production :) Let me know if you have any time!

[johlju]

It is on my todo list, I haven't been able to carve out enough free time to do it yet. Will do as soon as possible.

If another community member have time to review then go for it. Then I can merge it too. 😊

[johlju]

Reviewed 3 of 4 files at r3, 2 of 3 files at r4, 3 of 3 files at r5, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @Borgquite)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

Previously, Borgquite (Chris Hill) wrote…

Appreciate some direction here. As far as I can see there are two ways to convert a DistinguishedName into an NTName:

One is using Get-ADObject - but this takes two steps with Get-ADObject and Resolve-SamAccountName, and I'll need to fiddle with the stubs under ActiveDirectory_2019.psm1 to get the unit tests working

There's a more direct way using the COM object NameTranslate - this is probably quicker with less moving parts, and might be more resilient for e.g. domain trust situations, but not sure how to mock it.

Advice?

Looks like you went with first alternativt, that good, the other one would have been a bit complex to test I think. Jsut a question. If the configuration has DelegatedAdministratorAccountName = 'contoso\adm.pvdi', will Resolve-SamAccountName also return 'contoso\adm.pvdi so that the Test-TargetResource returns $true when comparing the property?

[johlju]

@Borgquite one comment then I think this is ready to merge.

[Borgquite]

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @johlju)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

Previously, johlju (Johan Ljunggren) wrote…

Looks like you went with first alternativt, that good, the other one would have been a bit complex to test I think. Jsut a question. If the configuration has DelegatedAdministratorAccountName = 'contoso\adm.pvdi', will Resolve-SamAccountName also return 'contoso\adm.pvdi so that the Test-TargetResource returns $true when comparing the property?

Yes, Resolve-SamAccount uses the System.Security.Principal.SecurityIdentifier 'Translate' method to return an System.Security.Principal.NTAccount, and the NTAccount's ToString() method returns Domain\Account as needed by Test-TargetResource. Running the relevant code in my environment gave the desired results.

https://github.com/dsccommunity/ActiveDirectoryDsc/blob/9544987c7b3fa95a8f6a84dc272dab8540cf4c50/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1#L2611C20-L2611C29

https://learn.microsoft.com/en-us/dotnet/api/system.security.principal.ntaccount.tostring?view=net-8.0

[johlju]

Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @Borgquite)

---

source/DSCResources/MSFT_ADDomainController/MSFT_ADDomainController.psm1 line 86 at r1 (raw file):

Previously, Borgquite (Chris Hill) wrote…

Yes, Resolve-SamAccount uses the System.Security.Principal.SecurityIdentifier 'Translate' method to return an System.Security.Principal.NTAccount, and the NTAccount's ToString() method returns Domain\Account as needed by Test-TargetResource. Running the relevant code in my environment gave the desired results.

https://github.com/dsccommunity/ActiveDirectoryDsc/blob/9544987c7b3fa95a8f6a84dc272dab8540cf4c50/source/Modules/ActiveDirectoryDsc.Common/ActiveDirectoryDsc.Common.psm1#L2611C20-L2611C29

https://learn.microsoft.com/en-us/dotnet/api/system.security.principal.ntaccount.tostring?view=net-8.0

Great! Thanks for the detailed explanation.