microsoftgraph / msgraph-bicep-types

Repo contains Microsoft Graph resource types to integrate with bicep templates.
MIT License
46 stars 7 forks source link

Federated Credentials? #142

Closed broll closed 2 months ago

broll commented 5 months ago

Bicep version Bicep CLI version 0.28.1 (ba1e9f8c1e)

Resource and API version Microsoft.Graph/applications/federatedIdentityCredentials@v1.0

Auth flow Interactive

Describe the bug The documentation for https://learn.microsoft.com/en-us/graph/templates/reference/federatedidentitycredentials?view=graph-bicep-1.0 doesn't even pass bicep compilation.

The resource type segment "Microsoft.Graph/applications/federatedIdentityCredentials@v1.0" is invalid. Nested resources must specify a single type segment, and optionally can specify an api version using the format "<type>@<apiVersion>".bicep(BCP156) If you remove the compile error you get deployment error: "Invalid identifier format for applications/federatedIdentityCredentials"

To Reproduce Try to deploy the example from: https://learn.microsoft.com/en-us/graph/templates/reference/federatedidentitycredentials?view=graph-bicep-1.0#resource-format

dkershaw10 commented 5 months ago

Thanks for bringing this up @broll. It looks like we have a bug in the auto-generation of the reference documentation.

Nested resources should not be fully qualified. You'll also see this if you use intellisense in VS Code, when creating the nested resource. It should read:

resource symbolicname 'Microsoft.Graph/applications@v1.0' = {
  displayName: 'string'
  uniqueName: 'string'

  resource symbolicname 'federatedIdentityCredentials@v1.0' = {
    audiences: [
      'string'
    ]
    description: 'string'
    issuer: 'string'
    name: 'string'
    subject: 'string'
  }
}  

There's also an issue with extensibility, and how it supports child resources. Please see this known issues section on child resources, which will say that the name property needs to be fully qualified.

If you want to see a full example, please look at create federated credentials for github actions

Hope this helps - but we'll definitely look to fix the doc bug (not sure how we missed it).

broll commented 5 months ago

Certainly gets me further using the github example. The problem I'm running into now is that the service principal running the bicep creating the app registration which has (Application.ReadWrite.OwnedBy), does not automatically become the owner like it would in az cli commands.

dkershaw10 commented 5 months ago

Thanks for reporting that @broll . It's tracked in #114 and according to that, we should have a fix for that deployed shortly. In the meantime you would need to explicitly set the owner property to the calling service principal's id.

broll commented 5 months ago

That issue was for azure groups and my issue is with app registrations, but assuming the same root problem...

It's trying to add the bicep extension as an owner to the app registration? But maybe failing?

With the groups everything gets created and there is a workaround to just explictly add. I'm not having the same luck with app registrations. It fails before it can create the service principal (it successfully creates the app registration)

Everything works fine when I run the code myself... but running under a principal that definitely has privs because it can do it using the az cli I get this generic error:

"When using this permission, the backing application of the service principal being created must in the local tenant Graph client request id"

I thought maybe setting the signInAudience to allow cross tenant might help but it didn't.

dkershaw10 commented 5 months ago

Yes - it's the same root problem (in terms of properly setting the owners property, and applies to groups, apps and SPs). If it's on track, the fix should be deployed and available next week.

It seems like you are running into a different issue now, when trying to create an SP from the application? I'm trying to understand where the failure is happening, if the app reg is successful and it fails before creating the service principal...

Can you describe exactly what you are trying to do here, as it looks like you are trying to create apps and SPs in different tenants, maybe?

broll commented 5 months ago

Ultimately replace a powershell script using az cli commands with it's bicep equivalent.

Here is a boiled down version showing what is working and what isn't.

      write-host "---------Az AD App Create---------"
      $app = az ad app create --sign-in-audience AzureADMyOrg --only-show-errors --display-name $this.PrincipalName | ConvertFrom-Json
      write-host $App
      write-host "----------------------------------"
      write-host "---------Az SP Create---------"
      $sp = az ad sp list --only-show-errors --display-name $this.PrincipalName | ConvertFrom-Json
      $sp = $sp | where appDisplayName -eq $this.PrincipalName
      if ( $null -eq $sp) {
        Write-host "No SP found, creating SP"
        $sp = az ad sp create --only-show-errors --id $app.AppId | ConvertFrom-Json
      }
      write-host $sp
      write-host "----------------------------------"
          - task: AzureCLI@2
            name: doStuff
            inputs:
              azureSubscription: $(myServiceConnection)
              scriptType: pscore
              scriptLocation: "inlineScript"
              inlineScript: |
                az deployment sub create --template-file $(System.DefaultWorkingDirectory)/provisioning/modules/Microsoft.Graph/principalCreation.bicep --location eastus2 --subscription $(mySub) --debug
