search

microsoftgraph / msgraph-bicep-types

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

Unique name for Microsoft.Graph/servicePrincipals type does it makes sense? #128

Open slavizh opened 4 months ago

slavizh commented 4 months ago

Is your feature request related to a problem? Please describe. This will be more of a question and may be after answered depending on the answer a feature request. I am not so deep into Microsoft Entra but my understanding is that SP is like child resource of Application. However as far as I could read the documentation you can have Application in Tenant A and SP for that application in Tenant B. With that said let's say I want am creating Application App1 in Tenant A via Bicep. To create the SP SP1 in TenantB I need to provide the appId as input for the resource. In this case I cannot use existing syntax as the App1 is another tenant, correct?

We now assume that App1 is created in tenant A and SP1 is created in Tenant B. Now I want to create group GP1 and add SP1 as member GP1. In this case in order to get the ID of SP1 I need to use existing syntax and provide the appId as input as App1 is in Tenant A and I cannot use existing syntax for it. If that is the case it would make sense that the servicePrincipals resource also have uniqueName property and you can use two ways to reference it:

If this assumption is correct I think it makes sense that servicePrincipals also have uniqueName property and you can either provide uniqueName or appId in existing syntax and when deploying/applying the resource.

If the above is not true may be when you have multi-tenant application in Tenant A and that one also creates the same or similar application in tenant B which serves as connection for the Service Principal.

Describe the solution you'd like N/A

Additional context N/A

dkershaw10 commented 4 months ago

You may have already read this article on apps and SPs, but providing anyways: https://learn.microsoft.com/entra/identity-platform/app-objects-and-service-principals. While there is a strong relationship between apps and SPs, the fact that for multi-tenant apps, SPs can exist in other tenants, make it difficult to model as a child resource (as Graph has strict tenant boundaries, at least for now - and no that's not an indication of any change coming soon - just that I'm not great at predicting the future). Basically the appId is the glue between an app and its SPs.

While servicePrincipals doesn't have a uniqueName property, the appId property is a unique name for the service principal (unique in a tenant) serving the same purpose - a client-provided key. We did consider adding a uniqueName property in addition to appId - but decided against it because that would require template authors to specify two client-provided keys, without any real benefit other than a friendlier client-provided key. You would still need to use appId and track that anyways.

Your scenario brings up a whole different set of cross-tenant or multi-tenant scenarios. To deploy a multi-tenant app in Tenant A and its associated SP in tenant B is totally doable - but - you would need to have two templates for this, because you would need to deploy these in two different tenant contexts (using a different security principal that has access to each tenant), as well as passing the appId from one template (deploying the app) to another (deploying the SP).

slavizh commented 4 months ago

Ok. So my assumptions were correct. In such case I think it makes sense to have uniqueName for SPs as well in order to be able to onboard properly every scenario on Bicep otherwise we end up in the same situation before where we have GUIDs that we do not know what they mean. uniqueName is not only a key to be able to anchor to specific resource but also human readable property to understand what is behind that resource. appId is key but as GUID is not human readable in the situation where the application is in another tenant. At least we can leave it open and see if anyone else will find this as a problem. One thing I am afraid as this is not so popular scenario to appear after the service is GA.

dkershaw10 commented 4 months ago

Yeah - would love to hear from others - I'm not dismissing this at all, as we did discuss the option of adding uniqueName.

dkershaw10 commented 2 months ago

@dkershaw10 to sync with app model team to see what they think of this suggestion, as we haven't seen anyone else from the community (other than irwins) comment on this ask.