1
0
Fork 0
mirror of https://github.com/ansible-collections/community.general.git synced 2024-09-14 20:13:21 +02:00

pamd - fixed issue+minor refactorings (#3285) (#3308)

* pamd - fixed issue+minor refactorings

* added changelog fragment

* added unit test suggested in issue

* Update tests/integration/targets/pamd/tasks/main.yml

* fixed per PR + additional adjustment

Co-authored-by: Felix Fontein <felix@fontein.de>
(cherry picked from commit edd7b84285)

Co-authored-by: Alexei Znamensky <103110+russoz@users.noreply.github.com>
This commit is contained in:
patchback[bot] 2021-08-31 12:59:29 +02:00 committed by GitHub
parent bea5a6266c
commit c64ab70c75
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 59 additions and 30 deletions

View file

@ -0,0 +1,4 @@
bugfixes:
- pamd - code for ``state=updated`` when dealing with the pam module arguments, made no distinction between ``None`` and an empty list (https://github.com/ansible-collections/community.general/issues/3260).
minor_changes:
- pamd - minor refactorings (https://github.com/ansible-collections/community.general/pull/3285).

View file

@ -274,8 +274,7 @@ RULE_REGEX = re.compile(r"""(?P<rule_type>-?(?:auth|account|session|password))\s
(?P<control>\[.*\]|\S*)\s+
(?P<path>\S*)\s*
(?P<args>.*)\s*""", re.X)
RULE_ARG_REGEX = re.compile(r"""(\[.*\]|\S*)""")
RULE_ARG_REGEX = re.compile(r"(\[.*\]|\S*)")
VALID_TYPES = ['account', '-account', 'auth', '-auth', 'password', '-password', 'session', '-session']
@ -358,11 +357,9 @@ class PamdRule(PamdLine):
# Method to check if a rule matches the type, control and path.
def matches(self, rule_type, rule_control, rule_path, rule_args=None):
if (rule_type == self.rule_type and
return (rule_type == self.rule_type and
rule_control == self.rule_control and
rule_path == self.rule_path):
return True
return False
rule_path == self.rule_path)
@classmethod
def rule_from_string(cls, line):
@ -507,25 +504,25 @@ class PamdService(object):
# Get a list of rules we want to change
rules_to_find = self.get(rule_type, rule_control, rule_path)
new_args = parse_module_arguments(new_args)
new_args = parse_module_arguments(new_args, return_none=True)
changes = 0
for current_rule in rules_to_find:
rule_changed = False
if new_type:
if(current_rule.rule_type != new_type):
if current_rule.rule_type != new_type:
rule_changed = True
current_rule.rule_type = new_type
if new_control:
if(current_rule.rule_control != new_control):
if current_rule.rule_control != new_control:
rule_changed = True
current_rule.rule_control = new_control
if new_path:
if(current_rule.rule_path != new_path):
if current_rule.rule_path != new_path:
rule_changed = True
current_rule.rule_path = new_path
if new_args:
if(current_rule.rule_args != new_args):
if new_args is not None:
if current_rule.rule_args != new_args:
rule_changed = True
current_rule.rule_args = new_args
@ -724,8 +721,9 @@ class PamdService(object):
current_line = self._head
while current_line is not None:
if not current_line.validate()[0]:
return current_line.validate()
curr_validate = current_line.validate()
if not curr_validate[0]:
return curr_validate
current_line = current_line.next
return True, "Module is valid"
@ -750,22 +748,25 @@ class PamdService(object):
return '\n'.join(lines) + '\n'
def parse_module_arguments(module_arguments):
# Return empty list if we have no args to parse
if not module_arguments:
return []
elif isinstance(module_arguments, list) and len(module_arguments) == 1 and not module_arguments[0]:
def parse_module_arguments(module_arguments, return_none=False):
# If args is None, return empty list by default.
# But if return_none is True, then return None
if module_arguments is None:
return None if return_none else []
if isinstance(module_arguments, list) and len(module_arguments) == 1 and not module_arguments[0]:
return []
if not isinstance(module_arguments, list):
module_arguments = [module_arguments]
parsed_args = list()
# From this point on, module_arguments is guaranteed to be a list, empty or not
parsed_args = []
re_clear_spaces = re.compile(r"\s*=\s*")
for arg in module_arguments:
for item in filter(None, RULE_ARG_REGEX.findall(arg)):
if not item.startswith("["):
re.sub("\\s*=\\s*", "=", item)
re_clear_spaces.sub("=", item)
parsed_args.append(item)
return parsed_args
@ -861,8 +862,7 @@ def main():
fd.write(str(service))
except IOError:
module.fail_json(msg='Unable to create temporary \
file %s' % temp_file)
module.fail_json(msg='Unable to create temporary file %s' % temp_file)
module.atomic_move(temp_file.name, os.path.realpath(fname))

View file

@ -5,11 +5,10 @@
set_fact:
test_pamd_file: "/tmp/pamd_file"
- name: Copy temporary pam.d file
- name: Create temporary pam.d file
copy:
content: "session required pam_lastlog.so silent showfailed"
dest: "{{ test_pamd_file }}"
- name: Test working on a single-line file works (2925)
community.general.pamd:
path: /tmp
@ -20,17 +19,37 @@
module_arguments: silent
state: args_absent
register: pamd_file_output
- name: Check if changes made
assert:
that:
- pamd_file_output is changed
- name: Copy temporary pam.d file
- name: Test removing all arguments from an entry (3260)
community.general.pamd:
path: /tmp
name: pamd_file
type: session
control: required
module_path: pam_lastlog.so
module_arguments: ""
state: updated
register: pamd_file_output_noargs
- name: Read back the file (3260)
slurp:
src: "{{ test_pamd_file }}"
register: pamd_file_slurp_noargs
- name: Check if changes made (3260)
vars:
line_array: "{{ (pamd_file_slurp_noargs.content|b64decode).split('\n')[2].split() }}"
assert:
that:
- pamd_file_output_noargs is changed
- line_array == ['session', 'required', 'pam_lastlog.so']
- name: Create temporary pam.d file
copy:
content: ""
dest: "{{ test_pamd_file }}"
# This test merely demonstrates that, as-is, module will not perform any changes on an empty file
# All the existing values for "state" will first search for a rule matching type, control, module_path
# and will not perform any change whatsoever if no existing rules match.
@ -43,12 +62,10 @@
module_path: pam_lastlog.so
module_arguments: silent
register: pamd_file_output_empty
- name: Read back the file
slurp:
src: "{{ test_pamd_file }}"
register: pamd_file_slurp
- name: Check if changes made
assert:
that:

View file

@ -218,6 +218,14 @@ auth required pam_deny.so
test_rule = PamdRule('auth', 'sufficient', 'pam_unix.so', 'nullok try_first_pass')
self.assertNotIn(str(test_rule), str(self.pamd))
def test_update_rule_remove_module_args(self):
self.assertTrue(self.pamd.update_rule('auth', 'sufficient', 'pam_unix.so', new_args=''))
test_rule = PamdRule('auth', 'sufficient', 'pam_unix.so', '')
self.assertIn(str(test_rule), str(self.pamd))
test_rule = PamdRule('auth', 'sufficient', 'pam_unix.so', 'nullok try_first_pass')
self.assertNotIn(str(test_rule), str(self.pamd))
def test_update_first_three(self):
self.assertTrue(self.pamd.update_rule('auth', 'required', 'pam_env.so',
new_type='one', new_control='two', new_path='three'))