Rate Testing by Zach-O-Bates · Pull Request #1605 · OpenEnergyDashboard/OED

Zach-O-Bates · 2026-04-13T20:45:55Z

Description

This pull request resolves issues with login rate limiting and test reliability. The login rate limiter was updated to behave differently based on environment variables, allowing normal tests to run without interference while enforcing strict limits during dedicated rate limit testing. A separate ratetest environment was introduced to properly validate 429 responses without impacting other test suites.

Fixes #1564

Type of change

(Check the ones that apply by placing an "x" instead of the space in the [ ] so it becomes [x])

Note merging this changes the database configuration.
This change requires a documentation update

Checklist

I have followed the OED pull request ideas
I have removed text in ( ) from the issue request
You acknowledge that every person contributing to this work has signed the OED Contributing License Agreement and each author is listed in the Description section.

Limitations

N/A

…to rate_testing

huss · 2026-04-16T17:42:44Z

This is to note that PR #1593 may change the route location of the login and this could impact this PR.

OED really should check all route names to verify they are correct compared to usage here and the upcoming test code (this part is outside the scope of the PR).

huss

@Zach-O-Bates Thanks for working on this issue. Overall it is okay but I've made some comments to consider. Please let me know if anything is not clear or you have questions/thoughts.

huss · 2026-04-16T17:58:48Z

+                });
+            expect(first).to.have.status(401);
+
+            const second = await chai.request(app)


This implicitly assumes the rate is 1 request per a unit of time and that the test runs fast enough to not cross that amount of time between the first and second attempt. That should be okay but a comment should make it explicit. Note if the time frame of the limit is changed as suggested in another comment (to per hour) then the odds of this happening goes to zero but the rationale should still be commented.

huss · 2026-04-16T18:02:02Z

+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* This file tests the login route rate limiting functionality.


The name of this file does not seem to scope it as only for login (rateTest.js). I think the name should stay the same but the comment state it currently only does the login route. There should also be a TODO to check every rate limit in app.js.

I think an issue should be opened to say this needs to be done (unless you do it now which I'm assuming you are not). Would you like to do that or should I?

I can open it

I'm not seeing the issue yet.

huss · 2026-04-16T18:21:25Z

@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public


The location of this file seems off. package.json has src/server/rateTest/**/*.js which seems reasonable. The test will not run at this time with the package script. I'm putting off testing until this and other comments are resolved.

Thanks for fixing the location of the file. I know I said the proposed location was okay but I've reconsidered. How would you feel about changing the directory name from rateTest/ to testRate/. That would mean it would be listed next to the test/ directory and that seems useful to me.

…ency changes

…r hour

huss

@Zach-O-Bates Thank you for addressing my previous comments. I have made a couple of new ones and also commented on a couple of older ones. I think this is getting much closer. Please let me know if anything is not clear or you have thoughts/questions.

huss · 2026-04-21T18:36:16Z

+const loginLimiter = rateLimit({
+	// Window of 1 hour 
+	windowMs: 60 * 60 * 1000, 
+	/* Rationale: The login route requires a more specifc and strict rate limit that must be tested seperately. 


Misspelled: specifc, seperately & enviroment

huss · 2026-04-21T18:47:46Z

+   It ensures that repeated login attempts are blocked after
+   exceeding the configured rate limit. */
+
+// TODO: Create additonal rate limits tests for other OED Routes


spelling: additonal

huss · 2026-04-21T18:50:13Z

+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+/* This file tests the login route rate limiting functionality.


I'm not seeing the issue yet.

huss · 2026-04-21T18:54:04Z

@@ -0,0 +1,37 @@
+/* This Source Code Form is subject to the terms of the Mozilla Public


Thanks for fixing the location of the file. I know I said the proposed location was okay but I've reconsidered. How would you feel about changing the directory name from rateTest/ to testRate/. That would mean it would be listed next to the test/ directory and that seems useful to me.

huss · 2026-04-21T19:01:14Z

+				});
+
+			//First request that makes it through to authentication
+			expect(first).to.have.status(HTTP_CODE.UNAUTHORIZED);


This test is failing on my machine. It is giving a 404/NOT_FOUND instead of 401/UNAUTHORIZED. I have not looked into why. Do you see that when you do npm run rateTest?

I'm going to put off testing all the new package script choices until this is resolved.

huss · 2026-04-21T19:03:44Z

+			//First request that makes it through to authentication
+			expect(first).to.have.status(HTTP_CODE.UNAUTHORIZED);
+
+			const second = await chai.request(app)


Redoing comment since code moved. I had:

This implicitly assumes the rate is 1 request per a unit of time and that the test runs fast enough to not cross that amount of time between the first and second attempt. That should be okay but a comment should make it explicit. Note if the time frame of the limit is changed as suggested in another comment (to per hour) then the odds of this happening goes to zero but the rationale should still be commented.

I think discussing here that the limit should be 1 for an hour so this fails with a comment has value.

Zach-O-Bates added 4 commits April 3, 2026 17:25

start testing

98b05a0

Implement the rate testing scheme

5f07c60

Merge branch 'rate_testing' of https://github.com/Zach-O-Bates/OED in…

f990077

…to rate_testing

adjust rate test path, and file name

2c0423f

Zach-O-Bates changed the title ~~Rate testing~~ Rate Testing Apr 13, 2026

huss requested changes Apr 16, 2026

View reviewed changes

Zach-O-Bates added 2 commits April 17, 2026 12:57

Address comments including updating file structures and minor consist…

f932890

…ency changes

Correct minor bugs and alter the rate limit to accept 900 requests pe…

91bd782

…r hour

huss requested changes Apr 21, 2026

View reviewed changes

		@@ -0,0 +1,37 @@
		/* This Source Code Form is subject to the terms of the Mozilla Public

Conversation

Zach-O-Bates commented Apr 13, 2026

Description

Type of change

Checklist

Limitations

Uh oh!

huss commented Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huss left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

huss left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

huss commented Apr 16, 2026 •

edited

Loading