Skip to content

Commit 18f127d

Browse files
rouiljrouilj
committed
refactor: consolidate sets of identical log messages, flake8 fixes
I had multiple places where identical messages were being logged in two places. Now I have one definition in each place and the same message is logged on two different code paths. Also wrapped some long strings making sure that they were included in extraction for translation. Fixed long comments.
1 parent cbc2cdc commit 18f127d

File tree

1 file changed

+71
-57
lines changed

1 file changed

+71
-57
lines changed

‎roundup/cgi/client.py‎

Lines changed: 71 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -1165,9 +1165,10 @@ def determine_user(self):
11651165
# if we got here token is valid, use the role
11661166
# and sub claims.
11671167
try:
1168-
# make sure to str(token['sub']) the subject. As decoded
1169-
# by json, it is unicode which thows an error when used
1170-
# with 'nodeid in db' down the call chain.
1168+
# make sure to str(token['sub']) the
1169+
# subject. As decoded by json, it is unicode
1170+
# which thows an error when used with 'nodeid
1171+
# in db' down the call chain.
11711172
user = self.db.user.get(str(token['sub']), 'username')
11721173
except IndexError:
11731174
raise LoginError("Token subject is invalid.")
@@ -1233,7 +1234,8 @@ def check_anonymous_access(self):
12331234
except TypeError:
12341235
pass
12351236
if isinstance(action, list):
1236-
raise SeriousError('broken form: multiple @action values submitted')
1237+
raise SeriousError(
1238+
self._('broken form: multiple @action values submitted'))
12371239
elif action != '':
12381240
action = action.value.lower()
12391241
if action in ('login', 'register'):
@@ -1404,7 +1406,8 @@ def handle_csrf(self, api=False):
14041406
if (config["WEB_CSRF_ENFORCE_HEADER_%s" % header] == 'required'
14051407
and "HTTP_%s" % header.replace('-', '_') not in self.env):
14061408
logger.error(self._(
1407-
"csrf header %(header)s required but missing for user%(userid)s.") % {
1409+
''"csrf header %(header)s required but missing "
1410+
''"for user%(userid)s.") % {
14081411
'header': header,
14091412
'userid': current_user})
14101413
raise Unauthorised(self._("Missing header: %s") % header)
@@ -1414,15 +1417,16 @@ def handle_csrf(self, api=False):
14141417
if 'HTTP_REFERER' in self.env and enforce != "no":
14151418
if not self.is_referer_header_ok(api=api):
14161419
referer = self.env['HTTP_REFERER']
1420+
logmsg = self._(
1421+
''"csrf Referer header check failed for user%(userid)s. "
1422+
''"Value=%(referer)s") % {'userid': current_user,
1423+
'referer': referer}
14171424
if enforce in ('required', 'yes'):
1418-
logger.error(self._(
1419-
"csrf Referer header check failed for user%(userid)s. Value=%(referer)s") % {'userid': current_user, 'referer': referer})
1425+
logger.error(logmsg)
14201426
raise Unauthorised(self._("Invalid Referer: %s") % (
14211427
referer))
14221428
elif enforce == 'logfailure':
1423-
logger.warning(self._(
1424-
"csrf Referer header check failed for user%(userid)s. Value=%(referer)s") % {
1425-
'userid': current_user, 'referer': referer})
1429+
logger.warning(logmsg)
14261430
else:
14271431
header_pass += 1
14281432

@@ -1433,14 +1437,15 @@ def handle_csrf(self, api=False):
14331437
if 'HTTP_ORIGIN' in self.env and enforce != "no":
14341438
if not self.is_origin_header_ok(api=api):
14351439
origin = self.env['HTTP_ORIGIN']
1440+
logmsg = self._(
1441+
''"csrf Origin header check failed for user%(userid)s. "
1442+
''"Value=%(origin)s") % {
1443+
'userid': current_user, 'origin': origin}
14361444
if enforce in ('required', 'yes'):
1437-
logger.error(self._(
1438-
"csrf Origin header check failed for user%(userid)s. Value=%(origin)s") % {
1439-
'userid': current_user, 'origin': origin})
1445+
logger.error(logmsg)
14401446
raise Unauthorised(self._("Invalid Origin %s" % origin))
14411447
elif enforce == 'logfailure':
1442-
logger.warning(self._(
1443-
"csrf Origin header check failed for user%(userid)s. Value=%(origin)s") % {'userid': current_user, 'origin': origin})
1448+
logger.warning(logmsg)
14441449
else:
14451450
header_pass += 1
14461451

