docs: Extend password entry of ansible.builtin.user #79694

Hofer-Julian · 2023-01-09T10:14:46Z

SUMMARY

Clarify that password sets the password hash and not the actual password.
Fixes part of #79684

ISSUE TYPE

Docs Pull Request

COMPONENT NAME

ansible.builtin.user

Clarify that `password` sets the password hash. Not the actual password. Fixes part of ansible#79684

Hofer-Julian · 2023-01-09T10:15:16Z

@samccann

ansibot · 2023-01-09T10:20:52Z

Thanks for your Ansible docs contribution! We talk about Ansible documentation on matrix at #docs:ansible.im and on libera IRC at #ansible-docs if you ever want to join us and chat about the docs! We meet there on Tuesdays (see the Ansible calendar) and welcome additions to our weekly agenda items - scroll down to find the upcoming agenda and add a comment to put something new on that agenda.

click here for bot help

sivel · 2023-01-09T15:41:42Z

lib/ansible/modules/user.py

@@ -86,12 +86,13 @@
        version_added: "2.0"
    password:
        description:
-            - Optionally set the user's password to this encrypted value.
+            - Optionally set the user's password hash to this value.
+            - Do B(not) put the actual password into this field.


This may cause confusion with the statement lower, that says that you are supposed to do this for macOS. So perhaps this statement and the macOS statement should be combined.

I was confused by the macOS sentence too.
So on macOS the password field expects the actual password, on Linux the hash?
What is expected on Windows?

to use win_user instead

I gave the phrasing another try.

Regarding the differing behavior of Linux and macOS: I wonder if I should open an issue to discuss this behavior. Using the actual password on one platform and the hash on another one sounds like a foot gun to me.

oraNod

Adding some suggestions to try and clarify the choices a little more, if I understand this correctly.

I would prefer more authoritative language that instructs users to enter exactly what each OS requires. That might get rid of some of the ambiguity/confusion. But it does seem concerning that macOS doesn't take a hashed password.

lib/ansible/modules/user.py

Co-authored-by: Don Naro <dnaro@redhat.com>

bcoca · 2023-01-10T15:47:07Z

lib/ansible/modules/user.py

@@ -86,12 +86,13 @@
        version_added: "2.0"
    password:
        description:
-            - Optionally set the user's password to this encrypted value.
-            - On macOS systems, this value has to be cleartext. Beware of security issues.
+            - Optionally set the user password.


If provided, set the user's password to the provided encrypted hash (linux/unix/POSIX) or plain text password (OS X/Mac OS)

Isn't that redundant with the text below?

Also, I don't think that Linux is "more Unix" than macOS :)

why its in list of linux/unix/posix otherwise it woudl be implied under unix

and yes, it makes other lines redundant, but i think its simple, concise and clear in one sentence and will avoid people 'trailing off' and not reading a wall of text.

samccann · 2023-01-26T19:03:13Z

Hi @Hofer-Julian - have you had a chance to consider the feedback above? Thanks for your help!

Hofer-Julian · 2023-01-26T19:23:50Z

Hi @Hofer-Julian - have you had a chance to consider the feedback above? Thanks for your help!

Thanks for the reminder, I've adapted the sentence.

samccann · 2023-01-26T21:01:45Z

LGTM but let's see if @bcoca has any final comments before we merge.

lib/ansible/modules/user.py

* docs: Extend password entry of ansible.builtin.user Clarify that `password` sets the password hash. Not the actual password. Fixes part of ansible#79684 (cherry picked from commit 6cd1a14)

