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

azuread_group - Allow creation of group in administrative unit #984

Merged
merged 28 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
5191899
implement creation of group directly in au via property
SwissGipfel Jan 17, 2023
085b186
enable local debugging with provider
SwissGipfel Jan 17, 2023
3407c29
Rewrite retry logic
ccadruvi Jan 18, 2023
3de0543
update manicminer/hamilton dependency to v0.54.1 replacement
SwissGipfel Jan 18, 2023
bc06134
Add hasChange functionality
ccadruvi Jan 19, 2023
f4763c4
Explain why we modify timeout values
ccadruvi Jan 19, 2023
e219827
Revert main.go, link PR in hamilton
ccadruvi Jan 19, 2023
af2c2db
Revert newlines in groups.go
ccadruvi Jan 19, 2023
9d2c9b8
Add doc for au_id in group
ccadruvi Jan 19, 2023
9cdd2ad
enable resource read regarding administrative unit id
SwissGipfel Jan 19, 2023
87a7dcb
fix typo at endpoint
SwissGipfel Jan 19, 2023
22eef73
Add acceptance test
ccadruvi Jan 23, 2023
6d33a2c
Update description for au_id field
ccadruvi Jan 23, 2023
4286bc5
Remove unnecessary comments
ccadruvi Jan 23, 2023
b69398f
Rename function
ccadruvi Jan 23, 2023
e9c3898
Add missing error declaration
ccadruvi Jan 24, 2023
ce72835
Update vendor
ccadruvi Jan 25, 2023
e98888f
fix spelling error
SwissGipfel Jan 27, 2023
6f349bb
Update to new version of hamilton
ccadruvi Jan 30, 2023
78ce177
Review suggestions
ccadruvi Jan 30, 2023
1e61441
Allow multiple administrative units for a group
ccadruvi Feb 14, 2023
0691885
Clarify required permission for AUs
ccadruvi Feb 14, 2023
fdf4153
Move listAuMemberships to sdk, make tf declarative
ccadruvi Feb 15, 2023
acd0ae5
Docs tidy
manicminer Feb 21, 2023
a0ea72c
Groups: retrieve current AU memberships when updating
manicminer Feb 21, 2023
ca89966
Merge branch 'main' into create-group-in-au-directly
manicminer Feb 21, 2023
bea1be0
Update to Hamilton v0.57.0
manicminer Feb 21, 2023
feef509
Fix client version
manicminer Feb 21, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/resources/group.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ If specifying owners for a group, which are user principals, this resource addit

When authenticated with a user principal, this resource requires one of the following directory roles: `Groups Administrator`, `User Administrator` or `Global Administrator`

When creating this resource in administrative units exclusively, the role `Groups Administrator` is required to be scoped on any administrative unit used.

