Skip to content

Commit 4ae77e5

Browse files
twangboydwoz
authored andcommitted
Fix LGPO duplicate policy error when ADMX files share an identical path
When multiple ADMX files (e.g. TerminalServer.admx and TerminalServer-Server.admx on Windows Server) define the same policy display name resolving to an identical full path, _lookup_admin_template now deduplicates the matches and returns success rather than a spurious "multiple policies" error. Closes #62732
1 parent 15684ce commit 4ae77e5

4 files changed

Lines changed: 196 additions & 5 deletions

File tree

changelog/62732.fixed.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Fixed LGPO ``get_policy_info`` incorrectly returning a "multiple policies" error when duplicate ADMX policy definitions (e.g. ``TerminalServer.admx`` and ``TerminalServer-Server.admx``) resolve to the same full path.

salt/modules/win_lgpo.py

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8563,6 +8563,7 @@ def _lookup_admin_template(policy_name, policy_class, adml_language="en-US"):
85638563
)
85648564
return False, None, [], msg
85658565
else:
8566+
all_paths = []
85668567
for possible_policy in admx_search_results:
85678568
this_parent_list = _build_parent_list(
85688569
policy_definition=possible_policy,
@@ -8571,12 +8572,41 @@ def _lookup_admin_template(policy_name, policy_class, adml_language="en-US"):
85718572
)
85728573
this_parent_list.reverse()
85738574
this_parent_list.append(policy_name)
8574-
if suggested_policies:
8575-
suggested_policies = ", ".join(
8576-
[suggested_policies, "\\".join(this_parent_list)]
8575+
all_paths.append("\\".join(this_parent_list))
8576+
unique_paths = list(dict.fromkeys(all_paths))
8577+
if len(unique_paths) == 1:
8578+
# All matches resolve to the same full path (e.g.
8579+
# duplicate policy definitions across ADMX files
8580+
# like TerminalServer.admx and
8581+
# TerminalServer-Server.admx). Treat as a single
8582+
# unambiguous policy.
8583+
search_result = admx_search_results[0]
8584+
if "name" in search_result.attrib:
8585+
policy_display_name = _getFullPolicyName(
8586+
policy_item=search_result,
8587+
policy_name=search_result.attrib["name"],
8588+
return_full_policy_names=True,
8589+
adml_language=adml_language,
85778590
)
8578-
else:
8579-
suggested_policies = "\\".join(this_parent_list)
8591+
policy_aliases.append(policy_display_name)
8592+
policy_aliases.append(search_result.attrib["name"])
8593+
full_path_list = _build_parent_list(
8594+
policy_definition=search_result,
8595+
return_full_policy_names=True,
8596+
adml_language=adml_language,
8597+
)
8598+
full_path_list.reverse()
8599+
full_path_list.append(policy_display_name)
8600+
policy_aliases.append("\\".join(full_path_list))
8601+
return True, search_result, policy_aliases, None
8602+
else:
8603+
for full_path in all_paths:
8604+
if suggested_policies:
8605+
suggested_policies = ", ".join(
8606+
[suggested_policies, full_path]
8607+
)
8608+
else:
8609+
suggested_policies = full_path
85808610
if suggested_policies:
85818611
msg = (
85828612
'ADML policy name "{}" is used as the display name for '

tests/pytests/functional/modules/win_lgpo/test_get_policy_info.py

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
Unit tests for the LGPO module
33
"""
44

5+
import os
56
import platform
67

78
import pytest
@@ -55,3 +56,26 @@ def test_61859(lgpo):
5556
policy_class="machine",
5657
)
5758
assert result["message"] == expected
59+
60+
61+
_TS_SERVER_ADMX = r"C:\Windows\PolicyDefinitions\TerminalServer-Server.admx"
62+
63+
64+
@pytest.mark.skipif(
65+
not os.path.exists(_TS_SERVER_ADMX),
66+
reason="TerminalServer-Server.admx only present on Windows Server editions",
67+
)
68+
def test_62732_duplicate_policy_identical_paths(lgpo):
69+
"""
70+
On Windows Server, TerminalServer.admx and TerminalServer-Server.admx
71+
both define "Do not allow Clipboard redirection" with the same full path.
72+
get_policy_info should resolve it as a single unambiguous policy rather
73+
than returning a "multiple policies" error.
74+
"""
75+
result = lgpo.get_policy_info(
76+
policy_name="Do not allow Clipboard redirection",
77+
policy_class="Machine",
78+
)
79+
assert result["policy_found"] is True
80+
assert not result["message"]
81+
assert result["policy_name"] == "Do not allow Clipboard redirection"
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
"""
2+
Unit tests for salt.modules.win_lgpo
3+
"""
4+
5+
import pytest
6+
7+
import salt.modules.win_lgpo as win_lgpo
8+
from tests.support.mock import MagicMock, patch
9+
10+
pytestmark = [
11+
pytest.mark.windows_whitelisted,
12+
pytest.mark.skip_unless_on_windows,
13+
]
14+
15+
_ADML_TAG = "{http://www.microsoft.com/GroupPolicy/2006/07/PolicyDefinitions}string"
16+
17+
18+
def _make_adml(policy_id):
19+
fake = MagicMock()
20+
fake.tag = _ADML_TAG
21+
fake.attrib = {"id": policy_id}
22+
return fake
23+
24+
25+
def _make_admx(name):
26+
fake = MagicMock()
27+
fake.attrib = {"name": name, "class": "Machine"}
28+
return fake
29+
30+
31+
def test_duplicate_policies_identical_paths_returns_success():
32+
"""
33+
When multiple ADMX entries share the same display name but all resolve
34+
to the same full path (e.g. TerminalServer.admx and
35+
TerminalServer-Server.admx for "Do not allow Clipboard redirection"),
36+
_lookup_admin_template should return True with policy info rather than
37+
a "multiple policies" error.
38+
"""
39+
policy_name = "Do not allow Clipboard redirection"
40+
policy_class = "Machine"
41+
42+
fake_adml = _make_adml("TS_CLIENT_CLIPBOARD")
43+
fake_policy_1 = _make_admx("TS_CLIENT_CLIPBOARD")
44+
fake_policy_2 = _make_admx("TS_CLIENT_CLIPBOARD")
45+
46+
# _build_parent_list returns innermost-first; the code reverses it
47+
# in-place, so return a fresh list on every call.
48+
shared_parent = [
49+
"Device and Resource Redirection",
50+
"Remote Desktop Session Host",
51+
"Remote Desktop Services",
52+
"Windows Components",
53+
]
54+
55+
with (
56+
patch.object(win_lgpo, "_get_policy_definitions", return_value=MagicMock()),
57+
patch.object(win_lgpo, "_get_policy_resources", return_value=MagicMock()),
58+
patch.object(win_lgpo, "ADMX_SEARCH_XPATH", MagicMock(return_value=[])),
59+
patch.object(
60+
win_lgpo,
61+
"ADML_SEARCH_XPATH",
62+
MagicMock(return_value=[fake_adml]),
63+
),
64+
patch.object(
65+
win_lgpo,
66+
"ADMX_DISPLAYNAME_SEARCH_XPATH",
67+
MagicMock(return_value=[fake_policy_1, fake_policy_2]),
68+
),
69+
patch.object(
70+
win_lgpo,
71+
"_build_parent_list",
72+
side_effect=lambda **_: list(shared_parent),
73+
),
74+
patch.object(win_lgpo, "_getFullPolicyName", return_value=policy_name),
75+
):
76+
found, policy_xml, aliases, msg = win_lgpo._lookup_admin_template(
77+
policy_name, policy_class
78+
)
79+
80+
assert found is True
81+
assert msg is None
82+
assert policy_xml is fake_policy_1
83+
assert policy_name in aliases
84+
85+
86+
def test_duplicate_policies_distinct_paths_returns_error():
87+
"""
88+
When multiple ADMX entries share the same display name but resolve to
89+
genuinely different full paths (e.g. "Access data sources across
90+
domains" in multiple Internet Explorer sub-trees), _lookup_admin_template
91+
should still return False with the existing "multiple policies" error.
92+
"""
93+
policy_name = "Access data sources across domains"
94+
policy_class = "Machine"
95+
96+
fake_adml = _make_adml("SomePolicyId")
97+
fake_policy_1 = _make_admx("PolicyA")
98+
fake_policy_2 = _make_admx("PolicyB")
99+
100+
parent_path_1 = ["Security Features", "Internet Explorer", "Windows Components"]
101+
parent_path_2 = [
102+
"Internet Control Panel",
103+
"Internet Explorer",
104+
"Windows Components",
105+
]
106+
107+
with (
108+
patch.object(win_lgpo, "_get_policy_definitions", return_value=MagicMock()),
109+
patch.object(win_lgpo, "_get_policy_resources", return_value=MagicMock()),
110+
patch.object(win_lgpo, "ADMX_SEARCH_XPATH", MagicMock(return_value=[])),
111+
patch.object(
112+
win_lgpo,
113+
"ADML_SEARCH_XPATH",
114+
MagicMock(return_value=[fake_adml]),
115+
),
116+
patch.object(
117+
win_lgpo,
118+
"ADMX_DISPLAYNAME_SEARCH_XPATH",
119+
MagicMock(return_value=[fake_policy_1, fake_policy_2]),
120+
),
121+
patch.object(
122+
win_lgpo,
123+
"_build_parent_list",
124+
side_effect=[list(parent_path_1), list(parent_path_2)],
125+
),
126+
patch.object(win_lgpo, "_getFullPolicyName", return_value=policy_name),
127+
):
128+
found, policy_xml, aliases, msg = win_lgpo._lookup_admin_template(
129+
policy_name, policy_class
130+
)
131+
132+
assert found is False
133+
assert policy_xml is None
134+
assert aliases == []
135+
assert "multiple policies" in msg
136+
assert policy_name in msg

0 commit comments

Comments
 (0)