Skip to content

1459 login problems intermittent#1489

Open
phycodurus wants to merge 16 commits intodevfrom
1459-login-problems-intermittent
Open

1459 login problems intermittent#1489
phycodurus wants to merge 16 commits intodevfrom
1459-login-problems-intermittent

Conversation

@phycodurus
Copy link
Copy Markdown
Member

@phycodurus phycodurus commented Mar 30, 2026

resolves login issue by reworking the encryption scheme

Removes old encryption scheme and implements envelope encryption where one master encryption key (TOMTOOLKIT_DEK_ENCRYPTION_KEY) encrypts a user-specific data encryption key (DEK), Profile.encrypted_dek, which is used to encrypt that user's sensitive data. TOMTOOLKIT_DEK_ENCRYPTION_KEY should be treated like Django's SECRET_KEY and is documented (in the code) as such. If the TOM admin wants to rotate the TOMTOOLKIT_DEK_ENCRYPTION_KEY (there's a management command for that), the user data is not touched, only the user's (new) Profile.encrypted_dek is decrypted and re-encrypted. When the user's sensitive data is accessed, this encrypted_dek is decrypted with the TOMTOOLKIT_DEK_ENCRYPTION_KEY and then used to create the cipher that is used to access (i.e. decrypt/encrypt) the sensitive data.

Unchanged:

  • configuration of Models with encrypted data: EncryptableModelMixin
  • the Python descriptor that does uses the _cipher to access the encrypted Field EncryptedProperty
  • the accessor functions in session_utils: {get,set}_encrypted_field()
    • these create the _cipher and attach it temporarily to the EncryptedProperty descriptor to access the data.

Changes:

  • adds TOMTOOLKIT_DEK_ENCRYPTION_KEY to
    • tom_base/settings.py
    • tom_setup settings.toml
  • adds encrypted_dek BinaryField to every user Profile model
    • migration for this
    • BinaryFields are not included in Django's model_to_dict() function which provides context dictionary with the data to display in the Profile card template. So, the encrypted_dek (intentionally does not appear in the Profile card).
  • removes UserSession model and associated logic
    • migration for this
  • removes encrypted data re-encryption upon password change logic
    • models.reencrypt_model_fields goes away
    • models.clear_encrypt_fields goes away
    • session_utils.reencrypt_data(user) goes away
  • add management command for master key rotation
    • with tests
    • treats session_utils as "service layer" keeping business logic out of the management command (which is a Django thing that should remain pure Django (i.e. no business logic).
  • User Profile signals handle data encryption key (DEK) creation
  • UserUpdateView doesn't care about password changes (warning removed).

closes #1459

TOMTOOKIT_FIELD_ENCRYPTION_KEY is used as the master encryption
key to encrypt/decrypt each User's encrypted_data_encryption_key,
which is saved in the Profile model.
This is the user-specific, envelope-encrypted (by the master cipher),
Data Encryption Field (DEK). It is created with the user's Profile and
encrypted with the master cipher (created with the master
TOMTOOLKIT_FIELD_ENCRYPTION_KEY). If that master key changes, then
each user's Profile.encrypted_dek must be re-encrypted, but that
user's encrypted-data itself doesn't have to change.
the (way better) envelope encryption scheme doesn't save
anything to the session
(I gather that) the term of art in envelope encryption is to
wrap(encrypt) and unwrap(decrypt) the secret encryption keys
(with the TOMTOOLKIT_FIELD_ENCRYPTION_KEY-created cipher in
our case). This commit removes that conceptual jargon in favor
of what's literally going on: encrypting and decrypting.
@phycodurus phycodurus linked an issue Mar 30, 2026 that may be closed by this pull request
@jchate6 jchate6 moved this to Needs Review in TOM Toolkit Apr 1, 2026
Shorten the comment and add a note explaining that BinaryField is
excluded by Django's model_to_dict(), which is why encrypted_dek
intentionally does not appear on the user Profile card.
This warning isn't needed with the new ecryption scheme.
Add a check in TomCommonConfig.ready() that raises
ImproperlyConfigured if the encryption key is missing. This prevents
the TOM from starting in a half-configured state where logins would
crash when the signal tries to generate a DEK.

The error message includes step-by-step instructions for generating
and configuring the key.
Naming Rationale:
The new name is more descriptive because in the "envelope encryption"
scheme, the TOMTOOLKIT_DEK_ENCRYPTION_KEY is the encryption key that
encrypts and decrypts the user's DEK (data encryption key). So it's
the user-DEK encryptiion key for TOM Toolkit.
@phycodurus phycodurus requested review from Fingel and jchate6 April 1, 2026 20:31
@phycodurus phycodurus marked this pull request as ready for review April 1, 2026 21:10
phycodurus and others added 4 commits April 1, 2026 15:31
There were some straggling references to the "field"
encryption key that are now changed to "DEK" encryption
key (including the name of the key rotation management
command).
Copy link
Copy Markdown
Contributor

@jchate6 jchate6 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's talk about this before we publish it.

Comment thread tom_common/apps.py
"""
key = getattr(settings, 'TOMTOOLKIT_DEK_ENCRYPTION_KEY', '')
if not key:
raise ImproperlyConfigured(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty bad user experience.

Not having this secret key should not keep a TOM with no encryption needs from booting. There needs to be some way to bypass this functionality if it's not needed. Perhaps with a warning message on the profile page if it isn't set. "Warning, this TOM does not have User Encryption set up. Contact an administrator or see docs."

We should not have a multi-step process described in an error message.
We should have a script that performs these steps, or a link to troubleshooting documentation.

This has no backwards compatibility. If we want this to be a permanent part of TOM Base, we should roll this out as part of V3. If there is no reason for that (and there doesn't currently appear to be any), then adding an encryption key should be described in the docs and as part of any apps that require one.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setting also needs to be described in customsettings docs.


Usage:
1. Generate a new Fernet key:
python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is there a reason this isn't done as part of this command?

python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())"
2. Run the rotation:
python manage.py rotate_dek_encryption_key --new-key <new_key>
3. Update your environment / settings.py with the new key.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updating the environment is non-trivial. I can find no help for this within the TOMToolkit docs.
We should have at least some help here linked in the encryption docs.

"""
key = getattr(settings, 'TOMTOOLKIT_DEK_ENCRYPTION_KEY', '')
if not key:
from django.core.exceptions import ImproperlyConfigured
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's talk about what other options might be possible here instead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Needs Review

Development

Successfully merging this pull request may close these issues.

TOMToolkit Login problems (intermittent)

3 participants