# HG changeset patch # User Marcin Lulek # Date 2017-06-19 13:14:02 # Node ID 7cb6e1ce0083fd4bf0b0e06bd200e7a91929036d # Parent a1111aae75c874264cf108b9d955c0b604ba65e1 security: use new safe escaped user attributes across the application. - will fix all possible XSS attack vectors. diff --git a/rhodecode/apps/admin/views/users.py b/rhodecode/apps/admin/views/users.py --- a/rhodecode/apps/admin/views/users.py +++ b/rhodecode/apps/admin/views/users.py @@ -127,8 +127,8 @@ class AdminUsersView(BaseAppView, DataGr users_data.append({ "username": h.gravatar_with_user(user.username), "email": user.email, - "first_name": h.escape(user.name), - "last_name": h.escape(user.lastname), + "first_name": user.first_name, + "last_name": user.last_name, "last_login": h.format_date(user.last_login), "last_activity": h.format_date(user.last_activity), "active": h.bool2icon(user.active), diff --git a/rhodecode/apps/home/tests/test_home.py b/rhodecode/apps/home/tests/test_home.py --- a/rhodecode/apps/home/tests/test_home.py +++ b/rhodecode/apps/home/tests/test_home.py @@ -111,8 +111,8 @@ class TestHomeController(TestController) user_util.create_repo(owner=username) response = self.app.get(route_path('home')) - response.mustcontain(h.html_escape(h.escape(user.name))) - response.mustcontain(h.html_escape(h.escape(user.lastname))) + response.mustcontain(h.html_escape(user.first_name)) + response.mustcontain(h.html_escape(user.last_name)) @pytest.mark.parametrize("name, state", [ ('Disabled', False), diff --git a/rhodecode/apps/repository/utils.py b/rhodecode/apps/repository/utils.py --- a/rhodecode/apps/repository/utils.py +++ b/rhodecode/apps/repository/utils.py @@ -36,8 +36,8 @@ def reviewer_as_json(user, reasons=None, 'reasons': reasons or [], 'mandatory': mandatory, 'username': user.username, - 'firstname': user.firstname, - 'lastname': user.lastname, + 'first_name': user.first_name, + 'last_name': user.last_name, 'gravatar_link': h.gravatar_url(user.email, 14), } diff --git a/rhodecode/controllers/admin/user_groups.py b/rhodecode/controllers/admin/user_groups.py --- a/rhodecode/controllers/admin/user_groups.py +++ b/rhodecode/controllers/admin/user_groups.py @@ -493,8 +493,8 @@ class UserGroupsController(BaseControlle group_members = [ { 'id': user.user_id, - 'first_name': user.name, - 'last_name': user.lastname, + 'first_name': user.first_name, + 'last_name': user.last_name, 'username': user.username, 'icon_link': h.gravatar_url(user.email, 30), 'value_display': h.person(user.email), diff --git a/rhodecode/controllers/pullrequests.py b/rhodecode/controllers/pullrequests.py --- a/rhodecode/controllers/pullrequests.py +++ b/rhodecode/controllers/pullrequests.py @@ -21,8 +21,6 @@ """ pull requests controller for rhodecode for initializing pull requests """ -import types - import peppercorn import formencode import logging @@ -33,6 +31,7 @@ from pylons import request, tmpl_context from pylons.controllers.util import redirect from pylons.i18n.translation import _ from pyramid.threadlocal import get_current_registry +from pyramid.httpexceptions import HTTPFound from sqlalchemy.sql import func from sqlalchemy.sql.expression import or_ diff --git a/rhodecode/lib/auth.py b/rhodecode/lib/auth.py --- a/rhodecode/lib/auth.py +++ b/rhodecode/lib/auth.py @@ -807,6 +807,8 @@ class AuthUser(object): self.ip_addr = ip_addr self.name = '' self.lastname = '' + self.first_name = '' + self.last_name = '' self.email = '' self.is_authenticated = False self.admin = False diff --git a/rhodecode/lib/channelstream.py b/rhodecode/lib/channelstream.py --- a/rhodecode/lib/channelstream.py +++ b/rhodecode/lib/channelstream.py @@ -77,8 +77,8 @@ def get_user_data(user_id): return { 'id': user.user_id, 'username': user.username, - 'first_name': user.name, - 'last_name': user.lastname, + 'first_name': user.first_name, + 'last_name': user.last_name, 'icon_link': h.gravatar_url(user.email, 60), 'display_name': h.person(user, 'username_or_name_or_email'), 'display_link': h.link_to_user(user), diff --git a/rhodecode/lib/helpers.py b/rhodecode/lib/helpers.py --- a/rhodecode/lib/helpers.py +++ b/rhodecode/lib/helpers.py @@ -893,9 +893,9 @@ def author_string(email): if email: user = User.get_by_email(email, case_insensitive=True, cache=True) if user: - if user.firstname or user.lastname: + if user.first_name or user.last_name: return '%s %s <%s>' % ( - escape(user.firstname), escape(user.lastname), email) + user.first_name, user.last_name, email) else: return email else: @@ -1144,14 +1144,14 @@ class InitialsGravatar(object): # first push the email initials prefix, server = email_address.split('@', 1) - # check if prefix is maybe a 'firstname.lastname' syntax + # check if prefix is maybe a 'first_name.last_name' syntax _dot_split = prefix.rsplit('.', 1) if len(_dot_split) == 2: initials = [_dot_split[0][0], _dot_split[1][0]] else: initials = [prefix[0], server[0]] - # then try to replace either firtname or lastname + # then try to replace either first_name or last_name fn_letter = (first_name or " ")[0].strip() ln_letter = (last_name.split(' ', 1)[-1] or " ")[0].strip() diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -574,12 +574,16 @@ class User(Base, BaseModel): @hybrid_property def first_name(self): from rhodecode.lib import helpers as h - return h.escape(self.name) + if self.name: + return h.escape(self.name) + return self.name @hybrid_property def last_name(self): from rhodecode.lib import helpers as h - return h.escape(self.lastname) + if self.lastname: + return h.escape(self.lastname) + return self.lastname @hybrid_property def api_key(self): @@ -700,7 +704,7 @@ class User(Base, BaseModel): @property def username_and_name(self): - return '%s (%s %s)' % (self.username, self.firstname, self.lastname) + return '%s (%s %s)' % (self.username, self.first_name, self.last_name) @property def username_or_name_or_email(self): @@ -709,20 +713,20 @@ class User(Base, BaseModel): @property def full_name(self): - return '%s %s' % (self.firstname, self.lastname) + return '%s %s' % (self.first_name, self.last_name) @property def full_name_or_username(self): - return ('%s %s' % (self.firstname, self.lastname) - if (self.firstname and self.lastname) else self.username) + return ('%s %s' % (self.first_name, self.last_name) + if (self.first_name and self.last_name) else self.username) @property def full_contact(self): - return '%s %s <%s>' % (self.firstname, self.lastname, self.email) + return '%s %s <%s>' % (self.first_name, self.last_name, self.email) @property def short_contact(self): - return '%s %s' % (self.firstname, self.lastname) + return '%s %s' % (self.first_name, self.last_name) @property def is_admin(self): diff --git a/rhodecode/model/pull_request.py b/rhodecode/model/pull_request.py --- a/rhodecode/model/pull_request.py +++ b/rhodecode/model/pull_request.py @@ -1291,8 +1291,8 @@ class PullRequestModel(BaseModel): 'user': { 'user_id': repo.user.user_id, 'username': repo.user.username, - 'firstname': repo.user.firstname, - 'lastname': repo.user.lastname, + 'firstname': repo.user.first_name, + 'lastname': repo.user.last_name, 'gravatar_link': h.gravatar_url(repo.user.email, 14), }, 'description': h.chop_at_smart(repo.description, '\n'), diff --git a/rhodecode/model/user.py b/rhodecode/model/user.py --- a/rhodecode/model/user.py +++ b/rhodecode/model/user.py @@ -70,8 +70,8 @@ class UserModel(BaseModel): return { 'id': user.user_id, - 'first_name': h.escape(user.name), - 'last_name': h.escape(user.lastname), + 'first_name': user.first_name, + 'last_name': user.last_name, 'username': user.username, 'email': user.email, 'icon_link': h.gravatar_url(user.email, 30), @@ -679,6 +679,11 @@ class UserModel(BaseModel): # TODO: johbo: Think about this and find a clean solution user_data = dbuser.get_dict() user_data.update(dbuser.get_api_data(include_secrets=True)) + user_data.update({ + # set explicit the safe escaped values + 'first_name': dbuser.first_name, + 'last_name': dbuser.last_name, + }) for k, v in user_data.iteritems(): # properties of auth user we dont update diff --git a/rhodecode/public/js/src/rhodecode/pullrequests.js b/rhodecode/public/js/src/rhodecode/pullrequests.js --- a/rhodecode/public/js/src/rhodecode/pullrequests.js +++ b/rhodecode/public/js/src/rhodecode/pullrequests.js @@ -227,8 +227,8 @@ ReviewersController = function () { for (var i = 0; i < data.reviewers.length; i++) { var reviewer = data.reviewers[i]; self.addReviewMember( - reviewer.user_id, reviewer.firstname, - reviewer.lastname, reviewer.username, + reviewer.user_id, reviewer.first_name, + reviewer.last_name, reviewer.username, reviewer.gravatar_link, reviewer.reasons, reviewer.mandatory); } diff --git a/rhodecode/templates/admin/my_account/my_account_profile.mako b/rhodecode/templates/admin/my_account/my_account_profile.mako --- a/rhodecode/templates/admin/my_account/my_account_profile.mako +++ b/rhodecode/templates/admin/my_account/my_account_profile.mako @@ -32,7 +32,7 @@ ${_('First Name')}:
- ${c.user.firstname} + ${c.user.first_name}
@@ -40,7 +40,7 @@ ${_('Last Name')}:
- ${c.user.lastname} + ${c.user.last_name}
diff --git a/rhodecode/templates/base/root.mako b/rhodecode/templates/base/root.mako --- a/rhodecode/templates/base/root.mako +++ b/rhodecode/templates/base/root.mako @@ -12,8 +12,8 @@ if getattr(c, 'rhodecode_user', None) an c.template_context['rhodecode_user']['username'] = c.rhodecode_user.username c.template_context['rhodecode_user']['email'] = c.rhodecode_user.email c.template_context['rhodecode_user']['notification_status'] = c.rhodecode_user.get_instance().user_data.get('notification_status', True) - c.template_context['rhodecode_user']['first_name'] = c.rhodecode_user.name - c.template_context['rhodecode_user']['last_name'] = c.rhodecode_user.lastname + c.template_context['rhodecode_user']['first_name'] = c.rhodecode_user.first_name + c.template_context['rhodecode_user']['last_name'] = c.rhodecode_user.last_name c.template_context['visual']['default_renderer'] = h.get_visual_attr(c, 'default_renderer') c.template_context['default_user'] = { diff --git a/rhodecode/templates/email_templates/user_registration.mako b/rhodecode/templates/email_templates/user_registration.mako --- a/rhodecode/templates/email_templates/user_registration.mako +++ b/rhodecode/templates/email_templates/user_registration.mako @@ -10,7 +10,7 @@ RhodeCode new user registration: ${user. A new user `${user.username}` has registered on ${h.format_date(date)} - Username: ${user.username} -- Full Name: ${user.firstname} ${user.lastname} +- Full Name: ${user.first_name} ${user.last_name} - Email: ${user.email} - Profile link: ${h.route_url('user_profile', username=user.username)} @@ -21,7 +21,7 @@ A new user `${user.username}` has regist - +

${_('New user %(user)s has registered on %(date)s') % {'user': user.username, 'date': h.format_date(date)}}

${_('Username')} ${user.username}
${_('Full Name')}${user.firstname} ${user.lastname}
${_('Full Name')}${user.first_name} ${user.last_name}
${_('Email')}${user.email}
${_('Profile')}${h.route_url('user_profile', username=user.username)}
\ No newline at end of file diff --git a/rhodecode/templates/users/user_profile.mako b/rhodecode/templates/users/user_profile.mako --- a/rhodecode/templates/users/user_profile.mako +++ b/rhodecode/templates/users/user_profile.mako @@ -35,7 +35,7 @@ ${_('First name')}:
- ${c.user.firstname} + ${c.user.first_name}
@@ -43,7 +43,7 @@ ${_('Last name')}:
- ${c.user.lastname} + ${c.user.last_name}
diff --git a/rhodecode/tests/functional/test_admin_gists.py b/rhodecode/tests/functional/test_admin_gists.py --- a/rhodecode/tests/functional/test_admin_gists.py +++ b/rhodecode/tests/functional/test_admin_gists.py @@ -336,9 +336,7 @@ class TestGistsController(TestController def test_user_first_name_is_escaped(self, user_util, create_gist): xss_atack_string = '">' - xss_escaped_string = ( - '"><script>alert('First Name')</script' - '>') + xss_escaped_string = h.html_escape(h.escape(xss_atack_string)) password = 'test' user = user_util.create_user( firstname=xss_atack_string, password=password) @@ -348,8 +346,7 @@ class TestGistsController(TestController def test_user_last_name_is_escaped(self, user_util, create_gist): xss_atack_string = '">' - xss_escaped_string = ( - '"><script>alert('Last Name')</script>') + xss_escaped_string = h.html_escape(h.escape(xss_atack_string)) password = 'test' user = user_util.create_user( lastname=xss_atack_string, password=password)