Skip to content
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

core: expiring model: don't synchronously delete #11688

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions authentik/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -802,12 +802,25 @@
return self.delete(*args, **kwargs)

@classmethod
def filter_not_expired(cls, **kwargs) -> QuerySet["Token"]:
def _not_expired_filter(cls):
return Q(expires__gt=now(), expiring=True) | Q(expiring=False)

@classmethod
def filter_not_expired(cls, delete_expired=False, **kwargs) -> QuerySet["ExpiringModel"]:
"""Filer for tokens which are not expired yet or are not expiring,
and match filters in `kwargs`"""
for obj in cls.objects.filter(**kwargs).filter(Q(expires__lt=now(), expiring=True)):
obj.delete()
return cls.objects.filter(**kwargs)
if delete_expired:
cls.delete_expired(**kwargs)
return cls.objects.filter(cls._not_expired_filter()).filter(**kwargs)

@classmethod
def delete_expired(cls, **kwargs) -> int:
objects = cls.objects.all().exclude(cls._not_expired_filter()).filter(**kwargs)
amount = 0
for obj in objects:
obj.expire_action()
amount += 1

Check warning on line 822 in authentik/core/models.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/models.py#L821-L822

Added lines #L821 - L822 were not covered by tests
return amount

@property
def is_expired(self) -> bool:
Expand Down
7 changes: 1 addition & 6 deletions authentik/core/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,7 @@
messages = []
for cls in ExpiringModel.__subclasses__():
cls: ExpiringModel
objects = (
cls.objects.all().exclude(expiring=False).exclude(expiring=True, expires__gt=now())
)
amount = objects.count()
for obj in objects:
obj.expire_action()
amount = cls.delete_expired()

Check warning on line 33 in authentik/core/tasks.py

View check run for this annotation

Codecov / codecov/patch

authentik/core/tasks.py#L33

Added line #L33 was not covered by tests
LOGGER.debug("Expired models", model=cls, amount=amount)
messages.append(f"Expired {amount} {cls._meta.verbose_name_plural}")
# Special case
Expand Down
30 changes: 13 additions & 17 deletions authentik/outposts/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from dacite.core import from_dict
from django.contrib.auth.models import Permission
from django.core.cache import cache
from django.db import IntegrityError, models, transaction
from django.db import models, transaction
from django.db.models.base import Model
from django.utils.translation import gettext_lazy as _
from guardian.models import UserObjectPermission
Expand Down Expand Up @@ -380,26 +380,22 @@ def token(self) -> Token:
"""Get/create token for auto-generated user"""
managed = f"goauthentik.io/outpost/{self.token_identifier}"
tokens = Token.filter_not_expired(
delete_expired=True,
identifier=self.token_identifier,
intent=TokenIntents.INTENT_API,
managed=managed,
)
if tokens.exists():
return tokens.first()
try:
return Token.objects.create(
user=self.user,
identifier=self.token_identifier,
intent=TokenIntents.INTENT_API,
description=f"Autogenerated by authentik for Outpost {self.name}",
expiring=False,
managed=managed,
)
except IntegrityError:
# Integrity error happens mostly when managed is reused
Token.objects.filter(managed=managed).delete()
Token.objects.filter(identifier=self.token_identifier).delete()
return self.token
token: Token | None = tokens.first()
if token:
return token
return Token.objects.create(
user=self.user,
identifier=self.token_identifier,
intent=TokenIntents.INTENT_API,
description=f"Autogenerated by authentik for Outpost {self.name}",
expiring=False,
managed=managed,
)

def get_required_objects(self) -> Iterable[models.Model | str]:
"""Get an iterator of all objects the user needs read access to"""
Expand Down
3 changes: 2 additions & 1 deletion authentik/stages/consent/stage.py
Original file line number Diff line number Diff line change
Expand Up @@ -96,8 +96,9 @@ def get(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
if PLAN_CONTEXT_PENDING_USER in self.executor.plan.context:
user = self.executor.plan.context[PLAN_CONTEXT_PENDING_USER]

# Remove expired consents to prevent database unique constraints errors
consent: UserConsent | None = UserConsent.filter_not_expired(
user=user, application=application
delete_expired=True, user=user, application=application
).first()
self.executor.plan.context[PLAN_CONTEXT_CONSENT] = consent

Expand Down
Loading