Skip to content

Commit ab85970

Browse files
authored
Merge pull request #194 from CodeForPhilly/193
Improved login robustness, removed non-JSON login, added dummy pw hash checks for malformed username and pw.
2 parents 46b7c40 + bdcb537 commit ab85970

File tree

1 file changed

+19
-37
lines changed

1 file changed

+19
-37
lines changed

src/server/api/user_api.py

Lines changed: 19 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -67,50 +67,30 @@ def user_test_fail():
6767
return jsonify("Here's your failure"), 401
6868

6969

70-
@user_api.route("/api/user/login", methods=["POST"])
71-
def user_login():
72-
""" Validate user in db, return JWT if legit and active.
73-
Expects non-json form data
74-
"""
75-
76-
with engine.connect() as connection:
77-
78-
pwhash = None
79-
s = text(
80-
"""select password, pdp_user_roles.role, active
81-
from pdp_users
82-
left join pdp_user_roles on pdp_users.role = pdp_user_roles._id
83-
where username=:u """
84-
)
85-
s = s.bindparams(u=request.form["username"])
86-
result = connection.execute(s)
87-
88-
if result.rowcount: # Did we get a match on username?
89-
pwhash, role, is_active = result.fetchone()
90-
else:
91-
log_user_action(request.form["username"], "Failure", "Invalid username")
92-
return jsonify("Bad credentials"), 401
93-
94-
if is_active.lower() == "y" and check_password(request.form["password"], pwhash):
95-
# Yes, user is active and password matches
96-
token = jwt_ops.create_token(request.form["username"], role)
97-
log_user_action(request.form["username"], "Success", "Logged in")
98-
return token
99-
100-
else:
101-
log_user_action(request.form["username"], "Failure", "Bad password or inactive")
102-
return jsonify("Bad credentials"), 401
103-
10470

10571
@user_api.route("/api/user/login_json", methods=["POST"])
10672
def user_login_json():
10773
""" Validate user in db, return JWT if legit and active.
10874
Expects json-encoded form data
10975
"""
11076

111-
post_dict = json.loads(request.data)
112-
username = post_dict["username"]
113-
presentedpw = post_dict["password"]
77+
def dummy_check():
78+
"""Perform a fake password hash check to take as much time as a real one."""
79+
pw_bytes = bytes('password', "utf8")
80+
check_password('password', pw_bytes)
81+
82+
try:
83+
post_dict = json.loads(request.data)
84+
username = post_dict["username"]
85+
presentedpw = post_dict["password"]
86+
except:
87+
dummy_check() # Take the same time as with well-formed requests
88+
return jsonify("Bad credentials"), 401
89+
90+
if not (isinstance(username, str) and isinstance(presentedpw, str) ):
91+
dummy_check() # Take the same time as with well-formed requests
92+
return jsonify("Bad credentials"), 401 # Don't give us ints, arrays, etc.
93+
11494

11595
with engine.connect() as connection:
11696

@@ -128,6 +108,7 @@ def user_login_json():
128108
pwhash, role, is_active = result.fetchone()
129109
else:
130110
log_user_action(username, "Failure", "Invalid username")
111+
dummy_check()
131112
return jsonify("Bad credentials"), 401
132113

133114
if is_active.lower() == "y" and check_password(presentedpw, pwhash):
@@ -138,6 +119,7 @@ def user_login_json():
138119

139120
else:
140121
log_user_action(username, "Failure", "Bad password or inactive")
122+
# No dummy_check needed as we ran a real one to get here
141123
return jsonify("Bad credentials"), 401
142124

143125

0 commit comments

Comments
 (0)