From 807cbbeaafd9062fa5f860d3669cdb5100b13269 Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Thu, 5 Dec 2019 16:14:08 +0100 Subject: [PATCH] audit: rewrite to be independent of django http requests, allow custom actions --- .../migrations/0003_auto_20191205_1407.py | 24 ++++ .../migrations/0004_auto_20191205_1502.py | 22 ++++ passbook/audit/models.py | 114 +++++++++++------- passbook/audit/signals.py | 14 +-- passbook/factors/otp/views.py | 19 +-- passbook/providers/oauth/views/oauth2.py | 11 +- passbook/providers/oidc/lib.py | 10 +- passbook/providers/saml/views.py | 18 ++- passbook/sources/oauth/views/core.py | 15 +-- 9 files changed, 146 insertions(+), 101 deletions(-) create mode 100644 passbook/audit/migrations/0003_auto_20191205_1407.py create mode 100644 passbook/audit/migrations/0004_auto_20191205_1502.py diff --git a/passbook/audit/migrations/0003_auto_20191205_1407.py b/passbook/audit/migrations/0003_auto_20191205_1407.py new file mode 100644 index 000000000..b57661317 --- /dev/null +++ b/passbook/audit/migrations/0003_auto_20191205_1407.py @@ -0,0 +1,24 @@ +# Generated by Django 2.2.8 on 2019-12-05 14:07 + +from django.db import migrations, models + +import passbook.audit.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_audit', '0002_auto_20191028_0829'), + ] + + operations = [ + migrations.AlterModelOptions( + name='event', + options={'verbose_name': 'Audit Event', 'verbose_name_plural': 'Audit Events'}, + ), + migrations.AlterField( + model_name='event', + name='action', + field=models.TextField(choices=[('LOGIN', 'login'), ('LOGIN_FAILED', 'login_failed'), ('LOGOUT', 'logout'), ('AUTHORIZE_APPLICATION', 'authorize_application'), ('SUSPICIOUS_REQUEST', 'suspicious_request'), ('SIGN_UP', 'sign_up'), ('PASSWORD_RESET', 'password_reset'), ('INVITE_CREATED', 'invitation_created'), ('INVITE_USED', 'invitation_used'), ('CUSTOM', 'custom')]), + ), + ] diff --git a/passbook/audit/migrations/0004_auto_20191205_1502.py b/passbook/audit/migrations/0004_auto_20191205_1502.py new file mode 100644 index 000000000..414c46934 --- /dev/null +++ b/passbook/audit/migrations/0004_auto_20191205_1502.py @@ -0,0 +1,22 @@ +# Generated by Django 2.2.8 on 2019-12-05 15:02 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('passbook_audit', '0003_auto_20191205_1407'), + ] + + operations = [ + migrations.RemoveField( + model_name='event', + name='request_ip', + ), + migrations.AddField( + model_name='event', + name='client_ip', + field=models.GenericIPAddressField(null=True), + ), + ] diff --git a/passbook/audit/models.py b/passbook/audit/models.py index 453c9c4ff..8fc4b6686 100644 --- a/passbook/audit/models.py +++ b/passbook/audit/models.py @@ -1,10 +1,16 @@ """passbook audit models""" +from enum import Enum +from inspect import getmodule, stack +from typing import Optional + 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.db import models +from django.http import HttpRequest from django.utils.translation import gettext as _ +from guardian.shortcuts import get_anonymous_user from structlog import get_logger from passbook.lib.models import UUIDModel @@ -12,64 +18,86 @@ from passbook.lib.utils.http import get_client_ip LOGGER = get_logger() +class EventAction(Enum): + """All possible actions to save into the audit log""" + + LOGIN = 'login' + LOGIN_FAILED = 'login_failed' + LOGOUT = 'logout' + AUTHORIZE_APPLICATION = 'authorize_application' + SUSPICIOUS_REQUEST = 'suspicious_request' + SIGN_UP = 'sign_up' + PASSWORD_RESET = 'password_reset' # noqa # nosec + INVITE_CREATED = 'invitation_created' + INVITE_USED = 'invitation_used' + CUSTOM = 'custom' + + @staticmethod + def as_choices(): + """Generate choices of actions used for database""" + return tuple((x, y.value) for x, y in EventAction.__members__.items()) + + class Event(UUIDModel): """An individual audit log event""" - ACTION_LOGIN = 'login' - ACTION_LOGIN_FAILED = 'login_failed' - ACTION_LOGOUT = 'logout' - ACTION_AUTHORIZE_APPLICATION = 'authorize_application' - ACTION_SUSPICIOUS_REQUEST = 'suspicious_request' - ACTION_SIGN_UP = 'sign_up' - ACTION_PASSWORD_RESET = 'password_reset' # noqa # nosec - ACTION_INVITE_CREATED = 'invitation_created' - ACTION_INVITE_USED = 'invitation_used' - ACTIONS = ( - (ACTION_LOGIN, ACTION_LOGIN), - (ACTION_LOGIN_FAILED, ACTION_LOGIN_FAILED), - (ACTION_LOGOUT, ACTION_LOGOUT), - (ACTION_AUTHORIZE_APPLICATION, ACTION_AUTHORIZE_APPLICATION), - (ACTION_SUSPICIOUS_REQUEST, ACTION_SUSPICIOUS_REQUEST), - (ACTION_SIGN_UP, ACTION_SIGN_UP), - (ACTION_PASSWORD_RESET, ACTION_PASSWORD_RESET), - (ACTION_INVITE_CREATED, ACTION_INVITE_CREATED), - (ACTION_INVITE_USED, ACTION_INVITE_USED), - ) - user = models.ForeignKey(settings.AUTH_USER_MODEL, null=True, on_delete=models.SET_NULL) - action = models.TextField(choices=ACTIONS) + action = models.TextField(choices=EventAction.as_choices()) date = models.DateTimeField(auto_now_add=True) app = models.TextField() context = JSONField(default=dict, blank=True) - request_ip = models.GenericIPAddressField() + client_ip = models.GenericIPAddressField(null=True) created = models.DateTimeField(auto_now_add=True) @staticmethod - def create(action, request, **kwargs): - """Create Event from arguments""" - client_ip = get_client_ip(request) - if not hasattr(request, 'user'): - user = None - else: - user = request.user - if isinstance(user, AnonymousUser): - user = kwargs.get('user', None) - entry = Event.objects.create( - action=action, - user=user, - # User 255.255.255.255 as fallback if IP cannot be determined - request_ip=client_ip or '255.255.255.255', + def _get_app_from_request(request: HttpRequest) -> str: + if not isinstance(request, HttpRequest): + return "" + return request.resolver_match.app_name + + @staticmethod + def new(action: EventAction, + app: Optional[str] = None, + _inspect_offset: int = 1, + **kwargs) -> 'Event': + """Create new Event instance from arguments. Instance is NOT saved.""" + if not isinstance(action, EventAction): + raise ValueError(f"action must be EventAction instance but was {type(action)}") + if not app: + app = getmodule(stack()[_inspect_offset][0]).__name__ + event = Event( + action=action.value, + app=app, context=kwargs) - LOGGER.debug("Created Audit entry", action=action, - user=user, from_ip=client_ip, context=kwargs) - return entry + LOGGER.debug("Created Audit event", action=action, context=kwargs) + return event + + def from_http(self, request: HttpRequest, + user: Optional[settings.AUTH_USER_MODEL] = None) -> 'Event': + """Add data from a Django-HttpRequest, allowing the creation of + Events independently from requests. + `user` arguments optionally overrides user from requests.""" + if hasattr(request, 'user'): + if isinstance(request.user, AnonymousUser): + self.user = get_anonymous_user() + else: + self.user = request.user + if user: + self.user = user + # User 255.255.255.255 as fallback if IP cannot be determined + self.client_ip = get_client_ip(request) or '255.255.255.255' + # If there's no app set, we get it from the requests too + if not self.app: + self.app = Event._get_app_from_request(request) + self.save() + return self def save(self, *args, **kwargs): if not self._state.adding: raise ValidationError("you may not edit an existing %s" % self._meta.model_name) - super().save(*args, **kwargs) + return super().save(*args, **kwargs) class Meta: - verbose_name = _('Audit Entry') - verbose_name_plural = _('Audit Entries') + verbose_name = _('Audit Event') + verbose_name_plural = _('Audit Events') diff --git a/passbook/audit/signals.py b/passbook/audit/signals.py index 643025e55..8362bd819 100644 --- a/passbook/audit/signals.py +++ b/passbook/audit/signals.py @@ -2,7 +2,7 @@ from django.contrib.auth.signals import user_logged_in, user_logged_out from django.dispatch import receiver -from passbook.audit.models import Event +from passbook.audit.models import Event, EventAction from passbook.core.signals import (invitation_created, invitation_used, user_signed_up) @@ -10,26 +10,24 @@ from passbook.core.signals import (invitation_created, invitation_used, @receiver(user_logged_in) def on_user_logged_in(sender, request, user, **kwargs): """Log successful login""" - Event.create(Event.ACTION_LOGIN, request) + Event.new(EventAction.LOGIN).from_http(request) @receiver(user_logged_out) def on_user_logged_out(sender, request, user, **kwargs): """Log successfully logout""" - Event.create(Event.ACTION_LOGOUT, request) + Event.new(EventAction.LOGOUT).from_http(request) @receiver(user_signed_up) def on_user_signed_up(sender, request, user, **kwargs): """Log successfully signed up""" - Event.create(Event.ACTION_SIGN_UP, request) + Event.new(EventAction.SIGN_UP).from_http(request) @receiver(invitation_created) def on_invitation_created(sender, request, invitation, **kwargs): """Log Invitation creation""" - Event.create(Event.ACTION_INVITE_CREATED, request, - invitation_uuid=invitation.uuid.hex) + Event.new(EventAction.INVITE_CREATED, invitation_uuid=invitation.uuid.hex).from_http(request) @receiver(invitation_used) def on_invitation_used(sender, request, invitation, **kwargs): """Log Invitation usage""" - Event.create(Event.ACTION_INVITE_USED, request, - invitation_uuid=invitation.uuid.hex) + Event.new(EventAction.INVITE_USED, invitation_uuid=invitation.uuid.hex).from_http(request) diff --git a/passbook/factors/otp/views.py b/passbook/factors/otp/views.py index f7488da43..7961992c6 100644 --- a/passbook/factors/otp/views.py +++ b/passbook/factors/otp/views.py @@ -16,6 +16,7 @@ from qrcode import make from qrcode.image.svg import SvgPathImage from structlog import get_logger +from passbook.audit.models import Event, EventAction from passbook.factors.otp.forms import OTPSetupForm from passbook.factors.otp.utils import otpauth_url from passbook.lib.boilerplate import NeverCacheMixin @@ -55,12 +56,7 @@ class DisableView(LoginRequiredMixin, View): token.delete() messages.success(request, 'Successfully disabled OTP') # Create event with email notification - # Event.create( - # user=request.user, - # message=_('You disabled TOTP.'), - # current=True, - # request=request, - # send_notification=True) + Event.new(EventAction.CUSTOM, message='User disabled OTP.').from_http(request) return redirect(reverse('passbook_factors_otp:otp-user-settings')) class EnableView(LoginRequiredMixin, FormView): @@ -77,7 +73,7 @@ class EnableView(LoginRequiredMixin, FormView): def get_context_data(self, **kwargs): kwargs['config'] = CONFIG.y('passbook') kwargs['is_login'] = True - kwargs['title'] = _('Configue OTP') + kwargs['title'] = _('Configure OTP') kwargs['primary_action'] = _('Setup') return super().get_context_data(**kwargs) @@ -134,14 +130,7 @@ class EnableView(LoginRequiredMixin, FormView): self.static_device.confirmed = True self.static_device.save() del self.request.session[OTP_SETTING_UP_KEY] - # Create event with email notification - # TODO: Create Audit Log entry - # Event.create( - # user=self.request.user, - # message=_('You activated TOTP.'), - # current=True, - # request=self.request, - # send_notification=True) + Event.new(EventAction.CUSTOM, message='User enabled OTP.').from_http(self.request) return redirect('passbook_factors_otp:otp-user-settings') class QRView(NeverCacheMixin, View): diff --git a/passbook/providers/oauth/views/oauth2.py b/passbook/providers/oauth/views/oauth2.py index 223bac2ea..b976eb4ea 100644 --- a/passbook/providers/oauth/views/oauth2.py +++ b/passbook/providers/oauth/views/oauth2.py @@ -8,7 +8,7 @@ from django.utils.translation import ugettext as _ from oauth2_provider.views.base import AuthorizationView from structlog import get_logger -from passbook.audit.models import Event +from passbook.audit.models import Event, EventAction from passbook.core.models import Application from passbook.core.views.access import AccessMixin from passbook.core.views.utils import LoadingView, PermissionDeniedView @@ -77,9 +77,8 @@ class PassbookAuthorizationView(AccessMixin, AuthorizationView): def form_valid(self, form): # User has clicked on "Authorize" - Event.create( - action=Event.ACTION_AUTHORIZE_APPLICATION, - request=self.request, - app=str(self._application)) - LOGGER.debug('user %s authorized %s', self.request.user, self._application) + Event.new(EventAction.AUTHORIZE_APPLICATION, + authorized_application=self._application).from_http(self.request) + LOGGER.debug('User authorized Application', + user=self.request.user, application=self._application) return super().form_valid(form) diff --git a/passbook/providers/oidc/lib.py b/passbook/providers/oidc/lib.py index 557471139..4797e8c2e 100644 --- a/passbook/providers/oidc/lib.py +++ b/passbook/providers/oidc/lib.py @@ -3,7 +3,7 @@ from django.contrib import messages from django.shortcuts import redirect from structlog import get_logger -from passbook.audit.models import Event +from passbook.audit.models import Event, EventAction from passbook.core.models import Application from passbook.policies.engine import PolicyEngine @@ -28,9 +28,7 @@ def check_permissions(request, user, client): messages.error(request, policy_message) return redirect('passbook_providers_oauth:oauth2-permission-denied') - Event.create( - action=Event.ACTION_AUTHORIZE_APPLICATION, - request=request, - app=application.name, - skipped_authorization=False) + Event.new(EventAction.AUTHORIZE_APPLICATION, + 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 b95eebdd5..09cfbbd18 100644 --- a/passbook/providers/saml/views.py +++ b/passbook/providers/saml/views.py @@ -13,7 +13,7 @@ from django.views.decorators.csrf import csrf_exempt from signxml.util import strip_pem_header from structlog import get_logger -from passbook.audit.models import Event +from passbook.audit.models import Event, EventAction from passbook.core.models import Application from passbook.lib.mixins import CSRFExemptMixin from passbook.lib.utils.template import render_to_string @@ -123,11 +123,9 @@ class LoginProcessView(AccessRequiredView): if self.provider.application.skip_authorization: ctx = self.provider.processor.generate_response() # Log Application Authorization - Event.create( - action=Event.ACTION_AUTHORIZE_APPLICATION, - request=request, - app=self.provider.application.name, - skipped_authorization=True) + Event.new(EventAction.AUTHORIZE_APPLICATION, + authorized_application=self.provider.application, + skipped_authorization=True).from_http(request) return RedirectToSPView.as_view()( request=request, acs_url=ctx['acs_url'], @@ -145,11 +143,9 @@ class LoginProcessView(AccessRequiredView): # Check if user has access if request.POST.get('ACSUrl', None): # User accepted request - Event.create( - action=Event.ACTION_AUTHORIZE_APPLICATION, - request=request, - app=self.provider.application.name, - skipped_authorization=False) + Event.new(EventAction.AUTHORIZE_APPLICATION, + authorized_application=self.provider.application, + skipped_authorization=False).from_http(request) return RedirectToSPView.as_view()( request=request, acs_url=request.POST.get('ACSUrl'), diff --git a/passbook/sources/oauth/views/core.py b/passbook/sources/oauth/views/core.py index e10ff67bf..12a575c81 100644 --- a/passbook/sources/oauth/views/core.py +++ b/passbook/sources/oauth/views/core.py @@ -11,8 +11,8 @@ from django.utils.translation import ugettext as _ from django.views.generic import RedirectView, View from structlog import get_logger +from passbook.audit.models import Event, EventAction from passbook.factors.view import AuthenticationView, _redirect_with_qs -from passbook.lib.utils.reflection import app from passbook.sources.oauth.clients import get_client from passbook.sources.oauth.models import (OAuthSource, UserOAuthSourceConnection) @@ -180,17 +180,8 @@ class OAuthCallback(OAuthClientMixin, View): access.user = user access.save() UserOAuthSourceConnection.objects.filter(pk=access.pk).update(user=user) - if app('passbook_audit'): - pass - # TODO: Create audit entry - # from passbook.audit.models import something - # something.event(user=user,) - # Event.create( - # user=user, - # message=_("Linked user with OAuth source %s" % self.source.name), - # request=self.request, - # hidden=True, - # current=False) + Event.new(EventAction.CUSTOM, message="Linked OAuth Source", + source=source).from_http(self.request) if was_authenticated: messages.success(self.request, _("Successfully linked %(source)s!" % { 'source': self.source.name