Skip to content

Commit

Permalink
Merge pull request #731 from snyk/fix/universal-webhook-selection-for…
Browse files Browse the repository at this point in the history
…-gh-and-ghe

fix: handle gh ghe webhooks in universal broker [HYB-261]
  • Loading branch information
aarlaud authored Mar 6, 2024
2 parents 3154e42 + 3943681 commit 55e2796
Show file tree
Hide file tree
Showing 6 changed files with 190 additions and 3 deletions.
34 changes: 34 additions & 0 deletions config.universaltest3.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
{
"BROKER_CLIENT_CONFIGURATION": {
"common": {
"default": {
"BROKER_SERVER_URL": "https://broker2.dev.snyk.io",
"BROKER_HA_MODE_ENABLED": "false",
"BROKER_DISPATCHER_BASE_URL": "https://api.dev.snyk.io"
}
},
"github": {
"validations":[{
"url": "https://notexists.notexists/no-such-url-ever"
}]
},
"gitlab": {
"validations":[{
"url": "https://notexists.notexists/no-such-url-ever"
}]
}
},
"CONNECTIONS": {
"my github connection": {
"type": "github-enterprise",
"identifier": "${BROKER_TOKEN_1}",
"GITHUB_TOKEN": "${GITHUB_TOKEN}"
},
"my gitlab connection": {
"type": "gitlab",
"identifier": "${BROKER_TOKEN_2}",
"GITLAB_TOKEN": "${GITLAB_TOKEN}",
"GITLAB":"gitlab.dev.snyk.io"
}
}
}
2 changes: 1 addition & 1 deletion lib/client/hooks/startup/processHooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ export const processStartUpHooks = async (
): Promise<HookResults> => {
try {
clientOpts.config.API_BASE_URL =
clientOpts.config.BROKER_DISPATCHER_BASE_URL ??
clientOpts.config.API_BASE_URL ??
clientOpts.config.BROKER_DISPATCHER_BASE_URL ??
clientOpts.config.BROKER_SERVER_URL?.replace(
'//broker.',
'//api.',
Expand Down
19 changes: 17 additions & 2 deletions lib/client/routesHandler/websocketConnectionMiddlewares.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,34 @@ export const websocketConnectionSelectorMiddleware = (
} else {
const websocketConnections = res.locals
.websocketConnections as WebSocketConnection[];
const availableConnectionsTypes = websocketConnections.map(
(x) => x.supportedIntegrationType,
);
let inboundRequestType = '';
//config.supportedBrokerTypes

// 2 cases: webhooks/TYPE, /v[1-2] Container registry.
if (req.path.startsWith('/webhook')) {
const splitUrl = req.path.split('/');

if (
splitUrl.length > 2 &&
config.supportedBrokerTypes.includes(splitUrl[2])
availableConnectionsTypes.includes(splitUrl[2])
) {
inboundRequestType = req.path.split('/')[2];
} else if (
splitUrl.length > 2 &&
splitUrl[2] == 'github' &&
availableConnectionsTypes.includes('github-enterprise')
) {
inboundRequestType = 'github-enterprise';
} else {
logger.warn({ url: req.path }, 'Unexpected type in webhook request');
res
.status(401)
.send(
'Unexpected type in webhook request, unable to forward to server.',
);
return;
}
} else if (
req.path.startsWith('/api/v1/') ||
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/client/filters-webhook.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@
{
"path": "/webhook/github/:webhookId",
"method": "POST"
},
{
"path": "/webhook/gitlab/:webhookId",
"method": "POST"
}
]
}
127 changes: 127 additions & 0 deletions test/functional/webhook-universal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
const PORT = 9999;
process.env.BROKER_SERVER_URL = `http://localhost:${PORT}`;
import path from 'path';
import { axiosClient } from '../setup/axios-client';
import {
BrokerClient,
closeBrokerClient,
waitForBrokerServerConnections,
} from '../setup/broker-client';
import {
BrokerServer,
closeBrokerServer,
createBrokerServer,
waitForUniversalBrokerClientsConnection,
} from '../setup/broker-server';
import { TestWebServer, createTestWebServer } from '../setup/test-web-server';
import { DEFAULT_TEST_WEB_SERVER_PORT } from '../setup/constants';
import { createUniversalBrokerClient } from '../setup/broker-universal-client';

const fixtures = path.resolve(__dirname, '..', 'fixtures');
const serverAccept = path.join(fixtures, 'server', 'filters-webhook.json');
const clientAccept = path.join(fixtures, 'client', 'filters-webhook.json');

describe('proxy requests originating from behind the broker client', () => {
let tws: TestWebServer;
let bs: BrokerServer;
let bc: BrokerClient;
process.env.API_BASE_URL = `http://localhost:${DEFAULT_TEST_WEB_SERVER_PORT}`;

beforeAll(async () => {
tws = await createTestWebServer();

bs = await createBrokerServer({ port: PORT, filters: serverAccept });

process.env.SNYK_BROKER_SERVER_UNIVERSAL_CONFIG_ENABLED = 'true';
process.env.UNIVERSAL_BROKER_ENABLED = 'true';
process.env.SERVICE_ENV = 'universaltest3';
process.env.BROKER_TOKEN_1 = 'brokertoken1';
process.env.BROKER_TOKEN_2 = 'brokertoken2';
process.env.GITHUB_TOKEN = 'ghtoken';
process.env.GITLAB_TOKEN = 'gltoken';
process.env.SNYK_BROKER_CLIENT_CONFIGURATION__common__default__BROKER_SERVER_URL = `http://localhost:${bs.port}`;
process.env.SNYK_FILTER_RULES_PATHS__gitlab = clientAccept;
process.env['SNYK_FILTER_RULES_PATHS__github-enterprise'] = clientAccept;

bc = await createUniversalBrokerClient();
await waitForUniversalBrokerClientsConnection(bs, 2);
});

afterAll(async () => {
await tws.server.close();
await closeBrokerClient(bc);
await closeBrokerServer(bs);
delete process.env.BROKER_SERVER_URL;
});

it('server identifies self to client', async () => {
const serverMetadata = await waitForBrokerServerConnections(bc);
expect(serverMetadata.map((x) => x.brokertoken)).toEqual(
expect.arrayContaining(['brokertoken1', 'brokertoken2']),
);
expect(serverMetadata.map((x) => x.capabilities)).toEqual(
expect.arrayContaining([
['receive-post-streams'],
['receive-post-streams'],
]),
);
});

it('successfully broker Webhook call via tunnel', async () => {
const response = await axiosClient.post(
`http://localhost:${bc.port}/webhook/gitlab/12345678-1234-1234-1234-123456789abc`,
{ some: { example: 'json' } },
);

expect(response.status).toEqual(200);
expect(response.data).toStrictEqual('Received webhook via websocket');
});

it('successfully broker GHE Webhook call via tunnel without github conn', async () => {
const response = await axiosClient.post(
`http://localhost:${bc.port}/webhook/github/12345678-1234-1234-1234-123456789abc`,
{ some: { example: 'json' } },
);

expect(response.status).toEqual(200);
expect(response.data).toStrictEqual('Received webhook via websocket');
});

it('successfully broker Webhook call via API without github conn', async () => {
await closeBrokerServer(bs);

const response = await axiosClient.post(
`http://localhost:${bc.port}/webhook/github/12345678-1234-1234-1234-000000000000`,
{ some: { example: 'json' } },
);

expect(response.status).toEqual(200);
expect(response.data).toStrictEqual('Received webhook via API');
});

it('successfully fails Webhook call without matching type', async () => {
const response = await axiosClient.post(
`http://localhost:${bc.port}/webhook/githubd/12345678-1234-1234-1234-000000000000`,
{ some: { example: 'json' } },
);

expect(response.status).toEqual(401);
expect(response.data).toStrictEqual(
'Unexpected type in webhook request, unable to forward to server.',
);
});

it('successfully fails Webhook call via API without matching type', async () => {
await closeBrokerServer(bs);

const response = await axiosClient.post(
`http://localhost:${bc.port}/webhook/githubd/12345678-1234-1234-1234-000000000000`,
{ some: { example: 'json' } },
);

expect(response.status).toEqual(401);
expect(response.data).toStrictEqual(
'Unexpected type in webhook request, unable to forward to server.',
);
});
});
7 changes: 7 additions & 0 deletions test/setup/test-web-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,13 @@ const applyEchoRoutes = (app: Express) => {
resp.send('Received webhook via websocket');
},
);
echoRouter.post(
'/webhook/gitlab/12345678-1234-1234-1234-123456789abc',
(_: express.Request, resp: express.Response) => {
resp.status(200);
resp.send('Received webhook via websocket');
},
);
echoRouter.post(
'/webhook/github/12345678-1234-1234-1234-000000000000',
(_: express.Request, resp: express.Response) => {
Expand Down

0 comments on commit 55e2796

Please sign in to comment.