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

[OTE-132] new compliance logic for comlink address-related endpoints #1048

Merged
merged 5 commits into from
Feb 20, 2024
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
6 changes: 6 additions & 0 deletions indexer/services/comlink/__tests__/helpers/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,15 @@ export async function sendRequestToApp({
errorMsg,
expressApp,
expectedStatus = 200,
headers = {},
}: {
type: RequestMethod,
path: string,
body?: {},
errorMsg?: string,
expressApp: e.Express,
expectedStatus?: number,
headers?: Record<string, string>,
}) {
let req: request.Test;

Expand All @@ -53,6 +55,10 @@ export async function sendRequestToApp({
throw new Error(`Invalid type of request: ${type}`);
}

if (Object.keys(headers).length) {
await req.set(headers);
}
Comment on lines +58 to +60
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 6, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using await with req.set(headers) is unnecessary since set does not return a promise. Remove await to avoid confusion.

-    await req.set(headers);
+    req.set(headers);

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
if (Object.keys(headers).length) {
await req.set(headers);
}
if (Object.keys(headers).length) {
req.set(headers);
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: this seems legit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Christopher-Li: Great, I'm glad you find the suggestion helpful! If you haven't already, please go ahead and apply the change. If there's anything else you need, feel free to ask.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it returns a Promise.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!


const response: request.Response = await req.send(body);
if (response.status !== expectedStatus) {
console.log(response.body); // eslint-disable-line no-console
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,239 @@
import express from 'express';

import { BlockedCode, RequestMethod } from '../../src/types';
import Server from '../../src/request-helpers/server';
import { sendRequestToApp } from '../helpers/helpers';
import { complianceAndGeoCheck } from '../../src/lib/compliance-and-geo-check';
import { handleValidationErrors } from '../../src/request-helpers/error-handler';
import { checkSchema } from 'express-validator';
import {
ComplianceStatus,
ComplianceStatusTable,
dbHelpers,
testConstants,
testMocks,
} from '@dydxprotocol-indexer/postgres';
import request from 'supertest';
import {
INDEXER_COMPLIANCE_BLOCKED_PAYLOAD,
INDEXER_GEOBLOCKED_PAYLOAD,
isRestrictedCountryHeaders,
} from '@dydxprotocol-indexer/compliance';
import config from '../../src/config';

jest.mock('@dydxprotocol-indexer/compliance');

// Create a router to test the middleware with
const router: express.Router = express.Router();

const restrictedHeaders = {
'cf-ipcountry': 'US',
};

const nonRestrictedHeaders = {
'cf-ipcountry': 'SA',
};

router.get(
'/check-compliance-query',
checkSchema({
address: {
in: ['query'],
isString: true,
optional: true,
},
}),
handleValidationErrors,
complianceAndGeoCheck,
(req: express.Request, res: express.Response) => {
res.sendStatus(200);
},
);

router.get(
'/check-compliance-param/:address',
checkSchema({
address: {
in: ['params'],
isString: true,
},
}),
handleValidationErrors,
complianceAndGeoCheck,
(req: express.Request, res: express.Response) => {
res.sendStatus(200);
},
);

export const complianceCheckApp = Server(router);

describe('compliance-check', () => {
let isRestrictedCountrySpy: jest.SpyInstance;

beforeAll(async () => {
config.INDEXER_LEVEL_GEOBLOCKING_ENABLED = true;
await dbHelpers.migrate();
});

beforeEach(async () => {
isRestrictedCountrySpy = isRestrictedCountryHeaders as unknown as jest.Mock;
await testMocks.seedData();
});

afterAll(async () => {
await dbHelpers.teardown();
});

afterEach(async () => {
jest.restoreAllMocks();
await dbHelpers.clearData();
});

it('does not return 403 if no address in request', async () => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await sendRequestToApp({
type: RequestMethod.GET,
path: '/v4/check-compliance-query',
expressApp: complianceCheckApp,
expectedStatus: 200,
});
});

it.each([
['query', '/v4/check-compliance-query?address=random'],
['param', '/v4/check-compliance-param/random'],
])('does not return 403 if address in request is not in database (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 200,
});
});

it.each([
['query', '/v4/check-compliance-query?address=random'],
['param', '/v4/check-compliance-param/random'],
])('does not return 403 if address in request is not in database (%s) and non-restricted country', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 200,
headers: nonRestrictedHeaders,
});
});

it.each([
['query', `/v4/check-compliance-query?address=${testConstants.defaultAddress}`],
['param', `/v4/check-compliance-param/${testConstants.defaultAddress}`],
])('does not return 403 if address in request is not blocked (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await ComplianceStatusTable.create(testConstants.compliantStatusData);
await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 200,
});
});

it.each([
['query', `/v4/check-compliance-query?address=${testConstants.defaultAddress}`],
['param', `/v4/check-compliance-param/${testConstants.defaultAddress}`],
])('does not return 403 if address in request is in CLOSE_ONLY (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await ComplianceStatusTable.create({
...testConstants.compliantStatusData,
status: ComplianceStatus.CLOSE_ONLY,
});
await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 200,
});
});

it.each([
['query', `/v4/check-compliance-query?address=${testConstants.defaultAddress}`],
['param', `/v4/check-compliance-param/${testConstants.defaultAddress}`],
])('does not return 403 if address in request is in CLOSE_ONLY and from restricted country (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(true);
await ComplianceStatusTable.create({
...testConstants.compliantStatusData,
status: ComplianceStatus.CLOSE_ONLY,
});
await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 200,
});
});

it.each([
['query', `/v4/check-compliance-query?address=${testConstants.defaultAddress}`],
['param', `/v4/check-compliance-param/${testConstants.defaultAddress}`],
])('does return 403 if request is from restricted country (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(true);
const response: request.Response = await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 403,
headers: restrictedHeaders,
});

expect(response.body).toEqual(expect.objectContaining({
errors: expect.arrayContaining([{
msg: INDEXER_GEOBLOCKED_PAYLOAD,
code: BlockedCode.GEOBLOCKED,
}]),
}));
});

it.each([
['query', `/v4/check-compliance-query?address=${testConstants.blockedAddress}`],
['param', `/v4/check-compliance-param/${testConstants.blockedAddress}`],
])('does return 403 if address in request is blocked (%s)', async (
_name: string,
path: string,
) => {
isRestrictedCountrySpy.mockReturnValueOnce(false);
await ComplianceStatusTable.create(testConstants.noncompliantStatusData);
const response: request.Response = await sendRequestToApp({
type: RequestMethod.GET,
path,
expressApp: complianceCheckApp,
expectedStatus: 403,
});

expect(response.body).toEqual(expect.objectContaining({
errors: expect.arrayContaining([{
msg: INDEXER_COMPLIANCE_BLOCKED_PAYLOAD,
code: BlockedCode.COMPLIANCE_BLOCKED,
}]),
}));
});
});
131 changes: 0 additions & 131 deletions indexer/services/comlink/__tests__/lib/compliance-check.test.ts

This file was deleted.

Loading
Loading