-
-
Notifications
You must be signed in to change notification settings - Fork 52
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
Add Secrets to the admin page #939
Conversation
Task linked: QF-3734 Add a way to configure secrets via the Admin |
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.
Code looks good, a few small comments.
Unfortunately checking out locally causes this error:
Environment:
Request Method: GET
Request URL: http://localhost:8002/admin/core/secret/
Django Version: 3.2.25
Python Version: 3.10.8
Installed Applications:
['qfieldcloud.web',
'jazzmin',
'django.contrib.admin',
'django.contrib.contenttypes',
'django.contrib.gis',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
'django.contrib.sites',
'django_filters',
'debug_toolbar',
'rest_framework',
'rest_framework.authtoken',
'drf_spectacular',
'allauth',
'allauth.account',
'allauth.socialaccount',
'storages',
'invitations',
'django_cron',
'django_countries',
'timezone_field',
'auditlog',
'qfieldcloud.core',
'django.contrib.auth',
'qfieldcloud.subscription',
'qfieldcloud.notifs',
'qfieldcloud.authentication',
'notifications',
'axes',
'migrate_sql',
'constance',
'constance.backends.database',
'django_extensions',
'bootstrap4',
'django_tables2',
'qfieldcloud.billing']
Installed Middleware:
['debug_toolbar.middleware.DebugToolbarMiddleware',
'django.middleware.security.SecurityMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.locale.LocaleMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'qfieldcloud.core.middleware.requests.attach_keys',
'log_request_id.middleware.RequestIDMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'django_currentuser.middleware.ThreadLocalUserMiddleware',
'auditlog.middleware.AuditlogMiddleware',
'qfieldcloud.core.middleware.timezone.TimezoneMiddleware',
'qfieldcloud.core.middleware.test.TestMiddleware',
'axes.middleware.AxesMiddleware']
Traceback (most recent call last):
File "/usr/local/lib/python3.10/site-packages/django_cryptography/core/signing.py", line 245, in unsign
self.signature(signed_value[:-d_size]).verify(sig)
During handling of the above exception (Signature did not match digest.), another exception occurred:
File "/usr/local/lib/python3.10/site-packages/django/core/handlers/exception.py", line 47, in inner
response = get_response(request)
File "/usr/local/lib/python3.10/site-packages/django/core/handlers/base.py", line 181, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
File "/usr/local/lib/python3.10/site-packages/sentry_sdk/integrations/django/views.py", line 85, in sentry_wrapped_callback
return callback(request, *args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 616, in wrapper
return self.admin_site.admin_view(view)(*args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/sites.py", line 232, in inner
return view(request, *args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 43, in _wrapper
return bound_method(*args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/utils/decorators.py", line 130, in _wrapped_view
response = view_func(request, *args, **kwargs)
File "/usr/local/lib/python3.10/site-packages/django/contrib/admin/options.py", line 1815, in changelist_view
'selection_note': _('0 of %(cnt)s selected') % {'cnt': len(cl.result_list)},
File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 262, in __len__
self._fetch_all()
File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 1324, in _fetch_all
self._result_cache = list(self._iterable_class(self))
File "/usr/local/lib/python3.10/site-packages/django/db/models/query.py", line 68, in __iter__
for row in compiler.results_iter(results):
File "/usr/local/lib/python3.10/site-packages/django/db/models/sql/compiler.py", line 1122, in apply_converters
value = converter(value, expression, connection)
File "/usr/local/lib/python3.10/site-packages/django_cryptography/fields.py", line 174, in from_db_value
return self._load(force_bytes(value))
File "/usr/local/lib/python3.10/site-packages/django_cryptography/fields.py", line 116, in _load
return pickle.loads(self._fernet.decrypt(value, self.ttl))
File "/usr/local/lib/python3.10/site-packages/django_cryptography/utils/crypto.py", line 143, in decrypt
data = self._signer.unsign(data, ttl)
File "/usr/local/lib/python3.10/site-packages/django_cryptography/core/signing.py", line 247, in unsign
raise BadSignature(
Exception Type: BadSignature at /admin/core/secret/
Exception Value: Signature "b'fMju/hqFTCst1kP8c5/Bp5tRZn30lRedc2R+HJl3A9A=\n'" does not match
Any ideas? Shall we look together?
@suricactus BadSignature error occurs when you have secrets in the db encrypted with a different key (e.g. a db dump from another system). Try to clean the secrets table and test with newly created secrets. |
There is now a read-only ProjectSecretInline: The only missing part is a button in the project admin to create a new secret for that project. @suricactus do you have an idea how this can be implemented (without a custom project template)? Otherwise I think this is ready to be merged. |
See potential fix to this issue here: #949
I did a small commit, check it's commit and message. In general, we always prefer to use the decorator for new admin stuff, the old way looks a bit too hacky. |
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.
LGTM
One thing which you might have considered is to move the validation from admin.ProjectSecretForm.clean()
to the models.Secret.clean()
.
632c24c
to
8b157fc
Compare
Since we are messing with the name in Just re-based on my machine. If the tests pass, I will squash and merge. |
Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com>
Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com>
Co-authored-by: Ivan Ivanov <suricactus@users.noreply.github.com>
1902184
to
eb02bba
Compare
Secrets list view:
Adding a new secret:
Name
,Type
,Value
andProject
can be specifiedCreated by
will be set to the current userModifying an existing secret:
Value
is not presented to the userValue
can be overwrittenType