* fix filename for sidecar docs (#79779) (cherry picked from commit f2707d1) * correct examples to use removed_from_collection not collection_name (#79803) (cherry picked from commit 7ab3de7) * Fix: documentation for per-task timeout (#79715) (cherry picked from commit 48e6bf8) * [Docs] maintainers_guidelines: add WG and real-time chat request info (#79750) (cherry picked from commit 6cb6d65) * doc fix for platform content #79794 (#79801) (cherry picked from commit d7a4152) * Expand docs for the import sanity test. (#79768) * Expand docs for the import sanity test. * Remove note about Python 2.7 compat. It should not be needed since there is a sanity test to enforce use of `__metaclass__ = type`. * Improve introductory paragraph. * Fix link typo. (cherry picked from commit 2164d56) * docs: Extend password entry of ansible.builtin.user (#79694) * docs: Extend password entry of ansible.builtin.user Clarify that `password` sets the password hash. Not the actual password. Fixes part of #79684 (cherry picked from commit 6cd1a14) * Update dev_guide.rst (#79625) (cherry picked from commit 65eb5c0) * Improve documentation on requirements.yml (#76140) Makes it clear that user can use range identifiers with collection versions inside requirements.yml files. (cherry picked from commit 44dcfde) --------- Co-authored-by: Evgeni Golov <evgeni@golov.de> Co-authored-by: Jo <jo@swagspace.org> Co-authored-by: Andrew Klychkov <aaklychkov@mail.ru> Co-authored-by: prasadpatil49 <51715670+prasadpatil49@users.noreply.github.com> Co-authored-by: Matt Clay <matt@mystile.com> Co-authored-by: Hofer-Julian <30049909+Hofer-Julian@users.noreply.github.com> Co-authored-by: Jens Timmerman <github@caret.be> Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>

docs: Extend password entry of ansible.builtin.user

b91b2eb

Clarify that `password` sets the password hash. Not the actual password. Fixes part of ansible#79684

sivel reviewed Jan 9, 2023

View reviewed changes

Update according to suggestions

4108628

oraNod reviewed Jan 10, 2023

View reviewed changes

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

Hofer-Julian and others added 3 commits January 10, 2023 13:58

Update lib/ansible/modules/user.py

f0a5c00

Co-authored-by: Don Naro <dnaro@redhat.com>

Update lib/ansible/modules/user.py

1950167

Co-authored-by: Don Naro <dnaro@redhat.com>

Update lib/ansible/modules/user.py

03313d8

Co-authored-by: Don Naro <dnaro@redhat.com>

mkrizek removed the needs_triage Needs a first human triage before being processed. label Jan 10, 2023

bcoca reviewed Jan 10, 2023

View reviewed changes

ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. and removed new_contributor This PR is the first contribution by a new community member. labels Jan 18, 2023

Address review comment

8c904c7

ansibot removed the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Jan 26, 2023

samccann approved these changes Jan 26, 2023

View reviewed changes

bcoca reviewed Jan 26, 2023

View reviewed changes

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

samccann reviewed Jan 26, 2023

View reviewed changes

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

lib/ansible/modules/user.py Outdated Show resolved Hide resolved

Apply suggestions from code review

832542d

samccann approved these changes Jan 26, 2023

View reviewed changes

samccann merged commit 6cd1a14 into ansible:devel Jan 26, 2023

Hofer-Julian deleted the password_docs branch January 26, 2023 21:34

samccann mentioned this pull request Jan 26, 2023

Docs Backportapalooza 01 26 #79828

Merged

ansible locked and limited conversation to collaborators Feb 2, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs: Extend password entry of ansible.builtin.user #79694

docs: Extend password entry of ansible.builtin.user #79694

Hofer-Julian commented Jan 9, 2023

Hofer-Julian commented Jan 9, 2023

ansibot commented Jan 9, 2023

sivel Jan 9, 2023

Hofer-Julian Jan 9, 2023

bcoca Jan 9, 2023

Hofer-Julian Jan 9, 2023

oraNod left a comment

bcoca Jan 10, 2023

Hofer-Julian Jan 10, 2023

bcoca Jan 10, 2023

bcoca Jan 10, 2023

samccann commented Jan 26, 2023

Hofer-Julian commented Jan 26, 2023

samccann commented Jan 26, 2023

docs: Extend password entry of ansible.builtin.user #79694

docs: Extend password entry of ansible.builtin.user #79694

Conversation

Hofer-Julian commented Jan 9, 2023

SUMMARY

ISSUE TYPE

COMPONENT NAME

Hofer-Julian commented Jan 9, 2023

ansibot commented Jan 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oraNod left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samccann commented Jan 26, 2023

Hofer-Julian commented Jan 26, 2023

samccann commented Jan 26, 2023