fix: Unlinking auth provider triggers auth data validation (#10045)

mtrezza · web-flow · commit b6b632755263 · 2026-02-12T02:28:48.000Z
diff --git a/spec/AuthenticationAdaptersV2.spec.js b/spec/AuthenticationAdaptersV2.spec.js
@@ -1338,6 +1338,244 @@ describe('Auth Adapter features', () => {
     expect(user.get('authData')).toEqual({ adapterB: { id: 'test' } });
   });
 
+  it('should unlink a code-based auth provider without triggering adapter validation', async () => {
+    const mockUserId = 'gpgamesUser123';
+    const mockAccessToken = 'mockAccessToken';
+
+    const otherAdapter = {
+      validateAppId: () => Promise.resolve(),
+      validateAuthData: () => Promise.resolve(),
+    };
+
+    mockFetch([
+      {
+        url: 'https://oauth2.googleapis.com/token',
+        method: 'POST',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ access_token: mockAccessToken }),
+        },
+      },
+      {
+        url: `https://www.googleapis.com/games/v1/players/${mockUserId}`,
+        method: 'GET',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ playerId: mockUserId }),
+        },
+      },
+    ]);
+
+    await reconfigureServer({
+      auth: {
+        gpgames: {
+          clientId: 'testClientId',
+          clientSecret: 'testClientSecret',
+        },
+        otherAdapter,
+      },
+    });
+
+    // Sign up with username/password, then link providers
+    const user = new Parse.User();
+    await user.signUp({ username: 'gpgamesTestUser', password: 'password123' });
+
+    // Link gpgames code-based provider
+    await user.save({
+      authData: {
+        gpgames: { id: mockUserId, code: 'authCode123', redirect_uri: 'https://example.com/callback' },
+      },
+    });
+
+    // Link a second provider
+    await user.save({ authData: { otherAdapter: { id: 'other1' } } });
+
+    // Reset fetch spy to track calls during unlink
+    global.fetch.calls.reset();
+
+    // Unlink gpgames by setting authData to null; should not call beforeFind / external APIs
+    const sessionToken = user.getSessionToken();
+    await user.save({ authData: { gpgames: null } }, { sessionToken });
+
+    // No external HTTP calls should have been made during unlink
+    expect(global.fetch.calls.count()).toBe(0);
+
+    // Verify gpgames was removed while the other provider remains
+    await user.fetch({ useMasterKey: true });
+    const authData = user.get('authData');
+    expect(authData).toBeDefined();
+    expect(authData.gpgames).toBeUndefined();
+    expect(authData.otherAdapter).toEqual({ id: 'other1' });
+  });
+
+  it('should unlink one code-based provider while echoing back another unchanged', async () => {
+    const gpgamesUserId = 'gpgamesUser1';
+    const instagramUserId = 'igUser1';
+
+    // Mock gpgames API for initial login
+    mockFetch([
+      {
+        url: 'https://oauth2.googleapis.com/token',
+        method: 'POST',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ access_token: 'gpgamesToken' }),
+        },
+      },
+      {
+        url: `https://www.googleapis.com/games/v1/players/${gpgamesUserId}`,
+        method: 'GET',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ playerId: gpgamesUserId }),
+        },
+      },
+    ]);
+
+    await reconfigureServer({
+      auth: {
+        gpgames: {
+          clientId: 'testClientId',
+          clientSecret: 'testClientSecret',
+        },
+        instagram: {
+          clientId: 'testClientId',
+          clientSecret: 'testClientSecret',
+          redirectUri: 'https://example.com/callback',
+        },
+      },
+    });
+
+    // Login with gpgames
+    const user = await Parse.User.logInWith('gpgames', {
+      authData: { id: gpgamesUserId, code: 'gpCode1', redirect_uri: 'https://example.com/callback' },
+    });
+    const sessionToken = user.getSessionToken();
+
+    // Mock instagram API for linking
+    mockFetch([
+      {
+        url: 'https://api.instagram.com/oauth/access_token',
+        method: 'POST',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ access_token: 'igToken' }),
+        },
+      },
+      {
+        url: `https://graph.instagram.com/me?fields=id&access_token=igToken`,
+        method: 'GET',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ id: instagramUserId }),
+        },
+      },
+    ]);
+
+    // Link instagram as second provider
+    await user.save(
+      { authData: { instagram: { id: instagramUserId, code: 'igCode1' } } },
+      { sessionToken }
+    );
+
+    // Fetch to get current authData (afterFind strips credentials, leaving only { id })
+    await user.fetch({ sessionToken });
+    const currentAuthData = user.get('authData');
+    expect(currentAuthData.gpgames).toBeDefined();
+    expect(currentAuthData.instagram).toBeDefined();
+
+    // Reset fetch spy
+    global.fetch.calls.reset();
+
+    // Unlink gpgames while echoing back instagram unchanged — the common client pattern:
+    // fetch current state, spread it, set the one to unlink to null
+    user.set('authData', { ...currentAuthData, gpgames: null });
+    await user.save(null, { sessionToken });
+
+    // No external HTTP calls during unlink (no code exchange for unchanged instagram)
+    expect(global.fetch.calls.count()).toBe(0);
+
+    // Verify gpgames removed, instagram preserved
+    await user.fetch({ useMasterKey: true });
+    const finalAuthData = user.get('authData');
+    expect(finalAuthData).toBeDefined();
+    expect(finalAuthData.gpgames).toBeUndefined();
+    expect(finalAuthData.instagram).toBeDefined();
+    expect(finalAuthData.instagram.id).toBe(instagramUserId);
+  });
+
+  it('should reject changing an existing code-based provider id without credentials', async () => {
+    const mockUserId = 'gpgamesUser123';
+    const mockAccessToken = 'mockAccessToken';
+
+    mockFetch([
+      {
+        url: 'https://oauth2.googleapis.com/token',
+        method: 'POST',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ access_token: mockAccessToken }),
+        },
+      },
+      {
+        url: `https://www.googleapis.com/games/v1/players/${mockUserId}`,
+        method: 'GET',
+        response: {
+          ok: true,
+          json: () => Promise.resolve({ playerId: mockUserId }),
+        },
+      },
+    ]);
+
+    await reconfigureServer({
+      auth: {
+        gpgames: {
+          clientId: 'testClientId',
+          clientSecret: 'testClientSecret',
+        },
+      },
+    });
+
+    // Sign up and link gpgames with valid credentials
+    const user = new Parse.User();
+    await user.save({
+      authData: {
+        gpgames: { id: mockUserId, code: 'authCode123', redirect_uri: 'https://example.com/callback' },
+      },
+    });
+    const sessionToken = user.getSessionToken();
+
+    // Attempt to change gpgames id without credentials (no code or access_token)
+    await expectAsync(
+      user.save({ authData: { gpgames: { id: 'differentUserId' } } }, { sessionToken })
+    ).toBeRejectedWith(
+      jasmine.objectContaining({ message: jasmine.stringContaining('code is required') })
+    );
+  });
+
+  it('should reject linking a new code-based provider with only an id and no credentials', async () => {
+    await reconfigureServer({
+      auth: {
+        gpgames: {
+          clientId: 'testClientId',
+          clientSecret: 'testClientSecret',
+        },
+      },
+    });
+
+    // Sign up with username/password (no gpgames linked)
+    const user = new Parse.User();
+    await user.signUp({ username: 'linkTestUser', password: 'password123' });
+    const sessionToken = user.getSessionToken();
+
+    // Attempt to link gpgames with only { id } — no code or access_token
+    await expectAsync(
+      user.save({ authData: { gpgames: { id: 'victimUserId' } } }, { sessionToken })
+    ).toBeRejectedWith(
+      jasmine.objectContaining({ message: jasmine.stringContaining('code is required') })
+    );
+  });
+
   it('should handle multiple providers: add one while another remains unchanged (code-based)', async () => {
     await reconfigureServer({
       auth: {
diff --git a/src/Adapters/Auth/BaseCodeAuthAdapter.js b/src/Adapters/Auth/BaseCodeAuthAdapter.js
@@ -72,32 +72,47 @@ export default class BaseAuthCodeAdapter extends AuthAdapter {
     throw new Error('getAccessTokenFromCode is not implemented');
   }
 
+  /**
+   * Validates auth data on login. In the standard auth flows (login, signup,
+   * update), `beforeFind` runs first and validates credentials, so no
+   * additional credential check is needed here.
+   */
   validateLogin(authData) {
-    // User validation is already done in beforeFind
     return {
       id: authData.id,
     }
   }
 
+  /**
+   * Validates auth data on first setup or when linking a new provider.
+   * In the standard auth flows, `beforeFind` runs first and validates
+   * credentials, so no additional credential check is needed here.
+   */
   validateSetUp(authData) {
-    // User validation is already done in beforeFind
     return {
       id: authData.id,
     }
   }
 
+  /**
+   * Returns the auth data to expose to the client after a query.
+   */
   afterFind(authData) {
     return {
       id: authData.id,
     }
   }
 
+  /**
+   * Validates auth data on update. In the standard auth flows, `beforeFind`
+   * runs first for any changed auth data and validates credentials, so no
+   * additional credential check is needed here. Unchanged (echoed-back) data
+   * skips both `beforeFind` and validation entirely.
+   */
   validateUpdate(authData) {
-    // User validation is already done in beforeFind
     return {
       id: authData.id,
     }
-
   }
 
   parseResponseData(data) {
diff --git a/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js b/src/Adapters/Storage/Postgres/PostgresStorageAdapter.js
@@ -1607,20 +1607,28 @@ export class PostgresStorageAdapter implements StorageAdapter {
         const generate = (jsonb: string, key: string, value: any) => {
           return `json_object_set_key(COALESCE(${jsonb}, '{}'::jsonb), ${key}, ${value})::jsonb`;
         };
+        const generateRemove = (jsonb: string, key: string) => {
+          return `(COALESCE(${jsonb}, '{}'::jsonb) - ${key})`;
+        };
         const lastKey = `$${index}:name`;
         const fieldNameIndex = index;
         index += 1;
         values.push(fieldName);
         const update = Object.keys(fieldValue).reduce((lastKey: string, key: string) => {
+          let value = fieldValue[key];
+          if (value && value.__op === 'Delete') {
+            value = null;
+          }
+          if (value === null) {
+            const str = generateRemove(lastKey, `$${index}::text`);
+            values.push(key);
+            index += 1;
+            return str;
+          }
           const str = generate(lastKey, `$${index}::text`, `$${index + 1}::jsonb`);
           index += 2;
-          let value = fieldValue[key];
           if (value) {
-            if (value.__op === 'Delete') {
-              value = null;
-            } else {
-              value = JSON.stringify(value);
-            }
+            value = JSON.stringify(value);
           }
           values.push(key, value);
           return str;
diff --git a/src/Auth.js b/src/Auth.js
@@ -417,15 +417,29 @@ Auth.prototype._getAllRolesNamesForRoleIds = function (roleIDs, names = [], quer
     });
 };
 
-const findUsersWithAuthData = async (config, authData, beforeFind) => {
+const findUsersWithAuthData = async (config, authData, beforeFind, currentUserAuthData) => {
   const providers = Object.keys(authData);
 
   const queries = await Promise.all(
     providers.map(async provider => {
       const providerAuthData = authData[provider];
 
+      // Skip providers being unlinked (null value)
+      if (providerAuthData === null) {
+        return null;
+      }
+
+      // Skip beforeFind only when incoming data is confirmed unchanged from stored data.
+      // This handles echoed-back authData from afterFind (e.g. client sends back { id: 'x' }
+      // alongside a provider unlink). On login/signup, currentUserAuthData is undefined so
+      // beforeFind always runs, preserving it as the security gate for missing credentials.
+      const storedProviderData = currentUserAuthData?.[provider];
+      const incomingKeys = Object.keys(providerAuthData || {});
+      const isUnchanged = storedProviderData && incomingKeys.length > 0 &&
+        !incomingKeys.some(key => !isDeepStrictEqual(providerAuthData[key], storedProviderData[key]));
+
       const adapter = config.authDataManager.getValidatorForProvider(provider)?.adapter;
-      if (beforeFind && typeof adapter?.beforeFind === 'function') {
+      if (beforeFind && typeof adapter?.beforeFind === 'function' && !isUnchanged) {
         await adapter.beforeFind(providerAuthData);
       }
 
diff --git a/src/RestWrite.js b/src/RestWrite.js
@@ -541,7 +541,15 @@ RestWrite.prototype.ensureUniqueAuthDataId = async function () {
 };
 
 RestWrite.prototype.handleAuthData = async function (authData) {
-  const r = await Auth.findUsersWithAuthData(this.config, authData, true);
+  let currentUserAuthData;
+  if (this.query?.objectId) {
+    const [currentUser] = await this.config.database.find(
+      '_User',
+      { objectId: this.query.objectId }
+    );
+    currentUserAuthData = currentUser?.authData;
+  }
+  const r = await Auth.findUsersWithAuthData(this.config, authData, true, currentUserAuthData);
   const results = this.filteredObjectsByACL(r);
 
   const userId = this.getUserId();

Original file line number	Diff line number	Diff line change
`@@ -72,32 +72,47 @@ export default class BaseAuthCodeAdapter extends AuthAdapter {`
`72`	`72`	`throw new Error('getAccessTokenFromCode is not implemented');`
`73`	`73`	`}`
`74`	`74`
	`75`	`+ /**`
	`76`	`+ * Validates auth data on login. In the standard auth flows (login, signup,`
	`77`	+ * update), `beforeFind` runs first and validates credentials, so no
	`78`	`+ * additional credential check is needed here.`
	`79`	`+ */`
`75`	`80`	`validateLogin(authData) {`
`76`		`- // User validation is already done in beforeFind`
`77`	`81`	`return {`
`78`	`82`	`id: authData.id,`
`79`	`83`	`}`
`80`	`84`	`}`
`81`	`85`
	`86`	`+ /**`
	`87`	`+ * Validates auth data on first setup or when linking a new provider.`
	`88`	+ * In the standard auth flows, `beforeFind` runs first and validates
	`89`	`+ * credentials, so no additional credential check is needed here.`
	`90`	`+ */`
`82`	`91`	`validateSetUp(authData) {`
`83`		`- // User validation is already done in beforeFind`
`84`	`92`	`return {`
`85`	`93`	`id: authData.id,`
`86`	`94`	`}`
`87`	`95`	`}`
`88`	`96`
	`97`	`+ /**`
	`98`	`+ * Returns the auth data to expose to the client after a query.`
	`99`	`+ */`
`89`	`100`	`afterFind(authData) {`
`90`	`101`	`return {`
`91`	`102`	`id: authData.id,`
`92`	`103`	`}`
`93`	`104`	`}`
`94`	`105`
	`106`	`+ /**`
	`107`	+ * Validates auth data on update. In the standard auth flows, `beforeFind`
	`108`	`+ * runs first for any changed auth data and validates credentials, so no`
	`109`	`+ * additional credential check is needed here. Unchanged (echoed-back) data`
	`110`	+ * skips both `beforeFind` and validation entirely.
	`111`	`+ */`
`95`	`112`	`validateUpdate(authData) {`
`96`		`- // User validation is already done in beforeFind`
`97`	`113`	`return {`
`98`	`114`	`id: authData.id,`
`99`	`115`	`}`
`100`		`-`
`101`	`116`	`}`
`102`	`117`
`103`	`118`	`parseResponseData(data) {`