fix(bitwarden_item): honor check mode and preserve password on None

CHANGELOG.md

@@ -43,6 +43,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Fixed
 
+* **plugin:bitwarden_item**: The module no longer writes to the Bitwarden vault when run in check mode (`--check`); it reports the would-be change instead.
+* **plugin:bitwarden_item**: A run without `password` (the default `None`) no longer overwrites an existing item's password; the current password is preserved, matching the documented behavior.
 * **plugin:sqlite_query**: A failed query now fails the task instead of reporting success with the error text in `query_result`. Playbooks that relied on the previous silent success will now correctly fail.
 * **plugin:sqlite_query**: A `REGEXP` query against a column that contains NULL values no longer fails; a NULL value simply does not match.
 * **plugin:uptimerobot_\***: The modules no longer crash when the UptimeRobot API returns a non-list response for a list endpoint; the response is passed through instead.

plugins/modules/bitwarden_item.py

@@ -28,7 +28,7 @@
     - Only login items (Bitwarden type 1) are managed. Cards, secure notes and identities are out of scope.
     - TOTP secrets are not managed; the C(totp) field is set to an empty string on creation.
     - I(uris) replaces the URI list on every run. Omitting it on an existing item causes the URI list to be cleared.
-    - I(password) defaults to C(None), which translates to "do not set a password". Use the C(bitwarden_item) lookup plugin to generate one.
+    - I(password) defaults to C(None), which leaves the password unmanaged - a new item is created without one, and an existing item keeps its current password. Use the C(bitwarden_item) lookup plugin to generate one.
     - Attachments are matched by basename only; this module assumes that an attachment with the same basename has the same content. Pre-existing attachments are kept; only missing ones are uploaded.
     - Organization, collection and folder IDs can be copied from the URL in the Bitwarden web vault.
     - The cache file lives in C($XDG_RUNTIME_DIR) (falling back to C(/tmp)) and is shared across this module and the C(bitwarden_item) lookup within the same controller session.
@@ -80,7 +80,7 @@
         required: False
         type: str
     password:
-        description: Password to set on the login item. C(None) (the default) leaves the password field unset; existing passwords on already-existing items are overwritten by every non-C(None) value.
+        description: Password to set on the login item. C(None) (the default) leaves the password unmanaged - a new item gets no password and an existing item keeps its current one. Any non-C(None) value is written on every run.
         required: False
         type: str
     purpose:
@@ -367,31 +367,41 @@ def run_module():
         collection_id,
         folder_id,
     )
+
+    # A None password means "do not manage the password": preserve the existing
+    # item's password instead of overwriting it with null.
+    if password is None and current_item and current_item.get('login'):
+        target_item['login']['password'] = current_item['login'].get('password')
+
     if current_item:
         # check if changed, adjust if necessary
         changed, updated_item = diff_and_update(current_item, target_item)
-        if changed:
+        if changed and not module.check_mode:
             result = bw.edit_item(updated_item, updated_item['id'])
+        elif changed:
+            result = updated_item
         else:
             result = current_item
 
     else:
         # generate a new one
         changed = True
-        result = bw.create_item(target_item)
+        result = target_item if module.check_mode else bw.create_item(target_item)
 
     if attachments:
         current_attachments = set(current_attachment['fileName'] for current_attachment in result.get('attachments', []))
         attachments_changed = False
         for attachment in attachments:
             if os.path.basename(attachment) not in current_attachments:
                 attachments_changed = True
-                bw.add_attachment(result['id'], attachment)
+                if not module.check_mode:
+                    bw.add_attachment(result['id'], attachment)
 
         if attachments_changed:
             changed = True
             # we need to fetch the item again, so that it also contains the newly added attachments
-            result = bw.get_item_by_id(result['id'])
+            if not module.check_mode:
+                result = bw.get_item_by_id(result['id'])
 
     result['changed'] = changed
     # move username and password higher for easier access

tests/unit/plugins/modules/test_bitwarden_item.py

@@ -6,19 +6,26 @@
 #          https://www.linuxfabrik.ch/
 # License: The Unlicense, see LICENSE file.
 
-"""Unit tests for the bitwarden_item module's pure diff helper.
+"""Unit tests for the bitwarden_item module.
 
-The module itself runs via AnsiballZ, but `diff_and_update` is a plain
-function and is tested in isolation. The collection import is wired up by
+`diff_and_update` is a plain function tested in isolation. The main()
+behavior (check_mode must not write, a None password must not overwrite
+an existing one) is tested with a fake Bitwarden client and the shared
+ansible module harness. The collection import is wired up by
 tests/conftest.py.
 """
 
 from __future__ import absolute_import, division, print_function
 
 __metaclass__ = type
 
+import copy
 import unittest
+import unittest.mock
 
+import ansible_harness
+
+from ansible_collections.linuxfabrik.lfops.plugins.modules import bitwarden_item as mod
 from ansible_collections.linuxfabrik.lfops.plugins.modules.bitwarden_item import diff_and_update
 
 
@@ -62,5 +69,130 @@ def test_nested_dict_no_change(self):
         self.assertFalse(changed)
 
 
+_EXISTING_ITEM = {
+    'id': 'abc',
+    'name': 'host - db',
+    'login': {'username': 'dba', 'password': 'linuxfabrik-existing', 'totp': '', 'uris': []},
+    'notes': 'Generated by Ansible.',
+    'organizationId': None,
+    'collectionIds': None,
+    'folderId': None,
+}
+
+
+class _FakeBitwarden:
+    """Stand-in for the Bitwarden client; records writes instead of doing them."""
+
+    items = []
+    edited = []
+    created = []
+
+    def __init__(self, *args, **kwargs):
+        pass
+
+    @property
+    def is_unlocked(self):
+        return True
+
+    def sync(self, *args, **kwargs):
+        pass
+
+    def get_items(self, *args, **kwargs):
+        return [copy.deepcopy(i) for i in type(self).items]
+
+    def get_item_by_id(self, item_id):
+        return None
+
+    def get_template_item_login_uri(self, uris):
+        return list(uris or [])
+
+    def get_template_item_login(self, username=None, password=None, login_uris=None):
+        return {'username': username, 'password': password, 'totp': '', 'uris': login_uris or []}
+
+    def get_template_item(self, name, login=None, notes=None, organization_id=None,
+                          collection_ids=None, folder_id=None):
+        return {
+            'name': name, 'login': login, 'notes': notes,
+            'organizationId': organization_id, 'collectionIds': collection_ids,
+            'folderId': folder_id,
+        }
+
+    @staticmethod
+    def get_pretty_name(name, hostname=None, purpose=None):
+        return name or hostname
+
+    def edit_item(self, item, item_id):
+        type(self).edited.append((copy.deepcopy(item), item_id))
+        result = copy.deepcopy(item)
+        result['id'] = item_id
+        return result
+
+    def create_item(self, item):
+        type(self).created.append(copy.deepcopy(item))
+        result = copy.deepcopy(item)
+        result['id'] = 'new-id'
+        return result
+
+
+class TestMain(unittest.TestCase):
+
+    def setUp(self):
+        _FakeBitwarden.items = []
+        _FakeBitwarden.edited = []
+        _FakeBitwarden.created = []
+        self._patchers = [
+            unittest.mock.patch.object(mod, 'Bitwarden', _FakeBitwarden),
+            ansible_harness.patch_module(),
+        ]
+        for p in self._patchers:
+            p.start()
+
+    def tearDown(self):
+        for p in reversed(self._patchers):
+            p.stop()
+
+    def _run(self, args):
+        ansible_harness.set_module_args(args)
+        try:
+            mod.run_module()
+        except ansible_harness.AnsibleExitJson as exc:
+            return exc.args[0]
+        raise AssertionError('module did not call exit_json')
+
+    def test_check_mode_create_does_not_write(self):
+        _FakeBitwarden.items = []  # nothing exists -> would create
+        result = self._run({
+            'name': 'host - db', 'username': 'dba', 'password': 'linuxfabrik-new',
+            '_ansible_check_mode': True,
+        })
+        self.assertTrue(result['changed'])
+        self.assertEqual(_FakeBitwarden.created, [])  # no write in check mode
+
+    def test_check_mode_edit_does_not_write(self):
+        _FakeBitwarden.items = [_EXISTING_ITEM]
+        result = self._run({
+            'name': 'host - db', 'username': 'dba', 'password': 'a-different-password',
+            '_ansible_check_mode': True,
+        })
+        self.assertTrue(result['changed'])
+        self.assertEqual(_FakeBitwarden.edited, [])  # no write in check mode
+
+    def test_none_password_does_not_overwrite(self):
+        _FakeBitwarden.items = [_EXISTING_ITEM]
+        # password omitted entirely -> must preserve the existing one, no change
+        result = self._run({'name': 'host - db', 'username': 'dba'})
+        self.assertFalse(result['changed'])
+        self.assertEqual(_FakeBitwarden.edited, [])
+        self.assertEqual(result['password'], 'linuxfabrik-existing')
+
+    def test_changed_password_writes_when_not_check_mode(self):
+        _FakeBitwarden.items = [_EXISTING_ITEM]
+        result = self._run({
+            'name': 'host - db', 'username': 'dba', 'password': 'a-different-password',
+        })
+        self.assertTrue(result['changed'])
+        self.assertEqual(len(_FakeBitwarden.edited), 1)
+
+
 if __name__ == '__main__':
     unittest.main()