azuread_application_registration can't be destroyed by owner if using azuread_application_owner

agyss commented 4 months ago

Community Note

Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritise this request
Please do not leave "+1" or "me too" comments, they generate extra noise for issue followers and do not help prioritise the request
If you are interested in working on this issue or have submitted a pull request, please leave a comment

Terraform (and AzureAD Provider) Version

azuread 2.47.0

Affected Resource(s)

azuread_application_registration
azuread_application_owner

Terraform Configuration Files

resource "azuread_application_registration" "example" {
  display_name = "example"
}

resource "azuread_user" "jane" {
  user_principal_name = "jane.fischer@hashitown.com"
  display_name        = "Jane Fischer"
  password            = "Ch@ngeMe"
}

resource "azuread_application_owner" "example_jane" {
  application_id  = azuread_application_registration.example.id
  owner_object_id = azuread_user.jane.object_id
}

data "azurerm_client_config" "current" {}

resource "azuread_application_owner" "example_current" {
  application_id  = azuread_application_registration.example.id
  owner_object_id = data.azurerm_client_config.current.object_id, # current user

}

Expected behaviour

If the user who is performing a terraform destory on all objects above is in the owners list and does not have higher privileges (like global admin), Terraform should realize that it can't destroy the owner object as it's permissions are required to delete the application. This would result in the same behaviour as in azure cli or the azure portal.

Of course this is only true if the destroy action should delete the application_registration as well and not terraform --target=ownerobject is used.

Actual behaviour

The owner is removed from the application_registration before the application itself is deleted, leading to a 403 error as the user no longer has authorization to do so (if not assigned higher permissions like global admin).

agyss commented 4 months ago

Workaround

As I don't need the --terraform --target=ownerobject to destroy assignments (i.e., I accept I have to remove the assignments manually when necessary) I use this workaround to achieve desired functionality:

resource "terraform_data" "app_owners" {
  input = {
    app_object_id = azuread_application_registration.example.object_id
    owner_id = azuread_user.jane.object_id
  }

  provisioner "local-exec" {
    when = create
    command = "az ad app owner add --id ${self.input.app_object_id} --owner-object-id ${self.input.owner_id}"
  }
}

manicminer commented 4 months ago

Thanks for reporting @agyss. Unfortunately, the wider context of the destroy operation is entirely unavailable to providers due to the way Terraform works. This means there is no way for us to know which resources are going to be destroyed. The most appropriate way t resolve these kinds of dependency issues is by manipulating the graph itself - options include the lifecycle meta argument and/or targeted destroys. Sadly because Terraform does not support different graph ordering between create and destroy operations, there is no config-based solution to this problem.

It might be possible to fudge this inside the provider, but it requires some investigation and I'm not very confident that it's possible without breaking the expected contract and negatively affecting many users. Ultimately this should probably be a feature request of Terraform Core.

agyss commented 4 months ago

I see. In this case I would propose to invent a boolean field named only_destroy_with_parent and a use_existing. The first one if set to true effectively ignores deletes while the use_existing if set to true does not throw an error if the resource already exists but "imports" it (like done with https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/service_principal use_existing`).

The combination of both pretty much has the same effect as the workaround suggested before.

manicminer commented 4 months ago

Just a thought - do you really need the azuread_application_owner resource here? Typically when you create an app registration, using app credentials authorized via a service principal holding only the Application.ReadWrite.OwnedBy app role, the newly created application is automatically assigned the calling principal as the sole owner. I've just tested this to make sure this is still the case and it does appear to be.

This is one of the reasons the azuread_application_registration resource explicitly does not manage owners - so that the auto-assigned owner can be kept intact with the express intent of avoiding a scenario like the one you find yourself in. Have you tried just omitting the azuread_application_owner resource altogether?

hashicorp / terraform-provider-azuread