@@ -1451,16 +1456,16 @@ def handle_csrf(self, api=False):
14511456
foundat = self.base.find('://' + host + '/')
14521457
# 4 means self.base has http:/ prefix, 5 means https:/ prefix
14531458
if foundat not in [4, 5]:
1459+
logmsg = self._(
1460+
''"csrf X-FORWARDED-HOST header check failed "
1461+
''"for user%(userid)s. Value=%(host)s") % {
1462+
'userid': current_user, 'host': host}
14541463
if enforce in ('required', 'yes'):
1455-
logger.error(self._(
1456-
"csrf X-FORWARDED-HOST header check failed for user%(userid)s. Value=%(host)s") % {
1457-
'userid': current_user, 'host': host})
1464+
logger.error(logmsg)
14581465
raise Unauthorised(self._(
14591466
"Invalid X-FORWARDED-HOST %s") % host)
14601467
elif enforce == 'logfailure':
1461-
logger.warning(self._(
1462-
"csrf X-FORWARDED-HOST header check failed for user%(userid)s. Value=%(host)s") % {
1463-
'userid': current_user, 'host': host})
1468+
logger.warning(logmsg)
14641469
else:
14651470
header_pass += 1
14661471
else:
@@ -1476,11 +1481,15 @@ def handle_csrf(self, api=False):
14761481
foundat = self.base.find('://' + host + '/')
14771482
# 4 means http:// prefix, 5 means https:// prefix
14781483
if foundat not in [4, 5]:
1484+
logmsg = self._(
1485+
''"csrf HOST header check failed for "
1486+
''"user%(userid)s. Value=%(host)s") % {
1487+
'userid': current_user, 'host': host}
14791488
if enforce in ('required', 'yes'):
1480-
logger.error(self._("csrf HOST header check failed for user%(userid)s. Value=%(host)s") % {'userid': current_user, 'host': host})
1489+
logger.error(logmsg)
14811490
raise Unauthorised(self._("Invalid HOST %s") % host)
14821491
elif enforce == 'logfailure':
1483-
logger.warning(self._("csrf HOST header check failed for user%(userid)s. Value=%(host)s") % {'userid': current_user, 'host': host})
1492+
logger.warning(logmsg)
14841493
else:
14851494
header_pass += 1
14861495

@@ -1501,7 +1510,8 @@ def handle_csrf(self, api=False):
15011510
# see: https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)_Prevention_Cheat_Sheet#Protecting_REST_Services:_Use_of_Custom_Request_Headers
15021511
if 'HTTP_X_REQUESTED_WITH' not in self.env:
15031512
logger.error(self._(
1504-
"csrf X-REQUESTED-WITH xmlrpc required header check failed for user%s."),
1513+
''"csrf X-REQUESTED-WITH xmlrpc required header "
1514+
''"check failed for user%s."),
15051515
current_user)
15061516
raise UsageError(self._("Required Header Missing"))
15071517

