diff --git a/.changeset/dry-toes-hammer.md b/.changeset/dry-toes-hammer.md new file mode 100644 index 00000000000..c4526f66413 --- /dev/null +++ b/.changeset/dry-toes-hammer.md @@ -0,0 +1,6 @@ +--- +"@firebase/auth-compat": minor +"@firebase/auth": minor +--- + +Fix behavior on subsequent calls to `getRedirectResult()` diff --git a/packages/auth-compat/test/integration/webdriver/static/redirect.js b/packages/auth-compat/test/integration/webdriver/static/redirect.js index 4335b0b86c6..a3c23921748 100644 --- a/packages/auth-compat/test/integration/webdriver/static/redirect.js +++ b/packages/auth-compat/test/integration/webdriver/static/redirect.js @@ -39,8 +39,14 @@ export function idpLinkRedirect() { .currentUser.linkWithRedirect(new compat.auth.GoogleAuthProvider()); } -export function redirectResult() { - return compat.auth().getRedirectResult(); +export async function redirectResult() { + const result = await compat.auth().getRedirectResult(); + if (result.user === null && result.credential === null) { + // In the new SDK (and consequently the tests), null is returned instead of + // a credential with a null user/cred + return null; + } + return result; } export async function generateCredentialFromRedirectResultAndStore() { diff --git a/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts b/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts index 477b63919de..2e548f1e235 100644 --- a/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts +++ b/packages/auth/src/core/strategies/abstract_popup_redirect_operation.ts @@ -58,7 +58,7 @@ export abstract class AbstractPopupRedirectOperation filter: AuthEventType | AuthEventType[], protected readonly resolver: PopupRedirectResolverInternal, protected user?: UserInternal, - private readonly bypassAuthState = false + protected readonly bypassAuthState = false ) { this.filter = Array.isArray(filter) ? filter : [filter]; } diff --git a/packages/auth/src/core/strategies/redirect.test.ts b/packages/auth/src/core/strategies/redirect.test.ts index 0b94ae8bfd0..2da34a84e1f 100644 --- a/packages/auth/src/core/strategies/redirect.test.ts +++ b/packages/auth/src/core/strategies/redirect.test.ts @@ -116,7 +116,7 @@ describe('core/strategies/redirect', () => { expect(await promise).to.eq(cred); }); - it('returns the same value if called multiple times', async () => { + it('returns null after the first call', async () => { const cred = new UserCredentialImpl({ user: testUser(auth, 'uid'), providerId: ProviderId.GOOGLE, @@ -128,7 +128,7 @@ describe('core/strategies/redirect', () => { type: AuthEventType.SIGN_IN_VIA_REDIRECT }); expect(await promise).to.eq(cred); - expect(await redirectAction.execute()).to.eq(cred); + expect(await redirectAction.execute()).to.be.null; }); it('interacts with redirectUser loading from auth object', async () => { @@ -192,6 +192,8 @@ describe('core/strategies/redirect', () => { type: AuthEventType.REAUTH_VIA_REDIRECT }); expect(await promise).to.eq(cred); + + // In this case, bypassAuthState is true... The value won't be cleared expect(await redirectAction.execute()).to.eq(cred); }); diff --git a/packages/auth/src/core/strategies/redirect.ts b/packages/auth/src/core/strategies/redirect.ts index 0794d6576dc..032b6d71181 100644 --- a/packages/auth/src/core/strategies/redirect.ts +++ b/packages/auth/src/core/strategies/redirect.ts @@ -79,6 +79,12 @@ export class RedirectAction extends AbstractPopupRedirectOperation { redirectOutcomeMap.set(this.auth._key(), readyOutcome); } + // If we're not bypassing auth state, the ready outcome should be set to + // null. + if (!this.bypassAuthState) { + redirectOutcomeMap.set(this.auth._key(), () => Promise.resolve(null)); + } + return readyOutcome(); } diff --git a/packages/auth/src/platform_browser/strategies/redirect.test.ts b/packages/auth/src/platform_browser/strategies/redirect.test.ts index c722a231ceb..aa02d6d982a 100644 --- a/packages/auth/src/platform_browser/strategies/redirect.test.ts +++ b/packages/auth/src/platform_browser/strategies/redirect.test.ts @@ -329,7 +329,7 @@ describe('platform_browser/strategies/redirect', () => { expect(await promise).to.eq(cred); }); - it('returns the same value if called multiple times', async () => { + it('returns null after the first call', async () => { const cred = new UserCredentialImpl({ user: testUser(auth, 'uid'), providerId: ProviderId.GOOGLE, @@ -341,7 +341,7 @@ describe('platform_browser/strategies/redirect', () => { type: AuthEventType.SIGN_IN_VIA_REDIRECT }); expect(await promise).to.eq(cred); - expect(await getRedirectResult(auth, resolver)).to.eq(cred); + expect(await getRedirectResult(auth, resolver)).to.be.null; }); it('interacts with redirectUser loading from auth object', async () => { @@ -405,7 +405,7 @@ describe('platform_browser/strategies/redirect', () => { type: AuthEventType.REAUTH_VIA_REDIRECT }); expect(await promise).to.eq(cred); - expect(await getRedirectResult(auth, resolver)).to.eq(cred); + expect(await getRedirectResult(auth, resolver)).to.be.null; }); it('removes the redirect user and clears eventId from currentuser', async () => { diff --git a/packages/auth/test/integration/webdriver/redirect.test.ts b/packages/auth/test/integration/webdriver/redirect.test.ts index 4124241d630..bdbc965a06a 100644 --- a/packages/auth/test/integration/webdriver/redirect.test.ts +++ b/packages/auth/test/integration/webdriver/redirect.test.ts @@ -64,6 +64,10 @@ browserDescribe('WebDriver redirect IdP test', driver => { ); expect(redirectResult.operationType).to.eq(OperationType.SIGN_IN); expect(redirectResult.user).to.eql(currentUser); + + // After the first call to redirect result, redirect result should be + // null + expect(await driver.call(RedirectFunction.REDIRECT_RESULT)).to.be.null; }); it('can link with another account account', async () => {