From d597fe986656f1df292c790cb593d06f9cff634d Mon Sep 17 00:00:00 2001 From: kt Date: Wed, 11 Mar 2020 16:11:10 -0700 Subject: [PATCH 1/5] azuread_application - add logout_url & fix owner add/delete order --- azuread/data_application.go | 6 ++ azuread/resource_application.go | 38 ++++++--- azuread/resource_application_test.go | 99 +++++++++++++++--------- website/docs/r/application.html.markdown | 2 + 4 files changed, 99 insertions(+), 46 deletions(-) diff --git a/azuread/data_application.go b/azuread/data_application.go index 1996948e59..8c431c7c66 100644 --- a/azuread/data_application.go +++ b/azuread/data_application.go @@ -54,6 +54,11 @@ func dataApplication() *schema.Resource { }, }, + "logout_url": { + Type: schema.TypeString, + Computed: true, + }, + "available_to_other_tenants": { Type: schema.TypeBool, Computed: true, @@ -182,6 +187,7 @@ func dataApplicationRead(d *schema.ResourceData, meta interface{}) error { d.Set("name", app.DisplayName) d.Set("application_id", app.AppID) d.Set("homepage", app.Homepage) + d.Set("logout_url", app.LogoutURL) d.Set("available_to_other_tenants", app.AvailableToOtherTenants) d.Set("oauth2_allow_implicit_flow", app.Oauth2AllowImplicitFlow) diff --git a/azuread/resource_application.go b/azuread/resource_application.go index 056310600e..e071ab7845 100644 --- a/azuread/resource_application.go +++ b/azuread/resource_application.go @@ -70,6 +70,12 @@ func resourceApplication() *schema.Resource { }, }, + "logout_url": { + Type: schema.TypeString, + Optional: true, + ValidateFunc: validate.URLIsHTTPOrHTTPS, + }, + "oauth2_allow_implicit_flow": { Type: schema.TypeBool, Optional: true, @@ -226,12 +232,12 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error { DisplayName: &name, IdentifierUris: tf.ExpandStringSlicePtr(identUrls.([]interface{})), ReplyUrls: tf.ExpandStringSlicePtr(d.Get("reply_urls").(*schema.Set).List()), - AvailableToOtherTenants: p.Bool(d.Get("available_to_other_tenants").(bool)), + AvailableToOtherTenants: p.BoolI(d.Get("available_to_other_tenants")), RequiredResourceAccess: expandADApplicationRequiredResourceAccess(d), } if v, ok := d.GetOk("homepage"); ok { - properties.Homepage = p.String(v.(string)) + properties.Homepage = p.StringI(v) } else { // continue to automatically set the homepage with the type is not native if appType != "native" { @@ -239,12 +245,16 @@ func resourceApplicationCreate(d *schema.ResourceData, meta interface{}) error { } } + if v, ok := d.GetOk("logout_url"); ok { + properties.LogoutURL = p.StringI(v) + } + if v, ok := d.GetOk("oauth2_allow_implicit_flow"); ok { - properties.Oauth2AllowImplicitFlow = p.Bool(v.(bool)) + properties.Oauth2AllowImplicitFlow = p.BoolI(v) } if v, ok := d.GetOk("public_client"); ok { - properties.PublicClient = p.Bool(v.(bool)) + properties.PublicClient = p.BoolI(v) } if v, ok := d.GetOk("group_membership_claims"); ok { @@ -317,7 +327,11 @@ func resourceApplicationUpdate(d *schema.ResourceData, meta interface{}) error { } if d.HasChange("homepage") { - properties.Homepage = p.String(d.Get("homepage").(string)) + properties.Homepage = p.StringI(d.Get("homepage")) + } + + if v, ok := d.GetOk("logout_url"); ok { + properties.LogoutURL = p.StringI(v) } if d.HasChange("identifier_uris") { @@ -329,15 +343,15 @@ func resourceApplicationUpdate(d *schema.ResourceData, meta interface{}) error { } if d.HasChange("available_to_other_tenants") { - properties.AvailableToOtherTenants = p.Bool(d.Get("available_to_other_tenants").(bool)) + properties.AvailableToOtherTenants = p.BoolI(d.Get("available_to_other_tenants")) } if d.HasChange("oauth2_allow_implicit_flow") { - properties.Oauth2AllowImplicitFlow = p.Bool(d.Get("oauth2_allow_implicit_flow").(bool)) + properties.Oauth2AllowImplicitFlow = p.BoolI(d.Get("oauth2_allow_implicit_flow")) } if d.HasChange("public_client") { - properties.PublicClient = p.Bool(d.Get("public_client").(bool)) + properties.PublicClient = p.BoolI(d.Get("public_client").(bool)) } if d.HasChange("required_resource_access") { @@ -419,6 +433,7 @@ func resourceApplicationRead(d *schema.ResourceData, meta interface{}) error { d.Set("name", app.DisplayName) d.Set("application_id", app.AppID) d.Set("homepage", app.Homepage) + d.Set("logout_url", app.LogoutURL) d.Set("available_to_other_tenants", app.AvailableToOtherTenants) d.Set("oauth2_allow_implicit_flow", app.Oauth2AllowImplicitFlow) d.Set("public_client", app.PublicClient) @@ -624,6 +639,11 @@ func adApplicationSetOwnersTo(client graphrbac.ApplicationsClient, ctx context.C ownersForRemoval := slices.Difference(existingOwners, desiredOwners) ownersToAdd := slices.Difference(desiredOwners, existingOwners) + // add owners first to prevent a possible situation where terraform revokes its own access before adding it back. + if err := graph.ApplicationAddOwners(client, ctx, id, ownersToAdd); err != nil { + return err + } + for _, ownerToDelete := range ownersForRemoval { log.Printf("[DEBUG] Removing member with id %q from Azure AD group with id %q", ownerToDelete, id) if resp, err := client.RemoveOwner(ctx, id, ownerToDelete); err != nil { @@ -633,5 +653,5 @@ func adApplicationSetOwnersTo(client graphrbac.ApplicationsClient, ctx context.C } } - return graph.ApplicationAddOwners(client, ctx, id, ownersToAdd) + return nil } diff --git a/azuread/resource_application_test.go b/azuread/resource_application_test.go index 108a745030..bbdcdf5983 100644 --- a/azuread/resource_application_test.go +++ b/azuread/resource_application_test.go @@ -45,9 +45,10 @@ func TestAccAzureADApplication_basic(t *testing.T) { }) } -func TestAccAzureADApplication_http_homepage(t *testing.T) { +func TestAccAzureADApplication_complete(t *testing.T) { resourceName := "azuread_application.test" ri := tf.AccRandTimeInt() + pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -55,15 +56,17 @@ func TestAccAzureADApplication_http_homepage(t *testing.T) { CheckDestroy: testCheckADApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccADApplication_http_homepage(ri), + Config: testAccADApplication_complete(ri, pw), Check: resource.ComposeTestCheckFunc( testCheckADApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), - resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://homepage-%d", ri)), - resource.TestCheckResourceAttr(resourceName, "oauth2_allow_implicit_flow", "false"), - resource.TestCheckResourceAttr(resourceName, "type", "webapp/api"), - resource.TestCheckResourceAttr(resourceName, "oauth2_permissions.#", "1"), - resource.TestCheckResourceAttr(resourceName, "oauth2_permissions.0.admin_consent_description", fmt.Sprintf("Allow the application to access %s on behalf of the signed-in user.", fmt.Sprintf("acctest-APP-%[1]d", ri))), + resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://homepage-%d", ri)), + resource.TestCheckResourceAttr(resourceName, "oauth2_allow_implicit_flow", "true"), + resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "1"), + resource.TestCheckResourceAttr(resourceName, "identifier_uris.0", fmt.Sprintf("http://%d.hashicorptest.com/00000000-0000-0000-0000-00000000", ri)), + resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "1"), + resource.TestCheckResourceAttr(resourceName, "group_membership_claims", "All"), + resource.TestCheckResourceAttr(resourceName, "required_resource_access.#", "2"), resource.TestCheckResourceAttrSet(resourceName, "application_id"), resource.TestCheckResourceAttrSet(resourceName, "object_id"), ), @@ -77,9 +80,10 @@ func TestAccAzureADApplication_http_homepage(t *testing.T) { }) } -func TestAccAzureADApplication_complete(t *testing.T) { +func TestAccAzureADApplication_update(t *testing.T) { resourceName := "azuread_application.test" ri := tf.AccRandTimeInt() + updatedri := tf.AccRandTimeInt() pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) resource.ParallelTest(t, resource.TestCase{ @@ -88,19 +92,45 @@ func TestAccAzureADApplication_complete(t *testing.T) { CheckDestroy: testCheckADApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccADApplication_complete(ri, pw), + Config: testAccADApplication_basic(ri), Check: resource.ComposeTestCheckFunc( testCheckADApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), - resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://homepage-%d", ri)), - resource.TestCheckResourceAttr(resourceName, "oauth2_allow_implicit_flow", "true"), + resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://acctest-APP-%d", ri)), + resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "0"), + resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "0"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccADApplication_complete(updatedri, pw), + Check: resource.ComposeTestCheckFunc( + testCheckADApplicationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", updatedri)), + resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://homepage-%d", updatedri)), resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "1"), - resource.TestCheckResourceAttr(resourceName, "identifier_uris.0", fmt.Sprintf("http://%d.hashicorptest.com/00000000-0000-0000-0000-00000000", ri)), + resource.TestCheckResourceAttr(resourceName, "identifier_uris.0", fmt.Sprintf("http://%d.hashicorptest.com/00000000-0000-0000-0000-00000000", updatedri)), resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "1"), - resource.TestCheckResourceAttr(resourceName, "group_membership_claims", "All"), + resource.TestCheckResourceAttr(resourceName, "reply_urls.3714513888", "http://unittest.hashicorptest.com"), resource.TestCheckResourceAttr(resourceName, "required_resource_access.#", "2"), - resource.TestCheckResourceAttrSet(resourceName, "application_id"), - resource.TestCheckResourceAttrSet(resourceName, "object_id"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + { + Config: testAccADApplication_basic(ri), + Check: resource.ComposeTestCheckFunc( + testCheckADApplicationExists(resourceName), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), + resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "0"), + resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "0"), ), }, { @@ -112,7 +142,7 @@ func TestAccAzureADApplication_complete(t *testing.T) { }) } -func TestAccAzureADApplication_publicClient(t *testing.T) { +func TestAccAzureADApplication_http_homepage(t *testing.T) { resourceName := "azuread_application.test" ri := tf.AccRandTimeInt() @@ -122,10 +152,17 @@ func TestAccAzureADApplication_publicClient(t *testing.T) { CheckDestroy: testCheckADApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccADApplication_publicClient(ri), + Config: testAccADApplication_http_homepage(ri), Check: resource.ComposeTestCheckFunc( testCheckADApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "public_client", "true"), + resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), + resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("http://homepage-%d", ri)), + resource.TestCheckResourceAttr(resourceName, "oauth2_allow_implicit_flow", "false"), + resource.TestCheckResourceAttr(resourceName, "type", "webapp/api"), + resource.TestCheckResourceAttr(resourceName, "oauth2_permissions.#", "1"), + resource.TestCheckResourceAttr(resourceName, "oauth2_permissions.0.admin_consent_description", fmt.Sprintf("Allow the application to access %s on behalf of the signed-in user.", fmt.Sprintf("acctest-APP-%[1]d", ri))), + resource.TestCheckResourceAttrSet(resourceName, "application_id"), + resource.TestCheckResourceAttrSet(resourceName, "object_id"), ), }, { @@ -137,11 +174,9 @@ func TestAccAzureADApplication_publicClient(t *testing.T) { }) } -func TestAccAzureADApplication_update(t *testing.T) { +func TestAccAzureADApplication_publicClient(t *testing.T) { resourceName := "azuread_application.test" ri := tf.AccRandTimeInt() - updatedri := tf.AccRandTimeInt() - pw := "p@$$wR2" + acctest.RandStringFromCharSet(7, acctest.CharSetAlphaNum) resource.ParallelTest(t, resource.TestCase{ PreCheck: func() { testAccPreCheck(t) }, @@ -149,27 +184,16 @@ func TestAccAzureADApplication_update(t *testing.T) { CheckDestroy: testCheckADApplicationDestroy, Steps: []resource.TestStep{ { - Config: testAccADApplication_basic(ri), + Config: testAccADApplication_publicClient(ri), Check: resource.ComposeTestCheckFunc( testCheckADApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), - resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://acctest-APP-%d", ri)), - resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "0"), - resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "0"), + resource.TestCheckResourceAttr(resourceName, "public_client", "true"), ), }, { - Config: testAccADApplication_complete(updatedri, pw), - Check: resource.ComposeTestCheckFunc( - testCheckADApplicationExists(resourceName), - resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", updatedri)), - resource.TestCheckResourceAttr(resourceName, "homepage", fmt.Sprintf("https://homepage-%d", updatedri)), - resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "1"), - resource.TestCheckResourceAttr(resourceName, "identifier_uris.0", fmt.Sprintf("http://%d.hashicorptest.com/00000000-0000-0000-0000-00000000", updatedri)), - resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "1"), - resource.TestCheckResourceAttr(resourceName, "reply_urls.3714513888", "http://unittest.hashicorptest.com"), - resource.TestCheckResourceAttr(resourceName, "required_resource_access.#", "2"), - ), + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, }, }, }) @@ -610,6 +634,7 @@ resource "azuread_application" "test" { homepage = "https://homepage-%[2]d" identifier_uris = ["http://%[2]d.hashicorptest.com/00000000-0000-0000-0000-00000000"] reply_urls = ["http://unittest.hashicorptest.com"] + logout_url = "http://log.me.out" oauth2_allow_implicit_flow = true group_membership_claims = "All" diff --git a/website/docs/r/application.html.markdown b/website/docs/r/application.html.markdown index 959faf1264..e2261a56ee 100644 --- a/website/docs/r/application.html.markdown +++ b/website/docs/r/application.html.markdown @@ -80,6 +80,8 @@ The following arguments are supported: * `reply_urls` - (Optional) A list of URLs that user tokens are sent to for sign in, or the redirect URIs that OAuth 2.0 authorization codes and access tokens are sent to. +* `logout_url` - (Optional) The URL of the logout page. + * `available_to_other_tenants` - (Optional) Is this Azure AD Application available to other tenants? Defaults to `false`. * `public_client` - (Optional) Is this Azure AD Application a public client? Defaults to `false`. From d1ec165c069b9a5663637af0842b819d47d39501 Mon Sep 17 00:00:00 2001 From: kt Date: Wed, 11 Mar 2020 19:24:40 -0700 Subject: [PATCH 2/5] add logout_url to datasource --- website/docs/d/application.html.markdown | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/website/docs/d/application.html.markdown b/website/docs/d/application.html.markdown index 5f202531f6..0213f08c3d 100644 --- a/website/docs/d/application.html.markdown +++ b/website/docs/d/application.html.markdown @@ -34,6 +34,8 @@ output "azure_ad_object_id" { ## Attributes Reference +The following attributes are exported: + * `id` - the Object ID of the Azure Active Directory Application. * `application_id` - the Application ID of the Azure Active Directory Application. @@ -42,6 +44,8 @@ output "azure_ad_object_id" { * `identifier_uris` - A list of user-defined URI(s) that uniquely identify a Web application within it's Azure AD tenant, or within a verified custom domain if the application is multi-tenant. +* `logout_url` - (Optional) The URL of the logout page. + * `oauth2_allow_implicit_flow` - Does this Azure AD Application allow OAuth2.0 implicit flow tokens? * `object_id` - the Object ID of the Azure Active Directory Application. From 0d88c46e55ae948417f49d954061ee9afc158df3 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 12 Mar 2020 11:26:44 -0700 Subject: [PATCH 3/5] fix test --- azuread/resource_application_test.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/azuread/resource_application_test.go b/azuread/resource_application_test.go index bbdcdf5983..9ad93c7e74 100644 --- a/azuread/resource_application_test.go +++ b/azuread/resource_application_test.go @@ -125,7 +125,7 @@ func TestAccAzureADApplication_update(t *testing.T) { ImportStateVerify: true, }, { - Config: testAccADApplication_basic(ri), + Config: testAccADApplication_basicEmpty(ri), Check: resource.ComposeTestCheckFunc( testCheckADApplicationExists(resourceName), resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)), @@ -570,6 +570,16 @@ resource "azuread_application" "test" { `, ri) } +func testAccADApplication_basicEmpty(ri int) string { + return fmt.Sprintf(` +resource "azuread_application" "test" { + name = "acctest-APP-%[1]d" + identifier_uris = [] + reply_urls = [] +} +`, ri) +} + func testAccADApplication_http_homepage(ri int) string { return fmt.Sprintf(` resource "azuread_application" "test" { From 7c4b0682930b1f70674317066ad46830f94440a9 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 12 Mar 2020 11:27:32 -0700 Subject: [PATCH 4/5] Update website/docs/d/application.html.markdown Co-Authored-By: Matthew Frahry --- website/docs/d/application.html.markdown | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/docs/d/application.html.markdown b/website/docs/d/application.html.markdown index 0213f08c3d..a50bd51ab7 100644 --- a/website/docs/d/application.html.markdown +++ b/website/docs/d/application.html.markdown @@ -44,7 +44,7 @@ The following attributes are exported: * `identifier_uris` - A list of user-defined URI(s) that uniquely identify a Web application within it's Azure AD tenant, or within a verified custom domain if the application is multi-tenant. -* `logout_url` - (Optional) The URL of the logout page. +* `logout_url` - The URL of the logout page. * `oauth2_allow_implicit_flow` - Does this Azure AD Application allow OAuth2.0 implicit flow tokens? From 7dd845a99d762e0e208b4a8bc4aa6ac29a56c5b7 Mon Sep 17 00:00:00 2001 From: kt Date: Thu, 12 Mar 2020 11:56:19 -0700 Subject: [PATCH 5/5] Fix acctests --- azuread/resource_application.go | 4 ++-- azuread/resource_application_test.go | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/azuread/resource_application.go b/azuread/resource_application.go index e071ab7845..8c94a4665d 100644 --- a/azuread/resource_application.go +++ b/azuread/resource_application.go @@ -330,8 +330,8 @@ func resourceApplicationUpdate(d *schema.ResourceData, meta interface{}) error { properties.Homepage = p.StringI(d.Get("homepage")) } - if v, ok := d.GetOk("logout_url"); ok { - properties.LogoutURL = p.StringI(v) + if d.HasChange("logout_url") { + properties.LogoutURL = p.StringI(d.Get("logout_url")) } if d.HasChange("identifier_uris") { diff --git a/azuread/resource_application_test.go b/azuread/resource_application_test.go index 9ad93c7e74..fe13a6f57f 100644 --- a/azuread/resource_application_test.go +++ b/azuread/resource_application_test.go @@ -573,9 +573,10 @@ resource "azuread_application" "test" { func testAccADApplication_basicEmpty(ri int) string { return fmt.Sprintf(` resource "azuread_application" "test" { - name = "acctest-APP-%[1]d" - identifier_uris = [] - reply_urls = [] + name = "acctest-APP-%[1]d" + identifier_uris = [] + reply_urls = [] + group_membership_claims = "None" } `, ri) }