Merge pull request #56272 from twangboy/fix_lgpo_names

Properly resolve the policy name
This commit is contained in:
Daniel Wozniak 2020-03-11 15:11:01 -07:00 committed by GitHub
commit 2d78931eaf
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 150 additions and 71 deletions

View file

@ -5658,25 +5658,48 @@ def _getAdmlPresentationRefId(adml_data, ref_id):
helper function to check for a presentation label for a policy element
'''
search_results = adml_data.xpath('//*[@*[local-name() = "refId"] = "{0}"]'.format(ref_id))
prepended_text = ''
alternate_label = ''
if search_results:
for result in search_results:
the_localname = etree.QName(result.tag).localname
if the_localname == 'textBox' \
or the_localname == 'comboBox':
# We want to prefer result.text as the label, however, if it is none
# we will fall back to this method for getting the label
# Brings some code back from:
# https://github.com/saltstack/salt/pull/55823/files#diff-b2e4dac5ccc17ab548f245371ec5d6faL5658
if result.text is None:
# Get the label from the text element above the referenced
# element. For example:
# --- taken from AppPrivacy.adml ---
# <text>Force allow these specific apps (use Package Family Names):</text>
# <multiTextBox refId="LetAppsSyncWithDevices_ForceAllowTheseApps_List"/>
# In this case, the label for the refId is the text element
# above it.
presentation_element = PRESENTATION_ANCESTOR_XPATH(result)
if presentation_element:
presentation_element = presentation_element[0]
if TEXT_ELEMENT_XPATH(presentation_element):
for p_item in presentation_element.getchildren():
if p_item == result:
break
if etree.QName(p_item.tag).localname == 'text':
if getattr(p_item, 'text'):
alternate_label = getattr(p_item, 'text').rstrip()
if alternate_label.endswith('.'):
alternate_label = ''
if the_localname in ['textBox', 'comboBox']:
label_items = result.xpath('.//*[local-name() = "label"]')
for label_item in label_items:
if label_item.text:
return (prepended_text + ' ' + label_item.text.rstrip().rstrip(':')).lstrip()
elif the_localname == 'decimalTextBox' \
or the_localname == 'longDecimalTextBox' \
or the_localname == 'dropdownList' \
or the_localname == 'listBox' \
or the_localname == 'checkBox' \
or the_localname == 'text' \
or the_localname == 'multiTextBox':
return label_item.text.rstrip().rstrip(':')
elif the_localname in ['decimalTextBox', 'longDecimalTextBox',
'dropdownList', 'listBox', 'checkBox',
'text', 'multiTextBox']:
if result.text:
return (prepended_text + ' ' + result.text.rstrip().rstrip(':')).lstrip()
return result.text.rstrip().rstrip(':')
else:
return alternate_label.rstrip(':')
return None
@ -6139,6 +6162,10 @@ def _processValueItem(element, reg_key, reg_valuename, policy, parent_element,
if standard_element_expected_string and not check_deleted:
if this_element_value is not None:
# Sometimes values come in as strings
if isinstance(this_element_value, str):
log.debug('Converting {0} to bytes'.format(this_element_value))
this_element_value = this_element_value.encode('utf-32-le')
expected_string = b''.join(['['.encode('utf-16-le'),
reg_key,
encoded_null,

View file

@ -112,6 +112,7 @@ import logging
import salt.utils.data
import salt.utils.dictdiffer
import salt.utils.json
import salt.utils.stringutils
import salt.utils.versions
import salt.utils.win_functions
@ -152,6 +153,35 @@ def _compare_policies(new_policy, current_policy):
return False
def _convert_to_unicode(data):
'''
Helper function that makes sure all items in the dictionary are unicode for
comparing the existing state with the desired state. This function is only
needed for Python 2 and can be removed once we've migrated to Python 3.
The data returned by the current settings sometimes has a mix of unicode and
string values (these don't matter in Py3). This causes the comparison to
say it's not in the correct state even though it is. They basically compares
apples to apples, etc.
Also, in Python 2, the utf-16 encoded strings remain utf-16 encoded (each
character separated by `/x00`) In Python 3 it returns a utf-8 string. This
will just remove all the null bytes (`/x00`), again comparing apples to
apples.
'''
if isinstance(data, six.string_types):
data = data.replace('\x00', '')
return salt.utils.stringutils.to_unicode(data)
elif isinstance(data, dict):
return dict((_convert_to_unicode(k),
_convert_to_unicode(v))
for k, v in data.items())
elif isinstance(data, list):
return list(_convert_to_unicode(v) for v in data)
else:
return data
def set_(name,
setting=None,
policy_class=None,
@ -342,6 +372,9 @@ def set_(name,
requested_policy_check = salt.utils.json.loads(requested_policy_json)
current_policy_check = salt.utils.json.loads(current_policy_json)
if six.PY2:
current_policy_check = _convert_to_unicode(current_policy_check)
# Are the requested and current policies identical
policies_are_equal = _compare_policies(
requested_policy_check, current_policy_check)

View file

@ -15,31 +15,27 @@ from tests.support.unit import TestCase, skipIf
# Import Salt Libs
import salt.config
import salt.modules.cmdmod
import salt.modules.file
import salt.modules.win_file as win_file
import salt.loader
import salt.modules.win_lgpo as win_lgpo
import salt.states.win_lgpo
import salt.utils.platform
import salt.utils.win_dacl
import salt.utils.win_lgpo_auditpol
import salt.utils.win_reg
import salt.utils.stringutils
# Import 3rd Party Libs
import salt.ext.six as six
# We're going to actually use the loader, without grains (slow)
opts = salt.config.DEFAULT_MINION_OPTS.copy()
utils = salt.loader.utils(opts)
modules = salt.loader.minion_mods(opts, utils=utils)
LOADER_DICTS = {
win_lgpo: {
'__salt__': {
'file.file_exists': salt.modules.file.file_exists,
'file.makedirs': win_file.makedirs_,
'file.write': salt.modules.file.write,
'file.remove': win_file.remove,
'cmd.run': salt.modules.cmdmod.run},
'__opts__': salt.config.DEFAULT_MINION_OPTS.copy(),
'__utils__': {
'reg.read_value': salt.utils.win_reg.read_value,
'auditpol.get_auditpol_dump':
salt.utils.win_lgpo_auditpol.get_auditpol_dump}},
win_file: {
'__utils__': {
'dacl.set_perms': salt.utils.win_dacl.set_perms}}}
'__opts__': opts,
'__salt__': modules,
'__utils__': utils,
}
}
class WinLGPOTestCase(TestCase):
@ -687,13 +683,16 @@ class WinLGPOGetPointAndPrintENTestCase(TestCase, LoaderModuleMockMixin):
policy_class=policy_class,
adml_language='en-US')
if success:
return salt.modules.win_lgpo._get_policy_adm_setting(
results = salt.modules.win_lgpo._get_policy_adm_setting(
admx_policy=policy_obj,
policy_class=policy_class,
adml_language='en-US',
return_full_policy_names=return_full_policy_names,
hierarchical_return=hierarchical_return
)
if six.PY2:
results = salt.states.win_lgpo._convert_to_unicode(results)
return results
return 'Policy Not Found'
def test_point_and_print_enabled(self):
@ -713,7 +712,7 @@ class WinLGPOGetPointAndPrintENTestCase(TestCase, LoaderModuleMockMixin):
True,
'PointAndPrint_TrustedServers_Chk':
True,
u'PointAndPrint_TrustedServers_Edit':
'PointAndPrint_TrustedServers_Edit':
'fakeserver1;fakeserver2'}}
self.assertDictEqual(result, expected)
@ -737,7 +736,7 @@ class WinLGPOGetPointAndPrintENTestCase(TestCase, LoaderModuleMockMixin):
True,
'PointAndPrint_TrustedServers_Chk':
True,
u'PointAndPrint_TrustedServers_Edit':
'PointAndPrint_TrustedServers_Edit':
'fakeserver1;fakeserver2'}}}}}
self.assertDictEqual(result, expected)
@ -756,8 +755,8 @@ class WinLGPOGetPointAndPrintENTestCase(TestCase, LoaderModuleMockMixin):
'Show warning and elevation prompt',
'Users can only point and print to machines in their forest':
True,
u'Users can only point and print to these servers': True,
u'When updating drivers for an existing connection':
'Users can only point and print to these servers': True,
'When updating drivers for an existing connection':
'Show warning only'}}
self.assertDictEqual(result, expected)
@ -781,8 +780,36 @@ class WinLGPOGetPointAndPrintENTestCase(TestCase, LoaderModuleMockMixin):
'Users can only point and print to machines in '
'their forest':
True,
u'Users can only point and print to these servers':
'Users can only point and print to these servers':
True,
u'When updating drivers for an existing connection':
'When updating drivers for an existing connection':
'Show warning only'}}}}}
self.assertDictEqual(result, expected)
@skipIf(not salt.utils.platform.is_windows(), 'System is not Windows')
class WinLGPOGetPolicyFromPolicyResources(TestCase, LoaderModuleMockMixin):
'''
Test functions related to policy info gathered from ADMX/ADML files
'''
adml_data = None
def setup_loader_modules(self):
return LOADER_DICTS
def setUp(self):
if self.adml_data is None:
self.adml_data = win_lgpo._get_policy_resources('en-US')
def test__getAdmlPresentationRefId(self):
ref_id = 'LetAppsAccessAccountInfo_Enum'
expected = 'Default for all apps'
result = win_lgpo._getAdmlPresentationRefId(self.adml_data, ref_id)
self.assertEqual(result, expected)
def test__getAdmlPresentationRefId_result_text_is_none(self):
ref_id = 'LetAppsAccessAccountInfo_UserInControlOfTheseApps_List'
expected = 'Put user in control of these specific apps (use Package ' \
'Family Names)'
result = win_lgpo._getAdmlPresentationRefId(self.adml_data, ref_id)
self.assertEqual(result, expected)

View file

@ -13,37 +13,26 @@ from tests.support.unit import TestCase, skipIf
# Import Salt Libs
import salt.config
import salt.modules.cmdmod
import salt.modules.file
import salt.modules.win_file as win_file
import salt.modules.win_lgpo as win_lgpo_mod
import salt.loader
import salt.states.win_lgpo as win_lgpo
import salt.utils.platform
import salt.utils.win_dacl
import salt.utils.win_lgpo_auditpol
import salt.utils.win_reg
import salt.utils.stringutils
# Import 3rd Party Libs
import salt.ext.six as six
# We're going to actually use the loader, without grains (slow)
opts = salt.config.DEFAULT_MINION_OPTS.copy()
utils = salt.loader.utils(opts)
modules = salt.loader.minion_mods(opts, utils=utils)
LOADER_DICTS = {
win_lgpo: {
'__salt__': {
'lgpo.get_policy': win_lgpo_mod.get_policy,
'lgpo.get_policy_info': win_lgpo_mod.get_policy_info,
'lgpo.set': win_lgpo_mod.set_}},
win_lgpo_mod: {
'__salt__': {
'cmd.run': salt.modules.cmdmod.run,
'file.file_exists': salt.modules.file.file_exists,
'file.makedirs': win_file.makedirs_,
'file.remove': win_file.remove,
'file.write': salt.modules.file.write},
'__opts__': salt.config.DEFAULT_MINION_OPTS.copy(),
'__utils__': {
'reg.read_value': salt.utils.win_reg.read_value,
'auditpol.get_auditpol_dump':
salt.utils.win_lgpo_auditpol.get_auditpol_dump}},
win_file: {
'__utils__': {
'dacl.set_perms': salt.utils.win_dacl.set_perms}}}
'__opts__': opts,
'__salt__': modules,
'__utils__': utils,
}
}
class WinLGPOComparePoliciesTestCase(TestCase):
@ -193,6 +182,7 @@ class WinLGPOPolicyElementNames(TestCase, LoaderModuleMockMixin):
with patch.dict(win_lgpo.__opts__, {'test': False}):
result = win_lgpo.set_(name='test_state',
computer_policy=computer_policy)
result = win_lgpo._convert_to_unicode(result)
expected = {
'Point and Print Restrictions': {
'Enter fully qualified server names separated by '
@ -203,9 +193,9 @@ class WinLGPOPolicyElementNames(TestCase, LoaderModuleMockMixin):
'Users can only point and print to machines in '
'their forest':
True,
u'Users can only point and print to these servers':
'Users can only point and print to these servers':
True,
u'When updating drivers for an existing connection':
'When updating drivers for an existing connection':
'Show warning only'}}
self.assertDictEqual(
result['changes']['new']['Computer Configuration'], expected)
@ -231,6 +221,8 @@ class WinLGPOPolicyElementNames(TestCase, LoaderModuleMockMixin):
with patch.dict(win_lgpo.__opts__, {'test': False}):
result = win_lgpo.set_(name='test_state',
computer_policy=computer_policy)
if six.PY2:
result = win_lgpo._convert_to_unicode(result)
expected = {
'Point and Print Restrictions': {
'Enter fully qualified server names separated by '
@ -241,9 +233,9 @@ class WinLGPOPolicyElementNames(TestCase, LoaderModuleMockMixin):
'Users can only point and print to machines in '
'their forest':
True,
u'Users can only point and print to these servers':
'Users can only point and print to these servers':
True,
u'When updating drivers for an existing connection':
'When updating drivers for an existing connection':
'Show warning only'}}
self.assertDictEqual(
result['changes']['new']['Computer Configuration'], expected)
@ -332,7 +324,7 @@ class WinLGPOPolicyElementNamesTestTrue(TestCase, LoaderModuleMockMixin):
'comment': 'All specified policies are properly configured'}
self.assertDictEqual(result['changes'], expected['changes'])
self.assertTrue(result['result'])
self.assertEqual(result['comment'], result['comment'])
self.assertEqual(result['comment'], expected['comment'])
def test_old_element_naming_style(self):
computer_policy = {
@ -362,7 +354,7 @@ class WinLGPOPolicyElementNamesTestTrue(TestCase, LoaderModuleMockMixin):
'All specified policies are properly configured'}
self.assertDictEqual(result['changes'], expected['changes'])
self.assertTrue(result['result'])
self.assertEqual(result['comment'], result['comment'])
self.assertEqual(result['comment'], expected['comment'])
def test_invalid_elements(self):
computer_policy = {