From 1407e9c1c245a96cbe6e236990f95d0312af99f9 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 11:49:53 -0700 Subject: [PATCH 1/7] Add unlink(), linkWithCredential(), linkWithPhoneNumber() --- .../src/core/credentials/phone.test.ts | 42 ++++++ .../auth-exp/src/core/credentials/phone.ts | 10 +- .../src/core/strategies/credential.test.ts | 118 +++++++++++++---- .../src/core/strategies/credential.ts | 56 +++++++- .../src/core/strategies/phone.test.ts | 86 +++++++++++- .../auth-exp/src/core/strategies/phone.ts | 30 ++++- .../auth-exp/src/core/user/account_info.ts | 18 +-- .../auth-exp/src/core/user/unlink.test.ts | 125 ++++++++++++++++++ packages-exp/auth-exp/src/core/user/unlink.ts | 46 +++++++ .../auth-exp/src/core/user/user_impl.ts | 17 ++- packages-exp/auth-exp/src/model/user.d.ts | 2 +- .../auth-exp/test/mock_auth_credential.ts | 5 +- 12 files changed, 497 insertions(+), 58 deletions(-) create mode 100644 packages-exp/auth-exp/src/core/user/unlink.test.ts create mode 100644 packages-exp/auth-exp/src/core/user/unlink.ts diff --git a/packages-exp/auth-exp/src/core/credentials/phone.test.ts b/packages-exp/auth-exp/src/core/credentials/phone.test.ts index f0859ee43a7..873d407140c 100644 --- a/packages-exp/auth-exp/src/core/credentials/phone.test.ts +++ b/packages-exp/auth-exp/src/core/credentials/phone.test.ts @@ -77,6 +77,48 @@ describe('core/credentials/phone', () => { }); }); + context('#_linkToIdToken', () => { + const response: IdTokenResponse = { + idToken: '', + refreshToken: '', + kind: '', + expiresIn: '10', + localId: 'uid', + }; + + it('calls the endpoint with session and code', async () => { + const cred = new PhoneAuthCredential({ + verificationId: 'session-info', + verificationCode: 'code' + }); + + const route = mockEndpoint(Endpoint.SIGN_IN_WITH_PHONE_NUMBER, response); + + expect(await cred._linkToIdToken(auth, 'id-token')).to.eql(response); + expect(route.calls[0].request).to.eql({ + sessionInfo: 'session-info', + code: 'code', + idToken: 'id-token', + }); + }); + + it('calls the endpoint with proof and number', async () => { + const cred = new PhoneAuthCredential({ + temporaryProof: 'temp-proof', + phoneNumber: 'number' + }); + + const route = mockEndpoint(Endpoint.SIGN_IN_WITH_PHONE_NUMBER, response); + + expect(await cred._linkToIdToken(auth, 'id-token')).to.eql(response); + expect(route.calls[0].request).to.eql({ + temporaryProof: 'temp-proof', + phoneNumber: 'number', + idToken: 'id-token', + }); + }); + }); + context('#toJSON', () => { it('fills out the object with everything that is set', () => { const cred = new PhoneAuthCredential({ diff --git a/packages-exp/auth-exp/src/core/credentials/phone.ts b/packages-exp/auth-exp/src/core/credentials/phone.ts index 36c7f49979d..80aca34c828 100644 --- a/packages-exp/auth-exp/src/core/credentials/phone.ts +++ b/packages-exp/auth-exp/src/core/credentials/phone.ts @@ -19,8 +19,7 @@ import * as externs from '@firebase/auth-types-exp'; import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa'; import { - signInWithPhoneNumber, - SignInWithPhoneNumberRequest + linkWithPhoneNumber, signInWithPhoneNumber, SignInWithPhoneNumberRequest } from '../../api/authentication/sms'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; @@ -46,9 +45,10 @@ export class PhoneAuthCredential } _linkToIdToken(auth: Auth, idToken: string): Promise { - void auth; - void idToken; - return debugFail('not implemented'); + return linkWithPhoneNumber(auth, { + idToken, + ...this.makeVerificationRequest(), + }); } _matchIdTokenWithUid(auth: Auth, uid: string): Promise { diff --git a/packages-exp/auth-exp/src/core/strategies/credential.test.ts b/packages-exp/auth-exp/src/core/strategies/credential.test.ts index 79b908a96e5..5982afff0d8 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.test.ts @@ -18,25 +18,23 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { - OperationType, - ProviderId, - SignInMethod -} from '@firebase/auth-types-exp'; +import { OperationType, ProviderId, SignInMethod } from '@firebase/auth-types-exp'; +import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; -import { testAuth } from '../../../test/mock_auth'; +import { testAuth, testUser } from '../../../test/mock_auth'; import { MockAuthCredential } from '../../../test/mock_auth_credential'; import * as mockFetch from '../../../test/mock_fetch'; import { Endpoint } from '../../api'; import { APIUserInfo } from '../../api/account_management/account'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; -import { signInWithCredential } from './credential'; +import { User } from '../../model/user'; +import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; use(chaiAsPromised); -describe('core/strategies/signInWithCredential', () => { +describe('core/strategies/credential', () => { const serverUser: APIUserInfo = { localId: 'local-id', displayName: 'display-name', @@ -63,32 +61,106 @@ describe('core/strategies/signInWithCredential', () => { ); let auth: Auth; + let getAccountInfoEndpoint: mockFetch.Route; + let user: User; beforeEach(async () => { auth = await testAuth(); mockFetch.setUp(); authCredential._setIdTokenResponse(idTokenResponse); - mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + getAccountInfoEndpoint = mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { users: [serverUser] }); + + user = testUser(auth, 'uid', undefined, true); }); + afterEach(mockFetch.tearDown); - it('should return a valid user credential', async () => { - const { credential, user, operationType } = await signInWithCredential( - auth, - authCredential - ); - expect(credential!.providerId).to.eq(ProviderId.FIREBASE); - expect(credential!.signInMethod).to.eq(SignInMethod.EMAIL_LINK); - expect(user.uid).to.eq('local-id'); - expect(user.tenantId).to.eq('tenant-id'); - expect(user.displayName).to.eq('display-name'); - expect(operationType).to.eq(OperationType.SIGN_IN); + describe('signInWithCredential', () => { + it('should return a valid user credential', async () => { + const { credential, user, operationType } = await signInWithCredential( + auth, + authCredential + ); + expect(credential!.providerId).to.eq(ProviderId.FIREBASE); + expect(credential!.signInMethod).to.eq(SignInMethod.EMAIL_LINK); + expect(user.uid).to.eq('local-id'); + expect(user.tenantId).to.eq('tenant-id'); + expect(user.displayName).to.eq('display-name'); + expect(operationType).to.eq(OperationType.SIGN_IN); + }); + + it('should update the current user', async () => { + const { user } = await signInWithCredential(auth, authCredential); + expect(auth.currentUser).to.eq(user); + }); }); - it('should update the current user', async () => { - const { user } = await signInWithCredential(auth, authCredential); - expect(auth.currentUser).to.eq(user); + describe('linkWithCredential', () => { + it('should throw an error if the provider is already linked', async () => { + getAccountInfoEndpoint.response = { + users: [ + { + ...serverUser, + providerUserInfo: [ + {providerId: ProviderId.FIREBASE} + ] + } + ] + }; + + await expect(linkWithCredential(user, authCredential)).to.be.rejectedWith( + FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', + ); + }); + + it('should return a valid user credential', async () => { + const { credential, user: newUser, operationType } = await linkWithCredential(user, authCredential); + expect(operationType).to.eq(OperationType.LINK); + expect(newUser).to.eq(user); + expect(credential).to.be.null; + }); + }); + + describe('_assertLinkedStatus', () => { + it('should error with already linked if expectation is true', async () => { + getAccountInfoEndpoint.response = { + users: [ + { + ...serverUser, + providerUserInfo: [ + {providerId: ProviderId.PHONE} + ] + } + ] + }; + + await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).to.be.rejectedWith( + FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', + ); + }); + + it('should not error if provider is not linked', async () => { + await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).not.to.be.rejected; + }); + + it('should error if provider is not linked but it was expected to be', async () => { + await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).to.be.rejectedWith(FirebaseError, 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).'); + }); + + it('should not error if provider is linked and that is expected', async () => { + getAccountInfoEndpoint.response = { + users: [ + { + ...serverUser, + providerUserInfo: [ + {providerId: ProviderId.PHONE} + ] + } + ] + }; + await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).not.to.be.rejected; + }); }); }); diff --git a/packages-exp/auth-exp/src/core/strategies/credential.ts b/packages-exp/auth-exp/src/core/strategies/credential.ts index ff534453ab5..229d6e65225 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.ts @@ -16,11 +16,18 @@ */ import * as externs from '@firebase/auth-types-exp'; - import { OperationType, UserCredential } from '@firebase/auth-types-exp'; + +import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa'; +import { SignInWithPhoneNumberResponse } from '../../api/authentication/sms'; import { Auth } from '../../model/auth'; +import { User } from '../../model/user'; import { AuthCredential } from '../credentials'; +import { PhoneAuthCredential } from '../credentials/phone'; +import { AuthErrorCode } from '../errors'; +import { _reloadWithoutSaving } from '../user/reload'; import { UserCredentialImpl } from '../user/user_credential_impl'; +import { fail } from '../util/assert'; export async function signInWithCredential( authExtern: externs.Auth, @@ -39,3 +46,50 @@ export async function signInWithCredential( await auth.updateCurrentUser(userCredential.user); return userCredential; } + +export async function linkWithCredential( + userExtern: externs.User, + credentialExtern: externs.AuthCredential +): Promise { + const user = userExtern as User; + const credential = credentialExtern as AuthCredential; + await _assertLinkedStatus(false, user, credential.providerId); + + const response = await credential._linkToIdToken(user.auth, await user.getIdToken()); + + const newCred = _authCredentialFromTokenResponse(response); + await user._updateTokensIfNecessary(response, /* reload */ true); + return new UserCredentialImpl(user, newCred, OperationType.LINK); +} + +export function _authCredentialFromTokenResponse( + response: PhoneOrOauthTokenResponse +): AuthCredential | null { + const { + temporaryProof, + phoneNumber + } = response as SignInWithPhoneNumberResponse; + if (temporaryProof && phoneNumber) { + return new PhoneAuthCredential({temporaryProof, phoneNumber}); + } + + // TODO: Handle Oauth cases + return null; +} + +export async function _assertLinkedStatus( + expected: boolean, + user: User, + provider: externs.ProviderId, +): Promise { + await _reloadWithoutSaving(user); + const providerIds = user.providerData.map(({ providerId }) => providerId); + if (providerIds.includes(provider) !== expected) { + const code = + expected === false + ? AuthErrorCode.PROVIDER_ALREADY_LINKED + : AuthErrorCode.NO_SUCH_PROVIDER; + fail(user.auth.name, code); + } +} + diff --git a/packages-exp/auth-exp/src/core/strategies/phone.test.ts b/packages-exp/auth-exp/src/core/strategies/phone.test.ts index 5410f2b7705..deac4354956 100644 --- a/packages-exp/auth-exp/src/core/strategies/phone.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/phone.test.ts @@ -20,17 +20,18 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import * as sinonChai from 'sinon-chai'; -import { ApplicationVerifier, OperationType } from '@firebase/auth-types-exp'; +import { ApplicationVerifier, OperationType, ProviderId } from '@firebase/auth-types-exp'; import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; -import { testAuth } from '../../../test/mock_auth'; +import { testAuth, testUser } from '../../../test/mock_auth'; import * as fetch from '../../../test/mock_fetch'; import { Endpoint } from '../../api'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; +import { User } from '../../model/user'; import { RecaptchaVerifier } from '../../platform_browser/recaptcha/recaptcha_verifier'; -import { _verifyPhoneNumber, signInWithPhoneNumber } from './phone'; +import { _verifyPhoneNumber, linkWithPhoneNumber, signInWithPhoneNumber } from './phone'; use(chaiAsPromised); use(sinonChai); @@ -104,6 +105,85 @@ describe('core/strategies/phone', () => { }); }); + describe('linkWithPhoneNumber', () => { + let getAccountInfoEndpoint: fetch.Route; + let user: User; + + beforeEach(() => { + getAccountInfoEndpoint = mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [{uid: 'uid'}] + }); + + user = testUser(auth, 'uid', 'email', true); + }); + + it('rejects if a phone provider is already linked', async () => { + getAccountInfoEndpoint.response = { + users: [ + { + uid: 'uid', + providerUserInfo: [ + {providerId: ProviderId.PHONE} + ] + } + ] + }; + + await expect(linkWithPhoneNumber(user, 'number', verifier)) + .to.be.rejectedWith( + FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', + ); + }); + + it('calls verify phone number', async () => { + await linkWithPhoneNumber(user, '+15105550000', verifier); + + expect(sendCodeEndpoint.calls[0].request).to.eql({ + recaptchaToken: 'recaptcha-token', + phoneNumber: '+15105550000' + }); + }); + + context('ConfirmationResult', () => { + it('result contains verification id baked in', async () => { + const result = await linkWithPhoneNumber(user, 'number', verifier); + expect(result.verificationId).to.eq('session-info'); + }); + + it('calling #confirm finishes the sign in flow', async () => { + const idTokenResponse: IdTokenResponse = { + idToken: 'my-id-token', + refreshToken: 'my-refresh-token', + expiresIn: '1234', + localId: 'uid', + kind: 'my-kind' + }; + + // This endpoint is called from within the callback, in + // signInWithCredential + const signInEndpoint = mockEndpoint( + Endpoint.SIGN_IN_WITH_PHONE_NUMBER, + idTokenResponse + ); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [{ localId: 'uid' }] + }); + + const initialIdToken = await user.getIdToken(); + + const result = await linkWithPhoneNumber(user, 'number', verifier); + const userCred = await result.confirm('6789'); + expect(userCred.user.uid).to.eq('uid'); + expect(userCred.operationType).to.eq(OperationType.LINK); + expect(signInEndpoint.calls[0].request).to.eql({ + sessionInfo: 'session-info', + code: '6789', + idToken: initialIdToken, + }); + }); + }); + }); + describe('_verifyPhoneNumber', () => { it('works with a string phone number', async () => { await _verifyPhoneNumber(auth, 'number', verifier); diff --git a/packages-exp/auth-exp/src/core/strategies/phone.ts b/packages-exp/auth-exp/src/core/strategies/phone.ts index 30a82e676d6..39822fc4779 100644 --- a/packages-exp/auth-exp/src/core/strategies/phone.ts +++ b/packages-exp/auth-exp/src/core/strategies/phone.ts @@ -19,12 +19,17 @@ import * as externs from '@firebase/auth-types-exp'; import { sendPhoneVerificationCode } from '../../api/authentication/sms'; import { Auth } from '../../model/auth'; +import { User } from '../../model/user'; import { RECAPTCHA_VERIFIER_TYPE } from '../../platform_browser/recaptcha/recaptcha_verifier'; import { AuthErrorCode } from '../errors'; -import { PhoneAuthProvider } from '../providers/phone'; import { assert } from '../util/assert'; +<<<<<<< HEAD import { signInWithCredential } from './credential'; import { PhoneAuthCredential } from '../credentials/phone'; +======= +import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; +import { PhoneAuthCredential } from './phone_credential'; +>>>>>>> ffdb4d73... Add unlink(), linkWithCredential(), linkWithPhoneNumber() interface OnConfirmationCallback { (credential: PhoneAuthCredential): Promise; @@ -37,10 +42,10 @@ class ConfirmationResult implements externs.ConfirmationResult { ) {} confirm(verificationCode: string): Promise { - const authCredential = PhoneAuthProvider.credential( - this.verificationId, + const authCredential = new PhoneAuthCredential({ + verificationId: this.verificationId, verificationCode - ); + }); return this.onConfirmation(authCredential); } } @@ -60,6 +65,23 @@ export async function signInWithPhoneNumber( ); } +export async function linkWithPhoneNumber( + userExtern: externs.User, + phoneNumber: string, + appVerifier: externs.ApplicationVerifier +): Promise { + const user = userExtern as User; + await _assertLinkedStatus(false, user, externs.ProviderId.PHONE); + const verificationId = await _verifyPhoneNumber( + user.auth, + phoneNumber, + appVerifier + ); + return new ConfirmationResult(verificationId, cred => + linkWithCredential(user, cred) + ); +} + /** * Returns a verification ID to be used in conjunction with the SMS code that * is sent. diff --git a/packages-exp/auth-exp/src/core/user/account_info.ts b/packages-exp/auth-exp/src/core/user/account_info.ts index 5c7d01c93c8..50716fe0107 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.ts @@ -18,8 +18,7 @@ import * as externs from '@firebase/auth-types-exp'; import { - updateEmailPassword as apiUpdateEmailPassword, - UpdateEmailPasswordRequest + updateEmailPassword as apiUpdateEmailPassword, UpdateEmailPasswordRequest } from '../../api/account_management/email_and_password'; import { updateProfile as apiUpdateProfile } from '../../api/account_management/profile'; import { User } from '../../model/user'; @@ -39,7 +38,6 @@ export async function updateProfile( } const user = externUser as User; - const { auth } = user; const idToken = await user.getIdToken(); const profileRequest = { idToken, displayName, photoUrl }; const response = await apiUpdateProfile(user.auth, profileRequest); @@ -56,11 +54,7 @@ export async function updateProfile( passwordProvider.photoURL = user.photoURL; } - const tokensRefreshed = user._updateTokensIfNecessary(response); - await auth._persistUserIfCurrent(user); - if (tokensRefreshed) { - auth._notifyListenersIfCurrent(user); - } + await user._updateTokensIfNecessary(response); } export function updateEmail( @@ -97,11 +91,5 @@ async function updateEmailOrPassword( } const response = await apiUpdateEmailPassword(auth, request); - - const tokensRefreshed = user._updateTokensIfNecessary(response); - await _reloadWithoutSaving(user); - await auth._persistUserIfCurrent(user); - if (tokensRefreshed) { - auth._notifyListenersIfCurrent(user); - } + await user._updateTokensIfNecessary(response, /* reload */ true); } diff --git a/packages-exp/auth-exp/src/core/user/unlink.test.ts b/packages-exp/auth-exp/src/core/user/unlink.test.ts new file mode 100644 index 00000000000..02832d6753c --- /dev/null +++ b/packages-exp/auth-exp/src/core/user/unlink.test.ts @@ -0,0 +1,125 @@ +/** + * @license + * Copyright 2020 Google LLC + * + * 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. + */ + +import { expect, use } from 'chai'; +import * as chaiAsPromised from 'chai-as-promised'; + +import { ProviderId } from '@firebase/auth-types-exp'; +import { FirebaseError } from '@firebase/util'; + +import { mockEndpoint } from '../../../test/api/helper'; +import { testAuth, TestAuth, testUser } from '../../../test/mock_auth'; +import * as fetch from '../../../test/mock_fetch'; +import { Endpoint } from '../../api'; +import { User } from '../../model/user'; +import { unlink } from './unlink'; + +use(chaiAsPromised); + +describe('core/user/unlink', () => { + let user: User; + let auth: TestAuth; + + beforeEach(async () => { + auth = await testAuth(); + user = testUser(auth, 'uid', '', true); + await auth.updateCurrentUser(user); + fetch.setUp(); + }); + + afterEach(() => { + fetch.tearDown(); + }); + + it('rejects if the provider is not linked', async() => { + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [ + { + uid: 'uid', + } + ] + }); + + await expect(unlink(user, ProviderId.PHONE)).to.be.rejectedWith(FirebaseError, 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).'); + }); + + context('with properly linked account', () => { + let endpoint: fetch.Route; + beforeEach(() => { + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [ + { + uid: 'uid', + providerUserInfo: [ + {providerId: ProviderId.PHONE} + ] + } + ] + }); + + endpoint = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + providerUserInfo: [{ + providerId: ProviderId.GOOGLE, + }] + }); + }); + + it('removes the provider from the list and persists', async () => { + user.providerData = [ + { + providerId: ProviderId.PHONE, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '', + }, + { + providerId: ProviderId.GOOGLE, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '', + } + ]; + await unlink(user, ProviderId.PHONE); + expect(user.providerData).to.eql([ + { + providerId: ProviderId.GOOGLE, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '', + } + ]); + + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject(), + ); + }); + + it('calls the endpoint with the provider', async () => { + await unlink(user, ProviderId.PHONE); + expect(endpoint.calls[0].request).to.eql({ + idToken: await user.getIdToken(), + deleteProvider: [ProviderId.PHONE], + }); + }); + }); +}); \ No newline at end of file diff --git a/packages-exp/auth-exp/src/core/user/unlink.ts b/packages-exp/auth-exp/src/core/user/unlink.ts new file mode 100644 index 00000000000..a81bfea1f24 --- /dev/null +++ b/packages-exp/auth-exp/src/core/user/unlink.ts @@ -0,0 +1,46 @@ +/** + * @license + * 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. + */ + +import * as externs from '@firebase/auth-types-exp'; + +import { deleteLinkedAccounts } from '../../api/account_management/account'; +import { User } from '../../model/user'; +import { _assertLinkedStatus } from '../strategies/credential'; + +export async function unlink( + userExtern: externs.User, + providerId: externs.ProviderId +): Promise { + const user = userExtern as User; + await _assertLinkedStatus(true, user, providerId); + const { providerUserInfo } = await deleteLinkedAccounts(user.auth, { + idToken: await user.getIdToken(), + deleteProvider: [providerId] + }); + + const providersLeft = (providerUserInfo || []).map(i => i.providerId); + + user.providerData = user.providerData.filter(pd => + providersLeft.includes(pd.providerId || undefined) + ); + if (!providersLeft.includes(externs.ProviderId.PHONE)) { + user.phoneNumber = null; + } + + await user.auth._persistUserIfCurrent(user); + return user; +} diff --git a/packages-exp/auth-exp/src/core/user/user_impl.ts b/packages-exp/auth-exp/src/core/user/user_impl.ts index 1158a830614..75eeac07623 100644 --- a/packages-exp/auth-exp/src/core/user/user_impl.ts +++ b/packages-exp/auth-exp/src/core/user/user_impl.ts @@ -16,6 +16,7 @@ */ import { IdTokenResult, ProviderId } from '@firebase/auth-types-exp'; + import { deleteAccount } from '../../api/account_management/account'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; @@ -23,7 +24,7 @@ import { User } from '../../model/user'; import { PersistedBlob } from '../persistence'; import { assert } from '../util/assert'; import { getIdTokenResult } from './id_token_result'; -import { reload, _reloadWithoutSaving } from './reload'; +import { _reloadWithoutSaving, reload } from './reload'; import { StsTokenManager } from './token_manager'; export interface UserParameters { @@ -100,16 +101,24 @@ export class UserImpl implements User { return reload(this); } - _updateTokensIfNecessary(response: IdTokenResponse): boolean { + async _updateTokensIfNecessary(response: IdTokenResponse, reload = false): Promise { + let tokensRefreshed = false; if ( response.idToken && response.idToken !== this.stsTokenManager.accessToken ) { this.stsTokenManager.updateFromServerResponse(response); - return true; + tokensRefreshed = true; + } + + if (reload) { + await _reloadWithoutSaving(this); } - return false; + await this.auth._persistUserIfCurrent(this); + if (tokensRefreshed) { + this.auth._notifyListenersIfCurrent(this); + } } async delete(): Promise { diff --git a/packages-exp/auth-exp/src/model/user.d.ts b/packages-exp/auth-exp/src/model/user.d.ts index f10af3c575a..fed8c287d96 100644 --- a/packages-exp/auth-exp/src/model/user.d.ts +++ b/packages-exp/auth-exp/src/model/user.d.ts @@ -40,7 +40,7 @@ export interface User extends externs.User { providerData: MutableUserInfo[]; metadata: externs.UserMetadata; - _updateTokensIfNecessary(response: IdTokenResponse): boolean; + _updateTokensIfNecessary(response: IdTokenResponse, reload?: boolean): Promise; getIdToken(forceRefresh?: boolean): Promise; getIdTokenResult(forceRefresh?: boolean): Promise; diff --git a/packages-exp/auth-exp/test/mock_auth_credential.ts b/packages-exp/auth-exp/test/mock_auth_credential.ts index c0bf5d8c44c..8341460c407 100644 --- a/packages-exp/auth-exp/test/mock_auth_credential.ts +++ b/packages-exp/auth-exp/test/mock_auth_credential.ts @@ -16,6 +16,7 @@ */ import { ProviderId, SignInMethod } from '@firebase/auth-types-exp'; + import { PhoneOrOauthTokenResponse } from '../src/api/authentication/mfa'; import { Auth } from '../src/model/auth'; import { IdTokenResponse } from '../src/model/id_token'; @@ -49,8 +50,8 @@ export class MockAuthCredential implements AuthCredential { return this.response!; } - _linkToIdToken(_auth: Auth, _idToken: string): Promise { - throw new Error('Method not implemented.'); + async _linkToIdToken(_auth: Auth, _idToken: string): Promise { + return this.response!; } _matchIdTokenWithUid(_auth: Auth, _uid: string): Promise { From 4f46bc42fab2544496939c26a9c1e463d4a5a06b Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 11:50:22 -0700 Subject: [PATCH 2/7] Formatting --- .../src/core/credentials/phone.test.ts | 6 +- .../auth-exp/src/core/credentials/phone.ts | 6 +- .../src/core/strategies/credential.test.ts | 59 ++++++++++++------- .../src/core/strategies/credential.ts | 10 ++-- .../src/core/strategies/phone.test.ts | 30 ++++++---- .../auth-exp/src/core/strategies/phone.ts | 7 +-- .../auth-exp/src/core/user/account_info.ts | 3 +- .../auth-exp/src/core/user/unlink.test.ts | 35 +++++------ packages-exp/auth-exp/src/core/user/unlink.ts | 2 +- .../auth-exp/src/core/user/user_impl.ts | 5 +- packages-exp/auth-exp/src/model/user.d.ts | 5 +- .../auth-exp/test/mock_auth_credential.ts | 5 +- 12 files changed, 104 insertions(+), 69 deletions(-) diff --git a/packages-exp/auth-exp/src/core/credentials/phone.test.ts b/packages-exp/auth-exp/src/core/credentials/phone.test.ts index 873d407140c..998cec04067 100644 --- a/packages-exp/auth-exp/src/core/credentials/phone.test.ts +++ b/packages-exp/auth-exp/src/core/credentials/phone.test.ts @@ -83,7 +83,7 @@ describe('core/credentials/phone', () => { refreshToken: '', kind: '', expiresIn: '10', - localId: 'uid', + localId: 'uid' }; it('calls the endpoint with session and code', async () => { @@ -98,7 +98,7 @@ describe('core/credentials/phone', () => { expect(route.calls[0].request).to.eql({ sessionInfo: 'session-info', code: 'code', - idToken: 'id-token', + idToken: 'id-token' }); }); @@ -114,7 +114,7 @@ describe('core/credentials/phone', () => { expect(route.calls[0].request).to.eql({ temporaryProof: 'temp-proof', phoneNumber: 'number', - idToken: 'id-token', + idToken: 'id-token' }); }); }); diff --git a/packages-exp/auth-exp/src/core/credentials/phone.ts b/packages-exp/auth-exp/src/core/credentials/phone.ts index 80aca34c828..bbd8d9e048a 100644 --- a/packages-exp/auth-exp/src/core/credentials/phone.ts +++ b/packages-exp/auth-exp/src/core/credentials/phone.ts @@ -19,7 +19,9 @@ import * as externs from '@firebase/auth-types-exp'; import { PhoneOrOauthTokenResponse } from '../../api/authentication/mfa'; import { - linkWithPhoneNumber, signInWithPhoneNumber, SignInWithPhoneNumberRequest + linkWithPhoneNumber, + signInWithPhoneNumber, + SignInWithPhoneNumberRequest } from '../../api/authentication/sms'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; @@ -47,7 +49,7 @@ export class PhoneAuthCredential _linkToIdToken(auth: Auth, idToken: string): Promise { return linkWithPhoneNumber(auth, { idToken, - ...this.makeVerificationRequest(), + ...this.makeVerificationRequest() }); } diff --git a/packages-exp/auth-exp/src/core/strategies/credential.test.ts b/packages-exp/auth-exp/src/core/strategies/credential.test.ts index 5982afff0d8..aa63ad055be 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.test.ts @@ -18,7 +18,11 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { OperationType, ProviderId, SignInMethod } from '@firebase/auth-types-exp'; +import { + OperationType, + ProviderId, + SignInMethod +} from '@firebase/auth-types-exp'; import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; @@ -30,7 +34,11 @@ import { APIUserInfo } from '../../api/account_management/account'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; import { User } from '../../model/user'; -import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; +import { + _assertLinkedStatus, + linkWithCredential, + signInWithCredential +} from './credential'; use(chaiAsPromised); @@ -103,23 +111,26 @@ describe('core/strategies/credential', () => { users: [ { ...serverUser, - providerUserInfo: [ - {providerId: ProviderId.FIREBASE} - ] + providerUserInfo: [{ providerId: ProviderId.FIREBASE }] } ] }; await expect(linkWithCredential(user, authCredential)).to.be.rejectedWith( - FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', + FirebaseError, + 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).' ); }); it('should return a valid user credential', async () => { - const { credential, user: newUser, operationType } = await linkWithCredential(user, authCredential); - expect(operationType).to.eq(OperationType.LINK); - expect(newUser).to.eq(user); - expect(credential).to.be.null; + const { + credential, + user: newUser, + operationType + } = await linkWithCredential(user, authCredential); + expect(operationType).to.eq(OperationType.LINK); + expect(newUser).to.eq(user); + expect(credential).to.be.null; }); }); @@ -129,24 +140,31 @@ describe('core/strategies/credential', () => { users: [ { ...serverUser, - providerUserInfo: [ - {providerId: ProviderId.PHONE} - ] + providerUserInfo: [{ providerId: ProviderId.PHONE }] } ] }; - await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).to.be.rejectedWith( - FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', + await expect( + _assertLinkedStatus(false, user, ProviderId.PHONE) + ).to.be.rejectedWith( + FirebaseError, + 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).' ); }); it('should not error if provider is not linked', async () => { - await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).not.to.be.rejected; + await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).not.to.be + .rejected; }); it('should error if provider is not linked but it was expected to be', async () => { - await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).to.be.rejectedWith(FirebaseError, 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).'); + await expect( + _assertLinkedStatus(true, user, ProviderId.PHONE) + ).to.be.rejectedWith( + FirebaseError, + 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).' + ); }); it('should not error if provider is linked and that is expected', async () => { @@ -154,13 +172,12 @@ describe('core/strategies/credential', () => { users: [ { ...serverUser, - providerUserInfo: [ - {providerId: ProviderId.PHONE} - ] + providerUserInfo: [{ providerId: ProviderId.PHONE }] } ] }; - await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).not.to.be.rejected; + await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).not.to.be + .rejected; }); }); }); diff --git a/packages-exp/auth-exp/src/core/strategies/credential.ts b/packages-exp/auth-exp/src/core/strategies/credential.ts index 229d6e65225..19106094700 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.ts @@ -55,7 +55,10 @@ export async function linkWithCredential( const credential = credentialExtern as AuthCredential; await _assertLinkedStatus(false, user, credential.providerId); - const response = await credential._linkToIdToken(user.auth, await user.getIdToken()); + const response = await credential._linkToIdToken( + user.auth, + await user.getIdToken() + ); const newCred = _authCredentialFromTokenResponse(response); await user._updateTokensIfNecessary(response, /* reload */ true); @@ -70,7 +73,7 @@ export function _authCredentialFromTokenResponse( phoneNumber } = response as SignInWithPhoneNumberResponse; if (temporaryProof && phoneNumber) { - return new PhoneAuthCredential({temporaryProof, phoneNumber}); + return new PhoneAuthCredential({ temporaryProof, phoneNumber }); } // TODO: Handle Oauth cases @@ -80,7 +83,7 @@ export function _authCredentialFromTokenResponse( export async function _assertLinkedStatus( expected: boolean, user: User, - provider: externs.ProviderId, + provider: externs.ProviderId ): Promise { await _reloadWithoutSaving(user); const providerIds = user.providerData.map(({ providerId }) => providerId); @@ -92,4 +95,3 @@ export async function _assertLinkedStatus( fail(user.auth.name, code); } } - diff --git a/packages-exp/auth-exp/src/core/strategies/phone.test.ts b/packages-exp/auth-exp/src/core/strategies/phone.test.ts index deac4354956..daf3d8b2d53 100644 --- a/packages-exp/auth-exp/src/core/strategies/phone.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/phone.test.ts @@ -20,7 +20,11 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import * as sinonChai from 'sinon-chai'; -import { ApplicationVerifier, OperationType, ProviderId } from '@firebase/auth-types-exp'; +import { + ApplicationVerifier, + OperationType, + ProviderId +} from '@firebase/auth-types-exp'; import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; @@ -31,7 +35,11 @@ import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; import { User } from '../../model/user'; import { RecaptchaVerifier } from '../../platform_browser/recaptcha/recaptcha_verifier'; -import { _verifyPhoneNumber, linkWithPhoneNumber, signInWithPhoneNumber } from './phone'; +import { + _verifyPhoneNumber, + linkWithPhoneNumber, + signInWithPhoneNumber +} from './phone'; use(chaiAsPromised); use(sinonChai); @@ -111,7 +119,7 @@ describe('core/strategies/phone', () => { beforeEach(() => { getAccountInfoEndpoint = mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { - users: [{uid: 'uid'}] + users: [{ uid: 'uid' }] }); user = testUser(auth, 'uid', 'email', true); @@ -122,17 +130,17 @@ describe('core/strategies/phone', () => { users: [ { uid: 'uid', - providerUserInfo: [ - {providerId: ProviderId.PHONE} - ] + providerUserInfo: [{ providerId: ProviderId.PHONE }] } ] }; - await expect(linkWithPhoneNumber(user, 'number', verifier)) - .to.be.rejectedWith( - FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).', - ); + await expect( + linkWithPhoneNumber(user, 'number', verifier) + ).to.be.rejectedWith( + FirebaseError, + 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).' + ); }); it('calls verify phone number', async () => { @@ -178,7 +186,7 @@ describe('core/strategies/phone', () => { expect(signInEndpoint.calls[0].request).to.eql({ sessionInfo: 'session-info', code: '6789', - idToken: initialIdToken, + idToken: initialIdToken }); }); }); diff --git a/packages-exp/auth-exp/src/core/strategies/phone.ts b/packages-exp/auth-exp/src/core/strategies/phone.ts index 39822fc4779..5c77bf4d445 100644 --- a/packages-exp/auth-exp/src/core/strategies/phone.ts +++ b/packages-exp/auth-exp/src/core/strategies/phone.ts @@ -21,15 +21,10 @@ import { sendPhoneVerificationCode } from '../../api/authentication/sms'; import { Auth } from '../../model/auth'; import { User } from '../../model/user'; import { RECAPTCHA_VERIFIER_TYPE } from '../../platform_browser/recaptcha/recaptcha_verifier'; +import { PhoneAuthCredential } from '../credentials/phone'; import { AuthErrorCode } from '../errors'; import { assert } from '../util/assert'; -<<<<<<< HEAD -import { signInWithCredential } from './credential'; -import { PhoneAuthCredential } from '../credentials/phone'; -======= import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; -import { PhoneAuthCredential } from './phone_credential'; ->>>>>>> ffdb4d73... Add unlink(), linkWithCredential(), linkWithPhoneNumber() interface OnConfirmationCallback { (credential: PhoneAuthCredential): Promise; diff --git a/packages-exp/auth-exp/src/core/user/account_info.ts b/packages-exp/auth-exp/src/core/user/account_info.ts index 50716fe0107..73f62ceda9f 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.ts @@ -18,7 +18,8 @@ import * as externs from '@firebase/auth-types-exp'; import { - updateEmailPassword as apiUpdateEmailPassword, UpdateEmailPasswordRequest + updateEmailPassword as apiUpdateEmailPassword, + UpdateEmailPasswordRequest } from '../../api/account_management/email_and_password'; import { updateProfile as apiUpdateProfile } from '../../api/account_management/profile'; import { User } from '../../model/user'; diff --git a/packages-exp/auth-exp/src/core/user/unlink.test.ts b/packages-exp/auth-exp/src/core/user/unlink.test.ts index 02832d6753c..0777d503205 100644 --- a/packages-exp/auth-exp/src/core/user/unlink.test.ts +++ b/packages-exp/auth-exp/src/core/user/unlink.test.ts @@ -45,16 +45,19 @@ describe('core/user/unlink', () => { fetch.tearDown(); }); - it('rejects if the provider is not linked', async() => { + it('rejects if the provider is not linked', async () => { mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { users: [ { - uid: 'uid', + uid: 'uid' } ] }); - await expect(unlink(user, ProviderId.PHONE)).to.be.rejectedWith(FirebaseError, 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).'); + await expect(unlink(user, ProviderId.PHONE)).to.be.rejectedWith( + FirebaseError, + 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).' + ); }); context('with properly linked account', () => { @@ -64,17 +67,17 @@ describe('core/user/unlink', () => { users: [ { uid: 'uid', - providerUserInfo: [ - {providerId: ProviderId.PHONE} - ] + providerUserInfo: [{ providerId: ProviderId.PHONE }] } ] }); endpoint = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { - providerUserInfo: [{ - providerId: ProviderId.GOOGLE, - }] + providerUserInfo: [ + { + providerId: ProviderId.GOOGLE + } + ] }); }); @@ -86,7 +89,7 @@ describe('core/user/unlink', () => { phoneNumber: '', email: '', photoURL: '', - uid: '', + uid: '' }, { providerId: ProviderId.GOOGLE, @@ -94,7 +97,7 @@ describe('core/user/unlink', () => { phoneNumber: '', email: '', photoURL: '', - uid: '', + uid: '' } ]; await unlink(user, ProviderId.PHONE); @@ -105,21 +108,19 @@ describe('core/user/unlink', () => { phoneNumber: '', email: '', photoURL: '', - uid: '', + uid: '' } ]); - expect(auth.persistenceLayer.lastObjectSet).to.eql( - user.toPlainObject(), - ); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); it('calls the endpoint with the provider', async () => { await unlink(user, ProviderId.PHONE); expect(endpoint.calls[0].request).to.eql({ idToken: await user.getIdToken(), - deleteProvider: [ProviderId.PHONE], + deleteProvider: [ProviderId.PHONE] }); }); }); -}); \ No newline at end of file +}); diff --git a/packages-exp/auth-exp/src/core/user/unlink.ts b/packages-exp/auth-exp/src/core/user/unlink.ts index a81bfea1f24..ed19ea8a54e 100644 --- a/packages-exp/auth-exp/src/core/user/unlink.ts +++ b/packages-exp/auth-exp/src/core/user/unlink.ts @@ -1,6 +1,6 @@ /** * @license - * Copyright 2019 Google Inc. + * Copyright 2019 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/packages-exp/auth-exp/src/core/user/user_impl.ts b/packages-exp/auth-exp/src/core/user/user_impl.ts index 75eeac07623..86198dbe382 100644 --- a/packages-exp/auth-exp/src/core/user/user_impl.ts +++ b/packages-exp/auth-exp/src/core/user/user_impl.ts @@ -101,7 +101,10 @@ export class UserImpl implements User { return reload(this); } - async _updateTokensIfNecessary(response: IdTokenResponse, reload = false): Promise { + async _updateTokensIfNecessary( + response: IdTokenResponse, + reload = false + ): Promise { let tokensRefreshed = false; if ( response.idToken && diff --git a/packages-exp/auth-exp/src/model/user.d.ts b/packages-exp/auth-exp/src/model/user.d.ts index fed8c287d96..8a8a3d18758 100644 --- a/packages-exp/auth-exp/src/model/user.d.ts +++ b/packages-exp/auth-exp/src/model/user.d.ts @@ -40,7 +40,10 @@ export interface User extends externs.User { providerData: MutableUserInfo[]; metadata: externs.UserMetadata; - _updateTokensIfNecessary(response: IdTokenResponse, reload?: boolean): Promise; + _updateTokensIfNecessary( + response: IdTokenResponse, + reload?: boolean + ): Promise; getIdToken(forceRefresh?: boolean): Promise; getIdTokenResult(forceRefresh?: boolean): Promise; diff --git a/packages-exp/auth-exp/test/mock_auth_credential.ts b/packages-exp/auth-exp/test/mock_auth_credential.ts index 8341460c407..12ec8e1ae16 100644 --- a/packages-exp/auth-exp/test/mock_auth_credential.ts +++ b/packages-exp/auth-exp/test/mock_auth_credential.ts @@ -50,7 +50,10 @@ export class MockAuthCredential implements AuthCredential { return this.response!; } - async _linkToIdToken(_auth: Auth, _idToken: string): Promise { + async _linkToIdToken( + _auth: Auth, + _idToken: string + ): Promise { return this.response!; } From 5768cd411c8b2cf5fc1affcbdc067664b60f4e1c Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 11:56:43 -0700 Subject: [PATCH 3/7] Formatting --- packages-exp/auth-exp/src/core/strategies/phone.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages-exp/auth-exp/src/core/strategies/phone.ts b/packages-exp/auth-exp/src/core/strategies/phone.ts index 5c77bf4d445..639145cf569 100644 --- a/packages-exp/auth-exp/src/core/strategies/phone.ts +++ b/packages-exp/auth-exp/src/core/strategies/phone.ts @@ -24,7 +24,11 @@ import { RECAPTCHA_VERIFIER_TYPE } from '../../platform_browser/recaptcha/recapt import { PhoneAuthCredential } from '../credentials/phone'; import { AuthErrorCode } from '../errors'; import { assert } from '../util/assert'; -import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; +import { + _assertLinkedStatus, + linkWithCredential, + signInWithCredential +} from './credential'; interface OnConfirmationCallback { (credential: PhoneAuthCredential): Promise; From 64ff065101eafc122945909a648c8d86926f77de Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 15:28:22 -0700 Subject: [PATCH 4/7] PR feedback --- .../src/core/strategies/credential.test.ts | 24 ++++-------- .../src/core/strategies/credential.ts | 12 +++--- .../auth-exp/src/core/user/unlink.test.ts | 38 ++++++++++++++++++- packages-exp/auth-exp/src/core/user/unlink.ts | 7 ++-- .../auth-exp/src/core/util/providers.ts | 27 +++++++++++++ 5 files changed, 82 insertions(+), 26 deletions(-) create mode 100644 packages-exp/auth-exp/src/core/util/providers.ts diff --git a/packages-exp/auth-exp/src/core/strategies/credential.test.ts b/packages-exp/auth-exp/src/core/strategies/credential.test.ts index aa63ad055be..f69a76a6b24 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.test.ts @@ -18,11 +18,7 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { - OperationType, - ProviderId, - SignInMethod -} from '@firebase/auth-types-exp'; +import { OperationType, ProviderId, SignInMethod } from '@firebase/auth-types-exp'; import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; @@ -34,11 +30,7 @@ import { APIUserInfo } from '../../api/account_management/account'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; import { User } from '../../model/user'; -import { - _assertLinkedStatus, - linkWithCredential, - signInWithCredential -} from './credential'; +import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; use(chaiAsPromised); @@ -140,13 +132,13 @@ describe('core/strategies/credential', () => { users: [ { ...serverUser, - providerUserInfo: [{ providerId: ProviderId.PHONE }] + providerUserInfo: [{ providerId: ProviderId.GOOGLE }] } ] }; await expect( - _assertLinkedStatus(false, user, ProviderId.PHONE) + _assertLinkedStatus(false, user, ProviderId.GOOGLE) ).to.be.rejectedWith( FirebaseError, 'Firebase: User can only be linked to one identity for the given provider. (auth/provider-already-linked).' @@ -154,13 +146,13 @@ describe('core/strategies/credential', () => { }); it('should not error if provider is not linked', async () => { - await expect(_assertLinkedStatus(false, user, ProviderId.PHONE)).not.to.be + await expect(_assertLinkedStatus(false, user, ProviderId.GOOGLE)).not.to.be .rejected; }); it('should error if provider is not linked but it was expected to be', async () => { await expect( - _assertLinkedStatus(true, user, ProviderId.PHONE) + _assertLinkedStatus(true, user, ProviderId.GOOGLE) ).to.be.rejectedWith( FirebaseError, 'Firebase: User was not linked to an account with the given provider. (auth/no-such-provider).' @@ -172,11 +164,11 @@ describe('core/strategies/credential', () => { users: [ { ...serverUser, - providerUserInfo: [{ providerId: ProviderId.PHONE }] + providerUserInfo: [{ providerId: ProviderId.GOOGLE }] } ] }; - await expect(_assertLinkedStatus(true, user, ProviderId.PHONE)).not.to.be + await expect(_assertLinkedStatus(true, user, ProviderId.GOOGLE)).not.to.be .rejected; }); }); diff --git a/packages-exp/auth-exp/src/core/strategies/credential.ts b/packages-exp/auth-exp/src/core/strategies/credential.ts index 19106094700..24ad1d77206 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.ts @@ -27,7 +27,8 @@ import { PhoneAuthCredential } from '../credentials/phone'; import { AuthErrorCode } from '../errors'; import { _reloadWithoutSaving } from '../user/reload'; import { UserCredentialImpl } from '../user/user_credential_impl'; -import { fail } from '../util/assert'; +import { assert } from '../util/assert'; +import { providerDataAsNames } from '../util/providers'; export async function signInWithCredential( authExtern: externs.Auth, @@ -86,12 +87,11 @@ export async function _assertLinkedStatus( provider: externs.ProviderId ): Promise { await _reloadWithoutSaving(user); - const providerIds = user.providerData.map(({ providerId }) => providerId); - if (providerIds.includes(provider) !== expected) { - const code = + const providerIds = providerDataAsNames(user.providerData); + + const code = expected === false ? AuthErrorCode.PROVIDER_ALREADY_LINKED : AuthErrorCode.NO_SUCH_PROVIDER; - fail(user.auth.name, code); - } + assert(providerIds.has(provider) === expected, user.auth.name, code); } diff --git a/packages-exp/auth-exp/src/core/user/unlink.test.ts b/packages-exp/auth-exp/src/core/user/unlink.test.ts index 0777d503205..eeffc2f8331 100644 --- a/packages-exp/auth-exp/src/core/user/unlink.test.ts +++ b/packages-exp/auth-exp/src/core/user/unlink.test.ts @@ -81,7 +81,8 @@ describe('core/user/unlink', () => { }); }); - it('removes the provider from the list and persists', async () => { + it('removes the phone provider from the list and persists', async () => { + user.phoneNumber = 'number!'; user.providerData = [ { providerId: ProviderId.PHONE, @@ -112,6 +113,41 @@ describe('core/user/unlink', () => { } ]); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(user.phoneNumber).to.be.null; + }); + + it('removes non-phone provider from the list and persists', async () => { + user.providerData = [ + { + providerId: ProviderId.GOOGLE, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '' + }, + { + providerId: ProviderId.TWITTER, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '' + } + ]; + await unlink(user, ProviderId.TWITTER); + expect(user.providerData).to.eql([ + { + providerId: ProviderId.GOOGLE, + displayName: '', + phoneNumber: '', + email: '', + photoURL: '', + uid: '' + } + ]); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); diff --git a/packages-exp/auth-exp/src/core/user/unlink.ts b/packages-exp/auth-exp/src/core/user/unlink.ts index ed19ea8a54e..d2b73e81f30 100644 --- a/packages-exp/auth-exp/src/core/user/unlink.ts +++ b/packages-exp/auth-exp/src/core/user/unlink.ts @@ -20,6 +20,7 @@ import * as externs from '@firebase/auth-types-exp'; import { deleteLinkedAccounts } from '../../api/account_management/account'; import { User } from '../../model/user'; import { _assertLinkedStatus } from '../strategies/credential'; +import { providerDataAsNames } from '../util/providers'; export async function unlink( userExtern: externs.User, @@ -32,12 +33,12 @@ export async function unlink( deleteProvider: [providerId] }); - const providersLeft = (providerUserInfo || []).map(i => i.providerId); + const providersLeft = providerDataAsNames(providerUserInfo || []); user.providerData = user.providerData.filter(pd => - providersLeft.includes(pd.providerId || undefined) + providersLeft.has(pd.providerId) ); - if (!providersLeft.includes(externs.ProviderId.PHONE)) { + if (!providersLeft.has(externs.ProviderId.PHONE)) { user.phoneNumber = null; } diff --git a/packages-exp/auth-exp/src/core/util/providers.ts b/packages-exp/auth-exp/src/core/util/providers.ts new file mode 100644 index 00000000000..bf8dd012f3f --- /dev/null +++ b/packages-exp/auth-exp/src/core/util/providers.ts @@ -0,0 +1,27 @@ +/** + * @license + * Copyright 2020 Google LLC + * + * 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. + */ + +export interface ProviderAssociatedObject { + providerId?: string; +} + +/** + * Takes a set of UserInfo provider data and converts it to a set of names + */ +export function providerDataAsNames(providerData: T[]): Set { + return new Set(providerData.map(({providerId}) => providerId).filter(pid => !!pid) as string[]); +} \ No newline at end of file From 2d80d381fc9f87a12669b7aa0e9f7f61f2f2be54 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 15:28:46 -0700 Subject: [PATCH 5/7] Formatting --- .../src/core/strategies/credential.test.ts | 16 ++++++++++++---- .../auth-exp/src/core/strategies/credential.ts | 6 +++--- packages-exp/auth-exp/src/core/util/providers.ts | 12 +++++++++--- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/packages-exp/auth-exp/src/core/strategies/credential.test.ts b/packages-exp/auth-exp/src/core/strategies/credential.test.ts index f69a76a6b24..607859eb0ab 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.test.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.test.ts @@ -18,7 +18,11 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; -import { OperationType, ProviderId, SignInMethod } from '@firebase/auth-types-exp'; +import { + OperationType, + ProviderId, + SignInMethod +} from '@firebase/auth-types-exp'; import { FirebaseError } from '@firebase/util'; import { mockEndpoint } from '../../../test/api/helper'; @@ -30,7 +34,11 @@ import { APIUserInfo } from '../../api/account_management/account'; import { Auth } from '../../model/auth'; import { IdTokenResponse } from '../../model/id_token'; import { User } from '../../model/user'; -import { _assertLinkedStatus, linkWithCredential, signInWithCredential } from './credential'; +import { + _assertLinkedStatus, + linkWithCredential, + signInWithCredential +} from './credential'; use(chaiAsPromised); @@ -146,8 +154,8 @@ describe('core/strategies/credential', () => { }); it('should not error if provider is not linked', async () => { - await expect(_assertLinkedStatus(false, user, ProviderId.GOOGLE)).not.to.be - .rejected; + await expect(_assertLinkedStatus(false, user, ProviderId.GOOGLE)).not.to + .be.rejected; }); it('should error if provider is not linked but it was expected to be', async () => { diff --git a/packages-exp/auth-exp/src/core/strategies/credential.ts b/packages-exp/auth-exp/src/core/strategies/credential.ts index 24ad1d77206..c2fe6ff18bb 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.ts @@ -90,8 +90,8 @@ export async function _assertLinkedStatus( const providerIds = providerDataAsNames(user.providerData); const code = - expected === false - ? AuthErrorCode.PROVIDER_ALREADY_LINKED - : AuthErrorCode.NO_SUCH_PROVIDER; + expected === false + ? AuthErrorCode.PROVIDER_ALREADY_LINKED + : AuthErrorCode.NO_SUCH_PROVIDER; assert(providerIds.has(provider) === expected, user.auth.name, code); } diff --git a/packages-exp/auth-exp/src/core/util/providers.ts b/packages-exp/auth-exp/src/core/util/providers.ts index bf8dd012f3f..354ecb72706 100644 --- a/packages-exp/auth-exp/src/core/util/providers.ts +++ b/packages-exp/auth-exp/src/core/util/providers.ts @@ -22,6 +22,12 @@ export interface ProviderAssociatedObject { /** * Takes a set of UserInfo provider data and converts it to a set of names */ -export function providerDataAsNames(providerData: T[]): Set { - return new Set(providerData.map(({providerId}) => providerId).filter(pid => !!pid) as string[]); -} \ No newline at end of file +export function providerDataAsNames( + providerData: T[] +): Set { + return new Set( + providerData + .map(({ providerId }) => providerId) + .filter(pid => !!pid) as string[] + ); +} From ead029f6f5d74c569e18080486624bd940b82ce7 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 15:32:29 -0700 Subject: [PATCH 6/7] Add export to index.ts --- packages-exp/auth-exp/src/index.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages-exp/auth-exp/src/index.ts b/packages-exp/auth-exp/src/index.ts index d2db8095be8..cac92bb6e4c 100644 --- a/packages-exp/auth-exp/src/index.ts +++ b/packages-exp/auth-exp/src/index.ts @@ -33,7 +33,7 @@ export { PhoneAuthProvider } from './core/providers/phone'; // core/strategies export { signInAnonymously } from './core/strategies/anonymous'; -export { signInWithCredential } from './core/strategies/credential'; +export { signInWithCredential, linkWithCredential } from './core/strategies/credential'; export { signInWithCustomToken } from './core/strategies/custom_token'; export { sendPasswordResetEmail, @@ -51,7 +51,7 @@ export { fetchSignInMethodsForEmail, sendEmailVerification } from './core/strategies/email'; -export { signInWithPhoneNumber } from './core/strategies/phone'; +export { signInWithPhoneNumber, linkWithPhoneNumber } from './core/strategies/phone'; // core/user export { @@ -61,6 +61,7 @@ export { } from './core/user/account_info'; export { getIdToken, getIdTokenResult } from './core/user/id_token_result'; export { reload } from './core/user/reload'; +export { unlink } from './core/user/unlink'; // model export { Operation as ActionCodeOperationType } from './model/action_code_info'; From 60795582126d273fa87332d25c3aec6a18551ec9 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 12 Jun 2020 15:32:47 -0700 Subject: [PATCH 7/7] Formatting --- packages-exp/auth-exp/src/index.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/packages-exp/auth-exp/src/index.ts b/packages-exp/auth-exp/src/index.ts index cac92bb6e4c..904481508ed 100644 --- a/packages-exp/auth-exp/src/index.ts +++ b/packages-exp/auth-exp/src/index.ts @@ -33,7 +33,10 @@ export { PhoneAuthProvider } from './core/providers/phone'; // core/strategies export { signInAnonymously } from './core/strategies/anonymous'; -export { signInWithCredential, linkWithCredential } from './core/strategies/credential'; +export { + signInWithCredential, + linkWithCredential +} from './core/strategies/credential'; export { signInWithCustomToken } from './core/strategies/custom_token'; export { sendPasswordResetEmail, @@ -51,7 +54,10 @@ export { fetchSignInMethodsForEmail, sendEmailVerification } from './core/strategies/email'; -export { signInWithPhoneNumber, linkWithPhoneNumber } from './core/strategies/phone'; +export { + signInWithPhoneNumber, + linkWithPhoneNumber +} from './core/strategies/phone'; // core/user export {