Azure / bicep

Bicep is a declarative language for describing and deploying Azure resources
MIT License
3.25k stars 754 forks source link

Managed identity client id is not exposed #8775

Open olegd-superoffice opened 2 years ago

olegd-superoffice commented 2 years ago

I'm using Azure SQL and app service managed identity to access the database. My deployment is run by application which uses admin username/password to access Azure SQL (i.e - not using azure admin but normal SQL server authentication), so CREATE USER [<identity-name>] FROM EXTERNAL PROVIDER; statement doesn't work. Instead, I'm using managed identity client id as described in this blog post. It works fine, but I need to use reference function to get access to identity's client id. It would be really nice to have access to this property without resorting to reference function.

Here's example how I'm doing it now:

resource webApp 'Microsoft.Web/sites@2022-03-01' = {
  name: 'web-app-name'
 // ...
  identity: {
    type: 'SystemAssigned'
  }
}

output webAppIdentityClientId string = reference('${webApp.id}/providers/Microsoft.ManagedIdentity/Identities/default', '2018-11-30').clientId

What I would like to have instead:

output webAppIdentityClientId string = webApp.identity.clientId
alex-frankel commented 2 years ago

Does this work?

output foo string = webApp.identity.userAssignedIdentities.default.clientId

Or this? I am not sure if there is a distinction between identity.principalId and clientId

output foo string = webApp.identity.principalId
olegd-superoffice commented 2 years ago

@alex-frankel No, this wouldn't work. This is about system assigned identity, not user assigned one.

identity.principalId and identity.clientId are two different GUIDs in Azure AD where identity.principalId is Object ID and identity.clientId is Application ID

And identity.principalId wouldn't work here. To quote that blog post I mentioned:

Though the above powershell script will work to create a correct SID for AAD users and AAD groups object IDs. If you perform this on the object ID of a MSI (the ManagedServiceIdentityPrincipalId returned by the ARM template) your user will be created, but the MSI won’t actually have access. For service principals like the MSI, the SID needs to be created from the application ID (the ManagedServiceIdentityClientId returned by the ARM template).

alex-frankel commented 2 years ago

If clientId is not exposed and you need it for this scenario then we need to make a request to the RP to expose this. FWIW, our understanding is that this should be achievable with principalId, but we are not experts on this scenario.

Can you open a support case so this can be routed to the Web Resource Provider team. Also tagging @naveedaz / @seligj95 as FYI.

miqm commented 2 years ago

@olegd-superoffice in general in Bicep reference function is being replaced by existing keyword on a resource. Therefore your code should look like:

param location string = resourceGroup().location
resource appServicePlan 'Microsoft.Web/serverfarms@2020-12-01' = {
  name: uniqueString(resourceGroup().id)
  location: location
  sku: {
    name: 'F1'
    capacity: 1
  }
}

resource webApplication 'Microsoft.Web/sites@2021-01-15' = {
  name: uniqueString(resourceGroup().id)
  location: location
  properties: {
    serverFarmId: appServicePlan.id
  }
  identity: {
    type: 'SystemAssigned'
  }
}

resource identity 'Microsoft.ManagedIdentity/identities@2018-11-30' existing = {
  scope: webApplication
  name: 'default'
}

output webAppIdentityClientId string = identity.properties.clientId

Please let us know if this is what you're after.

olegd-superoffice commented 2 years ago

@miqm Yes, this seem to be working, thanks! Much cleaner than using reference() function directly in Bicep.

But when testing this I stumbled on another problem. First time I tried to deploy example you've provided I've got the following error on Type Microsoft.ManagedIdentity/identities Provisioning operation Read

{
    "status": "Failed",
    "error": {
        "code": "ResourceNotFound",
        "message": "The Resource 'Microsoft.Web/sites/mvyetqp3jhnsg' under resource group 'rg-TestIdentityClientId' was not found. For more details please go to https://aka.ms/ARMResourceNotFoundFix"
    }
}

And only second attempt was successful. It seems that ARM does not recognize that output depends on identity and tries to read it before it is actually provisioned. Is this something I can fix in the template or this should be reported somewhere else?

miqm commented 2 years ago

Output should be generated after all resources are deployed. In this case it seems that web app deployment finishes (returns 200) before the identity is set. Only thing we can do is to use a deployment script with wait command to wait a bit or hope that it will finish in time. You might try to output the principal id from the webapp but I do not guarantee it will work.

olegd-superoffice commented 2 years ago

