From 6d57cd2a37cebc57dc71666d38058bf13e0c6428 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 21 May 2020 09:22:10 -0700 Subject: [PATCH 01/11] Profile management --- .../auth-exp/src/core/auth/auth_impl.test.ts | 4 +- .../auth-exp/src/core/auth/auth_impl.ts | 6 + .../src/core/user/account_info.test.ts | 131 ++++++++++++++++++ .../auth-exp/src/core/user/account_info.ts | 67 +++++++++ packages-exp/auth-exp/src/core/user/reload.ts | 7 +- .../auth-exp/src/core/user/user_impl.ts | 18 ++- packages-exp/auth-exp/src/model/auth.d.ts | 1 + packages-exp/auth-exp/src/model/user.d.ts | 8 +- 8 files changed, 229 insertions(+), 13 deletions(-) create mode 100644 packages-exp/auth-exp/src/core/user/account_info.test.ts create mode 100644 packages-exp/auth-exp/src/core/user/account_info.ts diff --git a/packages-exp/auth-exp/src/core/auth/auth_impl.test.ts b/packages-exp/auth-exp/src/core/auth/auth_impl.test.ts index 9decd11b6c5..ac46ecc7526 100644 --- a/packages-exp/auth-exp/src/core/auth/auth_impl.test.ts +++ b/packages-exp/auth-exp/src/core/auth/auth_impl.test.ts @@ -208,13 +208,13 @@ describe('core/auth/auth_impl', () => { }); it('onAuthStateChange does not trigger for user props change', async () => { - user.refreshToken = 'hey look I changed'; + user.photoURL = 'blah'; await auth.updateCurrentUser(user); expect(authStateCallback).not.to.have.been.called; }); it('onIdTokenChange triggers for user props change', async () => { - user.refreshToken = 'hey look I changed'; + user.photoURL = 'hey look I changed'; await auth.updateCurrentUser(user); expect(idTokenCallback).to.have.been.calledWith(user); }); diff --git a/packages-exp/auth-exp/src/core/auth/auth_impl.ts b/packages-exp/auth-exp/src/core/auth/auth_impl.ts index 1b5c78966eb..3c44a59e760 100644 --- a/packages-exp/auth-exp/src/core/auth/auth_impl.ts +++ b/packages-exp/auth-exp/src/core/auth/auth_impl.ts @@ -129,6 +129,12 @@ export class AuthImpl implements Auth { ); } + async _persistAndNotifyIfCurrent(user: User): Promise { + if (user === this.currentUser) { + return this.updateCurrentUser(user); + } + } + _notifyStateListeners(): void { if (!this._isInitialized) { return; diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts new file mode 100644 index 00000000000..ce894b7e919 --- /dev/null +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -0,0 +1,131 @@ +/** + * @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 * as sinon from 'sinon'; +import * as sinonChai from 'sinon-chai'; + +import { UserInfo } from '@firebase/auth-types-exp'; + +// import { UserInfo } from '@firebase/auth-types-exp'; +import { mockEndpoint } from '../../../test/api/helper'; +import { testUser } from '../../../test/mock_auth'; +import * as fetch from '../../../test/mock_fetch'; +import { Endpoint } from '../../api'; +import { User } from '../../model/user'; +import { ProviderId } from '../providers'; +// import { ProviderId } from '../providers'; +import { updateProfile } from './account_info'; + +use(chaiAsPromised); +use(sinonChai); + +const PASSWORD_PROVIDER: UserInfo = { + providerId: ProviderId.PASSWORD, + uid: 'uid', + email: 'email', + displayName: 'old-name', + phoneNumber: 'phone-number', + photoURL: 'old-url' +}; + +describe('core/user/profile', () => { + let user: User; + + beforeEach(() => { + user = testUser('uid', '', true); + }); + + afterEach(() => { + sinon.restore(); + }); + + beforeEach(fetch.setUp); + afterEach(fetch.tearDown); + + it('returns immediately if profile object is empty', async () => { + const ep = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + await updateProfile(user, {}); + expect(ep.calls).to.be.empty; + }); + + it('calls the setAccountInfo endpoint', async () => { + const ep =mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + expect(ep.calls[0].request).to.eql({ + idToken: 'access-token', + displayName: 'displayname', + photoUrl: 'photo', + }); + }); + + it('sets the fields on the user based on the response', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + displayName: 'response-name', + photoUrl: 'response-photo', + }); + + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + expect(user.displayName).to.eq('response-name'); + expect(user.photoURL).to.eq('response-photo'); + }); + + it('sets the fields on the passwd provider', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + displayName: 'response-name', + photoUrl: 'response-photo', + }); + user.providerData = [{...PASSWORD_PROVIDER}]; + + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + const provider = user.providerData[0]; + expect(provider.displayName).to.eq('response-name'); + expect(provider.photoURL).to.eq('response-photo'); + }); + + describe('notifications', () => { + beforeEach(() => { + user.auth.currentUser = user; + }); + + it('triggers a token update if necessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'new-id-token', + refreshToken: 'new-refresh-token', + expiresIn: 300, + }); + + const notifySpy = sinon.stub(user.auth, '_notifyStateListeners'); + await updateProfile(user, {displayName: 'd'}); + expect(notifySpy).to.have.been.called; + }); + + it('does NOT trigger a token update if unnecessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'access-token', + refreshToken: 'refresh-token', + expiresIn: 300, + }); + + const notifySpy = sinon.stub(user.auth, '_notifyStateListeners'); + await updateProfile(user, {displayName: 'd'}); + expect(notifySpy).not.to.have.been.called; + }); + }); +}); \ No newline at end of file diff --git a/packages-exp/auth-exp/src/core/user/account_info.ts b/packages-exp/auth-exp/src/core/user/account_info.ts new file mode 100644 index 00000000000..96fe50e0d3b --- /dev/null +++ b/packages-exp/auth-exp/src/core/user/account_info.ts @@ -0,0 +1,67 @@ +/** + * @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 * as externs from '@firebase/auth-types-exp'; + +import { + updateEmailPassword as apiUpdateEmailPassword +} from '../../api/account_management/email_and_password'; +import { updateProfile as apiUpdateProfile } from '../../api/account_management/profile'; +import { User } from '../../model/user'; +import { ProviderId } from '../providers'; +import { _reloadWithoutSaving } from './reload'; + +interface Profile { + displayName?: string|null; + photoURL?: string|null; +} + +export async function updateProfile(externUser: externs.User, {displayName, photoURL: photoUrl}: Profile): Promise { + if (displayName === undefined && photoUrl === undefined) { + return; + } + + const user = externUser as User; + const idToken = await user.getIdToken(); + const profileRequest = {idToken, displayName, photoUrl}; + const response = await apiUpdateProfile(user.auth, profileRequest); + + user.displayName = response.displayName || null; + user.photoURL = response.photoUrl || null; + + // Update the password provider as well + const passwordProvider = user.providerData.find(p => p.providerId === ProviderId.PASSWORD); + if (passwordProvider) { + passwordProvider.displayName = user.displayName; + passwordProvider.photoURL = user.photoURL; + } + + user._updateTokensIfNecessary(response); + return user.auth._persistAndNotifyIfCurrent(user); +} + +export async function updateEmail(externUser: externs.User, newEmail: string): Promise { + const user = externUser as User; + const {auth} = user; + const idToken = await user.getIdToken(); + const response = await apiUpdateEmailPassword(auth, {idToken, email: newEmail}); + user._updateTokensIfNecessary(response); + + // To update + await _reloadWithoutSaving(user); + return auth._persistAndNotifyIfCurrent(user); +} diff --git a/packages-exp/auth-exp/src/core/user/reload.ts b/packages-exp/auth-exp/src/core/user/reload.ts index 174fbaaf044..8dda4b3a66e 100644 --- a/packages-exp/auth-exp/src/core/user/reload.ts +++ b/packages-exp/auth-exp/src/core/user/reload.ts @@ -17,10 +17,7 @@ import * as externs from '@firebase/auth-types-exp'; -import { - getAccountInfo, - ProviderUserInfo -} from '../../api/account_management/account'; +import { getAccountInfo, ProviderUserInfo } from '../../api/account_management/account'; import { User } from '../../model/user'; import { assert } from '../util/assert'; @@ -60,7 +57,7 @@ export async function reload(externUser: externs.User): Promise { // Even though the current user hasn't changed, update // current user will trigger a persistence update w/ the // new info. - return user.auth.updateCurrentUser(user); + return user.auth._persistAndNotifyIfCurrent(user); } function mergeProviderData( 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 a6b785f3c78..3cee431bac8 100644 --- a/packages-exp/auth-exp/src/core/user/user_impl.ts +++ b/packages-exp/auth-exp/src/core/user/user_impl.ts @@ -51,7 +51,6 @@ export class UserImpl implements User { // For the user object, provider is always Firebase. readonly providerId = ProviderId.FIREBASE; stsTokenManager: StsTokenManager; - refreshToken = ''; uid: string; auth: Auth; @@ -81,11 +80,10 @@ export class UserImpl implements User { const tokens = await this.stsTokenManager.getToken(this.auth, forceRefresh); assert(tokens, this.auth.name); - const { refreshToken, accessToken, wasRefreshed } = tokens; - this.refreshToken = refreshToken || ''; + const { accessToken, wasRefreshed } = tokens; - if (wasRefreshed && this.auth.currentUser === this) { - this.auth._notifyStateListeners(); + if (wasRefreshed) { + await this.auth._persistAndNotifyIfCurrent(this); } return accessToken; @@ -99,6 +97,12 @@ export class UserImpl implements User { return reload(this); } + _updateTokensIfNecessary(response: IdTokenResponse): void { + if (response.idToken && response.idToken !== this.stsTokenManager.accessToken) { + this.stsTokenManager.updateFromServerResponse(response); + } + } + async delete(): Promise { const idToken = await this.getIdToken(); await deleteAccount(this.auth, { idToken }); @@ -121,6 +125,10 @@ export class UserImpl implements User { }; } + get refreshToken(): string { + return this.stsTokenManager.refreshToken || ''; + } + static fromPlainObject(auth: Auth, object: PersistedBlob): User { const { uid, diff --git a/packages-exp/auth-exp/src/model/auth.d.ts b/packages-exp/auth-exp/src/model/auth.d.ts index d524bb00d96..37434aca1e5 100644 --- a/packages-exp/auth-exp/src/model/auth.d.ts +++ b/packages-exp/auth-exp/src/model/auth.d.ts @@ -52,6 +52,7 @@ export interface Auth extends externs.Auth { completed?: CompleteFn ): Unsubscribe; _notifyStateListeners(): void; + _persistAndNotifyIfCurrent(user: User): Promise; } export interface Dependencies { diff --git a/packages-exp/auth-exp/src/model/user.d.ts b/packages-exp/auth-exp/src/model/user.d.ts index 539e75a6626..c80fbe2a4b9 100644 --- a/packages-exp/auth-exp/src/model/user.d.ts +++ b/packages-exp/auth-exp/src/model/user.d.ts @@ -19,6 +19,11 @@ import * as externs from '@firebase/auth-types-exp'; import { PersistedBlob } from '../core/persistence'; import { Auth } from './auth'; +import { IdTokenResponse } from './id_token'; + +type ModifiableUserInfo = { + -readonly [K in keyof externs.UserInfo]: externs.UserInfo[K]; +}; export interface User extends externs.User { uid: string; @@ -32,8 +37,9 @@ export interface User extends externs.User { refreshToken: string; emailVerified: boolean; tenantId: string | null; - providerData: externs.UserInfo[]; + providerData: ModifiableUserInfo[]; metadata: externs.UserMetadata; + _updateTokensIfNecessary(response: IdTokenResponse): void; getIdToken(forceRefresh?: boolean): Promise; getIdTokenResult(forceRefresh?: boolean): Promise; From d637e2e2c440648c2a6c68cd8e17f88962afed3e Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Wed, 27 May 2020 15:35:00 -0700 Subject: [PATCH 02/11] Add updateProfile, updateEmail, updatePassword --- .../auth-exp/src/core/auth/auth_impl.ts | 28 ++- .../src/core/user/account_info.test.ts | 217 +++++++++++++----- .../auth-exp/src/core/user/account_info.ts | 43 +++- .../auth-exp/src/core/user/reload.test.ts | 14 +- packages-exp/auth-exp/src/core/user/reload.ts | 3 +- .../auth-exp/src/core/user/user_impl.ts | 8 +- packages-exp/auth-exp/src/model/auth.d.ts | 4 +- packages-exp/auth-exp/src/model/user.d.ts | 2 +- 8 files changed, 236 insertions(+), 83 deletions(-) diff --git a/packages-exp/auth-exp/src/core/auth/auth_impl.ts b/packages-exp/auth-exp/src/core/auth/auth_impl.ts index 3c44a59e760..4dda3a16be0 100644 --- a/packages-exp/auth-exp/src/core/auth/auth_impl.ts +++ b/packages-exp/auth-exp/src/core/auth/auth_impl.ts @@ -81,7 +81,7 @@ export class AuthImpl implements Auth { } this._isInitialized = true; - this._notifyStateListeners(); + this.notifyAuthListeners(); }); } @@ -89,12 +89,15 @@ export class AuthImpl implements Auth { throw new Error('Method not implemented.'); } - updateCurrentUser(user: User | null): Promise { - return this.queue(() => this.directlySetCurrentUser(user)); + async updateCurrentUser(user: User | null): Promise { + return this.queue(async () => { + await this.directlySetCurrentUser(user); + this.notifyAuthListeners(); + }); } - signOut(): Promise { - return this.queue(() => this.directlySetCurrentUser(null)); + async signOut(): Promise { + return this.updateCurrentUser(null); } setPersistence(persistence: Persistence): Promise { @@ -129,13 +132,20 @@ export class AuthImpl implements Auth { ); } - async _persistAndNotifyIfCurrent(user: User): Promise { + async _persistUserIfCurrent(user: User): Promise { if (user === this.currentUser) { - return this.updateCurrentUser(user); + return this.queue(async () => this.directlySetCurrentUser(user)); } } - _notifyStateListeners(): void { + /** Notifies listeners only if the user is current */ + _notifyListenersIfCurrent(user: User): void { + if (user === this.currentUser) { + this.notifyAuthListeners(); + } + } + + private notifyAuthListeners(): void { if (!this._isInitialized) { return; } @@ -184,8 +194,6 @@ export class AuthImpl implements Auth { } else { await this.assertedPersistence.removeCurrentUser(); } - - this._notifyStateListeners(); } private queue(action: AsyncAction): Promise { diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts index ce894b7e919..1afd652d99d 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.test.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -24,13 +24,13 @@ import { UserInfo } from '@firebase/auth-types-exp'; // import { UserInfo } from '@firebase/auth-types-exp'; import { mockEndpoint } from '../../../test/api/helper'; -import { testUser } from '../../../test/mock_auth'; +import { testPersistence, testUser } from '../../../test/mock_auth'; import * as fetch from '../../../test/mock_fetch'; import { Endpoint } from '../../api'; +import { Auth } from '../../model/auth'; import { User } from '../../model/user'; import { ProviderId } from '../providers'; -// import { ProviderId } from '../providers'; -import { updateProfile } from './account_info'; +import { updateEmail, updatePassword, updateProfile } from './account_info'; use(chaiAsPromised); use(sinonChai); @@ -49,83 +49,194 @@ describe('core/user/profile', () => { beforeEach(() => { user = testUser('uid', '', true); + fetch.setUp(); }); afterEach(() => { sinon.restore(); + fetch.tearDown(); }); - beforeEach(fetch.setUp); - afterEach(fetch.tearDown); - - it('returns immediately if profile object is empty', async () => { - const ep = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); - await updateProfile(user, {}); - expect(ep.calls).to.be.empty; - }); + describe('#updateProfile', () => { + it('returns immediately if profile object is empty', async () => { + const ep = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + await updateProfile(user, {}); + expect(ep.calls).to.be.empty; + }); - it('calls the setAccountInfo endpoint', async () => { - const ep =mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + it('calls the setAccountInfo endpoint', async () => { + const ep =mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); - expect(ep.calls[0].request).to.eql({ - idToken: 'access-token', - displayName: 'displayname', - photoUrl: 'photo', + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + expect(ep.calls[0].request).to.eql({ + idToken: 'access-token', + displayName: 'displayname', + photoUrl: 'photo', + }); }); - }); - it('sets the fields on the user based on the response', async () => { - mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { - displayName: 'response-name', - photoUrl: 'response-photo', + it('sets the fields on the user based on the response', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + displayName: 'response-name', + photoUrl: 'response-photo', + }); + + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + expect(user.displayName).to.eq('response-name'); + expect(user.photoURL).to.eq('response-photo'); }); - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); - expect(user.displayName).to.eq('response-name'); - expect(user.photoURL).to.eq('response-photo'); + it('sets the fields on the password provider', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + displayName: 'response-name', + photoUrl: 'response-photo', + }); + user.providerData = [{...PASSWORD_PROVIDER}]; + + await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + const provider = user.providerData[0]; + expect(provider.displayName).to.eq('response-name'); + expect(provider.photoURL).to.eq('response-photo'); + }); }); - it('sets the fields on the passwd provider', async () => { - mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { - displayName: 'response-name', - photoUrl: 'response-photo', + describe('#updateEmail', () => { + it('calls the setAccountInfo endpoint and reloads the user', async () => { + const set = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [ + {localId: 'new-uid-to-prove-refresh-got-called'}, + ]}); + + await updateEmail(user, 'hello@test.com'); + expect(set.calls[0].request).to.eql({ + idToken: 'access-token', + email: 'hello@test.com', + }); + + expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called'); }); - user.providerData = [{...PASSWORD_PROVIDER}]; + }); + + describe('#updatePassword', () => { + it('calls the setAccountInfo endpoint and reloads the user', async () => { + const set = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [ + {localId: 'new-uid-to-prove-refresh-got-called'}, + ]}); + + await updatePassword(user, 'pass'); + expect(set.calls[0].request).to.eql({ + idToken: 'access-token', + password: 'pass', + }); - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); - const provider = user.providerData[0]; - expect(provider.displayName).to.eq('response-name'); - expect(provider.photoURL).to.eq('response-photo'); + expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called'); + }); }); describe('notifications', () => { - beforeEach(() => { - user.auth.currentUser = user; + let auth: Auth; + let idTokenChange: sinon.SinonStub; + + beforeEach(async () => { + auth = user.auth; + idTokenChange = sinon.stub(); + auth.onIdTokenChanged(idTokenChange); + + // Flush token change promises which are floating + await auth.updateCurrentUser(user); + auth._isInitialized = true; + idTokenChange.resetHistory(); }); - it('triggers a token update if necessary', async () => { - mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { - idToken: 'new-id-token', - refreshToken: 'new-refresh-token', - expiresIn: 300, + describe('#updateProfile', () => { + it('triggers a token update if necessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'new-id-token', + refreshToken: 'new-refresh-token', + expiresIn: 300, + }); + + await updateProfile(user, {displayName: 'd'}); + expect(idTokenChange).to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); }); - const notifySpy = sinon.stub(user.auth, '_notifyStateListeners'); - await updateProfile(user, {displayName: 'd'}); - expect(notifySpy).to.have.been.called; + it('does NOT trigger a token update if unnecessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'access-token', + refreshToken: 'refresh-token', + expiresIn: 300, + }); + + await updateProfile(user, {displayName: 'd'}); + expect(idTokenChange).not.to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + }); }); - it('does NOT trigger a token update if unnecessary', async () => { - mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { - idToken: 'access-token', - refreshToken: 'refresh-token', - expiresIn: 300, + describe('#updateEmail', () => { + beforeEach(() => { + // This is necessary because this method calls reload; we don't care about that though, + // for these tests we're looking at the change listeners + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [{}]}); }); - const notifySpy = sinon.stub(user.auth, '_notifyStateListeners'); - await updateProfile(user, {displayName: 'd'}); - expect(notifySpy).not.to.have.been.called; + it('triggers a token update if necessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'new-id-token', + refreshToken: 'new-refresh-token', + expiresIn: 300, + }); + + await updatePassword(user, 'email@test.com'); + expect(idTokenChange).to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + }); + + it('does NOT trigger a token update if unnecessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'access-token', + refreshToken: 'refresh-token', + expiresIn: 300, + }); + + await updateEmail(user, 'email@test.com'); + expect(idTokenChange).not.to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + }); + }); + + describe('#updatePassword', () => { + beforeEach(() => { + // This is necessary because this method calls reload; we don't care about that though, + // for these tests we're looking at the change listeners + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [{}]}); + }); + + it('triggers a token update if necessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'new-id-token', + refreshToken: 'new-refresh-token', + expiresIn: 300, + }); + + await updatePassword(user, 'pass'); + expect(idTokenChange).to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + }); + + it('does NOT trigger a token update if unnecessary', async () => { + mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { + idToken: 'access-token', + refreshToken: 'refresh-token', + expiresIn: 300, + }); + + await updatePassword(user, 'pass'); + expect(idTokenChange).not.to.have.been.called; + expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + }); }); }); }); \ No newline at end of file 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 96fe50e0d3b..29156d089fd 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,7 @@ import * as externs from '@firebase/auth-types-exp'; import { - updateEmailPassword as apiUpdateEmailPassword + 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'; @@ -36,6 +36,7 @@ export async function updateProfile(externUser: externs.User, {displayName, phot } 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); @@ -50,18 +51,42 @@ export async function updateProfile(externUser: externs.User, {displayName, phot passwordProvider.photoURL = user.photoURL; } - user._updateTokensIfNecessary(response); - return user.auth._persistAndNotifyIfCurrent(user); + const tokensRefreshed = user._updateTokensIfNecessary(response); + await auth._persistUserIfCurrent(user); + if (tokensRefreshed) { + auth._notifyListenersIfCurrent(user); + } +} + +export function updateEmail(externUser: externs.User, newEmail: string): Promise { + const user = externUser as User; + return updateEmailOrPassword(user, newEmail, null); } -export async function updateEmail(externUser: externs.User, newEmail: string): Promise { +export function updatePassword(externUser: externs.User, newPassword: string): Promise { const user = externUser as User; + return updateEmailOrPassword(user, null, newPassword); +} + +async function updateEmailOrPassword(user: User, email: string|null, password: string|null): Promise { const {auth} = user; const idToken = await user.getIdToken(); - const response = await apiUpdateEmailPassword(auth, {idToken, email: newEmail}); - user._updateTokensIfNecessary(response); + const request: UpdateEmailPasswordRequest = {idToken}; + + if (email) { + request.email = email; + } + + if (password) { + request.password = password; + } - // To update + const response = await apiUpdateEmailPassword(auth, request); + + const tokensRefreshed = user._updateTokensIfNecessary(response); await _reloadWithoutSaving(user); - return auth._persistAndNotifyIfCurrent(user); -} + await auth._persistUserIfCurrent(user); + if (tokensRefreshed) { + auth._notifyListenersIfCurrent(user); + } +} \ No newline at end of file diff --git a/packages-exp/auth-exp/src/core/user/reload.test.ts b/packages-exp/auth-exp/src/core/user/reload.test.ts index 151c008f473..7330119c8c0 100644 --- a/packages-exp/auth-exp/src/core/user/reload.test.ts +++ b/packages-exp/auth-exp/src/core/user/reload.test.ts @@ -17,6 +17,7 @@ import { expect, use } from 'chai'; import * as chaiAsPromised from 'chai-as-promised'; +import * as sinon from 'sinon'; import * as sinonChai from 'sinon-chai'; import { ProviderId, UserInfo } from '@firebase/auth-types-exp'; @@ -25,10 +26,7 @@ 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 { - APIUserInfo, - ProviderUserInfo -} from '../../api/account_management/account'; +import { APIUserInfo, ProviderUserInfo } from '../../api/account_management/account'; import { _reloadWithoutSaving, reload } from './reload'; use(chaiAsPromised); @@ -150,13 +148,19 @@ describe('core/user/reload', () => { ]); }); - it('reload triggers a persistence change after completion', async () => { + it('reload persists the object and notifies listeners', async () => { mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { users: [{}] }); const user = testUser(auth, 'user', '', true); + user.auth.currentUser = user; + + const cb = sinon.stub(); + user.auth.onIdTokenChanged(cb); + await reload(user); + expect(cb).to.have.been.calledWith(user); expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); }); diff --git a/packages-exp/auth-exp/src/core/user/reload.ts b/packages-exp/auth-exp/src/core/user/reload.ts index 8dda4b3a66e..18af5895d45 100644 --- a/packages-exp/auth-exp/src/core/user/reload.ts +++ b/packages-exp/auth-exp/src/core/user/reload.ts @@ -57,7 +57,8 @@ export async function reload(externUser: externs.User): Promise { // Even though the current user hasn't changed, update // current user will trigger a persistence update w/ the // new info. - return user.auth._persistAndNotifyIfCurrent(user); + await user.auth._persistUserIfCurrent(user); + user.auth._notifyListenersIfCurrent(user); } function mergeProviderData( 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 3cee431bac8..021d6b7a67c 100644 --- a/packages-exp/auth-exp/src/core/user/user_impl.ts +++ b/packages-exp/auth-exp/src/core/user/user_impl.ts @@ -83,7 +83,8 @@ export class UserImpl implements User { const { accessToken, wasRefreshed } = tokens; if (wasRefreshed) { - await this.auth._persistAndNotifyIfCurrent(this); + await this.auth._persistUserIfCurrent(this); + this.auth._notifyListenersIfCurrent(this); } return accessToken; @@ -97,10 +98,13 @@ export class UserImpl implements User { return reload(this); } - _updateTokensIfNecessary(response: IdTokenResponse): void { + _updateTokensIfNecessary(response: IdTokenResponse): boolean { if (response.idToken && response.idToken !== this.stsTokenManager.accessToken) { this.stsTokenManager.updateFromServerResponse(response); + return true; } + + return false; } async delete(): Promise { diff --git a/packages-exp/auth-exp/src/model/auth.d.ts b/packages-exp/auth-exp/src/model/auth.d.ts index 37434aca1e5..9e4366ab964 100644 --- a/packages-exp/auth-exp/src/model/auth.d.ts +++ b/packages-exp/auth-exp/src/model/auth.d.ts @@ -51,8 +51,8 @@ export interface Auth extends externs.Auth { error?: ErrorFn, completed?: CompleteFn ): Unsubscribe; - _notifyStateListeners(): void; - _persistAndNotifyIfCurrent(user: User): Promise; + _notifyListenersIfCurrent(user: User): void; + _persistUserIfCurrent(user: User): Promise; } export interface Dependencies { diff --git a/packages-exp/auth-exp/src/model/user.d.ts b/packages-exp/auth-exp/src/model/user.d.ts index c80fbe2a4b9..252fa167c35 100644 --- a/packages-exp/auth-exp/src/model/user.d.ts +++ b/packages-exp/auth-exp/src/model/user.d.ts @@ -39,7 +39,7 @@ export interface User extends externs.User { tenantId: string | null; providerData: ModifiableUserInfo[]; metadata: externs.UserMetadata; - _updateTokensIfNecessary(response: IdTokenResponse): void; + _updateTokensIfNecessary(response: IdTokenResponse): boolean; getIdToken(forceRefresh?: boolean): Promise; getIdTokenResult(forceRefresh?: boolean): Promise; From 56456607e4db83072f77e5cdf2f5b6e43fd324d8 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 11:20:01 -0700 Subject: [PATCH 03/11] Fixing up tests --- packages-exp/auth-exp/src/core/user/account_info.test.ts | 3 +-- packages-exp/auth-exp/src/core/user/account_info.ts | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts index 1afd652d99d..242e0a0eb16 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.test.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -20,7 +20,7 @@ import * as chaiAsPromised from 'chai-as-promised'; import * as sinon from 'sinon'; import * as sinonChai from 'sinon-chai'; -import { UserInfo } from '@firebase/auth-types-exp'; +import { ProviderId, UserInfo } from '@firebase/auth-types-exp'; // import { UserInfo } from '@firebase/auth-types-exp'; import { mockEndpoint } from '../../../test/api/helper'; @@ -29,7 +29,6 @@ import * as fetch from '../../../test/mock_fetch'; import { Endpoint } from '../../api'; import { Auth } from '../../model/auth'; import { User } from '../../model/user'; -import { ProviderId } from '../providers'; import { updateEmail, updatePassword, updateProfile } from './account_info'; use(chaiAsPromised); 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 29156d089fd..17415096465 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.ts @@ -22,7 +22,6 @@ import { } from '../../api/account_management/email_and_password'; import { updateProfile as apiUpdateProfile } from '../../api/account_management/profile'; import { User } from '../../model/user'; -import { ProviderId } from '../providers'; import { _reloadWithoutSaving } from './reload'; interface Profile { @@ -45,7 +44,7 @@ export async function updateProfile(externUser: externs.User, {displayName, phot user.photoURL = response.photoUrl || null; // Update the password provider as well - const passwordProvider = user.providerData.find(p => p.providerId === ProviderId.PASSWORD); + const passwordProvider = user.providerData.find(p => p.providerId === externs.ProviderId.PASSWORD); if (passwordProvider) { passwordProvider.displayName = user.displayName; passwordProvider.photoURL = user.photoURL; From 1918a18ee6e8f6915cb4867741d1c0d2a9bd7aae Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 15:55:07 -0700 Subject: [PATCH 04/11] Update tests further --- .../src/core/user/account_info.test.ts | 23 +++++++++---------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts index 242e0a0eb16..e2db88be572 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.test.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -24,10 +24,9 @@ import { ProviderId, UserInfo } from '@firebase/auth-types-exp'; // import { UserInfo } from '@firebase/auth-types-exp'; import { mockEndpoint } from '../../../test/api/helper'; -import { testPersistence, testUser } from '../../../test/mock_auth'; +import { TestAuth, testAuth, testUser } from '../../../test/mock_auth'; import * as fetch from '../../../test/mock_fetch'; import { Endpoint } from '../../api'; -import { Auth } from '../../model/auth'; import { User } from '../../model/user'; import { updateEmail, updatePassword, updateProfile } from './account_info'; @@ -45,9 +44,11 @@ const PASSWORD_PROVIDER: UserInfo = { describe('core/user/profile', () => { let user: User; + let auth: TestAuth; - beforeEach(() => { - user = testUser('uid', '', true); + beforeEach(async () => { + auth = await testAuth(); + user = testUser(auth, 'uid', '', true); fetch.setUp(); }); @@ -134,11 +135,9 @@ describe('core/user/profile', () => { }); describe('notifications', () => { - let auth: Auth; let idTokenChange: sinon.SinonStub; beforeEach(async () => { - auth = user.auth; idTokenChange = sinon.stub(); auth.onIdTokenChanged(idTokenChange); @@ -158,7 +157,7 @@ describe('core/user/profile', () => { await updateProfile(user, {displayName: 'd'}); expect(idTokenChange).to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); it('does NOT trigger a token update if unnecessary', async () => { @@ -170,7 +169,7 @@ describe('core/user/profile', () => { await updateProfile(user, {displayName: 'd'}); expect(idTokenChange).not.to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); }); @@ -190,7 +189,7 @@ describe('core/user/profile', () => { await updatePassword(user, 'email@test.com'); expect(idTokenChange).to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); it('does NOT trigger a token update if unnecessary', async () => { @@ -202,7 +201,7 @@ describe('core/user/profile', () => { await updateEmail(user, 'email@test.com'); expect(idTokenChange).not.to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); }); @@ -222,7 +221,7 @@ describe('core/user/profile', () => { await updatePassword(user, 'pass'); expect(idTokenChange).to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); it('does NOT trigger a token update if unnecessary', async () => { @@ -234,7 +233,7 @@ describe('core/user/profile', () => { await updatePassword(user, 'pass'); expect(idTokenChange).not.to.have.been.called; - expect(testPersistence.lastPersistedBlob).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); }); }); }); From 1ed60a10ec6d56aadf2be33db48eb0d0f379b6ad Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 15:59:12 -0700 Subject: [PATCH 05/11] Add re-exports for public functions --- packages-exp/auth-exp/src/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages-exp/auth-exp/src/index.ts b/packages-exp/auth-exp/src/index.ts index a6fcb0c329a..37c45638a17 100644 --- a/packages-exp/auth-exp/src/index.ts +++ b/packages-exp/auth-exp/src/index.ts @@ -27,6 +27,7 @@ export { inMemoryPersistence } from './core/persistence/in_memory'; export { indexedDBLocalPersistence } from './core/persistence/indexed_db'; // core/strategies +export { signInWithCredential } from './core/strategies/credential'; export { sendPasswordResetEmail, confirmPasswordReset, @@ -43,6 +44,7 @@ export { } from './core/strategies/email'; // core/user +export { updateProfile, updateEmail, updatePassword } from './core/user/account_info'; export { getIdToken, getIdTokenResult } from './core/user/id_token_result'; export { reload } from './core/user/reload'; From 847f6d872b40596ae0131dbb0e0400483d8c04b0 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 16:03:08 -0700 Subject: [PATCH 06/11] Fix up authCredential types to use public versions --- packages-exp/auth-exp/src/core/strategies/credential.ts | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/packages-exp/auth-exp/src/core/strategies/credential.ts b/packages-exp/auth-exp/src/core/strategies/credential.ts index e70380756f5..b19bf297ac4 100644 --- a/packages-exp/auth-exp/src/core/strategies/credential.ts +++ b/packages-exp/auth-exp/src/core/strategies/credential.ts @@ -15,16 +15,20 @@ * limitations under the License. */ +import * as externs from '@firebase/auth-types-exp'; import { OperationType, UserCredential } from '@firebase/auth-types-exp'; + import { Auth } from '../../model/auth'; import { AuthCredential } from '../../model/auth_credential'; import { User } from '../../model/user'; import { UserCredentialImpl } from '../user/user_credential_impl'; export async function signInWithCredential( - auth: Auth, - credential: AuthCredential + authExtern: externs.Auth, + credentialExtern: externs.AuthCredential ): Promise { + const auth = authExtern as Auth; + const credential = credentialExtern as AuthCredential; // TODO: handle mfa by wrapping with callApiWithMfaContext const response = await credential._getIdTokenResponse(auth); const userCredential = await UserCredentialImpl._fromIdTokenResponse( From 91e399858544c91d3d7a6d1113a3488c8aa7d628 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 16:03:43 -0700 Subject: [PATCH 07/11] Formatting --- .../src/core/user/account_info.test.ts | 87 ++++++++++++------- .../auth-exp/src/core/user/account_info.ts | 46 ++++++---- .../auth-exp/src/core/user/reload.test.ts | 5 +- packages-exp/auth-exp/src/core/user/reload.ts | 5 +- .../auth-exp/src/core/user/user_impl.ts | 5 +- packages-exp/auth-exp/src/index.ts | 6 +- 6 files changed, 102 insertions(+), 52 deletions(-) diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts index e2db88be572..223209b2088 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.test.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -65,23 +65,29 @@ describe('core/user/profile', () => { }); it('calls the setAccountInfo endpoint', async () => { - const ep =mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); + const ep = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + await updateProfile(user, { + displayName: 'displayname', + photoURL: 'photo' + }); expect(ep.calls[0].request).to.eql({ idToken: 'access-token', displayName: 'displayname', - photoUrl: 'photo', + photoUrl: 'photo' }); }); it('sets the fields on the user based on the response', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { displayName: 'response-name', - photoUrl: 'response-photo', + photoUrl: 'response-photo' }); - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + await updateProfile(user, { + displayName: 'displayname', + photoURL: 'photo' + }); expect(user.displayName).to.eq('response-name'); expect(user.photoURL).to.eq('response-photo'); }); @@ -89,11 +95,14 @@ describe('core/user/profile', () => { it('sets the fields on the password provider', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { displayName: 'response-name', - photoUrl: 'response-photo', + photoUrl: 'response-photo' }); - user.providerData = [{...PASSWORD_PROVIDER}]; + user.providerData = [{ ...PASSWORD_PROVIDER }]; - await updateProfile(user, {displayName: 'displayname', photoURL: 'photo'}); + await updateProfile(user, { + displayName: 'displayname', + photoURL: 'photo' + }); const provider = user.providerData[0]; expect(provider.displayName).to.eq('response-name'); expect(provider.photoURL).to.eq('response-photo'); @@ -103,14 +112,14 @@ describe('core/user/profile', () => { describe('#updateEmail', () => { it('calls the setAccountInfo endpoint and reloads the user', async () => { const set = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); - mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [ - {localId: 'new-uid-to-prove-refresh-got-called'}, - ]}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [{ localId: 'new-uid-to-prove-refresh-got-called' }] + }); await updateEmail(user, 'hello@test.com'); expect(set.calls[0].request).to.eql({ idToken: 'access-token', - email: 'hello@test.com', + email: 'hello@test.com' }); expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called'); @@ -120,14 +129,14 @@ describe('core/user/profile', () => { describe('#updatePassword', () => { it('calls the setAccountInfo endpoint and reloads the user', async () => { const set = mockEndpoint(Endpoint.SET_ACCOUNT_INFO, {}); - mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [ - {localId: 'new-uid-to-prove-refresh-got-called'}, - ]}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { + users: [{ localId: 'new-uid-to-prove-refresh-got-called' }] + }); await updatePassword(user, 'pass'); expect(set.calls[0].request).to.eql({ idToken: 'access-token', - password: 'pass', + password: 'pass' }); expect(user.uid).to.eq('new-uid-to-prove-refresh-got-called'); @@ -152,24 +161,28 @@ describe('core/user/profile', () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'new-id-token', refreshToken: 'new-refresh-token', - expiresIn: 300, + expiresIn: 300 }); - await updateProfile(user, {displayName: 'd'}); + await updateProfile(user, { displayName: 'd' }); expect(idTokenChange).to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); it('does NOT trigger a token update if unnecessary', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'access-token', refreshToken: 'refresh-token', - expiresIn: 300, + expiresIn: 300 }); - await updateProfile(user, {displayName: 'd'}); + await updateProfile(user, { displayName: 'd' }); expect(idTokenChange).not.to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); }); @@ -177,31 +190,35 @@ describe('core/user/profile', () => { beforeEach(() => { // This is necessary because this method calls reload; we don't care about that though, // for these tests we're looking at the change listeners - mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [{}]}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { users: [{}] }); }); it('triggers a token update if necessary', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'new-id-token', refreshToken: 'new-refresh-token', - expiresIn: 300, + expiresIn: 300 }); await updatePassword(user, 'email@test.com'); expect(idTokenChange).to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); it('does NOT trigger a token update if unnecessary', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'access-token', refreshToken: 'refresh-token', - expiresIn: 300, + expiresIn: 300 }); await updateEmail(user, 'email@test.com'); expect(idTokenChange).not.to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); }); @@ -209,32 +226,36 @@ describe('core/user/profile', () => { beforeEach(() => { // This is necessary because this method calls reload; we don't care about that though, // for these tests we're looking at the change listeners - mockEndpoint(Endpoint.GET_ACCOUNT_INFO, {users: [{}]}); + mockEndpoint(Endpoint.GET_ACCOUNT_INFO, { users: [{}] }); }); it('triggers a token update if necessary', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'new-id-token', refreshToken: 'new-refresh-token', - expiresIn: 300, + expiresIn: 300 }); await updatePassword(user, 'pass'); expect(idTokenChange).to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); it('does NOT trigger a token update if unnecessary', async () => { mockEndpoint(Endpoint.SET_ACCOUNT_INFO, { idToken: 'access-token', refreshToken: 'refresh-token', - expiresIn: 300, + expiresIn: 300 }); await updatePassword(user, 'pass'); expect(idTokenChange).not.to.have.been.called; - expect(auth.persistenceLayer.lastObjectSet).to.eql(user.toPlainObject()); + expect(auth.persistenceLayer.lastObjectSet).to.eql( + user.toPlainObject() + ); }); }); }); -}); \ No newline at end of file +}); 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 17415096465..0b3a222495b 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.ts @@ -18,33 +18,39 @@ 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'; import { _reloadWithoutSaving } from './reload'; interface Profile { - displayName?: string|null; - photoURL?: string|null; + displayName?: string | null; + photoURL?: string | null; } -export async function updateProfile(externUser: externs.User, {displayName, photoURL: photoUrl}: Profile): Promise { +export async function updateProfile( + externUser: externs.User, + { displayName, photoURL: photoUrl }: Profile +): Promise { if (displayName === undefined && photoUrl === undefined) { return; } const user = externUser as User; - const {auth} = user; + const { auth } = user; const idToken = await user.getIdToken(); - const profileRequest = {idToken, displayName, photoUrl}; + const profileRequest = { idToken, displayName, photoUrl }; const response = await apiUpdateProfile(user.auth, profileRequest); user.displayName = response.displayName || null; user.photoURL = response.photoUrl || null; // Update the password provider as well - const passwordProvider = user.providerData.find(p => p.providerId === externs.ProviderId.PASSWORD); + const passwordProvider = user.providerData.find( + p => p.providerId === externs.ProviderId.PASSWORD + ); if (passwordProvider) { passwordProvider.displayName = user.displayName; passwordProvider.photoURL = user.photoURL; @@ -57,35 +63,45 @@ export async function updateProfile(externUser: externs.User, {displayName, phot } } -export function updateEmail(externUser: externs.User, newEmail: string): Promise { +export function updateEmail( + externUser: externs.User, + newEmail: string +): Promise { const user = externUser as User; return updateEmailOrPassword(user, newEmail, null); } -export function updatePassword(externUser: externs.User, newPassword: string): Promise { +export function updatePassword( + externUser: externs.User, + newPassword: string +): Promise { const user = externUser as User; return updateEmailOrPassword(user, null, newPassword); } -async function updateEmailOrPassword(user: User, email: string|null, password: string|null): Promise { - const {auth} = user; +async function updateEmailOrPassword( + user: User, + email: string | null, + password: string | null +): Promise { + const { auth } = user; const idToken = await user.getIdToken(); - const request: UpdateEmailPasswordRequest = {idToken}; + const request: UpdateEmailPasswordRequest = { idToken }; if (email) { request.email = email; } - + if (password) { request.password = password; } const response = await apiUpdateEmailPassword(auth, request); - + const tokensRefreshed = user._updateTokensIfNecessary(response); await _reloadWithoutSaving(user); await auth._persistUserIfCurrent(user); if (tokensRefreshed) { auth._notifyListenersIfCurrent(user); } -} \ No newline at end of file +} diff --git a/packages-exp/auth-exp/src/core/user/reload.test.ts b/packages-exp/auth-exp/src/core/user/reload.test.ts index 7330119c8c0..ca3333b3785 100644 --- a/packages-exp/auth-exp/src/core/user/reload.test.ts +++ b/packages-exp/auth-exp/src/core/user/reload.test.ts @@ -26,7 +26,10 @@ 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 { APIUserInfo, ProviderUserInfo } from '../../api/account_management/account'; +import { + APIUserInfo, + ProviderUserInfo +} from '../../api/account_management/account'; import { _reloadWithoutSaving, reload } from './reload'; use(chaiAsPromised); diff --git a/packages-exp/auth-exp/src/core/user/reload.ts b/packages-exp/auth-exp/src/core/user/reload.ts index 18af5895d45..e0fec052988 100644 --- a/packages-exp/auth-exp/src/core/user/reload.ts +++ b/packages-exp/auth-exp/src/core/user/reload.ts @@ -17,7 +17,10 @@ import * as externs from '@firebase/auth-types-exp'; -import { getAccountInfo, ProviderUserInfo } from '../../api/account_management/account'; +import { + getAccountInfo, + ProviderUserInfo +} from '../../api/account_management/account'; import { User } from '../../model/user'; import { assert } from '../util/assert'; 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 021d6b7a67c..ffec2605b47 100644 --- a/packages-exp/auth-exp/src/core/user/user_impl.ts +++ b/packages-exp/auth-exp/src/core/user/user_impl.ts @@ -99,7 +99,10 @@ export class UserImpl implements User { } _updateTokensIfNecessary(response: IdTokenResponse): boolean { - if (response.idToken && response.idToken !== this.stsTokenManager.accessToken) { + if ( + response.idToken && + response.idToken !== this.stsTokenManager.accessToken + ) { this.stsTokenManager.updateFromServerResponse(response); return true; } diff --git a/packages-exp/auth-exp/src/index.ts b/packages-exp/auth-exp/src/index.ts index 37c45638a17..14d28b3c47e 100644 --- a/packages-exp/auth-exp/src/index.ts +++ b/packages-exp/auth-exp/src/index.ts @@ -44,7 +44,11 @@ export { } from './core/strategies/email'; // core/user -export { updateProfile, updateEmail, updatePassword } from './core/user/account_info'; +export { + updateProfile, + updateEmail, + updatePassword +} from './core/user/account_info'; export { getIdToken, getIdTokenResult } from './core/user/id_token_result'; export { reload } from './core/user/reload'; From f22451f8d22f36c6f1c5c67664f2b1144b0ec452 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 16:08:05 -0700 Subject: [PATCH 08/11] Code cleanup --- packages-exp/auth-exp/src/core/user/account_info.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 0b3a222495b..742834eb71f 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'; @@ -49,7 +48,7 @@ export async function updateProfile( // Update the password provider as well const passwordProvider = user.providerData.find( - p => p.providerId === externs.ProviderId.PASSWORD + ({providerId}) => providerId === externs.ProviderId.PASSWORD ); if (passwordProvider) { passwordProvider.displayName = user.displayName; From 366a6de8a927eaa1a70a5f764996f8fe09238330 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Thu, 28 May 2020 16:08:29 -0700 Subject: [PATCH 09/11] Formatting --- packages-exp/auth-exp/src/core/user/account_info.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) 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 742834eb71f..5c7d01c93c8 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'; @@ -48,7 +49,7 @@ export async function updateProfile( // Update the password provider as well const passwordProvider = user.providerData.find( - ({providerId}) => providerId === externs.ProviderId.PASSWORD + ({ providerId }) => providerId === externs.ProviderId.PASSWORD ); if (passwordProvider) { passwordProvider.displayName = user.displayName; From d948ca342b5939cac8b6988670aa861c542ee745 Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 29 May 2020 11:53:12 -0700 Subject: [PATCH 10/11] PR feedback --- packages-exp/auth-exp/src/core/user/account_info.test.ts | 1 - packages-exp/auth-exp/src/core/user/reload.ts | 5 +---- packages-exp/auth-exp/src/model/user.d.ts | 4 ++-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/packages-exp/auth-exp/src/core/user/account_info.test.ts b/packages-exp/auth-exp/src/core/user/account_info.test.ts index 223209b2088..ae1e989a389 100644 --- a/packages-exp/auth-exp/src/core/user/account_info.test.ts +++ b/packages-exp/auth-exp/src/core/user/account_info.test.ts @@ -22,7 +22,6 @@ import * as sinonChai from 'sinon-chai'; import { ProviderId, UserInfo } from '@firebase/auth-types-exp'; -// import { UserInfo } from '@firebase/auth-types-exp'; import { mockEndpoint } from '../../../test/api/helper'; import { TestAuth, testAuth, testUser } from '../../../test/mock_auth'; import * as fetch from '../../../test/mock_fetch'; diff --git a/packages-exp/auth-exp/src/core/user/reload.ts b/packages-exp/auth-exp/src/core/user/reload.ts index e0fec052988..18af5895d45 100644 --- a/packages-exp/auth-exp/src/core/user/reload.ts +++ b/packages-exp/auth-exp/src/core/user/reload.ts @@ -17,10 +17,7 @@ import * as externs from '@firebase/auth-types-exp'; -import { - getAccountInfo, - ProviderUserInfo -} from '../../api/account_management/account'; +import { getAccountInfo, ProviderUserInfo } from '../../api/account_management/account'; import { User } from '../../model/user'; import { assert } from '../util/assert'; diff --git a/packages-exp/auth-exp/src/model/user.d.ts b/packages-exp/auth-exp/src/model/user.d.ts index 252fa167c35..72596d73afe 100644 --- a/packages-exp/auth-exp/src/model/user.d.ts +++ b/packages-exp/auth-exp/src/model/user.d.ts @@ -21,7 +21,7 @@ import { PersistedBlob } from '../core/persistence'; import { Auth } from './auth'; import { IdTokenResponse } from './id_token'; -type ModifiableUserInfo = { +type MutableUserInfo = { -readonly [K in keyof externs.UserInfo]: externs.UserInfo[K]; }; @@ -37,7 +37,7 @@ export interface User extends externs.User { refreshToken: string; emailVerified: boolean; tenantId: string | null; - providerData: ModifiableUserInfo[]; + providerData: MutableUserInfo[]; metadata: externs.UserMetadata; _updateTokensIfNecessary(response: IdTokenResponse): boolean; From 6e22793fb02cfedbc338cbe522388dcda33e52de Mon Sep 17 00:00:00 2001 From: Sam Olsen Date: Fri, 29 May 2020 11:53:30 -0700 Subject: [PATCH 11/11] Formatting --- packages-exp/auth-exp/src/core/user/reload.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages-exp/auth-exp/src/core/user/reload.ts b/packages-exp/auth-exp/src/core/user/reload.ts index 18af5895d45..e0fec052988 100644 --- a/packages-exp/auth-exp/src/core/user/reload.ts +++ b/packages-exp/auth-exp/src/core/user/reload.ts @@ -17,7 +17,10 @@ import * as externs from '@firebase/auth-types-exp'; -import { getAccountInfo, ProviderUserInfo } from '../../api/account_management/account'; +import { + getAccountInfo, + ProviderUserInfo +} from '../../api/account_management/account'; import { User } from '../../model/user'; import { assert } from '../util/assert';