From 25e4207563e99c4b9d5bd035932e8d7d94000fc2 Mon Sep 17 00:00:00 2001 From: Deimos Date: Fri, 8 May 2020 18:22:17 -0600 Subject: [PATCH] Login: show whether username or password was wrong I get a fair number of "forgot password" emails where the person is actually trying to log in with the wrong username. Normally, a login system shouldn't display whether the username or password was the incorrect part, but since it's already public information which usernames exist on Tildes (simply by visiting /user/), this really isn't meaningfully hiding anything. It would only have any effect on the most absolutely naive attackers. I think it's an acceptable trade-off to help out people that are inadvertently trying to log in with the wrong username instead. --- tildes/tildes/views/login.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tildes/tildes/views/login.py b/tildes/tildes/views/login.py index 63b30a4..7c330f0 100644 --- a/tildes/tildes/views/login.py +++ b/tildes/tildes/views/login.py @@ -81,16 +81,37 @@ def post_login( .one_or_none() ) - # If that user doesn't exist or the password was wrong, error out - if not user or not user.is_correct_password(password): + # If the username doesn't exist, tell them so - usually this isn't considered a good + # practice, but it's completely trivial to check if a username exists on Tildes + # anyway (by visiting /user/), so it's better to just let people know if + # they're trying to log in with the wrong username + if not user: incr_counter("login_failures") # log the failure - need to manually commit because of the exception - log_entry = Log(LogEventType.USER_LOG_IN_FAIL, request, {"username": username}) + log_entry = Log( + LogEventType.USER_LOG_IN_FAIL, + request, + {"username": username, "reason": "Nonexistent username"}, + ) + request.db_session.add(log_entry) + request.tm.commit() + + raise HTTPUnprocessableEntity("That username does not exist") + + if not user.is_correct_password(password): + incr_counter("login_failures") + + # log the failure - need to manually commit because of the exception + log_entry = Log( + LogEventType.USER_LOG_IN_FAIL, + request, + {"username": username, "reason": "Incorrect password"}, + ) request.db_session.add(log_entry) request.tm.commit() - raise HTTPUnprocessableEntity("Incorrect username or password") + raise HTTPUnprocessableEntity("Incorrect password") # Don't allow banned users to log in if user.is_banned: