sfe: Add per IP rate limits to the overrides request form submissions #8366
Conversation
|
@beautifulentropy, this PR appears to contain configuration and/or SQL schema changes. Please ensure that a corresponding deployment ticket has been filed with the new values. |
beautifulentropy
changed the title
Skipping these tickets until we're ready to launch the new Web UI, then we'll bundle all of the changes into a single request. |
beautifulentropy
force-pushed
the
sfe-form-building
branch
from
905e0e8 to
cf3871f
Compare
beautifulentropy
dismissed
jprenken’s stale review
The base branch was changed.
beautifulentropy
force-pushed
the
sfe-override-request-form-rate-limit
branch
from
6ab6fa7 to
013cc6d
Compare
There was a problem hiding this comment.
Have you talked with SRE about how they'll want to load defaults into the SFE? My naive guess would be that they'd rather put this default in the extant defaults file, rather than managing another new file. But I certainly don't know for sure.
| // TODO(#8359): Check per-IP rate limits for this endpoint. | ||
| var refundLimits func() | ||
| var submissionSuccess bool | ||
| if sfe.limiter != nil && sfe.txnBuilder != nil { |
There was a problem hiding this comment.
The addition of all this new (nested) code has made me realize how fragile this handler function is. It, like many of our WFE methods, has eight bajillion pairs of http.Something followed by an immediate return, and if any of those returns are missing the behavior will quickly go off the rails.
Not necessarily a thing to be fixed now, but a thing to be aware of for sure. This pattern is already bad in the WFE. Let's avoid perpetuating it in the SFE if we can.
Prevent callers from blasting our Zendesk with junk requests and (eventually) our database with automatically approved overrides.
Closes #8359