Added DB backend for elasticsearch database by matllubos · Pull Request #97 · druids/django-security

matllubos · 2021-09-15T13:56:29Z

No description provided.

matllubos · 2021-09-24T12:32:27Z

.travis.yml

  - pip install Django~=$DJANGO_VERSION

+before_script:
+  - sleep 10


from travis documentation

matllubos · 2021-09-24T12:32:41Z

.travis.yml

+services:
+  - elasticsearch
+
+before_install:


I need specific version. Default is too old

matllubos · 2021-09-24T12:33:06Z

docs/logger.rst

        return a + b

 Task result will be automatically logged to the ``security.models.CeleryTaskLog``.
-


this method was moved to django-celery-extension

matllubos · 2021-09-24T12:33:43Z

example/apps/test_security/tests/__init__.py

-
-        with assert_raises(CommandError):
-            call_command('celery_health_check', max_created_at_diff=max_created_at_diff)
+from .celery_log import CeleryLogTestCase


all test was rewritten

matllubos · 2021-09-24T12:34:06Z

example/apps/test_security/tests/base.py

@@ -0,0 +1,69 @@
+from io import StringIO


several test helpers

matllubos · 2021-09-24T12:40:35Z

security/backends/elasticsearch/tests.py

+from .models import CommandLog, CeleryTaskRunLog, CeleryTaskInvocationLog, InputRequestLog, OutputRequestLog
+
+
+class store_elasticsearch_log(override_settings):


helper for project testing. We need separate index for parallel test processes. Therefore this context processor create new index for every log and remove it int the end of the test/block

matllubos · 2021-09-24T12:41:23Z

security/backends/signals.py

+    def backend_receiver(signal):
+        def _decorator(func):
+            def _wrapper(*args, **kwargs):
+                if settings.BACKENDS is None or backend_name in settings.BACKENDS:


we can turn of backend for storing. So you can read from log but not write in it (good for tests)

matllubos · 2021-09-24T12:42:39Z

security/contrib/debug_toolbar_log/middleware.py

-                )
+            input_request_logger = getattr(request, 'input_request_logger', None)
+            if input_request_logger:
+                input_request_logger.update_extra_data({'debug_toolbar': toolbar.render_toolbar()})


maybe toolbar is still broken. I will check it in next pull request

matllubos · 2021-09-24T12:43:52Z

security/logging/common.py

+from security.config import settings
+
+
+class SecurityLogger(ContextDecorator, local):


every logger exteds this class. Active loggers creates tree (property loggers) therefore you now parent loger. (For example output request inside input request)

matllubos · 2021-09-24T12:44:22Z

security/logging/common.py

+        self.id = id or (uuid4() if self.name else None)
+        self.parent = SecurityLogger.loggers[-1] if SecurityLogger.loggers else None
+        self.related_objects = set(related_objects) if related_objects else set()
+        self.slug = slug


related_objects, slug and extra_data is extended from parent logger

rubickcz · 2021-10-06T22:00:44Z

docs/installation.rst

+
+.. attribute:: SECURITY_BACKENDS
+
+  With this setting you can select which backends will be used to store logs. Default value is ``None`` which means all installed logs are used.


Maybe you meant all installed backends are used?

yes sure thanks

rubickcz · 2021-10-06T22:01:19Z

docs/installation.rst

+
+.. attribute:: SECURITY_ELASTICSEARCH_DATABASE
+
+  Setting can be used to set ElasticSearch database configuration.


Elasticsearch

rubickcz · 2021-10-06T22:01:24Z

docs/installation.rst

+
+.. attribute:: SECURITY_ELASTICSEARCH_AUTO_REFRESH
+
+  Every write to the elasticsearch database will automatically call auto refresh.


Elasticsearch

rubickcz · 2021-10-06T22:02:20Z

docs/installation.rst

+
+  Every write to the elasticsearch database will automatically call auto refresh.
+
+.. attribute:: SECURITY_LOG_STING_IO_FLUSH_TIMEOUT


Do you really mean STING and not STRING?

yes string thanks

rubickcz · 2021-10-06T22:04:25Z

example/apps/test_security/management/commands/test_command_with_response.py

+class Command(BaseCommand):
+
+    def handle(self, **options):
+        requests.post('http://test.cz/test')


You shouldn't try to access live servers in tests no matter what. Why not http://localhost?

but the tests should mock requests

Yeah, but that cannot be guaranteed, and in case of error, you might be touching live server.

sure I will change it

rubickcz · 2021-10-06T22:05:50Z

example/apps/test_security/tests/base.py

+    databases = ['default', 'security']
+
+    @data_provider
+    def create_user(self, username='test', email='test@test.cz'):


again, I like test@localhost.

rubickcz · 2021-10-06T22:11:36Z

example/apps/test_security/tests/celery_log.py

+                                    expected_run_succeeded_data) as run_succeeded_receiver, \
+                set_signal_receiver(celery_task_run_output_updated) as run_output_updated_receiver, \
+                set_signal_receiver(celery_task_run_failed) as run_failed_receiver, \
+                set_signal_receiver(celery_task_run_retried) as run_retried_receiver:


