Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add broker type validtion middlwr #565

Merged
merged 1 commit into from
Jun 22, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions config.default.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"BROKER_SERVER_UNIVERSAL_CONFIG_ENABLED": false
}
1 change: 0 additions & 1 deletion lib/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ dotenv.config({
expand(process.env);

const res = Object.assign({}, camelify(config), camelify(process.env));

if (res.caCert) {
res.caCert = fs.readFileSync(path.resolve(process.cwd(), res.caCert));
}
Expand Down
23 changes: 23 additions & 0 deletions lib/server/broker-middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
import { Request, Response, NextFunction } from 'express';
import * as logger from '../log';
import * as config from '../config';
export const validateBrokerTypeMiddleware = (
req: Request,
res: Response,
next: NextFunction,
) => {
const localConfig = config as unknown as Record<string, string>;
if (
localConfig.brokerServerUniversalConfigEnabled &&
!req?.headers['x-snyk-broker-type']
) {
const logContext = { url: req.url, headers: req.headers };
logger.warn(
{ logContext },
'Error: Request does not contain the x-snyk-broker-type header',
);
// Will eventually return an error when all services will have this enabled
// return res.status(400).json({ error: 'Missing x-broker-type header' });
}
next(); // Passes the request to the next middleware
};
3 changes: 2 additions & 1 deletion lib/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const version = require('../version');
const { maskToken, hashToken } = require('../token');
const metrics = require('../metrics');
const { applyPrometheusMiddleware } = require('./prometheus-middleware');
const { validateBrokerTypeMiddleware } = require('./broker-middleware');

module.exports = async ({ config = {}, port = null, filters = {} }) => {
logger.info({ version }, 'running in server mode');
Expand Down Expand Up @@ -33,7 +34,6 @@ module.exports = async ({ config = {}, port = null, filters = {} }) => {
if (!process.env.JEST_WORKER_ID) {
app.use(applyPrometheusMiddleware());
}

app.get('/connection-status/:token', (req, res) => {
const token = req.params.token;
const maskedToken = maskToken(token);
Expand Down Expand Up @@ -82,6 +82,7 @@ module.exports = async ({ config = {}, port = null, filters = {} }) => {

next();
},
validateBrokerTypeMiddleware,
relay.request(filters.public),
);

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@
"qs": "^6.9.1",
"request": "^2.88.1",
"request-promise-native": "^1.0.9",
"snyk-config": "^4.0.0-rc.2",
"snyk-config": "^4.0.0",
aarlaud marked this conversation as resolved.
Show resolved Hide resolved
"then-fs": "^2.0.0",
"tunnel": "0.0.6",
"undefsafe": "^2.0.2",
Expand Down
80 changes: 80 additions & 0 deletions test/functional/server-client-universal.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
process.env.SNYK_BROKER_SERVER_UNIVERSAL_CONFIG_ENABLED = 'true';
import * as path from 'path';
import { axiosClient } from '../setup/axios-client';
import {
BrokerClient,
closeBrokerClient,
createBrokerClient,
} from '../setup/broker-client';
import {
BrokerServer,
closeBrokerServer,
createBrokerServer,
waitForBrokerClientConnection,
} from '../setup/broker-server';
import { TestWebServer, createTestWebServer } from '../setup/test-web-server';

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

describe('proxy requests originating from behind the broker server', () => {
let tws: TestWebServer;
let bs: BrokerServer;
let bc: BrokerClient;
let brokerToken: string;

const spyLogWarn = jest
.spyOn(require('bunyan').prototype, 'warn')
.mockImplementation((value) => {
return value;
});

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

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

bc = await createBrokerClient({
brokerServerUrl: `http://localhost:${bs.port}`,
brokerToken: 'broker-token-12345',
filters: clientAccept,
type: 'client',
});
({ brokerToken } = await waitForBrokerClientConnection(bs));
});

afterEach(async () => {
spyLogWarn.mockReset();
});
afterAll(async () => {
spyLogWarn.mockReset();
await tws.server.close();
await closeBrokerClient(bc);
await closeBrokerServer(bs);
});

it('successfully broker GET', async () => {
const response = await axiosClient.get(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-param/xyz`,
);

expect(response.status).toEqual(200);
expect(response.data).toEqual('xyz');
});

it('successfully warn logs requests without x-snyk-broker-type header', async () => {
const response = await axiosClient.get(
`http://localhost:${bs.port}/broker/${brokerToken}/echo-param/xyz`,
);

expect(response.status).toEqual(200);
expect(response.data).toEqual('xyz');

expect(spyLogWarn).toHaveBeenCalledTimes(1);
expect(spyLogWarn).toHaveBeenCalledWith(
expect.any(Object),
'Error: Request does not contain the x-snyk-broker-type header',
);
});
});