Merge pull request #519 from hashicorp/bugfix/ownership-alignment

Fixes for some object ownership issues with applications, groups and service principals
hashicorp · Aug 20, 2021 · d25f966 · d25f966
2 parents 8bb1a9c + 2b368bf
commit d25f966
Show file tree

Hide file tree

Showing 52 changed files with 2,257 additions and 575 deletions.
diff --git a/docs/resources/application.md b/docs/resources/application.md
@@ -145,7 +145,10 @@ The following arguments are supported:
 * `marketing_url` - (Optional) URL of the application's marketing page.
 * `oauth2_post_response_required` - (Optional) Specifies whether, as part of OAuth 2.0 token requests, Azure AD allows POST requests, as opposed to GET requests. Defaults to `false`, which specifies that only GET requests are allowed.
 * `optional_claims` - (Optional) An `optional_claims` block as documented below.
-* `owners` - (Optional) A list of object IDs of principals that will be granted ownership of the application. It's recommended to specify the object ID of the authenticated principal running Terraform, to ensure sufficient permissions that the application can be subsequently updated.
+* `owners` - (Optional) A set of object IDs of principals that will be granted ownership of the application. Supported object types are users or service principals. By default, no owners are assigned.
+
+-> **Ownership of Applications** It's recommended to always specify one or more application owners, including the principal being used to execute Terraform, such as in the example above.
+
 * `prevent_duplicate_names` - (Optional) If `true`, will return an error if an existing application is found with the same name. Defaults to `false`.
 * `privacy_statement_url` - (Optional) URL of the application's privacy statement.
 * `public_client` - (Optional) A `public_client` block as documented below, which configures non-web app or non-web API application settings, for example mobile or other public clients such as an installed application running on a desktop device.

