Merge pull request #1406 from firebase/auth-1391

fix(auth): ensure pending credential is rehydrated before linkWithCredential

Author: russellwheatley

packages/core/src/auth.test.ts

@@ -48,6 +48,12 @@ vi.mock("firebase/auth", () => ({
   PhoneAuthProvider: Object.assign(vi.fn(), {
     credential: vi.fn(),
   }),
+  OAuthProvider: {
+    credentialFromJSON: vi.fn(),
+  },
+  SAMLAuthProvider: {
+    credentialFromJSON: vi.fn(),
+  },
   linkWithCredential: vi.fn(),
 }));
 
@@ -64,6 +70,9 @@ import {
   signInWithCredential as _signInWithCredential,
   EmailAuthProvider,
   PhoneAuthProvider,
+  OAuthProvider,
+  SAMLAuthProvider,
+  linkWithCredential as _linkWithCredential,
   createUserWithEmailAndPassword as _createUserWithEmailAndPassword,
   sendPasswordResetEmail as _sendPasswordResetEmail,
   sendSignInLinkToEmail as _sendSignInLinkToEmail,
@@ -81,8 +90,6 @@ import { FirebaseError } from "firebase/app";
 
 import { createMockUI } from "~/tests/utils";
 
-// TODO(ehesp): Add tests for handlePendingCredential.
-
 describe("signInWithEmailAndPassword", () => {
   beforeEach(() => {
     vi.clearAllMocks();
@@ -1007,6 +1014,131 @@ describe("signInAnonymously", () => {
   });
 });
 
+describe("handlePendingCredential", () => {
+  beforeEach(() => {
+    vi.clearAllMocks();
+    window.sessionStorage.clear();
+  });
+
+  afterEach(() => {
+    window.sessionStorage.clear();
+  });
+
+  const mockUserCredential = {
+    user: { uid: "anonymous-uid" },
+    providerId: "anonymous",
+    operationType: "signIn",
+  } as UserCredential;
+
+  it("should return the user unchanged when there is no pending credential", async () => {
+    const mockUI = createMockUI();
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(OAuthProvider.credentialFromJSON).not.toHaveBeenCalled();
+    expect(_linkWithCredential).not.toHaveBeenCalled();
+    expect(result).toBe(mockUserCredential);
+  });
+
+  it("should rehydrate an OAuth credential via OAuthProvider.credentialFromJSON and link it", async () => {
+    const mockUI = createMockUI();
+    const storedJSON = { providerId: "google.com", signInMethod: "google.com", idToken: "fake-id-token" };
+    window.sessionStorage.setItem("pendingCred", JSON.stringify(storedJSON));
+
+    const rehydratedCredential = { providerId: "google.com" } as any;
+    const linkedUserCredential = { ...mockUserCredential, providerId: "google.com" } as UserCredential;
+
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+    vi.mocked(OAuthProvider.credentialFromJSON).mockReturnValue(rehydratedCredential);
+    vi.mocked(_linkWithCredential).mockResolvedValue(linkedUserCredential);
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(OAuthProvider.credentialFromJSON).toHaveBeenCalledWith(storedJSON);
+    expect(_linkWithCredential).toHaveBeenCalledWith(mockUserCredential.user, rehydratedCredential);
+    expect(result).toBe(linkedUserCredential);
+    expect(window.sessionStorage.getItem("pendingCred")).toBeNull();
+  });
+
+  it("should fall back to SAMLAuthProvider when OAuthProvider cannot rehydrate the credential", async () => {
+    const mockUI = createMockUI();
+    const storedJSON = { providerId: "saml.my-provider", signInMethod: "saml.my-provider", pendingToken: "abc" };
+    window.sessionStorage.setItem("pendingCred", JSON.stringify(storedJSON));
+
+    const rehydratedCredential = { providerId: "saml.my-provider" } as any;
+    const linkedUserCredential = { ...mockUserCredential, providerId: "saml.my-provider" } as UserCredential;
+
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+    vi.mocked(OAuthProvider.credentialFromJSON).mockImplementation(() => {
+      throw new Error("not an OAuth credential");
+    });
+    vi.mocked(SAMLAuthProvider.credentialFromJSON).mockReturnValue(rehydratedCredential);
+    vi.mocked(_linkWithCredential).mockResolvedValue(linkedUserCredential);
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(SAMLAuthProvider.credentialFromJSON).toHaveBeenCalledWith(storedJSON);
+    expect(_linkWithCredential).toHaveBeenCalledWith(mockUserCredential.user, rehydratedCredential);
+    expect(result).toBe(linkedUserCredential);
+    expect(window.sessionStorage.getItem("pendingCred")).toBeNull();
+  });
+
+  it("should return the original user and clear storage when the credential cannot be rehydrated", async () => {
+    const mockUI = createMockUI();
+    const storedJSON = { providerId: "unknown", signInMethod: "unknown" };
+    window.sessionStorage.setItem("pendingCred", JSON.stringify(storedJSON));
+
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+    vi.mocked(OAuthProvider.credentialFromJSON).mockImplementation(() => {
+      throw new Error("not an OAuth credential");
+    });
+    vi.mocked(SAMLAuthProvider.credentialFromJSON).mockImplementation(() => {
+      throw new Error("not a SAML credential");
+    });
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(_linkWithCredential).not.toHaveBeenCalled();
+    expect(result).toBe(mockUserCredential);
+    expect(window.sessionStorage.getItem("pendingCred")).toBeNull();
+  });
+
+  it("should return the original user and clear storage when the stored credential is invalid JSON", async () => {
+    const mockUI = createMockUI();
+    window.sessionStorage.setItem("pendingCred", "{invalid-json");
+
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(OAuthProvider.credentialFromJSON).not.toHaveBeenCalled();
+    expect(_linkWithCredential).not.toHaveBeenCalled();
+    expect(result).toBe(mockUserCredential);
+    expect(window.sessionStorage.getItem("pendingCred")).toBeNull();
+  });
+
+  it("should return the original user and clear storage when linkWithCredential fails", async () => {
+    const mockUI = createMockUI();
+    const storedJSON = { providerId: "google.com", signInMethod: "google.com", idToken: "fake-id-token" };
+    window.sessionStorage.setItem("pendingCred", JSON.stringify(storedJSON));
+
+    const rehydratedCredential = { providerId: "google.com" } as any;
+
+    vi.mocked(_signInAnonymously).mockResolvedValue(mockUserCredential);
+    vi.mocked(OAuthProvider.credentialFromJSON).mockReturnValue(rehydratedCredential);
+    vi.mocked(_linkWithCredential).mockRejectedValue(
+      new FirebaseError("auth/credential-already-in-use", "Credential already in use")
+    );
+
+    const result = await signInAnonymously(mockUI);
+
+    expect(_linkWithCredential).toHaveBeenCalledWith(mockUserCredential.user, rehydratedCredential);
+    expect(result).toBe(mockUserCredential);
+    expect(window.sessionStorage.getItem("pendingCred")).toBeNull();
+  });
+});
+
 describe("signInWithCustomToken", () => {
   beforeEach(() => {
     vi.clearAllMocks();

packages/core/src/auth.ts

@@ -24,7 +24,9 @@ import {
   signInWithCustomToken as _signInWithCustomToken,
   EmailAuthProvider,
   linkWithCredential,
+  OAuthProvider,
   PhoneAuthProvider,
+  SAMLAuthProvider,
   TotpMultiFactorGenerator,
   multiFactor,
   type ActionCodeSettings,
@@ -44,17 +46,47 @@ import { hasBehavior, getBehavior } from "./behaviors/index";
 import { FirebaseError } from "firebase/app";
 import { getTranslation } from "./translations";
 
+/**
+ * Rehydrates a plain object (produced by `AuthCredential.toJSON()` and round-tripped
+ * through `JSON.stringify`/`JSON.parse`) back into a real `AuthCredential` instance.
+ *
+ * `linkWithCredential` requires an actual `AuthCredential` - a plain object lacks the
+ * internal methods (e.g. `_linkToIdToken`) that Firebase Auth relies on, so it must be
+ * rehydrated via the relevant provider's `credentialFromJSON` before use.
+ *
+ * @param json - The parsed JSON representation of the credential.
+ * @returns The rehydrated `AuthCredential`, or `null` if it could not be rehydrated.
+ */
+function credentialFromJSON(json: unknown): AuthCredential | null {
+  if (typeof json !== "object" || json === null) {
+    return null;
+  }
+
+  try {
+    return OAuthProvider.credentialFromJSON(json);
+  } catch {
+    // Not an OAuth credential - fall through and try other providers below.
+  }
+
+  try {
+    return SAMLAuthProvider.credentialFromJSON(json);
+  } catch {
+    return null;
+  }
+}
+
 async function handlePendingCredential(_ui: FirebaseUI, user: UserCredential): Promise<UserCredential> {
   const pendingCredString = window.sessionStorage.getItem("pendingCred");
   if (!pendingCredString) return user;
 
+  window.sessionStorage.removeItem("pendingCred");
+
   try {
-    const pendingCred = JSON.parse(pendingCredString);
-    const result = await linkWithCredential(user.user, pendingCred);
-    window.sessionStorage.removeItem("pendingCred");
-    return result;
+    const pendingCred = credentialFromJSON(JSON.parse(pendingCredString));
+    if (!pendingCred) return user;
+
+    return await linkWithCredential(user.user, pendingCred);
   } catch {
-    window.sessionStorage.removeItem("pendingCred");
     return user;
   }
 }

packages/core/src/errors.ts

@@ -52,6 +52,7 @@ export function handleFirebaseError(ui: FirebaseUI, error: unknown): never {
 
   // TODO(ehesp): Type error as unknown, check instance of FirebaseError
   // TODO(ehesp): Support via behavior
+  // Note: the credential is stored via `toJSON()`; it must be rehydrated (see handlePendingCredential in auth.ts) before being passed to linkWithCredential.
   if (error.code === "auth/account-exists-with-different-credential" && errorContainsCredential(error)) {
     window.sessionStorage.setItem("pendingCred", JSON.stringify(error.credential.toJSON()));
   }