targetScope = 'subscription'

provider microsoftGraph

param appRegistrationName string = 'deleteme-bicepBasedDeployOfEntraAppRegistration'

resource appReg 'Microsoft.Graph/applications@v1.0' = {
  displayName: appRegistrationName
  uniqueName: appRegistrationName
}

resource sp 'Microsoft.Graph/servicePrincipals@v1.0' = {
  appId: appReg.appId
}

Creating that SP fails with the error: "When using this permission, the backing application of the service principal being created must in the local tenant Graph client request id"

Which is a really generic error that can be given for a variety of reasons from lack of privs to using the wrong appId...

When run from my own interactive credentials the same bicep works fine and sets me as the application owner.

broll commented 5 months ago

Any thoughts @dkershaw10 ? Is it just a problem with the identity pass through from running it from Azure Devops? Has most of the existing testing been from Github actions?

dkershaw10 commented 5 months ago

@broll - we have no way to investigate further without a corrrelation-id/request-id/client-request-id and a timestamp.

My guess is that this error is all down to the related issue (that the originating client is not added to the application.) The app will get created, but I suspect there's a check that the calling principal has permissions to create a service principal from the referenced application - if the calling principal doesn't have the necessary role, a further check would be if the calling principal is an owner of the referenced application. With the bug, the latter is definitely not true, and is likely the cause of the error.

This error doesn't happen in the interactive case, likely because you are being added as an owner of the application. Even if you do this via Az CLI/PS running as an SP, that should work. The bug is in the flow we have explicitly for Bicep Graph (which uses an application on-behalf-of flow).

broll commented 5 months ago

Sounds good thanks for the reply. Below is the requested information.   'x-ms-client-request-id': '2dc133df-2e3f-11ef-831f-c3d80caaf9bf' 'x-ms-correlation-request-id': '5b019b61-ca00-4e44-b0c0-7aa8eae4beb7' 'x-ms-request-id': '5b019b61-ca00-4e44-b0c0-7aa8eae4beb7'

dkershaw10 commented 5 months ago

@broll We need the timestamp too please.

broll commented 5 months ago

Sorry not sure exactly which timestamp you're looking for. Hopefully it's the one in the routing request id

Here's a larger dump that hopefully has what you need.

VERB: POST
URL: https://management.azure.com/subscriptions/{ofuscated}/providers/Microsoft.Resources/deployments/principalCreation/validate?api-version=2022-09-01
//Request
'x-ms-client-request-id': '2dc133df-2e3f-11ef-831f-c3d80caaf9bf'

//Response
'x-ms-request-id': '9f231ac7-e09a-4ced-92b5-79f0404d44bc'
'x-ms-correlation-request-id': '9f231ac7-e09a-4ced-92b5-79f0404d44bc'
'x-ms-routing-request-id': 'WESTUS:20240619T132409Z:9f231ac7-e09a-4ced-92b5-
///

Verb: PUT
URL: https://management.azure.com/subscriptions/{ofuscated}/providers/Microsoft.Resources/deployments/principalCreation?api-version=2022-09-01
//Request
'x-ms-client-request-id': '2dc133df-2e3f-11ef-831f-c3d80caaf9bf'
Response:
'x-ms-request-id': '5b019b61-ca00-4e44-b0c0-7aa8eae4beb7'
'x-ms-correlation-request-id': '5b019b61-ca00-4e44-b0c0-7aa8eae4beb7'
'x-ms-routing-request-id': 'WESTUS:20240619T132413Z:5b019b61-ca00-4e44-b0c0-7aa8eae4beb7'
dkershaw10 commented 5 months ago

