Page MenuHomePhabricator

Don't rate limit users clicking "Wait Patiently" at an MFA gate even if they typed some text earlier
ClosedPublic

Authored by epriestley on Jan 23 2019, 3:12 PM.
Tags
None
Referenced Files
F12834637: D20018.id47789.diff
Thu, Mar 28, 2:43 PM
Unknown Object (File)
Sat, Mar 23, 5:47 PM
Unknown Object (File)
Sat, Mar 23, 5:47 PM
Unknown Object (File)
Sat, Mar 23, 5:47 PM
Unknown Object (File)
Sat, Mar 23, 5:46 PM
Unknown Object (File)
Sat, Mar 23, 5:46 PM
Unknown Object (File)
Thu, Feb 29, 4:51 PM
Unknown Object (File)
Feb 19 2024, 5:40 PM
Subscribers
None

Details

Summary

Depends on D20017. Ref T13222. Currently, if you:

  • type some text at a TOTP gate;
  • wait ~60 seconds for the challenge to expire;
  • submit the form into a "Wait patiently" message; and
  • mash that wait button over and over again very patiently

...you still rack up rate limiting points, because the hidden text from your original request is preserved and triggers the "is the user responding to a challenge" test. Only perform this test if we haven't already decided that we're going to make them wait.

Test Plan
  • Did the above; before patch: rate limited; after patch: not rate limited.
  • Intentionally typed a bunch of bad answers which were actually evaluated: rate limited properly.

Diff Detail

Repository
rP Phabricator
Branch
mfa13
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21643
Build 29513: Run Core Tests
Build 29512: arc lint + arc unit

Event Timeline

epriestley created this revision.
  • Wordsmithing.
src/applications/auth/engine/PhabricatorAuthSessionEngine.php
579–583

We're doing the same check here, this change primarily makes the two tests consistent.

(These loops could possibly be combined; I'll take a look if I end up refactoring here for SMS.)

This revision is now accepted and ready to land.Jan 23 2019, 6:53 PM
This revision was automatically updated to reflect the committed changes.