Skip to content

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

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

import {deepCopy, deepExtend} from '../utils/deep-copy';
import {
UserIdentifier, isUidIdentifier, isEmailIdentifier, isPhoneIdentifier,
isProviderIdentifier, UidIdentifier, EmailIdentifier, PhoneIdentifier,
ProviderIdentifier,
} from './identifier';
import {FirebaseApp} from '../firebase-app';
import {AuthClientErrorCode, FirebaseAuthError} from '../utils/error';
import {
Expand Down Expand Up @@ -65,6 +70,12 @@ const MAX_DOWNLOAD_ACCOUNT_PAGE_SIZE = 1000;
/** Maximum allowed number of users to batch upload at one time. */
const MAX_UPLOAD_ACCOUNT_BATCH_SIZE = 1000;

/** Maximum allowed number of users to batch get at one time. */
const MAX_GET_ACCOUNTS_BATCH_SIZE = 100;

/** Maximum allowed numberof users to batch delete at one time. */
Copy link
Contributor

Choose a reason for hiding this comment

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

number of

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.

const MAX_DELETE_ACCOUNTS_BATCH_SIZE = 1000;

/** Minimum allowed session cookie duration in seconds (5 minutes). */
const MIN_SESSION_COOKIE_DURATION_SECS = 5 * 60;

Expand Down Expand Up @@ -463,12 +474,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 @@ -481,6 +501,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');
}
});


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

interface BatchDeleteAccountsRequest {
localIds?: string[];
force?: boolean;
}

interface BatchDeleteErrorInfo {
index?: number;
localId?: string;
message?: string;
}

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');
}
if (typeof request.force === 'undefined' || request.force !== true) {
throw new FirebaseAuthError(
AuthClientErrorCode.INTERNAL_ERROR,
'INTERNAL ASSERT FAILED: Server request is missing force=true field');
}
})
.setResponseValidator((response: BatchDeleteAccountsResponse) => {
const errors = response.errors || [];
errors.forEach((batchDeleteErrorInfo) => {
if (typeof 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 @@ -725,6 +805,47 @@ export abstract class AbstractAuthRequestHandler {
return (validator.isNonNullObject(response) && response.error && response.error.message) || null;
}

private static addUidToRequest(id: UidIdentifier, request: GetAccountInfoRequest): GetAccountInfoRequest {
if (!validator.isUid(id.uid)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_UID);
}
request.localId ? request.localId.push(id.uid) : request.localId = [id.uid];
return request;
}

private static addEmailToRequest(id: EmailIdentifier, request: GetAccountInfoRequest): GetAccountInfoRequest {
if (!validator.isEmail(id.email)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_EMAIL);
}
request.email ? request.email.push(id.email) : request.email = [id.email];
return request;
}

private static addPhoneToRequest(id: PhoneIdentifier, request: GetAccountInfoRequest): GetAccountInfoRequest {
if (!validator.isPhoneNumber(id.phoneNumber)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PHONE_NUMBER);
}
request.phoneNumber ? request.phoneNumber.push(id.phoneNumber) : request.phoneNumber = [id.phoneNumber];
return request;
}

private static addProviderToRequest(id: ProviderIdentifier, request: GetAccountInfoRequest): GetAccountInfoRequest {
if (!validator.isNonEmptyString(id.providerId)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_ID);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think invalid provider ID is different that invalid provider uid. The error is confusing here.

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.

}
if (!validator.isNonEmptyString(id.providerUid)) {
throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_UID);
}
const federatedUserId = {
providerId: id.providerId,
rawId: id.providerUid,
};
request.federatedUserId
? request.federatedUserId.push(federatedUserId)
: request.federatedUserId = [federatedUserId];
return request;
}

/**
* @param {FirebaseApp} app The app used to fetch access tokens to sign API requests.
* @constructor
Expand Down Expand Up @@ -811,6 +932,44 @@ 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) {
return Promise.resolve({users: []});
} else if (identifiers.length > MAX_GET_ACCOUNTS_BATCH_SIZE) {
throw new FirebaseAuthError(
AuthClientErrorCode.MAXIMUM_USER_COUNT_EXCEEDED,
'`identifiers` parameter must have <= ' + MAX_GET_ACCOUNTS_BATCH_SIZE + ' entries.');
}

let request: GetAccountInfoRequest = {};

for (const id of identifiers) {
if (isUidIdentifier(id)) {
request = AbstractAuthRequestHandler.addUidToRequest(id, request);
} else if (isEmailIdentifier(id)) {
request = AbstractAuthRequestHandler.addEmailToRequest(id, request);
} else if (isPhoneIdentifier(id)) {
request = AbstractAuthRequestHandler.addPhoneToRequest(id, request);
} else if (isProviderIdentifier(id)) {
request = AbstractAuthRequestHandler.addProviderToRequest(id, request);
} 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 @@ -908,6 +1067,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) {
return Promise.resolve({});
} else if (uids.length > MAX_DELETE_ACCOUNTS_BATCH_SIZE) {
throw new FirebaseAuthError(
AuthClientErrorCode.MAXIMUM_USER_COUNT_EXCEEDED,
'`uids` parameter must have <= ' + MAX_DELETE_ACCOUNTS_BATCH_SIZE + ' 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
Loading