Huh, this is sort of clumsy. Wouldn't be better to just create some helper class, that would automatically register to all these signals, and have internal counters that would increment when signal called. Or something like that. I believe this repeated initialization (signal registering) is not necessary. Something like

with TestSignalReceiver() as receiver: ... # do your stuff assert_equal(receiver.calls['celery_task_run_succeeded'], 1)

or

assert_equal(receiver.calls, { 'invocation_started_receiver' : 1, 'run_output_updated_receiver' : 6, ... })

The second way of asserting is even better, because it will fail if any unexpected signal is fired, so you don't have to explicitly assert signals that are not supposed to be sent. (the dict will only contain signals that were fired at least once)

yes you are right. I will try to use the decorator which I wrote for project log testing and yes I should rewrite the set_signal_receiver to this decorator. Thanks

rubickcz · 2021-10-06T22:36:16Z

security/backends/elasticsearch/throttling.py

+from .models import InputRequestLog
+
+
+class PerRequestThrottlingValidator(ThrottlingValidator):


For these throttling classes, I think they should not be imported directly, but there should be some factory function/class, that will return the right validator according to used backend. From the prespective of project code, I think you don't need to worry which log backend is used in settings. Also, how do you determine, which validator to use when you have multiple backends is use? (for example elastic and SQL)

Yes you are true. This is next step. I was thinging about the same thing. But again I cannot do whole change in one pull request. Therefore I have next tasks which will be solve it. But yes this is very good point

rubickcz · 2021-10-06T22:39:35Z

security/backends/elasticsearch/throttling.py

+            & Q('range', start={'gte': timezone.now() - timedelta(seconds=self.timeframe)})
+            & Q('slug', slug=slug)
+        ).count()
+        return count_same_requests < self.throttle_at


I think that this decision logic should be implemented in parent. The child classes should only implement counting of the requests, because that is the only thing that is different across the backends.

Yes you are right. I wil solve this in next PR. I want to do some api functions in the backends which will return number of requests for some input (some univerzal filter api). And throttling validator will be only one. This is temporary solution

rubickcz · 2021-10-06T22:43:08Z

security/backends/sql/throttling.py

+            start__gte=timezone.now() - timedelta(seconds=self.timeframe),
+            slug=self.slug
+        ).count()
+        return count_same_requests <= self.throttle_at


In the elastic validators, you have just the = operator here, why? That's why I suggested to unify this in the parent class.

It is problem with async saving to elasticsearch. In the RDS the result will contain currently logged request. In the elasticsearch DB the current request will not be returned.

matllubos force-pushed the ElasticsearchBackend branch 10 times, most recently from 39ff222 to ab85a08 Compare September 17, 2021 09:56

matllubos force-pushed the ElasticsearchBackend branch from ab85a08 to 0ff06e7 Compare September 23, 2021 18:45

Added DB backend for elasticsearch database

9be42e4

matllubos force-pushed the ElasticsearchBackend branch from 0ff06e7 to 9be42e4 Compare September 24, 2021 12:30

matllubos commented Sep 24, 2021

View reviewed changes

matllubos force-pushed the ElasticsearchBackend branch from b8a7e17 to acf5680 Compare October 4, 2021 12:11

rubickcz reviewed Oct 6, 2021

View reviewed changes

matllubos force-pushed the ElasticsearchBackend branch 2 times, most recently from 6a98de3 to ed9addd Compare October 12, 2021 12:27

Remove null values from querystring and headers

e3ee0e5

matllubos force-pushed the ElasticsearchBackend branch from ed9addd to e3ee0e5 Compare October 13, 2021 09:58

Formulka approved these changes Oct 13, 2021

View reviewed changes

matllubos merged commit 81204fa into master Oct 15, 2021

		return a + b

		Task result will be automatically logged to the ``security.models.CeleryTaskLog``.

		from .models import CommandLog, CeleryTaskRunLog, CeleryTaskInvocationLog, InputRequestLog, OutputRequestLog


		class store_elasticsearch_log(override_settings):

		from security.config import settings


		class SecurityLogger(ContextDecorator, local):


		.. attribute:: SECURITY_BACKENDS

		With this setting you can select which backends will be used to store logs. Default value is ``None`` which means all installed logs are used.


		.. attribute:: SECURITY_ELASTICSEARCH_DATABASE

		Setting can be used to set ElasticSearch database configuration.


		.. attribute:: SECURITY_ELASTICSEARCH_AUTO_REFRESH

		Every write to the elasticsearch database will automatically call auto refresh.


		Every write to the elasticsearch database will automatically call auto refresh.

		.. attribute:: SECURITY_LOG_STING_IO_FLUSH_TIMEOUT

		from .models import InputRequestLog


		class PerRequestThrottlingValidator(ThrottlingValidator):

Conversation

matllubos commented Sep 15, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rubickcz Oct 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rubickcz Oct 6, 2021 •

edited

Loading