fix(auth): require verified local account for OAuth email link (#3504) (#3511)

* fix(auth): require verified local account for OAuth email link

An unverified local signup squatting a victim's email could be silently
annexed by a later OAuth signin (Google/Apple) because the link-by-email
branch only checked `profil.emailVerifiedByProvider`. The pre-existing
local account's own `emailVerified` flag was ignored, so the OAuth signin
inherited the unverified account, its password, and any state attached.

Tighten the gate so the atomic link fires only when both sides vouch for
the email: OAuth provider verified AND local account `emailVerified: true`.
When the local account exists but is unverified, reject with a clear
`VALIDATION_ERROR` instead of falling through to account creation (which
would later throw on the unique-email index with a less actionable error).

- `UserRepository.linkProviderByEmail` filter now includes
  `emailVerified: true`; dropped the redundant `$set` of the same field
  since the filter already guarantees it.
- `AuthController.checkOAuthUserProfile` branch 3 follows up a null link
  with a `findByEmail` to detect the unverified-squatter case and throw
  `AppError('oAuth, cannot link to unverified local account', ...)`.
- Integration tests: updated the "link succeeds" case to mark the local
  account verified; added a new squatter-rejection test asserting the
  local doc is unchanged (no additionalProvidersData, emailVerified still
  false, providerData still null).

Closes #3504

* fix(auth): clarify branch-3 race behaviour in link-gate comment

Self-review follow-up: the prior comment didn't explain what happens if
`findByEmail` finds an already-verified user (the rare race where
verification completed between the atomic findOneAndUpdate and the
follow-up lookup). Document that falling through to branch 4 is the
intended behaviour — branch 4 will fail on the unique-email index, and
an OAuth retry will hit branch 3 and link cleanly.

* test(auth): mark signup-gate branch-3 seed as emailVerified

PR #3503 landed a new test for branch 3 that pre-dates this gate change.
It creates a local user then expects the OAuth signin to link — but with
the new gate the link now requires local emailVerified: true. Seed the
local user as verified (same pattern as the existing link-succeeds test)
so the test continues to exercise the happy path post-rebase.

Author: PierreBrisorgueil

modules/auth/controllers/auth.controller.js

@@ -358,15 +358,39 @@ const checkOAuthUserProfile = async (profil, key, provider) => {
   } catch (err) {
     throw new AppError('oAuth, find linked user failed', { code: 'SERVICE_ERROR', details: err });
   }
-  // 3. Link on verified email: if a local user exists with the same email and the OAuth
-  //    provider vouches for it, attach providerData under additionalProvidersData.{provider}
-  //    without overwriting user.provider (keeps password reset + local login intact).
-  //    Atomic findOneAndUpdate avoids TOCTOU races between concurrent OAuth callbacks.
+  // 3. Link on verified email: if a local user exists with the same email AND is
+  //    already emailVerified locally AND the OAuth provider vouches for the email,
+  //    attach providerData under additionalProvidersData.{provider} without
+  //    overwriting user.provider (keeps password reset + local login intact).
+  //    Atomic findOneAndUpdate (filter includes emailVerified: true) avoids TOCTOU
+  //    races and prevents an unverified-squatter local account from being annexed
+  //    by a later OAuth signin (issue #3504). If a matching email exists but is
+  //    not locally verified, we reject with VALIDATION_ERROR rather than fall
+  //    through to branch 4 (which would later fail on the unique-email index).
   if (profil.email && profil.emailVerifiedByProvider) {
     try {
       const linked = await UserService.linkProviderByEmail(profil.email, provider, profil.providerData);
       if (linked) return linked;
+      // Link returned null → either no local user with this email, or the local
+      // user exists but is not emailVerified. Disambiguate so we can reject the
+      // squatter case explicitly instead of falling through to branch 4 (which
+      // would later fail on the unique-email index with a less actionable error).
+      const existing = await UserService.findByEmail(profil.email);
+      if (existing && !existing.emailVerified) {
+        throw new AppError('oAuth, cannot link to unverified local account', {
+          code: 'VALIDATION_ERROR',
+          details: {
+            message: 'A pending account with this email is not verified. Verify the original signup first or contact support.',
+          },
+        });
+      }
+      // If `existing` is emailVerified here, a rare race between the atomic
+      // findOneAndUpdate and this findByEmail let verification complete in
+      // between. Falling through is safe: branch 4 will fail on the unique
+      // email index, the OAuth client will see the error, and a retry will
+      // hit the now-linkable state via branch 3.
     } catch (err) {
+      if (err instanceof AppError) throw err;
       throw new AppError('oAuth, link to existing user failed', { code: 'SERVICE_ERROR', details: err });
     }
   }

modules/auth/tests/auth.integration.tests.js

@@ -748,8 +748,9 @@ describe('Auth integration tests:', () => {
       } catch (_) { /* cleanup – ignore errors */ }
     });
 
