Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

azuread_application - add logout_url & fix owner add/delete order #226

Merged
merged 7 commits into from
Mar 12, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions azuread/data_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ func dataApplication() *schema.Resource {
},
},

"logout_url": {
Type: schema.TypeString,
Computed: true,
},

"available_to_other_tenants": {
Type: schema.TypeBool,
Computed: true,
Expand Down Expand Up @@ -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)

Expand Down
38 changes: 29 additions & 9 deletions azuread/resource_application.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -226,25 +232,29 @@ 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" {
properties.Homepage = p.String(fmt.Sprintf("https://%s", name))
}
}

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 {
Expand Down Expand Up @@ -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 d.HasChange("logout_url") {
properties.LogoutURL = p.StringI(d.Get("logout_url"))
}

if d.HasChange("identifier_uris") {
Expand All @@ -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") {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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.
katbyte marked this conversation as resolved.
Show resolved Hide resolved
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 {
Expand All @@ -633,5 +653,5 @@ func adApplicationSetOwnersTo(client graphrbac.ApplicationsClient, ctx context.C
}
}

return graph.ApplicationAddOwners(client, ctx, id, ownersToAdd)
return nil
}
110 changes: 73 additions & 37 deletions azuread/resource_application_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,28 @@ 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) },
Providers: testAccProviders,
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"),
),
Expand All @@ -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{
Expand All @@ -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_basicEmpty(ri),
Check: resource.ComposeTestCheckFunc(
testCheckADApplicationExists(resourceName),
resource.TestCheckResourceAttr(resourceName, "name", fmt.Sprintf("acctest-APP-%[1]d", ri)),
resource.TestCheckResourceAttr(resourceName, "identifier_uris.#", "0"),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like identifier_uris don't get removed

    testing.go:640: Step 4 error: Check failed: Check 3/4 error: azuread_application.test: Attribute 'identifier_uris.#' expected "0", got "1"

resource.TestCheckResourceAttr(resourceName, "reply_urls.#", "0"),
),
},
{
Expand All @@ -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()

Expand All @@ -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"),
),
},
{
Expand All @@ -137,39 +174,26 @@ 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) },
Providers: testAccProviders,
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,
},
},
})
Expand Down Expand Up @@ -546,6 +570,17 @@ 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 = []
group_membership_claims = "None"
}
`, ri)
}

func testAccADApplication_http_homepage(ri int) string {
return fmt.Sprintf(`
resource "azuread_application" "test" {
Expand Down Expand Up @@ -610,6 +645,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"
Expand Down
4 changes: 4 additions & 0 deletions website/docs/d/application.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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` - 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.
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/application.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down