hashicorp / terraform-provider-azuread

Terraform provider for Azure Active Directory
https://registry.terraform.io/providers/hashicorp/azuread/latest/docs
Mozilla Public License 2.0
419 stars 288 forks source link

Unable to assign AAD Group to Enterprise App #813

Closed ausfestivus closed 2 years ago

ausfestivus commented 2 years ago

Community Note

Terraform (and AzureAD Provider) Version

Terraform v1.2.1
on darwin_amd64
+ provider registry.terraform.io/hashicorp/azuread v2.22.0

Affected Resource(s)

Terraform Configuration Files

data "azuread_application" "tfc-application" {
  display_name = local.aad_application_name[var.environment]
}

data "azuread_service_principal" "tfc-application" {
  display_name = local.aad_application_name[var.environment]
}

resource "azuread_app_role_assignment" "name" {
  app_role_id         = data.azuread_service_principal.tfc-application.app_role_ids
  principal_object_id = azuread_group.cpt-aad-admins.object_id
  resource_object_id  = data.azuread_service_principal.tfc-application.object_id
}

Debug Output

Trimmed output of the data.azuread_service_principal.tfc-application data source. You can see that the app_role_ids attribute is an empty object.

      + account_enabled               = true
      + alternative_names             = []
      + app_role_assignment_required  = true
      + app_role_ids                  = {}
      + app_roles                     = [
          + {
              + allowed_member_types = [
                  + "User",
                ]
              + description          = "msiam_access"
              + display_name         = "msiam_access"
              + enabled              = true
              + id                   = "GUID"
              + value                = ""
            },
        ]
      + application_id                = "GUID"

Panic Output

nil

Expected Behavior

Terraform should add the group to the Enterprise Application so the group members can access the app.

Actual Behavior

 Error: Invalid index
│ 
│   on tenants_cpt.tf line 65, in resource "azuread_app_role_assignment" "name":
│   65:   app_role_id         = data.azuread_service_principal.tfc-application.app_role_ids[0]
│     ├────────────────
│     │ data.azuread_service_principal.tfc-application.app_role_ids is empty map of string
│ 
│ The given key does not identify an element in this collection value.

Steps to Reproduce

  1. terraform plan

Important Factoids

This seems like a pretty simple use case. The Enterprise App and Service Principal in AAD are created/maintained elsewhere by a different team, hence why I look them up via data sources. I create the AAD group, assign the users to the group and then add the group to the Enterprise App. Its all standard stuff except in this case the app_role_ids attribute on the SP is empty.

Its entirely possible im missing something basic here so feel free to point it out.

I DID have a poke through the Discuss forums because this really feels like one of those types of questions. Only found a single post that felt similar from someone trying to do the same thing. They never got an answer.

References

nil

manicminer commented 2 years ago

Hi @ausfestivus, thanks for reporting this. Can you post the output of terraform state show data.azuread_service_principal.tfc-application? I have tried to reproduce this but so far in my testing, app_role_ids was populated correctly. Thanks!

ausfestivus commented 2 years ago

Here you go @manicminer

terraform state show data.azuread_application.tfc-application

# data.azuread_application.tfc-application:
data "azuread_application" "tfc-application" {
    api                            = [
        {
            known_client_applications      = []
            mapped_claims_enabled          = false
            oauth2_permission_scopes       = [
                {
                    admin_consent_description  = "Allow the application to access Terraform Cloud (DEV) on behalf of the signed-in user."
                    admin_consent_display_name = "Access Terraform Cloud (DEV)"
                    enabled                    = true
                    id                         = "{GUID}"
                    type                       = "User"
                    user_consent_description   = "Allow the application to access Terraform Cloud (DEV) on your behalf."
                    user_consent_display_name  = "Access Terraform Cloud (DEV)"
                    value                      = "user_impersonation"
                },
            ]
            requested_access_token_version = 1
        },
    ]
    app_role_ids                   = {}
    app_roles                      = [
        {
            allowed_member_types = [
                "User",
            ]
            description          = "msiam_access"
            display_name         = "msiam_access"
            enabled              = true
            id                   = "{GUID}}"
            value                = ""
        },
    ]
    application_id                 = "{GUID}"
    device_only_auth_enabled       = false
    disabled_by_microsoft          = "<nil>"
    display_name                   = "Terraform Cloud (DEV)"
    fallback_public_client_enabled = false
    feature_tags                   = [
        {
            custom_single_sign_on = false
            enterprise            = false
            gallery               = false
            hide                  = false
        },
    ]
    group_membership_claims        = [
        "ApplicationGroup",
    ]
    id                             = "{GUID}"
    identifier_uris                = [
        "https://app.terraform.io/sso/saml/samlconf-NNNNNNNNN/metadata",
    ]
    oauth2_permission_scope_ids    = {
        "user_impersonation" = "{GUID}"
    }
    oauth2_post_response_required  = false
    object_id                      = "{GUID}"
    optional_claims                = [
        {
            access_token = []
            id_token     = []
            saml2_token  = [
                {
                    additional_properties = []
                    essential             = false
                    name                  = "groups"
                    source                = ""
                },
            ]
        },
    ]
    owners                         = [
        "{GUID}",
        "{GUID}",
        "{GUID}",
    ]
    public_client                  = [
        {
            redirect_uris = []
        },
    ]
    publisher_domain               = "OURTENTANTNAME.onmicrosoft.com"
    required_resource_access       = []
    sign_in_audience               = "AzureADMyOrg"
    single_page_application        = [
        {
            redirect_uris = []
        },
    ]
    tags                           = []
    web                            = [
        {
            homepage_url   = "https://app.terraform.io/sso/saml/acs?metadata=terraformcloud|ISV9.2|primary|z"
            implicit_grant = [
                {
                    access_token_issuance_enabled = false
                    id_token_issuance_enabled     = true
                },
            ]
            logout_url     = ""
            redirect_uris  = [
                "https://app.terraform.io/sso/saml/samlconf-NNNNNNNNN/acs",
            ]
        },
    ]
}
ausfestivus commented 2 years ago