diff --git a/docs/resources/group.md b/docs/resources/group.md
@@ -19,35 +19,56 @@ When authenticated with a user principal, this resource requires one of the foll
 *Basic example*
 
 ```terraform
+data "azuread_client_config" "current" {}
+
 resource "azuread_group" "example" {
   display_name     = "example"
+  owners           = [data.azuread_client_config.current.object_id]
   security_enabled = true
 }
 ```
 
 *Microsoft 365 group*
 
 ```terraform
+data "azuread_client_config" "current" {}
+
+resource "azuread_user" "group_owner" {
+  user_principal_name = "[email protected]"
+  display_name        = "Group Owner"
+  mail_nickname       = "example-group-owner"
+  password            = "SecretP@sswd99!"
+}
+
 resource "azuread_group" "example" {
   display_name     = "example"
   mail_enabled     = true
   mail_nickname    = "ExampleGroup"
   security_enabled = true
   types            = ["Unified"]
+
+  owners = [
+    data.azuread_client_config.current.object_id,
+    azuread_user.group_owner.object_id,
+  ]
 }
 ```
 
 *Group with members*
 
 ```terraform
+data "azuread_client_config" "current" {}
+
 resource "azuread_user" "example" {
   display_name        = "J Doe"
+  owners              = [data.azuread_client_config.current.object_id]
   password            = "notSecure123"
   user_principal_name = "[email protected]"
 }
 
 resource "azuread_group" "example" {
   display_name     = "MyGroup"
+  owners           = [data.azuread_client_config.current.object_id]
   security_enabled = true
 
   members = [
@@ -65,20 +86,21 @@ The following arguments are supported:
 * `behaviors` - (Optional) A set of behaviors for a Microsoft 365 group. Possible values are `AllowOnlyMembersToPost`, `HideGroupInOutlook`, `SubscribeNewGroupMembers` and `WelcomeEmailDisabled`. See [official documentation](https://docs.microsoft.com/en-us/graph/group-set-options) for more details. Changing this forces a new resource to be created.
 * `description` - (Optional) The description for the group.
 * `display_name` - (Required) The display name for the group.
-* `mail_enabled` - (Optional) Whether the group is a mail enabled, with a shared group mailbox. At least one of `mail_enabled` or `security_enabled` must be specified. A group can be mail enabled _and_ security enabled.
+* `mail_enabled` - (Optional) Whether the group is a mail enabled, with a shared group mailbox. At least one of `mail_enabled` or `security_enabled` must be specified. Only Microsoft 365 groups can be mail enabled (see the `types` property).
 * `mail_nickname` - (Optional) The mail alias for the group, unique in the organisation. Required for mail-enabled groups. Changing this forces a new resource to be created.
 * `members` - (Optional) A set of members who should be present in this group. Supported object types are Users, Groups or Service Principals.
-* `owners` - (Optional) A set of owners who own this group. Supported object types are Users or Service Principals.
-
-~> **Group Ownership and Permissions** Terraform always adds its own principal as a group owner to ensure that groups can continue to be managed. If using a user principal to execute Terraform, we recommend assigning the directory role `Groups Administrator` (or a role with the same effective permissions) to that user, in order to help prevent scenarios where groups may become unmanageable without administrative intervention.
+* `owners` - (Optional) A set of object IDs of principals that will be granted ownership of the group. Supported object types are users or service principals. By default, the principal being used to execute Terraform is assigned as the sole owner. Groups cannot be created with no owners or have all their owners removed.
 
--> **Ownership of Microsoft 365 Groups** Microsoft 365 groups are required to have at least one owner which _must be a user_ (i.e. not a service principal). If you are authenticated with an Azure AD user principal, you do not need to specify any owners for a group, although we suggest always specifying at least one user as an owner in your configuration.
+-> **Group Ownership**  It's recommended to always specify one or more group owners, including the principal being used to execute Terraform, such as in the example above. When removing group owners, if a user principal has been assigned ownership, the last user cannot be removed as an owner. Microsoft 365 groups are required to always have at least one owner which _must be a user_ (i.e. not a service principal).
 
 * `prevent_duplicate_names` - (Optional) If `true`, will return an error if an existing group is found with the same name. Defaults to `false`.
 * `provisioning_options` - (Optional) A set of provisioning options for a Microsoft 365 group. The only supported value is `Team`. See [official documentation](https://docs.microsoft.com/en-us/graph/group-set-options) for details. Changing this forces a new resource to be created.
-* `security_enabled` - (Optional) Whether the group is a security group for controlling access to in-app resources. At least one of `security_enabled` or `mail_enabled` must be specified. A group can be security enabled _and_ mail enabled.
+* `security_enabled` - (Optional) Whether the group is a security group for controlling access to in-app resources. At least one of `security_enabled` or `mail_enabled` must be specified. A Microsoft 365 group can be security enabled _and_ mail enabled (see the `types` property).
 * `theme` - (Optional) The colour theme for a Microsoft 365 group. Possible values are `Blue`, `Green`, `Orange`, `Pink`, `Purple`, `Red` or `Teal`. By default, no theme is set.
 * `types` - (Optional) A set of group types to configure for the group. The only supported type is `Unified`, which specifies a Microsoft 365 group. Required when `mail_enabled` is true. Changing this forces a new resource to be created.
+
+-> **Supported Group Types** At present, only security groups and Microsoft 365 groups can be created or managed with this resource. Distribution groups and mail-enabled security groups are not supported. Microsoft 365 groups can be security-enabled.
+
 * `visibility` - (Optional) The group join policy and group content visibility. Possible values are `Private`, `Public`, or `Hiddenmembership`. Only Microsoft 365 groups can have `Hiddenmembership` visibility and this value must be set when the group is created. By default, security groups will receive `Private` visibility and Microsoft 365 groups will receive `Public` visibility.
 
 -> **Group Name Uniqueness** Group names are not unique within Azure Active Directory. Use the `prevent_duplicate_names` argument to check for existing groups if you want to avoid name collisions.

diff --git a/docs/resources/service_principal.md b/docs/resources/service_principal.md
@@ -17,13 +17,17 @@ When authenticated with a user principal, this resource requires one of the foll
 ## Example Usage
 
 ```terraform
