From 66c22d2e9f5bf6bc5843b7bd7170ac275e20b1e9 Mon Sep 17 00:00:00 2001 From: Rebecca Graber Date: Tue, 18 Apr 2023 13:17:53 -0400 Subject: [PATCH] feat: add grace period when deleting accesstokens (#32040) --- .../management/commands/edx_clear_expired_tokens.py | 10 +++++----- .../commands/tests/test_clear_expired_tokens.py | 7 +++++-- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/openedx/core/djangoapps/oauth_dispatch/management/commands/edx_clear_expired_tokens.py b/openedx/core/djangoapps/oauth_dispatch/management/commands/edx_clear_expired_tokens.py index 60da129567..de587a89de 100644 --- a/openedx/core/djangoapps/oauth_dispatch/management/commands/edx_clear_expired_tokens.py +++ b/openedx/core/djangoapps/oauth_dispatch/management/commands/edx_clear_expired_tokens.py @@ -89,27 +89,27 @@ class Command(BaseCommand): # lint-amnesty, pylint: disable=missing-class-docst excluded_application_ids = [] now = timezone.now() - refresh_expire_at = self.get_expiration_time(now) + expiry_threshold = self.get_expiration_time(now) if options['refresh-tokens']: logger.info("Removing expired RefreshTokens") - query_set = RefreshToken.objects.filter(access_token__expires__lt=refresh_expire_at).exclude( + query_set = RefreshToken.objects.filter(access_token__expires__lt=expiry_threshold).exclude( application_id__in=excluded_application_ids) self.clear_table_data(query_set, batch_size, RefreshToken, sleep_time) if options['revoked-tokens']: logger.info("Removing revoked RefreshTokens") # remove revoked, as opposed to expired, RefreshTokens - revoked = RefreshToken.objects.filter(revoked__lt=refresh_expire_at).exclude( + revoked = RefreshToken.objects.filter(revoked__lt=expiry_threshold).exclude( application_id__in=excluded_application_ids) self.clear_table_data(revoked, batch_size, RefreshToken, sleep_time) if options['access-tokens']: logger.info("Removing expired AccessTokens") - query_set = AccessToken.objects.filter(refresh_token__isnull=True, expires__lt=now) + query_set = AccessToken.objects.filter(refresh_token__isnull=True, expires__lt=expiry_threshold) self.clear_table_data(query_set, batch_size, AccessToken, sleep_time) if options['grants']: logger.info("Removing expired Grants") - query_set = Grant.objects.filter(expires__lt=now) + query_set = Grant.objects.filter(expires__lt=expiry_threshold) self.clear_table_data(query_set, batch_size, Grant, sleep_time) diff --git a/openedx/core/djangoapps/oauth_dispatch/management/commands/tests/test_clear_expired_tokens.py b/openedx/core/djangoapps/oauth_dispatch/management/commands/tests/test_clear_expired_tokens.py index 15982ac8de..5e428947b5 100644 --- a/openedx/core/djangoapps/oauth_dispatch/management/commands/tests/test_clear_expired_tokens.py +++ b/openedx/core/djangoapps/oauth_dispatch/management/commands/tests/test_clear_expired_tokens.py @@ -102,18 +102,21 @@ class EdxClearExpiredTokensTests(TestCase): # lint-amnesty, pylint: disable=mis initial_count = 5 now = timezone.now() expires = now - timedelta(days=1) + refresh_expire_cutoff = now - timedelta(seconds=3600) users = UserFactory.create_batch(initial_count) for user in users: application = factories.ApplicationFactory(user=user) factories.AccessTokenFactory(user=user, application=application, expires=expires) - assert AccessToken.objects.filter(refresh_token__isnull=True, expires__lt=now).count() == initial_count + assert AccessToken.objects.filter(refresh_token__isnull=True, expires__lt=refresh_expire_cutoff).count()\ + == initial_count original_delete = QuerySet.delete QuerySet.delete = counter(QuerySet.delete) try: call_command('edx_clear_expired_tokens', batch_size=1, sleep_time=0) # four being the number of tables we'll end up unnecessarily calling .delete on once assert QuerySet.delete.invocations == initial_count + 4 # pylint: disable=no-member - assert AccessToken.objects.filter(refresh_token__isnull=True, expires__lt=now).count() == 0 + assert AccessToken.objects.filter(refresh_token__isnull=True, + expires__lt=refresh_expire_cutoff).count() == 0 finally: QuerySet.delete = original_delete