Implement rate limiting for e-mail verifications

[LawnGnome]

This applies a burst of 3 and a refill time of 30 minutes by default per user. (Not that I can imagine a scenario where we'd ever override this for a user, but using the same machinery as other user actions is obviously much simpler.)

I don't love that this ends up essentially prop-drilling the rate limiter into a bunch of new places, but I don't see an alternative other than prop-drilling the whole app struct through, which would be worse.

This doesn't address (sorry) per-address rate limiting, but is at least a reasonable starting point.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 89.61039% with 16 lines in your changes are missing coverage. Please review.

Project coverage is 87.63%. Comparing base (61c0870) to head (27637ae).

Files	Patch %	Lines
src/controllers/user/me.rs	74.07%	7 Missing :warning:
src/controllers/user/session.rs	41.66%	7 Missing :warning:
src/rate_limiter.rs	94.87%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #8419      +/-   ##
==========================================
+ Coverage   87.62%   87.63%   +0.01%     
==========================================
  Files         272      272              
  Lines       26333    26426      +93     
==========================================
+ Hits        23073    23158      +85     
- Misses       3260     3268       +8     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[Turbo87]

I don't love that this ends up essentially prop-drilling the rate limiter into a bunch of new places, but I don't see an alternative other than prop-drilling the whole app struct through, which would be worse.

I think the root cause of this is that the model layer is currently sending emails, which IMHO is a bit questionable, especially because we're using this code path in a couple of places in the test suite too. It might be better to decouple these things first before introducing the rate limiting.

[LawnGnome]

I think the root cause of this is that the model layer is currently sending emails, which IMHO is a bit questionable, especially because we're using this code path in a couple of places in the test suite too. It might be better to decouple these things first before introducing the rate limiting.

Yeah, I went back and forth on this yesterday — decoupling the e-mail sending out of the model layer is better from an architectural perspective, but it means the NewUser::create_or_update API is easier to misuse as it stands, since it now comes with an implicit need to figure out if the e-mail address needs verification after and action that.

The e-mail upsert code could be extracted into something that returns that state, but being in the same transaction as the user upsert is obviously a nice quality that would be bad to lose.

I'm sorta wondering if we end up with something like this (treat names as placeholders):

let new_user = NewUser::new(...);
let updater = new_user.create_or_update(&conn)?;  // returns some intermediate type holding a transaction
let verification_required = updater.set_email(&email)?;  // needs to be optional since not all updates might touch the e-mail?
updater.commit()?;  // consumes updater

// Has to be after the commit;
if verification_required {
  // send verification e-mail
}

Another, simpler option would obviously be to return something else from create_or_update than just the User — a struct enclosing the User along with a bool field indicating if verification is required.

Now I've typed this out, I probably have a slight preference for the second option (return something enclosing User and whatever other state we need), but I'm interested in what you think.

[bors]

:umbrella: The latest upstream changes (presumably 77d55e926feec96fc852a3f7966b3954f06255c5) made this pull request unmergeable. Please resolve the merge conflicts.