@@ -1540,8 +1550,11 @@ def handle_csrf(self, api=False):
15401550
enforce = config['WEB_CSRF_ENFORCE_TOKEN']
15411551
if key is None: # we do not have an @csrf token
15421552
if enforce == 'required':
1543-
logger.error(self._("Required csrf field missing for user%s"), current_user)
1544-
raise UsageError(self._("We can't validate your session (csrf failure). Re-enter any unsaved data and try again."))
1553+
logger.error(self._(
1554+
"Required csrf field missing for user%s"), current_user)
1555+
raise UsageError(self._(
1556+
''"We can't validate your session (csrf failure). "
1557+
''"Re-enter any unsaved data and try again."))
15451558
elif enforce == 'logfailure':
15461559
# FIXME include url
15471560
logger.warning(self._("csrf field not supplied by user%s"),
@@ -1590,39 +1603,39 @@ def handle_csrf(self, api=False):
15901603
'''
15911604
# validate against user and session
15921605
if current_user != nonce_user:
1593-
if enforce in ('required', "yes"):
1594-
logger.error(
1595-
self._("Csrf mismatch user: current user %(user)s != stored user %(stored)s, current session, stored session: %(cur_sess)s,%(stor_sess)s for key %(key)s.") % {
1596-
'user': current_user,
1597-
'stored': nonce_user,
1598-
'cur_sess': current_session,
1599-
'stor_sess': nonce_session,
1600-
'key': key})
1601-
raise UsageError(self._("We can't validate your session (csrf failure). Re-enter any unsaved data and try again."))
1606+
logmsg = self._(
1607+
''"Csrf mismatch user: current user %(user)s != stored "
1608+
''"user %(stored)s, current session, stored session: "
1609+
''"%(cur_sess)s,%(stor_sess)s for key %(key)s.") % {
1610+
'user': current_user,
1611+
'stored': nonce_user,
1612+
'cur_sess': current_session,
1613+
'stor_sess': nonce_session,
1614+
'key': key}
1615+
if enforce in ('required', 'yes'):
1616+
logger.error(logmsg)
1617+
raise UsageError(self._(
1618+
''"We can't validate your session (csrf failure). "
1619+
''"Re-enter any unsaved data and try again."))
16021620
elif enforce == 'logfailure':
1603-
logger.warning(
1604-
self._("Csrf mismatch user: current user %(user)s != stored user %(stored)s, current session, stored session: %(cur_sess)s,%(stor_sess)s for key %(key)s.") % {
1605-
'user': current_user,
1606-
'stored': nonce_user,
1607-
'cur_sess': current_session,
1608-
'stor_sess': nonce_session,
1609-
'key': key})
1621+
logger.warning(logmsg)
1622+
16101623
if current_session != nonce_session:
1611-
if enforce in ('required', "yes"):
1612-
logger.error(
1613-
self._("Csrf mismatch user: current session %(curr_sess)s != stored session %(stor_sess)s, current user/stored user is: %(user)s for key %(key)s.") % {
1614-
'curr_sess': current_session,
1615-
'stor_sess': nonce_session,
1616-
'user': current_user,
1617-
'key': key})
1618-
raise UsageError(self._("We can't validate your session (csrf failure). Re-enter any unsaved data and try again."))
1624+
logmsg = self._(
1625+
''"Csrf mismatch user: current session %(curr_sess)s "
1626+
''"!= stored session %(stor_sess)s, current user/stored "
1627+
''"user is: %(user)s for key %(key)s.") % {
1628+
'curr_sess': current_session,
1629+
'stor_sess': nonce_session,
1630+
'user': current_user,
1631+
'key': key}
1632+
if enforce in ('required', 'yes'):
1633+
logger.error(logmsg)
1634+
raise UsageError(self._(
1635+
''"We can't validate your session (csrf failure). "
1636+
''"Re-enter any unsaved data and try again."))
16191637
elif enforce == 'logfailure':
1620-
logger.warning(
1621-
self._("logged only: Csrf mismatch user: current session %(curr_sess)s != stored session %(stor_sess)s, current user/stored user is: %(user)s for key %(key)s.") % {
1622-
'curr_sess': current_session,
1623-
'stor_sess': nonce_session,
1624-
'user': current_user,
1625-
'key': key})
1638+
logger.warning(logmsg)
16261639

16271640
# we are done and the change can occur.
16281641
return True
@@ -2214,7 +2227,8 @@ def handle_action(self):
22142227
return None
22152228

22162229
if isinstance(action, list):
2217-
raise SeriousError('broken form: multiple @action values submitted')
2230+
raise SeriousError(
2231+
self._('broken form: multiple @action values submitted'))
22182232
else:
22192233
action = action.value.lower()
22202234

0 commit comments

Comments
 (0)