Merge pull request #971 from Tsanton/main

fix: oauth2Permissions and appRoles allow dupes

.gitignore

@@ -1,3 +1,4 @@
+.devcontainer/
 *.dll
 *.exe
 .DS_Store

internal/services/applications/application_resource_test.go

@@ -214,6 +214,35 @@ func TestAccApplication_duplicateAppRolesOauth2PermissionsValues(t *testing.T) {
 	})
 }
 
+func TestAccApplication_duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithCommonMetadata(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azuread_application", "test")
+	r := ApplicationResource{}
+
+	data.ResourceTest(t, r, []resource.TestStep{
+		{
+			Config: r.duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithCommonMetadata(data),
+			Check: resource.ComposeTestCheckFunc(
+				check.That(data.ResourceName).Key("app_role.#").HasValue("1"),
+				check.That(data.ResourceName).Key("app_role_ids.%").HasValue("1"),
+				check.That(data.ResourceName).Key("api.0.oauth2_permission_scope.#").HasValue("1"),
+				check.That(data.ResourceName).Key("oauth2_permission_scope_ids.%").HasValue("1"),
+			),
+		},
+	})
+}
+
+func TestAccApplication_duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithMismatchingMetadata(t *testing.T) {
+	data := acceptance.BuildTestData(t, "azuread_application", "test")
+	r := ApplicationResource{}
+
+	data.ResourceTest(t, r, []resource.TestStep{
+		{
+			Config:      r.duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithMismatchingMetadata(data),
+			ExpectError: regexp.MustCompile("validation failed: The following values must match for the"),
+		},
+	})
+}
+
 func TestAccApplication_groupMembershipClaimsUpdate(t *testing.T) {
 	data := acceptance.BuildTestData(t, "azuread_application", "test")
 	r := ApplicationResource{}
@@ -1236,11 +1265,6 @@ resource "azuread_application" "test" {
 func (ApplicationResource) duplicateAppRolesOauth2PermissionsIdsUnknown(data acceptance.TestData) string {
 	return fmt.Sprintf(`
 provider "azuread" {}
-provider "random" {}
-
-resource "random_uuid" "test" {
-  count = 2
-}
 
 resource "azuread_application" "test" {
   display_name = "acctest-APP-%[1]d"
@@ -1250,7 +1274,7 @@ resource "azuread_application" "test" {
       admin_consent_description  = "Administer the application"
       admin_consent_display_name = "Administer"
       enabled                    = true
-      id                         = random_uuid.test[0].id
+      id                         = "%[2]s"
       type                       = "Admin"
       value                      = "administer"
     }
@@ -1261,7 +1285,7 @@ resource "azuread_application" "test" {
     description          = "Admins can manage roles and perform all task actions"
     display_name         = "Admin"
     enabled              = true
-    id                   = random_uuid.test[1].id
+    id                   = "%[3]s"
     value                = "administrate"
   }
 }
@@ -1298,6 +1322,66 @@ resource "azuread_application" "test" {
 `, data.RandomInteger, data.UUID(), data.UUID())
 }
 
+func (ApplicationResource) duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithCommonMetadata(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azuread" {}
+
+resource "azuread_application" "test" {
+  display_name = "acctest-APP-%[1]d"
+
+  api {
+    oauth2_permission_scope {
+      admin_consent_description  = "Administer the application"
+      admin_consent_display_name = "Administer"
+      enabled                    = true
+      id                         = "%[2]s"
+      type                       = "Admin"
+      value                      = "administer"
+    }
+  }
+
+  app_role {
+    allowed_member_types = ["User"]
+    description          = "Administer the application"
+    display_name         = "Administer"
+    enabled              = true
+    id                   = "%[2]s"
+    value                = "administer"
+  }
+}
+`, data.RandomInteger, data.UUID())
+}
+
+func (ApplicationResource) duplicateAppRolesOauth2PermissionsMatchingIdAndValueWithMismatchingMetadata(data acceptance.TestData) string {
+	return fmt.Sprintf(`
+provider "azuread" {}
+
+resource "azuread_application" "test" {
+  display_name = "acctest-APP-%[1]d"
+
+  api {
+    oauth2_permission_scope {
+      admin_consent_description  = "This should (see app_role[0].description"
+      admin_consent_display_name = "Administer"
+      enabled                    = true
+      id                         = "%[2]s"
+      type                       = "Admin"
+      value                      = "administer"
+    }
+  }
+
+  app_role {
+    allowed_member_types = ["User"]
+    description          = "Not work"
+    display_name         = "Administer"
+    enabled              = true
+    id                   = "%[2]s"
+    value                = "administer"
+  }
+}
+`, data.RandomInteger, data.UUID())
+}
+
 func (ApplicationResource) templateThreeUsers(data acceptance.TestData) string {
 	return fmt.Sprintf(`
 provider "azuread" {}

internal/services/applications/applications.go

@@ -161,7 +161,7 @@ func applicationDisableAppRoles(ctx context.Context, client *msgraph.Application
 
 func applicationDisableOauth2PermissionScopes(ctx context.Context, client *msgraph.ApplicationsClient, application *msgraph.Application, newScopes *[]msgraph.PermissionScope) error {
 	if application.ID() == nil {
-		return fmt.Errorf("Cannot use Application model with nil ID")
+		return fmt.Errorf("cannot use Application model with nil ID")
 	}
 
 	if newScopes == nil {
@@ -310,18 +310,29 @@ func applicationParseLogoImage(encodedImage string) (string, []byte, error) {
 }
 
 func applicationValidateRolesScopes(appRoles, oauth2Permissions []interface{}) error {
-	var ids, values []string
+	type appPermission struct {
+		id          string
+		displayName string
+		description string
+		enabled     bool
+		value       string
+	}
+	var appPermissions []appPermission
 
 	for _, roleRaw := range appRoles {
 		if roleRaw == nil {
 			continue
 		}
 		role := roleRaw.(map[string]interface{})
-		if id := role["id"].(string); tf.ValueIsNotEmptyOrUnknown(id) {
-			ids = append(ids, id)
+		permission := appPermission{
+			id:          role["id"].(string),
+			displayName: role["display_name"].(string),
+			description: role["description"].(string),
+			enabled:     role["enabled"].(bool),
+			value:       role["value"].(string),
 		}
-		if val := role["value"].(string); tf.ValueIsNotEmptyOrUnknown(val) {
-			values = append(values, val)
+		if tf.ValueIsNotEmptyOrUnknown(permission.id) && tf.ValueIsNotEmptyOrUnknown(permission.value) {
+			appPermissions = append(appPermissions, permission)
 		}
 	}
 
@@ -330,32 +341,35 @@ func applicationValidateRolesScopes(appRoles, oauth2Permissions []interface{}) e
 			continue
 		}
 		scope := scopeRaw.(map[string]interface{})
-		if id := scope["id"].(string); tf.ValueIsNotEmptyOrUnknown(id) {
-			ids = append(ids, id)
+		permission := appPermission{
+			id:          scope["id"].(string),
+			displayName: scope["admin_consent_display_name"].(string),
+			description: scope["admin_consent_description"].(string),
+			enabled:     scope["enabled"].(bool),
+			value:       scope["value"].(string),
 		}
-		if val := scope["value"].(string); tf.ValueIsNotEmptyOrUnknown(val) {
-			values = append(values, val)
+		if tf.ValueIsNotEmptyOrUnknown(permission.id) && tf.ValueIsNotEmptyOrUnknown(permission.value) {
+			appPermissions = append(appPermissions, permission)
 		}
 	}
 
-	encounteredIds := make([]string, 0)
-	for _, id := range ids {
-		for _, en := range encounteredIds {
-			if en == id {
-				return fmt.Errorf("validation failed: duplicate ID found: %q", id)
+	encounteredPermissions := make([]appPermission, 0)
+	for _, ap := range appPermissions {
+		for _, ep := range encounteredPermissions {
+			if ap.id == ep.id && ap.value != ep.value {
+				return fmt.Errorf("validation failed: duplicate ID found: %q", ap.id)
 			}
-		}
-		encounteredIds = append(encounteredIds, id)
-	}
-
-	encounteredValues := make([]string, 0)
-	for _, val := range values {
-		for _, en := range encounteredValues {
-			if en == val {
-				return fmt.Errorf("validation failed: duplicate value found: %q", val)
+			if ap.value == ep.value && ap.id != ep.id {
+				return fmt.Errorf("validation failed: duplicate value found: %q", ap.value)
+			}
+			if ap.value == ep.value && ap.id == ep.id && !reflect.DeepEqual(ap, ep) {
+				return fmt.Errorf(`validation failed: The following values must match for the
+				'oauth2Permissions' and 'appRoles' properties with identifier '%q': (description, adminConsentDescription),
+				(displayName, adminConsentDisplayName),(isEnabled,isEnabled),(origin, origin),(value, value).
+				Ensure that you are intending to have entries with the same identifier, and if so, are updating them together`, ap.id)
 			}
 		}
-		encounteredValues = append(encounteredValues, val)
+		encounteredPermissions = append(encounteredPermissions, ap)
 	}
 
 	return nil