Skip to content

Commit c441fae

Browse files
committed
Stand-alone filtering of trivially invalid account requests on submit.
Extensions are planned in the janitor ervice but this lays the foundation to ease the load on operators and support.
1 parent 5fb42a7 commit c441fae

File tree

3 files changed

+119
-19
lines changed

3 files changed

+119
-19
lines changed

mig/shared/accountreq.py

Lines changed: 105 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -43,22 +43,23 @@
4343
iso3166 = None
4444

4545
from mig.shared.accountstate import default_account_expire
46-
from mig.shared.base import force_utf8, force_native_str_rec, canonical_user, \
46+
from mig.shared.base import auth_type_description, canonical_user, \
4747
client_id_dir, distinguished_name_to_user, fill_distinguished_name, \
48-
fill_user, auth_type_description, mask_creds
48+
fill_user, force_utf8, force_native_str_rec, get_user_id, mask_creds
4949
from mig.shared.defaults import peers_fields, peers_filename, \
5050
pending_peers_filename, keyword_auto, user_db_filename, \
5151
gdp_distinguished_field
5252
from mig.shared.fileio import delete_file, make_temp_file
5353
from mig.shared.notification import notify_user
54+
from mig.shared.pwcrypto import check_hash
5455
# Expose some helper variables for functionality backends
5556
from mig.shared.safeinput import name_extras, password_extras, \
5657
password_min_len, password_max_len, valid_password_chars, \
5758
valid_name_chars, dn_max_len, html_escape, validated_input, REJECT_UNSET
5859
from mig.shared.serial import load, dump, dumps
5960
from mig.shared.useradm import user_request_reject, user_account_notify, \
60-
default_search, search_users, create_user, load_user_dict
61-
from mig.shared.userdb import default_db_path
61+
default_search, search_users, create_user
62+
from mig.shared.userdb import default_db_path, load_user_dict
6263
from mig.shared.validstring import valid_email_addresses
6364

6465

@@ -1062,8 +1063,9 @@ def reject_account_req(req_id, configuration, reject_reason,
10621063
user_copy=True, admin_copy=True, auth_type='oid'):
10631064
"""Helper to reject a pending account request"""
10641065
_logger = configuration.logger
1065-
_logger.info('reject account request %s with msg %s' %
1066-
(req_id, reject_reason))
1066+
# NOTE: we strip newlines to avoid multi-line log entries
1067+
_logger.info('reject account request %s with reason: %r' %
1068+
(req_id, reject_reason.strip('\n')))
10671069
# NOTE: conf_path accepts configuration object
10681070
conf_path = configuration
10691071
db_path = default_db_path(configuration)
@@ -1173,6 +1175,103 @@ def list_country_codes(configuration):
11731175
return country_list
11741176

11751177

1178+
def existing_user_collision(configuration, raw_request, client_id):
1179+
"""Check if raw_request has collisions with existing users in user DB.
1180+
Mainly if email is already bound to another user with different full name,
1181+
organization, country or state.
1182+
"""
1183+
logger = configuration.logger
1184+
db_path = default_db_path(configuration)
1185+
search_filter = default_search()
1186+
search_filter['email'] = raw_request.get('email', 'UNSET')
1187+
(_, hits) = search_users(search_filter, configuration, db_path)
1188+
collisions = [user_id for (user_id, _) in hits if user_id != client_id]
1189+
if collisions:
1190+
logger.warning('one or more ID collisions in request from %r: %s' % \
1191+
(client_id, ', '.join(collisions)))
1192+
return True
1193+
else:
1194+
return False
1195+
1196+
def early_validation_checks(configuration, raw_request, service, username,
1197+
password):
1198+
"""Early validation checks including e.g. password check when change is not
1199+
authorized. Useful to allow janitor to request invalid requests with
1200+
sufficient delay to render various user enumeration, email and password
1201+
guessing scenarios infeasible"""
1202+
logger = configuration.logger
1203+
illegal_pw_change = """invalid password in renewal request.
1204+
Please use your existing password when renewing to prove account ownership. You
1205+
can use the 'Forgot password' link on the login page to securely reset it first
1206+
if needed"""
1207+
renewal_blocked = """account status blocks renewal request.
1208+
Please contact support if you haven't been informed why this might be and think
1209+
your account access should be renewed"""
1210+
id_collision = """invalid ID in account creation request.
1211+
An existing user has overlapping but not identical ID fields. You must reuse
1212+
your exact existing ID to renew account access. Please contact support if you
1213+
want corrections or affiliation changes in your site registration"""
1214+
invalid_full_name = """invalid full name in account creation request.
1215+
You must provide your full name to get account access due to various legal
1216+
requirements e.g. in relation to account abuse. Please contact support if you
1217+
have questions in that regard"""
1218+
missing_peers_info = """missing peer info in account creation request.
1219+
You must point to one or more persons allowed to invite peers with their full
1220+
name and email address they have used for site registration. You can ask them
1221+
to invite you instead if that is easier"""
1222+
# Lazy init
1223+
raw_request['invalid'] = raw_request.get('invalid', [])
1224+
1225+
client_id = get_user_id(configuration, raw_request)
1226+
db_path = default_db_path(configuration)
1227+
user_dict = load_user_dict(configuration, client_id, db_path)
1228+
if user_dict:
1229+
# Renewal or password change
1230+
authorized = raw_request.get('authorized', None)
1231+
reset_token = raw_request.get('reset_token', None)
1232+
account_status = raw_request.get('status', 'active')
1233+
if not authorized and not reset_token:
1234+
hashed = user_dict.get('password_hash', None)
1235+
if not check_hash(configuration, service, username, password,
1236+
hashed):
1237+
logger.warning('illegal password change in request from %r' % \
1238+
client_id)
1239+
raw_request['invalid'].append(illegal_pw_change)
1240+
elif account_status not in ('temporal', 'active', 'inactive'):
1241+
logger.warning('existing account for %r is %s and not renewable' \
1242+
% (client_id, account_status))
1243+
raw_request['invalid'].append(renewal_blocked)
1244+
else:
1245+
logger.debug('account renewal from %r looks alright' % client_id)
1246+
elif existing_user_collision(configuration, raw_request, client_id):
1247+
# TODO: drop and rely solely on live check in janitor to avoid races?
1248+
logger.warning('ID collision in request from %r' % client_id)
1249+
raw_request['invalid'].append(id_collision)
1250+
else:
1251+
# New user account
1252+
peers_full_name = raw_request.get('peers_full_name', None)
1253+
peers_email = raw_request.get('peers_email', None)
1254+
full_name = raw_request.get('full_name', 'UNSET')
1255+
if configuration.site_enable_peers and \
1256+
('email' in configuration.site_peers_explicit_fields and \
1257+
not peers_email or \
1258+
'full_name' in configuration.site_peers_explicit_fields and \
1259+
not peers_full_name):
1260+
logger.warning('missing peers field in request from %r: %r %r' % \
1261+
(client_id, peers_full_name, peers_email))
1262+
raw_request['invalid'].append(missing_peers_info)
1263+
elif len(full_name.split(' ')) < 2:
1264+
# TODO: prevent this at the source instead - sign up and peers
1265+
logger.warning('invalid single word full name in request from %r' \
1266+
% client_id)
1267+
raw_request['invalid'].append(invalid_full_name)
1268+
# TODO: check that specified peers have accounts and can act as peers
1269+
# TODO: check for other obvious signup errors ?
1270+
else:
1271+
logger.debug('account request from %r looks alright' % client_id)
1272+
return raw_request
1273+
1274+
11761275
def prefilter_potential_peers(peers_list, configuration):
11771276
"""Return entries from peers_list that fit local prefilter policy"""
11781277
potential_peers = []

mig/shared/functionality/reqcertaction.py

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,11 @@
3333

3434
import os
3535
import time
36-
import tempfile
3736

