[Auth] Fix getRedirectResult behavior (#5617)

* Fix getRedirectResult behavior * Fix compat test * Add changeset

Author: sam-gc

.changeset/dry-toes-hammer.md

@@ -0,0 +1,6 @@
+---
+"@firebase/auth-compat": minor
+"@firebase/auth": minor
+---
+
+Fix behavior on subsequent calls to `getRedirectResult()`

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() {

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];
  }

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);
  });
 

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();
  }
 

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 () => {

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 () => {