diff --git a/auth0/resource_auth0_user.go b/auth0/resource_auth0_user.go index 42eeb562b..7e218ec8e 100644 --- a/auth0/resource_auth0_user.go +++ b/auth0/resource_auth0_user.go @@ -167,22 +167,13 @@ func readUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag.D if err != nil { return diag.FromErr(err) } - result = multierror.Append( - result, - d.Set("roles", func() []interface{} { - var roles []interface{} - for _, role := range roleList.Roles { - roles = append(roles, auth0.StringValue(role.ID)) - } - return roles - }()), - ) + result = multierror.Append(result, d.Set("roles", flattenUserRoles(roleList))) return diag.FromErr(result.ErrorOrNil()) } func createUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { - user, err := buildUser(d) + user, err := expandUser(d) if err != nil { return diag.FromErr(err) } @@ -191,19 +182,18 @@ func createUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag if err := api.User.Create(user); err != nil { return diag.FromErr(err) } + d.SetId(auth0.StringValue(user.ID)) - d.Partial(true) - if err = assignUserRoles(d, m); err != nil { + if err = updateUserRoles(d, api); err != nil { return diag.FromErr(err) } - d.Partial(false) return readUser(ctx, d, m) } func updateUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag.Diagnostics { - user, err := buildUser(d) + user, err := expandUser(d) if err != nil { return diag.FromErr(err) } @@ -219,11 +209,9 @@ func updateUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag } } - d.Partial(true) - if err = assignUserRoles(d, m); err != nil { + if err = updateUserRoles(d, api); err != nil { return diag.Errorf("failed assigning user roles. %s", err) } - d.Partial(false) return readUser(ctx, d, m) } @@ -237,12 +225,13 @@ func deleteUser(ctx context.Context, d *schema.ResourceData, m interface{}) diag return nil } } + return diag.FromErr(err) } return nil } -func buildUser(d *schema.ResourceData) (*management.User, error) { +func expandUser(d *schema.ResourceData) (*management.User, error) { user := &management.User{ ID: String(d, "user_id", IsNewResource()), Connection: String(d, "connection_name"), @@ -276,13 +265,22 @@ func buildUser(d *schema.ResourceData) (*management.User, error) { return user, nil } +func flattenUserRoles(roleList *management.RoleList) []interface{} { + var roles []interface{} + for _, role := range roleList.Roles { + roles = append(roles, auth0.StringValue(role.ID)) + } + return roles +} + func validateUser(user *management.User) error { - var result *multierror.Error validations := []validateUserFunc{ validateNoUsernameAndPasswordSimultaneously(), validateNoUsernameAndEmailVerifiedSimultaneously(), validateNoPasswordAndEmailVerifiedSimultaneously(), } + + var result *multierror.Error for _, validationFunc := range validations { if err := validationFunc(user); err != nil { result = multierror.Append(result, err) @@ -294,80 +292,76 @@ func validateUser(user *management.User) error { func validateNoUsernameAndPasswordSimultaneously() validateUserFunc { return func(user *management.User) error { - var err error if user.Username != nil && user.Password != nil { - err = fmt.Errorf("cannot update username and password simultaneously") + return fmt.Errorf("cannot update username and password simultaneously") } - return err + return nil } } func validateNoUsernameAndEmailVerifiedSimultaneously() validateUserFunc { return func(user *management.User) error { - var err error if user.Username != nil && user.EmailVerified != nil { - err = fmt.Errorf("cannot update username and email_verified simultaneously") + return fmt.Errorf("cannot update username and email_verified simultaneously") } - return err + return nil } } func validateNoPasswordAndEmailVerifiedSimultaneously() validateUserFunc { return func(user *management.User) error { - var err error if user.Password != nil && user.EmailVerified != nil { - err = fmt.Errorf("cannot update password and email_verified simultaneously") + return fmt.Errorf("cannot update password and email_verified simultaneously") } - return err + return nil } } -func assignUserRoles(d *schema.ResourceData, m interface{}) error { - add, rm := Diff(d, "roles") +func updateUserRoles(d *schema.ResourceData, api *management.Management) error { + toAdd, toRemove := Diff(d, "roles") - var addRoles []*management.Role - for _, addRole := range add.List() { - addRoles = append( - addRoles, - &management.Role{ - ID: auth0.String(addRole.(string)), - }, - ) + if err := removeUserRoles(api, d.Id(), toRemove.List()); err != nil { + return err } + return assignUserRoles(api, d.Id(), toAdd.List()) +} + +func removeUserRoles(api *management.Management, userID string, userRolesToRemove []interface{}) error { var rmRoles []*management.Role - for _, rmRole := range rm.List() { - rmRoles = append( - rmRoles, - &management.Role{ - ID: auth0.String(rmRole.(string)), - }, - ) + for _, rmRole := range userRolesToRemove { + role := &management.Role{ID: auth0.String(rmRole.(string))} + rmRoles = append(rmRoles, role) } - api := m.(*management.Management) + if len(rmRoles) == 0 { + return nil + } - if len(rmRoles) > 0 { - if err := api.User.RemoveRoles(d.Id(), rmRoles); err != nil { - // Ignore 404 errors as the role may have been deleted - // prior to un-assigning them from the user. - if mErr, ok := err.(management.Error); ok { - if mErr.Status() != http.StatusNotFound { - return err - } - } else { - return err - } + err := api.User.RemoveRoles(userID, rmRoles) + if err != nil { + // Ignore 404 errors as the role may have been deleted + // prior to un-assigning them from the user. + if err, ok := err.(management.Error); ok && err.Status() == http.StatusNotFound { + return nil } } - if len(addRoles) > 0 { - if err := api.User.AssignRoles(d.Id(), addRoles); err != nil { - return err - } + return err +} + +func assignUserRoles(api *management.Management, userID string, userRolesToAdd []interface{}) error { + var addRoles []*management.Role + for _, addRole := range userRolesToAdd { + role := &management.Role{ID: auth0.String(addRole.(string))} + addRoles = append(addRoles, role) } - return nil + if len(addRoles) == 0 { + return nil + } + + return api.User.AssignRoles(userID, addRoles) } func userHasChange(u *management.User) bool { diff --git a/auth0/resource_auth0_user_test.go b/auth0/resource_auth0_user_test.go index 44944b14f..519174de9 100644 --- a/auth0/resource_auth0_user_test.go +++ b/auth0/resource_auth0_user_test.go @@ -63,44 +63,6 @@ func TestAccUserMissingRequiredParams(t *testing.T) { }) } -func TestAccUser(t *testing.T) { - httpRecorder := configureHTTPRecorder(t) - - resource.Test(t, resource.TestCase{ - ProviderFactories: testProviders(httpRecorder), - Steps: []resource.TestStep{ - { - Config: template.ParseTestName(testAccUserCreate, strings.ToLower(t.Name())), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.user", "user_id", fmt.Sprintf("auth0|%s", strings.ToLower(t.Name()))), - resource.TestCheckResourceAttr("auth0_user.user", "email", fmt.Sprintf("%s@acceptance.test.com", strings.ToLower(t.Name()))), - resource.TestCheckResourceAttr("auth0_user.user", "name", "Firstname Lastname"), - resource.TestCheckResourceAttr("auth0_user.user", "family_name", "Lastname"), - resource.TestCheckResourceAttr("auth0_user.user", "given_name", "Firstname"), - resource.TestCheckResourceAttr("auth0_user.user", "nickname", strings.ToLower(t.Name())), - resource.TestCheckResourceAttr("auth0_user.user", "connection_name", "Username-Password-Authentication"), - resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"), - resource.TestCheckResourceAttr("auth0_user.user", "picture", "https://www.example.com/picture.jpg"), - ), - }, - { - Config: template.ParseTestName(testAccUserAddRole, strings.ToLower(t.Name())), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "2"), - resource.TestCheckResourceAttr("auth0_role.owner", "name", "owner"), - resource.TestCheckResourceAttr("auth0_role.admin", "name", "admin"), - ), - }, - { - Config: template.ParseTestName(testAccUserRemoveRole, strings.ToLower(t.Name())), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "1"), - ), - }, - }, - }) -} - const testAccUserCreate = ` resource auth0_user user { connection_name = "Username-Password-Authentication" @@ -202,28 +164,45 @@ resource auth0_role admin { } ` -func TestAccUserIssue218(t *testing.T) { +func TestAccUser(t *testing.T) { httpRecorder := configureHTTPRecorder(t) resource.Test(t, resource.TestCase{ ProviderFactories: testProviders(httpRecorder), Steps: []resource.TestStep{ { - Config: template.ParseTestName(testAccUserIssue218, "issue#218"), + Config: template.ParseTestName(testAccUserCreate, strings.ToLower(t.Name())), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "user_id", "auth0|id_issue#218"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "username", "user_issue#218"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "email", "issue.218.issue#218@acceptance.test.com"), + resource.TestCheckResourceAttr("auth0_user.user", "user_id", fmt.Sprintf("auth0|%s", strings.ToLower(t.Name()))), + resource.TestCheckResourceAttr("auth0_user.user", "email", fmt.Sprintf("%s@acceptance.test.com", strings.ToLower(t.Name()))), + resource.TestCheckResourceAttr("auth0_user.user", "name", "Firstname Lastname"), + resource.TestCheckResourceAttr("auth0_user.user", "family_name", "Lastname"), + resource.TestCheckResourceAttr("auth0_user.user", "given_name", "Firstname"), + resource.TestCheckResourceAttr("auth0_user.user", "nickname", strings.ToLower(t.Name())), + resource.TestCheckResourceAttr("auth0_user.user", "connection_name", "Username-Password-Authentication"), + resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "0"), + resource.TestCheckResourceAttr("auth0_user.user", "picture", "https://www.example.com/picture.jpg"), + ), + }, + { + Config: template.ParseTestName(testAccUserAddRole, strings.ToLower(t.Name())), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "2"), + resource.TestCheckResourceAttr("auth0_role.owner", "name", "owner"), + resource.TestCheckResourceAttr("auth0_role.admin", "name", "admin"), ), }, { - Config: template.ParseTestName(testAccUserIssue218, "issue#218"), + Config: template.ParseTestName(testAccUserRemoveRole, strings.ToLower(t.Name())), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("auth0_user.user", "roles.#", "1"), + ), }, }, }) } -const testAccUserIssue218 = ` +const testAccUserCanSerializeEmptyMetadataFields = ` resource auth0_user auth0_user_issue_218 { connection_name = "Username-Password-Authentication" user_id = "id_{{.testName}}" @@ -234,31 +213,22 @@ resource auth0_user auth0_user_issue_218 { } ` -func TestAccUserChangeUsername(t *testing.T) { +func TestAccUserCanSerializeEmptyMetadataFields(t *testing.T) { httpRecorder := configureHTTPRecorder(t) resource.Test(t, resource.TestCase{ ProviderFactories: testProviders(httpRecorder), Steps: []resource.TestStep{ { - Config: template.ParseTestName(testAccUserChangeUsernameCreate, "terra"), - Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "username", "user_terra"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "email", "change.username.terra@acceptance.test.com"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "password", "MyPass123$"), - ), - }, - { - Config: template.ParseTestName(testAccUserChangeUsernameUpdate, "terra"), + Config: template.ParseTestName(testAccUserCanSerializeEmptyMetadataFields, "issue#218"), Check: resource.ComposeTestCheckFunc( - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "username", "user_x_terra"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "email", "change.username.terra@acceptance.test.com"), - resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "password", "MyPass123$"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "user_id", "auth0|id_issue#218"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "username", "user_issue#218"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_issue_218", "email", "issue.218.issue#218@acceptance.test.com"), ), }, { - Config: template.ParseTestName(testAccUserChangeUsernameAndPassword, "terra"), - ExpectError: regexp.MustCompile("cannot update username and password simultaneously"), + Config: template.ParseTestName(testAccUserCanSerializeEmptyMetadataFields, "issue#218"), }, }, }) @@ -293,3 +263,33 @@ resource auth0_user auth0_user_change_username { password = "MyPass123456$" } ` + +func TestAccUserChangeUsername(t *testing.T) { + httpRecorder := configureHTTPRecorder(t) + + resource.Test(t, resource.TestCase{ + ProviderFactories: testProviders(httpRecorder), + Steps: []resource.TestStep{ + { + Config: template.ParseTestName(testAccUserChangeUsernameCreate, "terra"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "username", "user_terra"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "email", "change.username.terra@acceptance.test.com"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "password", "MyPass123$"), + ), + }, + { + Config: template.ParseTestName(testAccUserChangeUsernameUpdate, "terra"), + Check: resource.ComposeTestCheckFunc( + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "username", "user_x_terra"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "email", "change.username.terra@acceptance.test.com"), + resource.TestCheckResourceAttr("auth0_user.auth0_user_change_username", "password", "MyPass123$"), + ), + }, + { + Config: template.ParseTestName(testAccUserChangeUsernameAndPassword, "terra"), + ExpectError: regexp.MustCompile("cannot update username and password simultaneously"), + }, + }, + }) +} diff --git a/auth0/testdata/recordings/TestAccUserIssue218.yaml b/auth0/testdata/recordings/TestAccUserCanSerializeEmptyMetadataFields.yaml similarity index 100% rename from auth0/testdata/recordings/TestAccUserIssue218.yaml rename to auth0/testdata/recordings/TestAccUserCanSerializeEmptyMetadataFields.yaml