diff --git a/authentik/policies/signals.py b/authentik/policies/signals.py index 7b806eeacc..6a86c8ad77 100644 --- a/authentik/policies/signals.py +++ b/authentik/policies/signals.py @@ -29,8 +29,18 @@ def monitoring_set_policies(sender, **kwargs): @receiver(post_save, sender=PolicyBindingModel) @receiver(post_save, sender=Group) @receiver(post_save, sender=User) -def invalidate_policy_cache(sender, instance, **_): - """Invalidate Policy cache when policy is updated""" +def invalidate_policy_cache(sender, instance, update_fields=None, **_): + """Invalidate Policy cache when policy is updated. + + Skips when the save touched only ``last_login`` — Django's auth flow runs + ``user.save(update_fields=["last_login"])`` on every successful login, and + the broad invalidation below would otherwise issue a full-table cache + scan on every login. ``last_login`` doesn't affect policy evaluation or + application access, so there's nothing to invalidate. + """ + if sender == User and update_fields and set(update_fields) <= {"last_login"}: + return + if sender == Policy: total = 0 for binding in PolicyBinding.objects.filter(policy=instance): diff --git a/authentik/policies/tests/test_signals.py b/authentik/policies/tests/test_signals.py new file mode 100644 index 0000000000..4081d7994a --- /dev/null +++ b/authentik/policies/tests/test_signals.py @@ -0,0 +1,96 @@ +"""Tests for ``authentik.policies.signals.invalidate_policy_cache``. + +Regression guards for the per-login cache-invalidation skip. ``last_login``- +only User saves must not trigger the broad ``cache.keys(...)`` invalidation. +""" + +from unittest import TestCase, mock + +from authentik.core.models import User +from authentik.policies import signals +from authentik.policies.models import Policy + + +class _FakeUser: + pk = 1 + + +class _FakePolicy: + pk = "fake-policy-pk" + + +class TestInvalidatePolicyCacheSkipsLastLoginOnly(TestCase): + """The handler must NOT touch the cache when a User save is purely a + ``last_login`` update.""" + + def _run_handler(self, sender, instance, update_fields): + """Run the handler with mocked cache/PolicyBinding; return the + cache mock for assertions.""" + + with ( + mock.patch.object(signals, "cache") as mock_cache, + mock.patch.object(signals, "PolicyBinding") as mock_pb, + ): + mock_cache.keys.return_value = [] + mock_pb.objects.filter.return_value = [] + signals.invalidate_policy_cache( + sender=sender, + instance=instance, + update_fields=update_fields, + ) + return mock_cache + + def test_user_save_with_only_last_login_does_not_invalidate(self): + """User save with update_fields=["last_login"] is the per-login hot + path. The handler must short-circuit without touching the cache.""" + mock_cache = self._run_handler( + sender=User, instance=_FakeUser(), update_fields=["last_login"] + ) + mock_cache.keys.assert_not_called() + mock_cache.delete_many.assert_not_called() + + def test_user_save_with_last_login_as_set_does_not_invalidate(self): + """``update_fields`` may be a set (Django supports any iterable). + The handler must treat ``{"last_login"}`` identically to + ``["last_login"]``.""" + mock_cache = self._run_handler( + sender=User, instance=_FakeUser(), update_fields={"last_login"} + ) + mock_cache.keys.assert_not_called() + + def test_user_save_with_other_fields_still_invalidates(self): + """A User save that updates ``email`` (or any non-last_login field) + must still invalidate the cache — those updates can affect policy + evaluation, group membership computation, etc.""" + mock_cache = self._run_handler(sender=User, instance=_FakeUser(), update_fields=["email"]) + mock_cache.keys.assert_called() + mock_cache.delete_many.assert_called() + + def test_user_save_with_last_login_plus_other_fields_invalidates(self): + """If ``update_fields`` contains ``last_login`` plus anything else, + we must invalidate — the other field could have policy implications.""" + mock_cache = self._run_handler( + sender=User, + instance=_FakeUser(), + update_fields=["last_login", "email"], + ) + mock_cache.keys.assert_called() + + def test_user_save_without_update_fields_invalidates(self): + """``update_fields=None`` means a full save — anything could have + changed, so we conservatively invalidate.""" + mock_cache = self._run_handler(sender=User, instance=_FakeUser(), update_fields=None) + mock_cache.keys.assert_called() + + def test_policy_save_still_invalidates(self): + """Non-User senders are unaffected by the new short-circuit. + Policy/PolicyBinding/PolicyBindingModel/Group saves must continue to + invalidate as before — those changes affect access decisions for + every user.""" + mock_cache = self._run_handler( + sender=Policy, + instance=_FakePolicy(), + update_fields=["last_login"], # irrelevant — sender isn't User + ) + mock_cache.keys.assert_called() + mock_cache.delete_many.assert_called()