From 3ba322b8fda6f68212b306a93b89ac5193929a3d Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Mon, 20 Sep 2021 13:13:19 +0530 Subject: [PATCH 1/8] [add] ability to delete provider in auth --- firebase_admin/_auth_client.py | 1 + firebase_admin/_user_mgt.py | 10 ++++++++-- integration/test_auth.py | 8 ++++++++ tests/test_user_mgt.py | 15 +++++++++++++++ 4 files changed, 32 insertions(+), 2 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index 4418a034d..780bc94bd 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -336,6 +336,7 @@ def update_user(self, uid, **kwargs): # pylint: disable=differing-param-doc valid_since: An integer signifying the seconds since the epoch (optional). This field is set by ``revoke_refresh_tokens`` and it is discouraged to set this field directly. + delete_providers: The list of provider IDs to unlink, eg: 'google.com', 'password', etc. Returns: UserRecord: An updated UserRecord instance for the user. diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index b60c4d100..3a220a9c8 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -688,7 +688,7 @@ def create_user(self, uid=None, display_name=None, email=None, phone_number=None 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): + valid_since=None, custom_claims=None, delete_providers=None): """Updates an existing user account with the specified properties""" payload = { 'localId': _auth_utils.validate_uid(uid, required=True), @@ -700,6 +700,7 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, } remove = [] + removeProvider = [] if display_name is not None: if display_name is DELETE_ATTRIBUTE: remove.append('DISPLAY_NAME') @@ -715,7 +716,7 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, if phone_number is not None: if phone_number is DELETE_ATTRIBUTE: - payload['deleteProvider'] = ['phone'] + removeProvider.append('phone') else: payload['phoneNumber'] = _auth_utils.validate_phone(phone_number) @@ -726,6 +727,11 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, custom_claims, dict) else custom_claims payload['customAttributes'] = _auth_utils.validate_custom_claims(json_claims) + if delete_providers is not None and isinstance(delete_providers, list): + removeProvider += delete_providers + if removeProvider: + payload['deleteProvider'] = list(set(removeProvider)) + payload = {k: v for k, v in payload.items() if v is not None} body, http_resp = self._make_request('post', '/accounts:update', json=payload) if not body or not body.get('localId'): diff --git a/integration/test_auth.py b/integration/test_auth.py index 55ddbb0a0..8433ed4d4 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -496,6 +496,14 @@ def test_disable_user(new_user_with_params): assert user.disabled is True assert len(user.provider_data) == 1 +def test_remove_provider(new_user_with_provider): + provider_ids = map(lambda x: x.provider_id, user.provider_data) + assert 'google.com' in provider_ids + user = auth.update_user(new_user_with_provider, delete_providers=['google.com']) + assert user.uid == new_user_with_params.uid + new_provider_ids = map(lambda x: x.provider_id, user.provider_data) + assert 'google.com' not in new_provider_ids + def test_delete_user(): user = auth.create_user() auth.delete_user(user.uid) diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 10dfe698f..6940c1b1f 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -663,6 +663,21 @@ def test_update_user_valid_since(self, user_mgt_app, arg): request = json.loads(recorder[0].body.decode()) assert request == {'localId': 'testuser', 'validSince': int(arg)} + @pytest.mark.parametrize('arg', [[], ['phone'], ['google.com', 'phone']]) + def test_update_user_delete_provider(self, user_mgt_app, arg): + user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') + user_mgt.update_user('testuser', delete_providers=arg) + request = json.loads(recorder[0].body.decode()) + assert request['deleteProvider'] == arg + + @pytest.mark.parametrize('arg', [[], ['phone'], ['google.com', 'phone']]) + def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): + user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') + user_mgt.update_user('testuser', delete_providers=arg, phone_number=auth.DELETE_ATTRIBUTE) + request = json.loads(recorder[0].body.decode()) + assert 'phone' in request['deleteProvider'] + assert len(set(request['deleteProvider'])) == len(request['deleteProvider']) + assert set(arg) - set(request['deleteProvider']) == set() class TestSetCustomUserClaims: From cf59514861a387d2f96f11c70eb7b16be64792b1 Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Mon, 20 Sep 2021 13:32:08 +0530 Subject: [PATCH 2/8] [fix] pylint --- firebase_admin/_auth_client.py | 2 +- firebase_admin/_user_mgt.py | 10 +++++----- integration/test_auth.py | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index 780bc94bd..02d17e044 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -336,7 +336,7 @@ def update_user(self, uid, **kwargs): # pylint: disable=differing-param-doc valid_since: An integer signifying the seconds since the epoch (optional). This field is set by ``revoke_refresh_tokens`` and it is discouraged to set this field directly. - delete_providers: The list of provider IDs to unlink, eg: 'google.com', 'password', etc. + delete_providers: The list of provider IDs to unlink, eg: 'google.com', 'password', etc. Returns: UserRecord: An updated UserRecord instance for the user. diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 3a220a9c8..9db80b415 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -700,7 +700,7 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, } remove = [] - removeProvider = [] + remove_provider = [] if display_name is not None: if display_name is DELETE_ATTRIBUTE: remove.append('DISPLAY_NAME') @@ -716,7 +716,7 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, if phone_number is not None: if phone_number is DELETE_ATTRIBUTE: - removeProvider.append('phone') + remove_provider.append('phone') else: payload['phoneNumber'] = _auth_utils.validate_phone(phone_number) @@ -728,9 +728,9 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, payload['customAttributes'] = _auth_utils.validate_custom_claims(json_claims) if delete_providers is not None and isinstance(delete_providers, list): - removeProvider += delete_providers - if removeProvider: - payload['deleteProvider'] = list(set(removeProvider)) + remove_provider += delete_providers + if remove_provider: + payload['deleteProvider'] = list(set(remove_provider)) payload = {k: v for k, v in payload.items() if v is not None} body, http_resp = self._make_request('post', '/accounts:update', json=payload) diff --git a/integration/test_auth.py b/integration/test_auth.py index 8433ed4d4..ac60bcd80 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -497,7 +497,7 @@ def test_disable_user(new_user_with_params): assert len(user.provider_data) == 1 def test_remove_provider(new_user_with_provider): - provider_ids = map(lambda x: x.provider_id, user.provider_data) + provider_ids = map(lambda x: x.provider_id, new_user_with_provider.provider_data) assert 'google.com' in provider_ids user = auth.update_user(new_user_with_provider, delete_providers=['google.com']) assert user.uid == new_user_with_params.uid From 2d6731e695c963c112c9820147d0db0271aa822f Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Mon, 20 Sep 2021 13:36:19 +0530 Subject: [PATCH 3/8] [fix] tests --- tests/test_user_mgt.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 6940c1b1f..57919b64a 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -663,14 +663,14 @@ def test_update_user_valid_since(self, user_mgt_app, arg): request = json.loads(recorder[0].body.decode()) assert request == {'localId': 'testuser', 'validSince': int(arg)} - @pytest.mark.parametrize('arg', [[], ['phone'], ['google.com', 'phone']]) + @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') user_mgt.update_user('testuser', delete_providers=arg) request = json.loads(recorder[0].body.decode()) assert request['deleteProvider'] == arg - @pytest.mark.parametrize('arg', [[], ['phone'], ['google.com', 'phone']]) + @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') user_mgt.update_user('testuser', delete_providers=arg, phone_number=auth.DELETE_ATTRIBUTE) From 6adade89453204009dbfac9202390859df635292 Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Mon, 20 Sep 2021 13:40:04 +0530 Subject: [PATCH 4/8] [fix] tests --- tests/test_user_mgt.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 57919b64a..fdd5e8239 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -668,7 +668,7 @@ def test_update_user_delete_provider(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') user_mgt.update_user('testuser', delete_providers=arg) request = json.loads(recorder[0].body.decode()) - assert request['deleteProvider'] == arg + assert set(request['deleteProvider']) == set(arg) @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): From 80591957c25100e51f6f053358ebf39763c2365a Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Sat, 25 Sep 2021 20:22:01 +0530 Subject: [PATCH 5/8] fix comments --- firebase_admin/_auth_client.py | 3 ++- firebase_admin/_auth_utils.py | 9 +++++++++ firebase_admin/_user_mgt.py | 6 ++---- integration/test_auth.py | 4 ++-- 4 files changed, 15 insertions(+), 7 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index 02d17e044..27dd5c7ce 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -336,7 +336,8 @@ def update_user(self, uid, **kwargs): # pylint: disable=differing-param-doc valid_since: An integer signifying the seconds since the epoch (optional). This field is set by ``revoke_refresh_tokens`` and it is discouraged to set this field directly. - delete_providers: The list of provider IDs to unlink, eg: 'google.com', 'password', etc. + providers_to_delete: The list of provider IDs to unlink, + eg: 'google.com', 'password', etc. Returns: UserRecord: An updated UserRecord instance for the user. diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index 02d32b659..b4b3e5824 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -266,6 +266,15 @@ def validate_action_type(action_type): Valid values are {1}'.format(action_type, ', '.join(VALID_EMAIL_ACTION_TYPES))) return action_type +def validate_provider_ids(provider_ids, required=False): + if provider_ids is None: + if required: + raise ValueError('Invalid provider IDs. The list must be non-empty.') + return [] + for provider_id in provider_ids: + validate_provider_id(provider_id, True) + return provider_ids + def build_update_mask(params): """Creates an update mask list from the given dictionary.""" mask = [] diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 9db80b415..c77c4d40d 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -688,7 +688,7 @@ def create_user(self, uid=None, display_name=None, email=None, phone_number=None 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, delete_providers=None): + valid_since=None, custom_claims=None, providers_to_delete=None): """Updates an existing user account with the specified properties""" payload = { 'localId': _auth_utils.validate_uid(uid, required=True), @@ -700,7 +700,7 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, } remove = [] - remove_provider = [] + remove_provider = _auth_utils.validate_provider_ids(providers_to_delete) if display_name is not None: if display_name is DELETE_ATTRIBUTE: remove.append('DISPLAY_NAME') @@ -727,8 +727,6 @@ def update_user(self, uid, display_name=None, email=None, phone_number=None, custom_claims, dict) else custom_claims payload['customAttributes'] = _auth_utils.validate_custom_claims(json_claims) - if delete_providers is not None and isinstance(delete_providers, list): - remove_provider += delete_providers if remove_provider: payload['deleteProvider'] = list(set(remove_provider)) diff --git a/integration/test_auth.py b/integration/test_auth.py index ac60bcd80..884fc62c8 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -497,11 +497,11 @@ def test_disable_user(new_user_with_params): assert len(user.provider_data) == 1 def test_remove_provider(new_user_with_provider): - provider_ids = map(lambda x: x.provider_id, new_user_with_provider.provider_data) + provider_ids = [provider.provider_id for provider in new_user_with_provider.provider_data] assert 'google.com' in provider_ids user = auth.update_user(new_user_with_provider, delete_providers=['google.com']) assert user.uid == new_user_with_params.uid - new_provider_ids = map(lambda x: x.provider_id, user.provider_data) + new_provider_ids = [provider.provider_id for provider in user.provider_data] assert 'google.com' not in new_provider_ids def test_delete_user(): From 5286b43f130d9211bf7f8ec77c9722d98b44998d Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Sat, 25 Sep 2021 20:24:23 +0530 Subject: [PATCH 6/8] fix tests --- integration/test_auth.py | 2 +- tests/test_user_mgt.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/integration/test_auth.py b/integration/test_auth.py index 884fc62c8..2dd2cb639 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -499,7 +499,7 @@ def test_disable_user(new_user_with_params): def test_remove_provider(new_user_with_provider): provider_ids = [provider.provider_id for provider in new_user_with_provider.provider_data] assert 'google.com' in provider_ids - user = auth.update_user(new_user_with_provider, delete_providers=['google.com']) + user = auth.update_user(new_user_with_provider, providers_to_delete=['google.com']) assert user.uid == new_user_with_params.uid new_provider_ids = [provider.provider_id for provider in user.provider_data] assert 'google.com' not in new_provider_ids diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index fdd5e8239..4b2fe4dd5 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -666,14 +666,14 @@ def test_update_user_valid_since(self, user_mgt_app, arg): @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') - user_mgt.update_user('testuser', delete_providers=arg) + user_mgt.update_user('testuser', providers_to_delete=arg) request = json.loads(recorder[0].body.decode()) assert set(request['deleteProvider']) == set(arg) @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') - user_mgt.update_user('testuser', delete_providers=arg, phone_number=auth.DELETE_ATTRIBUTE) + user_mgt.update_user('testuser', providers_to_delete=arg, phone_number=auth.DELETE_ATTRIBUTE) request = json.loads(recorder[0].body.decode()) assert 'phone' in request['deleteProvider'] assert len(set(request['deleteProvider'])) == len(request['deleteProvider']) From 81d0bff6c5e40f8306b1f6ab4a563fac057c3827 Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Sat, 25 Sep 2021 20:28:35 +0530 Subject: [PATCH 7/8] fix lint --- tests/test_user_mgt.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 4b2fe4dd5..979a12274 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -673,7 +673,9 @@ def test_update_user_delete_provider(self, user_mgt_app, arg): @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') - user_mgt.update_user('testuser', providers_to_delete=arg, phone_number=auth.DELETE_ATTRIBUTE) + user_mgt.update_user('testuser', + providers_to_delete=arg, + phone_number=auth.DELETE_ATTRIBUTE) request = json.loads(recorder[0].body.decode()) assert 'phone' in request['deleteProvider'] assert len(set(request['deleteProvider'])) == len(request['deleteProvider']) From a23e19e012a92d3e669af8ce0e9376aea762c40e Mon Sep 17 00:00:00 2001 From: Hardik Shah Date: Sat, 2 Oct 2021 11:42:22 +0530 Subject: [PATCH 8/8] [fix] address comments --- firebase_admin/_auth_utils.py | 4 ++-- tests/test_user_mgt.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index b4b3e5824..7aece495e 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -267,9 +267,9 @@ def validate_action_type(action_type): return action_type def validate_provider_ids(provider_ids, required=False): - if provider_ids is None: + if not provider_ids: if required: - raise ValueError('Invalid provider IDs. The list must be non-empty.') + raise ValueError('Invalid provider IDs. Provider ids should be provided') return [] for provider_id in provider_ids: validate_provider_id(provider_id, True) diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 979a12274..67447c6ba 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -670,7 +670,7 @@ def test_update_user_delete_provider(self, user_mgt_app, arg): request = json.loads(recorder[0].body.decode()) assert set(request['deleteProvider']) == set(arg) - @pytest.mark.parametrize('arg', [['phone'], ['google.com', 'phone']]) + @pytest.mark.parametrize('arg', [[], ['phone'], ['google.com'], ['google.com', 'phone']]) def test_update_user_delete_provider_and_phone(self, user_mgt_app, arg): user_mgt, recorder = _instrument_user_manager(user_mgt_app, 200, '{"localId":"testuser"}') user_mgt.update_user('testuser',