[#2816] Split user/password auth from token verification

[swrichards]

Previously, the UserModelEmailBackend was overloaded to support both username/password authentication, as well as TOTP validation. It did this, in part, by verifying the 2FA flag on the site configuration.

This was both confusing (an authentication backend should do one thing only, multiple logics means multiple backends), as well as mixing concerns (the view should decide which arguments to pass to to authentication backend based on the site configuration, authentication backends should only authenticate).

This commit separates both concerns into independent backends, and adds some tests to ensure that they are properly invoked.

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 94.61%. Comparing base (0ee4d7d) to head (d9a06c0).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1449      +/-   ##
===========================================
+ Coverage    94.59%   94.61%   +0.02%     
===========================================
  Files         1071     1071              
  Lines        39704    39794      +90     
===========================================
+ Hits         37557    37653      +96     
+ Misses        2147     2141       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pi-sigma]

Looking good, couple stylistic suggestiions

[pi-sigma]

Suggested change

	cls.make_token = lambda: totp(cls.user.seed, period=cls.expires_in)
	cls.token = lambda: totp(cls.user.seed, period=cls.expires_in)

The value for this will be the same for all tests, so it's a bit misleading to call it make_token (which to me suggests that a new token is made every time this is called)

[swrichards]

The totp function does use the current timestamp, and not all tests freeze the time (or in the same way), so I thought it'd still be easier to reason about if it's a callable. But I will add a freeze_time to the whole class, because time-based tests should be deterministic.

[alextreme]

This doesn't seem to check config.login_2fa_sms anymore? This is a PR we should discuss on monday, I may be overlooking something on how this works

[swrichards]

This doesn't seem to check config.login_2fa_sms anymore? This is a PR we should discuss on monday, I may be overlooking something on how this works

That was intentional -- these are authentication backends, their task should be solely to accept some kind of credentials and return a user upon successful validation. Whether or not a token is required is a view-level concern, where the actual session is created if the right conditions are met (in this case, the username/password view doesn't actually create a session, it sends a token if the credentials are a valid, and the token verification view creates the session. I added a test to validate that behavior).

The previous code was doing too much in two senses: it was validating both a token and a username/password, and it's behavior changed depending on the config, which is a controller/view level concern.

[alextreme]

Discussed and change is okay after tests pass