dsccommunity / ActiveDirectoryDsc

This module contains DSC resources for deployment and configuration of Active Directory Domain Services.
MIT License
344 stars 142 forks source link

ADGroup Members consequences should be highly remarked #610

Closed DanteNahuel closed 4 years ago

DanteNahuel commented 4 years ago

Details of the scenario you tried and the problem that is occurring

I'm testing ADGroup to not only create groups but also to add computers and users as members to previously existing AD Groups. However, when testing the configurations I found that members is not only adding the new members but deleting all members not mentioned in the DSC configuration.

After some testing I found out that what I needed to use was MembersToInclude and not Members. However the documentation, at least in my opinion, is not clear enough between these two and this can cause serious damages to a domain.

"Members" descriptoin should highly state that the entire existing AD Group membership will be replaced with the one in the DSC configuration and that it shouldn't be used unless strickly confident on what they are doing.

I'd even go beyond that and recommend adding a parameter that explicitly requires to be "true" to override all settings if the AD Group already exists, instead of just doing it intrinsically. Something like ReplaceExisting: $true or something like that.

I'm glad I tested this many times before applying it and now I'm even hesitant of using it at all on old pre existing AD Groups, even with MembersToInclude. I know testing should be part of everyone's best practices, but someone might not do it and screw things up

The DSC configuration that is used to reproduce the issue (as detailed as possible)

        ADGroup ADGroup1
        {
            GroupName  = $group1
            Members    = "$vmname$"
            Credential = $SecureCred
        }
X-Guardian commented 4 years ago

The documention for the ADGroup resource is on the ADGroup page of the Wiki and is quite clear about the three different member properties.

All the resources in this module have the ability to cause damage to an Active Directory if used incorrectly, so should always be tested first on a test Active Directory to understand their use.

briantist commented 4 years ago

I have to agree with @X-Guardian , I think the documentation is clear.

I also want to point out that typically in DSC you're looking for a specific state, and the behavior of Members follows that most closely.

MembersToInclude and MembersToExclude are sort of murky in terms of idempotence and are really the odd ones out (to be clear I think they're incredibly useful and needed though; it's difficult to manage AD and AD objects in a completely idempotent way; there's value in this approach of being able to ensure state of only certain members).

DanteNahuel commented 4 years ago

I guess it's only me, then. Although I did test everything and didn' "break" anything in my environment, I was confused at first on what was happening and, once I understood it, I got the chills when thinking of what could have happened if I didn't check every angle of it. But, again, could be just me. Closing this, thanks for your replies.