From 766518ee0e3a7c006b32e4e57d3b830474f45595 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Tue, 31 Dec 2019 13:33:07 +0100 Subject: [PATCH] audit: sanitize kwargs when creating audit event --- passbook/audit/models.py | 28 +++++++++++++++++++++--- passbook/audit/tests/__init__.py | 0 passbook/audit/tests/test_event.py | 16 ++++++++++++++ passbook/providers/oauth/views/oauth2.py | 3 +-- passbook/providers/oidc/lib.py | 2 +- passbook/providers/saml/views.py | 4 ++-- passbook/sources/oauth/views/core.py | 2 +- 7 files changed, 46 insertions(+), 9 deletions(-) create mode 100644 passbook/audit/tests/__init__.py create mode 100644 passbook/audit/tests/test_event.py diff --git a/passbook/audit/models.py b/passbook/audit/models.py index 50b55aa89..762e40fce 100644 --- a/passbook/audit/models.py +++ b/passbook/audit/models.py @@ -1,12 +1,13 @@ """passbook audit models""" from enum import Enum from inspect import getmodule, stack -from typing import Optional +from typing import Optional, Dict, Any from django.conf import settings from django.contrib.auth.models import AnonymousUser from django.contrib.postgres.fields import JSONField from django.core.exceptions import ValidationError +from django.contrib.contenttypes.models import ContentType from django.db import models from django.http import HttpRequest from django.utils.translation import gettext as _ @@ -19,6 +20,26 @@ from passbook.lib.utils.http import get_client_ip LOGGER = get_logger() +def sanitize_dict(source: Dict[Any, Any]) -> Dict[Any, Any]: + """clean source of all Models that would interfere with the JSONField. + Models are replaced with a dictionary of { + app: str, + name: str, + pk: Any + }""" + for key, value in source.items(): + if isinstance(value, dict): + source[key] = sanitize_dict(value) + elif isinstance(value, models.Model): + model_content_type = ContentType.objects.get_for_model(value) + source[key] = { + "app": model_content_type.app_label, + "name": model_content_type.model, + "pk": value.pk, + } + return source + + class EventAction(Enum): """All possible actions to save into the audit log""" @@ -72,8 +93,9 @@ class Event(UUIDModel): ) if not app: app = getmodule(stack()[_inspect_offset][0]).__name__ - event = Event(action=action.value, app=app, context=kwargs) - LOGGER.debug("Created Audit event", action=action, context=kwargs) + cleaned_kwargs = sanitize_dict(kwargs) + event = Event(action=action.value, app=app, context=cleaned_kwargs) + LOGGER.debug("Created Audit event", action=action, context=cleaned_kwargs) return event def from_http( diff --git a/passbook/audit/tests/__init__.py b/passbook/audit/tests/__init__.py new file mode 100644 index 000000000..e69de29bb diff --git a/passbook/audit/tests/test_event.py b/passbook/audit/tests/test_event.py new file mode 100644 index 000000000..d81e97718 --- /dev/null +++ b/passbook/audit/tests/test_event.py @@ -0,0 +1,16 @@ +"""audit event tests""" + +from django.test import TestCase +from guardian.shortcuts import get_anonymous_user + +from passbook.audit.models import Event, EventAction + + +class TestAuditEvent(TestCase): + """Test Audit Event""" + + def test_new_with_model(self): + """Create a new Event passing a model as kwarg""" + event = Event.new(EventAction.CUSTOM, model=get_anonymous_user()) + event.save() + self.assertIsNotNone(event.pk) diff --git a/passbook/providers/oauth/views/oauth2.py b/passbook/providers/oauth/views/oauth2.py index fc9a83256..1a8aa90d0 100644 --- a/passbook/providers/oauth/views/oauth2.py +++ b/passbook/providers/oauth/views/oauth2.py @@ -82,8 +82,7 @@ class PassbookAuthorizationView(AccessMixin, AuthorizationView): def form_valid(self, form): # User has clicked on "Authorize" Event.new( - EventAction.AUTHORIZE_APPLICATION, - authorized_application=self._application.pk, + EventAction.AUTHORIZE_APPLICATION, authorized_application=self._application, ).from_http(self.request) LOGGER.debug( "User authorized Application", diff --git a/passbook/providers/oidc/lib.py b/passbook/providers/oidc/lib.py index 62e2f5978..0a4168102 100644 --- a/passbook/providers/oidc/lib.py +++ b/passbook/providers/oidc/lib.py @@ -33,7 +33,7 @@ def check_permissions(request, user, client): Event.new( EventAction.AUTHORIZE_APPLICATION, - authorized_application=application.pk, + authorized_application=application, skipped_authorization=False, ).from_http(request) return None diff --git a/passbook/providers/saml/views.py b/passbook/providers/saml/views.py index f78e363a1..44865d179 100644 --- a/passbook/providers/saml/views.py +++ b/passbook/providers/saml/views.py @@ -137,7 +137,7 @@ class LoginProcessView(AccessRequiredView): # Log Application Authorization Event.new( EventAction.AUTHORIZE_APPLICATION, - authorized_application=self.provider.application.pk, + authorized_application=self.provider.application, skipped_authorization=True, ).from_http(request) return RedirectToSPView.as_view()( @@ -161,7 +161,7 @@ class LoginProcessView(AccessRequiredView): # User accepted request Event.new( EventAction.AUTHORIZE_APPLICATION, - authorized_application=self.provider.application.pk, + authorized_application=self.provider.application, skipped_authorization=False, ).from_http(request) return RedirectToSPView.as_view()( diff --git a/passbook/sources/oauth/views/core.py b/passbook/sources/oauth/views/core.py index d5db9eae4..73090555b 100644 --- a/passbook/sources/oauth/views/core.py +++ b/passbook/sources/oauth/views/core.py @@ -196,7 +196,7 @@ class OAuthCallback(OAuthClientMixin, View): access.save() UserOAuthSourceConnection.objects.filter(pk=access.pk).update(user=user) Event.new( - EventAction.CUSTOM, message="Linked OAuth Source", source=source.pk + EventAction.CUSTOM, message="Linked OAuth Source", source=source ).from_http(self.request) if was_authenticated: messages.success(