From 413ab782a811173c4dbd4ea83c1250e32bf53a21 Mon Sep 17 00:00:00 2001 From: Klaas Demter Date: Fri, 16 May 2025 16:39:51 +0200 Subject: [PATCH] Fixes #462 notice permission denied on authorized_key module --- .../fragments/639_fix_authorized_key.yml | 3 ++ plugins/modules/authorized_key.py | 24 ++++++----- .../tasks/check_permissions.yml | 41 +++++++++++++++++++ .../targets/authorized_key/tasks/main.yml | 3 ++ 4 files changed, 61 insertions(+), 10 deletions(-) create mode 100644 changelogs/fragments/639_fix_authorized_key.yml create mode 100644 tests/integration/targets/authorized_key/tasks/check_permissions.yml diff --git a/changelogs/fragments/639_fix_authorized_key.yml b/changelogs/fragments/639_fix_authorized_key.yml new file mode 100644 index 0000000..4477ce4 --- /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 1d68b4e..d712a10 100644 --- a/plugins/modules/authorized_key.py +++ b/plugins/modules/authorized_key.py @@ -225,6 +225,8 @@ import os.path 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 0000000..0b64ef2 --- /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.failed == True + +- 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 d687f17..761b6d5 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