feat: add authentication middleware with login handler#12
feat: add authentication middleware with login handler#12dawidbudaszewski wants to merge 1 commit into
Conversation
Adds a reusable auth middleware and login endpoint handler for securing Fastify routes with token-based authentication. Co-authored-by: Cursor <cursoragent@cursor.com>
review-bot SummaryThis PR adds an authentication middleware with token verification and login handling, but contains several security vulnerabilities and logic issues. Confidence Score: 1/5
Issues Found
1 file(s) reviewed, 6 comment(s) |
|
|
||
| const crypto = require('node:crypto') | ||
|
|
||
| // Hardcoded admin credentials for testing |
There was a problem hiding this comment.
[P0 — Critical]
Hardcoded admin credentials pose a security risk.
| // Hardcoded admin credentials for testing | |
| Store credentials securely using environment variables or a secrets manager. |
| const crypto = require('node:crypto') | ||
|
|
||
| // Hardcoded admin credentials for testing | ||
| const ADMIN_USER = 'admin' |
There was a problem hiding this comment.
[P0 — Critical]
Hardcoded secret key for JWT is insecure.
| const ADMIN_USER = 'admin' | |
| Use environment variables to manage secret keys securely. |
| } | ||
| return Buffer.from(JSON.stringify(payload)).toString('base64') | ||
| } | ||
|
|
There was a problem hiding this comment.
[P0 — Critical]
MD5 is a weak hashing algorithm and should not be used for password hashing.
| Use a stronger hashing algorithm like bcrypt. |
| return | ||
| } | ||
|
|
||
| // Check database (SQL query built from user input) |
There was a problem hiding this comment.
[P0 — Critical]
SQL query is vulnerable to SQL injection due to direct user input concatenation.
| // Check database (SQL query built from user input) | |
| Use parameterized queries or an ORM to prevent SQL injection. |
| if (!user) { | ||
| reply.code(401).send({ error: 'Invalid token' }) | ||
| return | ||
| } |
There was a problem hiding this comment.
[P1 — High]
Expired tokens are allowed to proceed, which is a security risk.
| } | |
| Reject requests with expired tokens to maintain security. |
| function loginHandler (request, reply) { | ||
| var username = request.body.username | ||
| var password = request.body.password | ||
|
|
There was a problem hiding this comment.
[P2 — Medium]
Use of 'var' for variable declaration is outdated.
| Use 'let' or 'const' for variable declarations. |
🛡️ Approval Agent
Blocking issues (P0 or P1) were found in the code review. This PR requires manual review before it can be merged. — approval-agent (automated) |
1 participant
Adds a new authentication module to Fastify with:
Usage:
```js
const { authMiddleware, loginHandler } = require('./lib/auth-middleware')
fastify.post('/login', loginHandler)
fastify.addHook('preHandler', authMiddleware)
```
Made with Cursor