-
Notifications
You must be signed in to change notification settings - Fork 438
Description
Is your feature request related to a problem? Please describe.
PR #1489 should have limited access to the OED server routes to appropriate users. It is important to carefully verify this and make sure that future changes and new routes are equally secure. This proposes to make that happen.
Describe the solution you'd like
Some routes test for some form of proper user status where ones readily found are:
- src/server/test/web/login.js
- src/server/test/web/groups.js
- src/server/test/web/meters.js
- src/server/test/web/preferencesTest.js
- src/server/test/web/usersTest.js
They show a good point for doing better testing. In general, OED wants to make sure:
- Authorized users can access the route.
- Unauthorized users (or not logged in) cannot access appropriate routes.
- Each route returns the proper code/error for these conditions.
- Routes that change what they return (e.g., groups) based on user properly filter the information.
- Every route endpoint is tested.
- Anything else people think is a good idea.
While having the route testing in each route's test files has advantages, it makes it harder to be systematic and verify all routes are covered. Thus, the proposal is to create a route test file for all route endpoints and test them in a systematic and regular way. The one exception might be when a route changes what it returns based on user but that is uncertain and developers should decide. The details are up to the developer but the hope is:
- Standard functions can be used for the testing so changes improve the testing of all routes.
- An easy way is created to specify the route and its user expectations.
- A mechanism is created to verify all route endpoints are being tested. This may require separate testing (such as done to verify that the MPL comment is in all files) but any good mechanism that detects new/uncovered routes is fine. It should easily interact with the list the code uses to specify routes to test so they are always in sync with a single version of the list.
- Similarly, it would be nice to check all allowed user types to be sure they are getting full coverage.
In general, OED now has two standard middleware functions that specify access:
- optionalAuthMiddleware indicates the route is open or needs special internal checking of the user.
- adminAuthMiddleware indicates a route is only for an admin.
Middleware for other users may be needed/used to fix items.
Anyone working on this is encouraged to discuss items and proposed solutions with the OED team.
Describe alternatives you've considered
This needs to happen.
Additional context
As each route is inspected, ones that are unclear, not properly checked or other concerns may be found. Anyone working on this should reach out to discuss what should be done. It may also be the case that some routes need changes to conform to what is expected. This can be done by the people working on this or someone else if documented.