@miqm I tested it several times now and it always fail on the first attempt. I tried adding output with the principal id from the webapp like this (if that's what you meant):

output webAppIdentityPrincipalId string = webApplication.identity.principalId

but it doesn't help. Looks like a bug in ARM?

anthony-c-martin commented 2 years ago

I think this is probably related to https://github.com/Azure/bicep/issues/7402

Eazyed commented 1 year ago

@olegd-superoffice It seems like the dependency chaning is failing within a given module/deployment.

So a simple solution to the problem is to separate your resources in different modules in order to create different deployments. In your case you could have a bicep file with your web app that ouputs your web app id and have a second module that takes your web app id as param, and then use the reference function or the existing resource block to get the related identity.

Passing the output of your wepApp module as param will create an implicit dependency between the deployments and will ensure that you're getting the identity only after it is created.

We encountered the same problem as you and we're using that as a workaround. Which is not that dirty as we already have a modular project.

Nonetheless, dependencies (implicit and explicits) should work as expected within modules and the original proposition of allowing : output webAppIdentityClientId string = webApp.identity.clientId Just as it is possible to do this : output webAppIdentityPrincipalId string = webApp.identity.principalId would make for a much better experience

miqm commented 1 year ago

Problem is not with dependency but with timing - managed identity seems to be created asynchronously and therefore get fails unless there's some optimisation on the runtime side that processes output before all resources are deployed - @anthony-c-martin?

anthony-c-martin commented 1 year ago

Here's the output JSON for that sample:

{
  "$schema": "https://schema.management.azure.com/schemas/2019-04-01/deploymentTemplate.json#",
  "contentVersion": "1.0.0.0",
  "metadata": {
    "_generator": {
      "name": "bicep",
      "version": "0.12.40.16777",
      "templateHash": "1286468113241814497"
    }
  },
  "parameters": {
    "location": {
      "type": "string",
      "defaultValue": "[resourceGroup().location]"
    }
  },
  "resources": [
    {
      "type": "Microsoft.Web/serverfarms",
      "apiVersion": "2020-12-01",
      "name": "[uniqueString(resourceGroup().id)]",
      "location": "[parameters('location')]",
      "sku": {
        "name": "F1",
        "capacity": 1
      }
    },
    {
      "type": "Microsoft.Web/sites",
      "apiVersion": "2021-01-15",
      "name": "[uniqueString(resourceGroup().id)]",
      "location": "[parameters('location')]",
      "properties": {
        "serverFarmId": "[resourceId('Microsoft.Web/serverfarms', uniqueString(resourceGroup().id))]"
      },
      "identity": {
        "type": "SystemAssigned"
      },
      "dependsOn": [
        "[resourceId('Microsoft.Web/serverfarms', uniqueString(resourceGroup().id))]"
      ]
    }
  ],
  "outputs": {
    "webAppIdentityClientId": {
      "type": "string",
      "value": "[reference(extensionResourceId(resourceId('Microsoft.Web/sites', uniqueString(resourceGroup().id)), 'Microsoft.ManagedIdentity/identities', 'default'), '2018-11-30').clientId]"
    }
  }
}

The deployment engine builds a DAG for all of the resources that are deployed or referenced. For the first 2 (not existing resources), we have a clear dependency order (sites depends on serverfarms). The 3rd (existing resource) is simply inlined as a reference() call in the outputs section.

Our current algorithm for building a DAG optimizes for speed, so it schedules every action as early as possible. Because there is no clear dependency from the reference() function call, this means it'll schedule that concurrently with the first resource, so the overall DAG would look something like:

image

miqm commented 1 year ago

so it is kind of a runtime problem. I'd expect output to be evaluated after all resources are successfully deployed since we do not have possibility to put a dependsOn on an output...

Eazyed commented 1 year ago

For more context about this problem, we got a sample of code from a client to add Azure AD auth to a function app

resource authSettings 'Microsoft.Web/sites/config@2022-03-01' = {
  parent: functionApp
  name: 'authsettingsV2'
  properties: {
    globalValidation: {
      unauthenticatedClientAction: 'Return401'
      requireAuthentication: true 
    }
    login: {
      tokenStore: {
        enabled: true
      }
    }
    identityProviders: {
      azureActiveDirectory: {
        enabled: true
        registration: {
          clientId: functionApp.identity.principalId
          clientSecretCertificateIssuer: issuer
        }
        validation: {
          allowedAudiences: [
            // Was initially a parameter
            reference('${functionApp.id}/providers/Microsoft.ManagedIdentity/Identities/default', '2018-11-30').clientId
          ]
        }
      }
    }
  }
}

The allowed audience was the clientID of the related function app and was passed as a parameter. It was very inconvenient so I tried to retrieve the clientId the same way the principalId was retrieved. We couldn't, so we tried with a reference() or an existing block but that triggered the ResourceNotFound error message.

This behavior was surprising as : clientId: functionApp.identity.principalId seemed to be working fine. And as far as my understanding went, it would access the same supposedly non-existing resource as with a reference() or an existing block. Moreover, we had an implicit dependency between the authSettings and the function app, so we expected the authSettings to be deployed only after the function app was fully deployed.

alex-frankel commented 1 year ago

@anthony-c-martin will follow up to check behavior with symbolic name

anthony-c-martin commented 1 year ago

@olegd-superoffice in general in Bicep reference function is being replaced by existing keyword on a resource. Therefore your code should look like:

param location string = resourceGroup().location
resource appServicePlan 'Microsoft.Web/serverfarms@2020-12-01' = {
  name: uniqueString(resourceGroup().id)
  location: location
  sku: {
    name: 'F1'
    capacity: 1
  }
}

resource webApplication 'Microsoft.Web/sites@2021-01-15' = {
  name: uniqueString(resourceGroup().id)
  location: location
  properties: {
    serverFarmId: appServicePlan.id
  }
  identity: {
    type: 'SystemAssigned'
  }
}

resource identity 'Microsoft.ManagedIdentity/identities@2018-11-30' existing = {
  scope: webApplication
  name: 'default'
}

output webAppIdentityClientId string = identity.properties.clientId

Please let us know if this is what you're after.

@olegd-superoffice if you get a chance, would you mind re-testing this with the 'symbolic names' feature enabled?

The easiest way to accomplish this is with a bicepconfig.json file in the same directory as your bicep file, with the following contents:

{
    "experimentalFeaturesEnabled": { 
        "symbolicNameCodegen": true
    }
}
olegd-superoffice commented 1 year ago

@anthony-c-martin I re-tested it with symbolicNameCodegen enabled and I get big fat warning, but it works. Is the plan to enable symbolicNameCodegen by default eventually?

anthony-c-martin commented 1 year ago

@anthony-c-martin I re-tested it with symbolicNameCodegen enabled and I get big fat warning, but it works. Is the plan to enable symbolicNameCodegen by default eventually?

Thanks for validating! Yes, that's the eventual plan, but we still have quite a bit of work to do in order to get there.