Skip to content

Commit 8c04e56

Browse files
hansotthansott
authored
Merge pull request #404 from AikidoSec/rate-limiting-user
Do not rate limit by ip if user is set
2 parents 582d667 + fd9314f commit 8c04e56

File tree

13 files changed

+454
-36
lines changed

13 files changed

+454
-36
lines changed

‎end2end/server/app.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
// This is a insecure mock server for testing purposes
12
const express = require("express");
23
const config = require("./src/handlers/getConfig");
34
const captureEvent = require("./src/handlers/captureEvent");
45
const listEvents = require("./src/handlers/listEvents");
56
const createApp = require("./src/handlers/createApp");
67
const checkToken = require("./src/middleware/checkToken");
8+
const updateConfig = require("./src/handlers/updateConfig");
79

810
const app = express();
911

@@ -12,9 +14,11 @@ const port = process.env.PORT || 3000;
1214
app.use(express.json());
1315

1416
app.get("/api/runtime/config", checkToken, config);
15-
app.post("/api/runtime/events", checkToken, captureEvent);
17+
app.post("/api/runtime/config", checkToken, updateConfig);
1618

1719
app.get("/api/runtime/events", checkToken, listEvents);
20+
app.post("/api/runtime/events", checkToken, captureEvent);
21+
1822
app.post("/api/runtime/apps", createApp);
1923

2024
app.listen(port, () => {

‎end2end/server/src/handlers/captureEvent.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
const { generateConfig } = require("../zen/config");
1+
const { getAppConfig } = require("../zen/config");
22
const { captureEvent: capture } = require("../zen/events");
33

44
module.exports = function captureEvent(req, res) {
@@ -14,5 +14,5 @@ module.exports = function captureEvent(req, res) {
1414
});
1515
}
1616

17-
return res.json(generateConfig(req.app));
17+
return res.json(getAppConfig(req.app));
1818
};

‎end2end/server/src/handlers/getConfig.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
1-
const { generateConfig } = require("../zen/config");
1+
const { getAppConfig } = require("../zen/config");
22
module.exports = function getConfig(req, res) {
33
if (!req.app) {
44
throw new Error("App is missing");
55
}
66

7-
res.json(generateConfig(req.app));
7+
res.json(getAppConfig(req.app));
88
};

‎end2end/server/src/handlers/updateConfig.js

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
const { updateAppConfig, getAppConfig } = require("../zen/config");
2+
module.exports = function updateConfig(req, res) {
3+
if (!req.app) {
4+
throw new Error("App is missing");
5+
}
6+
7+
// Insecure input validation - but this is only a mock server
8+
if (
9+
!req.body ||
10+
typeof req.body !== "object" ||
11+
Array.isArray(req.body) ||
12+
!Object.keys(req.body).length
13+
) {
14+
return res.status(400).json({
15+
message: "Request body is missing or invalid",
16+
});
17+
}
18+
res.json({ success: updateAppConfig(req.app, req.body) });
19+
};

