fix(throttling): Introduce proper throttling for PhoneDevice and display proper message

jazzband / django-two-factor-auth

Complete Two-Factor Authentication for Django providing the easiest integration into most Django projects.

MIT License

1.71k stars 448 forks source link

fix(throttling): Introduce proper throttling for PhoneDevice and display proper message #626

Closed mlec1 closed 1 year ago

mlec1 commented 1 year ago

Description

This pull requests implement the proper throttling for PhoneDevice as described in this issue. My MR is updating this MR and fixing the issues: it is passing all the tests.

Additionnaly, it is adding the proper error message instead of a general "Invalid token" as described in this issue

Motivation and Context

Refs:

How Has This Been Tested?

Ran make test and tox. The test cases needs a new pip package freezegun used to change the datetime of the tests at wish.

Types of changes

[x] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

[x] My code follows the code style of this project.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[x] I have added tests to cover my changes.
[x] All new and existing tests passed.

claudep commented 1 year ago

Maybe a rebase will re-trigger tests? Could you try?

codecov[bot] commented 1 year ago

Codecov Report

Merging #626 (8f2bdd7) into master (19e02f5) will decrease coverage by 0.09%. Report is 1 commits behind head on master. The diff coverage is 95.16%.

@@            Coverage Diff             @@
##           master     #626      +/-   ##
==========================================
- Coverage   95.47%   95.38%   -0.09%     
==========================================
  Files          75       75              
  Lines        3204     3252      +48     
  Branches      359      372      +13     
==========================================
+ Hits         3059     3102      +43     
- Misses        116      119       +3     
- Partials       29       31       +2

Files Changed	Coverage Δ
two_factor/forms.py	`98.94% <80.00%> (-1.06%)`	:arrow_down:
two_factor/plugins/phonenumber/models.py	`92.85% <87.50%> (-7.15%)`	:arrow_down:
tests/settings.py	`100.00% <100.00%> (ø)`
tests/test_views_login.py	`100.00% <100.00%> (ø)`
tests/test_views_phone.py	`100.00% <100.00%> (ø)`

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

mlec1 commented 1 year ago

I have done a rebase. Concerning the squash commits, I fucked up and push an additional one with all the commits squashed together. For me the easiest would be to enable squash commit for pull request https://docs.github.com/en/repositories/configuring-branches-and-merges-in-your-repository/configuring-pull-request-merges/configuring-commit-squashing-for-pull-requests