3837
from mig.shared import returnvalues
39-
from mig.shared.accountreq import existing_country_code, \
40-
prefilter_potential_peers, save_account_request, signup_prefilter_allowed, \
41-
user_manage_commands
38+
from mig.shared.accountreq import early_validation_checks, \
39+
existing_country_code, prefilter_potential_peers, save_account_request, \
40+
signup_prefilter_allowed, user_manage_commands
4241
from mig.shared.accountstate import default_account_expire
4342
from mig.shared.base import client_id_dir, canonical_user, mask_creds, \
4443
generate_https_urls, fill_distinguished_name
@@ -106,7 +105,6 @@ def main(client_id, user_arguments_dict):
106105
support_email = configuration.support_email
107106
admin_email = configuration.admin_email
108107
smtp_server = configuration.smtp_server
109-
user_pending = os.path.abspath(configuration.user_pending)
110108

111109
cert_name = accepted['cert_name'][-1].strip()
112110
country = accepted['country'][-1].strip()
@@ -204,7 +202,8 @@ def main(client_id, user_arguments_dict):
204202
You may also read more about the Peers system in the site documentation.
205203
'''})
206204
output_objects.append(
207-
{'object_type': 'link', 'destination': 'javascript:history.back();',
205+
{'object_type': 'link',
206+
'destination': 'javascript:history.back();',
208207
'class': 'genericbutton', 'text': "Try again"})
209208
return (output_objects, returnvalues.CLIENT_ERROR)
210209

@@ -249,11 +248,13 @@ def main(client_id, user_arguments_dict):
249248
if configuration.user_openid_providers and configuration.user_openid_alias:
250249
user_dict['openid_names'] += \
251250
[user_dict[configuration.user_openid_alias]]
251+
# Do simple early validation to reduce load on support team
252+
user_dict = early_validation_checks(configuration, user_dict, 'https',
253+
email, password)
252254
# IMPORTANT: do NOT log credentials
253255
logger.info('got account request from reqcert: %s' % mask_creds(user_dict))
254256

255257
# For testing only
256-
257258
if cert_name.upper().find('DO NOT SEND') != -1:
258259
output_objects.append(
259260
{'object_type': 'text', 'text': "Test request ignored!"})

mig/shared/functionality/reqoidaction.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,11 @@
3333

3434
import os
3535
import time
36-
import tempfile
3736

3837
from mig.shared import returnvalues
39-
from mig.shared.accountreq import existing_country_code, \
40-
prefilter_potential_peers, save_account_request, signup_prefilter_allowed, \
41-
user_manage_commands
38+
from mig.shared.accountreq import early_validation_checks, \
39+
existing_country_code, prefilter_potential_peers, save_account_request, \
40+
signup_prefilter_allowed, user_manage_commands
4241
from mig.shared.accountstate import default_account_expire
4342
from mig.shared.base import client_id_dir, canonical_user, mask_creds, \
4443
force_utf8, force_unicode, force_native_str, force_native_str_rec, \
@@ -110,7 +109,6 @@ def main(client_id, user_arguments_dict):
110109
support_email = configuration.support_email
111110
admin_email = configuration.admin_email
112111
smtp_server = configuration.smtp_server
113-
user_pending = os.path.abspath(configuration.user_pending)
114112

115113
cert_name = accepted['cert_name'][-1].strip()
116114
country = accepted['country'][-1].strip()
@@ -264,11 +262,13 @@ def main(client_id, user_arguments_dict):
264262
if configuration.user_openid_providers and configuration.user_openid_alias:
265263
user_dict['openid_names'].append(
266264
user_dict[configuration.user_openid_alias])
265+
# Do simple early validation to reduce load on support team
266+
user_dict = early_validation_checks(configuration, user_dict, 'https',
267+
email, password)
267268
# IMPORTANT: do NOT log credentials
268269
logger.info('got account request from reqoid: %s' % mask_creds(user_dict))
269270

270271
# For testing only
271-
272272
if cert_name.upper().find('DO NOT SEND') != -1:
273273
output_objects.append(
274274
{'object_type': 'text', 'text': "Test request ignored!"})

0 commit comments

Comments
 (0)