Skip to content

feat(auth): Add bulk get/delete methods #400

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 20 commits into from
May 12, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
63 changes: 63 additions & 0 deletions firebase_admin/_identifier.py
Original file line number Diff line number Diff line change
@@ -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
191 changes: 189 additions & 2 deletions firebase_admin/_user_mgt.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
from urllib import parse

import requests
import iso8601

from firebase_admin import _auth_utils
from firebase_admin import _identifier
from firebase_admin import _user_import


Expand All @@ -41,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):
Expand All @@ -65,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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it literally return the string "None?" The cap makes me thinks so . . . and if so this should be backticked probably.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it returns the python None value (roughly corresponding to null in other languages). But backticks seem appropriate regardless (done).

never active.
"""
return self._last_refresh_timestamp


class UserInfo:
"""A collection of standard profile information for a user.
Expand Down Expand Up @@ -216,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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

datetime.timestamp() is python 3.3. But I think that's ok now.

return UserMetadata(
_int_or_none('createdAt'), _int_or_none('lastLoginAt'), last_refresh_at_millis)

@property
def provider_data(self):
Expand Down Expand Up @@ -331,6 +351,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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


Args:
result: BatchDeleteAccountsResponse: The proto response, wrapped in a
BatchDeleteAccountsResponse instance.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal.

Is the result after the colon above code-fonted as well?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

backticks for literal

Done.

after the colon

Hm. I'm not sure that should even be there. I've removed it and rephrased it slightly.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal, and "are considered to be successfully deleted."

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done/done.

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."""

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


def __init__(self, errors=None):
"""Constructs a BatchDeleteAccountsResponse instance, corresponseing to

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal, here and just below.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done (also fixed spelling typo)

the json representing the BatchDeleteAccountsResponse proto.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest "JSON"

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.


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."""

Expand Down Expand Up @@ -483,6 +582,53 @@ 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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest backticks for literal.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done. (Also json->JSON)


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 []
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)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we push this validation and the subsequent marshaling to the corresponding classes?

Each identifier class can expose something like:

def marshal(self, payload):
   payload['localId'] = payload.get('localId', []).append(self.uid)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this (just now) but didn't really like the results. It makes this function shorter, but it alters the UserIdentifier types such that they need to know how the json payload is constructed. (For instance, should it be ProviderIdentifiers job to know that provider_uid maps to rawId on the wire?) It also spreads building the payload across 2 files and 5 classes, instead of having it in just one spot.

One possible improvement we could make here is to eliminate the validation (or at least, refactor it somewhat). As per one of your other comments, I've already moved some validation to the ctors, but that won't stop users from changing the values after construction, but before calling get_users(), so we should keep the validation here. But if we altered (eg) UidIdentifier.uid to be UidIdentifier._uid and added a getter (essentially making these objects immutable) I think that could allow us to skip validation here entirely, which would allow this block to focus solely on creating the payload. I've done that. PTAL.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like how validation turned out (and the immutability of the properties). I don't feel strongly about the marshalling part. When I first suggested it, I felt that having each class being in charge of marshalling was a good thing. If we add more implementations of the UserIdentifier interface in the future, that's when this will matter more. I'm ok with leaving this as is for now.

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:
Expand Down Expand Up @@ -592,6 +738,47 @@ 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.
UnexpectedResponseError: If the backend server responds with an
unexpected message.
"""
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:
Expand Down
Loading