Uh oh!
There was an error while loading. Please reload this page.
Event for failed login attempts - #19845
Conversation
ChristophWurst
left a comment
There was a problem hiding this comment.
New class needs @since annotations, but the rest seems fine :)
blizzz
left a comment
There was a problem hiding this comment.
terms are being mixed up here :(
Please use
- loginName, when it is about the value that is entered as name in the login form
- userId, or short uid, when the value exactly and without doubt matches the user id of the user
- username, rather don't
5d00e5d to
5135888Comparerullzer
commented
Mar 30, 2020
/compile amend / |
| 'preLoginNameUsedAsUserName', | ||
| ['uid' => &$uid] | ||
| ); | ||
| $this->dispatcher->dispatchTyped(new LoginFailedEvent($uid)); |
There was a problem hiding this comment.
possible doing a userExists checks against the uid, and only emitting it against existing users? of course it could be done by the consumer, too.
There was a problem hiding this comment.
I think it's to good to be clear in what is actually passed in the arguments, to avoid surprises.
There was a problem hiding this comment.
Problem is that this still doesn't give the right UID in all cases. Normal backend is case insensitive etc.
I mean in the password policy app we could always do a 'userExist' check.
There was a problem hiding this comment.
It should give the correct uid when the user exists, and leave it unchanged otherwise. But let me double check.
There might be a use case if you try to make fail2ban thing on NC level, when you don't care about an existing user behind a login name (just, it's on the wrong level, you have fail2ban already). But then, we should leave the preLoginNameUsedAsUserName to the consumer, too, as it is not necessary for this hypothetic use case.
So I'd say, when we start with preLoginNameUsedAsUserName, let's do the full thing and only emit when an existing user account is behind that, instead of a half hearted approach.
Update: committed
There was a problem hiding this comment.
Normal backend is case insensitive etc.
It returns the uid in the correct from
| 'preLoginNameUsedAsUserName', | ||
| ['uid' => &$uid] | ||
| ); | ||
| $this->dispatcher->dispatchTyped(new LoginFailedEvent($user)); |
5135888 to
826e243CompareUh oh!
There was an error while loading. Please reload this page.
ChristophWurst
commented
Mar 31, 2020
|
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
9906270 to
84f3d2dCompareRef nextcloud/server#19845 Ref nextcloud/server#19890 Ref nextcloud/server#19075 Ref nextcloud/server#19670 Ref nextcloud/server#19412 Ref nextcloud/server#14722 Ref nextcloud/server#19746 Ref nextcloud/server#19321 Ref nextcloud/server#19569 Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
Ref #1951 Ref nextcloud/server#19845 Ref nextcloud/server#19075 Ref nextcloud/server#14722 Ref nextcloud/server#19321 Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
First start of a failed login event.
So we can act on it in other parts.