Skip to content

fix(auth): Removing the ability to delete user properties by passing None #320

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

Merged
merged 1 commit into from
Aug 9, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 11 additions & 15 deletions firebase_admin/_user_mgt.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,10 +34,6 @@ def __init__(self, description):
self.description = description


# Use this internally, until sentinels are available in the public API.
_UNSPECIFIED = Sentinel('No value specified')


DELETE_ATTRIBUTE = Sentinel('Value used to delete an attribute from a user profile')


Expand Down Expand Up @@ -524,9 +520,9 @@ def create_user(self, uid=None, display_name=None, email=None, phone_number=None
'Failed to create new user.', http_response=http_resp)
return body.get('localId')

def update_user(self, uid, display_name=_UNSPECIFIED, email=None, phone_number=_UNSPECIFIED,
photo_url=_UNSPECIFIED, password=None, disabled=None, email_verified=None,
valid_since=None, custom_claims=_UNSPECIFIED):
def update_user(self, uid, display_name=None, email=None, phone_number=None,
photo_url=None, password=None, disabled=None, email_verified=None,
valid_since=None, custom_claims=None):
"""Updates an existing user account with the specified properties"""
payload = {
'localId': _auth_utils.validate_uid(uid, required=True),
Expand All @@ -538,27 +534,27 @@ def update_user(self, uid, display_name=_UNSPECIFIED, email=None, phone_number=_
}

remove = []
if display_name is not _UNSPECIFIED:
if display_name is None or display_name is DELETE_ATTRIBUTE:
if display_name is not None:
if display_name is DELETE_ATTRIBUTE:
remove.append('DISPLAY_NAME')
else:
payload['displayName'] = _auth_utils.validate_display_name(display_name)
if photo_url is not _UNSPECIFIED:
if photo_url is None or photo_url is DELETE_ATTRIBUTE:
if photo_url is not None:
if photo_url is DELETE_ATTRIBUTE:
remove.append('PHOTO_URL')
else:
payload['photoUrl'] = _auth_utils.validate_photo_url(photo_url)
if remove:
payload['deleteAttribute'] = remove

if phone_number is not _UNSPECIFIED:
if phone_number is None or phone_number is DELETE_ATTRIBUTE:
if phone_number is not None:
if phone_number is DELETE_ATTRIBUTE:
payload['deleteProvider'] = ['phone']
else:
payload['phoneNumber'] = _auth_utils.validate_phone(phone_number)

if custom_claims is not _UNSPECIFIED:
if custom_claims is None or custom_claims is DELETE_ATTRIBUTE:
if custom_claims is not None:
if custom_claims is DELETE_ATTRIBUTE:
custom_claims = {}
json_claims = json.dumps(custom_claims) if isinstance(
custom_claims, dict) else custom_claims
Expand Down
14 changes: 2 additions & 12 deletions tests/test_user_mgt.py
Original file line number Diff line number Diff line change
Expand Up @@ -457,16 +457,6 @@ def test_delete_user_custom_claims(self, user_mgt_app):
request = json.loads(recorder[0].body.decode())
assert request == {'localId' : 'testuser', 'customAttributes' : json.dumps({})}

def test_update_user_delete_fields_with_none(self, user_mgt_app):
user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
user_mgt.update_user('testuser', display_name=None, photo_url=None, phone_number=None)
request = json.loads(recorder[0].body.decode())
assert request == {
'localId' : 'testuser',
'deleteAttribute' : ['DISPLAY_NAME', 'PHOTO_URL'],
'deleteProvider' : ['phone'],
}

def test_update_user_delete_fields(self, user_mgt_app):
user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
user_mgt.update_user(
Expand Down Expand Up @@ -553,9 +543,9 @@ def test_set_custom_user_claims_str(self, user_mgt_app):
request = json.loads(recorder[0].body.decode())
assert request == {'localId' : 'testuser', 'customAttributes' : claims}

def test_set_custom_user_claims_none(self, user_mgt_app):
def test_set_custom_user_claims_remove(self, user_mgt_app):
_, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}')
auth.set_custom_user_claims('testuser', None, app=user_mgt_app)
auth.set_custom_user_claims('testuser', auth.DELETE_ATTRIBUTE, app=user_mgt_app)
request = json.loads(recorder[0].body.decode())
assert request == {'localId' : 'testuser', 'customAttributes' : json.dumps({})}

Expand Down