Stop password reset email spamming

[MorrisJobke]

• downstream of Stop password reset email spamming owncloud/core#27346

cc @nickvergessen

[mention-bot]

@MorrisJobke, thanks for your PR! By analyzing the history of the files in this pull request, we identified @LukasReschke, @rullzer and @BernhardPosselt to be potential reviewers.

[nickvergessen]

Should we just use our bruteforce protection?

[MorrisJobke]

Should we just use our bruteforce protection?

I guess that will not help, because this is more about "Only send the reset email once in x minutes" and does not follow the "make it slower after x failed retries"

[MorrisJobke]

I tested this and it works fine 👍

[MorrisJobke]

@nickvergessen @LukasReschke @rullzer @blizzz Please review :)

[blizzz]

There were 2 warnings:
1) Test\Authentication\Token\DefaultTokenProviderTest::testRenewSessionTokenWithoutPassword
Trying to configure method "getRemember" which cannot be configured because it does not exist, has not been specified, is final, or is static
2) Test\Authentication\Token\DefaultTokenProviderTest::testRenewSessionTokenWithPassword
Trying to configure method "getRemember" which cannot be configured because it does not exist, has not been specified, is final, or is static
--
There were 4 failures:
1) Tests\Core\Controller\LostControllerTest::testSpamEmail
Failed asserting that Array &0 (
    'status' => 'error'
    'msg' => 'Expectation failed for method name is equal to <string:getUserValue> when invoked 1 time(s)
Parameter 0 for invocation OCP\IConfig::getUserValue(Mock_IUser_fa903a8d Object (...), 'core', 'lostpassword', '') does not match expected value.
Mock_IUser_fa903a8d Object (...) does not match expected type "string".'
) is identical to Array &0 (
    'status' => 'error'
    'msg' => 'The email is not sent because a password reset email was sent recently.'
).
/drone/src/github.com/nextcloud/server/tests/Core/Controller/LostControllerTest.php:309
2) Tests\Core\Controller\LostControllerTest::testEmailSuccessful
Failed asserting that Array &0 (
    'status' => 'error'
    'msg' => 'Expectation failed for method name is equal to <string:getUserValue> when invoked 1 time(s)
Parameter 0 for invocation OCP\IConfig::getUserValue(Mock_IUser_fa903a8d Object (...), 'core', 'lostpassword', '') does not match expected value.
Mock_IUser_fa903a8d Object (...) does not match expected type "string".'
) is identical to Array &0 (
    'status' => 'success'
).
/drone/src/github.com/nextcloud/server/tests/Core/Controller/LostControllerTest.php:390
3) Tests\Core\Controller\LostControllerTest::testEmailWithMailSuccessful
Failed asserting that Array &0 (
    'status' => 'error'
    'msg' => 'Expectation failed for method name is equal to <string:setUserValue> when invoked 1 time(s)
Parameter 0 for invocation OCP\IConfig::setUserValue(Mock_IUser_fa903a8d Object (...), 'core', 'lostpassword', 'encryptedToken', null) does not match expected value.
Mock_IUser_fa903a8d Object (...) does not match expected type "string".'
) is identical to Array &0 (
    'status' => 'success'
).
/drone/src/github.com/nextcloud/server/tests/Core/Controller/LostControllerTest.php:461
4) Tests\Core\Controller\LostControllerTest::testEmailCantSendException
Failed asserting that Array &0 (
    'status' => 'error'
    'msg' => 'Expectation failed for method name is equal to <string:setUserValue> when invoked 1 time(s)
Parameter 0 for invocation OCP\IConfig::setUserValue(Mock_IUser_fa903a8d Object (...), 'core', 'lostpassword', 'encryptedToken', null) does not match expected value.
Mock_IUser_fa903a8d Object (...) does not match expected type "string".'
) is identical to Array &0 (
    'status' => 'error'
    'msg' => 'Couldn't send reset email. Please contact your administrator.'
).
/drone/src/github.com/nextcloud/server/tests/Core/Controller/LostControllerTest.php:527

[nickvergessen]

I guess that will not help, because this is more about "Only send the reset email once in x minutes" and does not follow the "make it slower after x failed retries"

With our new API we can do that at any time, not only failed attempts.
The advantage is, that it is a "by attacker" blocking instead of a "by victim".

[schiessle]

I would also prefer to use the brute force protection. If I deleted the mail by accident I don't want to wait 5 minutes until I can ask for another one. If people try to spam me, brute-force protection will help (not after the first mail but after a few). That's what it is for.

Also for the user it is completely in-transparent because he doesn't know if he need to wait 5 minutes, 10 minutes, 1 hour, 1 day. Does the counter starts again from zero after any attempt?

[nickvergessen]

The method now has @BruteForceProtection(action=passwordResetEmail) since #4336

I think this is enough, otherwise we can also add a AnonRateLimit of 60secs, if you think it's not enough?