Azure / azure-dev

A developer CLI that reduces the time it takes for you to get started on Azure. The Azure Developer CLI (azd) provides a set of developer-friendly commands that map to key stages in your workflow - code, build, deploy, monitor, repeat.
https://aka.ms/azd
MIT License
369 stars 166 forks source link

[Issue] azd sets AZURE_PRINCIPAL_ID env var to --client-id #3834

Open jongio opened 2 weeks ago

jongio commented 2 weeks ago

AZURE_PRINCIPAL_ID should exclusively refer to the local user's ID. However, azd is currently assigning it the --client-id used during user login via azd auth login --client-id.

This presents an issue because we rely on knowing the principalType for role assignments.

If azd sets AZURE_PRINCIPAL_ID to the Service Principal (SP) used for login, the type will be identified as ServicePrincipal rather than User.

Existing templates assume AZURE_PRINCIPAL_ID to represent the local user ID. Thus, this alteration will disrupt all GitHub actions that rely on this assumption, likely affecting all such actions.

Let's refrain from assigning anything other than the local user's ID to AZURE_PRINCIPAL_ID. Additionally, we need to ensure that it isn't overwritten with the client ID used during azd auth login with --client-id.

This appears to be a breaking change that should be addressed asap.

ellismg commented 2 weeks ago

Existing templates assume AZURE_PRINCIPAL_ID to represent the local user ID. Thus, this alteration will disrupt all GitHub actions that rely on this assumption, likely affecting all such actions.

I think this is a bad assumption for the templates to make. I suspect we are going to find that you're going to want to understand the object ID of the principal doing the deployment, regardless of if it represents a user account or a service account. For example, when running in CI, I suspect we are going to want to grant role assignments to the service account that is running azd deploy so that same principal can access these resources.

For example, thinking ahead to something like the bind mount support in Aspire - we are going to need to use the data-plane to upload files to the bind mount target. I suspect the same decisions that pushed us to add AZURE_PRINCIPAL_ID so we could grant access to the user is going to mean we want the object ID of the service principal in CI.

If we want to AZURE_PRINCIPAL_ID to mean just "the object ID of the currently logged in use, assuming they are logged in as a user account, not a service principal/managed identity" we can make that change. It will mean that sometimes AZURE_PRINCIPAL_ID is empty, since we support logging into azd as a non user account.

If we do that, I think we will want to add some new value that always contains the object ID (and perhaps a property that says it is a service account vs user account), but I do think that long term people are going to want to access to both IDs.

vhvb1989 commented 2 weeks ago

As an alternative, we could introduce AZURE_PRINCIPAL_TYPE that you can check to know if it is a user or a SP.

In Aspire world, a bicep module is currently asking azd to set the type of the principal and azd is currently hardcoding it to Service Princial because it operates with a managed identity.

What we can do is to make azd auth aware of how the login was performed (might be interesting on the cases of az-delegation) and set the principal type in the .env, next to the principal id. Then folks can check and take decisions based on that.

Would that work for your case @jongio ?

jongio commented 1 week ago

As an alternative, we could introduce AZURE_PRINCIPAL_TYPE that you can check to know if it is a user or a SP.

In Aspire world, a bicep module is currently asking azd to set the type of the principal and azd is currently hardcoding it to Service Princial because it operates with a managed identity.

What we can do is to make azd auth aware of how the login was performed (might be interesting on the cases of az-delegation) and set the principal type in the .env, next to the principal id. Then folks can check and take decisions based on that.

Would that work for your case @jongio ?

Yes, thanks