@broll Those are for ARM APIs (the deployment operation). I need the request IDs and timestamps for the Microsoft Graph resource errors. You should see those Graph errors in the output also and the requestIDs and timestamps should be with the SP creation errors like "When using this permission, the backing application of the service principal being created must in the local tenant Graph client request id"

broll commented 5 months ago

Sorry understood now.

When using this permission, the backing application of the service principal being created must in the local tenant Graph client request id: 1c4fcba0-c431-4efd-8661-b4b82a4e2fa9. Graph request timestamp: 2024-06-19T13:24:17Z

dkershaw10 commented 5 months ago

NP - I see this in the Graph logs. I'll need to get someone to look at the associated Entra ID logs for the root cause.

dkershaw10 commented 5 months ago

@broll Engineering has confirmed that the root cause is the owners bug. All being well the deployment should complete late next week.

eketo-msft commented 5 months ago

Hey @broll, deployment is complete and verified to address the owners issue.

broll commented 4 months ago

Thanks @eketo-msft

I can confirm creating the service principal works now at least. Using azure devops and a service connection (tied to an app registration service principal) in an azcli task it still doesn't seem to set an owner on it though so steps afterwords are still failing since the calling identity is never made owner.

xqrzd commented 4 months ago

Same for me. The recent deployment made it so I can create the app registration / service principal, but then all subsequent deployments fail with forbidden since the Azure Devops service connection isn't added as an owner.

eketo-msft commented 4 months ago

@broll and/or @xqrzd, can you share the failing Graph client request-id and timestamp? I can check why it's failing.

dkershaw10 commented 4 months ago

We'll continue to track the owner issue here too, but the original documentation bug has been fixed, if you now look at the federated identity credential reference topics.

broll commented 4 months ago

@eketo-msft sorry for the delay

Graph client request id: 908b7d41-bfcd-4a8f-bae7-2c5c6b4f9524. Graph request timestamp: 2024-07-26T18:20:20Z.

That would be the graph error when it's trying to add federated credentials to the service principal (but can't because it isn't an owner).

eketo-msft commented 3 months ago

The fix for this issue is fully deployed, but you may need to ensure that your SP Identity (OID 42f5aa90-74ee-47ec-b87e-d1975df4ee4c) which is running the deployment is added as an owner to your App and/or SP. This can be done in the Azure Portal.

broll commented 3 months ago

@eketo-msft

I just tried it again and it failed d7417459-4846-45a4-a6fc-bc719532615d. Graph request timestamp: 2024-08-02T21:43:43Z

The OID you posted there is the principal running a bicep script that is trying to create an app registration, service principal and setup a federated credential for the app registration it's creating.

The whole problem is that it can't setup the federated credential because it it's never being granted ownership of the service principal/app registration it's creating in the bicep script.

The goal is doing everything IaC and not using the portal. It's also easily achieved with az cli commands but since bicep added api's for these things it'd be ideal to do it all in bicep.

targetScope = 'subscription'

provider microsoftGraph

var microsoftEntraAudience = 'api://AzureADTokenExchange'
var issuer = 'https://vstoken.dev.azure.com/e330ea89-c446-4793-9de8-85981ac5d777'
var subject = 'sc://QuadVSTS/iamorg/iamserviceconnectionname'
var description = 'AzureDevops'

resource appReg 'Microsoft.Graph/applications@v1.0' = {
  displayName: 'iamappregid'
  uniqueName: 'iamappregid'
}

resource sp 'Microsoft.Graph/servicePrincipals@v1.0' = {
  appId: appReg.appId
}

resource federatedCredential 'Microsoft.Graph/applications/federatedIdentityCredentials@v1.0' = {
  name: '${appReg.uniqueName}/${description}'
  audiences: [microsoftEntraAudience]
  description: description
  issuer: issuer
  subject: subject
  dependsOn: [sp]
}

@sys.description('The Service Principal Id to back the teams service connection.')
output servicePrincipalId string = sp.id

@sys.description('The App Registration Id to back the teams service connection.')
output appId string = appReg.appId

So the expectation there is that OID 42f5aa90-74ee-47ec-b87e-d1975df4ee4c (the thing running the bicep script), would become an owner of the app registration and sp.

It's not, and as a result the bicep script fails because it can't create the federated credential.

broll commented 2 months ago

You can at least create a federated credential off an app registration now... There's still issues, but it seems to be getting tracked better in #134