Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

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

Merged
merged 23 commits into from
May 6, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
150 changes: 148 additions & 2 deletions src/auth/auth-api-request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
import * as validator from '../utils/validator';

import {deepCopy, deepExtend} from '../utils/deep-copy';
import {
UserIdentifier, isUidIdentifier, isEmailIdentifier, isPhoneIdentifier, isProviderIdentifier,
} from './identifier';
import {FirebaseApp} from '../firebase-app';
import {AuthClientErrorCode, FirebaseAuthError} from '../utils/error';
import {
Expand Down Expand Up @@ -434,12 +437,21 @@ export const FIREBASE_AUTH_DOWNLOAD_ACCOUNT = new ApiSettings('/accounts:batchGe
}
});

interface GetAccountInfoRequest {
localId?: string[];
email?: string[];
phoneNumber?: string[];
federatedUserId?: Array<{
providerId: string,
rawId: string,
}>;
}

/** Instantiates the getAccountInfo endpoint settings. */
export const FIREBASE_AUTH_GET_ACCOUNT_INFO = new ApiSettings('/accounts:lookup', 'POST')
// Set request validator.
.setRequestValidator((request: any) => {
if (!request.localId && !request.email && !request.phoneNumber) {
.setRequestValidator((request: GetAccountInfoRequest) => {
if (!request.localId && !request.email && !request.phoneNumber && !request.federatedUserId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you validate the federatedUserId content?

Copy link
Member Author

Choose a reason for hiding this comment

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

Only that it's a non-empty string. (I'm not aware of a way to do any better...)

throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server request is missing user identifier');
Expand All @@ -452,6 +464,21 @@ export const FIREBASE_AUTH_GET_ACCOUNT_INFO = new ApiSettings('/accounts:lookup'
}
});

/**
* Instantiates the getAccountInfo endpoint settings for use when fetching info
* for multiple accounts.
*/
export const FIREBASE_AUTH_GET_ACCOUNTS_INFO = new ApiSettings('/accounts:lookup', 'POST')
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you need to duplicate FIREBASE_AUTH_GET_ACCOUNT_INFO above? It is the same underlying API with similar request /response format.

Copy link
Member Author

Choose a reason for hiding this comment

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

The response validator differs. For singular lookups, a usernotfound error is raised if the user cannot be found. For bulk lookups, the return value is handled differently, so we don't want to throw this error.

An alternative would be to raise 'usernotfound' in all the calling locations rather than the response validator. This would make the FIREBASE_AUTH_GET_ACCOUNT[S]_INFO consistent (and match better with the actual backend api) but would involve duplicating the error handling code to a number of locations. (Which I don't object to. This file feels like the backend api, so removing things from it that adapt it to our client facing api seems to be a reasonable thing to do. The duplication could be minimized by using a helper.)

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, I think this is fine.

// Set request validator.
.setRequestValidator((request: GetAccountInfoRequest) => {
if (!request.localId && !request.email && !request.phoneNumber && !request.federatedUserId) {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server request is missing user identifier');
}
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
});


/** Instantiates the deleteAccount endpoint settings. */
export const FIREBASE_AUTH_DELETE_ACCOUNT = new ApiSettings('/accounts:delete', 'POST')
// Set request validator.
Expand All @@ -463,6 +490,45 @@ export const FIREBASE_AUTH_DELETE_ACCOUNT = new ApiSettings('/accounts:delete',
}
});

interface BatchDeleteAccountsRequest {
localIds?: string[];
force?: boolean;
}
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
interface BatchDeleteErrorInfo {
index?: number;
localId?: string;
message?: string;
}
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
export interface BatchDeleteAccountsResponse {
errors?: BatchDeleteErrorInfo[];
}

export const FIREBASE_AUTH_BATCH_DELETE_ACCOUNTS = new ApiSettings('/accounts:batchDelete', 'POST')
.setRequestValidator((request: BatchDeleteAccountsRequest) => {
if (!request.localIds) {
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't you also validate the force field?

Copy link
Member Author

Choose a reason for hiding this comment

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

Shield generators back online.

Copy link
Contributor

Choose a reason for hiding this comment

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

:)

throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server request is missing user identifiers');
}
})
.setResponseValidator((response: BatchDeleteAccountsResponse) => {
if (response.errors) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Eliminate the if condition with:

const errors = response.errors || [];

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.

response.errors.forEach((batchDeleteErrorInfo) => {
if (batchDeleteErrorInfo.index === undefined) {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server BatchDeleteAccountResponse is missing an errors.index field');
}
if (!batchDeleteErrorInfo.localId) {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server BatchDeleteAccountResponse is missing an errors.localId field');
}
// Allow the (error) message to be missing/undef.
});
}
});

