From d2421fb3fbe3059f0e236cb85d87f3075b04c305 Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Thu, 9 Mar 2017 19:50:26 +0700 Subject: [PATCH 1/6] The wrong return value was assumed, causing only the first digit to be read in the list_users view --- hel/views.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/hel/views.py b/hel/views.py index dad66d1..e39fdf1 100644 --- a/hel/views.py +++ b/hel/views.py @@ -493,7 +493,7 @@ def list_packages(context, request): offset = 0 length = request.registry.settings['controllers.packages.list_length'] if 'offset' in params: - offset = params.pop('offset')[0] + offset = params.pop('offset') try: offset = int(offset) except ValueError: @@ -606,7 +606,7 @@ def list_users(context, request): offset = 0 length = request.registry.settings['controllers.users.list_length'] if 'offset' in params: - offset = params.pop('offset')[0] + offset = params.pop('offset') try: offset = int(offset) except: From 4d8705b6e02e25fcd7959df5f3e4df41c69e77ab Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Thu, 9 Mar 2017 19:52:49 +0700 Subject: [PATCH 2/6] Once again, now the RIGHT way --- hel/views.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/hel/views.py b/hel/views.py index e39fdf1..f72fe26 100644 --- a/hel/views.py +++ b/hel/views.py @@ -493,7 +493,7 @@ def list_packages(context, request): offset = 0 length = request.registry.settings['controllers.packages.list_length'] if 'offset' in params: - offset = params.pop('offset') + offset = params.pop('offset')[0] try: offset = int(offset) except ValueError: @@ -602,16 +602,15 @@ def create_user(context, request): context=Users, permission='user_list') def list_users(context, request): - params = request.GET + params = request.GET.dict_of_lists() offset = 0 length = request.registry.settings['controllers.users.list_length'] if 'offset' in params: - offset = params.pop('offset') + offset = params.pop('offset')[0] try: offset = int(offset) except: offset = 0 - params = params.dict_of_lists() groups = None if 'groups' in params: groups = params['groups'] From 45914b2d625aaa481b1ef480e5cc7c71822936ba Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Thu, 9 Mar 2017 19:59:11 +0700 Subject: [PATCH 3/6] Bump version and update the changelog --- CHANGES.rst | 4 ++++ hel/utils/__init__.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index edc717e..d39712e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,7 @@ +3.3.1 +----- +- Fixed the issue in ``list_users`` where only the left-most digit of ``offset`` was read. + 3.3.0 ----- - Added ``versions.files.path``, deprected ``versions.files.dir`` and ``versions.files.name``. diff --git a/hel/utils/__init__.py b/hel/utils/__init__.py index f45a8f5..cf892a6 100644 --- a/hel/utils/__init__.py +++ b/hel/utils/__init__.py @@ -3,7 +3,7 @@ import json -VERSION = '3.3.0' +VERSION = '3.3.1' def parse_search_phrase(s): From 42b52f4e7535a7a21a607d9c34d38bb03bb74c6e Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Tue, 4 Apr 2017 11:31:37 +0700 Subject: [PATCH 4/6] Salt passwords. Also set up loggers in the development configuration --- CHANGES.rst | 12 +++++++++++ development.ini | 4 ++-- hel/__init__.py | 5 +++++ hel/utils/__init__.py | 2 +- hel/utils/authentication.py | 9 +++++++++ hel/utils/models.py | 4 +++- hel/views.py | 40 +++++++++++++++++++++++++++++-------- 7 files changed, 64 insertions(+), 12 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d39712e..ef71164 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,15 @@ +3.4.0 +----- +- Salt passwords. + + - Added ``user.salted`` field. + - Updated auth view to salt passwords when signing up. + - Updated auth view to replace old passwords in the database when logging in. + - Updated auth view to properly check for password validity. + - Updated user update view to salt password when changing it. + +- Configured logging in ``development.ini``. + 3.3.1 ----- - Fixed the issue in ``list_users`` where only the left-most digit of ``offset`` was read. diff --git a/development.ini b/development.ini index 3ac4c4f..35e8dfd 100644 --- a/development.ini +++ b/development.ini @@ -58,13 +58,13 @@ handlers = console [logger_hel] level = DEBUG -handlers = +handlers = console qualname = hel [handler_console] class = StreamHandler args = (sys.stderr,) -level = NOTSET +level = DEBUG formatter = generic [formatter_generic] diff --git a/hel/__init__.py b/hel/__init__.py index 7bddd07..f33e697 100644 --- a/hel/__init__.py +++ b/hel/__init__.py @@ -110,6 +110,11 @@ def main(global_config, **settings): settings['controllers.users.list_length'] = int(settings.get( 'controllers.users.list_length', '20')) + settings['authentication.salt'] = (b'\xc4x\xc1\x1a\x0f\xa5$\xb1\x95AT\x03' + b'\x8d\x03bk\xfc\xe9\x1c\x88') + if 'AUTH_SALT' in os.environ: + settings['authentication.salt'] = os.environ['AUTH_SALT'].encode() + config = Configurator(settings=settings, root_factory=Root) config.add_route_predicate('cors_preflight', CorsPreflightPredicate) diff --git a/hel/utils/__init__.py b/hel/utils/__init__.py index cf892a6..644f1de 100644 --- a/hel/utils/__init__.py +++ b/hel/utils/__init__.py @@ -3,7 +3,7 @@ import json -VERSION = '3.3.1' +VERSION = '3.4.0' def parse_search_phrase(s): diff --git a/hel/utils/authentication.py b/hel/utils/authentication.py index 966889a..bb09ee1 100644 --- a/hel/utils/authentication.py +++ b/hel/utils/authentication.py @@ -1,4 +1,6 @@ # -*- coding: utf-8 -*- +import hashlib + from pyramid.authentication import AuthTktAuthenticationPolicy as Policy from pyramid.httpexceptions import HTTPForbidden from pyramid.security import ACLAllowed, Everyone, Authenticated @@ -75,3 +77,10 @@ def has_permission(request, permission, context=None): if request.has_permission(permission, context): return True raise HTTPForbidden + + +def salt(request, password, username): + hash_data = (username.encode() + + request.registry.settings['authentication.salt'] + + password.encode()) + return hashlib.sha512(hash_data).hexdigest() diff --git a/hel/utils/models.py b/hel/utils/models.py index 9699382..c485ec4 100644 --- a/hel/utils/models.py +++ b/hel/utils/models.py @@ -135,7 +135,9 @@ def __init__(self, strict=False, **kwargs): 'password': '', 'email': '', 'activation_phrase': '', - 'activation_till': '' + 'activation_till': '', + # TODO: remove in hel@4.0.0 + 'salted': True } if strict: diff --git a/hel/views.py b/hel/views.py index f72fe26..781c546 100644 --- a/hel/views.py +++ b/hel/views.py @@ -24,7 +24,7 @@ from hel.resources import Package, Packages, User, Users from hel.utils import update -from hel.utils.authentication import has_permission +from hel.utils.authentication import has_permission, salt from hel.utils.constants import Constants from hel.utils.messages import Messages from hel.utils.models import ModelPackage, ModelUser @@ -83,7 +83,8 @@ def auth(request): email = '' try: params = request.json_body - except: + except Exception as e: + log.exception('1') raise HTTPBadRequest(detail=Messages.bad_request) if 'action' not in params: raise HTTPBadRequest(detail=Messages.bad_request) @@ -106,13 +107,32 @@ def auth(request): raise HTTPBadRequest(detail=Messages.empty_nickname) if password == '': raise HTTPBadRequest(detail=Messages.empty_password) - pass_hash = hashlib.sha512(password.encode()).hexdigest() user = request.db['users'].find_one({'nickname': nickname}) if not user: raise HTTPBadRequest(detail=Messages.failed_login) - correct_hash = user['password'] - if pass_hash != correct_hash: - raise HTTPBadRequest(detail=Messages.failed_login) + + if 'salted' not in user or not user['salted']: # pragma: no cover + correct_hash = user['password'] + pass_hash = hashlib.sha512(password.encode()).hexdigest() + if pass_hash != correct_hash: + raise HTTPBadRequest( # pragma: no cover + detail=Messages.failed_login) + + salted = salt(request, password, nickname) + request.db['users'].find_one_and_update({ + 'nickname': nickname + }, { + '$set': { + 'salted': True, + 'password': salted + } + }) + else: + correct_hash = user['password'] + pass_hash = salt(request, password, nickname) + if pass_hash != correct_hash: + raise HTTPBadRequest(detail=Messages.failed_login) + headers = remember(request, nickname) raise HTTPOk(body=Messages.logged_in, headers=headers) elif params['action'] == 'register': @@ -128,18 +148,21 @@ def auth(request): raise HTTPBadRequest(detail=Messages.empty_email) if password == '': raise HTTPBadRequest(detail=Messages.empty_password) - pass_hash = hashlib.sha512(password.encode()).hexdigest() user = request.db['users'].find_one({'nickname': nickname}) if user: raise HTTPBadRequest(detail=Messages.nickname_in_use) user = request.db['users'].find_one({'email': email}) if user: raise HTTPBadRequest(detail=Messages.email_in_use) + + pass_hash = salt(request, password, nickname) + act_phrase = ''.join('{:02x}'.format(x) for x in os.urandom( request.registry.settings ['activation.length'])) act_till = (datetime.datetime.now() + datetime.timedelta( seconds=request.registry.settings['activation.time'])) + subrequest = Request.blank('/users', method='POST', POST=( str(ModelUser(nickname=nickname, email=email, @@ -148,6 +171,7 @@ def auth(request): activation_till=act_till))), content_type='application/json') subrequest.no_permission_check = True + response = request.invoke_subrequest(subrequest, use_tweens=True) if response.status_code == 201: # TODO: send activation email @@ -550,7 +574,7 @@ def update_user(context, request): # TODO: send email if v == '': raise HTTPBadRequest(detail=Messages.empty_password) - pass_hash = hashlib.sha512(v.encode()).hexdigest() + pass_hash = salt(request, v, context.retrieve()['nickname']) query[k] = pass_hash query = update(old, query) context.update(query, True) From 4e85f145176de1564c6804af98bba650a0507cdc Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Wed, 5 Apr 2017 15:37:15 +0700 Subject: [PATCH 5/6] Increase the max short description length --- CHANGES.rst | 1 + hel/utils/constants.py | 3 +++ hel/utils/models.py | 2 +- hel/views.py | 2 +- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index ef71164..ed0ee8a 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -9,6 +9,7 @@ - Updated user update view to salt password when changing it. - Configured logging in ``development.ini``. +- Increased the maximum length of short description to 300 characters. 3.3.1 ----- diff --git a/hel/utils/constants.py b/hel/utils/constants.py index d1a2628..f73a300 100644 --- a/hel/utils/constants.py +++ b/hel/utils/constants.py @@ -16,3 +16,6 @@ class Constants: user_pattern = re.compile('^[A-Za-z0-9-_]+$') date_format = '%Y-%m-%d %H:%M:%S' + + # The maximum length of short description + sdesc_len = 300 diff --git a/hel/utils/models.py b/hel/utils/models.py index c485ec4..e268342 100644 --- a/hel/utils/models.py +++ b/hel/utils/models.py @@ -111,7 +111,7 @@ def __init__(self, strict=False, **kwargs): self.data[k] = {parse_url(str(url)): str(desc) for url, desc in v.items()} elif k == 'short_description': - self.data[k] = str(v)[:140] + self.data[k] = str(v)[:Constants.sdesc_len] @property def json(self): diff --git a/hel/views.py b/hel/views.py index 781c546..59cbaa4 100644 --- a/hel/views.py +++ b/hel/views.py @@ -219,7 +219,7 @@ def update_package(context, request): elif k == 'short_description': query[k] = check( v, str, - Messages.type_mismatch % (k, 'str',))[:120] + Messages.type_mismatch % (k, 'str',))[:Constants.sdesc_len] elif k == 'owners': check_list_of_strs( v, Messages.type_mismatch % (k, 'list of strs',)) From 8ebbaf0a59dc5ea41421691462cbd0d6ee19b810 Mon Sep 17 00:00:00 2001 From: Fingercomp Date: Wed, 5 Apr 2017 22:25:35 +0700 Subject: [PATCH 6/6] Forbid using uppercase characters. Update the changelog --- CHANGES.rst | 1 + hel/utils/constants.py | 2 +- hel/utils/messages.py | 3 ++- 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index ed0ee8a..d050ea5 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -10,6 +10,7 @@ - Configured logging in ``development.ini``. - Increased the maximum length of short description to 300 characters. +- Forbade using uppercase characters in package names. 3.3.1 ----- diff --git a/hel/utils/constants.py b/hel/utils/constants.py index f73a300..1b5be70 100644 --- a/hel/utils/constants.py +++ b/hel/utils/constants.py @@ -10,7 +10,7 @@ class Constants: key_replace_char = '\uf123' # The regexp used to validate a package name - name_pattern = re.compile('^[A-Za-z0-9-]+$') + name_pattern = re.compile('^[a-z0-9-]+$') # The regexp used to validate a nickname user_pattern = re.compile('^[A-Za-z0-9-_]+$') diff --git a/hel/utils/messages.py b/hel/utils/messages.py index 6715395..172e079 100644 --- a/hel/utils/messages.py +++ b/hel/utils/messages.py @@ -21,7 +21,8 @@ class Messages: no_values = 'No values given for %s.' partial_ver = 'Version data you provided was partial.' password_mismatch = 'Passwords do not match.' - pkg_bad_name = 'The name contains illegal characters.' + pkg_bad_name = ('The name contains illegal characters. Allowed are: ' + 'lowercase alphabetic characters, numbers and a dash (-).') pkg_name_conflict = 'The name is already used by other package.' too_many_values = 'Too many values (%s expected, got %s).' type_mismatch = 'Wrong value for param "%s" given: expected %s!'