diff --git a/azuread/data_group.go b/azuread/data_group.go index 5e3ccdccb5..779139724d 100644 --- a/azuread/data_group.go +++ b/azuread/data_group.go @@ -34,6 +34,18 @@ func dataGroup() *schema.Resource { ValidateFunc: validate.NoEmptyStrings, ConflictsWith: []string{"object_id"}, }, + + "members": { + Type: schema.TypeList, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, + + "owners": { + Type: schema.TypeList, + Computed: true, + Elem: &schema.Schema{Type: schema.TypeString}, + }, }, } } @@ -73,5 +85,18 @@ func dataSourceActiveDirectoryGroupRead(d *schema.ResourceData, meta interface{} d.Set("object_id", group.ObjectID) d.Set("name", group.DisplayName) + + members, err := graph.GroupAllMembers(client, ctx, d.Id()) + if err != nil { + return err + } + d.Set("members", members) + + owners, err := graph.GroupAllOwners(client, ctx, d.Id()) + if err != nil { + return err + } + d.Set("owners", owners) + return nil } diff --git a/azuread/data_group_test.go b/azuread/data_group_test.go index 3953ab0b80..b4e22f3180 100644 --- a/azuread/data_group_test.go +++ b/azuread/data_group_test.go @@ -4,6 +4,7 @@ import ( "fmt" "testing" + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/helper/resource" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/tf" ) @@ -17,9 +18,6 @@ func TestAccDataSourceAzureADGroup_byName(t *testing.T) { Providers: testAccProviders, CheckDestroy: testCheckAzureADGroupDestroy, Steps: []resource.TestStep{ - { - Config: testAccAzureADGroup_basic(id), - }, { Config: testAccDataSourceAzureADGroup_name(id), Check: resource.ComposeTestCheckFunc( @@ -41,13 +39,54 @@ func TestAccDataSourceAzureADGroup_byObjectId(t *testing.T) { CheckDestroy: testCheckAzureADGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureADGroup_basic(id), + Config: testAccDataSourceAzureADGroup_objectId(id), + Check: resource.ComposeTestCheckFunc( + testCheckAzureADGroupExists(dsn), + resource.TestCheckResourceAttr(dsn, "name", fmt.Sprintf("acctestGroup-%d", id)), + ), }, + }, + }) +} + +func TestAccDataSourceAzureADGroup_members(t *testing.T) { + dsn := "data.azuread_group.test" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ { - Config: testAccDataSourceAzureADGroup_objectId(id), + Config: testAccDataSourceAzureADGroup_members(id, pw), + Check: resource.ComposeTestCheckFunc( + testCheckAzureADGroupExists(dsn), + resource.TestCheckResourceAttr(dsn, "name", fmt.Sprintf("acctestGroup-%d", id)), + resource.TestCheckResourceAttr(dsn, "members.#", "3"), + ), + }, + }, + }) +} + +func TestAccDataSourceAzureADGroup_owners(t *testing.T) { + dsn := "data.azuread_group.test" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccDataSourceAzureADGroup_owners(id, pw), Check: resource.ComposeTestCheckFunc( testCheckAzureADGroupExists(dsn), resource.TestCheckResourceAttr(dsn, "name", fmt.Sprintf("acctestGroup-%d", id)), + resource.TestCheckResourceAttr(dsn, "owners.#", "3"), ), }, }, @@ -73,3 +112,23 @@ data "azuread_group" "test" { } `, testAccAzureADGroup_basic(id)) } + +func testAccDataSourceAzureADGroup_members(id int, password string) string { + return fmt.Sprintf(` +%s + +data "azuread_group" "test" { + object_id = "${azuread_group.test.object_id}" +} +`, testAccAzureADGroupWithThreeMembers(id, password)) +} + +func testAccDataSourceAzureADGroup_owners(id int, password string) string { + return fmt.Sprintf(` +%s + +data "azuread_group" "test" { + object_id = "${azuread_group.test.object_id}" +} +`, testAccAzureADGroupWithThreeOwners(id, password)) +} diff --git a/azuread/data_users_test.go b/azuread/data_users_test.go index b24f12c71d..13ba9b7d3c 100644 --- a/azuread/data_users_test.go +++ b/azuread/data_users_test.go @@ -57,7 +57,7 @@ func testAccAzureADUsersDataSource_byUserPrincipalNames(id int, password string) data "azuread_users" "test" { user_principal_names = ["${azuread_user.testA.user_principal_name}", "${azuread_user.testB.user_principal_name}"] } -`, testAccADUser_multiple(id, password)) +`, testAccADUser_threeUsersABC(id, password)) } func testAccAzureADUsersDataSource_byObjectIds(id int, password string) string { @@ -67,5 +67,5 @@ func testAccAzureADUsersDataSource_byObjectIds(id int, password string) string { data "azuread_users" "test" { object_ids = ["${azuread_user.testA.object_id}", "${azuread_user.testB.object_id}"] } -`, testAccADUser_multiple(id, password)) +`, testAccADUser_threeUsersABC(id, password)) } diff --git a/azuread/helpers/ar/response.go b/azuread/helpers/ar/response.go index 9dc805c874..1f557af964 100644 --- a/azuread/helpers/ar/response.go +++ b/azuread/helpers/ar/response.go @@ -9,7 +9,7 @@ import ( //todo move to azure helpers func ResponseWasNotFound(resp autorest.Response) bool { - return responseWasStatusCode(resp, http.StatusNotFound) + return ResponseWasStatusCode(resp, http.StatusNotFound) } func ResponseErrorIsRetryable(err error) bool { @@ -27,7 +27,7 @@ func ResponseErrorIsRetryable(err error) bool { return false } -func responseWasStatusCode(resp autorest.Response, statusCode int) bool { // nolint: unparam +func ResponseWasStatusCode(resp autorest.Response, statusCode int) bool { // nolint: unparam if r := resp.Response; r != nil { if r.StatusCode == statusCode { return true diff --git a/azuread/helpers/graph/credentials.go b/azuread/helpers/graph/credentials.go index ca7a744568..16c6cc87bf 100644 --- a/azuread/helpers/graph/credentials.go +++ b/azuread/helpers/graph/credentials.go @@ -90,7 +90,7 @@ func ParsePasswordCredentialId(id string) (PasswordCredentialId, error) { } if _, err := uuid.ParseUUID(parts[1]); err != nil { - return PasswordCredentialId{}, fmt.Errorf("Object ID isn't a valid UUID (%q): %+v", id[1], err) + return PasswordCredentialId{}, fmt.Errorf("Credential ID isn't a valid UUID (%q): %+v", id[1], err) } return PasswordCredentialId{ diff --git a/azuread/helpers/graph/group.go b/azuread/helpers/graph/group.go index 222373f903..abbe90a97a 100644 --- a/azuread/helpers/graph/group.go +++ b/azuread/helpers/graph/group.go @@ -8,6 +8,33 @@ import ( "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" ) +type GroupMemberId struct { + ObjectSubResourceId + GroupId string + MemberId string +} + +func GroupMemberIdFrom(groupId, memberId string) GroupMemberId { + return GroupMemberId{ + ObjectSubResourceId: ObjectSubResourceIdFrom(groupId, "member", memberId), + GroupId: groupId, + MemberId: memberId, + } +} + +func ParseGroupMemberId(idString string) (GroupMemberId, error) { + id, err := ParseObjectSubResourceId(idString, "member") + if err != nil { + return GroupMemberId{}, fmt.Errorf("Unable to parse Member ID: %v", err) + } + + return GroupMemberId{ + ObjectSubResourceId: id, + GroupId: id.objectId, + MemberId: id.subId, + }, nil +} + func GroupGetByDisplayName(client *graphrbac.GroupsClient, ctx context.Context, displayName string) (*graphrbac.ADGroup, error) { filter := fmt.Sprintf("displayName eq '%s'", displayName) @@ -39,41 +66,49 @@ func GroupGetByDisplayName(client *graphrbac.GroupsClient, ctx context.Context, return &group, nil } -func GroupAllMembers(client graphrbac.GroupsClient, ctx context.Context, groupId string) ([]string, error) { - it, err := client.GetGroupMembersComplete(ctx, groupId) +func DirectoryObjectListToIDs(objects graphrbac.DirectoryObjectListResultIterator, ctx context.Context) ([]string, error) { + ids := make([]string, 0) + for objects.NotDone() { + v := objects.Value() - if err != nil { - return nil, fmt.Errorf("Error listing existing group members from Azure AD Group with ID %q: %+v", groupId, err) - } - - existingMembers := make([]string, 0) - - var memberObjectID string - for it.NotDone() { // possible members are users, groups or service principals // we try to 'cast' each result as the corresponding type and diff // if we found the object we're looking for - user, _ := it.Value().AsUser() + user, _ := v.AsUser() if user != nil { - memberObjectID = *user.ObjectID + ids = append(ids, *user.ObjectID) } - group, _ := it.Value().AsADGroup() + group, _ := v.AsADGroup() if group != nil { - memberObjectID = *group.ObjectID + ids = append(ids, *group.ObjectID) } - servicePrincipal, _ := it.Value().AsServicePrincipal() + servicePrincipal, _ := v.AsServicePrincipal() if servicePrincipal != nil { - memberObjectID = *servicePrincipal.ObjectID + ids = append(ids, *servicePrincipal.ObjectID) } - existingMembers = append(existingMembers, memberObjectID) - if err := it.NextWithContext(ctx); err != nil { - return nil, fmt.Errorf("Error during pagination of group members from Azure AD Group with ID %q: %+v", groupId, err) + if err := objects.NextWithContext(ctx); err != nil { + return nil, fmt.Errorf("Error during pagination of directory objects: %+v", err) } } + return ids, nil +} + +func GroupAllMembers(client graphrbac.GroupsClient, ctx context.Context, groupId string) ([]string, error) { + members, err := client.GetGroupMembersComplete(ctx, groupId) + + if err != nil { + return nil, fmt.Errorf("Error listing existing group members from Azure AD Group with ID %q: %+v", groupId, err) + } + + existingMembers, err := DirectoryObjectListToIDs(members, ctx) + if err != nil { + return nil, fmt.Errorf("Error getting objects IDs of group members for Azure AD Group with ID %q: %+v", groupId, err) + } + log.Printf("[DEBUG] %d members in Azure AD group with ID: %q", len(existingMembers), groupId) return existingMembers, nil @@ -105,3 +140,46 @@ func GroupAddMembers(client graphrbac.GroupsClient, ctx context.Context, groupId return nil } + +func GroupAllOwners(client graphrbac.GroupsClient, ctx context.Context, groupId string) ([]string, error) { + owners, err := client.ListOwnersComplete(ctx, groupId) + + if err != nil { + return nil, fmt.Errorf("Error listing existing group owners from Azure AD Group with ID %q: %+v", groupId, err) + } + + existingMembers, err := DirectoryObjectListToIDs(owners, ctx) + if err != nil { + return nil, fmt.Errorf("Error getting objects IDs of group owners for Azure AD Group with ID %q: %+v", groupId, err) + } + + log.Printf("[DEBUG] %d members in Azure AD group with ID: %q", len(existingMembers), groupId) + return existingMembers, nil +} + +func GroupAddOwner(client graphrbac.GroupsClient, ctx context.Context, groupId string, owner string) error { + ownerGraphURL := fmt.Sprintf("https://graph.windows.net/%s/directoryObjects/%s", client.TenantID, owner) + + properties := graphrbac.AddOwnerParameters{ + URL: &ownerGraphURL, + } + + log.Printf("[DEBUG] Adding owner with id %q to Azure AD group with id %q", owner, groupId) + if _, err := client.AddOwner(ctx, groupId, properties); err != nil { + return fmt.Errorf("Error adding group owner %q to Azure AD Group with ID %q: %+v", owner, groupId, err) + } + + return nil +} + +func GroupAddOwners(client graphrbac.GroupsClient, ctx context.Context, groupId string, owner []string) error { + for _, ownerUuid := range owner { + err := GroupAddOwner(client, ctx, groupId, ownerUuid) + + if err != nil { + return fmt.Errorf("Error while adding owners to Azure AD Group with ID %q: %+v", groupId, err) + } + } + + return nil +} diff --git a/azuread/helpers/graph/object_resource.go b/azuread/helpers/graph/object_resource.go new file mode 100644 index 0000000000..7e562171b4 --- /dev/null +++ b/azuread/helpers/graph/object_resource.go @@ -0,0 +1,58 @@ +package graph + +import ( + "fmt" + "strings" + + "github.com/hashicorp/go-uuid" +) + +type ObjectSubResourceId struct { + objectId string + subId string + Type string +} + +func (id ObjectSubResourceId) String() string { + return id.objectId + "/" + id.Type + "/" + id.subId +} + +func ParseObjectSubResourceId(idString, expectedType string) (ObjectSubResourceId, error) { + parts := strings.Split(idString, "/") + if len(parts) != 3 { + return ObjectSubResourceId{}, fmt.Errorf("Object Resource ID should be in the format {objectId}/{keyId} - but got %q", idString) + } + + id := ObjectSubResourceId{ + objectId: parts[0], + Type: parts[1], + subId: parts[2], + } + + if _, err := uuid.ParseUUID(id.objectId); err != nil { + return ObjectSubResourceId{}, fmt.Errorf("Object ID isn't a valid UUID (%q): %+v", id.objectId, err) + } + + if id.Type == "" { + return ObjectSubResourceId{}, fmt.Errorf("Type in {objectID}/{type}/{subID} should not blank") + } + + if id.Type != expectedType { + return ObjectSubResourceId{}, fmt.Errorf("Type in {objectID}/{type}/{subID} was expected to be %s, got %s", expectedType, parts[2]) + } + + if _, err := uuid.ParseUUID(id.subId); err != nil { + return ObjectSubResourceId{}, fmt.Errorf("Object Sub Resource ID isn't a valid UUID (%q): %+v", id.subId, err) + } + + return id, nil + +} + +func ObjectSubResourceIdFrom(objectId, typeId, subId string) ObjectSubResourceId { + return ObjectSubResourceId{ + objectId: objectId, + Type: typeId, + subId: subId, + } +} diff --git a/azuread/helpers/tf/locks.go b/azuread/helpers/tf/locks.go new file mode 100644 index 0000000000..3419b365f9 --- /dev/null +++ b/azuread/helpers/tf/locks.go @@ -0,0 +1,15 @@ +package tf + +import "github.com/hashicorp/terraform/helper/mutexkv" + +// mutex is the instance of MutexKV for ARM resources +var mutex = mutexkv.NewMutexKV() + +// handles the case of using the same name for different kinds of resources +func LockByName(resourceType string, name string) { + mutex.Lock(resourceType + "." + name) +} + +func UnlockByName(resourceType string, name string) { + mutex.Unlock(resourceType + "." + name) +} diff --git a/azuread/helpers/validate/strings_test.go b/azuread/helpers/validate/strings_test.go index 0827dae7a5..5d6a51627a 100644 --- a/azuread/helpers/validate/strings_test.go +++ b/azuread/helpers/validate/strings_test.go @@ -105,17 +105,17 @@ func TestStringIsEmailAddress(t *testing.T) { ErrCount int }{ { - Value: "john.doe@hashicorp.com", + Value: "j.doe@hashicorp.com", TestName: "Valid_EmailAddress", ErrCount: 0, }, { - Value: "john.doehashicorp.com", + Value: "j.doehashicorp.com", TestName: "Invalid_EmailAddress_NoAtChar", ErrCount: 1, }, { - Value: "john/doe@ha$hicorp.com", + Value: "j/doe@ha$hicorp.com", TestName: "Invalid_EmailAddress_InvalidChars", ErrCount: 1, }, diff --git a/azuread/locks.go b/azuread/locks.go deleted file mode 100644 index f62b745b0f..0000000000 --- a/azuread/locks.go +++ /dev/null @@ -1,10 +0,0 @@ -package azuread - -// handles the case of using the same name for different kinds of resources -func azureADLockByName(resourceType string, name string) { - armMutexKV.Lock(resourceType + "." + name) -} - -func azureADUnlockByName(resourceType string, name string) { - armMutexKV.Unlock(resourceType + "." + name) -} diff --git a/azuread/provider.go b/azuread/provider.go index 04f6298144..534670a9f0 100644 --- a/azuread/provider.go +++ b/azuread/provider.go @@ -4,14 +4,10 @@ import ( "fmt" "github.com/hashicorp/go-azure-helpers/authentication" - "github.com/hashicorp/terraform/helper/mutexkv" "github.com/hashicorp/terraform/helper/schema" "github.com/hashicorp/terraform/terraform" ) -// armMutexKV is the instance of MutexKV for ARM resources -var armMutexKV = mutexkv.NewMutexKV() - // Provider returns a terraform.ResourceProvider. func Provider() terraform.ResourceProvider { p := &schema.Provider{ diff --git a/azuread/resource_application_password.go b/azuread/resource_application_password.go index bd20ab2e39..09ad01d221 100644 --- a/azuread/resource_application_password.go +++ b/azuread/resource_application_password.go @@ -108,8 +108,8 @@ func resourceApplicationPasswordCreate(d *schema.ResourceData, meta interface{}) } id := graph.PasswordCredentialIdFrom(objectId, *cred.KeyID) - azureADLockByName(resourceApplicationName, id.ObjectId) - defer azureADUnlockByName(resourceApplicationName, id.ObjectId) + tf.LockByName(resourceApplicationName, id.ObjectId) + defer tf.UnlockByName(resourceApplicationName, id.ObjectId) existingCreds, err := client.ListPasswordCredentials(ctx, id.ObjectId) if err != nil { @@ -194,8 +194,8 @@ func resourceApplicationPasswordDelete(d *schema.ResourceData, meta interface{}) return fmt.Errorf("Error parsing Application Password ID: %v", err) } - azureADLockByName(resourceApplicationName, id.ObjectId) - defer azureADUnlockByName(resourceApplicationName, id.ObjectId) + tf.LockByName(resourceApplicationName, id.ObjectId) + defer tf.UnlockByName(resourceApplicationName, id.ObjectId) // ensure the parent Application exists app, err := client.Get(ctx, id.ObjectId) diff --git a/azuread/resource_group.go b/azuread/resource_group.go index 980d355dd1..c155363444 100644 --- a/azuread/resource_group.go +++ b/azuread/resource_group.go @@ -47,7 +47,17 @@ func resourceGroup() *schema.Resource { Optional: true, Computed: true, Set: schema.HashString, - ForceNew: false, + Elem: &schema.Schema{ + Type: schema.TypeString, + ValidateFunc: validate.UUID, + }, + }, + + "owners": { + Type: schema.TypeSet, + Optional: true, + Computed: true, + Set: schema.HashString, Elem: &schema.Schema{ Type: schema.TypeString, ValidateFunc: validate.UUID, @@ -83,11 +93,20 @@ func resourceGroupCreate(d *schema.ResourceData, meta interface{}) error { if v, ok := d.GetOk("members"); ok { members := tf.ExpandStringSlicePtr(v.(*schema.Set).List()) + // we could lock here against the group member resource, but they should not be used together (todo conflicts with at a resource level?) if err := graph.GroupAddMembers(client, ctx, *group.ObjectID, *members); err != nil { return err } } + // Add owners if specified + if v, ok := d.GetOk("owners"); ok { + members := tf.ExpandStringSlicePtr(v.(*schema.Set).List()) + if err := graph.GroupAddOwners(client, ctx, *group.ObjectID, *members); err != nil { + return err + } + } + _, err = graph.WaitForReplication(func() (interface{}, error) { return client.Get(ctx, *group.ObjectID) }) @@ -120,9 +139,14 @@ func resourceGroupRead(d *schema.ResourceData, meta interface{}) error { if err != nil { return err } - d.Set("members", members) + owners, err := graph.GroupAllOwners(client, ctx, d.Id()) + if err != nil { + return err + } + d.Set("owners", owners) + return nil } @@ -154,6 +178,30 @@ func resourceGroupUpdate(d *schema.ResourceData, meta interface{}) error { } } + if v, ok := d.GetOkExists("owners"); ok && d.HasChange("owners") { + existingOwners, err := graph.GroupAllOwners(client, ctx, d.Id()) + if err != nil { + return err + } + + desiredOwners := *tf.ExpandStringSlicePtr(v.(*schema.Set).List()) + ownersForRemoval := slices.Difference(existingOwners, desiredOwners) + ownersToAdd := slices.Difference(desiredOwners, existingOwners) + + for _, ownerToDelete := range ownersForRemoval { + log.Printf("[DEBUG] Removing member with id %q from Azure AD group with id %q", ownerToDelete, d.Id()) + if resp, err := client.RemoveOwner(ctx, d.Id(), ownerToDelete); err != nil { + if !ar.ResponseWasNotFound(resp) { + return fmt.Errorf("Error Deleting group member %q from Azure AD Group with ID %q: %+v", ownerToDelete, d.Id(), err) + } + } + } + + if err := graph.GroupAddOwners(client, ctx, d.Id(), ownersToAdd); err != nil { + return err + } + } + return resourceGroupRead(d, meta) } diff --git a/azuread/resource_group_member.go b/azuread/resource_group_member.go index 0afc87a2c9..f8507279db 100644 --- a/azuread/resource_group_member.go +++ b/azuread/resource_group_member.go @@ -2,20 +2,22 @@ package azuread import ( "fmt" - "strings" - - "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" "github.com/hashicorp/terraform/helper/schema" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/tf" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/validate" ) +const groupMemberResourceName = "azuread_group_member" + func resourceGroupMember() *schema.Resource { return &schema.Resource{ Create: resourceGroupMemberCreate, Read: resourceGroupMemberRead, Delete: resourceGroupMemberDelete, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -27,6 +29,7 @@ func resourceGroupMember() *schema.Resource { ForceNew: true, ValidateFunc: validate.UUID, }, + "member_object_id": { Type: schema.TypeString, Required: true, @@ -44,13 +47,14 @@ func resourceGroupMemberCreate(d *schema.ResourceData, meta interface{}) error { groupID := d.Get("group_object_id").(string) memberID := d.Get("member_object_id").(string) + tf.LockByName(groupMemberResourceName, groupID) + defer tf.UnlockByName(groupMemberResourceName, groupID) + if err := graph.GroupAddMember(client, ctx, groupID, memberID); err != nil { return err } - id := fmt.Sprintf("%s/member/%s", groupID, memberID) - d.SetId(id) - + d.SetId(graph.GroupMemberIdFrom(groupID, memberID).String()) return resourceGroupMemberRead(d, meta) } @@ -58,33 +62,29 @@ func resourceGroupMemberRead(d *schema.ResourceData, meta interface{}) error { client := meta.(*ArmClient).groupsClient ctx := meta.(*ArmClient).StopContext - id := strings.Split(d.Id(), "/member/") - if len(id) != 2 { - return fmt.Errorf("ID should be in the format {groupObjectId}/member/{memberObjectId} - but got %q", d.Id()) + id, err := graph.ParseGroupMemberId(d.Id()) + if err != nil { + return fmt.Errorf("Unable to parse ID: %v", err) } - groupID := id[0] - memberID := id[1] - - members, err := graph.GroupAllMembers(client, ctx, groupID) + members, err := graph.GroupAllMembers(client, ctx, id.GroupId) if err != nil { - return fmt.Errorf("Error retrieving Azure AD Group members (groupObjectId: %q): %+v", groupID, err) + return fmt.Errorf("Error retrieving Azure AD Group members (groupObjectId: %q): %+v", id.GroupId, err) } var memberObjectID string - for _, objectID := range members { - if objectID == memberID { + if objectID == id.MemberId { memberObjectID = objectID } } if memberObjectID == "" { d.SetId("") - return fmt.Errorf("Azure AD Group Member not found - groupObjectId:%q / memberObjectId:%q", groupID, memberID) + return fmt.Errorf("Azure AD Group Member not found - groupObjectId:%q / memberObjectId:%q", id.GroupId, id.MemberId) } - d.Set("group_object_id", groupID) + d.Set("group_object_id", id.GroupId) d.Set("member_object_id", memberObjectID) return nil @@ -94,18 +94,18 @@ func resourceGroupMemberDelete(d *schema.ResourceData, meta interface{}) error { client := meta.(*ArmClient).groupsClient ctx := meta.(*ArmClient).StopContext - id := strings.Split(d.Id(), "/member/") - if len(id) != 2 { - return fmt.Errorf("ID should be in the format {groupObjectId}/member/{memberObjectId} - but got %q", d.Id()) + id, err := graph.ParseGroupMemberId(d.Id()) + if err != nil { + return fmt.Errorf("Unable to parse ID: %v", err) } - groupID := id[0] - memberID := id[1] + tf.LockByName(groupMemberResourceName, id.GroupId) + defer tf.UnlockByName(groupMemberResourceName, id.GroupId) - resp, err := client.RemoveMember(ctx, groupID, memberID) + resp, err := client.RemoveMember(ctx, id.GroupId, id.MemberId) if err != nil { if !ar.ResponseWasNotFound(resp) { - return fmt.Errorf("Error removing Member (memberObjectId: %q) from Azure AD Group (groupObjectId: %q): %+v", memberID, groupID, err) + return fmt.Errorf("Error removing Member (memberObjectId: %q) from Azure AD Group (groupObjectId: %q): %+v", id.MemberId, id.GroupId, err) } } diff --git a/azuread/resource_group_member_test.go b/azuread/resource_group_member_test.go index 4cc76762b2..bb2c98b049 100644 --- a/azuread/resource_group_member_test.go +++ b/azuread/resource_group_member_test.go @@ -8,11 +8,12 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/ar" + "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/graph" "github.com/terraform-providers/terraform-provider-azuread/azuread/helpers/tf" ) -func TestAccAzureADGroupMember_User(t *testing.T) { - rn := "azuread_group_member.test" +func TestAccAzureADGroupMember_user(t *testing.T) { + rn := "azuread_group_member.testA" id := tf.AccRandTimeInt() pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) @@ -22,7 +23,7 @@ func TestAccAzureADGroupMember_User(t *testing.T) { CheckDestroy: testCheckAzureADGroupMemberDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureADGroupMember_User(id, pw), + Config: testAccAzureADGroupMember_oneUser(id, pw), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet(rn, "group_object_id"), resource.TestCheckResourceAttrSet(rn, "member_object_id"), @@ -37,7 +38,68 @@ func TestAccAzureADGroupMember_User(t *testing.T) { }) } -func TestAccAzureADGroupMember_Group(t *testing.T) { +func TestAccAzureADGroupMember_multipleUser(t *testing.T) { + rna := "azuread_group_member.testA" + rnb := "azuread_group_member.testB" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupMemberDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureADGroupMember_oneUser(id, pw), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(rna, "group_object_id"), + resource.TestCheckResourceAttrSet(rna, "member_object_id"), + ), + }, + { + ResourceName: rna, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAzureADGroupMember_twoUsers(id, pw), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(rna, "group_object_id"), + resource.TestCheckResourceAttrSet(rna, "member_object_id"), + resource.TestCheckResourceAttrSet(rnb, "group_object_id"), + resource.TestCheckResourceAttrSet(rnb, "member_object_id"), + ), + }, + // we rerun the config so the group resource updates with the number of members + { + Config: testAccAzureADGroupMember_twoUsers(id, pw), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("azuread_group.test", "members.#", "2"), + ), + }, + { + ResourceName: rna, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccAzureADGroupMember_oneUser(id, pw), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttrSet(rna, "group_object_id"), + resource.TestCheckResourceAttrSet(rna, "member_object_id"), + ), + }, + { + Config: testAccAzureADGroupMember_oneUser(id, pw), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("azuread_group.test", "members.#", "1"), + ), + }, + }, + }) +} + +func TestAccAzureADGroupMember_group(t *testing.T) { rn := "azuread_group_member.test" id := tf.AccRandTimeInt() @@ -47,7 +109,7 @@ func TestAccAzureADGroupMember_Group(t *testing.T) { CheckDestroy: testCheckAzureADGroupMemberDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureADGroupMember_Group(id), + Config: testAccAzureADGroupMember_group(id), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet(rn, "group_object_id"), resource.TestCheckResourceAttrSet(rn, "member_object_id"), @@ -62,7 +124,7 @@ func TestAccAzureADGroupMember_Group(t *testing.T) { }) } -func TestAccAzureADGroupMember_ServicePrincipal(t *testing.T) { +func TestAccAzureADGroupMember_servicePrincipal(t *testing.T) { rn := "azuread_group_member.test" id := tf.AccRandTimeInt() @@ -72,7 +134,7 @@ func TestAccAzureADGroupMember_ServicePrincipal(t *testing.T) { CheckDestroy: testCheckAzureADGroupMemberDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureADGroupMember_ServicePrincipal(id), + Config: testAccAzureADGroupMember_servicePrincipal(id), Check: resource.ComposeTestCheckFunc( resource.TestCheckResourceAttrSet(rn, "group_object_id"), resource.TestCheckResourceAttrSet(rn, "member_object_id"), @@ -99,53 +161,24 @@ func testCheckAzureADGroupMemberDestroy(s *terraform.State) error { groupID := rs.Primary.Attributes["group_object_id"] memberID := rs.Primary.Attributes["member_object_id"] - members, err := client.GetGroupMembersComplete(ctx, groupID) - if err != nil { - if ar.ResponseWasNotFound(members.Response().Response) { - return nil + // see if group exists + if resp, err := client.Get(ctx, groupID); err != nil { + if ar.ResponseWasNotFound(resp.Response) { + continue } - return err + return fmt.Errorf("Error retrieving Azure AD Group with ID %q: %+v", groupID, err) } - var memberObjectID string - for members.NotDone() { - // possible members are users, groups or service principals - // we try to 'cast' each result as the corresponding type and diff - // if we found the object we're looking for - user, _ := members.Value().AsUser() - if user != nil { - if *user.ObjectID == memberID { - memberObjectID = *user.ObjectID - // we successfully found the directory object we're looking for, we can stop looping - // through the results - break - } - } - - group, _ := members.Value().AsADGroup() - if group != nil { - if *group.ObjectID == memberID { - memberObjectID = *group.ObjectID - // we successfully found the directory object we're looking for, we can stop looping - // through the results - break - } - } - - servicePrincipal, _ := members.Value().AsServicePrincipal() - if servicePrincipal != nil { - if *servicePrincipal.ObjectID == memberID { - memberObjectID = *servicePrincipal.ObjectID - // we successfully found the directory object we're looking for, we can stop looping - // through the results - break - } - } + members, err := graph.GroupAllMembers(client, ctx, groupID) + if err != nil { + return fmt.Errorf("Error retrieving Azure AD Group members (groupObjectId: %q): %+v", groupID, err) + } - err = members.NextWithContext(ctx) - if err != nil { - return fmt.Errorf("Error listing Azure AD Group Members: %s", err) + var memberObjectID string + for _, objectID := range members { + if objectID == memberID { + memberObjectID = objectID } } @@ -157,32 +190,44 @@ func testCheckAzureADGroupMemberDestroy(s *terraform.State) error { return nil } -func testAccAzureADGroupMember_User(id int, password string) string { +func testAccAzureADGroupMember_oneUser(id int, password string) string { return fmt.Sprintf(` +%[1]s + +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" +} -data "azuread_domains" "tenant_domain" { - only_initial = true +resource "azuread_group_member" "testA" { + group_object_id = "${azuread_group.test.object_id}" + member_object_id = "${azuread_user.testA.object_id}" } -resource "azuread_user" "test" { - user_principal_name = "acctestUser.%[1]d.A@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]d-A" - password = "%[2]s" +`, testAccADUser_threeUsersABC(id, password), id) } + +func testAccAzureADGroupMember_twoUsers(id int, password string) string { + return fmt.Sprintf(` +%[1]s resource "azuread_group" "test" { - name = "acctestGroup-%[1]d" + name = "acctestGroup-%[2]d" } -resource "azuread_group_member" "test" { +resource "azuread_group_member" "testA" { + group_object_id = "${azuread_group.test.object_id}" + member_object_id = "${azuread_user.testA.object_id}" +} + +resource "azuread_group_member" "testB" { group_object_id = "${azuread_group.test.object_id}" - member_object_id = "${azuread_user.test.object_id}" + member_object_id = "${azuread_user.testB.object_id}" } -`, id, password) +`, testAccADUser_threeUsersABC(id, password), id) } -func testAccAzureADGroupMember_Group(id int) string { +func testAccAzureADGroupMember_group(id int) string { return fmt.Sprintf(` resource "azuread_group" "test" { @@ -201,7 +246,7 @@ resource "azuread_group_member" "test" { `, id) } -func testAccAzureADGroupMember_ServicePrincipal(id int) string { +func testAccAzureADGroupMember_servicePrincipal(id int) string { return fmt.Sprintf(` resource "azuread_application" "test" { diff --git a/azuread/resource_group_test.go b/azuread/resource_group_test.go index 34a121b723..a86fb321bb 100644 --- a/azuread/resource_group_test.go +++ b/azuread/resource_group_test.go @@ -23,7 +23,52 @@ func TestAccAzureADGroup_basic(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAzureADGroup_basic(id), - Check: testCheckAzureAdGroupBasic(id, "0"), + Check: testCheckAzureAdGroupBasic(id, "0", "0"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAzureADGroup_complete(t *testing.T) { + rn := "azuread_group.test" + id := tf.AccRandTimeInt() + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureADGroup_basic(id), + Check: testCheckAzureAdGroupBasic(id, "0", "0"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAzureADGroup_owners(t *testing.T) { + rn := "azuread_group.test" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureADGroupWithThreeOwners(id, pw), + Check: testCheckAzureAdGroupBasic(id, "0", "3"), }, { ResourceName: rn, @@ -46,7 +91,7 @@ func TestAccAzureADGroup_members(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAzureADGroupWithThreeMembers(id, pw), - Check: testCheckAzureAdGroupBasic(id, "3"), + Check: testCheckAzureAdGroupBasic(id, "3", "0"), }, { ResourceName: rn, @@ -57,9 +102,10 @@ func TestAccAzureADGroup_members(t *testing.T) { }) } -func TestAccAzureADGroup_complete(t *testing.T) { +func TestAccAzureADGroup_membersAndOwners(t *testing.T) { rn := "azuread_group.test" id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -67,8 +113,8 @@ func TestAccAzureADGroup_complete(t *testing.T) { CheckDestroy: testCheckAzureADGroupDestroy, Steps: []resource.TestStep{ { - Config: testAccAzureADGroup_basic(id), - Check: testCheckAzureAdGroupBasic(id, "0"), + Config: testAccAzureADGroupWithOwnersAndMembers(id, pw), + Check: testCheckAzureAdGroupBasic(id, "2", "1"), }, { ResourceName: rn, @@ -79,7 +125,7 @@ func TestAccAzureADGroup_complete(t *testing.T) { }) } -func TestAccAzureADGroup_diverse(t *testing.T) { +func TestAccAzureADGroup_membersDiverse(t *testing.T) { rn := "azuread_group.test" id := tf.AccRandTimeInt() pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) @@ -91,7 +137,7 @@ func TestAccAzureADGroup_diverse(t *testing.T) { Steps: []resource.TestStep{ { Config: testAccAzureADGroupWithDiverseMembers(id, pw), - Check: testCheckAzureAdGroupBasic(id, "3"), + Check: testCheckAzureAdGroupBasic(id, "3", "0"), }, { ResourceName: rn, @@ -102,7 +148,30 @@ func TestAccAzureADGroup_diverse(t *testing.T) { }) } -func TestAccAzureADGroup_progression(t *testing.T) { +func TestAccAzureADGroup_ownersDiverse(t *testing.T) { + rn := "azuread_group.test" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAzureADGroupWithDiverseOwners(id, pw), + Check: testCheckAzureAdGroupBasic(id, "0", "2"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + +func TestAccAzureADGroup_membersUpdate(t *testing.T) { rn := "azuread_group.test" id := tf.AccRandTimeInt() pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) @@ -115,7 +184,7 @@ func TestAccAzureADGroup_progression(t *testing.T) { // Empty group with 0 members { Config: testAccAzureADGroup_basic(id), - Check: testCheckAzureAdGroupBasic(id, "0"), + Check: testCheckAzureAdGroupBasic(id, "0", "0"), }, { ResourceName: rn, @@ -125,7 +194,7 @@ func TestAccAzureADGroup_progression(t *testing.T) { // Group with 1 member { Config: testAccAzureADGroupWithOneMember(id, pw), - Check: testCheckAzureAdGroupBasic(id, "1"), + Check: testCheckAzureAdGroupBasic(id, "1", "0"), }, { ResourceName: rn, @@ -135,7 +204,7 @@ func TestAccAzureADGroup_progression(t *testing.T) { // Group with multiple members { Config: testAccAzureADGroupWithThreeMembers(id, pw), - Check: testCheckAzureAdGroupBasic(id, "3"), + Check: testCheckAzureAdGroupBasic(id, "3", "0"), }, { ResourceName: rn, @@ -144,8 +213,8 @@ func TestAccAzureADGroup_progression(t *testing.T) { }, // Group with a different member { - Config: testAccAzureADGroupWithServicePrincipal(id), - Check: testCheckAzureAdGroupBasic(id, "1"), + Config: testAccAzureADGroupWithServicePrincipalMember(id), + Check: testCheckAzureAdGroupBasic(id, "1", "0"), }, { ResourceName: rn, @@ -155,8 +224,63 @@ func TestAccAzureADGroup_progression(t *testing.T) { // Empty group with 0 members { Config: testAccAzureADGroup_basic(id), - Check: testCheckAzureAdGroupBasic(id, "0"), + Check: testCheckAzureAdGroupBasic(id, "0", "0"), + }, + }, + }) +} + +func TestAccAzureADGroup_ownersUpdate(t *testing.T) { + rn := "azuread_group.test" + id := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testCheckAzureADGroupDestroy, + Steps: []resource.TestStep{ + // Empty group with 0 owners + { + Config: testAccAzureADGroup_basic(id), + Check: testCheckAzureAdGroupBasic(id, "0", "0"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + // Group with multiple owners + { + Config: testAccAzureADGroupWithThreeOwners(id, pw), + Check: testCheckAzureAdGroupBasic(id, "0", "3"), + }, + // Group with 1 owners + { + Config: testAccAzureADGroupWithOneOwners(id, pw), + Check: testCheckAzureAdGroupBasic(id, "0", "1"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, + }, + // Group with a different owners + { + Config: testAccAzureADGroupWithServicePrincipalOwner(id), + Check: testCheckAzureAdGroupBasic(id, "0", "1"), + }, + { + ResourceName: rn, + ImportState: true, + ImportStateVerify: true, }, + // Empty group with 0 owners is not possible }, }) } @@ -207,7 +331,7 @@ func testCheckAzureADGroupDestroy(s *terraform.State) error { return nil } -func testCheckAzureAdGroupBasic(id int, memberCount string) resource.TestCheckFunc { +func testCheckAzureAdGroupBasic(id int, memberCount, ownerCount string) resource.TestCheckFunc { resourceName := "azuread_group.test" return resource.ComposeTestCheckFunc( @@ -215,6 +339,7 @@ func testCheckAzureAdGroupBasic(id int, memberCount string) resource.TestCheckFu resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctestGroup-%d", id)), resource.TestCheckResourceAttrSet(resourceName, "object_id"), resource.TestCheckResourceAttr(resourceName, "members.#", memberCount), + resource.TestCheckResourceAttr(resourceName, "owners.#", ownerCount), ) } @@ -227,7 +352,7 @@ resource "azuread_group" "test" { `, id) } -func testAccAzureADGroupWithDiverseMembers(id int, password string) string { +func testAccAzureADDiverseDirectoryObjects(id int, password string) string { return fmt.Sprintf(` data "azuread_domains" "tenant_domain" { only_initial = true @@ -250,65 +375,105 @@ resource "azuread_user" "test" { display_name = "acctestUser-%[1]d" password = "%[2]s" } +`, id, password) +} + +func testAccAzureADGroupWithDiverseMembers(id int, password string) string { + return fmt.Sprintf(` +%[1]s resource "azuread_group" "test" { - name = "acctestGroup-%[1]d" + name = "acctestGroup-%[2]d" members = [ azuread_user.test.object_id, azuread_group.member.object_id, azuread_service_principal.test.object_id ] } -`, id, password) +`, testAccAzureADDiverseDirectoryObjects(id, password), id) } -func testAccAzureADGroupWithOneMember(id int, password string) string { +func testAccAzureADGroupWithDiverseOwners(id int, password string) string { return fmt.Sprintf(` -data "azuread_domains" "tenant_domain" { - only_initial = true -} +%[1]s -resource "azuread_user" "test" { - user_principal_name = "acctestUser.%[1]d@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]d" - password = "%[2]s" +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" + owners = [ azuread_user.test.object_id, azuread_service_principal.test.object_id ] +} +`, testAccAzureADDiverseDirectoryObjects(id, password), id) } +func testAccAzureADGroupWithOneMember(id int, password string) string { + return fmt.Sprintf(` +%[1]s + resource "azuread_group" "test" { - name = "acctestGroup-%[1]d" + name = "acctestGroup-%[2]d" members = [ azuread_user.test.object_id ] } -`, id, password) +`, testAccADUser_basic(id, password), id) +} + +func testAccAzureADGroupWithOneOwners(id int, password string) string { + return fmt.Sprintf(` +%[1]s + +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" + owners = [ azuread_user.test.object_id ] +} +`, testAccADUser_basic(id, password), id) } func testAccAzureADGroupWithThreeMembers(id int, password string) string { return fmt.Sprintf(` -data "azuread_domains" "tenant_domain" { - only_initial = true +%[1]s + +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" + members = [ azuread_user.testA.object_id, azuread_user.testB.object_id, azuread_user.testC.object_id ] +} +`, testAccADUser_threeUsersABC(id, password), id) } -resource "azuread_user" "testA" { - user_principal_name = "acctestUser.%[1]d.A@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]dA" - password = "%[2]s" +func testAccAzureADGroupWithThreeOwners(id int, password string) string { + return fmt.Sprintf(` +%[1]s + +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" + owners = [ azuread_user.testA.object_id, azuread_user.testB.object_id, azuread_user.testC.object_id ] +} +`, testAccADUser_threeUsersABC(id, password), id) } -resource "azuread_user" "testB" { - user_principal_name = "acctestUser.%[1]d.B@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]d-B" - password = "%[2]s" +func testAccAzureADGroupWithOwnersAndMembers(id int, password string) string { + return fmt.Sprintf(` +%[1]s + +resource "azuread_group" "test" { + name = "acctestGroup-%[2]d" + owners = [ azuread_user.testA.object_id ] + members = [ azuread_user.testB.object_id, azuread_user.testC.object_id ] +} +`, testAccADUser_threeUsersABC(id, password), id) } -resource "azuread_user" "testC" { - user_principal_name = "acctestUser.%[1]dC@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]dC" - password = "%[2]s" +func testAccAzureADGroupWithServicePrincipalMember(id int) string { + return fmt.Sprintf(` +resource "azuread_application" "test" { + name = "acctestApp-%[1]d" +} + +resource "azuread_service_principal" "test" { + application_id = azuread_application.test.application_id } resource "azuread_group" "test" { name = "acctestGroup-%[1]d" - members = [ azuread_user.testA.object_id, azuread_user.testB.object_id, azuread_user.testC.object_id ] + members = [ azuread_service_principal.test.object_id ] } -`, id, password) +`, id) } -func testAccAzureADGroupWithServicePrincipal(id int) string { +func testAccAzureADGroupWithServicePrincipalOwner(id int) string { return fmt.Sprintf(` resource "azuread_application" "test" { name = "acctestApp-%[1]d" @@ -319,8 +484,8 @@ resource "azuread_service_principal" "test" { } resource "azuread_group" "test" { - name = "acctestGroup-%[1]d" - members = [ azuread_service_principal.test.object_id ] + name = "acctestGroup-%[1]d" + owners = [ azuread_service_principal.test.object_id ] } `, id) } diff --git a/azuread/resource_service_principal_password.go b/azuread/resource_service_principal_password.go index 9c7d57a204..abe8ba5048 100644 --- a/azuread/resource_service_principal_password.go +++ b/azuread/resource_service_principal_password.go @@ -39,8 +39,8 @@ func resourceServicePrincipalPasswordCreate(d *schema.ResourceData, meta interfa } id := graph.PasswordCredentialIdFrom(objectId, *cred.KeyID) - azureADLockByName(servicePrincipalResourceName, id.ObjectId) - defer azureADUnlockByName(servicePrincipalResourceName, id.ObjectId) + tf.LockByName(servicePrincipalResourceName, id.ObjectId) + defer tf.UnlockByName(servicePrincipalResourceName, id.ObjectId) existingCreds, err := client.ListPasswordCredentials(ctx, id.ObjectId) if err != nil { @@ -125,8 +125,8 @@ func resourceServicePrincipalPasswordDelete(d *schema.ResourceData, meta interfa return fmt.Errorf("Error parsing Application Password ID: %v", err) } - azureADLockByName(servicePrincipalResourceName, id.ObjectId) - defer azureADUnlockByName(servicePrincipalResourceName, id.ObjectId) + tf.LockByName(servicePrincipalResourceName, id.ObjectId) + defer tf.UnlockByName(servicePrincipalResourceName, id.ObjectId) // ensure the parent Service Principal exists servicePrincipal, err := client.Get(ctx, id.ObjectId) diff --git a/azuread/resource_user_test.go b/azuread/resource_user_test.go index 34f6470938..60e82c7993 100644 --- a/azuread/resource_user_test.go +++ b/azuread/resource_user_test.go @@ -114,7 +114,7 @@ func TestAccAzureADUser_update(t *testing.T) { ), }, { - Config: testAccADUser_multiple(id, pw1), + Config: testAccADUser_threeUsersABC(id, pw1), Check: resource.ComposeTestCheckFunc( testCheckADUserExists("azuread_user.testA"), testCheckADUserExists("azuread_user.testB"), @@ -207,23 +207,29 @@ resource "azuread_user" "test" { `, id, password) } -func testAccADUser_multiple(id int, password string) string { +func testAccADUser_threeUsersABC(id int, password string) string { return fmt.Sprintf(` data "azuread_domains" "tenant_domain" { only_initial = true } resource "azuread_user" "testA" { - user_principal_name = "acctestUser.%[1]d.A@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]d-A" - password = "%[2]s" + user_principal_name = "acctestUser.%[1]d.A@${data.azuread_domains.tenant_domain.domains.0.domain_name}" + display_name = "acctestUser-%[1]d-A" + password = "%[2]s" } resource "azuread_user" "testB" { - user_principal_name = "acctestUser.%[1]d.B@${data.azuread_domains.tenant_domain.domains.0.domain_name}" - display_name = "acctestUser-%[1]d-B" - mail_nickname = "acctestUser-%[1]d-B" - password = "%[2]s" + user_principal_name = "acctestUser.%[1]d.B@${data.azuread_domains.tenant_domain.domains.0.domain_name}" + display_name = "acctestUser-%[1]d-B" + mail_nickname = "acctestUser-%[1]d-B" + password = "%[2]s" +} + +resource "azuread_user" "testC" { + user_principal_name = "acctestUser.%[1]d.C@${data.azuread_domains.tenant_domain.domains.0.domain_name}" + display_name = "acctestUser-%[1]d-C" + password = "%[2]s" } `, id, password) } diff --git a/website/docs/r/group.markdown b/website/docs/r/group.markdown index 66e4be14b2..4b96497cd6 100644 --- a/website/docs/r/group.markdown +++ b/website/docs/r/group.markdown @@ -27,13 +27,13 @@ resource "azuread_group" "example" { ```hcl resource "azuread_user" "example" { - display_name = "John Doe" + display_name = "J Doe" password = "notSecure123" - user_principal_name = "john.doe@terraform.onmicrosoft.com" + user_principal_name = "j.doe@terraform.onmicrosoft.com" } resource "azuread_group" "example" { - name = "Group-With-Members" + name = "MyGroup" members = [ "${azuread_user.example.object_id}" /*, more users */ ] } ``` @@ -44,11 +44,14 @@ The following arguments are supported: * `name` - (Required) The display name for the Group. Changing this forces a new resource to be created. * `members` (Optional) A set of members who should be present in this Group. Supported Object types are Users, Groups or Service Principals. +* `owners` (Optional) A set of owners who own this Group. Supported Object types are Users or Service Principals. -> **NOTE:** Group names are not unique within Azure Active Directory. -> **NOTE:** Do not use `azuread_group_member` at the same time as the `members` argument. +-> **NOTE:** Do not use `azuread_group_owner` at the same time as the `owners` argument. + ## Attributes Reference The following attributes are exported: @@ -59,6 +62,8 @@ The following attributes are exported: * `members` - The Members of the Group. +* `owners` - The Members of the Group. + ## Import Azure Active Directory Groups can be imported using the `object id`, e.g. diff --git a/website/docs/r/group_member.markdown b/website/docs/r/group_member.markdown index 57d4edfa28..ce5e10a521 100644 --- a/website/docs/r/group_member.markdown +++ b/website/docs/r/group_member.markdown @@ -18,7 +18,7 @@ Manages a single Group Membership within Azure Active Directory. ```hcl data "azuread_user" "example" { - user_principal_name = "johndoe@hashicorp.com" + user_principal_name = "jdoe@hashicorp.com" } resource "azuread_group" "example" {