Skip to content

Commit a8ee7fb

Browse files
committed
Fix pw check logic in accountreq and useradm which was previously inadvertently
inverted. Rename var to pw_match for consistence. Fix string corruption in pickled MiG-user.db fixture file to allow proper use of password_hash values. Refactor the accountreq unit test module to split into peers testing and filter testing with shared init of users in the latter, too. Relies on the pickled user db from fixtures now. Add test coverage on account renewal for pw check and ID collision. Still acouple of TODOs with test coverage of suspended accounts and the a password to match the pickled hash.
1 parent 946da5d commit a8ee7fb

File tree

3 files changed

+173
-116
lines changed

3 files changed

+173
-116
lines changed

mig/shared/accountreq.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1240,18 +1240,21 @@ def early_validation_checks(configuration, raw_request, service, username,
12401240
authorized = raw_request.get('authorized', None)
12411241
reset_token = raw_request.get('reset_token', None)
12421242
if hashed:
1243-
raw_request['pw_changed'] = check_hash(configuration, service,
1244-
username, password, hashed)
1243+
raw_request['pw_match'] = check_hash(configuration, service,
1244+
username, password, hashed)
12451245
elif scrambled:
1246-
raw_request['pw_changed'] = check_scramble(
1246+
raw_request['pw_match'] = check_scramble(
12471247
configuration, service, username, password, scrambled,
12481248
salt=configuration.site_password_salt)
1249+
else:
1250+
logger.debug('account for %r has no passwords set' % client_id)
1251+
raw_request['pw_match'] = True
12491252

12501253
if account_status not in ('temporal', 'active', 'inactive'):
12511254
logger.warning('existing account for %r is %s and not renewable'
12521255
% (client_id, account_status))
12531256
raw_request['invalid'].append(renewal_blocked)
1254-
if raw_request['pw_changed'] and not authorized and not reset_token:
1257+
if not raw_request['pw_match'] and not authorized and not reset_token:
12551258
logger.warning('illegal password change in request from %r' %
12561259
client_id)
12571260
raw_request['invalid'].append(illegal_pw_change)

mig/shared/useradm.py

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ def verify_user_peers(configuration, db_path, client_id, user, now, verify_peer,
459459

460460

461461
def create_user_in_db(configuration, db_path, client_id, user, now, authorized,
462-
reset_token, reset_auth_type, pw_changed,
462+
reset_token, reset_auth_type, pw_match,
463463
accepted_peer_list, force, verbose, ask_renew,
464464
default_renew, do_lock, from_edit_user, ask_change_pw,
465465
auto_create_db, create_backup):
@@ -571,12 +571,12 @@ def create_user_in_db(configuration, db_path, client_id, user, now, authorized,
571571
# existing password should be left alone on renewal.
572572
# The password_hash field is not guaranteed to exist and it varies
573573
# depending on the current random seed and assigned salt. So we use
574-
# check_password during submit and save result in pw_changed.
574+
# check_password during submit and save result in pw_match.
575575
if not user['password']:
576576
user['password'] = user['old_password']
577577
if not user.get('password_hash', ''):
578578
user['password_hash'] = user['old_password_hash']
579-
if pw_changed:
579+
if not pw_match:
580580
# Allow password change if it's directly authorized with login
581581
# or authorized through a simple reset challenge (reset_token).
582582
if not authorized and reset_token:
@@ -1062,11 +1062,11 @@ def create_user(user, conf_path, db_path, force=False, verbose=False,
10621062
reset_auth_type = user.get('auth', ['UNKNOWN'])[-1]
10631063
# Always remove any reset_token fields before DB insert
10641064
del user['reset_token']
1065-
pw_changed = False
1066-
if 'pw_changed' in user:
1067-
pw_changed = user['pw_changed']
1068-
# Always remove any pw_changed fields before DB insert
1069-
del user['pw_changed']
1065+
pw_match = True
1066+
if 'pw_match' in user:
1067+
pw_match = user['pw_match']
1068+
# Always remove any pw_match fields before DB insert
1069+
del user['pw_match']
10701070

10711071
_logger.info('trying to create or renew user %r' % client_id)
10721072
if verbose:
@@ -1095,7 +1095,7 @@ def create_user(user, conf_path, db_path, force=False, verbose=False,
10951095

10961096
created = create_user_in_db(configuration, db_path, client_id, user, now,
10971097
authorized, reset_token, reset_auth_type,
1098-
pw_changed, accepted_peer_list, force, verbose,
1098+
pw_match, accepted_peer_list, force, verbose,
10991099
ask_renew, default_renew, do_lock,
11001100
from_edit_user, ask_change_pw, auto_create_db,
11011101
create_backup)

0 commit comments

Comments
 (0)