From beb62b38992b0676f7307e3663bbeb0786b323dc Mon Sep 17 00:00:00 2001
From: Tom Bamford <tom@bamford.io>
Date: Fri, 26 Jun 2020 03:58:42 +0100
Subject: [PATCH] azuread_application: validate values for app_role and
 oauth2_permissions to screen for duplicates

---
 azuread/resource_application.go          | 42 ++++++++++++++++++++++++
 azuread/resource_application_test.go     | 40 ++++++++++++++++++++++
 website/docs/r/application.html.markdown |  2 ++
 3 files changed, 84 insertions(+)

diff --git a/azuread/resource_application.go b/azuread/resource_application.go
index ba397e9de2..4cf9654344 100644
--- a/azuread/resource_application.go
+++ b/azuread/resource_application.go
@@ -307,6 +307,10 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	if err := adApplicationValidateRolesScopes(d.Get("app_role"), d.Get("oauth2_permissions")); err != nil {
+		return err
+	}
+
 	appType := d.Get("type")
 	identUrls, hasIdentUrls := d.GetOk("identifier_uris")
 	if appType == "native" {
@@ -420,6 +424,10 @@ func resourceApplicationUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
+	if err := adApplicationValidateRolesScopes(d.Get("app_role"), d.Get("oauth2_permissions")); err != nil {
+		return err
+	}
+
 	var properties graphrbac.ApplicationUpdateParameters
 
 	if d.HasChange("name") {
@@ -928,3 +936,37 @@ func adApplicationSetOwnersTo(client graphrbac.ApplicationsClient, ctx context.C
 
 	return nil
 }
+
+func adApplicationValidateRolesScopes(appRoles, oauth2Permissions interface{}) error {
+	var values []string
+
+	if appRoles != nil {
+		for _, roleRaw := range appRoles.(*schema.Set).List() {
+			role := roleRaw.(map[string]interface{})
+			if val := role["value"].(string); val != "" {
+				values = append(values, val)
+			}
+		}
+	}
+
+	if oauth2Permissions != nil {
+		for _, scopeRaw := range oauth2Permissions.(*schema.Set).List() {
+			scope := scopeRaw.(map[string]interface{})
+			if val := scope["value"].(string); val != "" {
+				values = append(values, val)
+			}
+		}
+	}
+
+	encountered := make([]string, len(values))
+	for _, val := range values {
+		for _, en := range encountered {
+			if en == val {
+				return fmt.Errorf("validation failed: duplicate app_role / oauth2_permissions value found: %q", val)
+			}
+		}
+		encountered = append(encountered, val)
+	}
+
+	return nil
+}
diff --git a/azuread/resource_application_test.go b/azuread/resource_application_test.go
index 27d9a62712..dab8eb1298 100644
--- a/azuread/resource_application_test.go
+++ b/azuread/resource_application_test.go
@@ -600,6 +600,22 @@ func TestAccAzureADApplication_preventDuplicateNames(t *testing.T) {
 	})
 }
 
+func TestAccAzureADApplication_duplicateAppRolesOauth2PermissionsValues(t *testing.T) {
+	ri := tf.AccRandTimeInt()
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testCheckADApplicationDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccADApplication_duplicateAppRolesOauth2PermissionsValues(ri),
+				ExpectError: regexp.MustCompile("validation failed: duplicate app_role / oauth2_permissions value found:"),
+			},
+		},
+	})
+}
+
 func testCheckADApplicationExists(name string) resource.TestCheckFunc {
 	return func(s *terraform.State) error {
 		rs, ok := s.RootModule().Resources[name]
@@ -940,3 +956,27 @@ resource "azuread_application" "duplicate" {
 }
 `, testAccADApplication_basic(ri))
 }
+
+func testAccADApplication_duplicateAppRolesOauth2PermissionsValues(ri int) string {
+	return fmt.Sprintf(`
+resource "azuread_application" "test" {
+  name = "acctest-APP-%[1]d"
+
+  app_role {
+    allowed_member_types = ["User"]
+    description          = "Admins can manage roles and perform all task actions"
+    display_name         = "Admin"
+    is_enabled           = true
+    value                = "administer"
+  }
+
+  oauth2_permissions {
+    admin_consent_description  = "Administer the application"
+    admin_consent_display_name = "Administer"
+    is_enabled                 = true
+    type                       = "Admin"
+    value                      = "administer"
+  }
+}
+`, ri)
+}
diff --git a/website/docs/r/application.html.markdown b/website/docs/r/application.html.markdown
index 070511245f..61cf5fa232 100644
--- a/website/docs/r/application.html.markdown
+++ b/website/docs/r/application.html.markdown
@@ -137,6 +137,8 @@ The following arguments are supported:
 
 * `oauth2_permissions` - (Optional) A collection of OAuth 2.0 permission scopes that the web API (resource) app exposes to client apps. Each permission is covered by `oauth2_permissions` blocks as documented below.
 
+-> **Note on roles and scopes/permissions:** in Azure Active Directory, roles (`app_role`) and scopes/permissions (`oauth2_permissions`) exported by an Application share the same namespace and cannot contain duplicate values. Terraform will attempt to detect this at plan time.
+
 * `prevent_duplicate_names` - (Optional) If `true`, will return an error when an existing Application is found with the same name. Defaults to `false`.
 
 ---