From fa9934b62420fb4ef581c5d0bf2b6f2dd3827e9f Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 20 Jan 2020 16:25:05 -0500 Subject: [PATCH 1/9] Implement getUserByFederatedId() --- src/auth/auth-api-request.ts | 17 +++++++- src/auth/auth.ts | 21 +++++++++ src/index.d.ts | 15 +++++++ test/integration/auth.spec.ts | 44 +++++++++++++++++++ test/unit/auth/auth.spec.ts | 81 +++++++++++++++++++++++++++++++++++ 5 files changed, 177 insertions(+), 1 deletion(-) diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index 3b3f7a7f21..f1ee905695 100755 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -468,7 +468,7 @@ export const FIREBASE_AUTH_DOWNLOAD_ACCOUNT = new ApiSettings('/accounts:batchGe 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) { + if (!request.localId && !request.email && !request.phoneNumber && !request.federatedUserId) { throw new FirebaseAuthError( AuthClientErrorCode.INTERNAL_ERROR, 'INTERNAL ASSERT FAILED: Server request is missing user identifier'); @@ -811,6 +811,21 @@ export abstract class AbstractAuthRequestHandler { return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNT_INFO, request); } + public getAccountInfoByFederatedId(federatedId: string, federatedUid: string): Promise { + if (!validator.isNonEmptyString(federatedId) || !validator.isNonEmptyString(federatedUid)) { + throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_ID); + } + + const request = { + federatedUserId: [{ + providerId: federatedId, + rawId: federatedUid, + }], + }; + + return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNT_INFO, request); + } + /** * Exports the users (single batch only) with a size of maxResults and starting from * the offset as specified by pageToken. diff --git a/src/auth/auth.ts b/src/auth/auth.ts index eddea69cb3..90439326d9 100755 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -200,6 +200,27 @@ export class BaseAuth { }); } + /** + * Gets the user data for the user corresponding to a given federated id. + * + * See [Retrieve user data](/docs/auth/admin/manage-users#retrieve_user_data) + * for code samples and detailed documentation. + * + * @param federatedId The provider ID, for example, "google.com" for the + * Google provider. + * @param federatedUid The user identifier for the given provider. + * + * @return A promise fulfilled with the user data corresponding to the + * provided federated id. + */ + public getUserByFederatedId(federatedId: string, federatedUid: string): Promise { + return this.authRequestHandler.getAccountInfoByFederatedId(federatedId, federatedUid) + .then((response: any) => { + // Returns the user record populated with server response. + return new UserRecord(response.users[0]); + }); + } + /** * 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. diff --git a/src/index.d.ts b/src/index.d.ts index 5177647da2..82d584750d 100755 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -1530,6 +1530,21 @@ declare namespace admin.auth { */ getUserByPhoneNumber(phoneNumber: string): Promise; + /** + * Gets the user data for the user corresponding to a given federated id. + * + * See [Retrieve user data](/docs/auth/admin/manage-users#retrieve_user_data) + * for code samples and detailed documentation. + * + * @param federatedId The provider ID, for example, "google.com" for the + * Google provider. + * @param federatedUid The user identifier for the given provider. + * + * @return A promise fulfilled with the user data corresponding to the + * provided federated id. + */ + getUserByFederatedId(federatedId: string, federatedUid: string): Promise; + /** * Retrieves a list of users (single batch only) with a size of `maxResults` * starting from the offset as specified by `pageToken`. This is used to diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index b78e1e6c43..85f9f0d57c 100755 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -66,6 +66,7 @@ const mockUserData = { displayName: 'Random User ' + newUserUid, photoURL: 'http://www.example.com/' + newUserUid + '/photo.png', disabled: false, + }; const actionCodeSettings = { url: 'http://localhost/?a=1&b=2#c=3', @@ -168,6 +169,44 @@ describe('admin.auth', () => { }); }); + it('getUserByFederatedId() returns a user record with the matching federated id', async () => { + // TODO(rsgowman): Once we can link a provider id with a user, just do that + // here instead of creating a new user. + const importUser: admin.auth.UserImportRecord = { + uid: 'uid', + email: 'user@example.com', + phoneNumber: '+15555550000', + emailVerified: true, + disabled: false, + metadata: { + lastSignInTime: 'Thu, 01 Jan 1970 00:00:00 UTC', + creationTime: 'Thu, 01 Jan 1970 00:00:00 UTC', + toJSON: () => { throw new Error('Unimplemented'); }, + }, + providerData: [{ + displayName: 'User Name', + email: 'user@example.com', + phoneNumber: '+15555550000', + photoURL: 'http://example.com/user', + toJSON: () => { throw new Error('Unimplemented'); }, + providerId: 'google.com', + uid: 'google_uid', + }], + }; + + await safeDelete(importUser.uid); + await admin.auth().importUsers([importUser]); + + try { + await admin.auth().getUserByFederatedId('google.com', 'google_uid') + .then((userRecord) => { + expect(userRecord.uid).to.equal(importUser.uid); + }); + } finally { + await safeDelete(importUser.uid); + } + }); + it('listUsers() returns up to the specified number of users', () => { const promises: Array> = []; uids.forEach((uid) => { @@ -356,6 +395,11 @@ describe('admin.auth', () => { .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); + it('getUserByFederatedId() fails when called with a non-existing federated id', () => { + return admin.auth().getUserByFederatedId('google.com', nonexistentUid) + .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); + }); + it('updateUser() fails when called with a non-existing UID', () => { return admin.auth().updateUser(nonexistentUid, { emailVerified: true, diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 0451be7288..28675b072d 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1130,6 +1130,87 @@ AUTH_CONFIGS.forEach((testConfig) => { }); }); + describe('getUserByFederatedId()', () => { + const federatedId = 'google.com'; + const federatedUid = 'google_uid'; + const tenantId = testConfig.supportsTenantManagement ? undefined : TENANT_ID; + const expectedGetAccountInfoResult = getValidGetAccountInfoResponse(tenantId); + const expectedUserRecord = getValidUserRecord(expectedGetAccountInfoResult); + const expectedError = new FirebaseAuthError(AuthClientErrorCode.USER_NOT_FOUND); + + // Stubs used to simulate underlying api calls. + let stubs: sinon.SinonStub[] = []; + beforeEach(() => sinon.spy(validator, 'isEmail')); + afterEach(() => { + (validator.isEmail as any).restore(); + _.forEach(stubs, (stub) => stub.restore()); + stubs = []; + }); + + it('should be rejected given no federated id', () => { + expect(() => (auth as any).getUserByFederatedId()) + .to.throw(FirebaseAuthError) + .with.property('code', 'auth/invalid-provider-id'); + }); + + it('should be rejected given an invalid federated id', () => { + expect(() => auth.getUserByFederatedId('', 'uid')) + .to.throw(FirebaseAuthError) + .with.property('code', 'auth/invalid-provider-id'); + }); + + it('should be rejected given an invalid federated uid', () => { + expect(() => auth.getUserByFederatedId('id', '')) + .to.throw(FirebaseAuthError) + .with.property('code', 'auth/invalid-provider-id'); + }); + + it('should be rejected given an app which returns null access tokens', () => { + return nullAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should be rejected given an app which returns invalid access tokens', () => { + return malformedAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should be rejected given an app which fails to generate access tokens', () => { + return rejectedPromiseAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); + }); + + it('should resolve with a UserRecord on success', () => { + // Stub getAccountInfoByEmail to return expected result. + const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') + .resolves(expectedGetAccountInfoResult); + stubs.push(stub); + return auth.getUserByFederatedId(federatedId, federatedUid) + .then((userRecord) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); + // Confirm expected user record response returned. + expect(userRecord).to.deep.equal(expectedUserRecord); + }); + }); + + it('should throw an error when the backend returns an error', () => { + // Stub getAccountInfoByFederatedId to throw a backend error. + const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') + .rejects(expectedError); + stubs.push(stub); + return auth.getUserByFederatedId(federatedId, federatedUid) + .then((userRecord) => { + throw new Error('Unexpected success'); + }, (error) => { + // Confirm underlying API called with expected parameters. + expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); + // Confirm expected error returned. + expect(error).to.equal(expectedError); + }); + }); + }); + describe('deleteUser()', () => { const uid = 'abcdefghijklmnopqrstuvwxyz'; const expectedDeleteAccountResult = {kind: 'identitytoolkit#DeleteAccountResponse'}; From b7458e23c6f191f483f39e87b46a4de2d72a1a32 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 20 Jan 2020 12:18:51 -0500 Subject: [PATCH 2/9] getAccountByFederatedId -> getAccountByProviderId --- src/auth/auth.ts | 23 +++++++++++----- src/index.d.ts | 10 +++---- test/integration/auth.spec.ts | 8 +++--- test/unit/auth/auth.spec.ts | 51 ++++++++++++++++++++++++++++------- 4 files changed, 67 insertions(+), 25 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 90439326d9..f3fb9ed484 100755 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -201,20 +201,29 @@ export class BaseAuth { } /** - * Gets the user data for the user corresponding to a given federated id. + * Gets the user data for the user corresponding to a given provider id. * * See [Retrieve user data](/docs/auth/admin/manage-users#retrieve_user_data) * for code samples and detailed documentation. * - * @param federatedId The provider ID, for example, "google.com" for the + * @param providerId The provider ID, for example, "google.com" for the * Google provider. - * @param federatedUid The user identifier for the given provider. + * @param providerUid The user identifier for the given provider. * * @return A promise fulfilled with the user data corresponding to the - * provided federated id. - */ - public getUserByFederatedId(federatedId: string, federatedUid: string): Promise { - return this.authRequestHandler.getAccountInfoByFederatedId(federatedId, federatedUid) + * given provider id. + */ + public getUserByProviderId(providerId: string, providerUid: string): Promise { + // Although we don't really advertise it, we want to also handle + // non-federated idps with this call. So if we detect one of them, we'll + // reroute this request appropriately. + if (providerId === 'phone') { + return this.getUserByPhoneNumber(providerUid); + } else if (providerId === 'email') { + return this.getUserByEmail(providerUid); + } + + return this.authRequestHandler.getAccountInfoByFederatedId(providerId, providerUid) .then((response: any) => { // Returns the user record populated with server response. return new UserRecord(response.users[0]); diff --git a/src/index.d.ts b/src/index.d.ts index 82d584750d..5af0e30a00 100755 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -1531,19 +1531,19 @@ declare namespace admin.auth { getUserByPhoneNumber(phoneNumber: string): Promise; /** - * Gets the user data for the user corresponding to a given federated id. + * Gets the user data for the user corresponding to a given provider id. * * See [Retrieve user data](/docs/auth/admin/manage-users#retrieve_user_data) * for code samples and detailed documentation. * - * @param federatedId The provider ID, for example, "google.com" for the + * @param providerId The provider ID, for example, "google.com" for the * Google provider. - * @param federatedUid The user identifier for the given provider. + * @param providerUid The user identifier for the given provider. * * @return A promise fulfilled with the user data corresponding to the - * provided federated id. + * given provider id. */ - getUserByFederatedId(federatedId: string, federatedUid: string): Promise; + getUserByProviderId(providerId: string, providerUid: string): Promise; /** * Retrieves a list of users (single batch only) with a size of `maxResults` diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 85f9f0d57c..d86f9101a1 100755 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -169,7 +169,7 @@ describe('admin.auth', () => { }); }); - it('getUserByFederatedId() returns a user record with the matching federated id', async () => { + it('getUserByProviderId() returns a user record with the matching provider id', async () => { // TODO(rsgowman): Once we can link a provider id with a user, just do that // here instead of creating a new user. const importUser: admin.auth.UserImportRecord = { @@ -198,7 +198,7 @@ describe('admin.auth', () => { await admin.auth().importUsers([importUser]); try { - await admin.auth().getUserByFederatedId('google.com', 'google_uid') + await admin.auth().getUserByProviderId('google.com', 'google_uid') .then((userRecord) => { expect(userRecord.uid).to.equal(importUser.uid); }); @@ -395,8 +395,8 @@ describe('admin.auth', () => { .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); - it('getUserByFederatedId() fails when called with a non-existing federated id', () => { - return admin.auth().getUserByFederatedId('google.com', nonexistentUid) + it('getUserByProviderId() fails when called with a non-existing federated id', () => { + return admin.auth().getUserByProviderId('google.com', nonexistentUid) .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 28675b072d..dbdad66968 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1130,7 +1130,7 @@ AUTH_CONFIGS.forEach((testConfig) => { }); }); - describe('getUserByFederatedId()', () => { + describe('getUserByProviderId()', () => { const federatedId = 'google.com'; const federatedUid = 'google_uid'; const tenantId = testConfig.supportsTenantManagement ? undefined : TENANT_ID; @@ -1148,35 +1148,35 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('should be rejected given no federated id', () => { - expect(() => (auth as any).getUserByFederatedId()) + expect(() => (auth as any).getUserByProviderId()) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an invalid federated id', () => { - expect(() => auth.getUserByFederatedId('', 'uid')) + expect(() => auth.getUserByProviderId('', 'uid')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an invalid federated uid', () => { - expect(() => auth.getUserByFederatedId('id', '')) + expect(() => auth.getUserByProviderId('id', '')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an app which returns null access tokens', () => { - return nullAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + return nullAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which returns invalid access tokens', () => { - return malformedAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + return malformedAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which fails to generate access tokens', () => { - return rejectedPromiseAccessTokenAuth.getUserByFederatedId(federatedId, federatedUid) + return rejectedPromiseAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); @@ -1185,7 +1185,7 @@ AUTH_CONFIGS.forEach((testConfig) => { const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') .resolves(expectedGetAccountInfoResult); stubs.push(stub); - return auth.getUserByFederatedId(federatedId, federatedUid) + return auth.getUserByProviderId(federatedId, federatedUid) .then((userRecord) => { // Confirm underlying API called with expected parameters. expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); @@ -1194,12 +1194,45 @@ AUTH_CONFIGS.forEach((testConfig) => { }); }); + describe('non-federated providers', () => { + let invokeRequestHandlerStub: sinon.SinonStub; + beforeEach(() => { + invokeRequestHandlerStub = sinon.stub(testConfig.RequestHandler.prototype, 'invokeRequestHandler') + .resolves({ + // nothing here is checked; we just need enough to not crash. + users: [{ + localId: 1, + }], + }); + + }); + afterEach(() => { + invokeRequestHandlerStub.restore(); + }); + + it('phone lookups should use phoneNumber field', async () => { + await auth.getUserByProviderId('phone', '+15555550001'); + expect(invokeRequestHandlerStub).to.have.been.calledOnce.and.calledWith( + sinon.match.any, sinon.match.any, { + phoneNumber: ['+15555550001'], + }); + }); + + it('email lookups should use email field', async () => { + await auth.getUserByProviderId('email', 'user@example.com'); + expect(invokeRequestHandlerStub).to.have.been.calledOnce.and.calledWith( + sinon.match.any, sinon.match.any, { + email: ['user@example.com'], + }); + }); + }); + it('should throw an error when the backend returns an error', () => { // Stub getAccountInfoByFederatedId to throw a backend error. const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') .rejects(expectedError); stubs.push(stub); - return auth.getUserByFederatedId(federatedId, federatedUid) + return auth.getUserByProviderId(federatedId, federatedUid) .then((userRecord) => { throw new Error('Unexpected success'); }, (error) => { From 46091919e4a0dec06494995ebbb5bc75411621af Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 22 Jan 2020 15:43:11 -0500 Subject: [PATCH 3/9] review feedback --- src/auth/auth-api-request.ts | 2 +- src/auth/auth.ts | 4 ++-- src/index.d.ts | 2 +- test/integration/auth.spec.ts | 9 ++++----- test/unit/auth/auth.spec.ts | 28 ++++++++++++++-------------- 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index f1ee905695..a49f57f7c0 100755 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -811,7 +811,7 @@ export abstract class AbstractAuthRequestHandler { return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNT_INFO, request); } - public getAccountInfoByFederatedId(federatedId: string, federatedUid: string): Promise { + public getAccountInfoByFederatedUid(federatedId: string, federatedUid: string): Promise { if (!validator.isNonEmptyString(federatedId) || !validator.isNonEmptyString(federatedUid)) { throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_ID); } diff --git a/src/auth/auth.ts b/src/auth/auth.ts index f3fb9ed484..38da237b98 100755 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -213,7 +213,7 @@ export class BaseAuth { * @return A promise fulfilled with the user data corresponding to the * given provider id. */ - public getUserByProviderId(providerId: string, providerUid: string): Promise { + public getUserByProviderUid(providerId: string, providerUid: string): Promise { // Although we don't really advertise it, we want to also handle // non-federated idps with this call. So if we detect one of them, we'll // reroute this request appropriately. @@ -223,7 +223,7 @@ export class BaseAuth { return this.getUserByEmail(providerUid); } - return this.authRequestHandler.getAccountInfoByFederatedId(providerId, providerUid) + return this.authRequestHandler.getAccountInfoByFederatedUid(providerId, providerUid) .then((response: any) => { // Returns the user record populated with server response. return new UserRecord(response.users[0]); diff --git a/src/index.d.ts b/src/index.d.ts index 5af0e30a00..47a85b53d1 100755 --- a/src/index.d.ts +++ b/src/index.d.ts @@ -1543,7 +1543,7 @@ declare namespace admin.auth { * @return A promise fulfilled with the user data corresponding to the * given provider id. */ - getUserByProviderId(providerId: string, providerUid: string): Promise; + getUserByProviderUid(providerId: string, providerUid: string): Promise; /** * Retrieves a list of users (single batch only) with a size of `maxResults` diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index d86f9101a1..97124b26fc 100755 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -66,7 +66,6 @@ const mockUserData = { displayName: 'Random User ' + newUserUid, photoURL: 'http://www.example.com/' + newUserUid + '/photo.png', disabled: false, - }; const actionCodeSettings = { url: 'http://localhost/?a=1&b=2#c=3', @@ -169,7 +168,7 @@ describe('admin.auth', () => { }); }); - it('getUserByProviderId() returns a user record with the matching provider id', async () => { + it('getUserByProviderUid() returns a user record with the matching provider id', async () => { // TODO(rsgowman): Once we can link a provider id with a user, just do that // here instead of creating a new user. const importUser: admin.auth.UserImportRecord = { @@ -198,7 +197,7 @@ describe('admin.auth', () => { await admin.auth().importUsers([importUser]); try { - await admin.auth().getUserByProviderId('google.com', 'google_uid') + await admin.auth().getUserByProviderUid('google.com', 'google_uid') .then((userRecord) => { expect(userRecord.uid).to.equal(importUser.uid); }); @@ -395,8 +394,8 @@ describe('admin.auth', () => { .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); - it('getUserByProviderId() fails when called with a non-existing federated id', () => { - return admin.auth().getUserByProviderId('google.com', nonexistentUid) + it('getUserByProviderUid() fails when called with a non-existing federated id', () => { + return admin.auth().getUserByProviderUid('google.com', nonexistentUid) .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index dbdad66968..d8435c2cd0 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1130,7 +1130,7 @@ AUTH_CONFIGS.forEach((testConfig) => { }); }); - describe('getUserByProviderId()', () => { + describe('getUserByProviderUid()', () => { const federatedId = 'google.com'; const federatedUid = 'google_uid'; const tenantId = testConfig.supportsTenantManagement ? undefined : TENANT_ID; @@ -1148,44 +1148,44 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('should be rejected given no federated id', () => { - expect(() => (auth as any).getUserByProviderId()) + expect(() => (auth as any).getUserByProviderUid()) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an invalid federated id', () => { - expect(() => auth.getUserByProviderId('', 'uid')) + expect(() => auth.getUserByProviderUid('', 'uid')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an invalid federated uid', () => { - expect(() => auth.getUserByProviderId('id', '')) + expect(() => auth.getUserByProviderUid('id', '')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an app which returns null access tokens', () => { - return nullAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) + return nullAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which returns invalid access tokens', () => { - return malformedAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) + return malformedAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which fails to generate access tokens', () => { - return rejectedPromiseAccessTokenAuth.getUserByProviderId(federatedId, federatedUid) + return rejectedPromiseAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should resolve with a UserRecord on success', () => { // Stub getAccountInfoByEmail to return expected result. - const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') + const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedUid') .resolves(expectedGetAccountInfoResult); stubs.push(stub); - return auth.getUserByProviderId(federatedId, federatedUid) + return auth.getUserByProviderUid(federatedId, federatedUid) .then((userRecord) => { // Confirm underlying API called with expected parameters. expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); @@ -1211,7 +1211,7 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('phone lookups should use phoneNumber field', async () => { - await auth.getUserByProviderId('phone', '+15555550001'); + await auth.getUserByProviderUid('phone', '+15555550001'); expect(invokeRequestHandlerStub).to.have.been.calledOnce.and.calledWith( sinon.match.any, sinon.match.any, { phoneNumber: ['+15555550001'], @@ -1219,7 +1219,7 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('email lookups should use email field', async () => { - await auth.getUserByProviderId('email', 'user@example.com'); + await auth.getUserByProviderUid('email', 'user@example.com'); expect(invokeRequestHandlerStub).to.have.been.calledOnce.and.calledWith( sinon.match.any, sinon.match.any, { email: ['user@example.com'], @@ -1228,11 +1228,11 @@ AUTH_CONFIGS.forEach((testConfig) => { }); it('should throw an error when the backend returns an error', () => { - // Stub getAccountInfoByFederatedId to throw a backend error. - const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedId') + // Stub getAccountInfoByFederatedUid to throw a backend error. + const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedUid') .rejects(expectedError); stubs.push(stub); - return auth.getUserByProviderId(federatedId, federatedUid) + return auth.getUserByProviderUid(federatedId, federatedUid) .then((userRecord) => { throw new Error('Unexpected success'); }, (error) => { From a59ef605e1ca7dd46a54f785fb8a0f750fe526a3 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 17:51:54 -0500 Subject: [PATCH 4/9] review feedback 2 --- src/auth/auth-api-request.ts | 8 ++++---- test/integration/auth.spec.ts | 4 ++-- test/unit/auth/auth.spec.ts | 24 ++++++++++++------------ 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/auth/auth-api-request.ts b/src/auth/auth-api-request.ts index a49f57f7c0..39da7ecb14 100755 --- a/src/auth/auth-api-request.ts +++ b/src/auth/auth-api-request.ts @@ -811,15 +811,15 @@ export abstract class AbstractAuthRequestHandler { return this.invokeRequestHandler(this.getAuthUrlBuilder(), FIREBASE_AUTH_GET_ACCOUNT_INFO, request); } - public getAccountInfoByFederatedUid(federatedId: string, federatedUid: string): Promise { - if (!validator.isNonEmptyString(federatedId) || !validator.isNonEmptyString(federatedUid)) { + public getAccountInfoByFederatedUid(providerId: string, rawId: string): Promise { + if (!validator.isNonEmptyString(providerId) || !validator.isNonEmptyString(rawId)) { throw new FirebaseAuthError(AuthClientErrorCode.INVALID_PROVIDER_ID); } const request = { federatedUserId: [{ - providerId: federatedId, - rawId: federatedUid, + providerId, + rawId, }], }; diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 97124b26fc..ae5fe05272 100755 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -171,8 +171,9 @@ describe('admin.auth', () => { it('getUserByProviderUid() returns a user record with the matching provider id', async () => { // TODO(rsgowman): Once we can link a provider id with a user, just do that // here instead of creating a new user. + const randomUid = 'import_' + generateRandomString(20).toLowerCase(); const importUser: admin.auth.UserImportRecord = { - uid: 'uid', + uid: randomUid, email: 'user@example.com', phoneNumber: '+15555550000', emailVerified: true, @@ -193,7 +194,6 @@ describe('admin.auth', () => { }], }; - await safeDelete(importUser.uid); await admin.auth().importUsers([importUser]); try { diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index d8435c2cd0..0610c0610a 100755 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1131,8 +1131,8 @@ AUTH_CONFIGS.forEach((testConfig) => { }); describe('getUserByProviderUid()', () => { - const federatedId = 'google.com'; - const federatedUid = 'google_uid'; + const providerId = 'google.com'; + const providerUid = 'google_uid'; const tenantId = testConfig.supportsTenantManagement ? undefined : TENANT_ID; const expectedGetAccountInfoResult = getValidGetAccountInfoResponse(tenantId); const expectedUserRecord = getValidUserRecord(expectedGetAccountInfoResult); @@ -1147,36 +1147,36 @@ AUTH_CONFIGS.forEach((testConfig) => { stubs = []; }); - it('should be rejected given no federated id', () => { + it('should be rejected given no provider id', () => { expect(() => (auth as any).getUserByProviderUid()) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); - it('should be rejected given an invalid federated id', () => { + it('should be rejected given an invalid provider id', () => { expect(() => auth.getUserByProviderUid('', 'uid')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); - it('should be rejected given an invalid federated uid', () => { + it('should be rejected given an invalid provider uid', () => { expect(() => auth.getUserByProviderUid('id', '')) .to.throw(FirebaseAuthError) .with.property('code', 'auth/invalid-provider-id'); }); it('should be rejected given an app which returns null access tokens', () => { - return nullAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) + return nullAccessTokenAuth.getUserByProviderUid(providerId, providerUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which returns invalid access tokens', () => { - return malformedAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) + return malformedAccessTokenAuth.getUserByProviderUid(providerId, providerUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); it('should be rejected given an app which fails to generate access tokens', () => { - return rejectedPromiseAccessTokenAuth.getUserByProviderUid(federatedId, federatedUid) + return rejectedPromiseAccessTokenAuth.getUserByProviderUid(providerId, providerUid) .should.eventually.be.rejected.and.have.property('code', 'app/invalid-credential'); }); @@ -1185,10 +1185,10 @@ AUTH_CONFIGS.forEach((testConfig) => { const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedUid') .resolves(expectedGetAccountInfoResult); stubs.push(stub); - return auth.getUserByProviderUid(federatedId, federatedUid) + return auth.getUserByProviderUid(providerId, providerUid) .then((userRecord) => { // Confirm underlying API called with expected parameters. - expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); + expect(stub).to.have.been.calledOnce.and.calledWith(providerId, providerUid); // Confirm expected user record response returned. expect(userRecord).to.deep.equal(expectedUserRecord); }); @@ -1232,12 +1232,12 @@ AUTH_CONFIGS.forEach((testConfig) => { const stub = sinon.stub(testConfig.RequestHandler.prototype, 'getAccountInfoByFederatedUid') .rejects(expectedError); stubs.push(stub); - return auth.getUserByProviderUid(federatedId, federatedUid) + return auth.getUserByProviderUid(providerId, providerUid) .then((userRecord) => { throw new Error('Unexpected success'); }, (error) => { // Confirm underlying API called with expected parameters. - expect(stub).to.have.been.calledOnce.and.calledWith(federatedId, federatedUid); + expect(stub).to.have.been.calledOnce.and.calledWith(providerId, providerUid); // Confirm expected error returned. expect(error).to.equal(expectedError); }); From e9197f94e49b522b679e910c4254473e04e37276 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Wed, 5 Feb 2020 18:03:55 -0500 Subject: [PATCH 5/9] review feedback 3 --- test/unit/auth/auth-api-request.spec.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index be4aa83a6c..01f31a376f 100755 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -338,6 +338,12 @@ describe('FIREBASE_AUTH_GET_ACCOUNT_INFO', () => { return requestValidator(validRequest); }).not.to.throw(); }); + it('should succeed with federatedUserId passed', () => { + const validRequest = {federatedUserId: {providerId: 'google.com', rawId: 'google_uid_1234'}}; + expect(() => { + return requestValidator(validRequest); + }).not.to.throw(); + }); it('should fail when neither localId, email or phoneNumber are passed', () => { const invalidRequest = {bla: ['1234']}; expect(() => { From 40e00601ce8e370daca4ef5a909e03c6ebfd63cb Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 21 Dec 2020 16:05:40 -0500 Subject: [PATCH 6/9] review nits --- src/auth/auth.ts | 10 +++++----- src/auth/index.ts | 6 +++--- test/integration/auth.spec.ts | 2 +- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/auth/auth.ts b/src/auth/auth.ts index 1c09fbed59..00782452a0 100644 --- a/src/auth/auth.ts +++ b/src/auth/auth.ts @@ -197,22 +197,22 @@ export class BaseAuth implements BaseAuthI * * @param providerId The provider ID, for example, "google.com" for the * Google provider. - * @param providerUid The user identifier for the given provider. + * @param uid The user identifier for the given provider. * * @return A promise fulfilled with the user data corresponding to the * given provider id. */ - public getUserByProviderUid(providerId: string, providerUid: string): Promise { + public getUserByProviderUid(providerId: string, uid: string): Promise { // Although we don't really advertise it, we want to also handle // non-federated idps with this call. So if we detect one of them, we'll // reroute this request appropriately. if (providerId === 'phone') { - return this.getUserByPhoneNumber(providerUid); + return this.getUserByPhoneNumber(uid); } else if (providerId === 'email') { - return this.getUserByEmail(providerUid); + return this.getUserByEmail(uid); } - return this.authRequestHandler.getAccountInfoByFederatedUid(providerId, providerUid) + return this.authRequestHandler.getAccountInfoByFederatedUid(providerId, uid) .then((response: any) => { // Returns the user record populated with server response. return new UserRecord(response.users[0]); diff --git a/src/auth/index.ts b/src/auth/index.ts index 16f6f228c8..a1ff346b97 100644 --- a/src/auth/index.ts +++ b/src/auth/index.ts @@ -1517,19 +1517,19 @@ export namespace auth { getUserByPhoneNumber(phoneNumber: string): Promise; /** - * Gets the user data for the user corresponding to a given provider id. + * Gets the user data for the user corresponding to a given provider ID. * * See [Retrieve user data](/docs/auth/admin/manage-users#retrieve_user_data) * for code samples and detailed documentation. * * @param providerId The provider ID, for example, "google.com" for the * Google provider. - * @param providerUid The user identifier for the given provider. + * @param uid The user identifier for the given provider. * * @return A promise fulfilled with the user data corresponding to the * given provider id. */ - getUserByProviderUid(providerId: string, providerUid: string): Promise; + getUserByProviderUid(providerId: string, uid: string): Promise; /** * Gets the user data corresponding to the specified identifiers. diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 72df48bf1a..62e35ccefa 100644 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -635,7 +635,7 @@ describe('admin.auth', () => { .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); - it('getUserByProviderUid() fails when called with a non-existing federated id', () => { + it('getUserByProviderUid() fails when called with a non-existing provider id', () => { return admin.auth().getUserByProviderUid('google.com', nonexistentUid) .should.eventually.be.rejected.and.have.property('code', 'auth/user-not-found'); }); From 1cb117815a6025d19279c5cd54348e18275acf46 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Tue, 22 Dec 2020 10:15:20 -0500 Subject: [PATCH 7/9] lint --- test/unit/auth/auth-api-request.spec.ts | 2 +- test/unit/auth/auth.spec.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/unit/auth/auth-api-request.spec.ts b/test/unit/auth/auth-api-request.spec.ts index 3577b6d33d..2df01889dd 100644 --- a/test/unit/auth/auth-api-request.spec.ts +++ b/test/unit/auth/auth-api-request.spec.ts @@ -361,7 +361,7 @@ describe('FIREBASE_AUTH_GET_ACCOUNT_INFO', () => { }).not.to.throw(); }); it('should succeed with federatedUserId passed', () => { - const validRequest = {federatedUserId: {providerId: 'google.com', rawId: 'google_uid_1234'}}; + const validRequest = { federatedUserId: { providerId: 'google.com', rawId: 'google_uid_1234' } }; expect(() => { return requestValidator(validRequest); }).not.to.throw(); diff --git a/test/unit/auth/auth.spec.ts b/test/unit/auth/auth.spec.ts index 281cb7eff1..194f64fd8b 100644 --- a/test/unit/auth/auth.spec.ts +++ b/test/unit/auth/auth.spec.ts @@ -1254,7 +1254,7 @@ AUTH_CONFIGS.forEach((testConfig) => { .rejects(expectedError); stubs.push(stub); return auth.getUserByProviderUid(providerId, providerUid) - .then((userRecord) => { + .then(() => { throw new Error('Unexpected success'); }, (error) => { // Confirm underlying API called with expected parameters. From f6c19a983fad49fb0811233c7fb26e1332414806 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Tue, 22 Dec 2020 11:41:08 -0500 Subject: [PATCH 8/9] api-extractor --- etc/firebase-admin.api.md | 1 + 1 file changed, 1 insertion(+) diff --git a/etc/firebase-admin.api.md b/etc/firebase-admin.api.md index b5b810a5e6..3941798ebd 100644 --- a/etc/firebase-admin.api.md +++ b/etc/firebase-admin.api.md @@ -106,6 +106,7 @@ export namespace auth { getUser(uid: string): Promise; getUserByEmail(email: string): Promise; getUserByPhoneNumber(phoneNumber: string): Promise; + getUserByProviderUid(providerId: string, uid: string): Promise; getUsers(identifiers: UserIdentifier[]): Promise; importUsers(users: UserImportRecord[], options?: UserImportOptions): Promise; listProviderConfigs(options: AuthProviderConfigFilter): Promise; From 655b20fec28a0980df2fac2c863bfa268923fe59 Mon Sep 17 00:00:00 2001 From: Rich Gowman Date: Mon, 8 Feb 2021 14:59:41 -0500 Subject: [PATCH 9/9] Add a few extra tests --- test/integration/auth.spec.ts | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/integration/auth.spec.ts b/test/integration/auth.spec.ts index 2cf23a5f6d..1eb7bfe6a8 100644 --- a/test/integration/auth.spec.ts +++ b/test/integration/auth.spec.ts @@ -257,6 +257,20 @@ describe('admin.auth', () => { } }); + it('getUserByProviderUid() redirects to getUserByEmail if given an email', () => { + return admin.auth().getUserByProviderUid('email', mockUserData.email) + .then((userRecord) => { + expect(userRecord.uid).to.equal(newUserUid); + }); + }); + + it('getUserByProviderUid() redirects to getUserByPhoneNumber if given a phone number', () => { + return admin.auth().getUserByProviderUid('phone', mockUserData.phoneNumber) + .then((userRecord) => { + expect(userRecord.uid).to.equal(newUserUid); + }); + }); + describe('getUsers()', () => { /** * Filters a list of object to another list of objects that only contains