Skip to content

Commit

Permalink
fix(auth): reup connectionId check (#3413)
Browse files Browse the repository at this point in the history
## 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.
  • Loading branch information
bodinsamuel authored Jan 30, 2025
1 parent 4397ac2 commit 70985e9
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 60 deletions.
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postApiKey.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postAppStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postBasic.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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 {
Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postBill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postJwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postSignature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postTableau.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postTba.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postTwoStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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');

Expand Down
10 changes: 5 additions & 5 deletions packages/server/lib/controllers/auth/postUnauthenticated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand Down Expand Up @@ -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;

Expand Down
18 changes: 9 additions & 9 deletions packages/server/lib/controllers/oauth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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;

Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 70985e9

Please sign in to comment.