+data "azuread_client_config" "current" {}
+
 resource "azuread_application" "example" {
   display_name = "example"
+  owners       = [data.azuread_client_config.current.object_id]
 }
 
 resource "azuread_service_principal" "example" {
   application_id               = azuread_application.example.application_id
   app_role_assignment_required = false
+  owners                       = [data.azuread_client_config.current.object_id]
 
   tags = ["example", "tags", "here"]
 }
@@ -41,6 +45,10 @@ The following arguments are supported:
 * `login_url` - (Optional) The URL where the service provider redirects the user to Azure AD to authenticate. Azure AD uses the URL to launch the application from Microsoft 365 or the Azure AD My Apps. When blank, Azure AD performs IdP-initiated sign-on for applications configured with SAML-based single sign-on.
 * `notes` - (Optional) A free text field to capture information about the service principal, typically used for operational purposes.
 * `notification_email_addresses` - (Optional) A set of email addresses where Azure AD sends a notification when the active certificate is near the expiration date. This is only for the certificates used to sign the SAML token issued for Azure AD Gallery applications.
+* `owners` - (Optional) A set of object IDs of principals that will be granted ownership of the service principal. Supported object types are users or service principals. By default, no owners are assigned.
+
+-> **Ownership of Service Principals** It's recommended to always specify one or more service principal owners, including the principal being used to execute Terraform, such as in the example above.
+
 * `preferred_single_sign_on_mode` - (Optional) The single sign-on mode configured for this application. Azure AD uses the preferred single sign-on mode to launch the application from Microsoft 365 or the Azure AD My Apps. Supported values are `oidc`, `password`, `saml` or `notSupported`. Omit this property or specify a blank string to unset.
 * `tags` - (Optional) A set of tags to apply to the service principal.
 * `use_existing` - (Optional) When true, any existing service principal linked to the same application will be automatically imported. When false, an import error will be raised for any pre-existing service principal.

