hashicorp / terraform-provider-ad

Active Directory provider for HashiCorp Terraform (experimental)
https://registry.terraform.io/providers/hashicorp/ad/latest
Mozilla Public License 2.0
140 stars 72 forks source link

ad_group_membership does not quote parameters to Add-ADGroupMember #102

Open jfcantu opened 3 years ago

jfcantu commented 3 years ago

Terraform Version and Provider Version

TF: v0.15.1 Provider: v0.4.2

Windows Version

Local: Windows 10 Pro Remote: Windows Server 2016 Standard

Affected Resource(s)

Terraform Configuration Files

resource "ad_group_membership" "universal_group" {
  group_id = ad_group.u_group.id
  group_members = [
    "CN=example1,OU=Groups,DC=example,DC=com"
  ]
}

Expected Behavior

According to the provider documentation, group_members is "A list of member AD Principals. Each principal can be identified by its GUID, SID, Distinguished Name, or SAM Account Name."

Quotes are not part of a DN, and should not need to be included when a DN is provided to identify a group.

Actual Behavior

The provider calls the PowerShell cmdlet Add-ADGroupMember. According to the Add-ADGroupMember documentation, multiple parameters may be passed to the -Members argument as a comma-separated list.

Because DNs contain commas internally, this requires that they be quoted. However, the provider does not do this, either when the cmdlet arguments are constructed or when the group member list is converted to a comma-separated string.

2021-05-03T11:45:14.423-0700 [INFO] plugin.terraform-provider-ad_v0.4.2_x5.exe: 2021/05/03 11:45:14 [DEBUG] Running command Add-ADGroupMember -Identity "7e88fc82-b652-4007-8fef-f14893d32eae" -Members CN=example1,OU=Groups,DC=example,DC=com via powershell: timestamp=2021-05-03T11:45:14.422-0700

Steps to Reproduce

  1. Attempt to specify a DN as a group member of ad_group_membership, and run terraform apply.

Community Note

jpatigny commented 3 years ago

Hello,

I could submit a fix for this issue (add double quotes for each member) but unfortunately it will not really help because at next run, terraform will tell you that he has to replace members (refer to issue)

There is a reflection ongoing by @koikonom about how to handle this. In the meantime, to avoid endless membership replacement, I would advise you to use GUID for members instead of DN

Example

resource "ad_group" "g2" {
  name             = "tstgroup2"
  sam_account_name = "tstgroup2"
  container        = "OU=POC2,OU=Servers,DC=jej,DC=net"
}

resource "ad_group_membership" "universal_group" {
  group_id = ad_group.g.id
  group_members = [
    ad_group.g2.id
  ]
}

@koikonom , according to you, does it worth to make a fix for this one ?