Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[stable14] Bruteforce protection handling in combination with #12160

Merged
merged 3 commits into from Nov 1, 2018

Conversation

@Dagefoerde
Copy link
Member

@Dagefoerde Dagefoerde commented Oct 31, 2018

Backports

  • Reset bruteforce on token refresh OAuth #12130
  • Expired tokens should not trigger bruteforce protection #12140

Cheers!

Signed-off-by: Jan C. Dageförde jan.dagefoerde@ercis.uni-muenster.de

rullzer added 3 commits Oct 29, 2018
When using atoken obtained via OAuth the token expires. Resulting in
brute force attempts hitting the requesting IP.

This resets the brute force attempts for that UID on a valid refresh of
the token.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
Fixes #12131

If we hit an expired token there is no need to continue checking. Since
we know it is a token.

We also should not register this with the bruteforce throttler as it is
actually a valid token. Just expired. Instead the authentication should
fail. And buisness continues as usual.

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer
Copy link
Member

@rullzer rullzer commented Oct 31, 2018

@Dagefoerde mind to backport to 13 as well?

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Code change looks good 👍

@MorrisJobke MorrisJobke merged commit d1d82fc into nextcloud:stable14 Nov 1, 2018
1 check passed
1 check passed
continuous-integration/drone/pr the build was successful
Details
@Dagefoerde
Copy link
Member Author

@Dagefoerde Dagefoerde commented Nov 1, 2018

@Dagefoerde mind to backport to 13 as well?

generally, sure :)

in this case, I had better not. Sorry. Rebasing your second (cleanup) commit gives funny conflicts

DU lib/private/Authentication/Token/Manager.php
DU lib/private/Authentication/Token/PublicKeyTokenProvider.php
[...]
DU tests/lib/Authentication/Token/PublicKeyTokenProviderTest.php

... and since I'm not familiar with (the rest of) Nextcloud authentication, I'd rather leave it up to you. Not sure what the consequences would be. It's not really necessary for Moodle, though. I'll advise 14.0.4 as the minimum Nextcloud version anyway ;)

@MorrisJobke MorrisJobke mentioned this pull request Nov 13, 2018
@MorrisJobke MorrisJobke mentioned this pull request Nov 22, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants