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 1 commit
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
4 changes: 4 additions & 0 deletions CHANGELOG.rst
Expand Up @@ -8,6 +8,10 @@ Changes
-------
Unreleased
==========
2019-01-17: v0.7.6
^^^^^^^^^^^^^^^^^^
* :lock: Stop exposing sensitive data in deprecated `PUT /user/{:id}`

2019-12-18: v0.7.5
^^^^^^^^^^^^^^^^^^
* :racehorse: Check if user requesting user list is a coordinator just once
Expand Down
31 changes: 20 additions & 11 deletions girderformindlogger/api/v1/user.py
Expand Up @@ -669,7 +669,6 @@ def getUsersDetails(self):
return {'nUsers': nUsers}

@access.user
@filtermodel(model=UserModel)
@autoDescribeRoute(
Description("Update a user's information.")
.modelParam('id', model=UserModel, level=AccessType.WRITE)
Expand Down Expand Up @@ -714,7 +713,8 @@ def updateUser(
status=None,
firstName=None,
lastName=None
): # 🔥 delete firstName and lastName once fully deprecated
):
# 🔥 delete firstName and lastName once fully deprecated
user['firstName'] = displayName if len(
displayName
) else firstName if firstName is not None else ""
Expand All @@ -727,16 +727,25 @@ def updateUser(
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
# 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

return self._model.save(user)
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(
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