fix(auth): guard OAuth :strategy route — 404 unknown/disabled + stateless authenticate

modules/auth/controllers/auth.controller.js

@@ -439,6 +439,31 @@ const token = async (req, res) => {
     .json({ user, tokenExpiresIn: Date.now() + config.jwt.expiresIn * 1000, abilities, pendingRequests });
 };
 
+/**
+ * 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 Check whether `:strategy` (from `req.params.strategy`) is both an allowlisted
+ * provider AND actually registered with passport at boot time. A provider in
+ * ALLOWED_PROVIDERS is only registered (modules/auth/strategies/local/{provider}.js)
+ * when its config carries valid credentials — otherwise passport never sees it.
+ * Guards the `/api/auth/:strategy` and `/api/auth/:strategy/callback` routes so an
+ * unknown or disabled strategy name is rejected with a clean 404 before it can reach
+ * passport.authenticate(), which otherwise throws synchronously ("Unknown
+ * authentication strategy") for an unregistered name.
+ * `passport._strategy` is documented `@api private` in passport's own source, but it
+ * is the single source of truth for "is this strategy registered" — asking config
+ * directly would mean duplicating each provider's own credential-presence check here
+ * and risking drift from modules/auth/strategies/local/*.js over time.
+ * @param {string} strategy - strategy name from req.params.strategy
+ * @returns {boolean} true when strategy is both allowlisted and registered with passport
+ */
+const isEnabledOAuthProvider = (strategy) => ALLOWED_PROVIDERS.has(strategy) && Boolean(passport._strategy(strategy));
+
 /**
  * @desc Endpoint for oautCall
  * @param {Object} req - Express request object
@@ -447,16 +472,13 @@ const token = async (req, res) => {
  */
 const oauthCall = (req, res, next) => {
   const strategy = req.params.strategy;
-  passport.authenticate(strategy)(req, res, next);
+  if (!isEnabledOAuthProvider(strategy)) {
+    return next(new AppError('oAuth, unsupported provider', { status: 404, code: 'OAUTH_PROVIDER_NOT_FOUND' }));
+  }
+  // session: false — stateless JWT stack, no express-session middleware is mounted.
+  return passport.authenticate(strategy, { session: false })(req, res, next);
 };
 
