ACME: add diff to acme_account, account_public_key to acme_account_facts, and general refactoring (#49410)

* Only one exit point.

* Refactoring account handling.

* Add diff support for acme_account.

* Insert public_account_key into acme_account_facts result and into acme_account diff.

* Add changelog.

changelogs/fragments/49410-acme-diff.yml

@@ -0,0 +1,3 @@
+minor_changes:
+- "acme_account: add support for diff mode."
+- "acme_account_facts: also return ``public_account_key`` in JWK format."

lib/ansible/module_utils/acme.py

@@ -646,12 +646,14 @@ class ACMEAccount(object):
 
     def _new_reg(self, contact=None, agreement=None, terms_agreed=False, allow_creation=True):
         '''
-        Registers a new ACME account. Returns True if the account was
-        created and False if it already existed (e.g. it was not newly
-        created).
+        Registers a new ACME account. Returns a pair ``(created, data)``.
+        Here, ``created`` is ``True`` if the account was created and
+        ``False`` if it already existed (e.g. it was not newly created),
+        or does not exist. In case the account was created or exists,
+        ``data`` contains the account data; otherwise, it is ``None``.
         https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-7.3
         '''
-        contact = [] if contact is None else contact
+        contact = contact or []
 
         if self.version == 1:
             new_reg = {
@@ -668,6 +670,7 @@ class ACMEAccount(object):
                 'contact': contact
             }
             if not allow_creation:
+                # https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-7.3.1
                 new_reg['onlyReturnExisting'] = True
             if terms_agreed:
                 new_reg['termsOfServiceAgreed'] = True
@@ -679,7 +682,7 @@ class ACMEAccount(object):
             # Account did not exist
             if 'location' in info:
                 self.set_account_uri(info['location'])
-            return True
+            return True, result
         elif info['status'] == (409 if self.version == 1 else 200):
             # Account did exist
             if result.get('status') == 'deactivated':
@@ -689,22 +692,22 @@ class ACMEAccount(object):
                 #     "Once an account is deactivated, the server MUST NOT accept further
                 #      requests authorized by that account's key."
                 if not allow_creation:
-                    return False
+                    return False, None
                 else:
                     raise ModuleFailException("Account is deactivated")
             if 'location' in info:
                 self.set_account_uri(info['location'])
-            return False
+            return False, result
         elif info['status'] == 400 and result['type'] == 'urn:ietf:params:acme:error:accountDoesNotExist' and not allow_creation:
             # Account does not exist (and we didn't try to create it)
-            return False
+            return False, None
         else:
             raise ModuleFailException("Error registering: {0} {1}".format(info['status'], result))
 
     def get_account_data(self):
         '''
         Retrieve account information. Can only be called when the account
-        URI is already known (such as after calling init_account).
+        URI is already known (such as after calling setup_account).
         Return None if the account was deactivated, or a dict otherwise.
         '''
         if self.uri is None:
@@ -732,66 +735,82 @@ class ACMEAccount(object):
             raise ModuleFailException("Error getting account data from {2}: {0} {1}".format(info['status'], result, self.uri))
         return result
 
-    def init_account(self, contact, agreement=None, terms_agreed=False, allow_creation=True, update_contact=True, remove_account_uri_if_not_exists=False):
+    def setup_account(self, contact=None, agreement=None, terms_agreed=False, allow_creation=True, remove_account_uri_if_not_exists=False):
         '''
-        Create or update an account on the ACME server. For ACME v1,
+        Detect or create an account on the ACME server. For ACME v1,
         as the only way (without knowing an account URI) to test if an
         account exists is to try and create one with the provided account
         key, this method will always result in an account being present
         (except on error situations). For ACME v2, a new account will
-        only be created if allow_creation is set to True.
+        only be created if ``allow_creation`` is set to True.
 
-        For ACME v2, check_mode is fully respected. For ACME v1, the account
-        might be created if it does not yet exist.
+        For ACME v2, ``check_mode`` is fully respected. For ACME v1, the
+        account might be created if it does not yet exist.
 
-        If the account already exists and if update_contact is set to
-        True, this method will update the contact information.
+        Return a pair ``(created, account_data)``. Here, ``created`` will
+        be ``True`` in case the account was created or would be created
+        (check mode). ``account_data`` will be the current account data,
+        or ``None`` if the account does not exist.
 
-        Return True in case something changed (account was created, contact
-        info updated) or would be changed (check_mode). The account URI
-        will be stored in self.uri; if it is None, the account does not
-        exist.
+        The account URI will be stored in ``self.uri``; if it is ``None``,
+        the account does not exist.
 
         https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-7.3
         '''
 
-        new_account = True
-        changed = False
         if self.uri is not None:
-            new_account = False
-            if not update_contact:
-                # Verify that the account key belongs to the URI.
-                # (If update_contact is True, this will be done below.)
-                if self.get_account_data() is None:
-                    if remove_account_uri_if_not_exists and not allow_creation:
-                        self.uri = None
-                        return False
+            created = False
+            # Verify that the account key belongs to the URI.
+            # (If update_contact is True, this will be done below.)
+            account_data = self.get_account_data()
+            if account_data is None:
+                if remove_account_uri_if_not_exists and not allow_creation:
+                    self.uri = None
+                else:
                     raise ModuleFailException("Account is deactivated or does not exist!")
         else:
-            new_account = self._new_reg(
+            created, account_data = self._new_reg(
                 contact,
                 agreement=agreement,
                 terms_agreed=terms_agreed,
                 allow_creation=allow_creation and not self.module.check_mode
             )
             if self.module.check_mode and self.uri is None and allow_creation:
-                return True
-        if not new_account and self.uri and update_contact:
-            result = self.get_account_data()
-            if result is None:
-                if not allow_creation:
-                    self.uri = None
-                    return False
-                raise ModuleFailException("Account is deactivated or does not exist!")
+                created = True
+                account_data = {
+                    'contact': contact or []
+                }
+        return created, account_data
 
-            # ...and check if update is necessary
-            if result.get('contact', []) != contact:
-                if not self.module.check_mode:
-                    upd_reg = result
-                    upd_reg['contact'] = contact
-                    result, dummy = self.send_signed_request(self.uri, upd_reg)
-                changed = True
-        return new_account or changed
+    def update_account(self, account_data, contact=None):
+        '''
+        Update an account on the ACME server. Check mode is fully respected.
+
+        The current account data must be provided as ``account_data``.
+
+        Return a pair ``(updated, account_data)``, where ``updated`` is
+        ``True`` in case something changed (contact info updated) or
+        would be changed (check mode), and ``account_data`` the updated
+        account data.
+
+        https://tools.ietf.org/html/draft-ietf-acme-acme-14#section-7.3.2
+        '''
+        # Create request
+        update_request = {}
+        if contact is not None and account_data.get('contact', []) != contact:
+            update_request['contact'] = list(contact)
+
+        # No change?
+        if not update_request:
+            return False, dict(account_data)
+
+        # Apply change
+        if self.module.check_mode:
+            account_data = dict(account_data)
+            account_data.update(update_request)
+        else:
+            account_data, dummy = self.send_signed_request(self.uri, update_request)
+        return True, account_data
 
 
 def cryptography_get_csr_domains(module, csr_filename):

lib/ansible/modules/crypto/acme/acme_account.py

@@ -166,43 +166,51 @@ def main():
 
     try:
         account = ACMEAccount(module)
+        changed = False
         state = module.params.get('state')
+        diff_before = {}
+        diff_after = {}
         if state == 'absent':
-            changed = account.init_account(
-                [],
-                allow_creation=False,
-                update_contact=False,
-            )
-            if changed:
-                raise AssertionError('Unwanted account change')
-            if account.uri is not None:
-                # Account does exist
-                account_data = account.get_account_data()
-                if account_data is not None:
-                    # Account is not yet deactivated
-                    if not module.check_mode:
-                        # Deactivate it
-                        payload = {
-                            'status': 'deactivated'
-                        }
-                        result, info = account.send_signed_request(account.uri, payload)
-                        if info['status'] != 200:
-                            raise ModuleFailException('Error deactivating account: {0} {1}'.format(info['status'], result))
-                    module.exit_json(changed=True, account_uri=account.uri)
-            module.exit_json(changed=False, account_uri=account.uri)
+            created, account_data = account.setup_account(allow_creation=False)
+            if account_data:
+                diff_before = dict(account_data)
+                diff_before['public_account_key'] = account.key_data['jwk']
+            if created:
+                raise AssertionError('Unwanted account creation')
+            if account_data is not None:
+                # Account is not yet deactivated
+                if not module.check_mode:
+                    # Deactivate it
+                    payload = {
+                        'status': 'deactivated'
+                    }
+                    result, info = account.send_signed_request(account.uri, payload)
+                    if info['status'] != 200:
+                        raise ModuleFailException('Error deactivating account: {0} {1}'.format(info['status'], result))
+                changed = True
         elif state == 'present':
             allow_creation = module.params.get('allow_creation')
             # Make sure contact is a list of strings (unfortunately, Ansible doesn't do that for us)
             contact = [str(v) for v in module.params.get('contact')]
             terms_agreed = module.params.get('terms_agreed')
-            changed = account.init_account(
+            created, account_data = account.setup_account(
                 contact,
                 terms_agreed=terms_agreed,
                 allow_creation=allow_creation,
             )
-            if account.uri is None:
+            if account_data is None:
                 raise ModuleFailException(msg='Account does not exist or is deactivated.')
-            module.exit_json(changed=changed, account_uri=account.uri)
+            if created:
+                diff_before = {}
+            else:
+                diff_before = dict(account_data)
+                diff_before['public_account_key'] = account.key_data['jwk']
+            updated = False
+            if not created:
+                updated, account_data = account.update_account(account_data, contact)
+            changed = created or updated
+            diff_after = dict(account_data)
+            diff_after['public_account_key'] = account.key_data['jwk']
         elif state == 'changed_key':
             # Parse new account key
             error, new_key_data = account.parse_key(
@@ -212,15 +220,13 @@ def main():
             if error:
                 raise ModuleFailException("error while parsing account key: %s" % error)
             # Verify that the account exists and has not been deactivated
-            changed = account.init_account(
-                [],
-                allow_creation=False,
-                update_contact=False,
-            )
-            if changed:
-                raise AssertionError('Unwanted account change')
-            if account.uri is None or account.get_account_data() is None:
+            created, account_data = account.setup_account(allow_creation=False)
+            if created:
+                raise AssertionError('Unwanted account creation')
+            if account_data is None:
                 raise ModuleFailException(msg='Account does not exist or is deactivated.')
+            diff_before = dict(account_data)
+            diff_before['public_account_key'] = account.key_data['jwk']
             # Now we can start the account key rollover
             if not module.check_mode:
                 # Compose inner signed message
@@ -241,7 +247,25 @@ def main():
                 result, info = account.send_signed_request(url, data)
                 if info['status'] != 200:
                     raise ModuleFailException('Error account key rollover: {0} {1}'.format(info['status'], result))
-            module.exit_json(changed=True, account_uri=account.uri)
+                if module._diff:
+                    account.key_data = new_key_data
+                    account.jws_header['alg'] = new_key_data['alg']
+                    diff_after = account.get_account_data()
+            elif module._diff:
+                # Kind of fake diff_after
+                diff_after = dict(diff_before)
+            diff_after['public_account_key'] = new_key_data['jwk']
+            changed = True
+        result = {
+            'changed': changed,
+            'account_uri': account.uri,
+        }
+        if module._diff:
+            result['diff'] = {
+                'before': diff_before,
+                'after': diff_after,
+            }
+        module.exit_json(**result)
     except ModuleFailException as e:
         e.do_fail(module)
 

lib/ansible/modules/crypto/acme/acme_account_facts.py

@@ -89,6 +89,11 @@ account:
       returned: always
       type: str
       sample: https://example.ca/account/1/orders
+    public_account_key:
+      description: the public account key as a L(JSON Web Key,https://tools.ietf.org/html/rfc7517).
+      returned: always
+      type: str
+      sample: https://example.ca/account/1/orders
 '''
 
 from ansible.module_utils.acme import (
@@ -129,24 +134,25 @@ def main():
     try:
         account = ACMEAccount(module)
         # Check whether account exists
-        changed = account.init_account(
+        created, account_data = account.setup_account(
             [],
             allow_creation=False,
-            update_contact=False,
             remove_account_uri_if_not_exists=True,
         )
-        if changed:
-            raise AssertionError('Unwanted account change')
-        if account.uri is None:
-            # Account does exist
-            module.exit_json(changed=False, exists=False, account_uri=None)
-        else:
-            # Account exists: retrieve account information
-            data = account.get_account_data()
+        if created:
+            raise AssertionError('Unwanted account creation')
+        result = {
+            'changed': False,
+            'exists': account.uri is not None,
+            'account_uri': account.uri,
+        }
+        if account.uri is not None:
             # Make sure promised data is there
-            if 'contact' not in data:
-                data['contact'] = []
-            module.exit_json(changed=False, exists=True, account_uri=account.uri, account=data)
+            if 'contact' not in account_data:
+                account_data['contact'] = []
+            account_data['public_account_key'] = account.key_data['jwk']
+            result['account'] = account_data
+        module.exit_json(**result)
     except ModuleFailException as e:
         e.do_fail(module)
 

lib/ansible/modules/crypto/acme/acme_certificate.py

@@ -406,16 +406,21 @@ class ACMEClient(object):
             contact = []
             if module.params['account_email']:
                 contact.append('mailto:' + module.params['account_email'])
-            self.changed = self.account.init_account(
+            created, account_data = self.account.setup_account(
                 contact,
                 agreement=module.params.get('agreement'),
                 terms_agreed=module.params.get('terms_agreed'),
                 allow_creation=modify_account,
-                update_contact=modify_account
             )
+            if account_data is None:
+                raise ModuleFailException(msg='Account does not exist or is deactivated.')
+            updated = False
+            if not created and account_data and modify_account:
+                updated, account_data = self.account.update_account(account_data, contact)
+            self.changed = created or updated
         else:
             # This happens if modify_account is False and the ACME v1
-            # protocol is used. In this case, we do not call init_account()
+            # protocol is used. In this case, we do not call setup_account()
             # to avoid accidental creation of an account. This is OK
             # since for ACME v1, the account URI is not needed to send a
             # signed ACME request.

lib/ansible/modules/crypto/acme/acme_certificate_revoke.py

@@ -177,13 +177,11 @@ def main():
             result, info = account.send_signed_request(endpoint, payload, key_data=private_key_data, jws_header=jws_header)
         else:
             # Step 1: get hold of account URI
-            changed = account.init_account(
-                [],
-                allow_creation=False,
-                update_contact=False,
-            )
-            if changed:
-                raise AssertionError('Unwanted account change')
+            created, account_data = account.setup_account(allow_creation=False)
+            if created:
+                raise AssertionError('Unwanted account creation')
+            if account_data is None:
+                raise ModuleFailException(msg='Account does not exist or is deactivated.')
             # Step 2: sign revokation request with account key
             result, info = account.send_signed_request(endpoint, payload)
         if info['status'] != 200:

test/integration/targets/acme_account/tasks/impl.yml

@@ -16,6 +16,22 @@
   ignore_errors: yes
   register: account_not_created
 
+- name: Create it now (check mode, diff)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_src: "{{ output_dir }}/accountkey.pem"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    state: present
+    allow_creation: yes
+    terms_agreed: yes
+    contact:
+    - mailto:example@example.org
+  check_mode: yes
+  diff: yes
+  register: account_created_check
+
 - name: Create it now
   acme_account:
     select_crypto_backend: "{{ select_crypto_backend }}"
@@ -30,6 +46,35 @@
     - mailto:example@example.org
   register: account_created
 
+- name: Create it now (idempotent)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_src: "{{ output_dir }}/accountkey.pem"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    state: present
+    allow_creation: yes
+    terms_agreed: yes
+    contact:
+    - mailto:example@example.org
+  register: account_created_idempotent
+
+- name: Change email address (check mode, diff)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_content: "{{ lookup('file', output_dir ~ '/accountkey.pem') }}"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    state: present
+    # allow_creation: no
+    contact:
+    - mailto:example@example.com
+  check_mode: yes
+  diff: yes
+  register: account_modified_check
+
 - name: Change email address
   acme_account:
     select_crypto_backend: "{{ select_crypto_backend }}"
@@ -70,6 +115,20 @@
   ignore_errors: yes
   register: account_modified_wrong_uri
 
+- name: Clear contact email addresses (check mode, diff)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_src: "{{ output_dir }}/accountkey.pem"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    state: present
+    # allow_creation: no
+    contact: []
+  check_mode: yes
+  diff: yes
+  register: account_modified_2_check
+
 - name: Clear contact email addresses
   acme_account:
     select_crypto_backend: "{{ select_crypto_backend }}"
@@ -100,6 +159,21 @@
 - name: Parse account key (to ease debugging some test failures)
   command: openssl ec -in {{ output_dir }}/accountkey2.pem -noout -text
 
+- name: Change account key (check mode, diff)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_src: "{{ output_dir }}/accountkey.pem"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    new_account_key_src: "{{ output_dir }}/accountkey2.pem"
+    state: changed_key
+    contact:
+    - mailto:example@example.com
+  check_mode: yes
+  diff: yes
+  register: account_change_key_check
+
 - name: Change account key
   acme_account:
     select_crypto_backend: "{{ select_crypto_backend }}"
@@ -113,6 +187,18 @@
     - mailto:example@example.com
   register: account_change_key
 
+- name: Deactivate account (check mode, diff)
+  acme_account:
+    select_crypto_backend: "{{ select_crypto_backend }}"
+    account_key_src: "{{ output_dir }}/accountkey2.pem"
+    acme_version: 2
+    acme_directory: https://{{ acme_host }}:14000/dir
+    validate_certs: no
+    state: absent
+  check_mode: yes
+  diff: yes
+  register: account_deactivate_check
+
 - name: Deactivate account
   acme_account:
     select_crypto_backend: "{{ select_crypto_backend }}"

test/integration/targets/acme_account/tests/validate.yml

@@ -3,6 +3,18 @@
   assert:
     that:
       - account_not_created is failed
+      - account_not_created.msg == 'Account does not exist or is deactivated.'
+
+- name: Validate that account was created in the second step (check mode)
+  assert:
+    that:
+      - account_created_check is changed
+      - account_created_check.account_uri is none
+      - "'diff' in account_created_check"
+      - "account_created_check.diff.before == {}"
+      - "'after' in account_created_check.diff"
+      - account_created_check.diff.after.contact | length == 1
+      - account_created_check.diff.after.contact[0] == 'mailto:example@example.org'
 
 - name: Validate that account was created in the second step
   assert:
@@ -10,6 +22,23 @@
       - account_created is changed
       - account_created.account_uri is not none
 
+- name: Validate that account was created in the second step (idempotency)
+  assert:
+    that:
+      - account_created_idempotent is not changed
+      - account_created_idempotent.account_uri is not none
+
+- name: Validate that email address was changed (check mode)
+  assert:
+    that:
+      - account_modified_check is changed
+      - account_modified_check.account_uri is not none
+      - "'diff' in account_modified_check"
+      - account_modified_check.diff.before.contact | length == 1
+      - account_modified_check.diff.before.contact[0] == 'mailto:example@example.org'
+      - account_modified_check.diff.after.contact | length == 1
+      - account_modified_check.diff.after.contact[0] == 'mailto:example@example.com'
+
 - name: Validate that email address was changed
   assert:
     that:
@@ -27,6 +56,16 @@
     that:
       - account_modified_wrong_uri is failed
 
+- name: Validate that email address was cleared (check mode)
+  assert:
+    that:
+      - account_modified_2_check is changed
+      - account_modified_2_check.account_uri is not none
+      - "'diff' in account_modified_2_check"
+      - account_modified_2_check.diff.before.contact | length == 1
+      - account_modified_2_check.diff.before.contact[0] == 'mailto:example@example.com'
+      - account_modified_2_check.diff.after.contact | length == 0
+
 - name: Validate that email address was cleared
   assert:
     that:
@@ -39,12 +78,29 @@
       - account_modified_2_idempotent is not changed
       - account_modified_2_idempotent.account_uri is not none
 
+- name: Validate that the account key was changed (check mode)
+  assert:
+    that:
+      - account_change_key_check is changed
+      - account_change_key_check.account_uri is not none
+      - "'diff' in account_change_key_check"
+      - account_change_key_check.diff.before.public_account_key != account_change_key_check.diff.after.public_account_key
+
 - name: Validate that the account key was changed
   assert:
     that:
       - account_change_key is changed
       - account_change_key.account_uri is not none
 
+- name: Validate that the account was deactivated (check mode)
+  assert:
+    that:
+      - account_deactivate_check is changed
+      - account_deactivate_check.account_uri is not none
+      - "'diff' in account_deactivate_check"
+      - "account_deactivate_check.diff.before != {}"
+      - "account_deactivate_check.diff.after == {}"
+
 - name: Validate that the account was deactivated
   assert:
     that:
@@ -61,8 +117,10 @@
   assert:
     that:
       - account_not_created_2 is failed
+      - account_not_created_2.msg == 'Account does not exist or is deactivated.'
 
 - name: Validate that the account is gone (old account key)
   assert:
     that:
       - account_not_created_3 is failed
+      - account_not_created_3.msg == 'Account does not exist or is deactivated.'

test/integration/targets/acme_account_facts/tests/validate.yml

@@ -13,6 +13,7 @@
       - account_created.account_uri is not none
       - "'account' in account_created"
       - "'contact' in account_created.account"
+      - "'public_account_key' in account_created.account"
       - account_created.account.contact | length == 1
       - "account_created.account.contact[0] == 'mailto:example@example.org'"
 
@@ -23,6 +24,7 @@
       - account_modified.account_uri is not none
       - "'account' in account_modified"
       - "'contact' in account_modified.account"
+      - "'public_account_key' in account_modified.account"
       - account_modified.account.contact | length == 0
 
 - name: Validate that account does not exist with wrong account URI