FYI we have opened a support case with HC about this issue. #76435

manicminer commented 2 years ago

@ausfestivus I noticed in your initial config snippet, you're referencing app_role_ids as if it were a list when it's actually a map indexed by the role value. If the value of a role is empty - as it is in your state - then that role will not be added to the app_role_ids map.

For example, having the following app roles:

    [
        {
            allowed_member_types = [
                "User",
            ]
            description          = "msiam_access"
            display_name         = "msiam_access"
            enabled              = true
            id                   = "00000000-0000-0000-0000-000000000000"
            value                = ""
        },
        {
            allowed_member_types = [
                "User",
            ]
            description          = "second_role"
            display_name         = "second_role"
            enabled              = true
            id                   = "11111111-1111-1111-1111-111111111111"
            value                = "the_second_role"
        }
    ]

Then you'll see app_role_ids populated like this:

    {
        the_second_role = "11111111-1111-1111-1111-111111111111"
    }

Since this seems to be working as expected, I'm going to close this issue for now. If you think there's a bug here that I've overlooked, please feel free to comment further and I'll be happy to take another look. Thanks!

ausfestivus commented 2 years ago

Morning @manicminer,

I will revisit the available documentation when I start work a little later and see if your commentary can help me identify where I've got it wrong.

The support team is still working with me to understand the issue.

I don't yet have a functional config despite following the available documentation. I feel like it is premature to close this issue as, from my POV as the requestor, the issue still exists.

ausfestivus commented 2 years ago

Perhaps if someone can share their code here that they use to achieve this outcome I can see where this isnt working?

manicminer commented 2 years ago

@ausfestivus I do recognise that you haven't yet arrived at a config that works for your use case, however this tracker is primarily for handling bugs and feature requests. I recommend continuing to follow up with our support team who will be able to help you work out the best approach.

From a provider perspective, if you set a value for your app roles, they will show up in the app_role_ids map (note as before, this is a map rather than a list). If you need more flexibility, it's probably worth looking into a forexpression to pick out the role(s) you need from the app_roles list.

ausfestivus commented 2 years ago

The APJ support team came through for me. For any future people that hit this same issue here is the code that worked:

locals {
  # list of team members with access to the tenantConfiguration Workspace
  tenantConfigurationAdmins = [
    "alice@domain.com.au",
    "bob@domain.com.au"
  ]
  msiam_access_index = length(data.azuread_service_principal.tfc-application.app_roles) - 1
  aad_application_name = {
      dev  = "Terraform Cloud (DEV)"
      prod = "Terraform Cloud"
  }
}

# https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/data-sources/users
data "azuread_users" "tenantConfigurationAdmins" {
  user_principal_names = local.tenantConfigurationAdmins
}

# AzureAD Group creation
# https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/group
resource "azuread_group" "tenantConfigurationAdmins" {
  display_name     = "ACL_TFC_tenantConfiguration_${var.environment}_admins"
  members          = data.azuread_users.tenantConfigurationAdmins.object_ids
  owners           = data.azuread_users.tenantConfigurationAdmins.object_ids
  security_enabled = true
}

# AzureAD Enterprise Application config
# we need to add our AAD Group to the necessary Enterprise App so their group members SAML
# sign-on to TFC works
data "azuread_application" "tfc-application" {
  display_name = local.aad_application_name[var.environment]
}

data "azuread_service_principal" "tfc-application" {
  display_name = local.aad_application_name[var.environment]
}

# https://registry.terraform.io/providers/hashicorp/azuread/latest/docs/resources/app_role_assignment
resource "azuread_app_role_assignment" "tenantConfigurationAdmins" {
  app_role_id         = data.azuread_service_principal.tfc-application.app_roles[local.msiam_access_index].id
  principal_object_id = azuread_group.tenantConfigurationAdmins.object_id
  resource_object_id  = data.azuread_service_principal.tfc-application.object_id
}
ausfestivus commented 2 years ago

@manicminer would you be open to PR with a working example for inclusion in https://github.com/hashicorp/terraform-provider-azuread/tree/main/examples ?

manicminer commented 2 years ago

@ausfestivus Glad you found a solution! We're open to adding more examples but ideally they should be suitably generic and easy to read. This may be a better fit in the TFC docs, I will ask internally.

github-actions[bot] commented 2 years ago

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.