Added individual attempt expiry feature for #1302 by kuldeepkhatke · Pull Request #1313 · jazzband/django-axes

kuldeepkhatke · 2025-05-27T19:37:28Z

Individual attempt based longest expiry attempt restriction feature

Implemented a AXES_INDIVIDUAL_ATTEMPT_EXPIRY flag based feature,
such that if we enable it then it will handle attempts cleanup based on the longest expiry of the attempt.
Hence added expire_at field to implement this feature.

All testcases passed after implementation.

Fixes #1302

aleksihakli · 2025-06-06T16:47:05Z

Comparing to #1310 this adds state to the database which is only used if the flag is switched on, otherwise growing the database table size and seldom using the expiration times. I would separate the new expiration time field into it's own table if it's not used often, and only write it to the database when necessary.

Individual attempt based longest expiry attempt restriction feature

Implemented a AXES_INDIVIDUAL_ATTEMPT_EXPIRY flag based feature, such that if we enable it then it will handle attempts cleanup based on the longest expiry of the attempt. Hence added expire_at field to implement this feature.

All testcases passed after implementation.

Fixes #1302

Existing test cases pass, but there are no test cases for the new flag / feature, which need to be added if this is to be merged upstream.

kuldeepkhatke

I'm modifying the changes as per suggestions, & will add test cases accordingly.

kuldeepkhatke · 2025-06-07T17:12:05Z

Hey @aleksihakli ,
I've made the required changes as per suggestion.

Added testcases for the feature.

Please review the changes.

kuldeepkhatke · 2025-06-07T17:22:15Z

Also, i'm curious if we can consider adding a AUTHORS file to add the name of list of contributors

aleksihakli

A few notes here and there to polish the implementation up, looks good otherwise 👍

codecov · 2025-06-07T18:23:14Z

Codecov Report

Attention: Patch coverage is 55.81395% with 19 lines in your changes missing coverage. Please review.

Project coverage is 92.17%. Comparing base (3ff5ada) to head (5a3a53c).
Report is 17 commits behind head on master.

Files with missing lines	Patch %	Lines
axes/attempts.py	12.50%	7 Missing ⚠️
axes/handlers/database.py	56.25%	6 Missing and 1 partial ⚠️
axes/admin.py	28.57%	4 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1313      +/-   ##
==========================================
- Coverage   93.55%   92.17%   -1.38%     
==========================================
  Files          36       37       +1     
  Lines        1179     1214      +35     
  Branches      158      165       +7     
==========================================
+ Hits         1103     1119      +16     
- Misses         57       74      +17     
- Partials       19       21       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

aleksihakli · 2025-06-07T18:26:13Z

Hey @aleksihakli , I've made the required changes as per suggestion.

Added testcases for the feature.

Please review the changes.

Looks good, nice job 👍 I made a few suggestions for further improvements but I think this is close to merge ready.

…ATION

…et_attempt_expiration()

kuldeepkhatke · 2025-06-08T09:40:53Z

Hi @aleksihakli ,
Updated the code as per suggestions, rebased with latest code.

Thanks

aleksihakli · 2025-06-09T20:31:31Z

                    )

+                if settings.AXES_USE_ATTEMPT_EXPIRATION:
+                    if not hasattr(attempt, "expiration") or attempt.expiration is None:


Maybe just create the object if it doesn't exist?

Is the expiration timestamp ever earlier than it previously was? Why should it only be updated to a later value if the get_attempt_expiration returns an earlier value?

Suggested change

if not hasattr(attempt, "expiration") or attempt.expiration is None:

if settings.AXES_USE_ATTEMPT_EXPIRATION:

expiration, created = AccessAttemptExpiration.objects.update_or_create(

access_attempt=attempt,

defaults={

"expires_at": get_attempt_expiration(request),

}

)

if created:

log.debug("AXES: Created new AccessAttemptExpiration for %s", client_str)

else:

log.debug("AXES: Updated existing AccessAttemptExpiration for %s", client_str)

Yes, it can be earlier as explaind in the initial communication of the issue also #1302

What if some kept cool_off 60 mins in first time and then changed to 1 min.
Then as per initial discussions we shold hold it till longest expiration.
Thats what this feature is meant to be.

If someone changes the expiration then we can just use the new expiration value, I don't think there's much value added in adding dynamic logic for calculating maximum expiration values - if the user is specializing the expiration times they can calculate them as they see fit.

consider the below example with a twist like UserA is only tried before with 60min cool_off & later with 5min cool_off.

I think its good to have flow.

…iration logic simplify

blockchainGuru1018 · 2025-06-16T13:38:54Z

approved

aleksihakli · 2025-06-18T14:00:43Z

                    )

+                if settings.AXES_USE_ATTEMPT_EXPIRATION:
+                    if not hasattr(attempt, "expiration") or attempt.expiration is None:


If someone changes the expiration then we can just use the new expiration value, I don't think there's much value added in adding dynamic logic for calculating maximum expiration values - if the user is specializing the expiration times they can calculate them as they see fit.

aleksihakli · 2025-06-18T14:11:29Z

