Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🔒 🔈 Plug a security vulnerability and alert users to deprecated endpoint #618

Merged
merged 4 commits into from Feb 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.rst
Expand Up @@ -9,10 +9,15 @@ Changes
Unreleased
==========


2019-01-17: v0.10.1
^^^^^^^^^^^^^^^^^^
* :lock: Stop exposing sensitive data in deprecated `PUT /user/{:id}`

2020-02-13: v0.9.1
^^^^^^^^^^^^^^^^^^
* :sparkles: Returns one applet per child for parent-report applets
=======

2020-02-10: v0.8.2
^^^^^^^^^^^^^^^^^^
* Fixed issue with Pandas version in CI test
Expand Down
80 changes: 80 additions & 0 deletions girderformindlogger/api/v1/user.py
Expand Up @@ -646,6 +646,86 @@ def getUsersDetails(self):
)).count()
return {'nUsers': nUsers}


@access.user
@autoDescribeRoute(
Description("Update a user's information.")
.modelParam('id', model=UserModel, level=AccessType.WRITE)
.param(
'displayName',
'Display name of the user, usually just their first name.',
default="",
required=False
)
.param('admin', 'Is the user a site admin (admin access required)',
required=False, dataType='boolean')
.param('status', 'The account status (admin access required)',
required=False, enum=('pending', 'enabled', 'disabled'))
.param(
'email',
'Deprecated. Do not use.',
required=False,
dataType='string'
)
.param(
'firstName',
'Deprecated. Do not use.',
deprecated=True,
required=False
)
.param(
'lastName',
'Deprecated. Do not use.',
deprecated=True,
required=False
)
.errorResponse()
.errorResponse(('You do not have write access for this user.',
'Must be an admin to create an admin.'), 403)
)
def updateUser(
self,
user,
displayName="",
email="",
admin=False,
status=None,
firstName=None,
lastName=None
):
# 🔥 delete firstName and lastName once fully deprecated
user['firstName'] = displayName if len(
displayName
) else firstName if firstName is not None else ""
user['email'] = email

# Only admins can change admin state
if admin is not None:
if self.getCurrentUser()['admin']:
user['admin'] = admin
elif user['admin'] is not admin:
raise AccessException('Only admins may change admin status.')

# Only admins can change status
if status is not None and status != user.get('status', 'enabled'):
if not self.getCurrentUser()['admin']:
raise AccessException('Only admins may change status.')
if user['status'] == 'pending' and status == 'enabled':
# Send email on the 'pending' -> 'enabled' transition
self._model._sendApprovedEmail(user)
user['status'] = status

try:
self._model.save(user)
except:
raise RestException(
'Update failed, and `PUT /user/{:id}` is deprecated.'
)

return(
{'message': 'Update saved, but `PUT /user/{:id}` is deprecated.'}
)

@access.admin
@autoDescribeRoute(
Description("Change a user's password.")
Expand Down
3 changes: 3 additions & 0 deletions girderformindlogger/models/user.py
Expand Up @@ -58,6 +58,9 @@ def validate(self, doc):
"""
Validate the user every time it is stored in the database.
"""
for s in ['email', 'displayName', 'firstName']:
if s in doc and doc[s] is None:
doc[s] = ''
doc['login'] = doc.get('login', '').lower().strip()
doc['email'] = doc.get('email', '').lower().strip()
doc['displayName'] = doc.get(
Expand Down