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

Better TOTP Management #51

Open
mkalioby opened this issue Jun 24, 2021 · 3 comments
Open

Better TOTP Management #51

mkalioby opened this issue Jun 24, 2021 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@mkalioby
Copy link
Owner

TOTP shall be immue against repeat acts and against brute force acts.

@mkalioby mkalioby self-assigned this Jun 24, 2021
@mkalioby mkalioby added the enhancement New feature or request label Jun 24, 2021
@mkalioby
Copy link
Owner Author

@xi, I handled these 2 scenarios in branch Better_TOTP, can you please try it and let me know.

Thanks.

@xi
Copy link
Contributor

xi commented Jun 25, 2021

I had a quick look at the code. Here are some thoughts:

  • Looks good in general
  • I like that it is self contained, even though I would not have an issue with delegating some of the functionality to django-axes.
  • I think those same counter measures could also be useful for other methods, e.g. email.
  • If I understand correctly this code would store successful login attempts indefinitely. This might be a data protection issue in some legislations. It is also not really required as TOTPs expire quickly. On top of that, the risk of blocking a valid TOTP increases with each successful login.
  • I know from experience with django-axes that users tend to enter the wrong credentials, so an admin UI that allows to review and reset login attempts is crucial.

@mkalioby
Copy link
Owner Author

I had a quick look at the code. Here are some thoughts:

  • Looks good in general
  • I like that it is self contained, even though I would not have an issue with delegating some of the functionality to django-axes.

I don't have an experience in that and I think it will be good to integrate it under a flag to help admins manage through one system.

  • I think those same counter measures could also be useful for other methods, e.g. email.

Good point.

  • If I understand correctly this code would store successful login attempts indefinitely. This might be a data protection issue in some legislations. It is also not really required as TOTPs expire quickly. On top of that, the risk of blocking a valid TOTP increases with each successful login.

We can create a management command that clear records older than the predefined window.

  • I know from experience with django-axes that users tend to enter the wrong credentials, so an admin UI that allows to review and reset login attempts is crucial.

The code doesn't suspend the users for good, it just suspends the usage of TOTP for some time (cool off period) based onMFA_TOTP_FAILURE_WINDOW setting, so the users can try another method or wait till the suspension is lifted automatically.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants