Repository navigation
Feature: Temporarily lock a local account after repeated incorrect password attempts #529
Replies: 1 comment
|
Thank you for the suggestion. I implemented the lock per account and client address. A lock per account alone would allow anyone who knows a username, including the administrator's, to keep that account locked with a few wrong guesses. After A locked account has a lock icon in User Management, and Unlock login in the Edit User dialog ends the lock. A correct password before the limit resets the count, and a new password set by an administrator or through a reset email ends the lock. IPv6 addresses are counted by their /64 network. As with the existing per-address limit, the client address is read from the reverse proxy's |
Uh oh!
There was an error while loading. Please reload this page.
Description
A wrong password on
/loginonly costs one request against the existing per-IP limit of 10 per minute. That limit resets every minute, and withoutRATELIMIT_STORAGE_URIeach gunicorn worker counts separately, so a patient client or several source addresses can try passwords against one account all day.The operator cannot stop that without blocking the IP, and cannot see or clear a locked account from the user list. An account with a local password has no failed-attempt counter and no lock time.
Affected Version
v0.10.17-alpha
Requested Feature
After five wrong passwords on one local account, further password sign-ins for that account fail for 60 minutes, including a later correct password. Both numbers are integer system settings,
login_max_failed_attempts(default 5) andlogin_lockout_minutes(default 60). Setting the attempt count to 0 turns this lock off. The login page keeps the flashloginFailed. It does not say that the account exists, that it is locked, or how long remains.The per-IP limit of 10 requests per minute on
/loginstays as it is. SSO sign-in, forgot-password, reset-password, andchange_passwordkeep their current responses. A correct password before the fifth failure signs in as today and clears the counter. A correct password on an unverified account still shows the check-email page and also clears the counter, because the password was right.While
login_locked_untilis in the future, the admin user list shows a small lock icon beside that username and the Edit User modal shows an Unlock button. Both are absent when the account is not locked, including when a few failures have not yet reached the maximum. The button sends the existing user update with onlyunlock_login: true. It does not submit the rest of the form. Saving a new password from that modal, or finishing the email reset, also clears the lock so the owner can sign in immediately.Proposed Implementation
login()insrc/api/auth.py:272-322is the only unauthenticated password check. The new branch belongs insideif user and user.passwordatsrc/api/auth.py:288-313. Unknown email (src/api/auth.py:312-313), an account with no local password (src/api/auth.py:310-311), and password login disabled for a non-admin (src/api/auth.py:290-291) stay on their current flashes and do not write the counter.change_passwordatsrc/api/auth.py:1057-1115already requires a session. SSOlogin_useratsrc/api/auth.py:427does not check a local password. Leave both alone.flowchart TD post[Valid login POST] lookup[Account with a local password] off[Attempts set to 0] hash[bcrypt] locked{locked_until in the future} ok{Hash matches} inc[SQL increment and commit] trip{Count reaches the maximum} setlock[Set locked_until and zero the count] clear[Zero the count and locked_until] session[Current path: email check or login_user] fail[Flash loginFailed] post --> lookup lookup --> off off -->|yes| session off -->|no| hash hash --> locked locked -->|yes| fail locked -->|no| ok ok -->|yes| clear --> session ok -->|no| inc --> trip trip -->|yes| setlock --> fail trip -->|no| failWhen the attempt setting is 0,
sessionis the current branch atsrc/api/auth.py:290-307, which still checks the password. When the account is locked, the hash still runs so the response is not faster than a wrong password, and the result is then discarded.Add
clear_login_lock(user)insrc/api/auth.py. It setsfailed_login_countto 0 andlogin_locked_untiltoNone. Call it from three places that already commit a password or a sign-in: a matching hash inlogin()before the email check atsrc/api/auth.py:294,reset_passwordwhere it assignsuser.passwordatsrc/api/auth.py:652, andadmin_update_userwhen it sets a password atsrc/api/admin.py:297-300or when the JSON containsunlock_login: true.On a mismatch, and only when the attempt setting is at least 1, increment in SQL so two workers cannot both read the same count:
Read
login_max_failed_attempts(default 5) andlogin_lockout_minutes(default 60) withSystemSetting.get_settingon that request.get_settingalready returns an int forsetting_type='integer'(src/models/system.py:45-47). A lock duration below 1 is treated as 60, so an empty value cannot lock the account forever. Further failures whilelogin_locked_untilis in the future do not add to the count and do not move the timestamp. When the timestamp is in the past, the next mismatch starts again from 1.Columns on
User, next to the password-reset fields atsrc/models/user.py:65-67:src/init_db.py:529addssession_versionwithadd_column_if_not_exists. Addfailed_login_countasINTEGER DEFAULT 0andlogin_locked_untilasDATETIMEin that same block.src/init_db.py:1008-1014seedsmax_file_size_mbonly when the row is missing. Seed the two integer settings the same way, with descriptions that say 0 attempts disables the lock and that the duration is minutes.POST /admin/settingsalready rejects a non-integer (src/api/admin.py:917-920). The settings modal already rendersinput type=numberfor an integer with no per-key widget (templates/admin.html:1596-1599). Keys absent fromsettingOrderalready sort to the end (templates/admin.html:2458-2460), so that array stays as it is.admin_get_usersbuilds each row atsrc/api/admin.py:171-188. Addlogin_locked, true only whenlogin_locked_untilis in the future. Do not add the raw count.admin_update_useratsrc/api/admin.py:266-277already requires a site admin (403 otherwise), loads the user (404 otherwise), and applies only the keys present in the JSON. Handleunlock_loginthere.src/api/admin.py:341only signs the admin back into their own session after they change their own password. It is not a password guess, so it does not record a failure.In
templates/admin.html:254, after the username, show<i class="fas fa-lock">whenuser.login_lockedis true, with:title="t('adminDashboard.loginLocked')". In the modal footer attemplates/admin.html:1454-1456, add atype="button"that is visible wheneditingUser.login_lockedis true, labeledt('adminDashboard.unlockLogin'). Its click handlerPUTs/admin/users/${id}with{"unlock_login": true}and the same CSRF headerupdateUseralready sends (templates/admin.html:2941-2956). On 200 it setslogin_lockedfalse on the open user and on the list row. It does not close the modal and does not send username, email, or password.Add
adminDashboard.loginLocked("Login locked") andadminDashboard.unlockLogin("Unlock login") besideupdateUserinstatic/locales/en.json:560, and the same two keys ines.json,fr.json,de.json,zh.json,ru.json, andpt-BR.json.Add a short "Login lock" section to
docs/admin-guide/system-settings.md, whose introduction at lines 1-7 is the page that lists instance-wide settings. Name both keys, the defaults, and that 0 attempts disables the lock. State that the login page text does not change.Tests, added beside the ones that already pass. Do not rewrite
test_login_wrong_password_no_session(tests/test_cov_auth.py:136-142) ortest_update_user_toggles_can_share(tests/test_cov_admin.py:286-292).test_login_fifth_failure_rejects_correct_passwordintests/test_cov_auth.py: five wrong passwords, then the right one, still 200 with no_user_id, andlogin_locked_untilset.test_login_fourth_success_clears_countintests/test_cov_auth.py: four wrong passwords, then the right one, 302, and both columns clear.test_login_after_lock_expiry_succeedsintests/test_cov_auth.py:login_locked_untilin the past, right password, 302, columns clear.test_login_lockout_off_when_max_attempts_zerointests/test_cov_auth.py: setting0, six wrong passwords, then the right one still signs in.test_admin_unlock_loginintests/test_cov_admin.py:PUT /admin/users/<id>with{"unlock_login": true}returns 200, both columns clear, andlogin_lockedonGET /admin/usersis false.test_admin_unlock_login_requires_site_adminintests/test_cov_admin.py: a signed-in non-admin receives 403 and the columns stay.Run them with:
Expected Behavior
loginFailedand no session, astest_login_wrong_password_no_sessionalready asserts (tests/test_cov_auth.py:136-142). The fifth wrong password does the same, and the next correct password does too, until the 60 minutes pass./loginwith_user_idset, astest_login_success_sets_session_and_redirectsdoes (tests/test_cov_auth.py:126-133), and the counter is 0.loginFailedanduseSsoLogin. Their rows gain no lock.GET /logindoes not increment anything.login_max_failed_attemptsat 0, wrong passwords never setlogin_locked_until./forgot-passwordstill accepts a request during a lock. Completingreset_passwordclears the columns in the same commit that stores the new hash (src/api/auth.py:652-662), and the following sign-in works.login_locked_untilis still in the future. The Unlock button is the same condition. After a successful unlockPUT, both disappear without a full save of the edit form.src/api/admin.py:268-269. The 10 per minute IP limit onlogin()(src/api/auth.py:273) still applies together with the account lock.change_passwordwith a wrong current password still flashescurrentPasswordIncorrect(src/api/auth.py:1113-1115) and does not changefailed_login_count.Root Cause Analysis
login()decides a password sign-in atsrc/api/auth.py:292-309. A mismatch flashesloginFailedand returns the form. Nothing onUserstores the failure: the model fields around the password stop at the reset token (src/models/user.py:20andsrc/models/user.py:65-67).The nearby control is
rate_limit("10 per minute")on that route (src/api/auth.py:81-104andsrc/api/auth.py:273). The limiter key is the client address (src/app.py:175-180). Its default storage is per process (src/app.py:584-592). That is a speed limit on one address, not a hold on one account. A window of "5 per hour" would also be the wrong shape: it starts at the first hit and forgets old hits, and a successful sign-in does not clear it.deduct_whenonly sees the finished response, and a wrong password and an unknown email both render the sameloginFailedpage (src/api/auth.py:309andsrc/api/auth.py:313), so the decorator cannot count real accounts only.admin_get_users(src/api/admin.py:171-188) has nowhere to read a lock from, andadmin_update_user(src/api/admin.py:297-300) can replace a password without clearing a counter that does not exist yet.Additional Context
No new dependency.
requirements.txt:4-7already pins Flask-Login 0.6.3, Flask-Bcrypt 1.0.1, and Flask-Limiter 4.1.1. Flask-Login stores the session. Flask-Limiter stays the IP limit. Flask-Security 5.8 can signal a failed authentication and ask whether a user is locked, but only on its own login endpoints, and the application still stores the counter. Adopting it would replace this login, the forms, the reset, and SSO.This request does not add a route, does not change login copy, does not show the remaining time or the failure count in the admin UI, does not lock SSO, and does not count failures from
change_password. The account row is shared by every worker, so the count stays exact withoutRATELIMIT_STORAGE_URI.All reactions