Page MenuHomePhabricator

Add a garbage collector for MFA challenges
ClosedPublic

Authored by epriestley on Dec 14 2018, 1:32 PM.
Tags
None
Referenced Files
F13243855: D19888.diff
Thu, May 23, 4:17 AM
F13222311: D19888.diff
Sun, May 19, 3:30 AM
F13204914: D19888.diff
Wed, May 15, 1:14 AM
F13188141: D19888.diff
Sat, May 11, 5:03 AM
Unknown Object (File)
Tue, May 7, 11:15 AM
Unknown Object (File)
Tue, May 7, 8:42 AM
Unknown Object (File)
Tue, May 7, 8:15 AM
Unknown Object (File)
Fri, May 3, 7:58 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.