feat(oidc): provide better logging and output for the user when oidc attempt is made #6737
Conversation
dosubot
bot
added
the
size:XL
perfectra1n
changed the title
perfectra1n
changed the title
dosubot
bot
added
size:XXL
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| app.use(auth(openID.generateOAuthConfig())); | ||
|
|
||
| // Add OAuth error logging middleware AFTER auth middleware | ||
| app.use(openID.oauthErrorLogger); |
Check failure
Code scanning / CodeQL
Missing rate limiting High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 1 month ago
To fix the issue, we should add rate-limiting middleware to the application in order to prevent abuse of the authentication/authorization chain, including the openID.oauthErrorLogger middleware. The best practice is to use the popular express-rate-limit package. You should import this package near the top of your file, configure a sensible rate limit (for example, 100 requests per 15 minutes), and use app.use(limiter) immediately before the authentication middlewares, so that all upstream authentication attempts (and error loggers) will be protected. This approach ensures that expensive operations in the authentication chain are not abused by high-frequency requests. All changes are to be done in the shown region of apps/server/src/app.ts, including the import statement, definition/configuration of the limiter, and the application of the middleware.
| @@ -1,3 +1,4 @@ | ||
| import RateLimit from "express-rate-limit"; | ||
| import express from "express"; | ||
| import path from "path"; | ||
| import favicon from "serve-favicon"; | ||
| @@ -144,6 +145,13 @@ | ||
| }); | ||
| } | ||
|
|
||
| // Apply rate limiter before all authentication and error logging middleware | ||
| const limiter = RateLimit({ | ||
| windowMs: 15 * 60 * 1000, // 15 minutes | ||
| max: 100, // limit each IP to 100 requests per windowMs | ||
| }); | ||
| app.use(limiter); | ||
|
|
||
| // Register OAuth middleware | ||
| app.use(auth(openID.generateOAuthConfig())); | ||
|
|
| @@ -4,7 +4,8 @@ | ||
| "description": "The server-side component of TriliumNext, which exposes the client via the web, allows for sync and provides a REST API for both internal and external use.", | ||
| "private": true, | ||
| "dependencies": { | ||
| "better-sqlite3": "12.2.0" | ||
| "better-sqlite3": "12.2.0", | ||
| "express-rate-limit": "^8.0.1" | ||
| }, | ||
| "devDependencies": { | ||
| "@electron/remote": "2.1.3", |
| Package | Version | Security advisories |
| express-rate-limit (npm) | 8.0.1 | None |
|
Example output from this commit: |
Try to add some more logging (and tests) to try and help resolving #6444