fix: resolve race condition in Clerk auth signup flow to prevent duplicate login attempts

.changeset/clerk-auth-signup-race-fix.md

@@ -0,0 +1,6 @@
+---
+"jazz-tools": patch
+---
+
+Bugfix: fixed a race condition in Clerk auth where the signup flow could trigger a duplicate login attempt
+

CHANGELOG.md

@@ -1,5 +1,6 @@
 Released Jazz 0.19.16:
 - Improved sync timeout error messages to include known state, peer state, and any error information when waiting for sync times out
+- Bugfix: fixed a race condition in Clerk auth where the signup flow could trigger a duplicate login attempt
 
 Released Jazz 0.19.15:
 - Added a locking system for session IDs in React Native to make mounting multiple JazzProviders safer (still not advised as duplicate the data loading effort)

packages/jazz-tools/src/tools/auth/clerk/index.ts

@@ -53,18 +53,21 @@ export class JazzClerkAuth {
   }
 
   private isFirstCall = true;
+  private previousUser: Pick<
+    NonNullable<MinimalClerkClient["user"]>,
+    "unsafeMetadata"
+  > | null = null;
 
   registerListener(clerkClient: MinimalClerkClient) {
-    let previousUser: MinimalClerkClient["user"] | null =
-      clerkClient.user ?? null;
+    this.previousUser = clerkClient.user ?? null;
 
     // Need to use addListener because the clerk user object is not updated when the user logs in
     return clerkClient.addListener((event) => {
       const user = (event as Pick<MinimalClerkClient, "user">).user ?? null;
 
-      if (!isClerkAuthStateEqual(previousUser, user) || this.isFirstCall) {
+      if (!isClerkAuthStateEqual(this.previousUser, user) || this.isFirstCall) {
+        this.previousUser = user;
         this.onClerkUserChange({ user });
-        previousUser = user;
         this.isFirstCall = false;
       }
     });
@@ -137,13 +140,20 @@ export class JazzClerkAuth {
       ? Array.from(credentials.secretSeed)
       : undefined;
 
-    await clerkClient.user?.update({
-      unsafeMetadata: {
-        jazzAccountID: credentials.accountID,
-        jazzAccountSecret: credentials.accountSecret,
-        jazzAccountSeed,
-      } satisfies ClerkCredentials,
-    });
+    const clerkCredentials = {
+      jazzAccountID: credentials.accountID,
+      jazzAccountSecret: credentials.accountSecret,
+      jazzAccountSeed,
+    };
+    // user.update will cause the Clerk user change listener to fire; updating this.previousUser beforehand
+    // ensures the listener sees the new credentials and does not trigger an unnecessary logIn operation
+    this.previousUser = { unsafeMetadata: clerkCredentials };
+
+    if (clerkClient.user) {
+      await clerkClient.user.update({
+        unsafeMetadata: clerkCredentials,
+      });
+    }
 
     const currentAccount = await Account.getMe().$jazz.ensureLoaded({
       resolve: {

packages/jazz-tools/src/tools/auth/clerk/tests/JazzClerkAuth.test.ts

@@ -282,6 +282,83 @@ describe("JazzClerkAuth", () => {
 
       expect(onClerkUserChangeSpy).toHaveBeenCalledTimes(1);
     });
+
+    it("should complete signup flow when new Clerk user is detected", async () => {
+      // 1. Setup local credentials (simulating anonymous user)
+      await authSecretStorage.set({
+        accountID: "test-account-id" as ID<Account>,
+        secretSeed: new Uint8Array([1, 2, 3]),
+        accountSecret: "test-secret" as AgentSecret,
+        provider: "anonymous",
+      });
+
+      const { client, triggerUserChange } = setupMockClerk(null);
+
+      const auth = new JazzClerkAuth(
+        mockAuthenticate,
+        mockLogOut,
+        authSecretStorage,
+      );
+
+      // 2. Register listener with null user (no one logged in yet)
+      auth.registerListener(client);
+
+      // Initial trigger with no user
+      triggerUserChange(null);
+
+      // 3. Trigger event with new Clerk user (no Jazz credentials yet)
+      const mockUserUpdate = vi.fn((data) => {
+        triggerUserChange(data);
+      });
+
+      const signInSpy = vi.spyOn(auth, "signIn");
+      const logInSpy = vi.spyOn(auth, "logIn");
+
+      const newClerkUser = {
+        fullName: "Test User",
+        firstName: "Test",
+        lastName: "User",
+        username: "testuser",
+        id: "clerk-user-123",
+        primaryEmailAddress: { emailAddress: "[email protected]" },
+        unsafeMetadata: {}, // No Jazz credentials yet
+        update: mockUserUpdate,
+      };
+
+      triggerUserChange(newClerkUser);
+
+      // Wait for async operations to complete
+      await vi.waitFor(() => {
+        expect(mockUserUpdate).toHaveBeenCalled();
+      });
+
+      // 4. Verify credentials synced to Clerk
+      expect(mockUserUpdate).toHaveBeenCalledWith({
+        unsafeMetadata: {
+          jazzAccountID: "test-account-id",
+          jazzAccountSecret: "test-secret",
+          jazzAccountSeed: [1, 2, 3],
+        },
+      });
+
+      // Verify profile name was updated from Clerk username
+      const me = await Account.getMe().$jazz.ensureLoaded({
+        resolve: { profile: true },
+      });
+      expect(me.profile.name).toBe("Test User");
+
+      // Verify authSecretStorage is updated with provider "clerk"
+      const storedCredentials = await authSecretStorage.get();
+      expect(storedCredentials).toEqual({
+        accountID: "test-account-id",
+        accountSecret: "test-secret",
+        secretSeed: new Uint8Array([1, 2, 3]),
+        provider: "clerk",
+      });
+
+      expect(signInSpy).toHaveBeenCalled();
+      expect(logInSpy).not.toHaveBeenCalled();
+    });
   });
 
   describe("initializeAuth", () => {

packages/jazz-tools/src/tools/auth/clerk/tests/isClerkAuthStateEqual.test.ts

@@ -0,0 +1,124 @@
+import { describe, expect, it } from "vitest";
+import { isClerkAuthStateEqual } from "../types";
+
+describe("isClerkAuthStateEqual", () => {
+  const validCredentials = {
+    jazzAccountID: "account-123",
+    jazzAccountSecret: "secret-123",
+    jazzAccountSeed: [1, 2, 3],
+  };
+
+  const differentCredentials = {
+    jazzAccountID: "account-456",
+    jazzAccountSecret: "secret-456",
+    jazzAccountSeed: [4, 5, 6],
+  };
+
+  describe("both users null/undefined", () => {
+    it.each([
+      { previous: null, next: null, description: "both null" },
+      { previous: undefined, next: undefined, description: "both undefined" },
+      { previous: null, next: undefined, description: "null and undefined" },
+      { previous: undefined, next: null, description: "undefined and null" },
+    ])("returns true when $description", ({ previous, next }) => {
+      expect(isClerkAuthStateEqual(previous, next)).toBe(true);
+    });
+  });
+
+  describe("one user null, other exists", () => {
+    it.each([
+      {
+        previous: null,
+        next: { unsafeMetadata: validCredentials },
+        description: "previous null, next exists",
+      },
+      {
+        previous: { unsafeMetadata: validCredentials },
+        next: null,
+        description: "previous exists, next null",
+      },
+      {
+        previous: undefined,
+        next: { unsafeMetadata: validCredentials },
+        description: "previous undefined, next exists",
+      },
+      {
+        previous: { unsafeMetadata: validCredentials },
+        next: undefined,
+        description: "previous exists, next undefined",
+      },
+    ])("returns false when $description", ({ previous, next }) => {
+      expect(isClerkAuthStateEqual(previous, next)).toBe(false);
+    });
+  });
+
+  describe("same jazzAccountID", () => {
+    it("returns true when both users have the same jazzAccountID", () => {
+      const previous = { unsafeMetadata: validCredentials };
+      const next = {
+        unsafeMetadata: {
+          ...validCredentials,
+          jazzAccountSecret: "different-secret",
+        },
+      };
+      expect(isClerkAuthStateEqual(previous, next)).toBe(true);
+    });
+  });
+
+  describe("different jazzAccountID", () => {
+    it("returns false when users have different jazzAccountID", () => {
+      const previous = { unsafeMetadata: validCredentials };
+      const next = { unsafeMetadata: differentCredentials };
+      expect(isClerkAuthStateEqual(previous, next)).toBe(false);
+    });
+  });
+
+  describe("neither user has valid credentials", () => {
+    it.each([
+      {
+        previous: { unsafeMetadata: {} },
+        next: { unsafeMetadata: {} },
+        description: "both have empty metadata",
+      },
+      {
+        previous: { unsafeMetadata: { someOtherField: "value" } },
+        next: { unsafeMetadata: { anotherField: "value" } },
+        description: "both have non-credential metadata",
+      },
+      {
+        previous: { unsafeMetadata: { jazzAccountID: "123" } },
+        next: { unsafeMetadata: { jazzAccountSecret: "456" } },
+        description: "both have incomplete credentials",
+      },
+    ])("returns true when $description", ({ previous, next }) => {
+      expect(isClerkAuthStateEqual(previous, next)).toBe(true);
+    });
+  });
+
+  describe("one has credentials, other doesn't", () => {
+    it.each([
+      {
+        previous: { unsafeMetadata: validCredentials },
+        next: { unsafeMetadata: {} },
+        description: "previous has credentials, next empty",
+      },
+      {
+        previous: { unsafeMetadata: {} },
+        next: { unsafeMetadata: validCredentials },
+        description: "previous empty, next has credentials",
+      },
+      {
+        previous: { unsafeMetadata: validCredentials },
+        next: { unsafeMetadata: { jazzAccountID: "123" } },
+        description: "previous has credentials, next has incomplete",
+      },
+      {
+        previous: { unsafeMetadata: { jazzAccountSecret: "456" } },
+        next: { unsafeMetadata: validCredentials },
+        description: "previous has incomplete, next has credentials",
+      },
+    ])("returns false when $description", ({ previous, next }) => {
+      expect(isClerkAuthStateEqual(previous, next)).toBe(false);
+    });
+  });
+});

packages/jazz-tools/src/tools/auth/clerk/types.ts

@@ -36,21 +36,38 @@ export type ClerkCredentials = {
  * **Note**: It does not validate the credentials, only checks if the necessary fields are present in the metadata object.
  */
 export function isClerkCredentials(
-  data: NonNullable<MinimalClerkClient["user"]>["unsafeMetadata"] | undefined,
+  data:
+    | NonNullable<MinimalClerkClient["user"]>["unsafeMetadata"]
+    | null
+    | undefined,
 ): data is ClerkCredentials {
   return !!data && "jazzAccountID" in data && "jazzAccountSecret" in data;
 }
 
 export function isClerkAuthStateEqual(
-  previousUser: MinimalClerkClient["user"] | null | undefined,
-  newUser: MinimalClerkClient["user"] | null | undefined,
+  previousUser:
+    | Pick<NonNullable<MinimalClerkClient["user"]>, "unsafeMetadata">
+    | null
+    | undefined,
+  newUser:
+    | Pick<NonNullable<MinimalClerkClient["user"]>, "unsafeMetadata">
+    | null
+    | undefined,
 ) {
   if (Boolean(previousUser) !== Boolean(newUser)) {
     return false;
   }
 
-  const previousCredentials = isClerkCredentials(previousUser?.unsafeMetadata);
-  const newCredentials = isClerkCredentials(newUser?.unsafeMetadata);
+  const previousCredentials = isClerkCredentials(previousUser?.unsafeMetadata)
+    ? previousUser?.unsafeMetadata
+    : null;
+  const newCredentials = isClerkCredentials(newUser?.unsafeMetadata)
+    ? newUser?.unsafeMetadata
+    : null;
+
+  if (!previousCredentials || !newCredentials) {
+    return previousCredentials === newCredentials;
+  }
 
-  return previousCredentials === newCredentials;
+  return previousCredentials.jazzAccountID === newCredentials.jazzAccountID;
 }