diff --git a/src/azure-cli/HISTORY.rst b/src/azure-cli/HISTORY.rst index 59fae05291c..37b1d035e23 100644 --- a/src/azure-cli/HISTORY.rst +++ b/src/azure-cli/HISTORY.rst @@ -16,6 +16,7 @@ Release History * `az acr update`: Add `--endpoint-protocol` parameter to support specifying the endpoint protocol for the registry (#33089) * `az acr login`: Fix regional endpoint matching for registries with DNL suffix (#33381) * `az acr config content-trust/show/update`: Add deprecation labels and notices (#33174) +* `az acr login`: Fix incorrect AAD resource URL for local/private (Azure Local) registries (#33674) **AKS** diff --git a/src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py b/src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py index dd30846daa3..95236957cc8 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py +++ b/src/azure-cli/azure/cli/command_modules/acr/_docker_utils.py @@ -19,7 +19,7 @@ from knack.log import get_logger from azure.cli.core.util import should_disable_connection_verify -from azure.cli.core.cloud import CloudSuffixNotSetException +from azure.cli.core.cloud import CloudSuffixNotSetException, CloudNameEnum from azure.cli.core._profile import _AZ_LOGIN_MESSAGE from azure.cli.core.commands.client_factory import get_subscription_id from azure.cli.core.azclierror import AzureResponseError @@ -34,6 +34,17 @@ logger = get_logger(__name__) +# Known standard Azure cloud names whose ACR instances share the common +# ``https://containerregistry.azure.net`` audience. Custom / private clouds +# (e.g. Azure Local) are NOT in this set and require per-registry audience +# derived from the login server. +_STANDARD_CLOUD_NAMES = frozenset({ + CloudNameEnum.AzureCloud, + CloudNameEnum.AzureChinaCloud, + CloudNameEnum.AzureUSGovernment, + CloudNameEnum.AzureGermanCloud, + CloudNameEnum.AzureBleuCloud, +}) EMPTY_GUID = '00000000-0000-0000-0000-000000000000' ALLOWED_HTTP_METHOD = ['get', 'patch', 'put', 'delete', 'post'] @@ -126,17 +137,23 @@ def _handle_challenge_phase(login_server, return token_params -def _resolve_acr_scope(cli_ctx): +def _resolve_acr_scope(cli_ctx, login_server=None): """Determine the AAD resource (audience) to request a token for, for the ACR /oauth2/exchange endpoint. Resolution order: 1. ``az config set acr.audience_resource=`` — operator override. If the value starts with ``https://`` it is used verbatim, otherwise it is treated as a short name and expanded to ``https://.azure.net``. - 2. The default public ACR audience ``https://containerregistry.azure.net``. - - This lets disconnected environments (e.g. Azure Local Disconnected Operations) pin - the audience their local IDP knows about, instead of relying on runtime fallback. + 2. For private/local clouds (cloud name not in the set of known standard Azure cloud names), + the resource is derived from ``login_server``: ``https://``. + Standard Azure clouds (AzureCloud, AzureChinaCloud, AzureUSGovernment, etc.) always + fall through to the default audience. + 3. The default public ACR audience ``https://containerregistry.azure.net``. + + Private/disconnected environments (e.g. Azure Local) automatically use the registry's + own URL as the audience, because their local IDPs do not recognise the public + ``containerregistry.azure.net`` application. Operators may still override via + ``az config set acr.audience_resource=`` for any remaining edge cases. """ configured = None try: @@ -145,6 +162,16 @@ def _resolve_acr_scope(cli_ctx): configured = None if configured: return configured if configured.startswith('https://') else "https://{}.azure.net".format(configured) + + # For private / local clouds the per-registry URL is the correct audience. + if login_server: + try: + cloud_name = cli_ctx.cloud.name + except AttributeError: + cloud_name = None + if cloud_name and cloud_name not in _STANDARD_CLOUD_NAMES: + return "https://{}".format(login_server) + return "https://{}.azure.net".format(ACR_AUDIENCE_RESOURCE_NAME) @@ -163,7 +190,7 @@ def _get_aad_token_after_challenge(cli_ctx, from azure.cli.core._profile import Profile profile = Profile(cli_ctx=cli_ctx) - scope = _resolve_acr_scope(cli_ctx) + scope = _resolve_acr_scope(cli_ctx, login_server=login_server) # this might be a cross tenant scenario, so pass subscription to get_raw_token creds, _, tenant = profile.get_raw_token(subscription=get_subscription_id(cli_ctx), diff --git a/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py index 95c8e3c34db..a19becad388 100644 --- a/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py +++ b/src/azure-cli/azure/cli/command_modules/acr/tests/latest/test_acr_commands_mock.py @@ -1458,9 +1458,10 @@ class ResolveAcrScopeTests(unittest.TestCase): """ @staticmethod - def _ctx(configured): + def _ctx(configured, cloud_name='AzureCloud'): cli_ctx = mock.MagicMock() cli_ctx.config.get.return_value = configured + cli_ctx.cloud.name = cloud_name return cli_ctx def test_default_when_unset(self): @@ -1488,3 +1489,39 @@ def test_config_exception_falls_back_to_default(self): _resolve_acr_scope(cli_ctx), "https://{}.azure.net".format(ACR_AUDIENCE_RESOURCE_NAME), ) + + def test_private_cloud_derives_resource_from_login_server(self): + """For private/local clouds (e.g. Azure Local), the audience is derived from the + registry's login server URL so the local IDP can recognise the resource.""" + cli_ctx = self._ctx(None, cloud_name='CustomLocalCloud') + self.assertEqual( + _resolve_acr_scope(cli_ctx, login_server='edgeartifacts.edgeacr.local.private'), + "https://edgeartifacts.edgeacr.local.private", + ) + + def test_standard_cloud_uses_default_audience_regardless_of_login_server(self): + """Standard Azure clouds always use the shared containerregistry.azure.net audience.""" + for cloud_name in ('AzureCloud', 'AzureChinaCloud', 'AzureUSGovernment', + 'AzureGermanCloud', 'AzureBleuCloud'): + with self.subTest(cloud_name=cloud_name): + cli_ctx = self._ctx(None, cloud_name=cloud_name) + self.assertEqual( + _resolve_acr_scope(cli_ctx, login_server='myregistry.azurecr.io'), + "https://{}.azure.net".format(ACR_AUDIENCE_RESOURCE_NAME), + ) + + def test_private_cloud_no_login_server_uses_default(self): + """If login_server is not provided even on a private cloud, fall back to the default.""" + cli_ctx = self._ctx(None, cloud_name='CustomLocalCloud') + self.assertEqual( + _resolve_acr_scope(cli_ctx), + "https://{}.azure.net".format(ACR_AUDIENCE_RESOURCE_NAME), + ) + + def test_config_overrides_private_cloud_derivation(self): + """An explicit acr.audience_resource config always wins, even on a private cloud.""" + cli_ctx = self._ctx("https://override.example.com", cloud_name='CustomLocalCloud') + self.assertEqual( + _resolve_acr_scope(cli_ctx, login_server='edgeartifacts.edgeacr.local.private'), + "https://override.example.com", + )