From 70985e900b267b3c297fec74525274ae3ad5f626 Mon Sep 17 00:00:00 2001
From: Samuel Bodin <1637651+bodinsamuel@users.noreply.github.com>
Date: Thu, 30 Jan 2025 16:58:16 +0100
Subject: [PATCH] fix(auth): reup connectionId check (#3413)

## Changes

Fixes https://linear.app/nango/issue/NAN-2558/re-up-connectionid-check

- Reup connectionId check
This was reverted when a customer did something forbidden in prod
(sending a hardcoded connectionId with sessionConnect) and just wanted
them to fix the issue before re-upping that.
---
 .../server/lib/controllers/auth/postApiKey.ts  | 10 +++++-----
 .../lib/controllers/auth/postAppStore.ts       | 10 +++++-----
 .../server/lib/controllers/auth/postBasic.ts   | 10 +++++-----
 .../server/lib/controllers/auth/postBill.ts    | 10 +++++-----
 .../server/lib/controllers/auth/postJwt.ts     | 10 +++++-----
 .../lib/controllers/auth/postSignature.ts      | 10 +++++-----
 .../server/lib/controllers/auth/postTableau.ts | 10 +++++-----
 .../server/lib/controllers/auth/postTba.ts     | 10 +++++-----
 .../server/lib/controllers/auth/postTwoStep.ts | 10 +++++-----
 .../postUnauthenticated.integration.test.ts    |  2 +-
 .../controllers/auth/postUnauthenticated.ts    | 10 +++++-----
 .../server/lib/controllers/oauth.controller.ts | 18 +++++++++---------
 12 files changed, 60 insertions(+), 60 deletions(-)

diff --git a/packages/server/lib/controllers/auth/postApiKey.ts b/packages/server/lib/controllers/auth/postApiKey.ts
index 8be767f0740..a35382cf0b2 100644
--- a/packages/server/lib/controllers/auth/postApiKey.ts
+++ b/packages/server/lib/controllers/auth/postApiKey.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook, connectionTest } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -76,10 +76,10 @@ export const postPublicApiKeyAuthorization = asyncWrapper<PostPublicApiKeyAuthor
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
     try {
diff --git a/packages/server/lib/controllers/auth/postAppStore.ts b/packages/server/lib/controllers/auth/postAppStore.ts
index 8b0be5f62cc..3a1c5548f96 100644
--- a/packages/server/lib/controllers/auth/postAppStore.ts
+++ b/packages/server/lib/controllers/auth/postAppStore.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -79,10 +79,10 @@ export const postPublicAppStoreAuthorization = asyncWrapper<PostPublicAppStoreAu
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
     try {
diff --git a/packages/server/lib/controllers/auth/postBasic.ts b/packages/server/lib/controllers/auth/postBasic.ts
index 863f26ebef0..ed486617ec0 100644
--- a/packages/server/lib/controllers/auth/postBasic.ts
+++ b/packages/server/lib/controllers/auth/postBasic.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook, connectionTest } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -77,10 +77,10 @@ export const postPublicBasicAuthorization = asyncWrapper<PostPublicBasicAuthoriz
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
     try {
diff --git a/packages/server/lib/controllers/auth/postBill.ts b/packages/server/lib/controllers/auth/postBill.ts
index b35c0fbcf0e..dbd4b4e0046 100644
--- a/packages/server/lib/controllers/auth/postBill.ts
+++ b/packages/server/lib/controllers/auth/postBill.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -80,10 +80,10 @@ export const postPublicBillAuthorization = asyncWrapper<PostPublicBillAuthorizat
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postJwt.ts b/packages/server/lib/controllers/auth/postJwt.ts
index 99a1c3991ea..79b9d686d10 100644
--- a/packages/server/lib/controllers/auth/postJwt.ts
+++ b/packages/server/lib/controllers/auth/postJwt.ts
@@ -25,7 +25,7 @@ import {
 } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -89,10 +89,10 @@ export const postPublicJwtAuthorization = asyncWrapper<PostPublicJwtAuthorizatio
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postSignature.ts b/packages/server/lib/controllers/auth/postSignature.ts
index 1fe9cf88344..6f26e1e1002 100644
--- a/packages/server/lib/controllers/auth/postSignature.ts
+++ b/packages/server/lib/controllers/auth/postSignature.ts
@@ -25,7 +25,7 @@ import {
 } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -83,10 +83,10 @@ export const postPublicSignatureAuthorization = asyncWrapper<PostPublicSignature
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postTableau.ts b/packages/server/lib/controllers/auth/postTableau.ts
index 086d67b68d3..f68cf967707 100644
--- a/packages/server/lib/controllers/auth/postTableau.ts
+++ b/packages/server/lib/controllers/auth/postTableau.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -79,10 +79,10 @@ export const postPublicTableauAuthorization = asyncWrapper<PostPublicTableauAuth
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postTba.ts b/packages/server/lib/controllers/auth/postTba.ts
index 73aa7471c0b..7f2ffcefd84 100644
--- a/packages/server/lib/controllers/auth/postTba.ts
+++ b/packages/server/lib/controllers/auth/postTba.ts
@@ -24,7 +24,7 @@ import {
 } from '../../hooks/hooks.js';
 import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z
     .object({
@@ -85,10 +85,10 @@ export const postPublicTbaAuthorization = asyncWrapper<PostPublicTbaAuthorizatio
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postTwoStep.ts b/packages/server/lib/controllers/auth/postTwoStep.ts
index 9fa9a69e1bf..78e1893634e 100644
--- a/packages/server/lib/controllers/auth/postTwoStep.ts
+++ b/packages/server/lib/controllers/auth/postTwoStep.ts
@@ -21,7 +21,7 @@ import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated as connectionCreatedHook, connectionCreationFailed as connectionCreationFailedHook } from '../../hooks/hooks.js';
 import { connectionIdSchema, providerConfigKeySchema, connectionCredential } from '../../helpers/validation.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const bodyValidation = z.object({}).catchall(z.any()).strict();
 
@@ -73,10 +73,10 @@ export const postPublicTwoStepAuthorization = asyncWrapper<PostPublicTwoStepAuth
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/auth/postUnauthenticated.integration.test.ts b/packages/server/lib/controllers/auth/postUnauthenticated.integration.test.ts
index b19d81f9bd4..792367bdb32 100644
--- a/packages/server/lib/controllers/auth/postUnauthenticated.integration.test.ts
+++ b/packages/server/lib/controllers/auth/postUnauthenticated.integration.test.ts
@@ -96,7 +96,7 @@ describe(`GET ${endpoint}`, () => {
         });
     });
 
-    it.skip('should not be allowed to pass a connection_id with session token', async () => {
+    it('should not be allowed to pass a connection_id with session token', async () => {
         const env = await seeders.createEnvironmentSeed();
         const config = await seeders.createConfigSeed(env, 'unauthenticated', 'unauthenticated');
 
diff --git a/packages/server/lib/controllers/auth/postUnauthenticated.ts b/packages/server/lib/controllers/auth/postUnauthenticated.ts
index c2f0022f9ff..d193f53c509 100644
--- a/packages/server/lib/controllers/auth/postUnauthenticated.ts
+++ b/packages/server/lib/controllers/auth/postUnauthenticated.ts
@@ -10,7 +10,7 @@ import type { LogContext } from '@nangohq/logs';
 import { hmacCheck } from '../../utils/hmac.js';
 import { connectionCreated, connectionCreationFailed } from '../../hooks/hooks.js';
 import db from '@nangohq/database';
-import { isIntegrationAllowed } from '../../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../../utils/auth.js';
 
 const queryStringValidation = z
     .object({
@@ -51,10 +51,10 @@ export const postPublicUnauthenticated = asyncWrapper<PostPublicUnauthenticatedA
     const hmac = 'hmac' in queryString ? queryString.hmac : undefined;
     const isConnectSession = res.locals['authType'] === 'connectSession';
 
-    // if (isConnectSession && queryString.connection_id) {
-    //     errorRestrictConnectionId(res);
-    //     return;
-    // }
+    if (isConnectSession && queryString.connection_id) {
+        errorRestrictConnectionId(res);
+        return;
+    }
 
     let logCtx: LogContext | undefined;
 
diff --git a/packages/server/lib/controllers/oauth.controller.ts b/packages/server/lib/controllers/oauth.controller.ts
index c77b5ca2d9b..c2dc93c5c6a 100644
--- a/packages/server/lib/controllers/oauth.controller.ts
+++ b/packages/server/lib/controllers/oauth.controller.ts
@@ -51,7 +51,7 @@ import db from '@nangohq/database';
 import type { ConnectSessionAndEndUser } from '../services/connectSession.service.js';
 import { getConnectSession } from '../services/connectSession.service.js';
 import { hmacCheck } from '../utils/hmac.js';
-import { isIntegrationAllowed } from '../utils/auth.js';
+import { errorRestrictConnectionId, isIntegrationAllowed } from '../utils/auth.js';
 
 class OAuthController {
     public async oauthRequest(req: Request, res: Response<any, Required<RequestLocals>>, _next: NextFunction) {
@@ -65,10 +65,10 @@ class OAuthController {
         let userScope = req.query['user_scope'] as string | undefined;
         const isConnectSession = res.locals['authType'] === 'connectSession';
 
-        // if (isConnectSession && receivedConnectionId) {
-        //     errorRestrictConnectionId(res);
-        //     return;
-        // }
+        if (isConnectSession && receivedConnectionId) {
+            errorRestrictConnectionId(res);
+            return;
+        }
 
         let logCtx: LogContext | undefined;
 
@@ -309,10 +309,10 @@ class OAuthController {
 
         const { client_id, client_secret }: Record<string, string> = body;
 
-        // if (isConnectSession && receivedConnectionId) {
-        //     errorRestrictConnectionId(res);
-        //     return;
-        // }
+        if (isConnectSession && receivedConnectionId) {
+            errorRestrictConnectionId(res);
+            return;
+        }
 
         let logCtx: LogContext | undefined;