-
-
Notifications
You must be signed in to change notification settings - Fork 529
Refactor Plugin Config #2575
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
Refactor Plugin Config #2575
Conversation
drosetti
left a comment
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.
I found a bug in the user experience:
- After I save a plugin config in the analyzer the modal doesn't close
- I pruned the volumes, customized classic_dns with "AAAA" and the moved to scan page and in the runtime config still was present the default value even if it ran correctly with the new value.
- The create playbook's modal is empty
- The create pivot's modal send 2 requests on save the first one to save the pivot works correctly, the second one fails:
AttributeError at /api/pivot/testtttt21/plugin_config 'str' object has no attribute 'get'
I suggest to split tests/api_app/test_views.py into multiple tests. Also you could use different classes, as you prefer: class for user test and class for org test or a different class for each method or as you want.
drosetti
left a comment
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.
drosetti
left a comment
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.
I didn't find any bug! Excellent work!
Just one question: can you add a message in the org config for the user without admin or owner role where is specified that they can view the config, but they cannot edit it because is an operation reserved to owner and admins ?
Reminder: convert this pr from draft to normal one.
drosetti
left a comment
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.
Admin cannot modify the org config, also they cannot change their own config
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.
Some food for thought. View full project report here.
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.
Some things to consider. View full project report here.
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.
Worth considering. View full project report here.
drosetti
left a comment
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.
Everything works fine in the plugin config section, but I found a bug when I try to create a pivot. here the stacktrace:
Environment:
Request Method: PATCH
Request URL: http://localhost:80/api/pivot/test_pivot/plugin_config
Django Version: 4.2.16
Python Version: 3.11.7
Installed Applications:
['django.contrib.admin',
'django.contrib.auth',
'django.contrib.contenttypes',
'django.contrib.sessions',
'django.contrib.messages',
'django.contrib.staticfiles',
'django.contrib.postgres',
'prettyjson',
'django_celery_results',
'django_elasticsearch_dsl',
'rest_framework',
'rest_framework_filters',
'rest_framework.authtoken',
'drf_spectacular',
'durin',
'certego_saas',
'certego_saas.apps.user',
'certego_saas.apps.notifications',
'certego_saas.apps.organization',
'authentication',
'api_app',
'api_app.analyzers_manager',
'api_app.connectors_manager',
'api_app.visualizers_manager',
'api_app.playbooks_manager',
'api_app.pivots_manager',
'api_app.ingestors_manager',
'api_app.investigations_manager',
'api_app.data_model_manager',
'rest_email_auth',
'silk',
'django_celery_beat',
'channels',
'treebeard']
Installed Middleware:
['django.middleware.security.SecurityMiddleware',
'whitenoise.middleware.WhiteNoiseMiddleware',
'certego_saas.ext.middlewares.StatsMiddleware',
'django.contrib.sessions.middleware.SessionMiddleware',
'django.middleware.common.CommonMiddleware',
'django.middleware.csrf.CsrfViewMiddleware',
'django.contrib.auth.middleware.AuthenticationMiddleware',
'django.contrib.messages.middleware.MessageMiddleware',
'django.middleware.clickjacking.XFrameOptionsMiddleware',
'certego_saas.ext.middlewares.LogMiddleware',
'silk.middleware.SilkyMiddleware']
Traceback (most recent call last):
File "/usr/local/lib/python3.11/site-packages/django/core/handlers/exception.py", line 55, in inner
response = get_response(request)
^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/core/handlers/base.py", line 197, in _get_response
response = wrapped_callback(request, *callback_args, **callback_kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/django/views/decorators/csrf.py", line 56, in wrapper_view
return view_func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/rest_framework/viewsets.py", line 124, in view
return self.dispatch(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/rest_framework/views.py", line 509, in dispatch
response = self.handle_exception(exc)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/rest_framework/views.py", line 469, in handle_exception
self.raise_uncaught_exception(exc)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/rest_framework/views.py", line 480, in raise_uncaught_exception
raise exc
^^^^^^^^^
File "/usr/local/lib/python3.11/site-packages/rest_framework/views.py", line 506, in dispatch
response = handler(request, *args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/opt/deploy/intel_owl/api_app/views.py", line 1634, in update
instance = PluginConfig.objects.get(id=data["id"])
^^^^^^^^^^
Exception Type: KeyError at /api/pivot/test_pivot/plugin_config
Exception Value: 'id'
drosetti
left a comment
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.
Everything works! Just make the note message to modify the org config visible only for the normal user. Or add a part where it's specified if you are allowed or not to change the config.

Description
Refactor Plugin Config
Type of change
Please delete options that are not relevant.
Checklist
developdumpplugincommand and added it in the project as a data migration. ("How to share a plugin with the community")test_files.zipand you added the default tests for that mimetype in test_classes.py.FREE_TO_USE_ANALYZERSplaybook by following this guide.urlthat contains this information. This is required for Health Checks._monkeypatch()was used in its class to apply the necessary decorators.MockUpResponseof the_monkeypatch()method. This serves us to provide a valid sample for testing.Black,Flake,Isort) gave 0 errors. If you have correctly installed pre-commit, it does these checks and adjustments on your behalf.testsfolder). All the tests (new and old ones) gave 0 errors.DeepSource,Django Doctorsor other third-party linters have triggered any alerts during the CI checks, I have solved those alerts.Important Rules