Azure / acr

Azure Container Registry samples, troubleshooting tips and references
https://aka.ms/acr
Other
162 stars 106 forks source link

ACR tasks managed identity token format inconsistent #699

Closed andreygoran closed 11 months ago

andreygoran commented 1 year ago

Description

ACR Tasks can be created with --assign-identity flag so that Managed Identity is used when running the tasks:

az acr task create --assign-identity [system]

When access token is pulled from the running task, the JSON format is not consistent with the format used in other Azure services (such as VMs, container instances, etc) which is described here:

type responseJson struct {
  AccessToken string `json:"access_token"`
  RefreshToken string `json:"refresh_token"`
  ExpiresIn string `json:"expires_in"`
  ExpiresOn string `json:"expires_on"`
  NotBefore string `json:"not_before"`
  Resource string `json:"resource"`
  TokenType string `json:"token_type"`
}

Access token pulled from a VM:

{
"access_token":"ey..29",
"expires_in":"86400",
"expires_on":"1688823124",
"ext_expires_in":"86399",
"not_before":"1688736424",
"resource":"https://management.azure.com/",
"token_type":"Bearer"
}

Access token pulled from a running ACR Tasks:

{
"access_token":"ey...zw",
"refresh_token":"",
"expires_in":86398.9998935,
"expires_on":1688822789,
"not_before":0,
"resource":"https://management.azure.com/",
"token_type":"Bearer",
"scopes":["https://management.azure.com//.default"]
}

The problem is that expires_in, expires_on, not_before values are NOT strings and this breaks tools that rely on using managed identity because they cannot parse JSON.

Packer by HashiCorp is one example of a tool that crashes because of this inconsistency:

==> azure-arm: Running builder ...
==> azure-arm: Getting tokens using Managed Identity for Azure
Build 'azure-arm' errored after 2 seconds 610 milliseconds: adal: Failed to unmarshal the service principal token during refresh. Error = 'json: cannot unmarshal number into Go struct field .expires_on of type string' JSON = '{"access_token":"ey...4g","refresh_token":"","expires_in":86399,"expires_on":1688821394,"not_before":0,"resource":"https://management.azure.com/","token_type":"Bearer","scopes":["https://management.azure.com//.default"]}'

To Reproduce

  1. Create task with a Managed Identity:
    az acr task create --name curl-task --registry myregistry --base-image-trigger-enabled false --commit-trigger-enabled false --assign-identity [system] --context /dev/null --cmd 'curlimages/curl http://169.254.169.254/metadata/identity/oauth2/token?api-version=2018-02-01&resource=https%3A%2F%2Fmanagement.azure.com%2F -H Metadata:true -s'
  2. Run the task and observe JSON in the output:
    az acr task run --name curl-task --registry myregistry
northtyphoon commented 12 months ago

The values can be number or string. If you check the official adal go library, you can see how they parse the payload

https://github.com/Azure/go-autorest/blob/d3f8f8a6cdf252e58b19b99523c25c5ad6664ac7/autorest/adal/token.go#L1139

andreygoran commented 12 months ago

Hi @northtyphoon,

According to documentation https://learn.microsoft.com/en-us/azure/virtual-machines/instance-metadata-service?tabs=windows#schema:

In json responses, all primitives will be of type string, and missing or inapplicable values are always included but will be set to an empty string.

From the client's point of view, all Managed Identity endpoints (http://169.254.169.254/metadata/identity/oauth2/token) are the same service, no matter if called from a VM, Container Instance, or Container Registry Task. So it would be logical to return data in the same format. Which is actually also documented here:

https://github.com/Azure/azure-rest-api-specs/blob/main/specification/imds/data-plane/Microsoft.InstanceMetadataService/stable/2021-12-13/imds.json

northtyphoon commented 11 months ago

@andreygoran we have deployed a fix to change the property to string. Can you try it again?

andreygoran commented 11 months ago

@andreygoran we have deployed a fix to change the property to string. Can you try it again?

I tested it, and it is fixed now. Thank you!