Page MenuHomePhabricator

Add a garbage collector for MFA challenges
ClosedPublic

Authored by epriestley on Dec 14 2018, 1:32 PM.

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Dec 14 2018, 1:32 PM
epriestley requested review of this revision.Dec 14 2018, 1:34 PM
amckinley accepted this revision.Dec 14 2018, 8:14 PM

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.