diff --git a/salt/modules/win_lgpo.py b/salt/modules/win_lgpo.py index 0c9c30376ad3..1892fa497f88 100644 --- a/salt/modules/win_lgpo.py +++ b/salt/modules/win_lgpo.py @@ -4918,7 +4918,7 @@ def _parse_xml(adm_file): for file_path in file_list: os.remove(file_path) - # Load the file + # Lowercase all the keys with salt.utils.files.fopen(adm_file, 'rb') as rfh: encoding = 'utf-8' @@ -4938,6 +4938,13 @@ def _parse_xml(adm_file): found_key = True modified_xml += line + '\r\n' + # Convert smart quotes to regular quotes + modified_xml = modified_xml.replace('\u201c', '"').replace('\u201d', '"') + modified_xml = modified_xml.replace('\u2018', '\'').replace('\u2019', '\'') + + # Convert em dash and en dash to dash + modified_xml = modified_xml.replace('\u2013', '-').replace('\u2014', '-') + with salt.utils.files.fopen(out_file, 'wb') as wfh: wfh.write(modified_xml.encode(encoding)) @@ -5655,23 +5662,6 @@ def _getAdmlPresentationRefId(adml_data, ref_id): if search_results: for result in search_results: the_localname = etree.QName(result.tag).localname - 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 - else: - if etree.QName(p_item.tag).localname == 'text': - if prepended_text: - prepended_text = ' '.join((text for text in (prepended_text, getattr(p_item, 'text', '').rstrip()) if text)) - else: - prepended_text = getattr(p_item, 'text', '').rstrip() if getattr(p_item, 'text', '') else '' - else: - prepended_text = '' - if prepended_text.endswith('.'): - prepended_text = '' if the_localname == 'textBox' \ or the_localname == 'comboBox': label_items = result.xpath('.//*[local-name() = "label"]') @@ -6652,13 +6642,14 @@ def _checkAllAdmxPolicies(policy_class, unpathed_dict[policy_namespace] = {} unpathed_dict[policy_namespace][full_names[policy_namespace][policy_item]] = policy_item # go back and remove any "unpathed" policies that need a full path - for path_needed in unpathed_dict[policy_namespace]: - # remove the item with the same full name and re-add it w/a path'd version - full_path_list = hierarchy[policy_namespace][unpathed_dict[policy_namespace][path_needed]] - full_path_list.reverse() - full_path_list.append(path_needed) - log.trace('full_path_list == %s', full_path_list) - policy_vals['\\'.join(full_path_list)] = policy_vals[policy_namespace].pop(path_needed) + if policy_namespace in unpathed_dict: + for path_needed in unpathed_dict[policy_namespace]: + # remove the item with the same full name and re-add it w/a path'd version + full_path_list = hierarchy[policy_namespace][unpathed_dict[policy_namespace][path_needed]] + full_path_list.reverse() + full_path_list.append(path_needed) + log.trace('full_path_list == %s', full_path_list) + policy_vals['\\'.join(full_path_list)] = policy_vals[policy_namespace].pop(path_needed) log.trace('Compilation complete: %s seconds', time.time() - start_time) for policy_namespace in list(policy_vals): if policy_vals[policy_namespace] == {}: @@ -8226,10 +8217,8 @@ def _get_policy_adm_setting(admx_policy, configured_elements[this_element_name] = True log.trace('element %s is configured true', child_item.attrib['id']) - elif etree.QName(child_item).localname == 'decimal' \ - or etree.QName(child_item).localname == 'text' \ - or etree.QName(child_item).localname == 'longDecimal' \ - or etree.QName(child_item).localname == 'multiText': + elif etree.QName(child_item).localname in [ + 'decimal', 'text', 'longDecimal', 'multiText']: # https://msdn.microsoft.com/en-us/library/dn605987(v=vs.85).aspx if _regexSearchRegPolData( re.escape(_processValueItem( @@ -8301,10 +8290,12 @@ def _get_policy_adm_setting(admx_policy, configured_elements[this_element_name] = _getAdmlDisplayName( adml_xml_data=adml_policy_resources, display_name=enum_item.attrib['displayName']) + break else: configured_elements[this_element_name] = _getAdmlDisplayName( adml_xml_data=adml_policy_resources, display_name=enum_item.attrib['displayName']) + break elif etree.QName(child_item).localname == 'list': return_value_name = False if 'explicitValue' in child_item.attrib \ diff --git a/salt/states/win_lgpo.py b/salt/states/win_lgpo.py index 3c72d42e23a1..52feecc864af 100644 --- a/salt/states/win_lgpo.py +++ b/salt/states/win_lgpo.py @@ -112,6 +112,7 @@ import salt.utils.data import salt.utils.dictdiffer import salt.utils.json +import salt.utils.versions import salt.utils.win_functions # Import 3rd party libs @@ -266,6 +267,7 @@ def set_(name, 'policy_lookup': {}}} current_policy = {} + deprecation_comments = [] for p_class, p_data in six.iteritems(pol_data): if p_data['requested_policy']: for p_name, _ in six.iteritems(p_data['requested_policy']): @@ -283,10 +285,37 @@ def set_(name, policy_class=p_class, adml_language=adml_language, return_value_only=True) + # Validate element names + if isinstance(p_data['requested_policy'][p_name], dict): + valid_names = [] + for element in lookup['policy_elements']: + valid_names.extend(element['element_aliases']) + for e_name in p_data['requested_policy'][p_name]: + if e_name not in valid_names: + new_e_name = e_name.split(':')[-1].strip() + # If we find an invalid name, test the new + # format. If found, replace the old with the + # new + if new_e_name in valid_names: + msg = 'The LGPO module changed the way ' \ + 'it gets policy element names.\n'\ + '"{0}" is no longer valid.\n'\ + 'Please use "{1}" instead.' \ + ''.format(e_name, new_e_name) + salt.utils.versions.warn_until('Phosphorus', msg) + pol_data[p_class]['requested_policy'][p_name][new_e_name] = \ + pol_data[p_class]['requested_policy'][p_name].pop(e_name) + deprecation_comments.append(msg) + else: + msg = 'Invalid element name: {0}'.format(e_name) + ret['comment'] = '\n'.join([ret['comment'], msg]).strip() + ret['result'] = False else: - ret['comment'] = ' '.join([ret['comment'], lookup['message']]) + ret['comment'] = '\n'.join([ret['comment'], lookup['message']]).strip() ret['result'] = False if not ret['result']: + deprecation_comments.append(ret['comment']) + ret['comment'] = '\n'.join(deprecation_comments).strip() return ret log.debug('pol_data == %s', pol_data) @@ -299,8 +328,6 @@ def set_(name, if requested_policy: for p_name, p_setting in six.iteritems(requested_policy): if p_name in current_policy[class_map[p_class]]: - currently_set = True - if currently_set: # compare log.debug('need to compare %s from current/requested ' 'policy', p_name) @@ -320,52 +347,38 @@ def set_(name, requested_policy_check, current_policy_check) if not policies_are_equal: - additional_policy_comments = [] if p_data['policy_lookup'][p_name]['rights_assignment'] and cumulative_rights_assignments: for user in p_data['requested_policy'][p_name]: if user not in current_policy[class_map[p_class]][p_name]: user = salt.utils.win_functions.get_sam_name(user) if user not in current_policy[class_map[p_class]][p_name]: changes = True - else: - additional_policy_comments.append('"{0}" is already granted the right'.format(user)) - else: - additional_policy_comments.append('"{0}" is already granted the right'.format(user)) else: changes = True if changes: - log.debug('%s current policy != requested policy', - p_name) - log.debug( - 'we compared %s to %s', - requested_policy_json, current_policy_json - ) + log.debug('%s current policy != requested policy', p_name) + log.debug('We compared %s to %s', requested_policy_json, current_policy_json) policy_changes.append(p_name) - else: - if additional_policy_comments: - ret['comment'] = '"{0}" is already set ({1})\n'.format(p_name, ', '.join(additional_policy_comments)) - else: - ret['comment'] = '"{0}" is already set\n'.format(p_name) + ret['comment'] else: - log.debug('%s current setting matches ' - 'the requested setting', p_name) - ret['comment'] = '"{0}" is already set\n'.format(p_name) + ret['comment'] + msg = '"{0}" is already set'.format(p_name) + log.debug(msg) else: policy_changes.append(p_name) - log.debug('policy %s is not set, we will configure it', - p_name) + log.debug('policy %s is not set, we will configure it', p_name) if __opts__['test']: if policy_changes: + msg = 'The following policies are set to change:\n{0}' \ + ''.format('\n'.join(policy_changes)) ret['result'] = None - ret['comment'] = 'The following policies are set to change:\n{0}'.format( - '\n'.join(policy_changes)) else: - ret['comment'] = 'All specified policies are properly configured' + msg = 'All specified policies are properly configured' + deprecation_comments.append(msg) + ret['comment'] = '\n'.join(deprecation_comments).strip() else: if policy_changes: _ret = __salt__['lgpo.set']( - computer_policy=computer_policy, - user_policy=user_policy, + computer_policy=pol_data['machine']['requested_policy'], + user_policy=pol_data['user']['requested_policy'], cumulative_rights_assignments=cumulative_rights_assignments, adml_language=adml_language) if _ret: @@ -373,8 +386,7 @@ def set_(name, new_policy = {} for p_class, p_data in six.iteritems(pol_data): if p_data['requested_policy']: - for p_name, p_setting in six.iteritems( - p_data['requested_policy']): + for p_name, p_setting in six.iteritems(p_data['requested_policy']): new_policy.setdefault(class_map[p_class], {}) new_policy[class_map[p_class]][p_name] = __salt__['lgpo.get_policy']( policy_name=p_name, @@ -384,13 +396,20 @@ def set_(name, ret['changes'] = salt.utils.dictdiffer.deep_diff( old=current_policy, new=new_policy) if ret['changes']: - ret['comment'] = 'The following policies changed:\n{0}' \ - ''.format('\n'.join(policy_changes)) + msg = 'The following policies changed:\n{0}' \ + ''.format('\n'.join(policy_changes)) else: - ret['comment'] = 'The following policies are in the correct state:\n{0}' \ - ''.format('\n'.join(policy_changes)) + msg = 'The following policies are in the correct ' \ + 'state:\n{0}'.format('\n'.join(policy_changes)) else: + msg = 'Errors occurred while attempting to configure ' \ + 'policies: {0}'.format(_ret) ret['result'] = False - ret['comment'] = 'Errors occurred while attempting to configure policies: {0}'.format(_ret) + deprecation_comments.append(msg) + ret['comment'] = '\n'.join(deprecation_comments).strip() + else: + msg = 'All specified policies are properly configured' + deprecation_comments.append(msg) + ret['comment'] = '\n'.join(deprecation_comments).strip() return ret diff --git a/salt/version.py b/salt/version.py index def1cbd91a94..26738c5e316b 100644 --- a/salt/version.py +++ b/salt/version.py @@ -109,9 +109,9 @@ class SaltStackVersion(object): 'Sodium' : (MAX_SIZE - 98, 0), 'Magnesium' : (MAX_SIZE - 97, 0), 'Aluminium' : (MAX_SIZE - 96, 0), + 'Silicon' : (MAX_SIZE - 95, 0), + 'Phosphorus' : (MAX_SIZE - 94, 0), # pylint: disable=E8265 - #'Silicon' : (MAX_SIZE - 95, 0), - #'Phosphorus' : (MAX_SIZE - 94, 0), #'Sulfur' : (MAX_SIZE - 93, 0), #'Chlorine' : (MAX_SIZE - 92, 0), #'Argon' : (MAX_SIZE - 91, 0), diff --git a/tests/integration/modules/test_win_lgpo.py b/tests/integration/modules/test_win_lgpo.py index f4ec94651322..a66e4e51c577 100644 --- a/tests/integration/modules/test_win_lgpo.py +++ b/tests/integration/modules/test_win_lgpo.py @@ -215,7 +215,7 @@ def test_set_user_policy_point_and_print_restrictions(self): 'Users can only point and print to these servers': True, 'Enter fully qualified server names separated by semicolons': 'fakeserver1;fakeserver2', 'Users can only point and print to machines in their forest': True, - 'Security Prompts: When installing drivers for a new connection': 'Show warning and elevation prompt', + 'When installing drivers for a new connection': 'Show warning and elevation prompt', 'When updating drivers for an existing connection': 'Show warning only', }, [ diff --git a/tests/unit/modules/test_win_lgpo.py b/tests/unit/modules/test_win_lgpo.py index 7b54e5d8e89c..3373e88aad79 100644 --- a/tests/unit/modules/test_win_lgpo.py +++ b/tests/unit/modules/test_win_lgpo.py @@ -585,8 +585,7 @@ def setUp(self): 'Users can only point and print to machines in their ' 'forest': True, - 'Security Prompts: When installing drivers for a new ' - 'connection': + 'When installing drivers for a new connection': 'Show warning and elevation prompt', 'When updating drivers for an existing connection': 'Show warning only', @@ -671,8 +670,7 @@ def test_point_and_print_enabled_full_names(self): 'Printers\\Point and Print Restrictions': { 'Enter fully qualified server names separated by semicolons': 'fakeserver1;fakeserver2', - 'Security Prompts: When installing drivers for a new ' - 'connection': + 'When installing drivers for a new connection': 'Show warning and elevation prompt', 'Users can only point and print to machines in their forest': True, @@ -696,8 +694,7 @@ def test_point_and_print_enabled_full_names_hierarchical(self): 'Enter fully qualified server names separated by ' 'semicolons': 'fakeserver1;fakeserver2', - 'Security Prompts: When installing drivers for a ' - 'new connection': + 'When installing drivers for a new connection': 'Show warning and elevation prompt', 'Users can only point and print to machines in ' 'their forest': diff --git a/tests/unit/states/test_win_lgpo.py b/tests/unit/states/test_win_lgpo.py index cec2246c49d3..e96751129bbb 100644 --- a/tests/unit/states/test_win_lgpo.py +++ b/tests/unit/states/test_win_lgpo.py @@ -2,24 +2,54 @@ ''' :codeauthor: Shane Lee ''' - # Import Python Libs from __future__ import absolute_import, unicode_literals, print_function # Import Salt Testing Libs +from tests.support.helpers import destructiveTest +from tests.support.mixins import LoaderModuleMockMixin +from tests.support.mock import patch from tests.support.unit import TestCase, skipIf -import salt.utils.platform # 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.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 + +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}}} -@skipIf(not salt.utils.platform.is_windows(), 'LGPO not applicable') -class WinSystemTestCase(TestCase): +class WinLGPOComparePoliciesTestCase(TestCase): ''' Test cases for the win_lgpo state ''' - def test__compare_policies_string(self): ''' ``_compare_policies`` should only return ``True`` when the string values @@ -127,3 +157,229 @@ def test__compare_policies_integer(self): self.assertFalse( win_lgpo._compare_policies(compare_integer, None) ) + + +@destructiveTest +@skipIf(not salt.utils.platform.is_windows(), 'System is not Windows') +class WinLGPOPolicyElementNames(TestCase, LoaderModuleMockMixin): + ''' + Test variations of the Point and Print Restrictions policy when Not + Configured (NC) + ''' + def setup_loader_modules(self): + return LOADER_DICTS + + def setUp(self): + computer_policy = {'Point and Print Restrictions': 'Not Configured'} + with patch.dict(win_lgpo.__opts__, {'test': False}): + win_lgpo.set_(name='nc_state', computer_policy=computer_policy) + + def test_current_element_naming_style(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Users can only point and print to these servers': + True, + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'Users can only point and print to machines in their ' + 'forest': + True, + 'When installing drivers for a new connection': + 'Show warning and elevation prompt', + 'When updating drivers for an existing connection': + 'Show warning only', + }} + with patch.dict(win_lgpo.__opts__, {'test': False}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'Point and Print Restrictions': { + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'When installing drivers for a new connection': + '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': + 'Show warning only'}} + self.assertDictEqual( + result['changes']['new']['Computer Configuration'], expected) + + def test_old_element_naming_style(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Users can only point and print to these servers': + True, + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'Users can only point and print to machines in their ' + 'forest': + True, + # Here's the old one + 'Security Prompts: When installing drivers for a new connection': + 'Show warning and elevation prompt', + 'When updating drivers for an existing connection': + 'Show warning only', + }} + + with patch.dict(win_lgpo.__opts__, {'test': False}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'Point and Print Restrictions': { + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'When installing drivers for a new connection': + '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': + 'Show warning only'}} + self.assertDictEqual( + result['changes']['new']['Computer Configuration'], expected) + expected = 'The LGPO module changed the way it gets policy element names.\n' \ + '"Security Prompts: When installing drivers for a new connection" is no longer valid.\n' \ + 'Please use "When installing drivers for a new connection" instead.\n' \ + 'The following policies changed:\n' \ + 'Point and Print Restrictions' + self.assertEqual(result['comment'], expected) + + def test_invalid_elements(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Invalid element spongebob': True, + 'Invalid element squidward': False}} + + with patch.dict(win_lgpo.__opts__, {'test': False}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'changes': {}, + 'comment': 'Invalid element name: Invalid element squidward\n' + 'Invalid element name: Invalid element spongebob', + 'name': 'test_state', + 'result': False} + self.assertDictEqual(result['changes'], expected['changes']) + self.assertIn('Invalid element squidward', result['comment']) + self.assertIn('Invalid element spongebob', result['comment']) + self.assertFalse(expected['result']) + + +@destructiveTest +@skipIf(not salt.utils.platform.is_windows(), 'System is not Windows') +class WinLGPOPolicyElementNamesTestTrue(TestCase, LoaderModuleMockMixin): + ''' + Test variations of the Point and Print Restrictions policy when Not + Configured (NC) + ''' + configured = False + + def setup_loader_modules(self): + return LOADER_DICTS + + def setUp(self): + if not self.configured: + computer_policy = { + 'Point and Print Restrictions': { + 'Users can only point and print to these servers': + True, + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'Users can only point and print to machines in their ' + 'forest': + True, + 'When installing drivers for a new connection': + 'Show warning and elevation prompt', + 'When updating drivers for an existing connection': + 'Show warning only', + }} + with patch.dict(win_lgpo.__opts__, {'test': False}): + win_lgpo.set_(name='nc_state', computer_policy=computer_policy) + self.configured = True + + def test_current_element_naming_style(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Users can only point and print to these servers': + True, + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'Users can only point and print to machines in their ' + 'forest': + True, + 'When installing drivers for a new connection': + 'Show warning and elevation prompt', + 'When updating drivers for an existing connection': + 'Show warning only', + }} + with patch.dict(win_lgpo.__opts__, {'test': True}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'changes': {}, + 'comment': 'All specified policies are properly configured'} + self.assertDictEqual(result['changes'], expected['changes']) + self.assertTrue(result['result']) + self.assertEqual(result['comment'], result['comment']) + + def test_old_element_naming_style(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Users can only point and print to these servers': + True, + 'Enter fully qualified server names separated by ' + 'semicolons': + 'fakeserver1;fakeserver2', + 'Users can only point and print to machines in their ' + 'forest': + True, + # Here's the old one + 'Security Prompts: When installing drivers for a new connection': + 'Show warning and elevation prompt', + 'When updating drivers for an existing connection': + 'Show warning only', + }} + with patch.dict(win_lgpo.__opts__, {'test': True}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'changes': {}, + 'comment': 'The LGPO module changed the way it gets policy element names.\n' + '"Security Prompts: When installing drivers for a new connection" is no longer valid.\n' + 'Please use "When installing drivers for a new connection" instead.\n' + 'All specified policies are properly configured'} + self.assertDictEqual(result['changes'], expected['changes']) + self.assertTrue(result['result']) + self.assertEqual(result['comment'], result['comment']) + + def test_invalid_elements(self): + computer_policy = { + 'Point and Print Restrictions': { + 'Invalid element spongebob': True, + 'Invalid element squidward': False}} + + with patch.dict(win_lgpo.__opts__, {'test': True}): + result = win_lgpo.set_(name='test_state', + computer_policy=computer_policy) + expected = { + 'changes': {}, + 'comment': 'Invalid element name: Invalid element squidward\n' + 'Invalid element name: Invalid element spongebob', + 'name': 'test_state', + 'result': False} + self.assertDictEqual(result['changes'], expected['changes']) + self.assertIn('Invalid element squidward', result['comment']) + self.assertIn('Invalid element spongebob', result['comment']) + self.assertFalse(expected['result'])