Skip to content

feat: add example API with auth and user management - #1

Open
antoniociccia wants to merge 1 commit into
mainfrom
feat/example-api
Open

feat: add example API with auth and user management#1
antoniociccia wants to merge 1 commit into
mainfrom
feat/example-api

Conversation

@antoniociccia

Copy link
Copy Markdown
Owner

Added a sample Express API with:

  • Login endpoint with JWT authentication
  • User profile and management endpoints
  • Admin query endpoint
  • File upload handler
  • Rate limiting middleware

@antoniociccia

Copy link
Copy Markdown
Owner Author

github-bro Review

Summary: The code is vulnerable to SQL injection, file path traversal, and lacks proper rate limiting configuration.

🔴 Critical

SQL Injection (examples/vulnerable-api.ts:16)

The login endpoint is vulnerable to SQL injection. Use parameterized queries instead of string concatenation.

const user = db.prepare("SELECT * FROM users WHERE email = ? AND password = ?").get(email, password);

🔴 Critical

File Path Traversal (examples/vulnerable-api.ts:41)

The file upload handler is vulnerable to file path traversal. Use a safe directory and validate the filename.

const safeFilename = path.basename(req.body.filename);
fs.writeFileSync(`/uploads/${safeFilename}`, req.body.data);

🔴 Critical

Rate Limiting Configuration (examples/vulnerable-api.ts:47)

The rate limiter should be applied to specific routes, not globally. This can lead to false positives.

app.use("/login", (req, res, next) => {
  // Rate limiting logic for /login
});

🟡 Warning

Hardcoded Secret (examples/vulnerable-api.ts:13)

The JWT secret is hardcoded. Consider using environment variables or a secure vault to store secrets.

const SECRET = process.env.JWT_SECRET || "supersecret123";

🟢 Suggestion

Database Schema (examples/vulnerable-api.ts:5)

Consider adding schema validation for the database to ensure data integrity.

db.prepare("CREATE TABLE IF NOT EXISTS users (id INTEGER PRIMARY KEY, email TEXT UNIQUE, password TEXT, role TEXT)");

Verdict: REQUEST_CHANGES

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant