From 37a7b85394316900a38dc026511169fb714c335a Mon Sep 17 00:00:00 2001 From: Shaun Donnelly <152335024+ShaunDonn2@users.noreply.github.com> Date: Wed, 2 Oct 2024 09:57:37 -0700 Subject: [PATCH] Update AAD token request target resource uri for ACR access (#4654) * update target resource uri * resolve build errors * update comment * re-add ArmResourceManagerId * add obsolete attribute * make uri non-configurable * remove local var * move uri to config * set as default ARM uri to avoid breaking OSS scenarios --- .../AzureContainerRegistryAccessTokenProvider.cs | 2 +- .../Configs/ConvertDataConfiguration.cs | 11 ++++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/src/Microsoft.Health.Fhir.Azure/ContainerRegistry/AzureContainerRegistryAccessTokenProvider.cs b/src/Microsoft.Health.Fhir.Azure/ContainerRegistry/AzureContainerRegistryAccessTokenProvider.cs index dfe1a42522..8ee85e9d5e 100644 --- a/src/Microsoft.Health.Fhir.Azure/ContainerRegistry/AzureContainerRegistryAccessTokenProvider.cs +++ b/src/Microsoft.Health.Fhir.Azure/ContainerRegistry/AzureContainerRegistryAccessTokenProvider.cs @@ -59,7 +59,7 @@ public async Task GetTokenAsync(string registryServer, CancellationToken { EnsureArg.IsNotNullOrEmpty(registryServer, nameof(registryServer)); - var aadResourceUri = new Uri(_convertDataConfiguration.ArmResourceManagerId); + var aadResourceUri = _convertDataConfiguration.AcrTargetResourceUri; string aadToken; try { diff --git a/src/Microsoft.Health.Fhir.Core/Configs/ConvertDataConfiguration.cs b/src/Microsoft.Health.Fhir.Core/Configs/ConvertDataConfiguration.cs index 2b94ee4d1d..8df3ce6e74 100644 --- a/src/Microsoft.Health.Fhir.Core/Configs/ConvertDataConfiguration.cs +++ b/src/Microsoft.Health.Fhir.Core/Configs/ConvertDataConfiguration.cs @@ -22,10 +22,19 @@ public class ConvertDataConfiguration public ICollection ContainerRegistryServers { get; } = new List(); /// - /// ArmResourceManagerId to aquire AAD token for ACR access token since ACR is not an AAD resource. + /// AcrTargetResourceUri to acquire AAD token for ACR access token since ACR is not an AAD resource. + /// The value is "https://management.azure.com/" for AzureCloud and DogFood. Could be changed to "https://management.usgovcloudapi.net/" for Azure Government and "https://management.chinacloudapi.cn/ " for Azure China. + /// To enable Trusted Services scenarios, we must use the ACR-specific URI rather than the more generic ARM URI. https://dev.azure.com/msazure/AzureContainerRegistry/_wiki/wikis/ACR%20Specs/480000/TrustedServicesPatterns + /// The value is "https://containerregistry.azure.net/" for all cloud environments. + /// + public Uri AcrTargetResourceUri { get; set; } = new Uri("https://management.azure.com/"); + + /// + /// ArmResourceManagerId to acquire AAD token for ACR access token since ACR is not an AAD resource. /// The value is "https://management.azure.com/" for AzureCloud and DogFood. /// Could be changed to "https://management.usgovcloudapi.net/" for Azure Government and "https://management.chinacloudapi.cn/ " for Azure China. /// + [Obsolete("Use AcrTargetResourceUri instead.")] public string ArmResourceManagerId { get; set; } = "https://management.azure.com/"; ///