-    test('should link OAuth signin to existing local user when provider verifies email', async () => {
-      // Seed a local user with a password (provider=local), email not yet linked via OAuth
+    test('should link OAuth signin to existing local user when both local emailVerified and provider verify', async () => {
+      // Seed a local user with a password (provider=local) that has ALREADY verified its email
+      // — the branch-3 link-on-verified-email gate requires both sides to vouch (issue #3504).
       const localEmail = 'oauthlink-local@test.com';
       const localUser = await UserService.create({
         firstName: 'Local',
@@ -759,6 +760,9 @@ describe('Auth integration tests:', () => {
         provider: 'local',
         roles: ['user'],
       });
+      // Mark the local account as email-verified (done normally by the verification flow).
+      const brutLocal = await UserService.getBrut({ email: localEmail });
+      await UserService.update(brutLocal, { emailVerified: true }, 'recover');
 
       // OAuth signin arrives with matching email + provider-verified flag
       const profil = {
@@ -786,6 +790,52 @@ describe('Auth integration tests:', () => {
       try { await UserService.remove(linked); } catch (_) { /* cleanup */ }
     });
 
+    test('should reject link when local account is NOT emailVerified even if OAuth provider verified (#3504)', async () => {
+      // Squatter scenario (issue #3504): someone signed up locally with victim's email
+      // but never verified it. A later OAuth signin by the real owner must NOT silently
+      // annex the unverified local account.
+      const squatterEmail = 'oauthlink-squatter@test.com';
+      const localUser = await UserService.create({
+        firstName: 'Squatter',
+        lastName: 'Pending',
+        email: squatterEmail,
+        password: credentials[0].password,
+        provider: 'local',
+        roles: ['user'],
+      });
+      // Sanity: freshly created local user is not emailVerified by default.
+      const brutBefore = await UserService.getBrut({ email: squatterEmail });
+      expect(brutBefore.emailVerified).toBe(false);
+      expect(brutBefore.additionalProvidersData?.google).toBeUndefined();
+      expect(brutBefore.providerData == null).toBe(true);
+
+      const profil = {
+        firstName: 'Real',
+        lastName: 'Owner',
+        email: squatterEmail,
+        avatar: '',
+        providerData: { sub: 'google-owner-sub-3504', email_verified: true },
+        emailVerifiedByProvider: true,
+      };
+
+      await expect(
+        AuthController.checkOAuthUserProfile(profil, 'sub', 'google'),
+      ).rejects.toMatchObject({
+        code: 'VALIDATION_ERROR',
+        message: 'oAuth, cannot link to unverified local account',
+      });
+
+      // The unverified local account must be untouched — no silent annexation.
+      const users = await UserService.search({ email: squatterEmail });
+      expect(users.length).toBe(1);
+      const brutAfter = await UserService.getBrut({ email: squatterEmail });
+      expect(brutAfter.emailVerified).toBe(false);
+      expect(brutAfter.additionalProvidersData?.google).toBeUndefined();
+      expect(brutAfter.providerData == null).toBe(true);
+
+      try { await UserService.remove(localUser); } catch (_) { /* cleanup */ }
+    });
+
     test('should NOT link when OAuth provider did not verify the email', async () => {
       const sharedEmail = 'oauthlink-unverified@test.com';
       const localUser = await UserService.create({
@@ -995,6 +1045,10 @@ describe('Auth integration tests:', () => {
           provider: 'local',
           roles: ['user'],
         });
+        // Mark the local account as email-verified — branch 3 link-gate now
+        // requires both OAuth provider AND local emailVerified (issue #3504).
+        const brutSeed = await UserService.getBrut({ email });
+        await UserService.update(brutSeed, { emailVerified: true }, 'recover');
 
         config.sign.up = false;
         const profil = {

modules/users/repositories/users.repository.js

@@ -194,15 +194,20 @@ const updateMany = (filter, data) => User.updateMany(filter, data, { runValidato
 /**
  * @desc Atomically attach an OAuth provider to an existing user matched by email.
  * Uses findOneAndUpdate to avoid TOCTOU races between concurrent OAuth callbacks.
+ * Filter requires `emailVerified: true` so an unverified-squatter local signup
+ * cannot be silently annexed by a later OAuth signin (issue #3504).
  * @param {string} email - The email to match
  * @param {string} provider - The OAuth provider key (e.g. 'google', 'apple')
  * @param {Object} providerData - The provider's identity data to store
- * @returns {Promise<Object|null>} Updated user document or null if no match
+ * @returns {Promise<Object|null>} Updated user document, or null when no match
+ *   OR when a match exists but is not email-verified — the caller is expected
+ *   to follow up with findByEmail to distinguish the two cases if it needs to
+ *   return a specific error.
  */
 const linkProviderByEmail = (email, provider, providerData) =>
   User.findOneAndUpdate(
-    { email },
-    { $set: { [`additionalProvidersData.${provider}`]: providerData, emailVerified: true } },
+    { email, emailVerified: true },
+    { $set: { [`additionalProvidersData.${provider}`]: providerData } },
     { returnDocument: 'after', runValidators: true },
   ).exec();