From aa8207d4994c0436afec3b8eeb560578b58c8d9b Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 7 Nov 2019 11:28:01 -0500 Subject: [PATCH 01/17] Add get_users() bulk lookup method --- firebase_admin/_identifier.py | 63 +++++++++++++++++++++++++++++++ firebase_admin/_user_mgt.py | 46 +++++++++++++++++++++++ firebase_admin/auth.py | 44 ++++++++++++++++++++++ integration/test_auth.py | 71 +++++++++++++++++++++++++++++++++++ tests/test_user_mgt.py | 41 ++++++++++++++++++++ 5 files changed, 265 insertions(+) create mode 100644 firebase_admin/_identifier.py diff --git a/firebase_admin/_identifier.py b/firebase_admin/_identifier.py new file mode 100644 index 000000000..d56cef2e0 --- /dev/null +++ b/firebase_admin/_identifier.py @@ -0,0 +1,63 @@ +# Copyright 2019 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Classes to uniquely identify a user.""" + +class UserIdentifier: + """Identifies a user to be looked up.""" + + +class UidIdentifier(UserIdentifier): + """Used for looking up an account by uid. + + See ``auth.get_user()``. + """ + + def __init__(self, uid): + """Constructs a new UidIdentifier. + + Args: + uid: A user ID string. + """ + self.uid = uid + + +class EmailIdentifier(UserIdentifier): + """Used for looking up an account by email. + + See ``auth.get_user()``. + """ + + def __init__(self, email): + """Constructs a new EmailIdentifier. + + Args: + email: A user email address string. + """ + self.email = email + + +class PhoneIdentifier(UserIdentifier): + """Used for looking up an account by phone number. + + See ``auth.get_user()``. + """ + + def __init__(self, phone_number): + """Constructs a new PhoneIdentifier. + + Args: + phone_number: A phone number string. + """ + self.phone_number = phone_number diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 533259e70..a4b2ea48b 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -21,6 +21,7 @@ import requests from firebase_admin import _auth_utils +from firebase_admin import _identifier from firebase_admin import _user_import @@ -483,6 +484,51 @@ def get_user(self, **kwargs): http_response=http_resp) return body['users'][0] + def get_users(self, identifiers): + """Looks up multiple users by their identifiers (uid, email, etc.) + + Args: + identifiers: UserIdentifier[]: The identifiers indicating the user + to be looked up. Must have <= 100 entries. + + Returns: + list[dict[string, string]]: List of dicts representing the json + UserInfo responses from the server. + + Raises: + ValueError: If any of the identifiers are invalid or if more than + 100 identifiers are specified. + """ + if not identifiers: + return [] + if len(identifiers) > 100: + raise ValueError('`identifiers` parameter must have <= 100 entries.') + + payload = {} + for identifier in identifiers: + if isinstance(identifier, _identifier.UidIdentifier): + _auth_utils.validate_uid(identifier.uid, required=True) + payload['localId'] = payload.get('localId', []) + [identifier.uid] + elif isinstance(identifier, _identifier.EmailIdentifier): + _auth_utils.validate_email(identifier.email, required=True) + payload['email'] = payload.get('email', []) + [identifier.email] + elif isinstance(identifier, _identifier.PhoneIdentifier): + _auth_utils.validate_phone(identifier.phone_number, required=True) + payload['phoneNumber'] = payload.get('phoneNumber', []) + [identifier.phone_number] + else: + raise ValueError('Invalid argument `identifiers`. Unrecognized type.') + + try: + body, http_resp = self._client.body_and_response( + 'post', '/accounts:lookup', json=payload) + except requests.exceptions.RequestException as error: + raise _auth_utils.handle_auth_backend_error(error) + else: + if not http_resp.ok: + raise _auth_utils.UnexpectedResponseError( + 'Failed to get users.', http_response=http_resp) + return body.get('users', []) + def list_users(self, page_token=None, max_results=MAX_LIST_USERS_RESULTS): """Retrieves a batch of users.""" if page_token is not None: diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 6f85e622c..2af15de43 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -24,6 +24,7 @@ import firebase_admin from firebase_admin import _auth_utils from firebase_admin import _http_client +from firebase_admin import _identifier from firebase_admin import _token_gen from firebase_admin import _user_import from firebase_admin import _user_mgt @@ -62,6 +63,10 @@ 'UserProvider', 'UserRecord', + 'UidIdentifier', + 'EmailIdentifier', + 'PhoneIdentifier', + 'create_custom_token', 'create_session_cookie', 'create_user', @@ -72,6 +77,7 @@ 'get_user', 'get_user_by_email', 'get_user_by_phone_number', + 'get_users', 'import_users', 'list_users', 'revoke_refresh_tokens', @@ -109,6 +115,10 @@ UserProvider = _user_import.UserProvider UserRecord = _user_mgt.UserRecord +UidIdentifier = _identifier.UidIdentifier +EmailIdentifier = _identifier.EmailIdentifier +PhoneIdentifier = _identifier.PhoneIdentifier + def _get_auth_service(app): """Returns an _AuthService instance for an App. @@ -309,6 +319,40 @@ def get_user_by_phone_number(phone_number, app=None): return UserRecord(response) +def get_users(identifiers, app=None): + """Gets the user data corresponding to the specified identifiers. + + There are no ordering guarantees; in particular, the nth entry in the + result list is not guaranteed to correspond to the nth entry in the input + parameters list. + + If a given user doesn't exist, then no entry will be returned for it in the + results, implying that the results list may have fewer entries than the + identifiers list. + + Only a maximum of 100 identifiers may be supplied. If more than 100 + identifiers are supplied, this method will immediately raise a ValueError. + + Args: + identifiers (list[Identifier]): A list of ``Identifier`` instances used + to indicate which user records should be returned. Must have <= 100 + entries. + app: An App instance (optional). + + Returns: + list[UserRecord]: A list of ``UserRecord`` instances corresponding to the + specified identifiers. This could be empty if no users were + successfully looked up. + + Raises: + ValueError: If any of the identifiers are invalid or if more than 100 + identifiers are specified. + """ + user_manager = _get_auth_service(app).user_manager + response = user_manager.get_users(identifiers=identifiers) + return [UserRecord(data) for data in response] + + def list_users(page_token=None, max_results=_user_mgt.MAX_LIST_USERS_RESULTS, app=None): """Retrieves a page of user accounts from a Firebase project. diff --git a/integration/test_auth.py b/integration/test_auth.py index 5d26dd9f1..3fe8ab18a 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -17,6 +17,7 @@ import datetime import random import time +from typing import List from urllib import parse import uuid @@ -188,6 +189,23 @@ def new_user_list(): for uid in users: auth.delete_user(uid) +@pytest.fixture +def new_user_record_list() -> List[auth.UserRecord]: + uid1, email1 = _random_id() + uid2, email2 = _random_id() + uid3, email3 = _random_id() + users = [ + auth.create_user( + uid=uid1, email=email1, password='password', phone_number=_random_phone()), + auth.create_user( + uid=uid2, email=email2, password='password', phone_number=_random_phone()), + auth.create_user( + uid=uid3, email=email3, password='password', phone_number=_random_phone()), + ] + yield users + for user in users: + auth.delete_user(user.uid) + @pytest.fixture def new_user_email_unverified(): random_id, email = _random_id() @@ -219,6 +237,59 @@ def test_get_user(new_user_with_params): provider_ids = sorted([provider.provider_id for provider in user.provider_data]) assert provider_ids == ['password', 'phone'] +class TestGetUsers: + @staticmethod + def _map_user_record_to_uid_email_phones(user_record): + return { + 'uid': user_record.uid, + 'email': user_record.email, + 'phone_number': user_record.phone_number + } + + def test_returns_users_by_various_identifier_types_in_a_single_call(self, new_user_record_list): + users = auth.get_users([ + auth.UidIdentifier(new_user_record_list[0].uid), + auth.EmailIdentifier(new_user_record_list[1].email), + auth.PhoneIdentifier(new_user_record_list[2].phone_number)]) + actual = sorted( + list(map(self._map_user_record_to_uid_email_phones, users)), + key=lambda user: user['uid']) + expected = sorted( + list(map(self._map_user_record_to_uid_email_phones, new_user_record_list)), + key=lambda user: user['uid']) + + assert actual == expected + + def test_returns_found_users_and_ignores_non_existing_users(self, new_user_record_list): + users = auth.get_users([ + auth.UidIdentifier(new_user_record_list[0].uid), + auth.UidIdentifier('uid_that_doesnt_exist'), + auth.UidIdentifier(new_user_record_list[2].uid)]) + actual = sorted( + list(map(self._map_user_record_to_uid_email_phones, users)), + key=lambda user: user['uid']) + expected = sorted( + list(map( + self._map_user_record_to_uid_email_phones, + [new_user_record_list[0], new_user_record_list[2]])), + key=lambda user: user['uid']) + + assert actual == expected + + def test_returns_nothing_when_queried_for_only_non_existing_users(self): + users = auth.get_users([auth.UidIdentifier('non-existing user')]) + + assert users == [] + + def test_de_dups_duplicate_users(self, new_user): + users = auth.get_users([ + auth.UidIdentifier(new_user.uid), + auth.UidIdentifier(new_user.uid)]) + actual = list(map(self._map_user_record_to_uid_email_phones, users)) + expected = list(map(self._map_user_record_to_uid_email_phones, [new_user])) + assert actual == expected + + def test_list_users(new_user_list): err_msg_template = ( 'Missing {field} field. A common cause would be forgetting to add the "Firebase ' + diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 958bbf9c4..16092eeca 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -301,6 +301,47 @@ def test_get_user_by_phone_http_error(self, user_mgt_app): assert excinfo.value.cause is not None +class TestGetUsers: + + def test_should_reject_when_given_more_than_100_identifiers(self, user_mgt_app): + identifiers = [auth.UidIdentifier('id' + str(i)) for i in range(101)] + with pytest.raises(ValueError): + auth.get_users(identifiers, app=user_mgt_app) + + def test_should_return_no_results_when_given_no_identifiers(self, user_mgt_app): + assert auth.get_users([], app=user_mgt_app) == [] + + def test_should_return_no_results_when_given_identifiers_that_do_not_exist(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 200, '{}') + user_records = auth.get_users( + [auth.UidIdentifier('id that doesnt exist')], + app=user_mgt_app) + assert user_records == [] + + def test_should_be_rejected_when_given_an_invalid_uid(self, user_mgt_app): + with pytest.raises(ValueError): + auth.get_users([auth.UidIdentifier('too long ' + '.'*128)], app=user_mgt_app) + + def test_should_be_rejected_when_given_an_invalid_email(self, user_mgt_app): + with pytest.raises(ValueError): + auth.get_users([auth.EmailIdentifier('invalid email addr')], app=user_mgt_app) + + def test_should_be_rejected_when_given_an_invalid_phone_number(self, user_mgt_app): + with pytest.raises(ValueError): + auth.get_users([auth.PhoneIdentifier('invalid phone number')], app=user_mgt_app) + + def test_should_be_rejected_when_given_a_single_bad_identifier(self, user_mgt_app): + identifiers = [ + auth.UidIdentifier('valid_id1'), + auth.UidIdentifier('valid_id2'), + auth.UidIdentifier('invalid id; too long. ' + '.'*128), + auth.UidIdentifier('valid_id4'), + auth.UidIdentifier('valid_id5')] + + with pytest.raises(ValueError): + auth.get_users(identifiers, app=user_mgt_app) + + class TestCreateUser: already_exists_errors = { From a83f66e8e51e2aadd988e7e54a108114ef54aed3 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 6 Nov 2019 13:50:12 -0500 Subject: [PATCH 02/17] Add delete_users() bulk deletion method --- firebase_admin/_user_mgt.py | 118 ++++++++++++++++++++++++++++++++++++ firebase_admin/auth.py | 37 +++++++++++ integration/test_auth.py | 67 ++++++++++++++++++++ tests/test_user_mgt.py | 41 +++++++++++++ 4 files changed, 263 insertions(+) diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index a4b2ea48b..195861131 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -332,6 +332,85 @@ def iterate_all(self): return _UserIterator(self) +class DeleteUsersResult: + """Represents the result of the ``auth.delete_users()`` API.""" + + def __init__(self, result, total): + """Constructs a DeleteUsersResult. + + Args: + result: BatchDeleteAccountsResponse: The proto response, wrapped in a + BatchDeleteAccountsResponse instance. + total: integer: Total number of deletion attempts. + """ + errors = result.errors + self._success_count = total - len(errors) + self._failure_count = len(errors) + self._errors = errors + + @property + def success_count(self): + """Returns the number of users that were deleted successfully (possibly + zero). + + Users that did not exist prior to calling delete_users() will be + considered to be successfully deleted. + """ + return self._success_count + + @property + def failure_count(self): + """Returns the number of users that failed to be deleted (possibly + zero). + """ + return self._failure_count + + @property + def errors(self): + """A list of ``auth.BatchDeleteErrorInfo`` instances describing the + errors that were encountered during the deletion. Length of this list + is equal to `failure_count`. + """ + return self._errors + + +class BatchDeleteErrorInfo: + """Represents an error that occurred while attempting to delete a batch of + users. + """ + + def __init__(self, err): + """Constructs a BatchDeleteErrorInfo instance, corresponding to the + json representing the BatchDeleteErrorInfo proto. + + Args: + err: A dictionary with 'index', 'local_id' and 'message' fields, + representing the BatchDeleteErrorInfo dictionary that's + returned by the server. + """ + self.index = err.get('index', 0) + self.local_id = err.get('local_id', "") + self.message = err.get('message', "") + + +class BatchDeleteAccountsResponse: + """Represents the results of a delete_users() call.""" + + def __init__(self, errors=None): + """Constructs a BatchDeleteAccountsResponse instance, corresponseing to + the json representing the BatchDeleteAccountsResponse proto. + + Args: + errors: List of dictionaries, with each dictionary representing a + BatchDeleteErrorInfo instance as returned by the server. None + implies an empty list. + """ + if errors: + self.errors = [BatchDeleteErrorInfo(err) for err in errors] + else: + self.errors = [] + + class ProviderUserInfo(UserInfo): """Contains metadata regarding how a user is known by a particular identity provider.""" @@ -638,6 +717,45 @@ def delete_user(self, uid): raise _auth_utils.UnexpectedResponseError( 'Failed to delete user: {0}.'.format(uid), http_response=http_resp) + def delete_users(self, uids, force_delete=False): + """Deletes the users identified by the specified user ids. + + Args: + uids: A list of strings indicating the uids of the users to be deleted. + Must have <= 1000 entries. + force_delete: Optional parameter that indicates if users should be + deleted, even if they're not disabled. Defaults to False. + + + Returns: + BatchDeleteAccountsResponse: Server's proto response, wrapped in a + python object. + + Raises: + ValueError: If any of the identifiers are invalid or if more than 1000 + identifiers are specified. + """ + if not uids: + return BatchDeleteAccountsResponse() + + if len(uids) > 100: + raise ValueError("`uids` paramter must have <= 100 entries.") + for uid in uids: + _auth_utils.validate_uid(uid, required=True) + + try: + body, http_resp = self._client.body_and_response( + 'post', '/accounts:batchDelete', + json={'localIds': uids, 'force': force_delete}) + except requests.exceptions.RequestException as error: + raise _auth_utils.handle_auth_backend_error(error) + else: + if not isinstance(body, dict): + raise _auth_utils.UnexpectedResponseError( + 'Unexpected response from server while attempting to delete users.', + http_response=http_resp) + return BatchDeleteAccountsResponse(body.get('errors', [])) + def import_users(self, users, hash_alg=None): """Imports the given list of users to Firebase Auth.""" try: diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 2af15de43..60cafee47 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -71,6 +71,7 @@ 'create_session_cookie', 'create_user', 'delete_user', + 'delete_users', 'generate_email_verification_link', 'generate_password_reset_link', 'generate_sign_in_with_email_link', @@ -90,6 +91,7 @@ ActionCodeSettings = _user_mgt.ActionCodeSettings CertificateFetchError = _token_gen.CertificateFetchError DELETE_ATTRIBUTE = _user_mgt.DELETE_ATTRIBUTE +DeleteUsersResult = _user_mgt.DeleteUsersResult EmailAlreadyExistsError = _auth_utils.EmailAlreadyExistsError ErrorInfo = _user_import.ErrorInfo ExpiredIdTokenError = _token_gen.ExpiredIdTokenError @@ -490,6 +492,41 @@ def delete_user(uid, app=None): user_manager.delete_user(uid) +def delete_users(uids, force_delete=False, app=None): + """Deletes the users specified by the given identifiers. + + Deleting a non-existing user won't generate an error. (i.e. this method is + idempotent.) Non-existing users will be considered to be successfully + deleted, and will therefore be counted in the + DeleteUserResult.success_count value. + + Unless the optional force_delete flag is set to true, only users that are + already disabled will be deleted. + + Only a maximum of 1000 identifiers may be supplied. If more than 1000 + identifiers are supplied, this method will immediately raise a ValueError. + + Args: + uids: A list of strings indicating the uids of the users to be deleted. + Must have <= 1000 entries. + force_delete: Optional parameter that indicates if users should be + deleted, even if they're not disabled. Defaults to False. + app: An App instance (optional). + + Returns: + DeleteUsersResult: The total number of successful/failed deletions, as + well as the array of errors that correspond to the failed + deletions. + + Raises: + ValueError: If any of the identifiers are invalid or if more than 1000 + identifiers are specified. + """ + user_manager = _get_auth_service(app).user_manager + result = user_manager.delete_users(uids, force_delete) + return DeleteUsersResult(result, len(uids)) + + def import_users(users, hash_alg=None, app=None): """Imports the specified list of users into Firebase Auth. diff --git a/integration/test_auth.py b/integration/test_auth.py index 3fe8ab18a..b1e4497e5 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -408,6 +408,73 @@ def test_delete_user(): with pytest.raises(auth.UserNotFoundError): auth.get_user(user.uid) + +class TestDeleteUsers: + def test_delete_multiple_disabled_users(self): + uid1 = auth.create_user(disabled=True).uid + uid2 = auth.create_user(disabled=True).uid + uid3 = auth.create_user(disabled=True).uid + + delete_users_result = auth.delete_users([uid1, uid2, uid3]) + assert delete_users_result.success_count == 3 + assert delete_users_result.failure_count == 0 + assert len(delete_users_result.errors) == 0 + + user_records = auth.get_users( + [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) + assert len(user_records) == 0 + + def test_fails_to_delete_enabled_users(self): + uid1 = auth.create_user(disabled=False).uid + uid2 = auth.create_user(disabled=True).uid + uid3 = auth.create_user(disabled=False).uid + + try: + delete_users_result = auth.delete_users([uid1, uid2, uid3]) + assert delete_users_result.success_count == 1 + assert delete_users_result.failure_count == 2 + assert len(delete_users_result.errors) == 2 + assert delete_users_result.errors[0].index == 0 + assert delete_users_result.errors[0].message.startswith('NOT_DISABLED') + assert delete_users_result.errors[1].index == 2 + assert delete_users_result.errors[1].message.startswith('NOT_DISABLED') + + user_records = auth.get_users( + [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) + assert len(user_records) == 2 + assert {ur.uid for ur in user_records} == set([uid1, uid3]) + + finally: + auth.delete_users([uid1, uid3], force_delete=True) + + def test_deletes_enabled_users_when_force_is_true(self): + uid1 = auth.create_user(disabled=False).uid + uid2 = auth.create_user(disabled=True).uid + uid3 = auth.create_user(disabled=False).uid + + delete_users_result = auth.delete_users([uid1, uid2, uid3], force_delete=True) + assert delete_users_result.success_count == 3 + assert delete_users_result.failure_count == 0 + assert len(delete_users_result.errors) == 0 + + user_records = auth.get_users( + [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) + assert len(user_records) == 0 + + def test_is_idempotent(self): + uid = auth.create_user(disabled=True).uid + + delete_users_result = auth.delete_users([uid]) + assert delete_users_result.success_count == 1 + assert delete_users_result.failure_count == 0 + + # Delete the user again, ensuring that everything still counts as a + # success. + delete_users_result = auth.delete_users([uid]) + assert delete_users_result.success_count == 1 + assert delete_users_result.failure_count == 0 + + def test_revoke_refresh_tokens(new_user): user = auth.get_user(new_user.uid) old_valid_after = user.tokens_valid_after_timestamp diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 16092eeca..5a7d43dfd 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -653,6 +653,47 @@ def test_delete_user_unexpected_response(self, user_mgt_app): assert isinstance(excinfo.value, exceptions.UnknownError) +class TestDeleteUsers: + + def test_empty_list(self, user_mgt_app): + delete_users_result = auth.delete_users([], app=user_mgt_app) + assert delete_users_result.success_count == 0 + assert delete_users_result.failure_count == 0 + assert len(delete_users_result.errors) == 0 + + def test_too_many_identifiers_should_fail(self, user_mgt_app): + ids = ['id' + str(i) for i in range(101)] + with pytest.raises(ValueError): + auth.delete_users(ids, app=user_mgt_app) + + def test_invalid_id_should_fail(self, user_mgt_app): + ids = ['too long ' + '.'*128] + with pytest.raises(ValueError): + auth.delete_users(ids, app=user_mgt_app) + + def test_should_index_errors_correctly_in_results(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 200, """{ + "errors": [{ + "index": 0, + "localId": "uid1", + "message": "NOT_DISABLED : Disable the account before batch deletion." + }, { + "index": 2, + "localId": "uid3", + "message": "something awful" + }] + }""") + + delete_users_result = auth.delete_users(['uid1', 'uid2', 'uid3', 'uid4'], app=user_mgt_app) + assert delete_users_result.success_count == 2 + assert delete_users_result.failure_count == 2 + assert len(delete_users_result.errors) == 2 + assert delete_users_result.errors[0].index == 0 + assert delete_users_result.errors[0].message.startswith('NOT_DISABLED') + assert delete_users_result.errors[1].index == 2 + assert delete_users_result.errors[1].message == 'something awful' + + class TestListUsers: @pytest.mark.parametrize('arg', [None, 'foo', list(), dict(), 0, -1, 1001, False]) From c5f57d08ea347cdf359331a6212058d39ed5cd67 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 30 Jan 2020 17:38:16 -0500 Subject: [PATCH 03/17] add last_refresh_timestamp --- firebase_admin/_user_mgt.py | 23 +++++++++++++++++++++-- integration/test_auth.py | 23 +++++++++++++++++++++-- requirements.txt | 1 + 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 195861131..0ead9f362 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -19,6 +19,7 @@ from urllib import parse import requests +import iso8601 from firebase_admin import _auth_utils from firebase_admin import _identifier @@ -42,11 +43,14 @@ def __init__(self, description): class UserMetadata: """Contains additional metadata associated with a user account.""" - def __init__(self, creation_timestamp=None, last_sign_in_timestamp=None): + def __init__(self, creation_timestamp=None, last_sign_in_timestamp=None, + last_refresh_timestamp=None): self._creation_timestamp = _auth_utils.validate_timestamp( creation_timestamp, 'creation_timestamp') self._last_sign_in_timestamp = _auth_utils.validate_timestamp( last_sign_in_timestamp, 'last_sign_in_timestamp') + self._last_refresh_timestamp = _auth_utils.validate_timestamp( + last_refresh_timestamp, 'last_refresh_timestamp') @property def creation_timestamp(self): @@ -66,6 +70,16 @@ def last_sign_in_timestamp(self): """ return self._last_sign_in_timestamp + @property + def last_refresh_timestamp(self): + """The time at which the user was last active (ID token refreshed). + + Returns: + integer: Milliseconds since epoch timestamp, or None if the user was + never active. + """ + return self._last_refresh_timestamp + class UserInfo: """A collection of standard profile information for a user. @@ -217,7 +231,12 @@ def _int_or_none(key): if key in self._data: return int(self._data[key]) return None - return UserMetadata(_int_or_none('createdAt'), _int_or_none('lastLoginAt')) + last_refresh_at_millis = None + last_refresh_at_iso8601 = self._data.get('lastRefreshAt', None) + if last_refresh_at_iso8601 is not None: + last_refresh_at_millis = iso8601.parse_date(last_refresh_at_iso8601).timestamp() * 1000 + return UserMetadata( + _int_or_none('createdAt'), _int_or_none('lastLoginAt'), last_refresh_at_millis) @property def provider_data(self): diff --git a/integration/test_auth.py b/integration/test_auth.py index b1e4497e5..293a8b037 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -47,7 +47,7 @@ def _sign_in(custom_token, api_key): return resp.json().get('idToken') def _sign_in_with_password(email, password, api_key): - body = {'email': email, 'password': password} + body = {'email': email, 'password': password, 'returnSecureToken': True} params = {'key' : api_key} resp = requests.request('post', _verify_password_url, params=params, json=body) resp.raise_for_status() @@ -163,7 +163,7 @@ def new_user(): auth.delete_user(user.uid) @pytest.fixture -def new_user_with_params(): +def new_user_with_params() -> auth.UserRecord: random_id, email = _random_id() phone = _random_phone() user = auth.create_user( @@ -289,6 +289,25 @@ def test_de_dups_duplicate_users(self, new_user): expected = list(map(self._map_user_record_to_uid_email_phones, [new_user])) assert actual == expected +def test_last_refresh_timestamp(new_user_with_params: auth.UserRecord, api_key): + # new users should not have a last_refresh_timestamp set + assert new_user_with_params.user_metadata.last_refresh_timestamp is None + + # login to cause the last_refresh_timestamp to be set + _sign_in_with_password(new_user_with_params.email, 'secret', api_key) + new_user_with_params = auth.get_user(new_user_with_params.uid) + + # Ensure the last refresh time occurred at approximately 'now'. (With a + # tolerance of up to 1 minute; we ideally want to ensure that any timezone + # considerations are handled properly, so as long as we're within an hour, + # we're in good shape.) + millis_per_second = 1000 + seconds_per_minute = 60 + millis_per_minute = millis_per_second * seconds_per_minute + + last_refresh_timestamp = new_user_with_params.user_metadata.last_refresh_timestamp + assert last_refresh_timestamp == pytest.approx( + time.time()*millis_per_second, 1*millis_per_minute) def test_list_users(new_user_list): err_msg_template = ( diff --git a/requirements.txt b/requirements.txt index d7fb6d736..a6bdb5c42 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,3 +8,4 @@ google-api-core[grpc] >= 1.14.0, < 2.0.0dev; platform.python_implementation != ' google-api-python-client >= 1.7.8 google-cloud-firestore >= 1.4.0; platform.python_implementation != 'PyPy' google-cloud-storage >= 1.18.0 +iso8601 >= 0.1.12 From 0e282f7aebf6df5d6020fe29bb58966f6f32132c Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 30 Jan 2020 17:45:28 -0500 Subject: [PATCH 04/17] Fix lint error --- firebase_admin/_user_mgt.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 0ead9f362..d8796a6d5 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -596,6 +596,8 @@ def get_users(self, identifiers): Raises: ValueError: If any of the identifiers are invalid or if more than 100 identifiers are specified. + UnexpectedResponseError: If the backend server responds with an + unexpected message. """ if not identifiers: return [] @@ -753,6 +755,8 @@ def delete_users(self, uids, force_delete=False): Raises: ValueError: If any of the identifiers are invalid or if more than 1000 identifiers are specified. + UnexpectedResponseError: If the backend server responds with an + unexpected message. """ if not uids: return BatchDeleteAccountsResponse() From 070c1a8c597220ad3ddeba2659054e110cb3687f Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 30 Jan 2020 18:41:18 -0500 Subject: [PATCH 05/17] fixup! Add delete_users() bulk deletion method --- firebase_admin/auth.py | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 60cafee47..0bb167db4 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -492,7 +492,7 @@ def delete_user(uid, app=None): user_manager.delete_user(uid) -def delete_users(uids, force_delete=False, app=None): +def delete_users(uids, app=None): """Deletes the users specified by the given identifiers. Deleting a non-existing user won't generate an error. (i.e. this method is @@ -500,17 +500,12 @@ def delete_users(uids, force_delete=False, app=None): deleted, and will therefore be counted in the DeleteUserResult.success_count value. - Unless the optional force_delete flag is set to true, only users that are - already disabled will be deleted. - Only a maximum of 1000 identifiers may be supplied. If more than 1000 identifiers are supplied, this method will immediately raise a ValueError. Args: uids: A list of strings indicating the uids of the users to be deleted. Must have <= 1000 entries. - force_delete: Optional parameter that indicates if users should be - deleted, even if they're not disabled. Defaults to False. app: An App instance (optional). Returns: @@ -523,7 +518,7 @@ def delete_users(uids, force_delete=False, app=None): identifiers are specified. """ user_manager = _get_auth_service(app).user_manager - result = user_manager.delete_users(uids, force_delete) + result = user_manager.delete_users(uids, force_delete=True) return DeleteUsersResult(result, len(uids)) From 466890bc7e8cf049e88a81eef560ef1c279cf091 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 30 Jan 2020 18:43:21 -0500 Subject: [PATCH 06/17] Remove force_delete flag --- integration/test_auth.py | 43 +++------------------------------------- 1 file changed, 3 insertions(+), 40 deletions(-) diff --git a/integration/test_auth.py b/integration/test_auth.py index 293a8b037..890b4a987 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -429,9 +429,9 @@ def test_delete_user(): class TestDeleteUsers: - def test_delete_multiple_disabled_users(self): + def test_delete_multiple_users(self): uid1 = auth.create_user(disabled=True).uid - uid2 = auth.create_user(disabled=True).uid + uid2 = auth.create_user(disabled=False).uid uid3 = auth.create_user(disabled=True).uid delete_users_result = auth.delete_users([uid1, uid2, uid3]) @@ -443,45 +443,8 @@ def test_delete_multiple_disabled_users(self): [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) assert len(user_records) == 0 - def test_fails_to_delete_enabled_users(self): - uid1 = auth.create_user(disabled=False).uid - uid2 = auth.create_user(disabled=True).uid - uid3 = auth.create_user(disabled=False).uid - - try: - delete_users_result = auth.delete_users([uid1, uid2, uid3]) - assert delete_users_result.success_count == 1 - assert delete_users_result.failure_count == 2 - assert len(delete_users_result.errors) == 2 - assert delete_users_result.errors[0].index == 0 - assert delete_users_result.errors[0].message.startswith('NOT_DISABLED') - assert delete_users_result.errors[1].index == 2 - assert delete_users_result.errors[1].message.startswith('NOT_DISABLED') - - user_records = auth.get_users( - [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) - assert len(user_records) == 2 - assert {ur.uid for ur in user_records} == set([uid1, uid3]) - - finally: - auth.delete_users([uid1, uid3], force_delete=True) - - def test_deletes_enabled_users_when_force_is_true(self): - uid1 = auth.create_user(disabled=False).uid - uid2 = auth.create_user(disabled=True).uid - uid3 = auth.create_user(disabled=False).uid - - delete_users_result = auth.delete_users([uid1, uid2, uid3], force_delete=True) - assert delete_users_result.success_count == 3 - assert delete_users_result.failure_count == 0 - assert len(delete_users_result.errors) == 0 - - user_records = auth.get_users( - [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) - assert len(user_records) == 0 - def test_is_idempotent(self): - uid = auth.create_user(disabled=True).uid + uid = auth.create_user().uid delete_users_result = auth.delete_users([uid]) assert delete_users_result.success_count == 1 From 598ffc917e92a6bd797d7f64136f8c1c434b31ca Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 10:21:28 -0500 Subject: [PATCH 07/17] review feedback (except iso8601) --- .../{_identifier.py => _user_identifier.py} | 2 +- firebase_admin/_user_mgt.py | 63 ++++++++++--- firebase_admin/auth.py | 40 ++++++--- integration/test_auth.py | 55 +++++++----- tests/test_user_mgt.py | 88 +++++++++++++++---- 5 files changed, 185 insertions(+), 63 deletions(-) rename firebase_admin/{_identifier.py => _user_identifier.py} (98%) diff --git a/firebase_admin/_identifier.py b/firebase_admin/_user_identifier.py similarity index 98% rename from firebase_admin/_identifier.py rename to firebase_admin/_user_identifier.py index d56cef2e0..cc76c0869 100644 --- a/firebase_admin/_identifier.py +++ b/firebase_admin/_user_identifier.py @@ -1,4 +1,4 @@ -# Copyright 2019 Google Inc. +# Copyright 2020 Google Inc. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index d8796a6d5..93d81ced7 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -22,7 +22,7 @@ import iso8601 from firebase_admin import _auth_utils -from firebase_admin import _identifier +from firebase_admin import _user_identifier from firebase_admin import _user_import @@ -233,7 +233,7 @@ def _int_or_none(key): return None last_refresh_at_millis = None last_refresh_at_iso8601 = self._data.get('lastRefreshAt', None) - if last_refresh_at_iso8601 is not None: + if last_refresh_at_iso8601: last_refresh_at_millis = iso8601.parse_date(last_refresh_at_iso8601).timestamp() * 1000 return UserMetadata( _int_or_none('createdAt'), _int_or_none('lastLoginAt'), last_refresh_at_millis) @@ -300,6 +300,33 @@ def password_salt(self): return self._data.get('salt') +class GetUsersResult: + """Represents the result of the ``auth.get_users()`` API.""" + + def __init__(self, users, not_found): + """Constructs a GetUsersResult. + + Args: + users: List of `UserRecord` instances. + not_found: List of `UserIdentifier` instances. + """ + self._users = users + self._not_found = not_found + + @property + def users(self): + """Set of UserRecords, corresponding to the set of users that were + requested. Only users that were found are listed here. The result set + is unordered. + """ + return self._users + + @property + def not_found(self): + """Set of UserIdentifiers that were requested, but not found.""" + return self._not_found + + class ListUsersPage: """Represents a page of user records exported from a Firebase project. @@ -407,9 +434,21 @@ def __init__(self, err): representing the BatchDeleteErrorInfo dictionary that's returned by the server. """ - self.index = err.get('index', 0) - self.local_id = err.get('local_id', "") - self.message = err.get('message', "") + self._index = err.get('index', 0) + self._local_id = err.get('local_id', "") + self._reason = err.get('message', '') + + @property + def index(self): + return self._index + + @property + def reason(self): + return self._reason + + @property + def local_id(self): + return self._local_id class BatchDeleteAccountsResponse: @@ -606,17 +645,19 @@ def get_users(self, identifiers): payload = {} for identifier in identifiers: - if isinstance(identifier, _identifier.UidIdentifier): + if isinstance(identifier, _user_identifier.UidIdentifier): _auth_utils.validate_uid(identifier.uid, required=True) payload['localId'] = payload.get('localId', []) + [identifier.uid] - elif isinstance(identifier, _identifier.EmailIdentifier): + elif isinstance(identifier, _user_identifier.EmailIdentifier): _auth_utils.validate_email(identifier.email, required=True) payload['email'] = payload.get('email', []) + [identifier.email] - elif isinstance(identifier, _identifier.PhoneIdentifier): + elif isinstance(identifier, _user_identifier.PhoneIdentifier): _auth_utils.validate_phone(identifier.phone_number, required=True) payload['phoneNumber'] = payload.get('phoneNumber', []) + [identifier.phone_number] else: - raise ValueError('Invalid argument `identifiers`. Unrecognized type.') + raise ValueError( + 'Invalid entry in "identifiers" list. Unsupported type: {}' + .format(type(identifier))) try: body, http_resp = self._client.body_and_response( @@ -761,8 +802,8 @@ def delete_users(self, uids, force_delete=False): if not uids: return BatchDeleteAccountsResponse() - if len(uids) > 100: - raise ValueError("`uids` paramter must have <= 100 entries.") + if len(uids) > 1000: + raise ValueError("`uids` paramter must have <= 1000 entries.") for uid in uids: _auth_utils.validate_uid(uid, required=True) diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 0bb167db4..1f77e9aa4 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -24,7 +24,7 @@ import firebase_admin from firebase_admin import _auth_utils from firebase_admin import _http_client -from firebase_admin import _identifier +from firebase_admin import _user_identifier from firebase_admin import _token_gen from firebase_admin import _user_import from firebase_admin import _user_mgt @@ -63,6 +63,7 @@ 'UserProvider', 'UserRecord', + 'UserIdentifier', 'UidIdentifier', 'EmailIdentifier', 'PhoneIdentifier', @@ -97,6 +98,7 @@ ExpiredIdTokenError = _token_gen.ExpiredIdTokenError ExpiredSessionCookieError = _token_gen.ExpiredSessionCookieError ExportedUserRecord = _user_mgt.ExportedUserRecord +GetUsersResult = _user_mgt.GetUsersResult ImportUserRecord = _user_import.ImportUserRecord InsufficientPermissionError = _auth_utils.InsufficientPermissionError InvalidDynamicLinkDomainError = _auth_utils.InvalidDynamicLinkDomainError @@ -117,9 +119,10 @@ UserProvider = _user_import.UserProvider UserRecord = _user_mgt.UserRecord -UidIdentifier = _identifier.UidIdentifier -EmailIdentifier = _identifier.EmailIdentifier -PhoneIdentifier = _identifier.PhoneIdentifier +UserIdentifier = _user_identifier.UserIdentifier +UidIdentifier = _user_identifier.UidIdentifier +EmailIdentifier = _user_identifier.EmailIdentifier +PhoneIdentifier = _user_identifier.PhoneIdentifier def _get_auth_service(app): @@ -328,10 +331,6 @@ def get_users(identifiers, app=None): result list is not guaranteed to correspond to the nth entry in the input parameters list. - If a given user doesn't exist, then no entry will be returned for it in the - results, implying that the results list may have fewer entries than the - identifiers list. - Only a maximum of 100 identifiers may be supplied. If more than 100 identifiers are supplied, this method will immediately raise a ValueError. @@ -342,9 +341,8 @@ def get_users(identifiers, app=None): app: An App instance (optional). Returns: - list[UserRecord]: A list of ``UserRecord`` instances corresponding to the - specified identifiers. This could be empty if no users were - successfully looked up. + GetUsersResult: A ``GetUsersResult`` instance corresponding to the + specified identifiers. Raises: ValueError: If any of the identifiers are invalid or if more than 100 @@ -352,7 +350,25 @@ def get_users(identifiers, app=None): """ user_manager = _get_auth_service(app).user_manager response = user_manager.get_users(identifiers=identifiers) - return [UserRecord(data) for data in response] + + def _matches(identifier, user_record): + if isinstance(identifier, UidIdentifier): + return identifier.uid == user_record.uid + if isinstance(identifier, EmailIdentifier): + return identifier.email == user_record.email + if isinstance(identifier, PhoneIdentifier): + return identifier.phone_number == user_record.phone_number + raise TypeError("Unexpected type: {}".format(type(identifier))) + + def _is_user_found(identifier, user_records): + return next( + (True for user_record in user_records if _matches(identifier, user_record)), + False) + + users = [UserRecord(user) for user in response] + not_found = [identifier for identifier in identifiers if not _is_user_found(identifier, users)] + + return GetUsersResult(users=users, not_found=not_found) def list_users(page_token=None, max_results=_user_mgt.MAX_LIST_USERS_RESULTS, app=None): diff --git a/integration/test_auth.py b/integration/test_auth.py index 890b4a987..880f4e818 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -186,6 +186,10 @@ def new_user_list(): auth.create_user(password='password').uid, ] yield users + # TODO(rsgowman): Using auth.delete_users() would make more sense here, but + # that's currently rate limited to 1qps, so using it in this context would + # almost certainly trigger errors. When/if that limit is relaxed, switch to + # batch delete. for uid in users: auth.delete_user(uid) @@ -246,47 +250,51 @@ def _map_user_record_to_uid_email_phones(user_record): 'phone_number': user_record.phone_number } - def test_returns_users_by_various_identifier_types_in_a_single_call(self, new_user_record_list): - users = auth.get_users([ + def test_multiple_uid_types(self, new_user_record_list): + get_users_results = auth.get_users([ auth.UidIdentifier(new_user_record_list[0].uid), auth.EmailIdentifier(new_user_record_list[1].email), auth.PhoneIdentifier(new_user_record_list[2].phone_number)]) actual = sorted( - list(map(self._map_user_record_to_uid_email_phones, users)), + [self._map_user_record_to_uid_email_phones(user) for user in get_users_results.users], key=lambda user: user['uid']) expected = sorted( - list(map(self._map_user_record_to_uid_email_phones, new_user_record_list)), + [self._map_user_record_to_uid_email_phones(user) for user in new_user_record_list], key=lambda user: user['uid']) assert actual == expected - def test_returns_found_users_and_ignores_non_existing_users(self, new_user_record_list): - users = auth.get_users([ + def test_existing_and_non_existing_users(self, new_user_record_list): + get_users_results = auth.get_users([ auth.UidIdentifier(new_user_record_list[0].uid), auth.UidIdentifier('uid_that_doesnt_exist'), auth.UidIdentifier(new_user_record_list[2].uid)]) - actual = sorted( - list(map(self._map_user_record_to_uid_email_phones, users)), - key=lambda user: user['uid']) - expected = sorted( - list(map( - self._map_user_record_to_uid_email_phones, - [new_user_record_list[0], new_user_record_list[2]])), - key=lambda user: user['uid']) + actual = sorted([ + self._map_user_record_to_uid_email_phones(user) + for user in get_users_results.users + ], key=lambda user: user['uid']) + expected = sorted([ + self._map_user_record_to_uid_email_phones(user) + for user in [new_user_record_list[0], new_user_record_list[2]] + ], key=lambda user: user['uid']) assert actual == expected - def test_returns_nothing_when_queried_for_only_non_existing_users(self): - users = auth.get_users([auth.UidIdentifier('non-existing user')]) + def test_non_existing_users(self): + not_found_ids = [auth.UidIdentifier('non-existing user')] + get_users_results = auth.get_users(not_found_ids) - assert users == [] + assert get_users_results.users == [] + assert get_users_results.not_found == not_found_ids def test_de_dups_duplicate_users(self, new_user): - users = auth.get_users([ + get_users_results = auth.get_users([ auth.UidIdentifier(new_user.uid), auth.UidIdentifier(new_user.uid)]) - actual = list(map(self._map_user_record_to_uid_email_phones, users)) - expected = list(map(self._map_user_record_to_uid_email_phones, [new_user])) + actual = [ + self._map_user_record_to_uid_email_phones(user) + for user in get_users_results.users] + expected = [self._map_user_record_to_uid_email_phones(new_user)] assert actual == expected def test_last_refresh_timestamp(new_user_with_params: auth.UserRecord, api_key): @@ -302,8 +310,7 @@ def test_last_refresh_timestamp(new_user_with_params: auth.UserRecord, api_key): # considerations are handled properly, so as long as we're within an hour, # we're in good shape.) millis_per_second = 1000 - seconds_per_minute = 60 - millis_per_minute = millis_per_second * seconds_per_minute + millis_per_minute = millis_per_second * 60 last_refresh_timestamp = new_user_with_params.user_metadata.last_refresh_timestamp assert last_refresh_timestamp == pytest.approx( @@ -439,9 +446,9 @@ def test_delete_multiple_users(self): assert delete_users_result.failure_count == 0 assert len(delete_users_result.errors) == 0 - user_records = auth.get_users( + get_users_results = auth.get_users( [auth.UidIdentifier(uid1), auth.UidIdentifier(uid2), auth.UidIdentifier(uid3)]) - assert len(user_records) == 0 + assert len(get_users_results.users) == 0 def test_is_idempotent(self): uid = auth.create_user().uid diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 5a7d43dfd..b091bbb84 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -303,34 +303,44 @@ def test_get_user_by_phone_http_error(self, user_mgt_app): class TestGetUsers: - def test_should_reject_when_given_more_than_100_identifiers(self, user_mgt_app): + @staticmethod + def _map_user_record_to_uid_email_phones(user_record): + return { + 'uid': user_record.uid, + 'email': user_record.email, + 'phone_number': user_record.phone_number + } + + def test_more_than_100_identifiers(self, user_mgt_app): identifiers = [auth.UidIdentifier('id' + str(i)) for i in range(101)] with pytest.raises(ValueError): auth.get_users(identifiers, app=user_mgt_app) - def test_should_return_no_results_when_given_no_identifiers(self, user_mgt_app): - assert auth.get_users([], app=user_mgt_app) == [] + def test_no_identifiers(self, user_mgt_app): + get_users_results = auth.get_users([], app=user_mgt_app) + assert get_users_results.users == [] + assert get_users_results.not_found == [] - def test_should_return_no_results_when_given_identifiers_that_do_not_exist(self, user_mgt_app): + def test_identifiers_that_do_not_exist(self, user_mgt_app): _instrument_user_manager(user_mgt_app, 200, '{}') - user_records = auth.get_users( - [auth.UidIdentifier('id that doesnt exist')], - app=user_mgt_app) - assert user_records == [] + not_found_ids = [auth.UidIdentifier('id that doesnt exist')] + get_users_results = auth.get_users(not_found_ids, app=user_mgt_app) + assert get_users_results.users == [] + assert get_users_results.not_found == not_found_ids - def test_should_be_rejected_when_given_an_invalid_uid(self, user_mgt_app): + def test_invalid_uid(self, user_mgt_app): with pytest.raises(ValueError): auth.get_users([auth.UidIdentifier('too long ' + '.'*128)], app=user_mgt_app) - def test_should_be_rejected_when_given_an_invalid_email(self, user_mgt_app): + def test_invalid_email(self, user_mgt_app): with pytest.raises(ValueError): auth.get_users([auth.EmailIdentifier('invalid email addr')], app=user_mgt_app) - def test_should_be_rejected_when_given_an_invalid_phone_number(self, user_mgt_app): + def test_invalid_phone_number(self, user_mgt_app): with pytest.raises(ValueError): auth.get_users([auth.PhoneIdentifier('invalid phone number')], app=user_mgt_app) - def test_should_be_rejected_when_given_a_single_bad_identifier(self, user_mgt_app): + def test_single_bad_identifier(self, user_mgt_app): identifiers = [ auth.UidIdentifier('valid_id1'), auth.UidIdentifier('valid_id2'), @@ -341,6 +351,47 @@ def test_should_be_rejected_when_given_a_single_bad_identifier(self, user_mgt_ap with pytest.raises(ValueError): auth.get_users(identifiers, app=user_mgt_app) + def test_success(self, user_mgt_app): + mock_users = [{ + "localId": "uid1", + "email": "user1@example.com", + "phoneNumber": "+15555550001" + }, { + "localId": "uid2", + "email": "user2@example.com", + "phoneNumber": "+15555550002" + }, { + "localId": "uid3", + "email": "user3@example.com", + "phoneNumber": "+15555550003" + }, { + "localId": "uid4", + "email": "user4@example.com", + "phoneNumber": "+15555550004", + "providerUserInfo": [{ + "providerId": "google.com", + "rawId": "google_uid4" + }] + }] + _instrument_user_manager(user_mgt_app, 200, '{ "users": ' + json.dumps(mock_users) + '}') + + get_users_results = auth.get_users([ + auth.UidIdentifier('uid1'), + auth.EmailIdentifier('user2@example.com'), + auth.PhoneIdentifier('+15555550003'), + auth.UidIdentifier('this-user-doesnt-exist'), + ], app=user_mgt_app) + + actual = sorted( + [self._map_user_record_to_uid_email_phones(user) for user in get_users_results.users], + key=lambda user: user['uid']) + expected = sorted([ + self._map_user_record_to_uid_email_phones(auth.UserRecord(user)) + for user in mock_users + ], key=lambda user: user['uid']) + assert actual == expected + assert [u.uid for u in get_users_results.not_found] == ['this-user-doesnt-exist'] + class TestCreateUser: @@ -662,7 +713,7 @@ def test_empty_list(self, user_mgt_app): assert len(delete_users_result.errors) == 0 def test_too_many_identifiers_should_fail(self, user_mgt_app): - ids = ['id' + str(i) for i in range(101)] + ids = ['id' + str(i) for i in range(1001)] with pytest.raises(ValueError): auth.delete_users(ids, app=user_mgt_app) @@ -689,9 +740,16 @@ def test_should_index_errors_correctly_in_results(self, user_mgt_app): assert delete_users_result.failure_count == 2 assert len(delete_users_result.errors) == 2 assert delete_users_result.errors[0].index == 0 - assert delete_users_result.errors[0].message.startswith('NOT_DISABLED') + assert delete_users_result.errors[0].reason.startswith('NOT_DISABLED') assert delete_users_result.errors[1].index == 2 - assert delete_users_result.errors[1].message == 'something awful' + assert delete_users_result.errors[1].reason == 'something awful' + + def test_success(self, user_mgt_app): + _instrument_user_manager(user_mgt_app, 200, '{}') + delete_users_result = auth.delete_users(['uid1', 'uid2', 'uid3'], app=user_mgt_app) + assert delete_users_result.success_count == 3 + assert delete_users_result.failure_count == 0 + assert len(delete_users_result.errors) == 0 class TestListUsers: From 19930077b1bc2f0a3f8007b8f65b42f8110618a7 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 11:23:27 -0500 Subject: [PATCH 08/17] Add (missing) bulk lookup by provider --- firebase_admin/_auth_utils.py | 9 ++++++ firebase_admin/_user_identifier.py | 17 ++++++++++++ firebase_admin/_user_mgt.py | 7 +++++ firebase_admin/auth.py | 9 ++++++ integration/test_auth.py | 44 ++++++++++++++++++++++++------ tests/test_user_mgt.py | 7 +++++ 6 files changed, 85 insertions(+), 8 deletions(-) diff --git a/firebase_admin/_auth_utils.py b/firebase_admin/_auth_utils.py index 2f7383c0b..fabad74dd 100644 --- a/firebase_admin/_auth_utils.py +++ b/firebase_admin/_auth_utils.py @@ -100,6 +100,15 @@ def validate_provider_id(provider_id, required=True): 'string.'.format(provider_id)) return provider_id +def validate_provider_uid(provider_uid, required=True): + if provider_uid is None and not required: + return None + if not isinstance(provider_uid, str) or not provider_uid: + raise ValueError( + 'Invalid provider UID: "{0}". Provider UID must be a non-empty ' + 'string.'.format(provider_uid)) + return provider_uid + def validate_photo_url(photo_url, required=False): """Parses and validates the given URL string.""" if photo_url is None and not required: diff --git a/firebase_admin/_user_identifier.py b/firebase_admin/_user_identifier.py index cc76c0869..2fb0c65d3 100644 --- a/firebase_admin/_user_identifier.py +++ b/firebase_admin/_user_identifier.py @@ -61,3 +61,20 @@ def __init__(self, phone_number): phone_number: A phone number string. """ self.phone_number = phone_number + + +class ProviderIdentifier(UserIdentifier): + """Used for looking up an account by provider. + + See ``auth.get_user()``. + """ + + def __init__(self, provider_id, provider_uid): + """Constructs a new ProviderIdentifier. + +   Args: +     provider_id: A provider ID string. +     provider_uid: A provider UID string. + """ + self.provider_id = provider_id + self.provider_uid = provider_uid diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 93d81ced7..24fdbdd85 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -654,6 +654,13 @@ def get_users(self, identifiers): elif isinstance(identifier, _user_identifier.PhoneIdentifier): _auth_utils.validate_phone(identifier.phone_number, required=True) payload['phoneNumber'] = payload.get('phoneNumber', []) + [identifier.phone_number] + elif isinstance(identifier, _user_identifier.ProviderIdentifier): + _auth_utils.validate_provider_id(identifier.provider_id, required=True) + _auth_utils.validate_provider_uid(identifier.provider_uid, required=True) + payload['federatedUserId'] = payload.get('federatedUserId', []) + [{ + 'providerId': identifier.provider_id, + 'rawId': identifier.provider_uid + }] else: raise ValueError( 'Invalid entry in "identifiers" list. Unsupported type: {}' diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 1f77e9aa4..e45762630 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -67,6 +67,7 @@ 'UidIdentifier', 'EmailIdentifier', 'PhoneIdentifier', + 'ProviderIdentifier', 'create_custom_token', 'create_session_cookie', @@ -123,6 +124,7 @@ UidIdentifier = _user_identifier.UidIdentifier EmailIdentifier = _user_identifier.EmailIdentifier PhoneIdentifier = _user_identifier.PhoneIdentifier +ProviderIdentifier = _user_identifier.ProviderIdentifier def _get_auth_service(app): @@ -358,6 +360,13 @@ def _matches(identifier, user_record): return identifier.email == user_record.email if isinstance(identifier, PhoneIdentifier): return identifier.phone_number == user_record.phone_number + if isinstance(identifier, ProviderIdentifier): + return next(( + True + for user_info in user_record.provider_data + if identifier.provider_id == user_info.provider_id + and identifier.provider_uid == user_info.uid + ), False) raise TypeError("Unexpected type: {}".format(type(identifier))) def _is_user_found(identifier, user_records): diff --git a/integration/test_auth.py b/integration/test_auth.py index 880f4e818..6d45abf39 100644 --- a/integration/test_auth.py +++ b/integration/test_auth.py @@ -210,6 +210,28 @@ def new_user_record_list() -> List[auth.UserRecord]: for user in users: auth.delete_user(user.uid) +@pytest.fixture +def new_user_with_provider() -> auth.UserRecord: + uid4, email4 = _random_id() + google_uid, google_email = _random_id() + import_user1 = auth.ImportUserRecord( + uid=uid4, + email=email4, + provider_data=[ + auth.UserProvider( + uid=google_uid, + provider_id='google.com', + email=google_email, + ) + ]) + user_import_result = auth.import_users([import_user1]) + assert user_import_result.success_count == 1 + assert user_import_result.failure_count == 0 + + user = auth.get_user(uid4) + yield user + auth.delete_user(user.uid) + @pytest.fixture def new_user_email_unverified(): random_id, email = _random_id() @@ -250,17 +272,23 @@ def _map_user_record_to_uid_email_phones(user_record): 'phone_number': user_record.phone_number } - def test_multiple_uid_types(self, new_user_record_list): + def test_multiple_uid_types(self, new_user_record_list, new_user_with_provider): get_users_results = auth.get_users([ auth.UidIdentifier(new_user_record_list[0].uid), auth.EmailIdentifier(new_user_record_list[1].email), - auth.PhoneIdentifier(new_user_record_list[2].phone_number)]) - actual = sorted( - [self._map_user_record_to_uid_email_phones(user) for user in get_users_results.users], - key=lambda user: user['uid']) - expected = sorted( - [self._map_user_record_to_uid_email_phones(user) for user in new_user_record_list], - key=lambda user: user['uid']) + auth.PhoneIdentifier(new_user_record_list[2].phone_number), + auth.ProviderIdentifier( + new_user_with_provider.provider_data[0].provider_id, + new_user_with_provider.provider_data[0].uid, + )]) + actual = sorted([ + self._map_user_record_to_uid_email_phones(user) + for user in get_users_results.users + ], key=lambda user: user['uid']) + expected = sorted([ + self._map_user_record_to_uid_email_phones(user) + for user in new_user_record_list + [new_user_with_provider] + ], key=lambda user: user['uid']) assert actual == expected diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index b091bbb84..a8b48b9b1 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -340,6 +340,12 @@ def test_invalid_phone_number(self, user_mgt_app): with pytest.raises(ValueError): auth.get_users([auth.PhoneIdentifier('invalid phone number')], app=user_mgt_app) + def test_invalid_provider(self, user_mgt_app): + with pytest.raises(ValueError): + auth.get_users( + [auth.ProviderIdentifier(provider_id='', provider_uid='')], + app=user_mgt_app) + def test_single_bad_identifier(self, user_mgt_app): identifiers = [ auth.UidIdentifier('valid_id1'), @@ -379,6 +385,7 @@ def test_success(self, user_mgt_app): auth.UidIdentifier('uid1'), auth.EmailIdentifier('user2@example.com'), auth.PhoneIdentifier('+15555550003'), + auth.ProviderIdentifier(provider_id='google.com', provider_uid='google_uid4'), auth.UidIdentifier('this-user-doesnt-exist'), ], app=user_mgt_app) From 0b182034c1bcb5587a959824148c9f5bb47e26d2 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 14:19:36 -0500 Subject: [PATCH 09/17] Remove iso8601 dependency. Replace with a custom (ugh) rfc3339 module. --- firebase_admin/_rfc3339.py | 89 +++++++++++++++++++++++++++++++++++++ firebase_admin/_user_mgt.py | 8 ++-- requirements.txt | 1 - tests/test_rfc3339.py | 67 ++++++++++++++++++++++++++++ 4 files changed, 160 insertions(+), 5 deletions(-) create mode 100644 firebase_admin/_rfc3339.py create mode 100644 tests/test_rfc3339.py diff --git a/firebase_admin/_rfc3339.py b/firebase_admin/_rfc3339.py new file mode 100644 index 000000000..dc893b556 --- /dev/null +++ b/firebase_admin/_rfc3339.py @@ -0,0 +1,89 @@ +# Copyright 2020 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Parse RFC3339 date strings""" + +from datetime import datetime, timezone +import re + +def parse_to_epoch(datestr): + """Parses an RFC3339 date string and return the number of seconds since the + epoch (as a float). + + In particular, this method is meant to parse the strings returned by the + JSON mapping of protobuf google.protobuf.timestamp.Timestamp instances: + https://github.com/protocolbuffers/protobuf/blob/4cf5bfee9546101d98754d23ff378ff718ba8438/src/google/protobuf/timestamp.proto#L99 + + This method has microsecond precision; i.e. nanoseconds will be truncated. + + Args: + datestr: A string in RFC3339 format. + Returns: + Float: The number of seconds since the unix epoch. + Raises: + ValueError: Raised if the datestr is not a valid RFC3339 date string. + """ + return _parse_to_datetime(datestr).timestamp() + + +def _parse_to_datetime(datestr): + """Parse an RFC3339 date string and return a python datetime instance. + + Args: + datestr: A string in RFC3339 format. + Returns: + datetime: The corresponding datetime (with timezone information). + Raises: + ValueError: Raised if the datestr is not a valid RFC3339 date string. + """ + # If more than 6 digits appear in the fractional seconds position, truncate + # to just the most significant 6. (i.e. we only have microsecond precision; + # nanos are truncated.) + datestr_modified = re.sub(r'(\.\d{6})\d*', r'\1', datestr) + + # This format is the one we actually expect to occur from our backend. The + # others are only present because the spec says we *should* accept them. + try: + return datetime.strptime( + datestr_modified, '%Y-%m-%dT%H:%M:%S.%fZ' + ).replace(tzinfo=timezone.utc) + except ValueError: + pass + + try: + return datetime.strptime( + datestr_modified, '%Y-%m-%dT%H:%M:%SZ' + ).replace(tzinfo=timezone.utc) + except ValueError: + pass + + # Note: %z parses timezone offsets, but requires the timezone offset *not* + # include a separating ':'. As of python 3.7, this was relaxed. + # TODO(rsgowman): Once python3.7 becomes our floor, we can drop the regex + # replacement. + datestr_modified = re.sub(r'(\d\d):(\d\d)$', r'\1\2', datestr_modified) + + try: + print("trying with micros") + return datetime.strptime(datestr_modified, '%Y-%m-%dT%H:%M:%S.%f%z') + except ValueError: + pass + + try: + print("trying without micros") + return datetime.strptime(datestr_modified, '%Y-%m-%dT%H:%M:%S%z') + except ValueError: + pass + + raise ValueError('time data {0} does not match RFC3339 format'.format(datestr)) diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 24fdbdd85..34d2e8a48 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -19,9 +19,9 @@ from urllib import parse import requests -import iso8601 from firebase_admin import _auth_utils +from firebase_admin import _rfc3339 from firebase_admin import _user_identifier from firebase_admin import _user_import @@ -232,9 +232,9 @@ def _int_or_none(key): return int(self._data[key]) return None last_refresh_at_millis = None - last_refresh_at_iso8601 = self._data.get('lastRefreshAt', None) - if last_refresh_at_iso8601: - last_refresh_at_millis = iso8601.parse_date(last_refresh_at_iso8601).timestamp() * 1000 + last_refresh_at_rfc3339 = self._data.get('lastRefreshAt', None) + if last_refresh_at_rfc3339: + last_refresh_at_millis = int(_rfc3339.parse_to_epoch(last_refresh_at_rfc3339) * 1000) return UserMetadata( _int_or_none('createdAt'), _int_or_none('lastLoginAt'), last_refresh_at_millis) diff --git a/requirements.txt b/requirements.txt index a6bdb5c42..d7fb6d736 100644 --- a/requirements.txt +++ b/requirements.txt @@ -8,4 +8,3 @@ google-api-core[grpc] >= 1.14.0, < 2.0.0dev; platform.python_implementation != ' google-api-python-client >= 1.7.8 google-cloud-firestore >= 1.4.0; platform.python_implementation != 'PyPy' google-cloud-storage >= 1.18.0 -iso8601 >= 0.1.12 diff --git a/tests/test_rfc3339.py b/tests/test_rfc3339.py new file mode 100644 index 000000000..5a844b07e --- /dev/null +++ b/tests/test_rfc3339.py @@ -0,0 +1,67 @@ +# Copyright 2020 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Test cases for the firebase_admin._rfc3339 module.""" + +import pytest + +from firebase_admin import _rfc3339 + +def test_epoch(): + expected = pytest.approx(0) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00Z") == expected + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00z") == expected + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00+00:00") == expected + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00-00:00") == expected + assert _rfc3339.parse_to_epoch("1970-01-01T01:00:00+01:00") == expected + assert _rfc3339.parse_to_epoch("1969-12-31T23:00:00-01:00") == expected + +def test_pre_epoch(): + expected = -5617641600 + assert _rfc3339.parse_to_epoch("1791-12-26T00:00:00Z") == expected + assert _rfc3339.parse_to_epoch("1791-12-26T00:00:00+00:00") == expected + assert _rfc3339.parse_to_epoch("1791-12-26T00:00:00-00:00") == expected + assert _rfc3339.parse_to_epoch("1791-12-26T01:00:00+01:00") == expected + assert _rfc3339.parse_to_epoch("1791-12-25T23:00:00-01:00") == expected + +def test_post_epoch(): + expected = 904892400 + assert _rfc3339.parse_to_epoch("1998-09-04T07:00:00Z") == expected + assert _rfc3339.parse_to_epoch("1998-09-04T07:00:00+00:00") == expected + assert _rfc3339.parse_to_epoch("1998-09-04T08:00:00+01:00") == expected + assert _rfc3339.parse_to_epoch("1998-09-04T06:00:00-01:00") == expected + +def test_micros_millis(): + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00Z") == pytest.approx(0) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.1Z") == pytest.approx(0.1) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.001Z") == pytest.approx(0.001) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.000001Z") == pytest.approx(0.000001) + + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00+00:00") == pytest.approx(0) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.1+00:00") == pytest.approx(0.1) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.001+00:00") == pytest.approx(0.001) + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.000001+00:00") == pytest.approx(0.000001) + +def test_nanos(): + assert _rfc3339.parse_to_epoch("1970-01-01T00:00:00.0000001Z") == pytest.approx(0) + +@pytest.mark.parametrize('datestr', [ + 'not a date string', + '1970-01-01 00:00:00Z', + '1970-01-01 00:00:00+00:00', + '1970-01-01T00:00:00', + ]) +def test_bad_datestrs(datestr): + with pytest.raises(ValueError): + _rfc3339.parse_to_epoch(datestr) From 65d7d635c77beb6e1cce3113ec351a17e09447c2 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 17:28:39 -0500 Subject: [PATCH 10/17] review feedback 2 --- firebase_admin/_rfc3339.py | 2 -- firebase_admin/_user_mgt.py | 11 ++++------- firebase_admin/auth.py | 2 ++ 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/firebase_admin/_rfc3339.py b/firebase_admin/_rfc3339.py index dc893b556..d0efc38f8 100644 --- a/firebase_admin/_rfc3339.py +++ b/firebase_admin/_rfc3339.py @@ -75,13 +75,11 @@ def _parse_to_datetime(datestr): datestr_modified = re.sub(r'(\d\d):(\d\d)$', r'\1\2', datestr_modified) try: - print("trying with micros") return datetime.strptime(datestr_modified, '%Y-%m-%dT%H:%M:%S.%f%z') except ValueError: pass try: - print("trying without micros") return datetime.strptime(datestr_modified, '%Y-%m-%dT%H:%M:%S%z') except ValueError: pass diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 34d2e8a48..80071e186 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -435,7 +435,7 @@ def __init__(self, err): returned by the server. """ self._index = err.get('index', 0) - self._local_id = err.get('local_id', "") + self._uid = err.get('local_id', "") self._reason = err.get('message', '') @property @@ -447,8 +447,8 @@ def reason(self): return self._reason @property - def local_id(self): - return self._local_id + def uid(self): + return self._uid class BatchDeleteAccountsResponse: @@ -463,10 +463,7 @@ def __init__(self, errors=None): BatchDeleteErrorInfo instance as returned by the server. None implies an empty list. """ - if errors: - self.errors = [BatchDeleteErrorInfo(err) for err in errors] - else: - self.errors = [] + self.errors = [BatchDeleteErrorInfo(err) for err in errors] if errors else [] class ProviderUserInfo(UserInfo): diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index e45762630..38b9479d4 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -91,6 +91,8 @@ ] ActionCodeSettings = _user_mgt.ActionCodeSettings +BatchDeleteAccountsResponse = _user_mgt.BatchDeleteAccountsResponse +BatchDeleteErrorInfo = _user_mgt.BatchDeleteErrorInfo CertificateFetchError = _token_gen.CertificateFetchError DELETE_ATTRIBUTE = _user_mgt.DELETE_ATTRIBUTE DeleteUsersResult = _user_mgt.DeleteUsersResult From b21f23ef46d0b754fb23ecb5651537b2d6e3b361 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 6 Feb 2020 12:09:33 -0500 Subject: [PATCH 11/17] review feedback 3 --- firebase_admin/_user_import.py | 11 ++++++--- firebase_admin/_user_mgt.py | 44 ++++++---------------------------- firebase_admin/auth.py | 2 -- 3 files changed, 15 insertions(+), 42 deletions(-) diff --git a/firebase_admin/_user_import.py b/firebase_admin/_user_import.py index 21cc8082d..aca33f6f7 100644 --- a/firebase_admin/_user_import.py +++ b/firebase_admin/_user_import.py @@ -472,11 +472,16 @@ def standard_scrypt(cls, memory_cost, parallelization, block_size, derived_key_l class ErrorInfo: - """Represents an error encountered while importing an ``ImportUserRecord``.""" + """Represents an error encountered while performing a batch operation such + as importing users or deleting multiple user accounts. + """ + # TODO(rsgowman): This class used to be specific to importing users (hence + # it's home in _user_import.py). It's now also used by bulk deletion of + # users. Move this to a more common location. def __init__(self, error): - self._index = error['index'] - self._reason = error['message'] + self._index = error.get('index', 0) + self._reason = error.get('message', '') @property def index(self): diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 80071e186..a325fe24b 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -24,6 +24,7 @@ from firebase_admin import _rfc3339 from firebase_admin import _user_identifier from firebase_admin import _user_import +from firebase_admin._user_import import ErrorInfo MAX_LIST_USERS_RESULTS = 1000 @@ -413,44 +414,13 @@ def failure_count(self): @property def errors(self): - """A list of ``auth.BatchDeleteErrorInfo`` instances describing the - errors that were encountered during the deletion. Length of this list - is equal to `failure_count`. + """A list of ``auth.ErrorInfo`` instances describing the errors that + were encountered during the deletion. Length of this list is equal to + `failure_count`. """ return self._errors -class BatchDeleteErrorInfo: - """Represents an error that occurred while attempting to delete a batch of - users. - """ - - def __init__(self, err): - """Constructs a BatchDeleteErrorInfo instance, corresponding to the - json representing the BatchDeleteErrorInfo proto. - - Args: - err: A dictionary with 'index', 'local_id' and 'message' fields, - representing the BatchDeleteErrorInfo dictionary that's - returned by the server. - """ - self._index = err.get('index', 0) - self._uid = err.get('local_id', "") - self._reason = err.get('message', '') - - @property - def index(self): - return self._index - - @property - def reason(self): - return self._reason - - @property - def uid(self): - return self._uid - - class BatchDeleteAccountsResponse: """Represents the results of a delete_users() call.""" @@ -460,10 +430,10 @@ def __init__(self, errors=None): Args: errors: List of dictionaries, with each dictionary representing a - BatchDeleteErrorInfo instance as returned by the server. None - implies an empty list. + ErrorInfo instance as returned by the server. None implies an + empty list. """ - self.errors = [BatchDeleteErrorInfo(err) for err in errors] if errors else [] + self.errors = [ErrorInfo(err) for err in errors] if errors else [] class ProviderUserInfo(UserInfo): diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index 38b9479d4..e45762630 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -91,8 +91,6 @@ ] ActionCodeSettings = _user_mgt.ActionCodeSettings -BatchDeleteAccountsResponse = _user_mgt.BatchDeleteAccountsResponse -BatchDeleteErrorInfo = _user_mgt.BatchDeleteErrorInfo CertificateFetchError = _token_gen.CertificateFetchError DELETE_ATTRIBUTE = _user_mgt.DELETE_ATTRIBUTE DeleteUsersResult = _user_mgt.DeleteUsersResult From d4a8a3ee23622e4fdab5d5b145a5cb2043241fb6 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Thu, 20 Feb 2020 12:29:57 -0500 Subject: [PATCH 12/17] (mostly) doc review feedback --- firebase_admin/_rfc3339.py | 12 ++++----- firebase_admin/_user_identifier.py | 8 +++--- firebase_admin/_user_import.py | 4 +-- firebase_admin/_user_mgt.py | 40 ++++++++++++++++-------------- firebase_admin/auth.py | 12 ++++----- 5 files changed, 39 insertions(+), 37 deletions(-) diff --git a/firebase_admin/_rfc3339.py b/firebase_admin/_rfc3339.py index d0efc38f8..2c720bdd1 100644 --- a/firebase_admin/_rfc3339.py +++ b/firebase_admin/_rfc3339.py @@ -18,21 +18,21 @@ import re def parse_to_epoch(datestr): - """Parses an RFC3339 date string and return the number of seconds since the + """Parse an RFC3339 date string and return the number of seconds since the epoch (as a float). In particular, this method is meant to parse the strings returned by the JSON mapping of protobuf google.protobuf.timestamp.Timestamp instances: https://github.com/protocolbuffers/protobuf/blob/4cf5bfee9546101d98754d23ff378ff718ba8438/src/google/protobuf/timestamp.proto#L99 - This method has microsecond precision; i.e. nanoseconds will be truncated. + This method has microsecond precision; nanoseconds will be truncated. Args: datestr: A string in RFC3339 format. Returns: - Float: The number of seconds since the unix epoch. + Float: The number of seconds since the Unix epoch. Raises: - ValueError: Raised if the datestr is not a valid RFC3339 date string. + ValueError: Raised if the `datestr` is not a valid RFC3339 date string. """ return _parse_to_datetime(datestr).timestamp() @@ -43,9 +43,9 @@ def _parse_to_datetime(datestr): Args: datestr: A string in RFC3339 format. Returns: - datetime: The corresponding datetime (with timezone information). + datetime: The corresponding `datetime` (with timezone information). Raises: - ValueError: Raised if the datestr is not a valid RFC3339 date string. + ValueError: Raised if the `datestr` is not a valid RFC3339 date string. """ # If more than 6 digits appear in the fractional seconds position, truncate # to just the most significant 6. (i.e. we only have microsecond precision; diff --git a/firebase_admin/_user_identifier.py b/firebase_admin/_user_identifier.py index 2fb0c65d3..d67d2ac56 100644 --- a/firebase_admin/_user_identifier.py +++ b/firebase_admin/_user_identifier.py @@ -25,7 +25,7 @@ class UidIdentifier(UserIdentifier): """ def __init__(self, uid): - """Constructs a new UidIdentifier. + """Constructs a new `UidIdentifier` object. Args: uid: A user ID string. @@ -40,7 +40,7 @@ class EmailIdentifier(UserIdentifier): """ def __init__(self, email): - """Constructs a new EmailIdentifier. + """Constructs a new `EmailIdentifier` object. Args: email: A user email address string. @@ -55,7 +55,7 @@ class PhoneIdentifier(UserIdentifier): """ def __init__(self, phone_number): - """Constructs a new PhoneIdentifier. + """Constructs a new `PhoneIdentifier` object. Args: phone_number: A phone number string. @@ -70,7 +70,7 @@ class ProviderIdentifier(UserIdentifier): """ def __init__(self, provider_id, provider_uid): - """Constructs a new ProviderIdentifier. + """Constructs a new `ProviderIdentifier` object.   Args:     provider_id: A provider ID string. diff --git a/firebase_admin/_user_import.py b/firebase_admin/_user_import.py index aca33f6f7..7834b232a 100644 --- a/firebase_admin/_user_import.py +++ b/firebase_admin/_user_import.py @@ -480,8 +480,8 @@ class ErrorInfo: # users. Move this to a more common location. def __init__(self, error): - self._index = error.get('index', 0) - self._reason = error.get('message', '') + self._index = error['index'] + self._reason = error['message'] @property def index(self): diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index a325fe24b..4c62d6ba0 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -76,7 +76,7 @@ def last_refresh_timestamp(self): """The time at which the user was last active (ID token refreshed). Returns: - integer: Milliseconds since epoch timestamp, or None if the user was + integer: Milliseconds since epoch timestamp, or `None` if the user was never active. """ return self._last_refresh_timestamp @@ -305,7 +305,7 @@ class GetUsersResult: """Represents the result of the ``auth.get_users()`` API.""" def __init__(self, users, not_found): - """Constructs a GetUsersResult. + """Constructs a `GetUsersResult` object. Args: users: List of `UserRecord` instances. @@ -316,15 +316,17 @@ def __init__(self, users, not_found): @property def users(self): - """Set of UserRecords, corresponding to the set of users that were - requested. Only users that were found are listed here. The result set - is unordered. + """Set of `UserRecord` instances, corresponding to the set of users + that were requested. Only users that were found are listed here. The + result set is unordered. """ return self._users @property def not_found(self): - """Set of UserIdentifiers that were requested, but not found.""" + """Set of `UserIdentifier` instances that were requested, but not + found. + """ return self._not_found @@ -383,12 +385,12 @@ class DeleteUsersResult: """Represents the result of the ``auth.delete_users()`` API.""" def __init__(self, result, total): - """Constructs a DeleteUsersResult. + """Constructs a `DeleteUsersResult` object. Args: - result: BatchDeleteAccountsResponse: The proto response, wrapped in a - BatchDeleteAccountsResponse instance. - total: integer: Total number of deletion attempts. + result: The proto response, wrapped in a + `BatchDeleteAccountsResponse` instance. + total: Total integer number of deletion attempts. """ errors = result.errors self._success_count = total - len(errors) @@ -400,7 +402,7 @@ def success_count(self): """Returns the number of users that were deleted successfully (possibly zero). - Users that did not exist prior to calling delete_users() will be + Users that did not exist prior to calling `delete_users()` are considered to be successfully deleted. """ return self._success_count @@ -422,16 +424,16 @@ def errors(self): class BatchDeleteAccountsResponse: - """Represents the results of a delete_users() call.""" + """Represents the results of a `delete_users()` call.""" def __init__(self, errors=None): - """Constructs a BatchDeleteAccountsResponse instance, corresponseing to - the json representing the BatchDeleteAccountsResponse proto. + """Constructs a `BatchDeleteAccountsResponse` instance, corresponding to + the JSON representing the `BatchDeleteAccountsResponse` proto. Args: - errors: List of dictionaries, with each dictionary representing a - ErrorInfo instance as returned by the server. None implies an - empty list. + errors: List of dictionaries, with each dictionary representing an + `ErrorInfo` instance as returned by the server. `None` implies + an empty list. """ self.errors = [ErrorInfo(err) for err in errors] if errors else [] @@ -596,8 +598,8 @@ def get_users(self, identifiers): to be looked up. Must have <= 100 entries. Returns: - list[dict[string, string]]: List of dicts representing the json - UserInfo responses from the server. + list[dict[string, string]]: List of dicts representing the JSON + `UserInfo` responses from the server. Raises: ValueError: If any of the identifiers are invalid or if more than diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index e45762630..d308b3cba 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -333,8 +333,8 @@ def get_users(identifiers, app=None): result list is not guaranteed to correspond to the nth entry in the input parameters list. - Only a maximum of 100 identifiers may be supplied. If more than 100 - identifiers are supplied, this method will immediately raise a ValueError. + A maximum of 100 identifiers may be supplied. If more than 100 + identifiers are supplied, this method raises a `ValueError`. Args: identifiers (list[Identifier]): A list of ``Identifier`` instances used @@ -521,12 +521,12 @@ def delete_users(uids, app=None): """Deletes the users specified by the given identifiers. Deleting a non-existing user won't generate an error. (i.e. this method is - idempotent.) Non-existing users will be considered to be successfully + idempotent.) Non-existing users are considered to be successfully deleted, and will therefore be counted in the - DeleteUserResult.success_count value. + `DeleteUserResult.success_count` value. - Only a maximum of 1000 identifiers may be supplied. If more than 1000 - identifiers are supplied, this method will immediately raise a ValueError. + A maximum of 1000 identifiers may be supplied. If more than 1000 + identifiers are supplied, this method raises a `ValueError`. Args: uids: A list of strings indicating the uids of the users to be deleted. From 9ade7a862f1c3a143244d73aa1d629eee6b96dd5 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Fri, 8 May 2020 17:11:33 -0400 Subject: [PATCH 13/17] Fixes to address changes since last merge master --- firebase_admin/_auth_client.py | 79 ++++++++++++++++++++++++++++++++++ firebase_admin/_user_mgt.py | 31 +++++-------- firebase_admin/auth.py | 35 ++------------- 3 files changed, 94 insertions(+), 51 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index b7af6ddb6..cfc59da5c 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -21,6 +21,7 @@ from firebase_admin import _auth_utils from firebase_admin import _http_client from firebase_admin import _token_gen +from firebase_admin import _user_identifier from firebase_admin import _user_import from firebase_admin import _user_mgt @@ -182,6 +183,57 @@ def get_user_by_phone_number(self, phone_number): response = self._user_manager.get_user(phone_number=phone_number) return _user_mgt.UserRecord(response) + def get_users(self, identifiers): + """Gets the user data corresponding to the specified identifiers. + + There are no ordering guarantees; in particular, the nth entry in the + result list is not guaranteed to correspond to the nth entry in the input + parameters list. + + A maximum of 100 identifiers may be supplied. If more than 100 + identifiers are supplied, this method raises a `ValueError`. + + Args: + identifiers (list[Identifier]): A list of ``Identifier`` instances used + to indicate which user records should be returned. Must have <= 100 + entries. + + Returns: + GetUsersResult: A ``GetUsersResult`` instance corresponding to the + specified identifiers. + + Raises: + ValueError: If any of the identifiers are invalid or if more than 100 + identifiers are specified. + """ + response = self._user_manager.get_users(identifiers=identifiers) + + def _matches(identifier, user_record): + if isinstance(identifier, _user_identifier.UidIdentifier): + return identifier.uid == user_record.uid + if isinstance(identifier, _user_identifier.EmailIdentifier): + return identifier.email == user_record.email + if isinstance(identifier, _user_identifier.PhoneIdentifier): + return identifier.phone_number == user_record.phone_number + if isinstance(identifier, _user_identifier.ProviderIdentifier): + return next(( + True + for user_info in user_record.provider_data + if identifier.provider_id == user_info.provider_id + and identifier.provider_uid == user_info.uid + ), False) + raise TypeError("Unexpected type: {}".format(type(identifier))) + + def _is_user_found(identifier, user_records): + return next( + (True for user_record in user_records if _matches(identifier, user_record)), + False) + + users = [_user_mgt.UserRecord(user) for user in response] + not_found = [identifier for identifier in identifiers if not _is_user_found(identifier, users)] + + return _user_mgt.GetUsersResult(users=users, not_found=not_found) + def list_users(self, page_token=None, max_results=_user_mgt.MAX_LIST_USERS_RESULTS): """Retrieves a page of user accounts from a Firebase project. @@ -306,6 +358,33 @@ def delete_user(self, uid): """ self._user_manager.delete_user(uid) + def delete_users(self, uids): + """Deletes the users specified by the given identifiers. + + Deleting a non-existing user won't generate an error. (i.e. this method is + idempotent.) Non-existing users are considered to be successfully + deleted, and will therefore be counted in the + `DeleteUserResult.success_count` value. + + A maximum of 1000 identifiers may be supplied. If more than 1000 + identifiers are supplied, this method raises a `ValueError`. + + Args: + uids: A list of strings indicating the uids of the users to be deleted. + Must have <= 1000 entries. + + Returns: + DeleteUsersResult: The total number of successful/failed deletions, as + well as the array of errors that correspond to the failed + deletions. + + Raises: + ValueError: If any of the identifiers are invalid or if more than 1000 + identifiers are specified. + """ + result = self._user_manager.delete_users(uids, force_delete=True) + return _user_mgt.DeleteUsersResult(result, len(uids)) + def import_users(self, users, hash_alg=None): """Imports the specified list of users into Firebase Auth. diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 83ac53d66..ba646eed5 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -644,16 +644,12 @@ def get_users(self, identifiers): 'Invalid entry in "identifiers" list. Unsupported type: {}' .format(type(identifier))) - try: - body, http_resp = self._client.body_and_response( + body, http_resp = self._make_request( 'post', '/accounts:lookup', json=payload) - except requests.exceptions.RequestException as error: - raise _auth_utils.handle_auth_backend_error(error) - else: - if not http_resp.ok: - raise _auth_utils.UnexpectedResponseError( - 'Failed to get users.', http_response=http_resp) - return body.get('users', []) + if not http_resp.ok: + raise _auth_utils.UnexpectedResponseError( + 'Failed to get users.', http_response=http_resp) + return body.get('users', []) def list_users(self, page_token=None, max_results=MAX_LIST_USERS_RESULTS): """Retrieves a batch of users.""" @@ -776,18 +772,13 @@ def delete_users(self, uids, force_delete=False): for uid in uids: _auth_utils.validate_uid(uid, required=True) - try: - body, http_resp = self._client.body_and_response( - 'post', '/accounts:batchDelete', + body, http_resp = self._make_request('post', '/accounts:batchDelete', json={'localIds': uids, 'force': force_delete}) - except requests.exceptions.RequestException as error: - raise _auth_utils.handle_auth_backend_error(error) - else: - if not isinstance(body, dict): - raise _auth_utils.UnexpectedResponseError( - 'Unexpected response from server while attempting to delete users.', - http_response=http_resp) - return BatchDeleteAccountsResponse(body.get('errors', [])) + if not isinstance(body, dict): + raise _auth_utils.UnexpectedResponseError( + 'Unexpected response from server while attempting to delete users.', + http_response=http_resp) + return BatchDeleteAccountsResponse(body.get('errors', [])) def import_users(self, users, hash_alg=None): """Imports the given list of users to Firebase Auth.""" diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index aad441484..cd52c2534 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -369,34 +369,8 @@ def get_users(identifiers, app=None): ValueError: If any of the identifiers are invalid or if more than 100 identifiers are specified. """ - user_manager = _get_auth_service(app).user_manager - response = user_manager.get_users(identifiers=identifiers) - - def _matches(identifier, user_record): - if isinstance(identifier, UidIdentifier): - return identifier.uid == user_record.uid - if isinstance(identifier, EmailIdentifier): - return identifier.email == user_record.email - if isinstance(identifier, PhoneIdentifier): - return identifier.phone_number == user_record.phone_number - if isinstance(identifier, ProviderIdentifier): - return next(( - True - for user_info in user_record.provider_data - if identifier.provider_id == user_info.provider_id - and identifier.provider_uid == user_info.uid - ), False) - raise TypeError("Unexpected type: {}".format(type(identifier))) - - def _is_user_found(identifier, user_records): - return next( - (True for user_record in user_records if _matches(identifier, user_record)), - False) - - users = [UserRecord(user) for user in response] - not_found = [identifier for identifier in identifiers if not _is_user_found(identifier, users)] - - return GetUsersResult(users=users, not_found=not_found) + client = _get_client(app) + return client.get_users(identifiers) def list_users(page_token=None, max_results=_user_mgt.MAX_LIST_USERS_RESULTS, app=None): @@ -556,9 +530,8 @@ def delete_users(uids, app=None): ValueError: If any of the identifiers are invalid or if more than 1000 identifiers are specified. """ - user_manager = _get_auth_service(app).user_manager - result = user_manager.delete_users(uids, force_delete=True) - return DeleteUsersResult(result, len(uids)) + client = _get_client(app) + return client.delete_users(uids) def import_users(users, hash_alg=None, app=None): From 278ee4a473a161902fb907d2833eb223e2f93884 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Fri, 8 May 2020 17:19:46 -0400 Subject: [PATCH 14/17] Doc nits --- firebase_admin/_auth_client.py | 4 ++-- firebase_admin/_user_mgt.py | 2 +- firebase_admin/auth.py | 7 +++---- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index cfc59da5c..3f34abba1 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -361,9 +361,9 @@ def delete_user(self, uid): def delete_users(self, uids): """Deletes the users specified by the given identifiers. - Deleting a non-existing user won't generate an error. (i.e. this method is + Deleting a non-existing user does not generate an error (the method is idempotent.) Non-existing users are considered to be successfully - deleted, and will therefore be counted in the + deleted and are therefore included in the `DeleteUserResult.success_count` value. A maximum of 1000 identifiers may be supplied. If more than 1000 diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index ba646eed5..794ed19f0 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -425,7 +425,7 @@ def failure_count(self): @property def errors(self): - """A list of ``auth.ErrorInfo`` instances describing the errors that + """A list of `auth.ErrorInfo` instances describing the errors that were encountered during the deletion. Length of this list is equal to `failure_count`. """ diff --git a/firebase_admin/auth.py b/firebase_admin/auth.py index cd52c2534..5d2fe0f68 100644 --- a/firebase_admin/auth.py +++ b/firebase_admin/auth.py @@ -508,10 +508,9 @@ def delete_user(uid, app=None): def delete_users(uids, app=None): """Deletes the users specified by the given identifiers. - Deleting a non-existing user won't generate an error. (i.e. this method is - idempotent.) Non-existing users are considered to be successfully - deleted, and will therefore be counted in the - `DeleteUserResult.success_count` value. + Deleting a non-existing user does not generate an error (the method is + idempotent.) Non-existing users are considered to be successfully deleted + and are therefore included in the `DeleteUserResult.success_count` value. A maximum of 1000 identifiers may be supplied. If more than 1000 identifiers are supplied, this method raises a `ValueError`. From 708701655530e2863e63e8ae6dc41ca7dbf2e2ca Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Fri, 8 May 2020 17:23:47 -0400 Subject: [PATCH 15/17] lint --- firebase_admin/_auth_client.py | 5 +++-- firebase_admin/_user_mgt.py | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index 3f34abba1..c3c005bf9 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -230,7 +230,8 @@ def _is_user_found(identifier, user_records): False) users = [_user_mgt.UserRecord(user) for user in response] - not_found = [identifier for identifier in identifiers if not _is_user_found(identifier, users)] + not_found = [ + identifier for identifier in identifiers if not _is_user_found(identifier, users)] return _user_mgt.GetUsersResult(users=users, not_found=not_found) @@ -382,7 +383,7 @@ def delete_users(self, uids): ValueError: If any of the identifiers are invalid or if more than 1000 identifiers are specified. """ - result = self._user_manager.delete_users(uids, force_delete=True) + result = self._user_manager.delete_users(uids, force_delete=True) return _user_mgt.DeleteUsersResult(result, len(uids)) def import_users(self, users, hash_alg=None): diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 794ed19f0..60e3115f9 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -645,7 +645,7 @@ def get_users(self, identifiers): .format(type(identifier))) body, http_resp = self._make_request( - 'post', '/accounts:lookup', json=payload) + 'post', '/accounts:lookup', json=payload) if not http_resp.ok: raise _auth_utils.UnexpectedResponseError( 'Failed to get users.', http_response=http_resp) @@ -773,7 +773,7 @@ def delete_users(self, uids, force_delete=False): _auth_utils.validate_uid(uid, required=True) body, http_resp = self._make_request('post', '/accounts:batchDelete', - json={'localIds': uids, 'force': force_delete}) + json={'localIds': uids, 'force': force_delete}) if not isinstance(body, dict): raise _auth_utils.UnexpectedResponseError( 'Unexpected response from server while attempting to delete users.', From ba60c6503773d5b77df9453ba82dbab46969ab01 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 11 May 2020 14:42:09 -0400 Subject: [PATCH 16/17] more review feedback --- firebase_admin/_auth_client.py | 4 +--- firebase_admin/_user_identifier.py | 37 ++++++++++++++++++++++++++---- firebase_admin/_user_mgt.py | 18 ++++++--------- tests/test_user_mgt.py | 29 +++++++---------------- 4 files changed, 48 insertions(+), 40 deletions(-) diff --git a/firebase_admin/_auth_client.py b/firebase_admin/_auth_client.py index c3c005bf9..12d60592e 100644 --- a/firebase_admin/_auth_client.py +++ b/firebase_admin/_auth_client.py @@ -225,9 +225,7 @@ def _matches(identifier, user_record): raise TypeError("Unexpected type: {}".format(type(identifier))) def _is_user_found(identifier, user_records): - return next( - (True for user_record in user_records if _matches(identifier, user_record)), - False) + return any(_matches(identifier, user_record) for user_record in user_records) users = [_user_mgt.UserRecord(user) for user in response] not_found = [ diff --git a/firebase_admin/_user_identifier.py b/firebase_admin/_user_identifier.py index d67d2ac56..072f9b054 100644 --- a/firebase_admin/_user_identifier.py +++ b/firebase_admin/_user_identifier.py @@ -14,6 +14,8 @@ """Classes to uniquely identify a user.""" +from firebase_admin import _auth_utils + class UserIdentifier: """Identifies a user to be looked up.""" @@ -30,7 +32,12 @@ def __init__(self, uid): Args: uid: A user ID string. """ - self.uid = uid + _auth_utils.validate_uid(uid, required=True) + self._uid = uid + + @property + def uid(self): + return self._uid class EmailIdentifier(UserIdentifier): @@ -45,7 +52,12 @@ def __init__(self, email): Args: email: A user email address string. """ - self.email = email + _auth_utils.validate_email(email, required=True) + self._email = email + + @property + def email(self): + return self._email class PhoneIdentifier(UserIdentifier): @@ -60,7 +72,12 @@ def __init__(self, phone_number): Args: phone_number: A phone number string. """ - self.phone_number = phone_number + _auth_utils.validate_phone(phone_number, required=True) + self._phone_number = phone_number + + @property + def phone_number(self): + return self._phone_number class ProviderIdentifier(UserIdentifier): @@ -76,5 +93,15 @@ def __init__(self, provider_id, provider_uid):     provider_id: A provider ID string.     provider_uid: A provider UID string. """ - self.provider_id = provider_id - self.provider_uid = provider_uid + _auth_utils.validate_provider_id(provider_id, required=True) + _auth_utils.validate_provider_uid(provider_uid, required=True) + self._provider_id = provider_id + self._provider_uid = provider_uid + + @property + def provider_id(self): + return self._provider_id + + @property + def provider_uid(self): + return self._provider_uid diff --git a/firebase_admin/_user_mgt.py b/firebase_admin/_user_mgt.py index 60e3115f9..0307959f3 100644 --- a/firebase_admin/_user_mgt.py +++ b/firebase_admin/_user_mgt.py @@ -15,6 +15,7 @@ """Firebase user management sub module.""" import base64 +from collections import defaultdict import json from urllib import parse @@ -621,24 +622,19 @@ def get_users(self, identifiers): if len(identifiers) > 100: raise ValueError('`identifiers` parameter must have <= 100 entries.') - payload = {} + payload = defaultdict(list) for identifier in identifiers: if isinstance(identifier, _user_identifier.UidIdentifier): - _auth_utils.validate_uid(identifier.uid, required=True) - payload['localId'] = payload.get('localId', []) + [identifier.uid] + payload['localId'].append(identifier.uid) elif isinstance(identifier, _user_identifier.EmailIdentifier): - _auth_utils.validate_email(identifier.email, required=True) - payload['email'] = payload.get('email', []) + [identifier.email] + payload['email'].append(identifier.email) elif isinstance(identifier, _user_identifier.PhoneIdentifier): - _auth_utils.validate_phone(identifier.phone_number, required=True) - payload['phoneNumber'] = payload.get('phoneNumber', []) + [identifier.phone_number] + payload['phoneNumber'].append(identifier.phone_number) elif isinstance(identifier, _user_identifier.ProviderIdentifier): - _auth_utils.validate_provider_id(identifier.provider_id, required=True) - _auth_utils.validate_provider_uid(identifier.provider_uid, required=True) - payload['federatedUserId'] = payload.get('federatedUserId', []) + [{ + payload['federatedUserId'].append({ 'providerId': identifier.provider_id, 'rawId': identifier.provider_uid - }] + }) else: raise ValueError( 'Invalid entry in "identifiers" list. Unsupported type: {}' diff --git a/tests/test_user_mgt.py b/tests/test_user_mgt.py index 2519e25a5..79e23373f 100644 --- a/tests/test_user_mgt.py +++ b/tests/test_user_mgt.py @@ -349,34 +349,21 @@ def test_identifiers_that_do_not_exist(self, user_mgt_app): assert get_users_results.users == [] assert get_users_results.not_found == not_found_ids - def test_invalid_uid(self, user_mgt_app): + def test_invalid_uid(self): with pytest.raises(ValueError): - auth.get_users([auth.UidIdentifier('too long ' + '.'*128)], app=user_mgt_app) + auth.UidIdentifier('too long ' + '.'*128) - def test_invalid_email(self, user_mgt_app): + def test_invalid_email(self): with pytest.raises(ValueError): - auth.get_users([auth.EmailIdentifier('invalid email addr')], app=user_mgt_app) + auth.EmailIdentifier('invalid email addr') - def test_invalid_phone_number(self, user_mgt_app): + def test_invalid_phone_number(self): with pytest.raises(ValueError): - auth.get_users([auth.PhoneIdentifier('invalid phone number')], app=user_mgt_app) + auth.PhoneIdentifier('invalid phone number') - def test_invalid_provider(self, user_mgt_app): + def test_invalid_provider(self): with pytest.raises(ValueError): - auth.get_users( - [auth.ProviderIdentifier(provider_id='', provider_uid='')], - app=user_mgt_app) - - def test_single_bad_identifier(self, user_mgt_app): - identifiers = [ - auth.UidIdentifier('valid_id1'), - auth.UidIdentifier('valid_id2'), - auth.UidIdentifier('invalid id; too long. ' + '.'*128), - auth.UidIdentifier('valid_id4'), - auth.UidIdentifier('valid_id5')] - - with pytest.raises(ValueError): - auth.get_users(identifiers, app=user_mgt_app) + auth.ProviderIdentifier(provider_id='', provider_uid='') def test_success(self, user_mgt_app): mock_users = [{ From e9dafc1dfa7ea0b5f2ff479c283ccf93f4378f07 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 11 May 2020 15:31:49 -0400 Subject: [PATCH 17/17] combine validate/assign in ctor --- firebase_admin/_user_identifier.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/firebase_admin/_user_identifier.py b/firebase_admin/_user_identifier.py index 072f9b054..85a224e0b 100644 --- a/firebase_admin/_user_identifier.py +++ b/firebase_admin/_user_identifier.py @@ -32,8 +32,7 @@ def __init__(self, uid): Args: uid: A user ID string. """ - _auth_utils.validate_uid(uid, required=True) - self._uid = uid + self._uid = _auth_utils.validate_uid(uid, required=True) @property def uid(self): @@ -52,8 +51,7 @@ def __init__(self, email): Args: email: A user email address string. """ - _auth_utils.validate_email(email, required=True) - self._email = email + self._email = _auth_utils.validate_email(email, required=True) @property def email(self): @@ -72,8 +70,7 @@ def __init__(self, phone_number): Args: phone_number: A phone number string. """ - _auth_utils.validate_phone(phone_number, required=True) - self._phone_number = phone_number + self._phone_number = _auth_utils.validate_phone(phone_number, required=True) @property def phone_number(self): @@ -93,10 +90,9 @@ def __init__(self, provider_id, provider_uid):     provider_id: A provider ID string.     provider_uid: A provider UID string. """ - _auth_utils.validate_provider_id(provider_id, required=True) - _auth_utils.validate_provider_uid(provider_uid, required=True) - self._provider_id = provider_id - self._provider_uid = provider_uid + self._provider_id = _auth_utils.validate_provider_id(provider_id, required=True) + self._provider_uid = _auth_utils.validate_provider_uid( + provider_uid, required=True) @property def provider_id(self):