Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,20 @@ Server starts at `http://localhost:4000` by default.
- `LOGIN_RATE_LIMIT_WINDOW_MS`
- `LOGIN_RATE_LIMIT_BLOCK_MS`

### Issue #30: Secure backend data access with signed auth tokens + authorization

- Login now returns an HMAC-signed bearer token (replacing predictable demo tokens).
- Tokens include user id, role, and expiry, and are validated with constant-time signature checks.
- Data endpoints now require `Authorization: Bearer <token>` and enforce role access:
- `GET /api/orders` → users can only read their own orders; admins can read all.
- `POST /api/orders` → users can create only for themselves; admins can create for any user.
- `GET /api/orders/:id` → users can read only their own order; admins can read any order.
- `GET /api/bills/:spotId` and `DELETE /api/users/:userId` → admin only.
- Configure via env vars:
- `AUTH_TOKEN_SECRET`
- `AUTH_TOKEN_TTL_SECONDS`
- `CORS_ALLOW_ORIGIN`


## Available endpoints

Expand All @@ -41,10 +55,11 @@ Server starts at `http://localhost:4000` by default.
- `GET /api/catalog`
- `GET /api/catalog/:category` (`drinks`, `food`, `cigarettes`)
- `GET /api/spots`
- `GET /api/orders?spotId=...&userId=...`
- `POST /api/orders`
- `GET /api/bills/:spotId`
- `DELETE /api/users/:userId` (removes the user and all related records)
- `GET /api/orders?spotId=...&userId=...` (auth required)
- `GET /api/orders/:id` (auth required)
- `POST /api/orders` (auth required)
- `GET /api/bills/:spotId` (admin only)
- `DELETE /api/users/:userId` (admin only; removes the user and all related records)

## Example login payload

Expand Down
20 changes: 13 additions & 7 deletions backend/env.js
Original file line number Diff line number Diff line change
@@ -1,24 +1,30 @@
const dotenv = require("dotenv");
const { z } = require("zod");
import dotenv from 'dotenv';
import { z } from 'zod';

dotenv.config();

const envSchema = z.object({
VITE_SUPABASE_URL: z.string().url(),
VITE_SUPABASE_ANON_KEY: z.string().min(10),
PORT: z.string().optional()
PORT: z.string().optional(),
AUTH_TOKEN_SECRET: z.string().min(16).optional(),
AUTH_TOKEN_TTL_SECONDS: z.string().regex(/^\d+$/).optional(),
CORS_ALLOW_ORIGIN: z.string().optional(),
LOGIN_RATE_LIMIT_MAX_ATTEMPTS: z.string().regex(/^\d+$/).optional(),
LOGIN_RATE_LIMIT_WINDOW_MS: z.string().regex(/^\d+$/).optional(),
LOGIN_RATE_LIMIT_BLOCK_MS: z.string().regex(/^\d+$/).optional(),
});

const result = envSchema.safeParse(process.env);

if (!result.success) {
console.error("\n❌ Invalid environment configuration:\n");
console.error('\n❌ Invalid environment configuration:\n');

result.error.errors.forEach((err) => {
console.error(`- ${err.path.join(".")}: ${err.message}`);
console.error(`- ${err.path.join('.')}: ${err.message}`);
});

process.exit(1); // 🔥 FAIL FAST
process.exit(1);
}

module.exports = result.data;
export default result.data;
104 changes: 95 additions & 9 deletions backend/server.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,17 @@
import { createServer } from 'node:http';
import { createHmac, timingSafeEqual } from 'node:crypto';
import { URL } from 'node:url';
import { database, dbPath } from './db.js';
require("./env");
import "./env.js";

const port = Number(process.env.PORT || 4000);

