NoLogPasswordsRule: catch password_lock false positives and migrate t…

…ests (#1562)

* catch password_lock false positives and migrate tests

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

* fix codespell issues

Signed-off-by: Thomas Sjögren <konstruktoid@users.noreply.github.com>

src/ansiblelint/rules/NoLogPasswordsRule.py

@@ -12,6 +12,8 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+"""NoLogPasswordsRule used with ansible-lint."""
+import sys
 from typing import TYPE_CHECKING, Any, Dict, Union
 
 from ansiblelint.rules import AnsibleLintRule
@@ -24,6 +26,8 @@
 
 
 class NoLogPasswordsRule(AnsibleLintRule):
+    """Describe and test the NoLogPasswordsRule."""
+
     id = "no-log-password"
     shortdesc = "password should not be logged."
     description = (
@@ -38,14 +42,173 @@ def matchtask(
         self, task: Dict[str, Any], file: 'Optional[Lintable]' = None
     ) -> Union[bool, str]:
 
-        for param in task["action"].keys():
-            if 'password' in param:
-                has_password = True
-                break
-        else:
+        if task["action"]["__ansible_module__"] == 'user' and (
+            (
+                task['action'].get('password_lock')
+                or task['action'].get('password_lock') is False
+            )
+            and not task['action'].get('password')
+        ):
             has_password = False
+        else:
+            for param in task["action"].keys():
+                if 'password' in param:
+                    has_password = True
+                    break
+            else:
+                has_password = False
 
         # No no_log and no_log: False behave the same way
         # and should return a failure (return True), so we
         # need to invert the boolean
         return bool(has_password and not convert_to_boolean(task.get('no_log', False)))
+
+
+if "pytest" in sys.modules:
+    import pytest
+
+    NO_LOG_UNUSED = '''
+- hosts: all
+  tasks:
+    - name: Fail when no_log is not used
+      user:
+        name: bidule
+        password: "wow"
+        state: absent
+'''
+
+    NO_LOG_FALSE = '''
+- hosts: all
+  tasks:
+    - name: Fail when no_log is set to False
+      user:
+        name: bidule
+        user_password: "wow"
+        state: absent
+      no_log: False
+'''
+
+    NO_LOG_NO = '''
+- hosts: all
+  tasks:
+    - name: Fail when no_log is set to no
+      user:
+        name: bidule
+        password: "wow"
+        state: absent
+      no_log: no
+'''
+
+    PASSWORD_WITH_LOCK = '''
+- hosts: all
+  tasks:
+    - name: Fail when password is set and password_lock is true
+      user:
+        name: lint
+        password: "wow"
+        password_lock: true
+'''
+
+    NO_LOG_YES = '''
+- hosts: all
+  tasks:
+    - name: Succeed when no_log is set to yes
+      user:
+        name: bidule
+        password: "wow"
+        state: absent
+      no_log: yes
+'''
+
+    NO_LOG_TRUE = '''
+- hosts: all
+  tasks:
+    - name: Succeed when no_log is set to True
+      user:
+        name: bidule
+        user_password: "wow"
+        state: absent
+      no_log: True
+'''
+
+    PASSWORD_LOCK_YES = '''
+- hosts: all
+  tasks:
+    - name: Succeed when only password locking account
+      user:
+        name: lint
+        password_lock: yes
+'''
+
+    PASSWORD_LOCK_FALSE = '''
+- hosts: all
+  tasks:
+    - name: Succeed when password_lock is false and password is not used
+      user:
+        name: lint
+        password_lock: False
+'''
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_no_log_unused(rule_runner: Any) -> None:
+        """The task does not use no_log."""
+        results = rule_runner.run_playbook(NO_LOG_UNUSED)
+        assert len(results) == 1
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_no_log_false(rule_runner: Any) -> None:
+        """The task sets no_log to false."""
+        results = rule_runner.run_playbook(NO_LOG_FALSE)
+        assert len(results) == 1
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_no_log_no(rule_runner: Any) -> None:
+        """The task sets no_log to no."""
+        results = rule_runner.run_playbook(NO_LOG_NO)
+        assert len(results) == 1
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_password_with_lock(rule_runner: Any) -> None:
+        """The task sets a password but also lock the user."""
+        results = rule_runner.run_playbook(PASSWORD_WITH_LOCK)
+        assert len(results) == 1
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_no_log_yes(rule_runner: Any) -> None:
+        """The task sets no_log to yes."""
+        results = rule_runner.run_playbook(NO_LOG_YES)
+        assert len(results) == 0
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_no_log_true(rule_runner: Any) -> None:
+        """The task sets no_log to true."""
+        results = rule_runner.run_playbook(NO_LOG_TRUE)
+        assert len(results) == 0
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_password_lock_yes(rule_runner: Any) -> None:
+        """The task only locks the user."""
+        results = rule_runner.run_playbook(PASSWORD_LOCK_YES)
+        assert len(results) == 0
+
+    @pytest.mark.parametrize(
+        'rule_runner', (NoLogPasswordsRule,), indirect=['rule_runner']
+    )
+    def test_password_lock_false(rule_runner: Any) -> None:
+        """The task does not actually lock the user."""
+        results = rule_runner.run_playbook(PASSWORD_LOCK_FALSE)
+        assert len(results) == 0