Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix issue with overly long names in the LGPO module #55823

Merged
merged 10 commits into from
Jan 15, 2020
Merged

Fix issue with overly long names in the LGPO module #55823

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
3b13460
Fix issue with overly long names
twangboy Jan 9, 2020
5595f67
Remove Microsoft Smart Characters with utf-8
twangboy Jan 9, 2020
e61154a
Support old element naming
twangboy Jan 9, 2020
059150e
Fix some lint
twangboy Jan 9, 2020
28b48f1
Change warn_until to Phosphorus
twangboy Jan 9, 2020
0c60cd0
Show Deprecation warning in comments
twangboy Jan 10, 2020
019e24c
Fix flakey test
twangboy Jan 10, 2020
cfcb83c
Fix comments, add tests
twangboy Jan 10, 2020
3c53578
Remove commented code
twangboy Jan 12, 2020
e015f15
Fix enum settings
twangboy Jan 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 20 additions & 29 deletions salt/modules/win_lgpo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -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))

Expand Down Expand Up @@ -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"]')
Expand Down Expand Up @@ -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] == {}:
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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 \
Expand Down
91 changes: 55 additions & 36 deletions salt/states/win_lgpo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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']):
Expand All @@ -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)
Expand All @@ -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)
Expand All @@ -320,61 +347,46 @@ 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:
ret['result'] = _ret
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,
Expand All @@ -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
4 changes: 2 additions & 2 deletions salt/version.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
2 changes: 1 addition & 1 deletion tests/integration/modules/test_win_lgpo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
},
[
Expand Down
9 changes: 3 additions & 6 deletions tests/unit/modules/test_win_lgpo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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,
Expand All @@ -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':
Expand Down
Loading