feat(auth): link OAuth signin to existing user by verified email (#3497)

* feat(auth): link OAuth signin to existing user by verified email (#3493)

- checkOAuthUserProfile lookup order: (provider, sub) → additional
  ProvidersData[provider][key] → email + provider-verified → create
- Store linked providers under additionalProvidersData, keep
  user.provider intact (password reset + local login still work)
- Require emailVerifiedByProvider=true before linking (prevents
  takeover via unverified OIDC claims)
- Set emailVerified=true on fresh OAuth create when provider
  vouches for the email (closes #3494)
- Pass email_verified through google + apple strategies

* fix(auth): fix layer violation, provider allowlist, token data leak, test accuracy

- Remove direct UserRepository import from auth controller (was layer violation)
- Replace UserRepository.update with UserService.update(brutUser, patch, 'recover') in step 3
- Add ALLOWED_PROVIDERS/ALLOWED_PROVIDER_KEYS allowlists before dynamic key construction
- Add sanitizeAdditionalProvidersData helper to strip accessToken/refreshToken from token endpoint
- Add additionalProvidersData to recover whitelist so service update can persist it
- Fix credentials.password → credentials[0].password (was array, not object)
- Fix unverified-email test to use same email and assert local account untouched
- Strengthen takeover test: assert specific error code + verify local account not modified
- Remove dead UserRepository mocks from unit tests (controller no longer imports it)

* fix(auth): fix takeover test error code assertion (CONTROLLER_ERROR not SERVICE_ERROR)

* fix(auth): atomic provider linking + JSDoc @returns on modified functions

- Replace search+update sequence with atomic findOneAndUpdate via UserRepository.linkProviderByEmail
  to eliminate TOCTOU race between concurrent OAuth callbacks (CodeRabbit race condition finding)
- Expose linkProviderByEmail on UserService with removeSensitive wrapping
- Controller step 3 now calls UserService.linkProviderByEmail (single round-trip, no race window)
- Add @returns JSDoc to checkOAuthUserProfile, apple.prepare, google.prepare

* test(auth): add coverage for provider/key allowlist validation and token sanitization

* docs(migrations): document OAuth account linking + Express 5 callback fix

Author: PierreBrisorgueil

MIGRATIONS.md

@@ -4,6 +4,52 @@ Breaking changes and upgrade notes for downstream projects.
 
 ---
 
+## OAuth account linking + Express 5 callback fix (2026-04-23)
+
+Two related auth fixes that ship together.
+
+### 1. Express 5 GET callback no longer crashes
+
+Enabling Google OAuth on a downstream project used to crash on first signin with `Cannot read properties of undefined (reading 'strategy')`. Root cause: Express 5 leaves `req.body` as `undefined` on GET requests (Express 4 initialized it to `{}`).
+
+- `modules/auth/controllers/auth.controller.js` — `oauthCallback` optional-chains `req.body` access
+- Apple OAuth (POST `form_post`) was never affected — no change to behavior
+
+### 2. Account linking by verified email
+
+Before: a local signup at `user@x.com` followed by a Google signin with the same email crashed on Mongo's unique-email index (E11000) — user locked out.
+
+Now: `checkOAuthUserProfile` follows a 4-step lookup:
+
+1. `(provider, providerData[key])` — primary identity (OAuth-first users)
+2. `additionalProvidersData[provider][key]` — linked users on subsequent signins
+3. `email` match **with provider-verified email** → atomic link (`UserService.linkProviderByEmail`)
+4. No match → create new user with `emailVerified` reflecting provider verification
+
+Linking attaches the OAuth `providerData` under `user.additionalProvidersData[provider]` and **does not overwrite `user.provider`** — so password reset (gated on `provider === 'local'`) and local login keep working for linked users.
+
+### Security gates
+
+- Provider + key allowlists (`ALLOWED_PROVIDERS = {google, apple}`, `ALLOWED_PROVIDER_KEYS = {id, sub, email}`) validate the dynamic query path before Mongo.
+- `emailVerifiedByProvider: true` required before linking — prevents takeover via a future OIDC provider that returns `email_verified: false` for someone else's address.
+- `/token` response sanitizes `accessToken` / `refreshToken` out of `additionalProvidersData` before serialization.
+
+### Action for downstream
+
+1. Run `/update-stack` to pull both fixes in one go.
+2. Env vars to set in prod K8s for Google (per project that wants OAuth enabled):
+   - `DEVKIT_NODE_oAuth_google_clientID`
+   - `DEVKIT_NODE_oAuth_google_clientSecret`
+   - `DEVKIT_NODE_oAuth_google_callbackURL` — e.g. `https://api.{project}.{tld}/api/auth/google/callback`
+3. Register the callback URL in Google Cloud Console (OAuth 2.0 client, Web type). For Apple: same pattern on `decodedIdToken.email_verified`.
+4. `/api/auth/config` returns `oAuth.google: true` once the clientID is set — the Vue signin/signup buttons activate automatically via `serverConfig.oAuth.google`.
+
+### Schema note
+
+`additionalProvidersData` already existed in the Mongoose user schema and is now exposed in the Zod user schema too. No Mongo migration needed — existing users have an empty field.
+
+---
+
 ## Analytics: request-aware feature-flag helpers (2026-04-23)
 
 `analytics` service gains two sugar helpers that extract the PostHog `distinctId` from an Express request, so routes no longer need to repeat `req.user?.id ?? req.sessionID ?? 'anonymous'` (and can never forget the anonymous fallback):

modules/auth/controllers/auth.controller.js

@@ -234,6 +234,25 @@ const signin = async (req, res) => {
  * @param {Object} res - Express response object
  * TODO: escape deprecated
  */
+/**
+ * @desc Strip OAuth tokens from an additionalProvidersData map before serializing to client.
+ * Only the provider identity fields are safe to expose; access/refresh tokens must stay server-side.
+ * @param {Object|undefined} apd - raw additionalProvidersData
+ * @returns {Object|undefined} sanitized map with accessToken/refreshToken removed per provider
+ */
+const sanitizeAdditionalProvidersData = (apd) => {
+  if (!apd || typeof apd !== 'object') return undefined;
+  const sanitized = {};
+  for (const [prov, data] of Object.entries(apd)) {
+    if (data && typeof data === 'object') {
+      // eslint-disable-next-line no-unused-vars
+      const { accessToken, refreshToken, ...safe } = data;
+      sanitized[prov] = safe;
+    }
+  }
+  return Object.keys(sanitized).length > 0 ? sanitized : undefined;
+};
+
 const token = async (req, res) => {
   let user = null;
   if (req.user) {
@@ -248,7 +267,7 @@ const token = async (req, res) => {
       email: req.user.email,
       lastName: req.user.lastName,
       firstName: req.user.firstName,
-      additionalProvidersData: req.user.additionalProvidersData,
+      additionalProvidersData: sanitizeAdditionalProvidersData(req.user.additionalProvidersData),
       emailVerified: req.user.emailVerified,
       currentOrganization: req.user.currentOrganization,
       lastLoginAt: req.user.lastLoginAt,
@@ -295,13 +314,32 @@ const oauthCall = (req, res, next) => {
 };
 
 /**
- * @desc Endpoint to save oAuthProfile
+ * Known OAuth providers — used to validate the `provider` argument and `key` argument
+ * before constructing dynamic query paths, preventing prototype-pollution-style injections.
+ */
+const ALLOWED_PROVIDERS = new Set(['google', 'apple']);
+const ALLOWED_PROVIDER_KEYS = new Set(['sub', 'id', 'email']);
+
+/**
+ * @desc Resolve or create a user from an OAuth profile. Lookup order:
+ *   1. Primary identity (provider + providerData[key])
+ *   2. Linked identity (additionalProvidersData[provider][key])
+ *   3. Link-on-verified-email (provider-verified email matches an existing local user)
+ *   4. Create new user
  * @param {Object} profil - OAuth user profile object
- * @param {string} key - Provider key to lookup providerData
- * @param {string} provider - OAuth provider name
+ * @param {string} key - Provider key to lookup providerData (must be in ALLOWED_PROVIDER_KEYS)
+ * @param {string} provider - OAuth provider name (must be in ALLOWED_PROVIDERS)
+ * @returns {Promise<Object>} sanitized user document (existing, linked, or newly created)
  */
 const checkOAuthUserProfile = async (profil, key, provider) => {
-  // check if user exist
+  // Guard: validate provider and key against allowlists before using as dynamic object keys
+  if (!ALLOWED_PROVIDERS.has(provider)) {
+    throw new AppError('oAuth, unsupported provider', { code: 'VALIDATION_ERROR', details: { provider } });
+  }
+  if (!ALLOWED_PROVIDER_KEYS.has(key)) {
+    throw new AppError('oAuth, unsupported provider key', { code: 'VALIDATION_ERROR', details: { key } });
+  }
+  // 1. Primary identity: match on (provider, providerData[key]) — OAuth-first users
   try {
     const query = {};
     query[`providerData.${key}`] = profil.providerData[key];
@@ -311,7 +349,28 @@ const checkOAuthUserProfile = async (profil, key, provider) => {
   } catch (err) {
     throw new AppError('oAuth, find user failed', { code: 'SERVICE_ERROR', details: err });
   }
-  // if no, generate
+  // 2. Linked identity: match on additionalProvidersData[provider][key] — locals already linked
+  try {
+    const query = {};
+    query[`additionalProvidersData.${provider}.${key}`] = profil.providerData[key];
+    const search = await UserService.search(query);
+    if (search.length === 1) return search[0];
+  } 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.
+  if (profil.email && profil.emailVerifiedByProvider) {
+    try {
+      const linked = await UserService.linkProviderByEmail(profil.email, provider, profil.providerData);
+      if (linked) return linked;
+    } catch (err) {
+      throw new AppError('oAuth, link to existing user failed', { code: 'SERVICE_ERROR', details: err });
+    }
+  }
+  // 4. No match → create new user
   try {
     const user = {
       firstName: profil.firstName,
@@ -320,6 +379,7 @@ const checkOAuthUserProfile = async (profil, key, provider) => {
       avatar: profil.avatar || '',
       provider,
       providerData: profil.providerData || null,
+      emailVerified: !!profil.emailVerifiedByProvider,
     };
     const result = model.getResultFromZod(user, UsersSchema.User);
     // check error

modules/auth/strategies/local/apple.js

@@ -12,12 +12,14 @@ const callbackURL = `${config.api.protocol}://${config.api.host}${config.api.por
 }/auth/apple/callback`;
 
 /**
- * @desc function to prepare map callback to user profile
- * @param {req}
- * @param {accessToken}
- * @param {refreshToken}
- * @param {profile}
- * @param {cb} callback
+ * @desc Map Apple OAuth callback to user profile and delegate to checkOAuthUserProfile
+ * @param {Object} req - Express request (passReqToCallback)
+ * @param {string} accessToken - Apple access token
+ * @param {string} refreshToken - Apple refresh token
+ * @param {Object} decodedIdToken - Decoded Apple ID token
+ * @param {Object} profile - Apple profile (may be empty on repeat sign-ins)
+ * @param {Function} cb - Passport callback (err, user)
+ * @returns {Promise<void>}
  */
 const prepare = async (req, accessToken, refreshToken, decodedIdToken, profile, cb) => {
   // Set the provider data and include tokens
@@ -35,6 +37,7 @@ const prepare = async (req, accessToken, refreshToken, decodedIdToken, profile,
     avatar: null,
     provider: 'apple',
     providerData,
+    emailVerifiedByProvider: decodedIdToken.email_verified === true || decodedIdToken.email_verified === 'true',
   };
   // Save the user OAuth profile
   try {

modules/auth/strategies/local/google.js

@@ -12,11 +12,12 @@ const callbackURL = `${config.api.protocol}://${config.api.host}${config.api.por
 }/auth/google/callback`;
 
 /**
- * @desc function to prepare map callback to user profile
- * @param {accessToken}
- * @param {refreshToken}
- * @param {profile}
- * @param {cb} callback
+ * @desc Map Google OAuth callback to user profile and delegate to checkOAuthUserProfile
+ * @param {string} accessToken - Google access token
+ * @param {string} refreshToken - Google refresh token
+ * @param {Object} profile - Google profile object
+ * @param {Function} cb - Passport callback (err, user)
+ * @returns {Promise<void>}
  */
 const prepare = async (accessToken, refreshToken, profile, cb) => {
   // Set the provider data and include tokens
@@ -31,6 +32,7 @@ const prepare = async (accessToken, refreshToken, profile, cb) => {
     avatar: providerData.picture ? providerData.picture : undefined,
     provider: 'google',
     providerData,
+    emailVerifiedByProvider: providerData.email_verified === true,
   };
   // Save the user OAuth profile
   try {