diff --git a/changelogs/fragments/639_fix_authorized_key.yml b/changelogs/fragments/639_fix_authorized_key.yml new file mode 100644 index 0000000000..4477ce4354 --- /dev/null +++ b/changelogs/fragments/639_fix_authorized_key.yml @@ -0,0 +1,3 @@ +--- +bugfixes: + - ansible.posix.authorized_key - fixes error on permission denied in authorized_key module (https://github.com/ansible-collections/ansible.posix/issues/462). diff --git a/plugins/modules/authorized_key.py b/plugins/modules/authorized_key.py index 1d68b4e0d2..d712a105eb 100644 --- a/plugins/modules/authorized_key.py +++ b/plugins/modules/authorized_key.py @@ -225,6 +225,8 @@ import tempfile import re import shlex +import errno +import traceback from operator import itemgetter from ansible.module_utils._text import to_native @@ -475,16 +477,18 @@ def parsekey(module, raw_key, rank=None): return (key, key_type, options, comment, rank) -def readfile(filename): - - if not os.path.isfile(filename): - return '' - - f = open(filename) +def readfile(module, filename): try: - return f.read() - finally: - f.close() + with open(filename, 'r') as f: + return f.read() + except IOError as e: + if e.errno == errno.EACCES: + module.fail_json(msg="Permission denied on file or path for authorized keys file: %s" % filename, + exception=traceback.format_exc()) + elif e.errno == errno.ENOENT: + return '' + else: + raise def parsekeys(module, lines): @@ -597,7 +601,7 @@ def enforce_state(module, params): # check current state -- just get the filename, don't create file do_write = False params["keyfile"] = keyfile(module, user, do_write, path, manage_dir) - existing_content = readfile(params["keyfile"]) + existing_content = readfile(module, params["keyfile"]) existing_keys = parsekeys(module, existing_content) # Add a place holder for keys that should exist in the state=present and diff --git a/tests/integration/targets/authorized_key/tasks/check_permissions.yml b/tests/integration/targets/authorized_key/tasks/check_permissions.yml new file mode 100644 index 0000000000..2d77059553 --- /dev/null +++ b/tests/integration/targets/authorized_key/tasks/check_permissions.yml @@ -0,0 +1,41 @@ +--- +# ------------------------------------------------------------- +# check permissions + +- name: Create a file that is not accessible + ansible.builtin.file: + state: touch + path: "{{ output_dir | expanduser }}/file_permissions" + owner: root + mode: '0000' + +- name: Create unprivileged user + ansible.builtin.user: + name: nopriv + create_home: true + +- name: Try to delete a key from an unreadable file + become: true + become_user: nopriv + ansible.posix.authorized_key: + user: root + key: "{{ dss_key_basic }}" + state: absent + path: "{{ output_dir | expanduser }}/file_permissions" + register: result + ignore_errors: true + +- name: Assert that the key deletion has failed + ansible.builtin.assert: + that: + - result is failed + +- name: Remove the file + ansible.builtin.file: + state: absent + path: "{{ output_dir | expanduser }}/file_permissions" + +- name: Remove the user + ansible.builtin.user: + name: nopriv + state: absent diff --git a/tests/integration/targets/authorized_key/tasks/main.yml b/tests/integration/targets/authorized_key/tasks/main.yml index d687f17b59..761b6d5681 100644 --- a/tests/integration/targets/authorized_key/tasks/main.yml +++ b/tests/integration/targets/authorized_key/tasks/main.yml @@ -34,3 +34,6 @@ - name: Test for specifying key as a path ansible.builtin.import_tasks: check_path.yml + +- name: Test for permission denied files + ansible.builtin.import_tasks: check_permissions.yml