-
Notifications
You must be signed in to change notification settings - Fork 53
1459 login problems intermittent #1489
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: version-3-0-alpha
Are you sure you want to change the base?
Changes from all commits
34676fc
8f7a55b
5de019c
67ee2e1
bf02e11
dfcfce6
6a13659
77680df
d4b1064
9c33cd5
1a993be
1c90633
4c2f672
63fc174
bcbf081
2386711
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,21 @@ | ||
| from django.apps import AppConfig | ||
| from django.conf import settings | ||
| from django.core.exceptions import ImproperlyConfigured | ||
| import plotly.io as pio | ||
|
|
||
|
|
||
| class TomCommonConfig(AppConfig): | ||
| name = 'tom_common' | ||
|
|
||
| def ready(self): | ||
| # Import signals for automatically saving profiles when updating User objects | ||
| # Import signals so their @receiver decorators are executed, which | ||
| # registers the signal handlers. Without this import, signal handlers | ||
| # in signals.py would never fire. | ||
| # https://docs.djangoproject.com/en/5.1/topics/signals/#connecting-receiver-functions | ||
| import tom_common.signals # noqa | ||
|
|
||
| self._check_dek_encryption_key() | ||
|
|
||
| # Set default plotly theme on startup | ||
| valid_themes = ['plotly', 'plotly_white', 'plotly_dark', 'ggplot2', 'seaborn', 'simple_white', 'none'] | ||
|
|
||
|
|
@@ -21,6 +26,35 @@ def ready(self): | |
|
|
||
| pio.templates.default = plotly_theme | ||
|
|
||
| def _check_dek_encryption_key(self) -> None: | ||
| """Verify that the DEK encryption master key is configured. | ||
|
|
||
| This key is required for encrypting sensitive user data (API keys, | ||
| observatory credentials) at rest in the database. Without it, the | ||
| TOM is prevented from starting. | ||
| """ | ||
| key = getattr(settings, 'TOMTOOLKIT_DEK_ENCRYPTION_KEY', '') | ||
| if not key: | ||
| raise ImproperlyConfigured( | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. 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. |
||
| "\n\n" | ||
| "TOMTOOLKIT_DEK_ENCRYPTION_KEY is not set.\n\n" | ||
| "This setting is required for encrypting sensitive user data at rest.\n" | ||
| "To fix this:\n\n" | ||
| " 1. Generate a key (requires the 'cryptography' package, which is\n" | ||
| " installed as a dependency of tom-base):\n\n" | ||
| " python -c \"from cryptography.fernet import Fernet; " | ||
| "print(Fernet.generate_key().decode())\"\n\n" | ||
| " 2. Set the key as an environment variable:\n\n" | ||
| " export TOMTOOLKIT_DEK_ENCRYPTION_KEY='<paste the generated key>'\n\n" | ||
| " Then reference it in your settings.py:\n\n" | ||
| " TOMTOOLKIT_DEK_ENCRYPTION_KEY = os.getenv(\n" | ||
| " 'TOMTOOLKIT_DEK_ENCRYPTION_KEY')\n\n" | ||
| " 3. Restart your TOM.\n\n" | ||
| "Treat this key like SECRET_KEY — keep it secret, do not commit it\n" | ||
| "to source control, and back it up. If this key is lost, users will\n" | ||
| "need to re-enter their saved external service credentials.\n" | ||
| ) | ||
|
|
||
| def profile_details(self): | ||
| """ | ||
| Integration point for adding items to the user profile page. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,73 @@ | ||
| """Management command to rotate the TOMTOOLKIT_DEK_ENCRYPTION_KEY. | ||
|
|
||
| This is a thin CLI wrapper around ``session_utils.rotate_master_key()``. | ||
| See that function for the actual rotation logic. | ||
|
|
||
| Usage: | ||
| 1. Generate a new Fernet key: | ||
| python -c "from cryptography.fernet import Fernet; print(Fernet.generate_key().decode())" | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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? |
||
| 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. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| 4. Restart the server. | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| from django.core.management.base import BaseCommand, CommandError | ||
|
|
||
| from tom_common.session_utils import rotate_master_key | ||
|
|
||
|
|
||
| class Command(BaseCommand): | ||
| help = ( | ||
| 'Re-encrypts all per-user Data Encryption Keys (DEKs) with a new master key. ' | ||
| 'Run this when rotating TOMTOOLKIT_DEK_ENCRYPTION_KEY.' | ||
| ) | ||
|
|
||
| def add_arguments(self, parser) -> None: | ||
| parser.add_argument( | ||
| '--new-key', | ||
| required=True, | ||
| help='The new Fernet master key (URL-safe base64-encoded, 32 bytes). ' | ||
| 'Generate with: python -c "from cryptography.fernet import Fernet; ' | ||
| 'print(Fernet.generate_key().decode())"', | ||
| ) | ||
|
|
||
| def handle(self, *args, **options) -> None: | ||
| new_key: str = options['new_key'] | ||
|
|
||
| try: | ||
| result = rotate_master_key(new_key) | ||
| except ValueError as e: | ||
| raise CommandError(str(e)) | ||
| except Exception as e: | ||
| raise CommandError(f"Cannot access current master key: {e}") | ||
|
|
||
| if result.total == 0: | ||
| self.stdout.write(self.style.WARNING( | ||
| "No profiles with encryption keys found. Nothing to rotate." | ||
| )) | ||
| return | ||
|
|
||
| self.stdout.write(f"Re-encrypting DEKs for {result.total} profile(s)...") | ||
|
|
||
| if result.success_count: | ||
| self.stdout.write(self.style.SUCCESS( | ||
| f"Done. {result.success_count} re-encrypted successfully." | ||
| )) | ||
|
|
||
| for error in result.errors: | ||
| self.stderr.write(self.style.ERROR( | ||
| f" FAILED: Profile pk={error.profile_pk} (user={error.username}) — {error.error}" | ||
| )) | ||
|
|
||
| if result.error_count: | ||
| self.stdout.write(self.style.ERROR( | ||
| f"{result.error_count} failed — see errors above." | ||
| )) | ||
|
|
||
| self.stdout.write("") | ||
| self.stdout.write(self.style.WARNING( | ||
| "IMPORTANT: Update TOMTOOLKIT_DEK_ENCRYPTION_KEY in your environment / " | ||
| "settings.py with the new key, then restart the server." | ||
| )) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| # Generated by Django 5.2.12 on 2026-03-27 22:29 | ||
|
|
||
| from django.db import migrations, models | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('tom_common', '0002_usersession'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.AddField( | ||
| model_name='profile', | ||
| name='encrypted_dek', | ||
| field=models.BinaryField(blank=True, null=True), | ||
| ), | ||
| ] |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| # Generated by Django 5.2.12 on 2026-03-27 22:29 | ||
|
|
||
| from django.db import migrations | ||
|
|
||
|
|
||
| class Migration(migrations.Migration): | ||
|
|
||
| dependencies = [ | ||
| ('tom_common', '0003_profile_encrypted_dek'), | ||
| ] | ||
|
|
||
| operations = [ | ||
| migrations.DeleteModel( | ||
| name='UserSession', | ||
| ), | ||
| ] |
There was a problem hiding this comment.
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
customsettingsdocs.