diff --git a/go.mod b/go.mod
@@ -18,7 +18,7 @@ require (
 	github.com/hashicorp/terraform-plugin-sdk/v2 v2.7.0
 	github.com/hashicorp/yamux v0.0.0-20210316155119-a95892c5f864 // indirect
 	github.com/klauspost/compress v1.12.2 // indirect
-	github.com/manicminer/hamilton v0.23.1
+	github.com/manicminer/hamilton v0.24.0
 	github.com/mitchellh/go-testing-interface v1.14.1 // indirect
 	github.com/mitchellh/go-wordwrap v1.0.1 // indirect
 	github.com/mitchellh/mapstructure v1.4.1 // indirect

diff --git a/go.sum b/go.sum
@@ -285,8 +285,8 @@ github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE=
 github.com/kylelemons/godebug v0.0.0-20170820004349-d65d576e9348/go.mod h1:B69LEHPfb2qLo0BaaOLcbitczOKLWTsrBG9LczfCD4k=
 github.com/kylelemons/godebug v1.1.0 h1:RPNrshWIDI6G2gRW9EHilWtl7Z6Sb1BR0xunSBf0SNc=
 github.com/kylelemons/godebug v1.1.0/go.mod h1:9/0rRGxNHcop5bhtWyNeEfOS8JIWk580+fNqagV/RAw=
-github.com/manicminer/hamilton v0.23.1 h1:7p5WviHLT0VKG7DKs/PggsUeapA1w6fd8B19ah62+LY=
-github.com/manicminer/hamilton v0.23.1/go.mod h1:4bnCX1oYiQuNa9CQnmT+WMHRNAEF4mT7ygW9Q/D3o+Y=
+github.com/manicminer/hamilton v0.24.0 h1:KUa8+NYP61esmb6QKGPy7t4zrxB0sujhWDRhiKpoScE=
+github.com/manicminer/hamilton v0.24.0/go.mod h1:QryxpD/4+cdKuXNi0UjLDvgxYdP0LLmYz7dYU7DAX4U=
 github.com/matryer/is v1.2.0/go.mod h1:2fLPjFQM9rhQ15aVEtbuwhJinnOqrmgXPNdZsdwlWXA=
 github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaOChaDxuIBZU=
 github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE=

diff --git a/internal/common/client_options.go b/internal/common/client_options.go
@@ -39,6 +39,9 @@ func (o ClientOptions) ConfigureClient(c *msgraph.Client) {
 	}
 	*c.RequestMiddlewares = append(*c.RequestMiddlewares, o.requestLogger)
 	*c.ResponseMiddlewares = append(*c.ResponseMiddlewares, o.responseLogger)
+
+	// Default retry limit, can be overridden from within a resource
+	c.RetryableClient.RetryMax = 8
 }
 
 func (o ClientOptions) requestLogger(req *http.Request) (*http.Request, error) {

diff --git a/internal/services/applications/application_certificate_resource.go b/internal/services/applications/application_certificate_resource.go
@@ -163,7 +163,9 @@ func applicationCertificateResourceCreate(ctx context.Context, d *schema.Resourc
 	newCredentials = append(newCredentials, *credential)
 
 	properties := msgraph.Application{
-		ID:             &id.ObjectId,
+		DirectoryObject: msgraph.DirectoryObject{
+			ID: &id.ObjectId,
+		},
 		KeyCredentials: &newCredentials,
 	}
 	if _, err := client.Update(ctx, properties); err != nil {
@@ -257,7 +259,9 @@ func applicationCertificateResourceDelete(ctx context.Context, d *schema.Resourc
 	}
 
 	properties := msgraph.Application{
-		ID:             &id.ObjectId,
+		DirectoryObject: msgraph.DirectoryObject{
+			ID: &id.ObjectId,
+		},
 		KeyCredentials: &newCredentials,
 	}
 	if _, err := client.Update(ctx, properties); err != nil {

diff --git a/internal/services/applications/application_data_source.go b/internal/services/applications/application_data_source.go
@@ -433,6 +433,7 @@ func applicationDataSource() *schema.Resource {
 
 func applicationDataSourceRead(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics {
 	client := meta.(*clients.Client).Applications.ApplicationsClient
+	client.BaseClient.DisableRetries = true
 
 	var app *msgraph.Application
 

diff --git a/internal/services/applications/application_pre_authorized_resource.go b/internal/services/applications/application_pre_authorized_resource.go
@@ -104,7 +104,9 @@ func applicationPreAuthorizedResourceCreate(ctx context.Context, d *schema.Resou
 	})
 
 	properties := msgraph.Application{
-		ID: app.ID,
+		DirectoryObject: msgraph.DirectoryObject{
+			ID: app.ID,
+		},
 		Api: &msgraph.ApplicationApi{
 			PreAuthorizedApplications: &newPreAuthorizedApps,
 		},
@@ -157,7 +159,9 @@ func applicationPreAuthorizedResourceUpdate(ctx context.Context, d *schema.Resou
 	}
 
 	properties := msgraph.Application{
-		ID: app.ID,
+		DirectoryObject: msgraph.DirectoryObject{
+			ID: app.ID,
+		},
 		Api: &msgraph.ApplicationApi{
 			PreAuthorizedApplications: &newPreAuthorizedApps,
 		},
@@ -245,7 +249,9 @@ func applicationPreAuthorizedResourceDelete(ctx context.Context, d *schema.Resou
 	}
 
 	properties := msgraph.Application{
-		ID: app.ID,
+		DirectoryObject: msgraph.DirectoryObject{
+			ID: app.ID,
+		},
 		Api: &msgraph.ApplicationApi{
 			PreAuthorizedApplications: &newPreAuthorizedApps,
 		},