/** Instantiates the setAccountInfo endpoint settings for updating existing accounts. */
export const FIREBASE_AUTH_SET_ACCOUNT_INFO = new ApiSettings('/accounts:update', 'POST')
// Set request validator.
Expand Down Expand Up @@ -785,6 +851,62 @@ export abstract class AbstractAuthRequestHandler {
return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNT_INFO, request);
}

/**
* Looks up multiple users by their identifiers (uid, email, etc).
*
* @param {UserIdentifier[]} identifiers The identifiers indicating the users
* to be looked up. Must have <= 100 entries.
* @param {Promise<object>} A promise that resolves with the set of successfully
* looked up users. Possibly empty if no users were looked up.
*/
public getAccountInfoByIdentifiers(identifiers: UserIdentifier[]): Promise<object> {
if (identifiers.length === 0) {
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
return Promise.resolve({users: []});
} else if (identifiers.length > 100) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe define 100 as a constant like we do for other similar max fields?

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 done for bulk delete (which was mistakenly set to 100 instead of 1000!)

throw new FirebaseAuthError(
AuthClientErrorCode.MAXIMUM_USER_COUNT_EXCEEDED,
'`identifiers` parameter must have <= 100 entries.');
}

const request: GetAccountInfoRequest = {};

for (const id of identifiers) {
if (isUidIdentifier(id)) {
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
if (!validator.isUid(id.uid)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_UID);
}
request.localId ? request.localId.push(id.uid) : request.localId = [id.uid];
} else if (isEmailIdentifier(id)) {
if (!validator.isEmail(id.email)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_EMAIL);
}
request.email ? request.email.push(id.email) : request.email = [id.email];
} else if (isPhoneIdentifier(id)) {
if (!validator.isPhoneNumber(id.phoneNumber)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PHONE_NUMBER);
}
request.phoneNumber ? request.phoneNumber.push(id.phoneNumber) : request.phoneNumber = [id.phoneNumber];
} else if (isProviderIdentifier(id)) {
if (!validator.isNonEmptyString(id.providerUid) || !validator.isNonEmptyString(id.providerId)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_ID);
}
const federatedUserId = {
providerId: id.providerId,
rawId: id.providerUid,
};
request.federatedUserId
? request.federatedUserId.push(federatedUserId)
: request.federatedUserId = [federatedUserId];
} else {
throw new FirebaseAuthError(
AuthClientErrorCode.INVALID_ARGUMENT,
'Unrecognized identifier: ' + id);
}
}

return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNTS_INFO, request);
}

/**
* Exports the users (single batch only) with a size of maxResults and starting from
* the offset as specified by pageToken.
Expand Down Expand Up @@ -882,6 +1004,30 @@ export abstract class AbstractAuthRequestHandler {
return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_DELETE_ACCOUNT, request);
}

public deleteAccounts(uids: string[], force: boolean): Promise<BatchDeleteAccountsResponse> {
if (uids.length === 0) {
hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
return Promise.resolve({});
} else if (uids.length > 100) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here regarding defining a const for this upper limit.

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.

throw new FirebaseAuthError(
AuthClientErrorCode.MAXIMUM_USER_COUNT_EXCEEDED,
'`uids` parameter must have <= 100 entries.');
}

const request: BatchDeleteAccountsRequest = {
localIds: [],
force,
};

uids.forEach((uid) => {
if (!validator.isUid(uid)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_UID);
}
request.localIds!.push(uid);
});

return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_BATCH_DELETE_ACCOUNTS, request);
}

/**
* Sets additional developer claims on an existing user identified by provided UID.
*
Expand Down
118 changes: 117 additions & 1 deletion src/auth/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,15 @@
*/

