Page MenuHomePhabricator

Add a garbage collector for MFA challenges
ClosedPublic

Authored by epriestley on Dec 14 2018, 1:32 PM.
Tags
None
Referenced Files
F15502317: D19888.id47499.diff
Sun, Apr 13, 11:43 PM
F15500823: D19888.id47490.diff
Sun, Apr 13, 7:28 PM
F15493071: D19888.id.diff
Sat, Apr 12, 9:18 PM
F15490116: D19888.diff
Fri, Apr 11, 2:25 PM
F15490091: D19888.id.diff
Fri, Apr 11, 2:13 PM
F15415371: D19888.id.diff
Mar 20 2025, 5:39 AM
F15415323: D19888.diff
Mar 20 2025, 5:24 AM
F15399040: D19888.id47499.diff
Mar 17 2025, 2:34 AM
Subscribers
None

Details

Summary

Depends on D19886. Ref T13222. Clean up MFA challenges after they expire.

(There's maybe some argument to keeping these around for a little while for debugging/forensics, but I suspect it would never actually be valuable and figure we can cross that bridge if we come to it.)

Test Plan
  • Ran bin/garbage collect --collector ... and saw old MFA challenges collected.
  • Triggered a new challenge, GC'd again, saw it survive GC while still active.

Diff Detail

Repository
rP Phabricator
Branch
mfa8
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21341
Build 29043: Run Core Tests
Build 29042: arc lint + arc unit

Event Timeline

I don't want to go down a bike shedding path here, but I feel like keeping challenges for a ~week would put us in a better position the first time someone wants to debug this stuff.

This revision is now accepted and ready to land.Dec 14 2018, 8:14 PM

Yeah -- I initially kept it for a week, but then I was like "it would be better to make that week configurable since it's kind of weird to hard-code it and there's support to make it configurable...", but that was kind of a bit more code and we'd end up with a mild mess removing it later since the configurable part gets stored in Config. I'd also guess there's a real possibility that we never actually look at this table to debug anything.

The GC normally only runs every ~4 hours so if someone reports a reproducible thing we'll probably be able to grab it even without a + 7 days thing as long as we're pretty quick about it and it's not a total phantom ghost thing.

This revision was automatically updated to reflect the committed changes.