+    if settings.AXES_USE_ATTEMPT_EXPIRATION:
+        list_display = (
+            "attempt_time",
+            "expiration",


Maybe just always display this field and avoid the if-else logic?

Always display may lead to confution of the field usecase, if exsting user don't aware about this new flag.

May be we can re-write it like below.

Suggested change

"expiration",

list_display = [

"attempt_time",

"ip_address",

"user_agent",

"username",

"path_info",

"failures_since_start",

]

if settings.AXES_USE_ATTEMPT_EXPIRATION:

list_display.append('expiration')

aleksihakli · 2025-06-18T14:11:43Z

    def has_add_permission(self, request: HttpRequest) -> bool:
        return False

+    def expiration(self, obj: AccessAttempt):


I'm wondering if this should be just part of the modeladmin as an inlines definition, if those happen to work with One-to-One models at the moment?

But i don't think we have any editable scope of this field at the moment.
so, doesn't benefit much to show separately ?

aleksihakli · 2025-06-18T14:16:21Z

        return f"P{days_str}T{time_str}"
    return f"P{days_str}"

+def get_attempt_expiration(request: Optional[HttpRequest] = None) -> datetime:


This duplicates existing code at

https://github.com/jazzband/django-axes/blob/master/axes/attempts.py#L12

This function have separate usecase than get_cool_off_threshold() function.

get_attempt_expiration() get call at the time of attempt executed.

while get_cool_off_threshold() get called inside the clean_expired_user_attempts() function.

Also get_attempt_expiration() works on adding the cool_off time

while get_cool_off_threshold() works on subtracting the cool_off time.

That's true, I was wondering if they should be co-located in the same file since they have very similar logic.

aleksihakli · 2025-07-05T13:19:16Z

Sorry for the slow merge on this one, I haven't had the opportunity to bake a new release in the last couple of weeks. Merging now 👍 Thank you for the effort and PR @kuldeepkhatke

kuldeepkhatke · 2025-07-06T09:55:26Z

Absolutely, No Problem @aleksihakli ,

Thanks for appreciation & merging my PR.

aleksihakli reviewed Jun 6, 2025

View reviewed changes

Comment thread axes/conf.py Outdated

aleksihakli reviewed Jun 6, 2025

View reviewed changes

Comment thread axes/migrations/0010_accessattempt_expires_at.py Outdated

aleksihakli mentioned this pull request Jun 6, 2025

User-dependant cool-off can reset attempts related to other users #1302

Closed

kuldeepkhatke commented Jun 6, 2025

View reviewed changes

kuldeepkhatke force-pushed the individual-attempt-expiry branch from b66023d to 5a3a53c Compare June 7, 2025 17:08

aleksihakli requested changes Jun 7, 2025

View reviewed changes

kuldeepkhatke added 9 commits June 8, 2025 14:36

Added individual attempt expiry feature

5b2a483

Renamed AXES_INDIVIDUAL_ATTEMPT_EXPIRY flag to AXES_USE_ATTEMPT_EXPIR…

21e1636

…ATION

Shifted epired_at filed to new model

d5e4268

Added unittest for AXES_USE_ATTEMPT_EXPIRATION flag

cdf911d

Modified verbiage changes & removed comment

fc60c44

Modified access_attempt as pk

1a293ff

Added actual queryset call UT

9efedb2

Modified expiration create queryset logic

8cac081

Updated get_individual_attempt_expiry() func placement & renamed to g…

dcdb83b

…et_attempt_expiration()

kuldeepkhatke force-pushed the individual-attempt-expiry branch from 9258277 to dcdb83b Compare June 8, 2025 09:06

kuldeepkhatke requested a review from aleksihakli June 8, 2025 13:40

aleksihakli requested changes Jun 9, 2025

View reviewed changes

kuldeepkhatke added 2 commits June 10, 2025 20:33

Updated expires_at for null, blank False, lte query update, admin exp…

717ec51

…iration logic simplify

Added separate UT for complete, partial & no delete senarios

5794c88

kuldeepkhatke requested a review from aleksihakli June 10, 2025 15:08

blockchainGuru1018 approved these changes Jun 16, 2025

View reviewed changes

aleksihakli requested changes Jun 18, 2025

View reviewed changes

Reverted , remove change from AxesDatabaseHandler.user_login_failed

6378589

aleksihakli merged commit 392dfa0 into jazzband:master Jul 5, 2025
17 checks passed

aleksihakli mentioned this pull request Jul 5, 2025

Locked out status feature #1322

Merged

-                    if not hasattr(attempt, "expiration") or attempt.expiration is None:
+                if settings.AXES_USE_ATTEMPT_EXPIRATION:
+                    expiration, created = AccessAttemptExpiration.objects.update_or_create(
+                            access_attempt=attempt,
+                            defaults={
+                                "expires_at": get_attempt_expiration(request),
+                            }
+                        )
+                    if created:
+                        log.debug("AXES: Created new AccessAttemptExpiration for %s", client_str)
+                    else:
+                        log.debug("AXES: Updated existing AccessAttemptExpiration for %s", client_str)

-            "expiration",
+    list_display = [
+        "attempt_time",
+        "ip_address",
+        "user_agent",
+        "username",
+        "path_info",
+        "failures_since_start",
+    ]
+    if settings.AXES_USE_ATTEMPT_EXPIRATION:
+        list_display.append('expiration')

Uh oh!

Conversation

kuldeepkhatke commented May 27, 2025

Individual attempt based longest expiry attempt restriction feature

Uh oh!

aleksihakli commented Jun 6, 2025

Individual attempt based longest expiry attempt restriction feature

Uh oh!

Uh oh!

Uh oh!

kuldeepkhatke left a comment

Choose a reason for hiding this comment

Uh oh!

kuldeepkhatke commented Jun 7, 2025

Uh oh!

kuldeepkhatke commented Jun 7, 2025

Uh oh!

aleksihakli left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Jun 7, 2025

Codecov Report

Uh oh!

aleksihakli commented Jun 7, 2025

Uh oh!

kuldeepkhatke commented Jun 8, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blockchainGuru1018 commented Jun 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aleksihakli commented Jul 5, 2025

Uh oh!

Uh oh!

kuldeepkhatke commented Jul 6, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!