‎end2end/server/src/zen/config.js

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
const configs = [];
2+
13
function generateConfig(app) {
24
return {
35
success: true,
@@ -11,6 +13,27 @@ function generateConfig(app) {
1113
};
1214
}
1315

16+
function getAppConfig(app) {
17+
const existingConf = configs.find((config) => config.serviceId === app.id);
18+
if (existingConf) {
19+
return existingConf;
20+
}
21+
const newConf = generateConfig(app);
22+
configs.push(newConf);
23+
return newConf;
24+
}
25+
26+
function updateAppConfig(app, newConfig) {
27+
let index = configs.findIndex((config) => config.serviceId === app.serviceId);
28+
if (index === -1) {
29+
getAppConfig(app);
30+
index = configs.length - 1;
31+
}
32+
configs[index] = { ...configs[index], ...newConfig };
33+
return true;
34+
}
35+
1436
module.exports = {
15-
generateConfig,
37+
getAppConfig,
38+
updateAppConfig,
1639
};

‎end2end/tests/hono-xml-rate-limiting.test.js

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,176 @@
1+
const t = require("tap");
2+
const { spawn } = require("child_process");
3+
const { resolve } = require("path");
4+
const timeout = require("../timeout");
5+
6+
const pathToApp = resolve(__dirname, "../../sample-apps/hono-xml", "app.js");
7+
const testServerUrl = "http://localhost:5874";
8+
9+
let token;
10+
t.beforeEach(async () => {
11+
const response = await fetch(`${testServerUrl}/api/runtime/apps`, {
12+
method: "POST",
13+
});
14+
const body = await response.json();
15+
token = body.token;
16+
17+
// Apply rate limiting
18+
const updateConfigResponse = await fetch(
19+
`${testServerUrl}/api/runtime/config`,
20+
{
21+
method: "POST",
22+
headers: {
23+
"Content-Type": "application/json",
24+
Authorization: token,
25+
},
26+
body: JSON.stringify({
27+
endpoints: [
28+
{
29+
route: "/add",
30+
method: "POST",
31+
forceProtectionOff: false,
32+
rateLimiting: {
33+
enabled: true,
34+
maxRequests: 1,
35+
windowSizeInMS: 60 * 1000,
36+
},
37+
},
38+
],
39+
}),
40+
}
41+
);
42+
t.same(updateConfigResponse.status, 200);
43+
});
44+
45+
t.test("it rate limits requests", (t) => {
46+
const server = spawn(`node`, ["--preserve-symlinks", pathToApp, "4002"], {
47+
env: {
48+
...process.env,
49+
AIKIDO_DEBUG: "true",
50+
AIKIDO_BLOCKING: "true",
51+
AIKIDO_TOKEN: token,
52+
AIKIDO_URL: testServerUrl,
53+
},
54+
});
55+
56+
server.on("close", () => {
57+
t.end();
58+
});
59+
60+
server.on("error", (err) => {
61+
t.fail(err.message);
62+
});
63+
64+
let stdout = "";
65+
server.stdout.on("data", (data) => {
66+
stdout += data.toString();
67+
});
68+
69+
let stderr = "";
70+
server.stderr.on("data", (data) => {
71+
stderr += data.toString();
72+
});
73+
74+
// Wait for the server to start
75+
timeout(2000)
76+
.then(async () => {
77+
const resp1 = await fetch("http://127.0.0.1:4002/add", {
78+
method: "POST",
79+
body: "<cat><name>Njuska</name></cat>",
80+
headers: {
81+
"Content-Type": "application/xml",
82+
},
83+
signal: AbortSignal.timeout(5000),
84+
});
85+
t.same(resp1.status, 200);
86+
87+
const resp2 = await fetch("http://127.0.0.1:4002/add", {
88+
method: "POST",
89+
body: "<cat><name>Harry</name></cat>",
90+
headers: {
91+
"Content-Type": "application/xml",
92+
},
93+
signal: AbortSignal.timeout(5000),
94+
});
95+
t.same(resp2.status, 429);
96+
})
97+
.catch((error) => {
98+
t.fail(error.message);
99+
})
100+
.finally(() => {
101+
server.kill();
102+
});
103+
});
104+
105+
t.test("user rate limiting works", (t) => {
106+
const server = spawn(`node`, ["--preserve-symlinks", pathToApp, "4003"], {
107+
env: {
108+
...process.env,
109+
AIKIDO_DEBUG: "true",
110+
AIKIDO_BLOCKING: "true",
111+
AIKIDO_TOKEN: token,
112+
AIKIDO_URL: testServerUrl,
113+
},
114+
});
115+
116+
server.on("close", () => {
117+
t.end();
118+
});
119+
120+
server.on("error", (err) => {
121+
t.fail(err.message);
122+
});
123+
124+
let stdout = "";
125+
server.stdout.on("data", (data) => {
126+
stdout += data.toString();
127+
});
128+
129+
let stderr = "";
130+
server.stderr.on("data", (data) => {
131+
stderr += data.toString();
132+
});
133+
134+
// Wait for the server to start
135+
timeout(2000)
136+
.then(async () => {
137+
const resp1 = await fetch("http://127.0.0.1:4003/add", {
138+
method: "POST",
139+
body: "<cat><name>Njuska</name></cat>",
140+
headers: {
141+
"Content-Type": "application/xml",
142+
"X-User-Id": "user1",
143+
},
144+
signal: AbortSignal.timeout(5000),
145+
});
146+
t.same(resp1.status, 200);
147+
148+
const resp2 = await fetch("http://127.0.0.1:4003/add", {
149+
method: "POST",
150+
body: "<cat><name>Harry</name></cat>",
151+
headers: {
152+
"Content-Type": "application/xml",
153+
"X-User-Id": "user2",
154+
},
155+
signal: AbortSignal.timeout(5000),
156+
});
157+
t.same(resp2.status, 200);
158+
159+
const resp3 = await fetch("http://127.0.0.1:4003/add", {
160+
method: "POST",
161+
body: "<cat><name>Njuska</name></cat>",
162+
headers: {
163+
"Content-Type": "application/xml",
164+
"X-User-Id": "user1",
165+
},
166+
signal: AbortSignal.timeout(5000),
167+
});
168+
t.same(resp3.status, 429);
169+
})
170+
.catch((error) => {
171+
t.fail(error.message);
172+
})
173+
.finally(() => {
174+
server.kill();
175+
});
176+
});

‎library/agent/Context.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,7 @@ export type Context = {
1616
body: unknown; // Can be an object, string or undefined (the body is parsed by something like body-parser)
1717
cookies: Record<string, string>;
1818
attackDetected?: boolean;
19-
consumedRateLimitForIP?: boolean;
20-
consumedRateLimitForUser?: boolean;
19+
consumedRateLimit?: boolean;
2120
user?: { id: string; name?: string };
2221
source: string;
2322
route: string | undefined;

‎library/agent/context/user.test.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ t.test("it logs when setUser has invalid input", async () => {
9292
// @ts-expect-error User is undefined
9393
setUser(undefined);
9494
t.same(logger.getMessages(), [
95-
"setUser(...) expects an object with 'id' and 'name' properties, found undefined instead.",
95+
"setUser(...) can not be called with null or undefined.",
9696
]);
9797
logger.clear();
9898

@@ -115,6 +115,12 @@ t.test("it logs when setUser has invalid input", async () => {
115115
"setUser(...) expects an object with 'id' property.",
116116
]);
117117
logger.clear();
118+
119+
// @ts-expect-error Testing invalid input
120+
setUser(null);
121+
t.same(logger.getMessages(), [
122+
"setUser(...) can not be called with null or undefined.",
123+
]);
118124
});
119125

120126
t.test(

‎library/agent/context/user.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable max-lines-per-function */
12
import { isPlainObject } from "../../helpers/isPlainObject";
23
import { getInstance } from "../AgentSingleton";
34
import type { User } from "../Context";
@@ -12,6 +13,11 @@ export function setUser(u: { id: string | number; name?: string }) {
1213

1314
const user = u as unknown;
1415

16+
if (user === null || user === undefined) {
17+
agent.log(`setUser(...) can not be called with null or undefined.`);
18+
return;
19+
}
20+
1521
if (!isPlainObject(user)) {
1622
agent.log(
1723
`setUser(...) expects an object with 'id' and 'name' properties, found ${typeof user} instead.`

0 commit comments

Comments
 (0)