From 30de4542efc6e656d7de698783764398a7663dd2 Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 09:53:29 -0400 Subject: [PATCH 1/2] [App Service] Fix #30357: `az webapp config ssl bind`: Use full Site object to avoid Azure Policy denial Previously, _update_host_name_ssl_state constructed a minimal Site object with only host_name_ssl_states, location, and tags. When passed to begin_create_or_update, this caused Azure Policy (e.g. 'HTTPS Only must be enabled') to deny the operation because policy-sensitive fields like httpsOnly were missing from the payload. The fix reuses the full existing Site object fetched from Azure, updating only the host_name_ssl_states field. This preserves all policy-sensitive properties (httpsOnly, ftpsState, etc.) in the PUT request. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../cli/command_modules/appservice/custom.py | 13 +++-- .../latest/test_webapp_commands_thru_mock.py | 54 +++++++++++++++++++ 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/custom.py b/src/azure-cli/azure/cli/command_modules/appservice/custom.py index 386ba088608..d04d9cd7ca5 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -6042,14 +6042,13 @@ def _check_service_principal_permissions(cmd, resource_group_name, key_vault_nam def _update_host_name_ssl_state(cmd, resource_group_name, webapp_name, webapp, host_name, ssl_state, thumbprint, slot=None): - Site, HostNameSslState = cmd.get_models('Site', 'HostNameSslState') - updated_webapp = Site(host_name_ssl_states=[HostNameSslState(name=host_name, - ssl_state=ssl_state, - thumbprint=thumbprint, - to_update=True)], - location=webapp.location, tags=webapp.tags) + HostNameSslState = cmd.get_models('HostNameSslState') + webapp.host_name_ssl_states = [HostNameSslState(name=host_name, + ssl_state=ssl_state, + thumbprint=thumbprint, + to_update=True)] return _generic_site_operation(cmd.cli_ctx, resource_group_name, webapp_name, 'begin_create_or_update', - slot, updated_webapp) + slot, webapp) def _update_ssl_binding(cmd, resource_group_name, name, certificate_thumbprint, ssl_type, hostname, slot=None): diff --git a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py index 853eadc1edd..71501756a5b 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py @@ -20,6 +20,7 @@ view_in_browser, sync_site_repo, _match_host_names_from_cert, + _update_host_name_ssl_state, bind_ssl_cert, list_publish_profiles, show_app, @@ -639,6 +640,59 @@ def test_update_webapp_platform_release_channel_latest(self): self.assertEqual(result.additional_properties["properties"]["platformReleaseChannel"], "Latest") +class TestUpdateHostNameSslState(unittest.TestCase): + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation', autospec=True) + def test_update_host_name_ssl_state_passes_full_site(self, generic_site_op_mock): + """Test that _update_host_name_ssl_state passes the full Site object (not a partial one) + to begin_create_or_update, preserving policy-sensitive fields like https_only.""" + cmd_mock = _get_test_cmd() + Site, HostNameSslState, SslState = cmd_mock.get_models('Site', 'HostNameSslState', 'SslState') + + webapp = Site(name='mySite', location='eastus', tags={'env': 'prod'}) + webapp.https_only = True + webapp.host_name_ssl_states = [ + HostNameSslState(name='existing.contoso.com', + ssl_state=SslState.sni_enabled, + thumbprint='EXISTINGTHUMB') + ] + + _update_host_name_ssl_state(cmd_mock, 'myRg', 'mySite', webapp, + 'www.contoso.com', SslState.sni_enabled, 'NEWTHUMB') + + generic_site_op_mock.assert_called_once() + call_args = generic_site_op_mock.call_args + passed_site = call_args[0][5] # (cli_ctx, rg, name, op, slot, site) + + # The passed object should be the original webapp, preserving all fields + self.assertTrue(passed_site.https_only, + "https_only must be preserved to avoid Azure Policy denial") + self.assertEqual(passed_site.location, 'eastus') + self.assertEqual(passed_site.tags, {'env': 'prod'}) + + # host_name_ssl_states should contain only the binding being updated + self.assertEqual(len(passed_site.host_name_ssl_states), 1) + ssl_state = passed_site.host_name_ssl_states[0] + self.assertEqual(ssl_state.name, 'www.contoso.com') + self.assertEqual(ssl_state.thumbprint, 'NEWTHUMB') + self.assertTrue(ssl_state.to_update) + + @mock.patch('azure.cli.command_modules.appservice.custom._generic_site_operation', autospec=True) + def test_update_host_name_ssl_state_with_slot(self, generic_site_op_mock): + """Test that slot parameter is correctly forwarded.""" + cmd_mock = _get_test_cmd() + Site, SslState = cmd_mock.get_models('Site', 'SslState') + webapp = Site(name='mySite', location='westus') + + _update_host_name_ssl_state(cmd_mock, 'myRg', 'mySite', webapp, + 'www.contoso.com', SslState.sni_enabled, 'THUMB', slot='staging') + + call_args = generic_site_op_mock.call_args + # slot is the 5th positional arg (index 4) after cli_ctx, rg, name, operation_name + self.assertEqual(call_args[0][4], 'staging') + # site is the 6th positional arg (index 5) + self.assertIs(call_args[0][5], webapp) + + class FakedResponse: # pylint: disable=too-few-public-methods def __init__(self, status_code): self.status_code = status_code From c95dc5bcbddee82bbc4c89de17fb69bf2d683bfd Mon Sep 17 00:00:00 2001 From: Jordan Selig Date: Thu, 26 Mar 2026 10:21:33 -0400 Subject: [PATCH 2/2] Address review: fetch slot Site and use property assertions - In _update_ssl_binding, use get_slot() when slot is provided to avoid sending production Site payload for slot updates (policy/settings issue) - Replace assertIs with property assertions in slot test for robustness Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../azure/cli/command_modules/appservice/custom.py | 5 ++++- .../tests/latest/test_webapp_commands_thru_mock.py | 6 ++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/custom.py b/src/azure-cli/azure/cli/command_modules/appservice/custom.py index d04d9cd7ca5..b90288918d1 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/custom.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/custom.py @@ -6053,7 +6053,10 @@ def _update_host_name_ssl_state(cmd, resource_group_name, webapp_name, webapp, def _update_ssl_binding(cmd, resource_group_name, name, certificate_thumbprint, ssl_type, hostname, slot=None): client = web_client_factory(cmd.cli_ctx) - webapp = client.web_apps.get(resource_group_name, name) + if slot: + webapp = client.web_apps.get_slot(resource_group_name, name, slot) + else: + webapp = client.web_apps.get(resource_group_name, name) if not webapp: raise ResourceNotFoundError("'{}' app doesn't exist".format(name)) diff --git a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py index 71501756a5b..5387fe8033c 100644 --- a/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py +++ b/src/azure-cli/azure/cli/command_modules/appservice/tests/latest/test_webapp_commands_thru_mock.py @@ -689,8 +689,10 @@ def test_update_host_name_ssl_state_with_slot(self, generic_site_op_mock): call_args = generic_site_op_mock.call_args # slot is the 5th positional arg (index 4) after cli_ctx, rg, name, operation_name self.assertEqual(call_args[0][4], 'staging') - # site is the 6th positional arg (index 5) - self.assertIs(call_args[0][5], webapp) + # site is the 6th positional arg (index 5); verify it has the expected properties + site_arg = call_args[0][5] + self.assertEqual(site_arg.name, webapp.name) + self.assertEqual(site_arg.location, webapp.location) class FakedResponse: # pylint: disable=too-few-public-methods