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

Allow bypassing getUser request when creating or updating a user #2104

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 6 additions & 2 deletions etc/firebase-admin.auth.api.md
Expand Up @@ -73,7 +73,9 @@ export abstract class BaseAuth {
createCustomToken(uid: string, developerClaims?: object): Promise<string>;
createProviderConfig(config: AuthProviderConfig): Promise<AuthProviderConfig>;
createSessionCookie(idToken: string, sessionCookieOptions: SessionCookieOptions): Promise<string>;
createUser(properties: CreateRequest): Promise<UserRecord>;
createUser(properties: CreateRequest, noFetchUserRecord?: boolean): Promise<UserRecord>;
// (undocumented)
createUser(properties: CreateRequest, noFetchUserRecord: true): Promise<void>;
deleteProviderConfig(providerId: string): Promise<void>;
deleteUser(uid: string): Promise<void>;
deleteUsers(uids: string[]): Promise<DeleteUsersResult>;
Expand All @@ -93,7 +95,9 @@ export abstract class BaseAuth {
revokeRefreshTokens(uid: string): Promise<void>;
setCustomUserClaims(uid: string, customUserClaims: object | null): Promise<void>;
updateProviderConfig(providerId: string, updatedConfig: UpdateAuthProviderRequest): Promise<AuthProviderConfig>;
updateUser(uid: string, properties: UpdateRequest): Promise<UserRecord>;
updateUser(uid: string, properties: UpdateRequest, noFetchUserRecord?: boolean): Promise<UserRecord>;
// (undocumented)
updateUser(uid: string, properties: UpdateRequest, noFetchUserRecord: true): Promise<void>;
// @alpha (undocumented)
_verifyAuthBlockingToken(token: string, audience?: string): Promise<DecodedAuthBlockingToken>;
verifyIdToken(idToken: string, checkRevoked?: boolean): Promise<DecodedIdToken>;
Expand Down
33 changes: 25 additions & 8 deletions src/auth/base-auth.ts
Expand Up @@ -413,15 +413,24 @@ export abstract class BaseAuth {
*
* @param properties - The properties to set on the
* new user record to be created.
*
* @param noFetchUserRecord - Defaults to `false` so that the updated
* user record will be fetched from the backend as a separate operation and returned.
* If `true`, then the updated user will not be fetched and will return `undefined`.
*
* @returns A promise fulfilled with the user
* data corresponding to the newly created user.
* data corresponding to the newly created user,
* or fulfilled with`undefined` if `noFetchUserRecord` was true.
*/
public createUser(properties: CreateRequest): Promise<UserRecord> {
public createUser(properties: CreateRequest, noFetchUserRecord?: boolean): Promise<UserRecord>
public createUser(properties: CreateRequest, noFetchUserRecord: true): Promise<void>
public createUser(properties: CreateRequest, noFetchUserRecord = false): Promise<UserRecord | void> {
return this.authRequestHandler.createNewAccount(properties)
.then((uid) => {
// Return the corresponding user record.
return this.getUser(uid);
if (!noFetchUserRecord) {
// Return the corresponding user record.
return this.getUser(uid);
}
})
.catch((error) => {
if (error.code === 'auth/user-not-found') {
Expand Down Expand Up @@ -527,11 +536,17 @@ export abstract class BaseAuth {
* @param uid - The `uid` corresponding to the user to update.
* @param properties - The properties to update on
* the provided user.
* @param noFetchUserRecord - Defaults to `false` so that the updated
* user record will be fetched from the backend as a separate operation and returned.
* If `true`, then the updated user will not be fetched and will return `undefined`.
*
* @returns A promise fulfilled with the
* updated user data.
* updated user data,
* or fulfilled with`undefined` if `noFetchUserRecord` was true.
*/
public updateUser(uid: string, properties: UpdateRequest): Promise<UserRecord> {
public updateUser(uid: string, properties: UpdateRequest, noFetchUserRecord?: boolean): Promise<UserRecord>
public updateUser(uid: string, properties: UpdateRequest, noFetchUserRecord: true): Promise<void>
public updateUser(uid: string, properties: UpdateRequest, noFetchUserRecord = false): Promise<UserRecord | void> {
// Although we don't really advertise it, we want to also handle linking of
// non-federated idps with this call. So if we detect one of them, we'll
// adjust the properties parameter appropriately. This *does* imply that a
Expand Down Expand Up @@ -578,8 +593,10 @@ export abstract class BaseAuth {

return this.authRequestHandler.updateExistingAccount(uid, properties)
.then((existingUid) => {
// Return the corresponding user record.
return this.getUser(existingUid);
if (!noFetchUserRecord) {
// Return the corresponding user record.
return this.getUser(existingUid);
}
});
}

Expand Down
40 changes: 40 additions & 0 deletions test/unit/auth/auth.spec.ts
Expand Up @@ -1772,6 +1772,26 @@ AUTH_CONFIGS.forEach((testConfig) => {
});
});

it('should resolve with undefined on createNewAccount request success with noFetchUserRecord == true', () => {
// Stub createNewAccount to return expected uid.
const createUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'createNewAccount')
.resolves(uid);
// Stub getAccountInfoByUid to return expected result.
const getUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByUid')
.resolves(expectedGetAccountInfoResult);
stubs.push(createUserStub);
stubs.push(getUserStub);
return auth.createUser(propertiesToCreate, true)
.then((resp) => {
// Confirm underlying API called with expected parameters.
// getUser should NOT be called
expect(createUserStub).to.have.been.calledOnce.and.calledWith(propertiesToCreate);
expect(getUserStub).not.to.have.been.called;
// Confirm expected user record response returned.
expect(resp).to.be.undefined;
});
});

it('should throw an error when createNewAccount returns an error', () => {
// Stub createNewAccount to throw a backend error.
const createUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'createNewAccount')
Expand Down Expand Up @@ -2218,6 +2238,26 @@ AUTH_CONFIGS.forEach((testConfig) => {
});
});

it('should resolve with undefined on updateExistingAccount request success with noFetchUserRecord=true', () => {
// Stub updateExistingAccount to return expected uid.
const updateUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'updateExistingAccount')
.resolves(uid);
// Stub getAccountInfoByUid to return expected result.
const getUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByUid')
.resolves(expectedGetAccountInfoResult);
stubs.push(updateUserStub);
stubs.push(getUserStub);
return auth.updateUser(uid, propertiesToEdit, true)
.then((resp) => {
// Confirm underlying API called with expected parameters.
// getUser should NOT be called
expect(updateUserStub).to.have.been.calledOnce.and.calledWith(uid, propertiesToEdit);
expect(getUserStub).not.to.have.been.called;
// Confirm expected user record response returned.
expect(resp).to.be.undefined;
});
});

it('should throw an error when updateExistingAccount returns an error', () => {
// Stub updateExistingAccount to throw a backend error.
const updateUserStub = sinon.stub(testConfig.RequestHandler.prototype, 'updateExistingAccount')
Expand Down