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 18 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
1 change: 1 addition & 0 deletions docs/data-sources/group.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ The following arguments are supported:

The following attributes are exported:

* `administrative_unit_id` - The optional administrative unit ID that the group is member of.
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved
* `assignable_to_role` - Indicates whether this group can be assigned to an Azure Active Directory role.
* `auto_subscribe_new_members` - Indicates whether new members added to the group will be auto-subscribed to receive email notifications. Only set for Unified groups.
* `behaviors` - A list of behaviors for a Microsoft 365 group, such as `AllowOnlyMembersToPost`, `HideGroupInOutlook`, `SubscribeNewGroupMembers` and `WelcomeEmailDisabled`. See [official documentation](https://docs.microsoft.com/en-us/graph/group-set-options) for more details.
Expand Down
1 change: 1 addition & 0 deletions docs/resources/group.md
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ resource "azuread_group" "example" {

The following arguments are supported:

* `administrative_unit_id` - (Optional) The administrative unit ID that the group is member of.
manicminer marked this conversation as resolved.
Show resolved Hide resolved
* `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: 3 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -57,3 +57,6 @@ require (
)

go 1.19

// can be removed once https://github.com/manicminer/hamilton/pull/206 is merged
replace github.com/manicminer/hamilton => github.com/SwissGipfel/hamilton v0.54.1
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ github.com/Microsoft/go-winio v0.4.16 h1:FtSW/jqD+l4ba5iPBj9CODVtgfYAD8w2wS923g/
github.com/Microsoft/go-winio v0.4.16/go.mod h1:XB6nPKklQyQ7GC9LdcBEcBl8PF76WugXOPRXwdLnMv0=
github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7 h1:YoJbenK9C67SkzkDfmQuVln04ygHj3vjZfd9FL+GmQQ=
github.com/ProtonMail/go-crypto v0.0.0-20210428141323-04723f9f07d7/go.mod h1:z4/9nQmJSSwwds7ejkxaJwO37dru3geImFUdJlaLzQo=
github.com/SwissGipfel/hamilton v0.54.1 h1:+/Arp5U30HP6urFUHe5AE18EnKTHxYyQTj8EXQtwexQ=
github.com/SwissGipfel/hamilton v0.54.1/go.mod h1:lbVyngC+/nCWuDp8UhC6Bw+bh7jcP/E+YwqzHTmzemk=
github.com/acomagu/bufpipe v1.0.3 h1:fxAGrHZTgQ9w5QqVItgzwj235/uYZYgbXitB+dLupOk=
github.com/acomagu/bufpipe v1.0.3/go.mod h1:mxdxdup/WdsKVreO5GpW4+M/1CE2sMG4jeGJ2sYmHc4=
github.com/agext/levenshtein v1.2.3 h1:YB2fHEn0UJagG8T1rrWknE3ZQzWM06O8AMAatNn7lmo=
Expand Down Expand Up @@ -224,8 +226,6 @@ 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/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
79 changes: 76 additions & 3 deletions internal/services/groups/group_resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,12 @@ func groupResource() *schema.Resource {
ValidateDiagFunc: validate.NoEmptyStrings,
},

"administrative_unit_id": {
Description: "The administrative unit ID in which the group should be created. If empty, the group will be created at the tenant level.",
Type: schema.TypeString,
Optional: true,
},

"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 @@ -451,7 +457,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 +577,25 @@ 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_id"); ok {
auClient := meta.(*clients.Client).AdministrativeUnits.AdministrativeUnitsClient

var status int
group, status, err = auClient.CreateGroup(ctx, v.(string), &properties)
if err != nil {
return tf.ErrorDiagF(err, "Creating group in administrative unit with ID %q, %q", v.(string), displayName)
}
if status != http.StatusCreated {
return tf.ErrorDiagF(err, "Invalid status code after creating group %q", displayName)
}
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved
} 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,6 +822,14 @@ 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_id"); 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
}
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved

return groupResourceRead(ctx, d, meta)
}

Expand Down Expand Up @@ -1065,6 +1100,29 @@ func groupResourceUpdate(ctx context.Context, d *schema.ResourceData, meta inter
}
}

if d.HasChange("administrative_unit_id") {
administrativeUnitClient := meta.(*clients.Client).AdministrativeUnits.AdministrativeUnitsClient
o, n := d.GetChange("administrative_unit_id")
oldAdministrativeUnitId := o.(string)
newAdministrativeUnitId := n.(string)
if newAdministrativeUnitId != "" {
members := msgraph.Members{
group.DirectoryObject,
}
members[0].ODataId = (*odata.Id)(utils.String(fmt.Sprintf("%s/v1.0/%s/directoryObjects/%s",
client.BaseClient.Endpoint, client.BaseClient.TenantId, d.Id())))
if _, err := administrativeUnitClient.AddMembers(ctx, newAdministrativeUnitId, &members); err != nil {
return tf.ErrorDiagF(err, "Could not add group to administrative unit with object ID: %q", newAdministrativeUnitId)
}
}
if oldAdministrativeUnitId != "" {
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)
}
}
manicminer marked this conversation as resolved.
Show resolved Hide resolved
}

return groupResourceRead(ctx, d, meta)
}

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

auId := d.Get("administrative_unit_id")
if auId.(string) != "" {
isMemberOfAu, _, err := isGroupInAdministrativeUnit(ctx, client, auId.(string), *group.ID())
if err != nil {
return tf.ErrorDiagPathF(err, "administrativeUnit", "Could not retrieve administrative Unit where this group belongs to %q", d.Id())
}
if isMemberOfAu {
tf.Set(d, "administrative_unit_id", auId)
} else {
tf.Set(d, "administrative_unit_id", nil)
}
} else {
tf.Set(d, "administrative_unit_id", nil)
}
ccadruvi marked this conversation as resolved.
Show resolved Hide resolved

preventDuplicates := false
if v := d.Get("prevent_duplicate_names").(bool); v {
preventDuplicates = v
Expand Down
59 changes: 59 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.administrativeUnit(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_id").Exists(),
),
},
data.ImportStep("administrative_unit_id"),
{
Config: r.administrativeUnitWithoutAssociation(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_id").IsEmpty(),
),
},
data.ImportStep("administrative_unit_id"),
{
Config: r.administrativeUnit(data),
Check: resource.ComposeTestCheckFunc(
check.That(data.ResourceName).ExistsInAzure(r),
check.That(data.ResourceName).Key("administrative_unit_id").Exists(),
),
},
data.ImportStep("administrative_unit_id"),
})
}

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,30 @@ resource "azuread_group" "test" {
}
`, data.RandomInteger)
}

