Skip to content

Commit

Permalink
Check every matching endpoint for allowed IPs
Browse files Browse the repository at this point in the history
  • Loading branch information
hansott committed Aug 7, 2024
1 parent 60eea73 commit eb140ff
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 21 deletions.
5 changes: 5 additions & 0 deletions library/agent/ServiceConfig.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { LimitedContext, matchEndpoint } from "../helpers/matchEndpoint";
import { matchEndpoints } from "../helpers/matchEndpoints";
import { Endpoint } from "./Config";

export class ServiceConfig {
Expand Down Expand Up @@ -29,6 +30,10 @@ export class ServiceConfig {
);
}

getEndpoints(context: LimitedContext) {
return matchEndpoints(context, this.nonGraphQLEndpoints);
}

getEndpoint(context: LimitedContext) {
return matchEndpoint(context, this.nonGraphQLEndpoints);
}
Expand Down
53 changes: 53 additions & 0 deletions library/helpers/matchEndpoints.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { Endpoint } from "../agent/Config";
import { Context } from "../agent/Context";
import { tryParseURLPath } from "./tryParseURLPath";

export type LimitedContext = Pick<Context, "url" | "method" | "route">;

Check warning on line 5 in library/helpers/matchEndpoints.ts

View workflow job for this annotation

GitHub Actions / build (18.x)

exported declaration 'LimitedContext' not used within other modules

export function matchEndpoints(context: LimitedContext, endpoints: Endpoint[]) {
const matches: Endpoint[] = [];

if (!context.method) {
return matches;
}

Check warning on line 12 in library/helpers/matchEndpoints.ts

View check run for this annotation

Codecov / codecov/patch

library/helpers/matchEndpoints.ts#L11-L12

Added lines #L11 - L12 were not covered by tests

const possible = endpoints.filter((endpoint) => {
if (endpoint.method === "*") {
return true;
}

Check warning on line 17 in library/helpers/matchEndpoints.ts

View check run for this annotation

Codecov / codecov/patch

library/helpers/matchEndpoints.ts#L16-L17

Added lines #L16 - L17 were not covered by tests

return endpoint.method === context.method;
});

const exact = possible.find((endpoint) => endpoint.route === context.route);
if (exact) {
matches.push(exact);
}

if (context.url) {
// req.url is relative, so we need to prepend a host to make it absolute
// We just match the pathname, we don't use the host for matching
const path = tryParseURLPath(context.url);
const wildcards = possible
.filter((endpoint) => endpoint.route.includes("*"))
.sort((a, b) => {
// Sort endpoints based on the amount of * in the route
return b.route.split("*").length - a.route.split("*").length;

Check warning on line 35 in library/helpers/matchEndpoints.ts

View check run for this annotation

Codecov / codecov/patch

library/helpers/matchEndpoints.ts#L34-L35

Added lines #L34 - L35 were not covered by tests
});

if (path) {
for (const wildcard of wildcards) {
const regex = new RegExp(
`^${wildcard.route.replace(/\*/g, "(.*)")}\/?$`,
"i"
);

if (regex.test(path)) {
matches.push(wildcard);
}

Check warning on line 47 in library/helpers/matchEndpoints.ts

View check run for this annotation

Codecov / codecov/patch

library/helpers/matchEndpoints.ts#L46-L47

Added lines #L46 - L47 were not covered by tests
}
}
}

return matches;
}
6 changes: 1 addition & 5 deletions library/sources/HTTPServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -638,11 +638,7 @@ t.test("it checks if IP can access route", async () => {
}),
]).then(([response1, response2, response3]) => {
t.equal(response1.statusCode, 200);
t.equal(response2.statusCode, 403);
t.same(
response2.body,
`Your IP address is not allowed to access this resource. (Your IP: ${getMajorNodeVersion() === 16 ? "::ffff:127.0.0.1" : "::1"})`
);
t.equal(response2.statusCode, 200);
t.equal(response3.statusCode, 403);
t.same(
response3.body,
Expand Down
42 changes: 42 additions & 0 deletions library/sources/http-server/ipAllowedToAccessRoute.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,3 +162,45 @@ t.test("it blocks request if not allowed IP address", async () => {
false
);
});

t.test("it checks every matching endpoint", async () => {
const agent = new Agent(
true,
new LoggerNoop(),
new ReportingAPIForTesting({
success: true,
allowedIPAddresses: [],
configUpdatedAt: 0,
blockedUserIds: [],
heartbeatIntervalInMS: 10 * 1000,
endpoints: [
{
route: "/posts/:id",
rateLimiting: undefined,
method: "POST",
allowedIPAddresses: ["3.4.5.6"],
forceProtectionOff: false,
},
{
route: "/posts/*",
rateLimiting: undefined,
method: "POST",
allowedIPAddresses: ["1.2.3.4"],
forceProtectionOff: false,
},
],
block: true,
}),
new Token("123"),
undefined
);

agent.start([]);

await new Promise((resolve) => setTimeout(resolve, 0));

t.same(
ipAllowedToAccessRoute({ ...context, remoteAddress: "3.4.5.6" }, agent),
true
);
});
32 changes: 16 additions & 16 deletions library/sources/http-server/ipAllowedToAccessRoute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,27 @@ export function ipAllowedToAccessRoute(context: Context, agent: Agent) {
return true;
}

const match = agent.getConfig().getEndpoint(context);
const matches = agent.getConfig().getEndpoints(context);

if (!match) {
return true;
}
for (const endpoint of matches) {
if (!Array.isArray(endpoint.allowedIPAddresses)) {
return true;
}

const { endpoint } = match;
if (endpoint.allowedIPAddresses.length === 0) {
return true;
}

if (!Array.isArray(endpoint.allowedIPAddresses)) {
return true;
}
if (!context.remoteAddress) {
return false;
}

if (endpoint.allowedIPAddresses.length === 0) {
return true;
}
const { allowedIPAddresses } = endpoint;

if (!context.remoteAddress) {
return false;
if (!allowedIPAddresses.includes(context.remoteAddress)) {
return false;
}
}

const { allowedIPAddresses } = endpoint;

return allowedIPAddresses.includes(context.remoteAddress);
return true;
}

0 comments on commit eb140ff

Please sign in to comment.