From aa701c5725a8824e078ae0ffc386c43a074068bd Mon Sep 17 00:00:00 2001 From: Jens Langhammer Date: Wed, 14 Jul 2021 21:47:32 +0200 Subject: [PATCH] core: don't delete expired tokens, rotate their key Signed-off-by: Jens Langhammer --- authentik/core/models.py | 19 +++++++ authentik/core/tasks.py | 10 ++-- authentik/core/tests/test_tasks.py | 18 ------- authentik/core/tests/test_token_api.py | 11 +++++ .../migrations/0017_alter_event_action.py | 47 ++++++++++++++++++ authentik/events/models.py | 1 + .../0018_alter_eventmatcherpolicy_action.py | 49 +++++++++++++++++++ schema.yml | 1 + 8 files changed, 134 insertions(+), 22 deletions(-) delete mode 100644 authentik/core/tests/test_tasks.py create mode 100644 authentik/events/migrations/0017_alter_event_action.py create mode 100644 authentik/policies/event_matcher/migrations/0018_alter_eventmatcherpolicy_action.py diff --git a/authentik/core/models.py b/authentik/core/models.py index b17831582..dd24cbd76 100644 --- a/authentik/core/models.py +++ b/authentik/core/models.py @@ -381,6 +381,13 @@ class ExpiringModel(models.Model): expires = models.DateTimeField(default=default_token_duration) expiring = models.BooleanField(default=True) + def expire_action(self, *args, **kwargs): + """Handler which is called when this object is expired. By + default the object is deleted. This is less efficient compared + to bulk deleting objects, but classes like Token() need to change + values instead of being deleted.""" + return self.delete(*args, **kwargs) + @classmethod def filter_not_expired(cls, **kwargs) -> QuerySet: """Filer for tokens which are not expired yet or are not expiring, @@ -425,6 +432,18 @@ class Token(ManagedModel, ExpiringModel): user = models.ForeignKey("User", on_delete=models.CASCADE, related_name="+") description = models.TextField(default="", blank=True) + def expire_action(self, *args, **kwargs): + """Handler which is called when this object is expired.""" + from authentik.events.models import Event, EventAction + + self.key = default_token_key() + self.save(*args, **kwargs) + Event.new( + action=EventAction.SECRET_ROTATE, + token=self, + message=f"Token {self.identifier}'s secret was rotated.", + ).save() + def __str__(self): description = f"{self.identifier}" if self.expiring: diff --git a/authentik/core/tasks.py b/authentik/core/tasks.py index 8a2f2d372..2e32f3d9e 100644 --- a/authentik/core/tasks.py +++ b/authentik/core/tasks.py @@ -26,14 +26,16 @@ def clean_expired_models(self: MonitoredTask): messages = [] for cls in ExpiringModel.__subclasses__(): cls: ExpiringModel - amount, _ = ( + objects = ( cls.objects.all() .exclude(expiring=False) .exclude(expiring=True, expires__gt=now()) - .delete() ) - LOGGER.debug("Deleted expired models", model=cls, amount=amount) - messages.append(f"Deleted {amount} expired {cls._meta.verbose_name_plural}") + for obj in objects: + obj.expire_action() + amount = objects.count() + LOGGER.debug("Expired models", model=cls, amount=amount) + messages.append(f"Expired {amount} {cls._meta.verbose_name_plural}") self.set_status(TaskResult(TaskResultStatus.SUCCESSFUL, messages)) diff --git a/authentik/core/tests/test_tasks.py b/authentik/core/tests/test_tasks.py deleted file mode 100644 index ff19843cc..000000000 --- a/authentik/core/tests/test_tasks.py +++ /dev/null @@ -1,18 +0,0 @@ -"""authentik core task tests""" -from django.test import TestCase -from django.utils.timezone import now -from guardian.shortcuts import get_anonymous_user - -from authentik.core.models import Token -from authentik.core.tasks import clean_expired_models - - -class TestTasks(TestCase): - """Test Tasks""" - - def test_token_cleanup(self): - """Test Token cleanup task""" - Token.objects.create(expires=now(), user=get_anonymous_user()) - self.assertEqual(Token.objects.all().count(), 1) - clean_expired_models.delay().get() - self.assertEqual(Token.objects.all().count(), 0) diff --git a/authentik/core/tests/test_token_api.py b/authentik/core/tests/test_token_api.py index ff9cd48cd..74295ab43 100644 --- a/authentik/core/tests/test_token_api.py +++ b/authentik/core/tests/test_token_api.py @@ -1,5 +1,7 @@ """Test token API""" from django.urls.base import reverse +from django.utils.timezone import now +from guardian.shortcuts import get_anonymous_user from rest_framework.test import APITestCase from authentik.core.models import ( @@ -8,6 +10,7 @@ from authentik.core.models import ( TokenIntents, User, ) +from authentik.core.tasks import clean_expired_models class TestTokenAPI(APITestCase): @@ -41,3 +44,11 @@ class TestTokenAPI(APITestCase): self.assertEqual(token.user, self.user) self.assertEqual(token.intent, TokenIntents.INTENT_API) self.assertEqual(token.expiring, False) + + def test_token_expire(self): + """Test Token expire task""" + token: Token = Token.objects.create(expires=now(), user=get_anonymous_user()) + key = token.key + clean_expired_models.delay().get() + token.refresh_from_db() + self.assertNotEqual(key, token.key) diff --git a/authentik/events/migrations/0017_alter_event_action.py b/authentik/events/migrations/0017_alter_event_action.py new file mode 100644 index 000000000..b385bc603 --- /dev/null +++ b/authentik/events/migrations/0017_alter_event_action.py @@ -0,0 +1,47 @@ +# Generated by Django 3.2.5 on 2021-07-14 19:15 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_events", "0016_add_tenant"), + ] + + operations = [ + migrations.AlterField( + model_name="event", + name="action", + field=models.TextField( + choices=[ + ("login", "Login"), + ("login_failed", "Login Failed"), + ("logout", "Logout"), + ("user_write", "User Write"), + ("suspicious_request", "Suspicious Request"), + ("password_set", "Password Set"), + ("secret_view", "Secret View"), + ("secret_rotate", "Secret Rotate"), + ("invitation_used", "Invite Used"), + ("authorize_application", "Authorize Application"), + ("source_linked", "Source Linked"), + ("impersonation_started", "Impersonation Started"), + ("impersonation_ended", "Impersonation Ended"), + ("policy_execution", "Policy Execution"), + ("policy_exception", "Policy Exception"), + ("property_mapping_exception", "Property Mapping Exception"), + ("system_task_execution", "System Task Execution"), + ("system_task_exception", "System Task Exception"), + ("system_exception", "System Exception"), + ("configuration_error", "Configuration Error"), + ("model_created", "Model Created"), + ("model_updated", "Model Updated"), + ("model_deleted", "Model Deleted"), + ("email_sent", "Email Sent"), + ("update_available", "Update Available"), + ("custom_", "Custom Prefix"), + ] + ), + ), + ] diff --git a/authentik/events/models.py b/authentik/events/models.py index 0364e5dd4..e52330d46 100644 --- a/authentik/events/models.py +++ b/authentik/events/models.py @@ -62,6 +62,7 @@ class EventAction(models.TextChoices): PASSWORD_SET = "password_set" # noqa # nosec SECRET_VIEW = "secret_view" # noqa # nosec + SECRET_ROTATE = "secret_rotate" # noqa # nosec INVITE_USED = "invitation_used" diff --git a/authentik/policies/event_matcher/migrations/0018_alter_eventmatcherpolicy_action.py b/authentik/policies/event_matcher/migrations/0018_alter_eventmatcherpolicy_action.py new file mode 100644 index 000000000..e2c767c56 --- /dev/null +++ b/authentik/policies/event_matcher/migrations/0018_alter_eventmatcherpolicy_action.py @@ -0,0 +1,49 @@ +# Generated by Django 3.2.5 on 2021-07-14 19:15 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("authentik_policies_event_matcher", "0017_alter_eventmatcherpolicy_action"), + ] + + operations = [ + migrations.AlterField( + model_name="eventmatcherpolicy", + name="action", + field=models.TextField( + blank=True, + choices=[ + ("login", "Login"), + ("login_failed", "Login Failed"), + ("logout", "Logout"), + ("user_write", "User Write"), + ("suspicious_request", "Suspicious Request"), + ("password_set", "Password Set"), + ("secret_view", "Secret View"), + ("secret_rotate", "Secret Rotate"), + ("invitation_used", "Invite Used"), + ("authorize_application", "Authorize Application"), + ("source_linked", "Source Linked"), + ("impersonation_started", "Impersonation Started"), + ("impersonation_ended", "Impersonation Ended"), + ("policy_execution", "Policy Execution"), + ("policy_exception", "Policy Exception"), + ("property_mapping_exception", "Property Mapping Exception"), + ("system_task_execution", "System Task Execution"), + ("system_task_exception", "System Task Exception"), + ("system_exception", "System Exception"), + ("configuration_error", "Configuration Error"), + ("model_created", "Model Created"), + ("model_updated", "Model Updated"), + ("model_deleted", "Model Deleted"), + ("email_sent", "Email Sent"), + ("update_available", "Update Available"), + ("custom_", "Custom Prefix"), + ], + help_text="Match created events with this action type. When left empty, all action types will be matched.", + ), + ), + ] diff --git a/schema.yml b/schema.yml index bf267a654..d5112a15c 100644 --- a/schema.yml +++ b/schema.yml @@ -19441,6 +19441,7 @@ components: - suspicious_request - password_set - secret_view + - secret_rotate - invitation_used - authorize_application - source_linked