[AAP-76852] Fix: catch InvalidToken in from_db() methods to log CRITICAL before crash (#1044)

## Summary

When `SECRET_KEY` changes (e.g., after `podman system reset` destroys
the Podman secret and the installer regenerates a new one), all existing
encrypted database rows become undecryptable. Two `from_db()` methods in
`django-ansible-base` called `decrypt_string()` with no error handling
for `cryptography.fernet.InvalidToken`, causing unhandled exceptions
that crash the service on startup with no actionable diagnostic output.

## Root Cause

- **`AbstractCommonModel.from_db()`**
(`ansible_base/lib/abstract_models/common.py:197-205`) — iterates over
`encrypted_fields` calling `ansible_encryption.decrypt_string()` with no
guard. Affects `ServiceKey.secret`, `AuthenticatorUser.extra_data`, and
any other model with `encrypted_fields`.
- **`Authenticator.from_db()`**
(`ansible_base/authentication/models/authenticator.py:76-90`) — has a
`try/except ImportError` but does not catch `InvalidToken`. Encrypted
authenticator config fields (LDAP `BIND_PASSWORD`, OIDC `SECRET`, SAML
`SP_PRIVATE_KEY`) cause an unhandled crash.

Per the staff engineering decision (June 18): catch `InvalidToken`, log
CRITICAL with model/PK/recovery context, then re-raise. This preserves
the crash behaviour while providing actionable diagnostic output before
the system fails.

## Changes

- `ansible_base/lib/abstract_models/common.py`: Wrap each
`decrypt_string()` call in the `encrypted_fields` loop in `from_db()`
with `try/except InvalidToken`. Log CRITICAL with model class name,
field name, and PK. Re-raise unchanged.
- `ansible_base/authentication/models/authenticator.py`: Add `except
InvalidToken` clause in `from_db()` alongside the existing `except
ImportError`. Log CRITICAL with authenticator name and type. Re-raise
unchanged. Add `from cryptography.fernet import InvalidToken` import and
a module-level logger.
- `test_app/tests/lib/abstract_models/test_common.py`: Add
`test_from_db_invalid_token_logs_and_reraises` — patches
`decrypt_string` to raise `InvalidToken`, asserts re-raise and CRITICAL
log.
- `test_app/tests/authentication/models/test_authenticator.py`: Add
`test_authenticator_from_db_invalid_token_logs_and_reraises` — same
pattern for the LDAP authenticator fixture.

## Risk Assessment

- **Confidence:** high
- **Risk:** low — exception is re-raised so runtime behaviour is
unchanged; the only addition is the CRITICAL log message before the
crash; no data is modified; no migrations; no API changes
- **Scope:** 4 files changed

## Testing

- [ ] Unit tests pass
- [ ] Regression test added (test patches decrypt_string to raise
InvalidToken, asserts re-raise + CRITICAL log)
- [ ] Lint checks pass

## JIRA

- Ticket: [AAP-76852](https://issues.redhat.com/browse/AAP-76852)

---
*Assisted-by: Debuggernaut (claude-opus-4-6) <noreply@redhat.com> |
Review carefully before merging.*

[AAP-76852]:
https://redhat.atlassian.net/browse/AAP-76852?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved error handling when encrypted configuration or model data
cannot be decrypted.
* Added critical logging with relevant details to make decryption
failures easier to diagnose.
* Prevented invalid encrypted data from being loaded silently by
preserving the original error.

* **Tests**
* Added coverage confirming decryption failures are logged and correctly
re-raised.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

---------

Co-authored-by: Aegis-bot <bot@ambient-code.local>
Co-authored-by: jeffh-oss <jheadley@redhat.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

Author: 4 people

ansible_base/authentication/models/authenticator.py

@@ -1,9 +1,14 @@
+import logging
+
+from cryptography.fernet import InvalidToken
 from django.db.models import SET_NULL, ForeignKey, JSONField, fields
 
 from ansible_base.authentication.authenticator_plugins.utils import generate_authenticator_slug, get_authenticator_plugin
 from ansible_base.lib.abstract_models.common import UniqueNamedCommonModel
 from ansible_base.lib.utils.models import prevent_search
 
+logger = logging.getLogger('ansible_base.authentication.models.authenticator')
+
 
 def get_next_authenticator_order():
     """
@@ -86,6 +91,16 @@ def from_db(cls, db, field_names, values):
         except ImportError:
             # A log message will already be displayed if this fails
             pass
+        except InvalidToken:
+            logger.critical(
+                "Failed to decrypt configuration field on Authenticator(name=%r, type=%r): "
+                "the SECRET_KEY may have changed. "
+                "Restore the original SECRET_KEY that encrypted this database, then restart. "
+                "Re-raising to prevent startup with corrupted authenticator configuration.",
+                instance.name,
+                instance.type,
+            )
+            raise
 
         return instance
 

ansible_base/lib/abstract_models/common.py

@@ -196,11 +196,24 @@ def save(self, *args, **kwargs):
 
     @classmethod
     def from_db(self, db, field_names, values):
+        from cryptography.fernet import InvalidToken
+
         instance = super().from_db(db, field_names, values)
 
         for field in self.encrypted_fields:
             field_value = getattr(instance, field, None)
-            setattr(instance, field, ansible_encryption.decrypt_string(field_value))
+            try:
+                setattr(instance, field, ansible_encryption.decrypt_string(field_value))
+            except InvalidToken:
+                logger.critical(
+                    "Failed to decrypt field %r on %s (pk=%r): the SECRET_KEY may have changed. "
+                    "Restore the original SECRET_KEY that encrypted this database, then restart. "
+                    "Re-raising to prevent the model from loading with corrupted data.",
+                    field,
+                    self.__name__,
+                    getattr(instance, 'pk', None),
+                )
+                raise
 
         return instance
 

test_app/tests/authentication/models/test_authenticator.py

@@ -1,4 +1,6 @@
+import logging
 from unittest import mock
+from unittest.mock import patch
 
 import pytest
 
@@ -47,3 +49,23 @@ def test_dupe_slug(ldap_authenticator):
 
     dupe.save()
     assert dupe.slug != ldap_slug, "authenticator slugs should be unique"
+
+
+@pytest.mark.django_db
+def test_authenticator_from_db_invalid_token_logs_and_reraises(ldap_authenticator, caplog):
+    """Authenticator.from_db() must log CRITICAL and re-raise InvalidToken.
+
+    Simulates a SECRET_KEY change making encrypted authenticator configuration
+    fields (e.g. LDAP BIND_PASSWORD) undecryptable -- part of AAP-76852.
+    """
+    from cryptography.fernet import InvalidToken
+
+    with patch("ansible_base.lib.utils.encryption.ansible_encryption.decrypt_string", side_effect=InvalidToken):
+        with caplog.at_level(logging.CRITICAL, logger="ansible_base.authentication.models.authenticator"):
+            with pytest.raises(InvalidToken):
+                Authenticator.objects.get(pk=ldap_authenticator.pk)
+
+    critical_records = [r for r in caplog.records if r.levelno == logging.CRITICAL]
+    assert critical_records, "Expected at least one CRITICAL log record"
+    assert any("SECRET_KEY" in r.message for r in critical_records), "Expected CRITICAL log to mention SECRET_KEY"
+    assert any(ldap_authenticator.name in r.message for r in critical_records), "Expected CRITICAL log to include the authenticator name"

test_app/tests/lib/abstract_models/test_common.py

@@ -366,3 +366,27 @@ def test_compare_data_types_after_decryption(input):
     decryptor = Fernet256()
     results = decryptor.decrypt_string(decryptor.encrypt_string(input))
     assert type(results) is type(input)
+
+
+@pytest.mark.django_db
+def test_from_db_invalid_token_logs_and_reraises(caplog):
+    """AbstractCommonModel.from_db() must log CRITICAL and re-raise InvalidToken.
+
+    Simulates a SECRET_KEY change that leaves encrypted_fields rows unreadable
+    -- the DAB side of AAP-76852.
+    """
+    import logging
+
+    from cryptography.fernet import InvalidToken
+
+    model = EncryptionModel.objects.create(testing1='sensitive_value')
+
+    with patch("ansible_base.lib.utils.encryption.ansible_encryption.decrypt_string", side_effect=InvalidToken):
+        with caplog.at_level(logging.CRITICAL, logger="ansible_base.lib.abstract_models.common"):
+            with pytest.raises(InvalidToken):
+                EncryptionModel.objects.get(pk=model.pk)
+
+    critical_records = [r for r in caplog.records if r.levelno == logging.CRITICAL]
+    assert critical_records, "Expected at least one CRITICAL log record"
+    assert any("SECRET_KEY" in r.message for r in critical_records), "Expected CRITICAL log to mention SECRET_KEY"
+    assert any(str(model.pk) in r.message for r in critical_records), "Expected CRITICAL log to include the model pk"