func (r GroupResource) administrativeUnit(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
administrative_unit_id = azuread_administrative_unit.test.id
}
`, data.RandomInteger)
}

func (r GroupResource) administrativeUnitWithoutAssociation(data acceptance.TestData) string {
return fmt.Sprintf(`
resource "azuread_administrative_unit" "test" {
display_name = "acctestGroup-administrative-unit-%[1]d"
}

resource "azuread_group" "test" {
display_name = "acctestGroup-%[1]d"
security_enabled = true
}
`, data.RandomInteger)
}
42 changes: 42 additions & 0 deletions internal/services/groups/groups.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ package groups

import (
"context"
"encoding/json"
"fmt"
"io"
"math/rand"
"net/http"
"time"

"github.com/manicminer/hamilton/msgraph"
Expand Down Expand Up @@ -42,6 +45,45 @@ func groupFindByName(ctx context.Context, client *msgraph.GroupsClient, displayN
return &result, nil
}

func isGroupInAdministrativeUnit(ctx context.Context, client *msgraph.GroupsClient, auId string, id string) (bool, int, error) {
resp, status, _, err := client.BaseClient.Get(ctx, msgraph.GetHttpRequestInput{
ConsistencyFailureFunc: msgraph.RetryOn404ConsistencyFailureFunc,
OData: odata.Query{
Select: []string{"id"},
},
ValidStatusCodes: []int{http.StatusOK},
Uri: msgraph.Uri{
Entity: fmt.Sprintf("/groups/%s/memberOf/microsoft.graph.administrativeUnit", id),
HasTenantId: true,
},
})
if err != nil {
return false, status, fmt.Errorf("GroupsClient.BaseClient.Get(): %v", err)
}
defer resp.Body.Close()
respBody, err := io.ReadAll(resp.Body)
if err != nil {
return false, status, fmt.Errorf("io.ReadAll(): %v", err)
}

var data struct {
Members []struct {
Id string `json:"id"`
} `json:"value"`
}
if err := json.Unmarshal(respBody, &data); err != nil {
return false, status, fmt.Errorf("json.Unmarshal(): %v", err)
}

for _, v := range data.Members {
if v.Id == auId {
return true, status, nil
}
}

return false, status, nil
}

manicminer marked this conversation as resolved.
Show resolved Hide resolved
func groupGetAdditional(ctx context.Context, client *msgraph.GroupsClient, id string) (*msgraph.Group, error) {
query := odata.Query{Select: []string{"allowExternalSenders", "autoSubscribeNewMembers", "hideFromAddressLists", "hideFromOutlookClients"}}
groupExtra, _, err := client.Get(ctx, id, query)
Expand Down

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

3 changes: 2 additions & 1 deletion vendor/modules.txt
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ github.com/hashicorp/terraform-svchost
# github.com/hashicorp/yamux v0.0.0-20211028200310-0bc27b27de87
## explicit; go 1.15
github.com/hashicorp/yamux
# github.com/manicminer/hamilton v0.54.0
# github.com/manicminer/hamilton v0.54.0 => github.com/SwissGipfel/hamilton v0.54.1
## explicit; go 1.16
github.com/manicminer/hamilton/auth
github.com/manicminer/hamilton/environments
Expand Down Expand Up @@ -361,3 +361,4 @@ google.golang.org/protobuf/types/known/anypb
google.golang.org/protobuf/types/known/durationpb
google.golang.org/protobuf/types/known/emptypb
google.golang.org/protobuf/types/known/timestamppb
# github.com/manicminer/hamilton => github.com/SwissGipfel/hamilton v0.54.1