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

Login Captcha #6049

Closed
metiftikci opened this issue Feb 12, 2019 · 10 comments · Fixed by #21638
Closed

Login Captcha #6049

metiftikci opened this issue Feb 12, 2019 · 10 comments · Fixed by #21638
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/proposal The new feature has not been accepted yet but needs to be discussed first.
Milestone

Comments

@metiftikci
Copy link
Contributor

How about login captcha?

Force to solve captcha if user tried to login 3 times with wrong password.

@lunny lunny added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Feb 13, 2019
@techknowlogick techknowlogick added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Feb 19, 2019
@savavirtosu
Copy link

Here is a potential development plan for solving the current issue:

Add to User model a new column LoginAttempts.
Every time a user provides a wrong password on the sign in, the LoginAttempts is incremented.
Every time a user provides a valid password on the sign in, the LoginAttempts is set to 0.

If the LoginAttempts become bigger than X, in the sign in form a new captcha field will appear.

image

I think we could use the existing configuration flag in order to know if we should force captcha checking after X failed attempts.
Also, I think that the number of failed login attempts X could be hardcoded to 5 directly in code.

@lunny
Copy link
Member

lunny commented Oct 17, 2019

@savionok I think a new column in user table is unnecessary. But the LoginAttempts could be saved in session.

@guillep2k
Copy link
Member

@lunny if we are trying to stop a hack attempt by putting a captcha and reducing the odds that it's a robot attempt, we should not trust the session cookies, I believe. 🤔

@lunny
Copy link
Member

lunny commented Oct 17, 2019

@guillep2k Currently we allow one user login serval times, maybe one for your personal computer, one for working computer and one for your mobile. So store it on database one column will result in other things. If we want to do that, we needs a device management table. Of course we can do that for a long consideration.

We have depended on session cookies on login I think. Once you have a different cookie value when you logined, you will be logout.

@guillep2k
Copy link
Member

@lunny Captchas are normally meant to tell humans from robots that can be doing a dictionary attack. If that's not our intention, why use a captcha at all? Robots will certainly not honor session cookies. 👾

Any cookies kept in the real user's computers should not be affected in any way. If they were valid, they should remain valid. No problems there.

As I understand it, the LoginAttempts column is actually meant to count login failures, not attempts (it should be named LoginFailures instead). It shouldn't affect the normal workflow of the real user, even if they use fifteen computers. If the real user logs in from another computer when LoginAttempts is maxed out, the login failure count will simply be reset to zero, even if the "robot" is failing repeatedly, but that would only give the robot three more tries. It may require the user to pass the captcha, but that's very good: knowing that someone else is trying to hack our account! 😱

Tools like RSS readers or even git's HTTP protocol will not be affected by this because they don't use the login form.

@lafriks
Copy link
Member

lafriks commented Oct 17, 2019

I agree @guillep2k this can be global per user and reset to zero on successful authorization

@lafriks
Copy link
Member

lafriks commented Oct 17, 2019

We should probably also keep last failed authorization timestamp so that we can discard failure count after x minutes has passed

@jaqra jaqra mentioned this issue Oct 28, 2019
1 task
@GAS85
Copy link

GAS85 commented Jul 25, 2022

What about this nice feature? Seems really useful. I would add, that admin should be able to configure login attempts to 0 that should drive to show captcha on each login attempt.

How about not existing users attempts?
That should be kind of option to track Session, IP and show Captcha to those user.

@hiqsociety
Copy link

i prefer a captcha just for login.
i mean to login, solve captcha.
when can this be implemented?

zeripath added a commit that referenced this issue Nov 22, 2022
Enable this to require captcha validation for user login. You also must
enable `ENABLE_CAPTCHA`.

Summary:
- Consolidate CAPTCHA template
- add CAPTCHA handle and context
- add `REQUIRE_CAPTCHA_FOR_LOGIN` config and docs
- Consolidate CAPTCHA set-up and verification code 

Partially resolved #6049 

Signed-off-by: Xinyu Zhou <i@sourcehut.net>
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: Andrew Thornton <art27@cantab.net>
@zeripath zeripath reopened this Nov 22, 2022
@lunny
Copy link
Member

lunny commented Nov 23, 2022

Closed as #21906 opened.

@lunny lunny closed this as completed Nov 23, 2022
@lunny lunny added this to the 1.19.0 milestone Nov 23, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! type/proposal The new feature has not been accepted yet but needs to be discussed first.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants