Skip to content

Commit

Permalink
fix(connect-ui): enforce session token integrations (#3032)
Browse files Browse the repository at this point in the history
## Describe your changes

Fixes
https://linear.app/nango/issue/NAN-1945/enforce-allowed-integrations-in-each-auth-endpoint

- Enforce session token's allowed integrations
  • Loading branch information
bodinsamuel authored Nov 21, 2024
1 parent 1732178 commit 4941f74
Show file tree
Hide file tree
Showing 14 changed files with 160 additions and 13 deletions.
9 changes: 9 additions & 0 deletions packages/server/lib/controllers/apiAuth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
import { linkConnection } from '../services/endUser.service.js';
import db from '@nangohq/database';
import { hmacCheck } from '../utils/hmac.js';
import { isIntegrationAllowed } from '../utils/auth.js';
import type { MessageRowInsert } from '@nangohq/types';

class ApiAuthController {
Expand Down Expand Up @@ -89,6 +90,10 @@ class ApiAuthController {
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

if (!req.body.apiKey) {
Expand Down Expand Up @@ -265,6 +270,10 @@ class ApiAuthController {
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

const credentials: BasicApiCredentials = {
type: 'BASIC',
username,
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/appStoreAuth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { connectionCreated as connectionCreatedHook, connectionCreationFailed as
import { linkConnection } from '../services/endUser.service.js';
import db from '@nangohq/database';
import { hmacCheck } from '../utils/hmac.js';
import { isIntegrationAllowed } from '../utils/auth.js';

class AppStoreAuthController {
async auth(req: Request, res: Response<any, Required<RequestLocals>>, next: NextFunction) {
Expand Down Expand Up @@ -74,6 +75,10 @@ class AppStoreAuthController {
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

if (!req.body.privateKeyId) {
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postBill.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { connectionCreated as connectionCreatedHook, connectionCreationFailed as
import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z
.object({
Expand Down Expand Up @@ -121,6 +122,10 @@ export const postPublicBillAuthorization = asyncWrapper<PostPublicBillAuthorizat
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const { success, error, response: credentials } = await connectionService.getBillCredentials(provider, userName, password, organizationId, devkey);
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postJwt.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z
.object({
Expand Down Expand Up @@ -130,6 +131,10 @@ export const postPublicJwtAuthorization = asyncWrapper<PostPublicJwtAuthorizatio
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const { success, error, response: credentials } = connectionService.getJwtCredentials(provider as ProviderJwt, privateKey, privateKeyId, issuerId);
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postSignature.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z
.object({
Expand Down Expand Up @@ -124,6 +125,10 @@ export const postPublicSignatureAuthorization = asyncWrapper<PostPublicSignature
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const { success, error, response: credentials } = connectionService.getSignatureCredentials(provider as ProviderSignature, username, password);
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postTableau.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { connectionCreated as connectionCreatedHook, connectionCreationFailed as
import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z
.object({
Expand Down Expand Up @@ -120,6 +121,10 @@ export const postPublicTableauAuthorization = asyncWrapper<PostPublicTableauAuth
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const {
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postTba.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
import { connectionCredential, connectionIdSchema, providerConfigKeySchema } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z
.object({
Expand Down Expand Up @@ -130,6 +131,10 @@ export const postPublicTbaAuthorization = asyncWrapper<PostPublicTbaAuthorizatio
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const tbaCredentials: TbaCredentials = {
Expand Down
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postTwoStep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import { connectionCreated as connectionCreatedHook, connectionCreationFailed as
import { connectionIdSchema, providerConfigKeySchema, connectionCredential } from '../../helpers/validation.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const bodyValidation = z.object({}).catchall(z.any()).strict();

Expand Down Expand Up @@ -114,6 +115,10 @@ export const postPublicTwoStepAuthorization = asyncWrapper<PostPublicTwoStepAuth
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { seeders } from '@nangohq/shared';
import { afterAll, beforeAll, describe, expect, it } from 'vitest';
import { isSuccess, runServer } from '../../utils/tests.js';
import { isError, isSuccess, runServer } from '../../utils/tests.js';

let api: Awaited<ReturnType<typeof runServer>>;

Expand All @@ -25,7 +25,7 @@ describe(`GET ${endpoint}`, () => {
params: { providerConfigKey: config.unique_key }
});

isSuccess(res);
isSuccess(res.json);
expect(res.json).toStrictEqual<typeof res.json>({
connectionId: 'a',
providerConfigKey: 'unauthenticated'
Expand All @@ -43,10 +43,59 @@ describe(`GET ${endpoint}`, () => {
params: { providerConfigKey: config.unique_key }
});

isSuccess(res);
isSuccess(res.json);
expect(res.json).toStrictEqual<typeof res.json>({
connectionId: expect.any(String),
providerConfigKey: 'unauthenticated'
});
});

it('should create one connection with sessionToken', async () => {
const env = await seeders.createEnvironmentSeed();
const config = await seeders.createConfigSeed(env, 'unauthenticated', 'unauthenticated');

const resSession = await api.fetch('/connect/sessions', {
method: 'POST',
token: env.secret_key,
body: { end_user: { id: '1', email: '[email protected]' } }
});
isSuccess(resSession.json);

const res = await api.fetch(endpoint, {
method: 'POST',
token: env.secret_key,
query: { connect_session_token: resSession.json.data.token },
params: { providerConfigKey: config.unique_key }
});

isSuccess(res.json);
expect(res.json).toStrictEqual<typeof res.json>({
connectionId: expect.any(String),
providerConfigKey: 'unauthenticated'
});
});

it('should not be allowed to create an integration if disallowed by sessionToken', async () => {
const env = await seeders.createEnvironmentSeed();
const config = await seeders.createConfigSeed(env, 'unauthenticated', 'unauthenticated');

const resSession = await api.fetch('/connect/sessions', {
method: 'POST',
token: env.secret_key,
body: { end_user: { id: '1', email: '[email protected]' }, allowed_integrations: ['not_this_one'] }
});
isSuccess(resSession.json);

const res = await api.fetch(endpoint, {
method: 'POST',
token: env.secret_key,
query: { connect_session_token: resSession.json.data.token },
params: { providerConfigKey: config.unique_key }
});

isError(res.json);
expect(res.json).toStrictEqual<typeof res.json>({
error: { code: 'integration_not_allowed' }
});
});
});
5 changes: 5 additions & 0 deletions packages/server/lib/controllers/auth/postUnauthenticated.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { hmacCheck } from '../../utils/hmac.js';
import { connectionCreated, connectionCreationFailed } from '../../hooks/hooks.js';
import { linkConnection } from '../../services/endUser.service.js';
import db from '@nangohq/database';
import { isIntegrationAllowed } from '../../utils/auth.js';

const queryStringValidation = z
.object({
Expand Down Expand Up @@ -90,6 +91,10 @@ export const postPublicUnauthenticated = asyncWrapper<PostPublicUnauthenticatedA
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

await logCtx.enrichOperation({ integrationId: config.id!, integrationName: config.unique_key, providerName: config.provider });

const [updatedConnection] = await connectionService.upsertUnauthConnection({
Expand Down
9 changes: 9 additions & 0 deletions packages/server/lib/controllers/oauth.controller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import { linkConnection } from '../services/endUser.service.js';
import db from '@nangohq/database';
import { getConnectSession } from '../services/connectSession.service.js';
import { hmacCheck } from '../utils/hmac.js';
import { isIntegrationAllowed } from '../utils/auth.js';

class OAuthController {
public async oauthRequest(req: Request, res: Response<any, Required<RequestLocals>>, _next: NextFunction) {
Expand Down Expand Up @@ -142,6 +143,10 @@ class OAuthController {
return publisher.notifyErr(res, wsClientId, providerConfigKey, connectionId, error);
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

const session: OAuthSession = {
providerConfigKey: providerConfigKey,
provider: config.provider,
Expand Down Expand Up @@ -326,6 +331,10 @@ class OAuthController {
return;
}

if (!(await isIntegrationAllowed({ config, res, logCtx }))) {
return;
}

if (missesInterpolationParam(tokenUrl, connectionConfig)) {
const error = WSErrBuilder.InvalidConnectionConfig(tokenUrl, JSON.stringify(connectionConfig));
await logCtx.error(error.message, { connectionConfig });
Expand Down
29 changes: 29 additions & 0 deletions packages/server/lib/utils/auth.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
import type { LogContext } from '@nangohq/logs';
import type { ApiError, IntegrationConfig } from '@nangohq/types';

import type { Response } from 'express';
import type { RequestLocals } from './express';

export async function isIntegrationAllowed({
config,
logCtx,
res
}: {
config: IntegrationConfig;
logCtx: LogContext;
res: Response<ApiError<'integration_not_allowed'>, Required<RequestLocals>>;
}): Promise<boolean> {
if (res.locals['authType'] !== 'connectSession') {
return true;
}

const session = res.locals['connectSession'];
if (!session.allowedIntegrations || session.allowedIntegrations.includes(config.unique_key)) {
return true;
}

await logCtx.error('Integration not allowed by this token', { integration: config.unique_key, allowed: session.allowedIntegrations });
await logCtx.failed();
res.status(400).send({ error: { code: 'integration_not_allowed' } });
return false;
}
10 changes: 7 additions & 3 deletions packages/server/lib/utils/tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Server } from 'node:http';
import { createServer } from 'node:http';
import express from 'express';
import { expect } from 'vitest';
import type { APIEndpoints, APIEndpointsPicker, APIEndpointsPickerWithPath, ApiError } from '@nangohq/types';
import type { APIEndpoints, APIEndpointsPicker, APIEndpointsPickerWithPath } from '@nangohq/types';
import getPort from 'get-port';
import { migrateLogsMapping } from '@nangohq/logs';
import db, { multipleMigrations } from '@nangohq/database';
Expand Down Expand Up @@ -78,7 +78,9 @@ export function apiFetch(baseUrl: string) {
/**
* Assert API response is an error
*/
export function isError(json: any): asserts json is ApiError<any> {
export function isError<TType extends Record<string, any>>(
json: TType extends { json: any } ? never : TType
): asserts json is Extract<TType extends { json: any } ? never : TType, { error: any }> {
if (!('error' in json)) {
console.log('isError', inspect(json, true, 100));
throw new Error('Response is not an error');
Expand All @@ -88,7 +90,9 @@ export function isError(json: any): asserts json is ApiError<any> {
/**
* Assert API response is a success
*/
export function isSuccess<TType extends Record<string, any>>(json: TType): asserts json is Exclude<TType, { error: any }> {
export function isSuccess<TType extends Record<string, any>>(
json: TType extends { json: any } ? never : TType
): asserts json is Exclude<TType extends { json: any } ? never : TType, { error: any }> {
if (json && 'error' in json) {
console.log('isSuccess', inspect(json, true, 100));
throw new Error('Response is not a success');
Expand Down
Loading

0 comments on commit 4941f74

Please sign in to comment.