terraform-provider-ad icon indicating copy to clipboard operation
terraform-provider-ad copied to clipboard

ad_group_membership does not quote parameters to Add-ADGroupMember

Open jfcantu opened this issue 3 years ago • 1 comments

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)

  • ad_group_membership

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

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

jfcantu avatar May 03 '21 19:05 jfcantu

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 ?

jpatigny avatar May 08 '21 09:05 jpatigny