The `external_senders_allowed`, `auto_subscribe_new_members`, `hide_from_address_lists` and `hide_from_outlook_clients` properties can only be configured when authenticating as a user and cannot be configured when authenticating as a service principal. Additionally, the user being used for authentication must be a Member of the tenant where the group is being managed and _not_ a Guest. This is a known API issue; please see the [Microsoft Graph Known Issues](https://docs.microsoft.com/en-us/graph/known-issues#groups) official documentation.

## Example Usage
Expand Down Expand Up @@ -106,6 +108,10 @@ resource "azuread_group" "example" {

The following arguments are supported:

* `administrative_unit_ids` - (Optional) The object IDs of administrative units in which the group is a member. If specified, new groups will be created in the scope of the first administrative unit and added to the others. If empty, new groups will be created at the tenant level.

!> **Warning** Do not use the `administrative_unit_ids` property at the same time as the [azuread_administrative_unit_member](https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/administrative_unit_member) resource, or the `members` property of the [azuread_administrative_unit](https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/administrative_unit#members) resource, _for the same group_. Doing so will cause a conflict and administrative unit members will be removed.

* `assignable_to_role` - (Optional) Indicates whether this group can be assigned to an Azure Active Directory role. Can only be `true` for security-enabled groups. Changing this forces a new resource to be created.
* `auto_subscribe_new_members` - (Optional) Indicates whether new members added to the group will be auto-subscribed to receive email notifications. Can only be set for Unified groups.

Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ require (
github.com/hashicorp/go-cty v1.4.1-0.20200414143053-d3edf31b6320
github.com/hashicorp/go-uuid v1.0.3
github.com/hashicorp/terraform-plugin-sdk/v2 v2.24.0
github.com/manicminer/hamilton v0.54.0
github.com/manicminer/hamilton v0.57.0
golang.org/x/text v0.3.7
)

Expand Down Expand Up @@ -54,6 +54,7 @@ require (
google.golang.org/genproto v0.0.0-20220407144326-9054f6ed7bac // indirect
google.golang.org/grpc v1.50.1 // indirect
google.golang.org/protobuf v1.28.1 // indirect
software.sslmate.com/src/go-pkcs12 v0.2.0 // indirect
)

go 1.19
8 changes: 5 additions & 3 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -224,8 +224,8 @@ github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY=
github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
github.com/manicminer/hamilton v0.54.0 h1:vnjgE7Eer9dnrAd78qZHPK6CTNCwwFmuVtvBjjl4OtY=
github.com/manicminer/hamilton v0.54.0/go.mod h1:lbVyngC+/nCWuDp8UhC6Bw+bh7jcP/E+YwqzHTmzemk=
github.com/manicminer/hamilton v0.57.0 h1:H+p4l7gfI4Fc6t0NGrJE0g6vAcPSw8gyjT1nQeSZrfQ=
github.com/manicminer/hamilton v0.57.0/go.mod h1:fFR5k3IJ/QCsEgT9TsD9K2l2dv2tmk7Qpeze1hsUJH4=
github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA=
github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
Expand Down Expand Up @@ -303,7 +303,7 @@ golang.org/x/crypto v0.0.0-20200622213623-75b288015ac9/go.mod h1:LzIPMQfyMNhhGPh
golang.org/x/crypto v0.0.0-20210322153248-0c34fe9e7dc2/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
golang.org/x/crypto v0.0.0-20210421170649-83a5a9bb288b/go.mod h1:T9bdIzuCu7OtxOm1hfPfRQxPLYneinmdGuTeoZ9dtd4=
golang.org/x/crypto v0.0.0-20210616213533-5ff15b29337e/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/crypto v0.0.0-20211215153901-e495a2d5b3d3/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220331220935-ae2d96664a29/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167 h1:O8uGbHCqlTp2P6QJSLmCojM4mN6UemYv8K+dCnmHmu0=
golang.org/x/crypto v0.0.0-20220517005047-85d78b3ac167/go.mod h1:IxCIyHEi3zRg3s0A5j5BB6A9Jmi73HwBIUl50j+osU4=
golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
Expand Down Expand Up @@ -602,3 +602,5 @@ honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=
rsc.io/sampler v1.3.0/go.mod h1:T1hPZKmBbMNahiBKFy5HrXp6adAjACjK9JXDnKaTXpA=
software.sslmate.com/src/go-pkcs12 v0.2.0 h1:nlFkj7bTysH6VkC4fGphtjXRbezREPgrHuJG20hBGPE=
software.sslmate.com/src/go-pkcs12 v0.2.0/go.mod h1:23rNcYsMabIc1otwLpTkCCPwUq6kQsTyowttG/as0kQ=
13 changes: 9 additions & 4 deletions internal/services/groups/client/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,15 @@ import (
)

type Client struct {
DirectoryObjectsClient *msgraph.DirectoryObjectsClient
GroupsClient *msgraph.GroupsClient
AdministrativeUnitsClient *msgraph.AdministrativeUnitsClient
DirectoryObjectsClient *msgraph.DirectoryObjectsClient
GroupsClient *msgraph.GroupsClient
}

func NewClient(o *common.ClientOptions) *Client {
administrativeUnitsClient := msgraph.NewAdministrativeUnitsClient(o.TenantID)
o.ConfigureClient(&administrativeUnitsClient.BaseClient)

directoryObjectsClient := msgraph.NewDirectoryObjectsClient(o.TenantID)
o.ConfigureClient(&directoryObjectsClient.BaseClient)

Expand All @@ -22,7 +26,8 @@ func NewClient(o *common.ClientOptions) *Client {
groupsClient.BaseClient.ApiVersion = msgraph.VersionBeta

return &Client{
DirectoryObjectsClient: directoryObjectsClient,
GroupsClient: groupsClient,
AdministrativeUnitsClient: administrativeUnitsClient,
DirectoryObjectsClient: directoryObjectsClient,
GroupsClient: groupsClient,
}
}
112 changes: 109 additions & 3 deletions internal/services/groups/group_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,16 @@ func groupResource() *schema.Resource {
ValidateDiagFunc: validate.NoEmptyStrings,
},

"administrative_unit_ids": {
Description: "The administrative unit IDs in which the group should be. If empty, the group will be created at the tenant level.",
Type: schema.TypeSet,
Optional: true,
Elem: &schema.Schema{
Type: schema.TypeString,
ValidateFunc: validation.IsUUID,
},
},

"assignable_to_role": {
Description: "Indicates whether this group can be assigned to an Azure Active Directory role. This property can only be `true` for security-enabled groups.",
Type: schema.TypeBool,
Expand Down Expand Up @@ -406,6 +416,7 @@ func groupResourceCustomizeDiff(ctx context.Context, diff *schema.ResourceDiff,
func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Groups.GroupsClient
directoryObjectsClient := meta.(*clients.Client).Groups.DirectoryObjectsClient
administrativeUnitsClient := meta.(*clients.Client).Groups.AdministrativeUnitsClient
callerId := meta.(*clients.Client).ObjectID

displayName := d.Get("display_name").(string)
Expand Down Expand Up @@ -451,7 +462,12 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter

description := d.Get("description").(string)

odataType := odata.TypeGroup

properties := msgraph.Group{
DirectoryObject: msgraph.DirectoryObject{
ODataType: &odataType,
},
Description: utils.NullableString(description),
DisplayName: utils.String(displayName),
GroupTypes: &groupTypes,
Expand Down Expand Up @@ -566,9 +582,30 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
// Set the initial owners, which either be the calling principal, or up to 20 of the owners specified in configuration
properties.Owners = &ownersFirst20

group, _, err := client.Create(ctx, properties)
if err != nil {
return tf.ErrorDiagF(err, "Creating group %q", displayName)
var group *msgraph.Group
var err error

if v, ok := d.GetOk("administrative_unit_ids"); ok {
administrativeUnitIds := tf.ExpandStringSlice(v.(*schema.Set).List())
for i, administrativeUnitId := range administrativeUnitIds {
// Create the group in the first administrative unit, as this requires fewer permissions than creating it at tenant level
if i == 0 {
group, _, err = administrativeUnitsClient.CreateGroup(ctx, administrativeUnitId, &properties)
if err != nil {
return tf.ErrorDiagF(err, "Creating group in administrative unit with ID %q, %q", administrativeUnitId, displayName)
}
} else {
err := addGroupToAdministrativeUnit(ctx, administrativeUnitsClient, administrativeUnitId, group)
if err != nil {
return tf.ErrorDiagF(err, "Could not add group %q to administrative unit with object ID: %q", *group.ID(), administrativeUnitId)
}
}
}
} else {
group, _, err = client.Create(ctx, properties)
if err != nil {
return tf.ErrorDiagF(err, "Creating group %q", displayName)
}
}

if group.ID() == nil {
Expand Down Expand Up @@ -795,12 +832,21 @@ func groupResourceCreate(ctx context.Context, d *schema.ResourceData, meta inter
}
}

// We have observed that when creating a group with an administrative_unit_id and querying the group with the /groups endpoint and specifying $select=allowExternalSenders,autoSubscribeNewMembers,hideFromAddressLists,hideFromOutlookClients, it returns a 404 for ~11 minutes.
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved
if _, ok := d.GetOk("administrative_unit_ids"); ok {
meta.(*clients.Client).Groups.GroupsClient.BaseClient.DisableRetries = false
meta.(*clients.Client).Groups.GroupsClient.BaseClient.RetryableClient.RetryWaitMax = 1 * time.Minute
meta.(*clients.Client).Groups.GroupsClient.BaseClient.RetryableClient.RetryWaitMin = 10 * time.Second
meta.(*clients.Client).Groups.GroupsClient.BaseClient.RetryableClient.RetryMax = 15
}

return groupResourceRead(ctx, d, meta)
}

func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
client := meta.(*clients.Client).Groups.GroupsClient
directoryObjectsClient := meta.(*clients.Client).Groups.DirectoryObjectsClient
administrativeUnitClient := meta.(*clients.Client).Groups.AdministrativeUnitsClient
callerId := meta.(*clients.Client).ObjectID

groupId := d.Id()
Expand Down Expand Up @@ -1065,6 +1111,40 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
}
}

if v := d.Get("administrative_unit_ids"); d.HasChange("administrative_unit_ids") {
administrativeUnits, _, err := client.ListAdministrativeUnitMemberships(ctx, *group.ID())
if err != nil {
return tf.ErrorDiagPathF(err, "administrative_units", "Could not retrieve administrative units for group with object ID %q", d.Id())
}

var existingAdministrativeUnits []string
for _, administrativeUnit := range *administrativeUnits {
existingAdministrativeUnits = append(existingAdministrativeUnits, *administrativeUnit.ID)
}

desiredAdministrativeUnits := tf.ExpandStringSlice(v.(*schema.Set).List())
administrativeUnitsToLeave := utils.Difference(existingAdministrativeUnits, desiredAdministrativeUnits)
administrativeUnitsToJoin := utils.Difference(desiredAdministrativeUnits, existingAdministrativeUnits)

if len(administrativeUnitsToJoin) > 0 {
for _, newAdministrativeUnitId := range administrativeUnitsToJoin {
err := addGroupToAdministrativeUnit(ctx, administrativeUnitClient, newAdministrativeUnitId, &group)
if err != nil {
return tf.ErrorDiagF(err, "Could not add group %q to administrative unit with object ID: %q", *group.ID(), newAdministrativeUnitId)
}
}
}

if len(administrativeUnitsToLeave) > 0 {
for _, oldAdministrativeUnitId := range administrativeUnitsToLeave {
memberIds := []string{d.Id()}
if _, err := administrativeUnitClient.RemoveMembers(ctx, oldAdministrativeUnitId, &memberIds); err != nil {
return tf.ErrorDiagF(err, "Could not remove group from administrative unit with object ID: %q", oldAdministrativeUnitId)
}
}
}
}

return groupResourceRead(ctx, d, meta)
}

Expand Down Expand Up @@ -1153,6 +1233,22 @@ func groupResourceRead(ctx context.Context, d *schema.ResourceData, meta interfa
}
tf.Set(d, "members", members)

administrativeUnits, _, err := client.ListAdministrativeUnitMemberships(ctx, *group.ID())
if err != nil {
return tf.ErrorDiagPathF(err, "administrative_units", "Could not retrieve administrative units for group with object ID %q", d.Id())
}

auIdMembers := make([]string, 0)
for _, administrativeUnit := range *administrativeUnits {
auIdMembers = append(auIdMembers, *administrativeUnit.ID)
}

if len(auIdMembers) > 0 {
tf.Set(d, "administrative_unit_ids", &auIdMembers)
} else {
tf.Set(d, "administrative_unit_ids", nil)
}

preventDuplicates := false
if v := d.Get("prevent_duplicate_names").(bool); v {
preventDuplicates = v
Expand Down Expand Up @@ -1194,3 +1290,13 @@ func groupResourceDelete(ctx context.Context, d *schema.ResourceData, meta inter

return nil
}

func addGroupToAdministrativeUnit(ctx context.Context, auClient *msgraph.AdministrativeUnitsClient, administrativeUnitId string, group *msgraph.Group) error {
members := msgraph.Members{
group.DirectoryObject,
}
members[0].ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
auClient.BaseClient.Endpoint, auClient.BaseClient.TenantId, *group.DirectoryObject.ID())))
_, err := auClient.AddMembers(ctx, administrativeUnitId, &members)
return err
}
67 changes: 67 additions & 0 deletions internal/services/groups/group_resource_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -434,6 +434,38 @@ func TestAccGroup_visibility(t *testing.T) {
})
}