import {UserRecord, CreateRequest, UpdateRequest} from './user-record';
import {
UserIdentifier, isUidIdentifier, isEmailIdentifier, isPhoneIdentifier, isProviderIdentifier,
} from './identifier';
import {FirebaseApp} from '../firebase-app';
import {FirebaseTokenGenerator, cryptoSignerFromApp} from './token-generator';
import {
AbstractAuthRequestHandler, AuthRequestHandler, TenantAwareAuthRequestHandler,
} from './auth-api-request';
import {AuthClientErrorCode, FirebaseAuthError, ErrorInfo} from '../utils/error';
import {AuthClientErrorCode, FirebaseAuthError, ErrorInfo, FirebaseArrayIndexError} from '../utils/error';
import {FirebaseServiceInterface, FirebaseServiceInternalsInterface} from '../firebase-service';
import {
UserImportOptions, UserImportRecord, UserImportResult,
Expand Down Expand Up @@ -53,13 +56,35 @@ class AuthInternals implements FirebaseServiceInternalsInterface {
}


/** Represents the result of the {@link admin.auth.getUsers()} API. */
export interface GetUsersResult {
/**
* Set of user records, corresponding to the set of users that were
* requested. Only users that were found are listed here. The result set is
* unordered.
*/
users: UserRecord[];

/** Set of identifiers that were requested, but not found. */
notFound: UserIdentifier[];
}


/** Response object for a listUsers operation. */
export interface ListUsersResult {
users: UserRecord[];
pageToken?: string;
}


/** Response object for deleteUsers operation. */
export interface DeleteUsersResult {
failureCount: number;
successCount: number;
errors: FirebaseArrayIndexError[];
}


/** Interface representing a decoded ID token. */
export interface DecodedIdToken {
aud: string;
Expand Down Expand Up @@ -192,6 +217,57 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> {
});
}

/**
* 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.
*
* Only a maximum of 100 identifiers may be supplied. If more than 100 identifiers are supplied,
* this method will immediately throw a FirebaseAuthError.
*
* @param identifiers The identifiers used to indicate which user records should be returned. Must
* have <= 100 entries.
* @return {Promise<GetUsersResult>} A promise that resolves to the corresponding user records.
* @throws FirebaseAuthError If any of the identifiers are invalid or if more than 100
* identifiers are specified.
*/
public getUsers(identifiers: UserIdentifier[]): Promise<GetUsersResult> {
return this.authRequestHandler
.getAccountInfoByIdentifiers(identifiers)
.then((response: any) => {
/**
* Checks if the specified identifier is within the list of
* UserRecords.
*/
const isUserFound = ((id: UserIdentifier, urs: UserRecord[]): boolean => {
Copy link
Contributor

Choose a reason for hiding this comment

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

what does urs stand for? Maybe pick a better name?

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.
(urs => userRecords, ur => userRecord)

return !!urs.find((ur) => {
if (isUidIdentifier(id)) {
return id.uid === ur.uid;
} else if (isEmailIdentifier(id)) {
return id.email === ur.email;
} else if (isPhoneIdentifier(id)) {
return id.phoneNumber === ur.phoneNumber;
} else if (isProviderIdentifier(id)) {
const matchingUserInfo = ur.providerData.find((userInfo) => {
return id.providerId === userInfo.providerId;
});
return !!matchingUserInfo && id.providerUid === matchingUserInfo.uid;
} else {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'Unhandled identifier type');
}
});
});

const users = response.users ? response.users.map((user: any) => new UserRecord(user)) : [];
const notFound = identifiers.filter((id) => !isUserFound(id, users));

return { users, notFound };
});
}

/**
* Exports a batch of user accounts. Batch size is determined by the maxResults argument.
* Starting point of the batch is determined by the pageToken argument.
Expand Down Expand Up @@ -263,6 +339,46 @@ export class BaseAuth<T extends AbstractAuthRequestHandler> {
});
}

public deleteUsers(uids: string[]): Promise<DeleteUsersResult> {
return this.authRequestHandler.deleteAccounts(uids, /*force=*/true)
.then((batchDeleteAccountsResponse) => {
const result: DeleteUsersResult = {
failureCount: 0,
successCount: uids.length,
errors: [],
};

hiranya911 marked this conversation as resolved.
Show resolved Hide resolved
if (batchDeleteAccountsResponse.errors) {
result.failureCount = batchDeleteAccountsResponse.errors.length;
result.successCount = uids.length - batchDeleteAccountsResponse.errors.length;
result.errors = batchDeleteAccountsResponse.errors.map((batchDeleteErrorInfo) => {
if (batchDeleteErrorInfo.index === undefined) {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'Corrupt BatchDeleteAccountsResponse detected');
}

const errMsgToError = (msg?: string): FirebaseAuthError => {
// We unconditionally set force=true, so the 'NOT_DISABLED' error
// should not be possible.
if (msg && msg.startsWith('NOT_DISABLED :')) {
return new FirebaseAuthError(AuthClientErrorCode.USER_NOT_DISABLED, batchDeleteErrorInfo.message);
} else {
return new FirebaseAuthError(AuthClientErrorCode.INTERNAL_ERROR, batchDeleteErrorInfo.message);
}
};

return {
index: batchDeleteErrorInfo.index,
error: errMsgToError(batchDeleteErrorInfo.message),
};
});
}

return result;
});
}

/**
* Updates an existing user with the properties provided.
*
Expand Down
Loading