const LOGIN_RATE_LIMIT_MAX_ATTEMPTS = Number(process.env.LOGIN_RATE_LIMIT_MAX_ATTEMPTS || 5);
const LOGIN_RATE_LIMIT_WINDOW_MS = Number(process.env.LOGIN_RATE_LIMIT_WINDOW_MS || 15 * 60 * 1000);
const LOGIN_RATE_LIMIT_BLOCK_MS = Number(process.env.LOGIN_RATE_LIMIT_BLOCK_MS || 15 * 60 * 1000);
const AUTH_TOKEN_SECRET = process.env.AUTH_TOKEN_SECRET || 'brocode-dev-secret-change-me';
const AUTH_TOKEN_TTL_SECONDS = Number(process.env.AUTH_TOKEN_TTL_SECONDS || 60 * 60 * 12);
const CORS_ALLOW_ORIGIN = process.env.CORS_ALLOW_ORIGIN || '*';
const loginAttempts = new Map();

const getLoginKey = (req, username) => {
Expand Down Expand Up @@ -62,18 +66,64 @@ const parseBearerToken = (authHeader) => {
return token;
};

const toBase64Url = (value) => Buffer.from(value).toString('base64url');

const signToken = (payload) =>
createHmac('sha256', AUTH_TOKEN_SECRET).update(payload).digest('base64url');

const generateAuthToken = (user) => {
const payload = {
sub: user.id,
role: user.role,
exp: Math.floor(Date.now() / 1000) + AUTH_TOKEN_TTL_SECONDS,
};

const payloadPart = toBase64Url(JSON.stringify(payload));
const signature = signToken(payloadPart);
return `${payloadPart}.${signature}`;
};

const verifyAuthToken = (token) => {
const [payloadPart, signaturePart] = token.split('.');
if (!payloadPart || !signaturePart) {
return null;
}

const expectedSignature = signToken(payloadPart);
const providedSignatureBuffer = Buffer.from(signaturePart, 'base64url');
const expectedSignatureBuffer = Buffer.from(expectedSignature, 'base64url');
if (providedSignatureBuffer.length !== expectedSignatureBuffer.length) {
return null;
}

if (!timingSafeEqual(providedSignatureBuffer, expectedSignatureBuffer)) {
return null;
}

try {
const payload = JSON.parse(Buffer.from(payloadPart, 'base64url').toString('utf-8'));
if (!payload.sub || !payload.exp || payload.exp < Math.floor(Date.now() / 1000)) {
return null;
}

return payload;
} catch {
return null;
}
};

const getUserFromAuthHeader = (authHeader) => {
const token = parseBearerToken(authHeader);
if (!token || !token.startsWith('demo-token-')) {
if (!token) {
return null;
}

const userId = token.slice('demo-token-'.length);
if (!userId) {
const verifiedPayload = verifyAuthToken(token);
if (!verifiedPayload) {
return null;
}

return database.getUserById(userId);
return database.getUserById(verifiedPayload.sub);
};

const recordFailedLoginAttempt = (key) => {
Expand All @@ -89,8 +139,8 @@ const recordFailedLoginAttempt = (key) => {
const sendJson = (res, statusCode, body) => {
res.writeHead(statusCode, {
'Content-Type': 'application/json',
'Access-Control-Allow-Origin': '*',
'Access-Control-Allow-Headers': 'Content-Type',
'Access-Control-Allow-Origin': CORS_ALLOW_ORIGIN,
'Access-Control-Allow-Headers': 'Content-Type, Authorization',
'Access-Control-Allow-Methods': 'GET,POST,DELETE,OPTIONS',
});
res.end(JSON.stringify(body));
Expand Down Expand Up @@ -166,7 +216,7 @@ const server = createServer(async (req, res) => {

clearRateLimitState(loginKey);

sendJson(res, 200, { token: `demo-token-${user.id}`, user });
sendJson(res, 200, { token: generateAuthToken(user), user });
return;
} catch (error) {
sendJson(res, 400, { error: error.message });
Expand Down Expand Up @@ -199,10 +249,23 @@ const server = createServer(async (req, res) => {
}

if (method === 'GET' && path === '/api/orders') {
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser) {
sendJson(res, 401, { error: 'Unauthorized' });
return;
}

const spotId = parsedUrl.searchParams.get('spotId');
const userId = parsedUrl.searchParams.get('userId');

const orders = database.getOrders({ spotId, userId });
if (authedUser.role !== 'admin' && userId && userId !== authedUser.id) {
sendJson(res, 403, { error: 'Forbidden' });
return;
}

const effectiveUserId = authedUser.role === 'admin' ? userId : authedUser.id;

const orders = database.getOrders({ spotId, userId: effectiveUserId });
sendJson(res, 200, orders);
return;
}
Expand Down Expand Up @@ -234,13 +297,24 @@ const server = createServer(async (req, res) => {

if (method === 'POST' && path === '/api/orders') {
try {
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser) {
sendJson(res, 401, { error: 'Unauthorized' });
return;
}

const { spotId, userId, items } = await readBody(req);

if (!spotId || !userId || !Array.isArray(items) || items.length === 0) {
sendJson(res, 400, { error: 'spotId, userId and at least one order item are required' });
return;
}

if (authedUser.role !== 'admin' && userId !== authedUser.id) {
sendJson(res, 403, { error: 'Forbidden' });
return;
}

if (!database.userExists(userId)) {
sendJson(res, 404, { error: `Unknown userId: ${userId}` });
return;
Expand All @@ -265,13 +339,25 @@ const server = createServer(async (req, res) => {
}

if (method === 'GET' && path.startsWith('/api/bills/')) {
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser || authedUser.role !== 'admin') {
sendJson(res, 403, { error: 'Forbidden' });
return;
Comment on lines 339 to +345

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Admin-only endpoints return 403 instead of 401 for unauthenticated requests

The GET /api/bills/:spotId and DELETE /api/users/:userId endpoints return HTTP 403 (Forbidden) when the request has no token or an invalid/expired token, instead of HTTP 401 (Unauthorized). Per HTTP semantics, 401 means "you are not authenticated" while 403 means "you are authenticated but lack permission." Returning 403 for unauthenticated requests conflates these two conditions.

Root Cause and Impact

At backend/server.js:339-345 and backend/server.js:350-354, the check if (!authedUser || authedUser.role !== 'admin') collapses both the unauthenticated case (!authedUser) and the unauthorized case (role !== 'admin') into a single 403 response.

Compare with the other protected endpoints like GET /api/orders at backend/server.js:252-256 and POST /api/orders at backend/server.js:300-303, which correctly return 401 first for unauthenticated users, and only check authorization afterward.

For example, a request with no Authorization header to GET /api/bills/spot-1 returns {"error": "Forbidden"} with status 403, when it should return {"error": "Unauthorized"} with status 401. This makes it harder for API clients to distinguish between "need to log in" vs "logged in but insufficient privileges," and breaks the pattern established by the other endpoints in this same PR.

Suggested change
}
if (method === 'GET' && path.startsWith('/api/bills/')) {
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser || authedUser.role !== 'admin') {
sendJson(res, 403, { error: 'Forbidden' });
return;
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser) {
sendJson(res, 401, { error: 'Unauthorized' });
return;
}
if (authedUser.role !== 'admin') {
sendJson(res, 403, { error: 'Forbidden' });
return;
}
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

}

const spotId = path.replace('/api/bills/', '');
const bill = database.getBillBySpotId(spotId);
sendJson(res, 200, bill);
return;
}

if (method === 'DELETE' && path.startsWith('/api/users/')) {
const authedUser = getUserFromAuthHeader(req.headers.authorization);
if (!authedUser || authedUser.role !== 'admin') {
sendJson(res, 403, { error: 'Forbidden' });
return;
}

const userId = path.replace('/api/users/', '');

if (!userId) {
Expand Down
Loading