func TestAccGroup_administrativeUnit(t *testing.T) {
data := acceptance.BuildTestData(t, "azuread_group", "test")
r := GroupResource{}

data.ResourceTest(t, r, []resource.TestStep{
{
Config: r.administrativeUnits(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_ids.#").HasValue("2"),
),
},
data.ImportStep("administrative_unit_ids"),
{
Config: r.administrativeUnitsWithoutAssociation(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_ids.#").HasValue("0"),
),
},
data.ImportStep("administrative_unit_ids"),
{
Config: r.administrativeUnits(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_ids.#").HasValue("2"),
),
},
data.ImportStep("administrative_unit_ids"),
})
}

func (r GroupResource) Exists(ctx context.Context, clients *clients.Client, state *terraform.InstanceState) (*bool, error) {
client := clients.Groups.GroupsClient
client.BaseClient.DisableRetries = true
Expand Down Expand Up @@ -925,3 +957,38 @@ resource "azuread_group" "test" {
}
`, data.RandomInteger)
}

func (r GroupResource) administrativeUnits(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}
resource "azuread_administrative_unit" "test2" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}
resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
administrative_unit_ids = [azuread_administrative_unit.test.id, azuread_administrative_unit.test2.id]
}
`, data.RandomInteger, data.RandomInteger)
}

func (r GroupResource) administrativeUnitsWithoutAssociation(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}
resource "azuread_administrative_unit" "test2" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}
resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
}
`, data.RandomInteger, data.RandomInteger)
}
4 changes: 2 additions & 2 deletions vendor/github.com/manicminer/hamilton/auth/auth.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading