diff --git a/internal/services/aadgraph/application_resource.go b/internal/services/aadgraph/application_resource.go index 0a38b58467..02ef64620f 100644 --- a/internal/services/aadgraph/application_resource.go +++ b/internal/services/aadgraph/application_resource.go @@ -108,9 +108,10 @@ func applicationResource() *schema.Resource { }, "app_role": { - Type: schema.TypeSet, - Optional: true, - Computed: true, + Type: schema.TypeSet, + Optional: true, + Computed: true, + ConfigMode: schema.SchemaConfigModeAttr, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "id": { @@ -158,6 +159,67 @@ func applicationResource() *schema.Resource { }, }, + "oauth2_permissions": { + Type: schema.TypeSet, + Optional: true, + Computed: true, + ConfigMode: schema.SchemaConfigModeAttr, + Elem: &schema.Resource{ + Schema: map[string]*schema.Schema{ + "admin_consent_description": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validate.NoEmptyStrings, + }, + + "admin_consent_display_name": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validate.NoEmptyStrings, + }, + + "id": { + Type: schema.TypeString, + Computed: true, + }, + + "is_enabled": { + Type: schema.TypeBool, + Optional: true, + Computed: true, + }, + + "type": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validation.StringInSlice([]string{"Admin", "User"}, false), + }, + + "user_consent_description": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + + "user_consent_display_name": { + Type: schema.TypeString, + Optional: true, + Computed: true, + }, + + "value": { + Type: schema.TypeString, + Optional: true, + Computed: true, + ValidateFunc: validate.NoEmptyStrings, + }, + }, + }, + }, + "optional_claims": { Type: schema.TypeList, Optional: true, @@ -228,67 +290,6 @@ func applicationResource() *schema.Resource { Computed: true, }, - "oauth2_permissions": { - Type: schema.TypeSet, - Optional: true, - Computed: true, - ConfigMode: schema.SchemaConfigModeAttr, - Elem: &schema.Resource{ - Schema: map[string]*schema.Schema{ - "admin_consent_description": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validate.NoEmptyStrings, - }, - - "admin_consent_display_name": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validate.NoEmptyStrings, - }, - - "id": { - Type: schema.TypeString, - Computed: true, - }, - - "is_enabled": { - Type: schema.TypeBool, - Optional: true, - Computed: true, - }, - - "type": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validation.StringInSlice([]string{"Admin", "User"}, false), - }, - - "user_consent_description": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - - "user_consent_display_name": { - Type: schema.TypeString, - Optional: true, - Computed: true, - }, - - "value": { - Type: schema.TypeString, - Optional: true, - Computed: true, - ValidateFunc: validate.NoEmptyStrings, - }, - }, - }, - }, - "prevent_duplicate_names": { Type: schema.TypeBool, Optional: true, @@ -402,6 +403,24 @@ func applicationResourceCreate(d *schema.ResourceData, meta interface{}) error { } } + if v, ok := d.GetOk("app_role"); ok { + appRoles := expandApplicationAppRoles(v) + if appRoles != nil { + if err := graph.AppRolesSet(ctx, client, *app.ObjectID, appRoles); err != nil { + return err + } + } + } + + if v, ok := d.GetOk("oauth2_permissions"); ok { + oauth2Permissions := expandApplicationOAuth2Permissions(v) + if oauth2Permissions != nil { + if err := graph.OAuth2PermissionsSet(ctx, client, *app.ObjectID, oauth2Permissions); err != nil { + return err + } + } + } + // there is a default owner that we must account so use this shared function if v, ok := d.GetOk("owners"); ok { desiredOwners := *tf.ExpandStringSlicePtr(v.(*schema.Set).List()) @@ -410,9 +429,7 @@ func applicationResourceCreate(d *schema.ResourceData, meta interface{}) error { } } - // After creating the application, we immediately update it to ensure we overwrite any default properties - // such as the `user_impersonation` scope the application may get, whether we define such a scope or not - return applicationResourceUpdate(d, meta) + return applicationResourceRead(d, meta) } func applicationResourceUpdate(d *schema.ResourceData, meta interface{}) error { @@ -474,58 +491,6 @@ func applicationResourceUpdate(d *schema.ResourceData, meta interface{}) error { properties.OptionalClaims = expandApplicationOptionalClaims(d) } - if d.HasChange("oauth2_permissions") { - // if the permission already exists then it must be disabled - // with no other changes before it can be edited or deleted - var app graphrbac.Application - var appProperties graphrbac.ApplicationUpdateParameters - resp, err := client.Get(ctx, d.Id()) - if err != nil { - if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Application with ID %q was not found", d.Id()) - } - - return fmt.Errorf("retrieving Application with ID %q: %+v", d.Id(), err) - } - app = resp - for _, OAuth2Permission := range *app.Oauth2Permissions { - *OAuth2Permission.IsEnabled = false - } - appProperties.Oauth2Permissions = app.Oauth2Permissions - if _, err := client.Patch(ctx, d.Id(), appProperties); err != nil { - return fmt.Errorf("disabling OAuth2 permissions for Application with ID %q: %+v", d.Id(), err) - } - - // now we can set the new state of the permission - properties.Oauth2Permissions = expandApplicationOAuth2Permissions(d.Get("oauth2_permissions")) - } - - if d.HasChange("app_role") { - // if the app role already exists then it must be disabled - // with no other changes before it can be edited or deleted - var app graphrbac.Application - var appRolesProperties graphrbac.ApplicationUpdateParameters - resp, err := client.Get(ctx, d.Id()) - if err != nil { - if utils.ResponseWasNotFound(resp.Response) { - return fmt.Errorf("Application with ID %q was not found", d.Id()) - } - - return fmt.Errorf("retrieving Application with ID %q: %+v", d.Id(), err) - } - app = resp - for _, appRole := range *app.AppRoles { - *appRole.IsEnabled = false - } - appRolesProperties.AppRoles = app.AppRoles - if _, err := client.Patch(ctx, d.Id(), appRolesProperties); err != nil { - return fmt.Errorf("disabling App Roles for Application with ID %q: %+v", d.Id(), err) - } - - // now we can set the new state of the app role - properties.AppRoles = expandApplicationAppRoles(d.Get("app_role")) - } - if d.HasChange("group_membership_claims") { properties.GroupMembershipClaims = graphrbac.GroupMembershipClaimTypes(d.Get("group_membership_claims").(string)) } @@ -547,6 +512,23 @@ func applicationResourceUpdate(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("patching Application with ID %q: %+v", d.Id(), err) } + if d.HasChange("app_role") { + appRoles := expandApplicationAppRoles(d.Get("app_role")) + if appRoles != nil { + if err := graph.AppRolesSet(ctx, client, d.Id(), appRoles); err != nil { + return err + } + } + } + + if d.HasChange("oauth2_permissions") { + oauth2Permissions := expandApplicationOAuth2Permissions(d.Get("oauth2_permissions")) + if oauth2Permissions != nil { + if err := graph.OAuth2PermissionsSet(ctx, client, d.Id(), oauth2Permissions); err != nil { + return err + } + } + } if v, ok := d.GetOkExists("owners"); ok && d.HasChange("owners") { desiredOwners := *tf.ExpandStringSlicePtr(v.(*schema.Set).List()) if err := applicationSetOwnersTo(ctx, client, d.Id(), desiredOwners); err != nil { @@ -623,9 +605,11 @@ func applicationResourceRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("setting `owners`: %+v", err) } - if preventDuplicates := d.Get("prevent_duplicate_names").(bool); !preventDuplicates { - d.Set("prevent_duplicate_names", false) + preventDuplicates := false + if v := d.Get("prevent_duplicate_names").(bool); v { + preventDuplicates = v } + d.Set("prevent_duplicate_names", preventDuplicates) return nil } @@ -837,11 +821,8 @@ func flattenApplicationOptionalClaimsList(in *[]graphrbac.OptionalClaim) []inter func expandApplicationAppRoles(i interface{}) *[]graphrbac.AppRole { input := i.(*schema.Set).List() - if len(input) == 0 { - return nil - } - output := make([]graphrbac.AppRole, 0, len(input)) + for _, appRoleRaw := range input { appRole := appRoleRaw.(map[string]interface{}) diff --git a/internal/services/aadgraph/application_resource_test.go b/internal/services/aadgraph/application_resource_test.go index 55cb1599cf..9ade05d226 100644 --- a/internal/services/aadgraph/application_resource_test.go +++ b/internal/services/aadgraph/application_resource_test.go @@ -246,33 +246,21 @@ func TestAccApplication_appRolesUpdate(t *testing.T) { CheckDestroy: testCheckApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccApplication_appRoles(data.RandomInteger), + Config: testAccApplication_basic(data.RandomInteger), Check: resource.ComposeTestCheckFunc( testCheckApplicationExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "0"), ), }, data.ImportStep(), { - Config: testAccApplication_appRolesUpdate(data.RandomInteger), + Config: testAccApplication_appRoles(data.RandomInteger), Check: resource.ComposeTestCheckFunc( testCheckApplicationExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "2"), + resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "1"), ), }, data.ImportStep(), - }, - }) -} - -func TestAccApplication_appRolesDelete(t *testing.T) { - data := acceptance.BuildTestData(t, "azuread_application", "test") - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acceptance.PreCheck(t) }, - Providers: acceptance.SupportedProviders, - CheckDestroy: testCheckApplicationDestroy, - Steps: []resource.TestStep{ { Config: testAccApplication_appRolesUpdate(data.RandomInteger), Check: resource.ComposeTestCheckFunc( @@ -282,10 +270,10 @@ func TestAccApplication_appRolesDelete(t *testing.T) { }, data.ImportStep(), { - Config: testAccApplication_appRoles(data.RandomInteger), + Config: testAccApplication_appRolesEmpty(data.RandomInteger), Check: resource.ComposeTestCheckFunc( testCheckApplicationExists(data.ResourceName), - resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "1"), + resource.TestCheckResourceAttr(data.ResourceName, "app_role.#", "0"), ), }, data.ImportStep(), @@ -490,7 +478,7 @@ func TestAccApplication_oauth2PermissionsUpdate(t *testing.T) { }) } -func TestAccApplication_preventDuplicateNames(t *testing.T) { +func TestAccApplication_preventDuplicateNamesOk(t *testing.T) { data := acceptance.BuildTestData(t, "azuread_application", "test") resource.ParallelTest(t, resource.TestCase{ @@ -499,7 +487,27 @@ func TestAccApplication_preventDuplicateNames(t *testing.T) { CheckDestroy: testCheckApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccApplication_duplicateName(data.RandomInteger), + Config: testAccApplication_preventDuplicateNamesOk(data.RandomInteger), + Check: resource.ComposeTestCheckFunc( + testCheckApplicationExists(data.ResourceName), + resource.TestCheckResourceAttr(data.ResourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", data.RandomInteger)), + ), + }, + data.ImportStep("prevent_duplicate_names"), + }, + }) +} + +func TestAccApplication_preventDuplicateNamesFail(t *testing.T) { + data := acceptance.BuildTestData(t, "azuread_application", "test") + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { acceptance.PreCheck(t) }, + Providers: acceptance.SupportedProviders, + CheckDestroy: testCheckApplicationDestroy, + Steps: []resource.TestStep{ + { + Config: testAccApplication_preventDuplicateNamesFail(data.RandomInteger), ExpectError: regexp.MustCompile("existing Application .+ was found"), }, }, @@ -842,6 +850,16 @@ resource "azuread_application" "test" { `, ri) } +func testAccApplication_appRolesEmpty(ri int) string { + return fmt.Sprintf(` +resource "azuread_application" "test" { + name = "acctestApp-%d" + + app_role = [] +} +`, ri) +} + func testAccApplication_oauth2Permissions(ri int) string { return fmt.Sprintf(` resource "azuread_application" "test" { @@ -906,7 +924,16 @@ resource "azuread_application" "test" { `, ri) } -func testAccApplication_duplicateName(ri int) string { +func testAccApplication_preventDuplicateNamesOk(ri int) string { + return fmt.Sprintf(` +resource "azuread_application" "test" { + name = "acctest-APP-%[1]d" + prevent_duplicate_names = true +} +`, ri) +} + +func testAccApplication_preventDuplicateNamesFail(ri int) string { return fmt.Sprintf(` %s diff --git a/internal/services/aadgraph/graph/application.go b/internal/services/aadgraph/graph/application.go index 1913227e9b..802a17b97f 100644 --- a/internal/services/aadgraph/graph/application.go +++ b/internal/services/aadgraph/graph/application.go @@ -4,6 +4,7 @@ import ( "context" "errors" "fmt" + "reflect" "strings" "github.com/Azure/azure-sdk-for-go/services/graphrbac/1.6/graphrbac" @@ -449,6 +450,56 @@ func AppRoleResultRemoveById(existing *[]graphrbac.AppRole, roleId string) *[]gr return &newRoles } +func AppRolesSet(ctx context.Context, client *graphrbac.ApplicationsClient, appId string, newRoles *[]graphrbac.AppRole) error { + // don't support setting nil roles, the sdk ignores them + // should instead be zero length slice of AppRole + if newRoles == nil { + return fmt.Errorf("cannot set nil App Roles for Application with ID %q", appId) + } + + // Roles must be disabled before they can be edited or removed. + // Since we cannot match them by ID, we have to disable all the roles, and replace them in one pass. + app, err := client.Get(ctx, appId) + if err != nil { + if utils.ResponseWasNotFound(app.Response) { + return fmt.Errorf("application with ID %q was not found", appId) + } + + return fmt.Errorf("retrieving Application with ID %q: %+v", appId, err) + } + + // don't update if no changes to be made + if app.AppRoles != nil && reflect.DeepEqual(*app.AppRoles, *newRoles) { + return nil + } + + // first disable any existing permissions + properties := graphrbac.ApplicationUpdateParameters{ + AppRoles: app.AppRoles, + } + + if properties.AppRoles != nil { + for _, role := range *properties.AppRoles { + *role.IsEnabled = false + } + + if _, err := client.Patch(ctx, appId, properties); err != nil { + return fmt.Errorf("disabling App Roles for Application with ID %q: %+v", appId, err) + } + } + + // then set the new permissions + properties = graphrbac.ApplicationUpdateParameters{ + AppRoles: newRoles, + } + + if _, err := client.Patch(ctx, appId, properties); err != nil { + return fmt.Errorf("setting App Roles for Application with ID %q: %+v", appId, err) + } + + return nil +} + type OAuth2PermissionId struct { ObjectId string PermissionId string @@ -592,3 +643,53 @@ func OAuth2PermissionResultRemoveById(existing *[]graphrbac.OAuth2Permission, pe return &newPermissions, nil } + +func OAuth2PermissionsSet(ctx context.Context, client *graphrbac.ApplicationsClient, appId string, newPermissions *[]graphrbac.OAuth2Permission) error { + // don't support setting nil permissions, the sdk ignores them + // should instead be zero length slice of OAuth2Permission + if newPermissions == nil { + return fmt.Errorf("cannot set nil OAuth2 Permissions for Application with ID %q", appId) + } + + // Permissions must be disabled before they can be edited or removed. + // Since we cannot match them by ID, we have to disable all the permissions, and replace them in one pass. + app, err := client.Get(ctx, appId) + if err != nil { + if utils.ResponseWasNotFound(app.Response) { + return fmt.Errorf("application with ID %q was not found", appId) + } + + return fmt.Errorf("retrieving Application with ID %q: %+v", appId, err) + } + + // don't update if no changes to be made + if app.Oauth2Permissions != nil && reflect.DeepEqual(*app.Oauth2Permissions, *newPermissions) { + return nil + } + + // first disable any existing permissions + properties := graphrbac.ApplicationUpdateParameters{ + Oauth2Permissions: app.Oauth2Permissions, + } + + if properties.Oauth2Permissions != nil { + for _, permission := range *properties.Oauth2Permissions { + *permission.IsEnabled = false + } + + if _, err := client.Patch(ctx, appId, properties); err != nil { + return fmt.Errorf("disabling OAuth2 Permissions for Application with ID %q: %+v", appId, err) + } + } + + // then set the new permissions + properties = graphrbac.ApplicationUpdateParameters{ + Oauth2Permissions: newPermissions, + } + + if _, err := client.Patch(ctx, appId, properties); err != nil { + return fmt.Errorf("setting OAuth2 Permissions for Application with ID %q: %+v", appId, err) + } + + return nil +}