-/**
- * 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])
@@ -659,12 +681,19 @@ const oauthErrorRedirect = (res, err, fallbackTitle) => {
  */
 const oauthCallback = async (req, res, next) => {
   const strategy = req.params.strategy;
+  if (!isEnabledOAuthProvider(strategy)) {
+    return next(new AppError('oAuth, unsupported provider', { status: 404, code: 'OAUTH_PROVIDER_NOT_FOUND' }));
+  }
   // The identity is always derived server-side from the provider's response via
   // passport.authenticate(). The part that was previously unsafe was trusting a
   // client-asserted identity from the request body (email, key, value) with no
   // provider-token verification — a caller who knows a victim's identifier could
   // have minted that victim's session. That branch is now removed entirely.
-  passport.authenticate(strategy, (err, user) => {
+  // No `{ session: false }` here: passport.authenticate()'s callback form (a
+  // function as the 2nd/3rd arg) never calls req.logIn() itself — session
+  // establishment is entirely the caller's responsibility below (JWT + cookie,
+  // no express-session) — so the `session` option has nothing to act on.
+  return passport.authenticate(strategy, (err, user) => {
     if (err) {
       logger.error(
         { err: { message: err?.message, code: err?.code, stack: err?.stack }, strategy },

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

@@ -558,6 +558,15 @@ describe('Auth integration tests:', () => {
       }
     });
 
+    // Safety net alongside each test's own manual mockRestore(): jest.config.js
+    // only sets `clearMocks` (resets calls, not implementations), so a spy left
+    // dangling by a failing assertion earlier in a test would otherwise leak its
+    // stubbed truthy passport._strategy()/mockRejectedValueOnce() into later
+    // tests in this block.
+    afterEach(() => {
+      jest.restoreAllMocks();
+    });
+
     test('should create user with default provider when none is specified', async () => {
       const result = await UserService.create({
         firstName: 'No',
@@ -625,6 +634,18 @@ describe('Auth integration tests:', () => {
       // identity payload. A request that claims to be a known account by email
       // (no server-side provider-token verification) must be rejected, not
       // turned into a victim session. The callback always delegates to passport.
+      //
+      // Simulate 'google' being registered with passport (bypasses the
+      // isEnabledOAuthProvider guard, covered separately for oauthCall/oauthCallback)
+      // and stub passport.authenticate the same way a real OAuth2 strategy would
+      // behave for this request: there is no valid provider code, so authentication
+      // fails regardless of what the attacker put in the body. Without this stub the
+      // request would 404 at the guard before ever reaching the identity-trust logic
+      // this test protects.
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
+      const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
+        (strategy, callback) => () => callback(null, false),
+      );
       const victimEmail = 'oauthcb-victim@test.com';
       let victim;
       try {
@@ -649,13 +670,26 @@ describe('Auth integration tests:', () => {
         const tokenCookie = result.headers['set-cookie']?.find((c) => c.startsWith('TOKEN='));
         expect(tokenCookie).toBeUndefined();
         expect(result.status).not.toBe(200);
+        // Confirm the request reached oauthCallback's post-authenticate
+        // identity-trust logic (not the 404 provider-registration guard) —
+        // the redirect must carry the "no user" error, never the guard's code.
+        expect(result.status).toBe(302);
+        expect(result.headers.location).toContain('/token');
+        expect(result.headers.location).not.toContain('OAUTH_PROVIDER_NOT_FOUND');
       } finally {
+        authenticateSpy.mockRestore();
+        strategySpy.mockRestore();
         try { if (victim) await UserService.remove(victim); } catch (_) { /* cleanup */ }
       }
     });
 
     test('should set tokenCookieOptions and redirect on classic web oAuth success', async () => {
       const mockUserId = 'mock-oauth-user-id-123';
+      // Simulate 'google' being registered with passport (isEnabledOAuthProvider's
+      // guard check) — this test targets oauthCallback's post-authenticate
+      // handling, not the provider-registration guard itself (covered separately
+      // for oauthCall).
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(null, { id: mockUserId }),
       );
@@ -675,9 +709,11 @@ describe('Auth integration tests:', () => {
       expect(redirectCalls[0]).toMatchObject({ code: 302 });
       expect(redirectCalls[0].url).toMatch(/\/token$/);
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should handle GET callback when req.body is undefined (Express 5)', async () => {
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(null, { id: 'mock-get-cb-user' }),
       );
@@ -694,11 +730,13 @@ describe('Auth integration tests:', () => {
       expect(cookies.TOKEN).toBeDefined();
       expect(redirectCalls[0]).toMatchObject({ code: 302 });
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should log and redirect with canonical error envelope when classic web oAuth errors out', async () => {
       const oauthErr = new Error('token exchange failed');
       oauthErr.code = 'OAUTH_TOKEN_EXCHANGE';
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(oauthErr, null),
       );
@@ -743,6 +781,7 @@ describe('Auth integration tests:', () => {
 
       loggerSpy.mockRestore();
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should redirect with canonical envelope preserving AppError details.message', async () => {
@@ -751,6 +790,7 @@ describe('Auth integration tests:', () => {
         code: 'VALIDATION_ERROR',
         details: { message: 'Registration is currently deactivated' },
       });
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(oauthErr, null),
       );
@@ -781,9 +821,11 @@ describe('Auth integration tests:', () => {
 
       loggerSpy.mockRestore();
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should fall back to OAUTH_ERROR code for plain Error without code', async () => {
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(new Error('boom'), null),
       );
@@ -805,9 +847,11 @@ describe('Auth integration tests:', () => {
 
       loggerSpy.mockRestore();
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should log and redirect with canonical envelope when no user is returned by passport', async () => {
+      const strategySpy = jest.spyOn(passport, '_strategy').mockReturnValue({});
       const authenticateSpy = jest.spyOn(passport, 'authenticate').mockImplementationOnce(
         (strategy, callback) => () => callback(null, null),
       );
@@ -851,6 +895,7 @@ describe('Auth integration tests:', () => {
 
       loggerSpy.mockRestore();
       authenticateSpy.mockRestore();
+      strategySpy.mockRestore();
     });
 
     test('should find an existing OAuth user via checkOAuthUserProfile', async () => {

modules/auth/tests/auth.oauthCall.controller.unit.tests.js

@@ -0,0 +1,166 @@
+/**
+ * Module dependencies.
+ */
+import { jest, describe, test, expect, beforeEach } from '@jest/globals';
+import { setupAuthControllerMocks } from './fixtures/auth-controller.mock-setup.js';
+
+/**
+ * Unit tests for auth.controller oauthCall() (issue #3900).
+ *
+ * Before this guard, `oauthCall` passed `req.params.strategy` straight into
+ * `passport.authenticate()` with no validation and no `{ session: false }`:
+ *  - an unknown strategy name made passport throw synchronously ("Unknown
+ *    authentication strategy") -> 500.
+ *  - an allowlisted-but-unregistered provider hit the same throw.
+ *  - a registered provider defaulted to `session: true`, which fails on this
+ *    stateless JWT stack ("Login sessions require session support").
+ *
+ * These tests verify the new isEnabledOAuthProvider() guard turns the first
+ * two cases into a clean 404 and never reaches passport.authenticate(), and
+ * that the third case delegates with `{ session: false }`.
+ */
+describe('auth.controller oauthCall:', () => {
+  let mockPassport;
+
+  beforeEach(() => {
+    mockPassport = setupAuthControllerMocks();
+  });
+
+  test('unknown strategy (not in ALLOWED_PROVIDERS) is rejected with a 404, passport.authenticate is never called', async () => {
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'me' } };
+    const res = {};
+    const next = jest.fn();
+
+    AuthController.oauthCall(req, res, next);
+
+    expect(mockPassport.authenticate).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalledTimes(1);
+    const err = next.mock.calls[0][0];
+    expect(err).toBeInstanceOf(Error);
+    expect(err.status).toBe(404);
+    expect(err.code).toBe('OAUTH_PROVIDER_NOT_FOUND');
+  });
+
+  test('another unknown strategy segment (e.g. "callback") is rejected with a 404', async () => {
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'callback' } };
+    const res = {};
+    const next = jest.fn();
+
+    AuthController.oauthCall(req, res, next);
+
+    expect(mockPassport.authenticate).not.toHaveBeenCalled();
+    const err = next.mock.calls[0][0];
+    expect(err.status).toBe(404);
+    expect(err.code).toBe('OAUTH_PROVIDER_NOT_FOUND');
+  });
+
+  test('allowlisted but unregistered provider (not enabled in config) is rejected with a 404', async () => {
+    // passport._strategy() returns undefined by default in this suite's mock —
+    // simulating a provider that is in ALLOWED_PROVIDERS but was never
+    // passport.use()'d at boot (e.g. missing clientID/clientSecret).
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'google' } };
+    const res = {};
+    const next = jest.fn();
+
+    AuthController.oauthCall(req, res, next);
+
+    expect(mockPassport._strategy).toHaveBeenCalledWith('google');
+    expect(mockPassport.authenticate).not.toHaveBeenCalled();
+    const err = next.mock.calls[0][0];
+    expect(err.status).toBe(404);
+    expect(err.code).toBe('OAUTH_PROVIDER_NOT_FOUND');
+  });
+
+  test('enabled + allowlisted provider delegates to passport.authenticate with { session: false }', async () => {
+    mockPassport._strategy.mockReturnValue({ name: 'google' }); // simulate registered strategy
+    const authenticateMiddleware = jest.fn();
+    mockPassport.authenticate.mockReturnValue(authenticateMiddleware);
+
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'google' } };
+    const res = {};
+    const next = jest.fn();
+
+    AuthController.oauthCall(req, res, next);
+
+    expect(mockPassport.authenticate).toHaveBeenCalledWith('google', { session: false });
+    expect(authenticateMiddleware).toHaveBeenCalledWith(req, res, next);
+    expect(next).not.toHaveBeenCalled();
+  });
+
+  test('apple (the other allowlisted provider) also delegates when registered', async () => {
+    mockPassport._strategy.mockReturnValue({ name: 'apple' });
+    const authenticateMiddleware = jest.fn();
+    mockPassport.authenticate.mockReturnValue(authenticateMiddleware);
+
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'apple' } };
+    const res = {};
+    const next = jest.fn();
+
+    AuthController.oauthCall(req, res, next);
+
+    expect(mockPassport.authenticate).toHaveBeenCalledWith('apple', { session: false });
+    expect(authenticateMiddleware).toHaveBeenCalledWith(req, res, next);
+  });
+});
+
+/**
+ * Unit tests for auth.controller oauthCallback()'s isEnabledOAuthProvider guard
+ * (issue #3900). The integration suite (auth.integration.tests.js) covers the
+ * post-authenticate handling with `_strategy`/`authenticate` both stubbed truthy;
+ * these tests cover the guard's reject branch specifically — an unknown or
+ * allowlisted-but-unregistered strategy must 404 before passport.authenticate()
+ * is ever reached, mirroring the oauthCall reject-path tests above.
+ */
+describe('auth.controller oauthCallback:', () => {
+  let mockPassport;
+
+  beforeEach(() => {
+    mockPassport = setupAuthControllerMocks();
+  });
+
+  test('unknown strategy (not in ALLOWED_PROVIDERS) is rejected with a 404, passport.authenticate is never called', async () => {
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'me' }, body: {} };
+    const res = {};
+    const next = jest.fn();
+
+    await AuthController.oauthCallback(req, res, next);
+
+    expect(mockPassport.authenticate).not.toHaveBeenCalled();
+    expect(next).toHaveBeenCalledTimes(1);
+    const err = next.mock.calls[0][0];
+    expect(err).toBeInstanceOf(Error);
+    expect(err.status).toBe(404);
+    expect(err.code).toBe('OAUTH_PROVIDER_NOT_FOUND');
+  });
+
+  test('allowlisted but unregistered provider (not enabled in config) is rejected with a 404', async () => {
+    // passport._strategy() returns undefined by default in this suite's mock —
+    // simulating a provider that is in ALLOWED_PROVIDERS but was never
+    // passport.use()'d at boot (e.g. missing clientID/clientSecret).
+    const { default: AuthController } = await import('../../../modules/auth/controllers/auth.controller.js');
+
+    const req = { params: { strategy: 'google' }, body: {} };
+    const res = {};
+    const next = jest.fn();
+
+    await AuthController.oauthCallback(req, res, next);
+
+    expect(mockPassport._strategy).toHaveBeenCalledWith('google');
+    expect(mockPassport.authenticate).not.toHaveBeenCalled();
+    const err = next.mock.calls[0][0];
+    expect(err.status).toBe(404);
+    expect(err.code).toBe('OAUTH_PROVIDER_NOT_FOUND');
+  });
+});