Skip to content

Commit 6e8fd73

Browse files
committed
security: fix auth vulnerabilities, add login audit trail
- handleRefresh: store auth_type/group_id server-side in users table instead of trusting client-controlled x-auth-type/x-group-id headers. Prevents privilege escalation via forged group membership. - handleTestConnection: added admin auth check (was unauthenticated SSRF oracle). - handleListUsers: restricted to admins only (was any authenticated user). - handleUpdateGroup: only full admins may reassign group adminId. - body size limits: readJsonBody 10MB, readRawBody 50MB. Oversized requests destroy the socket. - login_audit table: tracks login, group_login, refresh, logout events with user_id, group_id, ip_address, timestamp. Replaces users.last_login. - recordLoginAudit() helper + LoginAuditEvent enum in auth.ts. - getClientIp() respects x-forwarded-for header. - Tests: 2 new auth spec tests for LoginAuditEvent enum. Signed-off-by: Mike Lischke <mike@lischke-online.de>
1 parent 98404f5 commit 6e8fd73

4 files changed

Lines changed: 155 additions & 42 deletions

File tree

src/server/auth.ts

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -250,11 +250,15 @@ export const createRefreshToken = (): { raw: string; hash: string; maxAge: numbe
250250
* @returns The userId and new raw token if valid, or undefined.
251251
*/
252252
export const verifyAndRotateRefreshToken = async (adapter: IDatabaseAdapter,
253-
rawToken: string,): Promise<{ userId: number; newRawToken: string; } | undefined> => {
253+
rawToken: string,): Promise<{
254+
userId: number; newRawToken: string; authType?: string; groupId?: number;
255+
} | undefined> => {
254256
const hash = crypto.createHash("sha256").update(rawToken).digest("hex");
255257

256-
const rows = await adapter.query<{ id: number; }>(
257-
"SELECT id FROM users WHERE refresh_token_hash = ?",
258+
const rows = await adapter.query<{
259+
id: number; auth_type: string | null; group_id: number | null;
260+
}>(
261+
"SELECT id, auth_type, group_id FROM users WHERE refresh_token_hash = ?",
258262
[hash],
259263
);
260264

@@ -264,13 +268,43 @@ export const verifyAndRotateRefreshToken = async (adapter: IDatabaseAdapter,
264268
return undefined;
265269
}
266270

267-
const userId = rows[0].id;
271+
const { id: userId, auth_type: authType, group_id: groupId } = rows[0];
268272
const newRaw = crypto.randomBytes(32).toString("hex");
269273
const newHash = crypto.createHash("sha256").update(newRaw).digest("hex");
270274

271275
await adapter.execute("UPDATE users SET refresh_token_hash = ? WHERE id = ?", [newHash, userId]);
272276

273-
return { userId, newRawToken: newRaw };
277+
return {
278+
userId,
279+
newRawToken: newRaw,
280+
authType: authType ?? undefined,
281+
groupId: groupId ?? undefined,
282+
};
283+
};
284+
285+
export enum LoginAuditEvent {
286+
Login = "login",
287+
GroupLogin = "group_login",
288+
Refresh = "refresh",
289+
Logout = "logout",
290+
}
291+
292+
/**
293+
* Records a login audit event.
294+
*
295+
* @param adapter The database adapter.
296+
* @param userId The user ID.
297+
* @param event The type of login event.
298+
* @param groupId The group ID (only for group_login events).
299+
* @param ipAddress The client IP address (optional).
300+
*/
301+
export const recordLoginAudit = async (adapter: IDatabaseAdapter, userId: number, event: LoginAuditEvent,
302+
groupId?: number, ipAddress?: string): Promise<void> => {
303+
await adapter.execute(
304+
`INSERT INTO login_audit (user_id, event, group_id, ip_address)
305+
VALUES (?, ?, ?, ?)`,
306+
[userId, event, groupId ?? null, ipAddress ?? null],
307+
);
274308
};
275309

276310
/**

src/server/backend-db.sql

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,14 +56,29 @@ CREATE TABLE users (
5656
username VARCHAR(255) NOT NULL,
5757
password_hash VARCHAR(512) NOT NULL,
5858
refresh_token_hash VARCHAR(256) NULL,
59+
auth_type VARCHAR(16) NULL,
60+
group_id INT UNSIGNED NULL,
5961
display_name VARCHAR(255) NOT NULL,
60-
last_login TIMESTAMP NULL,
6162
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
6263
updated_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
6364
PRIMARY KEY (id),
6465
UNIQUE KEY uk_users_username (username)
6566
) ENGINE=InnoDB;
6667

68+
CREATE TABLE login_audit (
69+
id INT UNSIGNED NOT NULL AUTO_INCREMENT,
70+
user_id INT UNSIGNED NOT NULL,
71+
event ENUM('login', 'group_login', 'refresh', 'logout') NOT NULL,
72+
group_id INT UNSIGNED NULL,
73+
ip_address VARCHAR(45) NULL,
74+
created_at TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP,
75+
PRIMARY KEY (id),
76+
INDEX idx_audit_user_time (user_id, created_at),
77+
CONSTRAINT fk_audit_user
78+
FOREIGN KEY (user_id) REFERENCES users(id)
79+
ON DELETE CASCADE
80+
) ENGINE=InnoDB;
81+
6782
CREATE TABLE `groups` (
6883
id INT UNSIGNED NOT NULL AUTO_INCREMENT,
6984
name VARCHAR(255) NOT NULL,

src/server/backend.ts

Lines changed: 84 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
adminGroupName, worldGroupName, getWorldGroupId,
1717
buildCapabilities, checkPermission, createAccessToken, createRefreshToken,
1818
getPermissionSummary, hashPassword,
19-
hasUsers, isUserInAdminGroup, refreshTokenExpirySeconds,
19+
hasUsers, isUserInAdminGroup, LoginAuditEvent, recordLoginAudit, refreshTokenExpirySeconds,
2020
setOwner, addEntityGroup, removeEntityGroup, getExplicitEntityGroups, getExplicitOwner,
2121
verifyAndRotateRefreshToken, verifyPassword, verifyToken,
2222
AccessLevel,
@@ -91,6 +91,18 @@ const createAdapter = (): IDatabaseAdapter => {
9191
}
9292
};
9393

94+
// ---------- Network helpers ----------
95+
96+
const getClientIp = (req: IncomingMessage): string | undefined => {
97+
const forwarded = getHeader(req, "x-forwarded-for");
98+
99+
if (forwarded) {
100+
return forwarded.split(",")[0].trim() || undefined;
101+
}
102+
103+
return req.socket.remoteAddress ?? undefined;
104+
};
105+
94106
// ---------- JSON helpers ----------
95107

96108
const sendJson = (res: ServerResponse, data: unknown, status = 200): void => {
@@ -103,11 +115,22 @@ const sendError = (res: ServerResponse, message: string, status = 400): void =>
103115
sendJson(res, { error: message }, status);
104116
};
105117

106-
const readJsonBody = (req: IncomingMessage): Promise<Record<string, unknown>> => {
118+
const readJsonBody = (req: IncomingMessage, maxSize = 10 * 1024 * 1024): Promise<Record<string, unknown>> => {
107119
return new Promise((resolve, reject) => {
108120
const chunks: Buffer[] = [];
121+
let totalSize = 0;
109122

110123
req.on("data", (chunk: Buffer) => {
124+
totalSize += chunk.length;
125+
126+
if (totalSize > maxSize) {
127+
req.destroy();
128+
129+
reject(new Error("Request body too large"));
130+
131+
return;
132+
}
133+
111134
chunks.push(chunk);
112135
});
113136
req.on("end", () => {
@@ -471,6 +494,14 @@ const seedIfExists = async (targetAdapter: IDatabaseAdapter): Promise<void> => {
471494
};
472495

473496
const handleTestConnection = async (req: IncomingMessage, res: ServerResponse): Promise<void> => {
497+
const user = getAuthUser(req);
498+
499+
if (!user || !(await isUserInAdminGroup(adapter, user.userId))) {
500+
sendError(res, "Forbidden", 403);
501+
502+
return;
503+
}
504+
474505
const body = await readJsonBody(req);
475506

476507
const testConfig: IDatabaseConfig = {
@@ -976,12 +1007,14 @@ const handleLogin = async (req: IncomingMessage, res: ServerResponse): Promise<v
9761007
const accessToken = createAccessToken(payload);
9771008
const refreshToken = createRefreshToken();
9781009

979-
// Store the hash in the database for rotation.
1010+
// Store the hash in the database for rotation. Normal login: no group context.
9801011
await adapter.execute(
981-
"UPDATE users SET refresh_token_hash = ?, last_login = NOW() WHERE id = ?",
1012+
"UPDATE users SET refresh_token_hash = ?, auth_type = NULL, group_id = NULL WHERE id = ?",
9821013
[refreshToken.hash, user.id],
9831014
);
9841015

1016+
await recordLoginAudit(adapter, user.id, LoginAuditEvent.Login, undefined, getClientIp(req));
1017+
9851018
setRefreshTokenCookie(res, refreshToken.raw, refreshToken.maxAge);
9861019

9871020
const capabilities = await buildCapabilities(adapter, payload);
@@ -1056,10 +1089,12 @@ const handleGroupLogin = async (req: IncomingMessage, res: ServerResponse): Prom
10561089
const refreshToken = createRefreshToken();
10571090

10581091
await adapter.execute(
1059-
"UPDATE users SET refresh_token_hash = ?, last_login = NOW() WHERE id = ?",
1060-
[refreshToken.hash, anon.id],
1092+
"UPDATE users SET refresh_token_hash = ?, auth_type = 'group', group_id = ? WHERE id = ?",
1093+
[refreshToken.hash, group.id, anon.id],
10611094
);
10621095

1096+
await recordLoginAudit(adapter, anon.id, LoginAuditEvent.GroupLogin, group.id, getClientIp(req));
1097+
10631098
// Update group last_login.
10641099
await adapter.execute(
10651100
"UPDATE `groups` SET last_login = NOW() WHERE id = ?",
@@ -1120,28 +1155,11 @@ const handleRefresh = async (req: IncomingMessage, res: ServerResponse): Promise
11201155
const user = rows[0];
11211156
const admin = await isUserInAdminGroup(adapter, user.id);
11221157

1123-
// Preserve group-login info from the old access token, or from custom headers
1124-
// (sessionStorage backup for page reloads where the in-memory token is lost).
1125-
const authHeader = req.headers.authorization;
1126-
let authType: string | undefined;
1127-
let groupId: number | undefined;
1128-
1129-
if (authHeader?.startsWith("Bearer ")) {
1130-
const oldPayload = verifyToken(authHeader.slice(7));
1131-
1132-
if (oldPayload?.authType === "group") {
1133-
authType = oldPayload.authType;
1134-
groupId = oldPayload.groupId;
1135-
}
1136-
}
1137-
1138-
const headerAuthType = req.headers["x-auth-type"];
1139-
const headerGroupId = req.headers["x-group-id"];
1140-
1141-
if (!authType && headerAuthType === "group" && headerGroupId) {
1142-
authType = "group";
1143-
groupId = Number(headerGroupId);
1144-
}
1158+
// Restore group-login context from the database (set during handleLogin/handleGroupLogin).
1159+
// Never trust client-provided headers — they were only needed as a backup before this data
1160+
// was persisted server-side. The sessionStorage fallback on the frontend can now be removed.
1161+
const authType = result.authType;
1162+
const groupId = result.groupId;
11451163

11461164
const accessToken = createAccessToken({
11471165
userId: user.id,
@@ -1153,10 +1171,18 @@ const handleRefresh = async (req: IncomingMessage, res: ServerResponse): Promise
11531171

11541172
setRefreshTokenCookie(res, result.newRawToken, refreshTokenExpirySeconds);
11551173

1174+
await recordLoginAudit(adapter, user.id, LoginAuditEvent.Refresh, groupId, getClientIp(req));
1175+
11561176
sendJson(res, { token: accessToken });
11571177
};
11581178

1159-
const handleLogout = (req: IncomingMessage, res: ServerResponse): void => {
1179+
const handleLogout = async (req: IncomingMessage, res: ServerResponse): Promise<void> => {
1180+
const user = getAuthUser(req);
1181+
1182+
if (user) {
1183+
await recordLoginAudit(adapter, user.userId, LoginAuditEvent.Logout, undefined, getClientIp(req));
1184+
}
1185+
11601186
clearRefreshTokenCookie(res);
11611187
sendJson(res, { success: true });
11621188
};
@@ -1387,15 +1413,16 @@ const handleCreateInitialAdmin = async (req: IncomingMessage, res: ServerRespons
13871413
const handleListUsers = async (req: IncomingMessage, res: ServerResponse): Promise<void> => {
13881414
const user = getAuthUser(req);
13891415

1390-
if (!user) {
1416+
if (!user || !(await isUserInAdminGroup(adapter, user.userId))) {
13911417
sendError(res, "Forbidden", 403);
13921418

13931419
return;
13941420
}
13951421

13961422
const rows = await adapter.query(
1397-
`SELECT u.id, u.username, u.display_name, u.last_login, u.created_at, u.updated_at,
1398-
(ug.user_id IS NOT NULL) AS is_admin
1423+
`SELECT u.id, u.username, u.display_name, u.created_at, u.updated_at,
1424+
(ug.user_id IS NOT NULL) AS is_admin,
1425+
(SELECT MAX(la.created_at) FROM login_audit la WHERE la.user_id = u.id) AS last_login
13991426
FROM users u
14001427
LEFT JOIN user_groups ug ON u.id = ug.user_id
14011428
AND ug.group_id = (SELECT id FROM \`groups\` WHERE name = ?)
@@ -1727,6 +1754,15 @@ const handleUpdateGroup = async (req: IncomingMessage, res: ServerResponse): Pro
17271754
: undefined;
17281755
const adminId = body.adminId !== undefined ? (Number(body.adminId) || null) : undefined;
17291756

1757+
// Only full admins can reassign group ownership.
1758+
if (adminId !== undefined) {
1759+
if (!await isUserInAdminGroup(adapter, user.userId)) {
1760+
sendError(res, "Only admins can change the group owner.", 403);
1761+
1762+
return;
1763+
}
1764+
}
1765+
17301766
if (!name && description === undefined && color === undefined
17311767
&& password === undefined && adminId === undefined) {
17321768
sendError(res, "No fields to update");
@@ -2295,14 +2331,27 @@ interface IMultipartPart {
22952331
/**
22962332
* Reads the full raw request body.
22972333
*
2298-
* @param req The incoming HTTP request.
2334+
* @param req The incoming HTTP request.
2335+
* @param maxSize Maximum allowed body size in bytes (default 50 MB).
2336+
*
22992337
* @returns The full body as a Buffer.
23002338
*/
2301-
const readRawBody = (req: IncomingMessage): Promise<Buffer> => {
2339+
const readRawBody = (req: IncomingMessage, maxSize = 50 * 1024 * 1024): Promise<Buffer> => {
23022340
return new Promise((resolve, reject) => {
23032341
const chunks: Buffer[] = [];
2342+
let totalSize = 0;
23042343

23052344
req.on("data", (chunk: Buffer) => {
2345+
totalSize += chunk.length;
2346+
2347+
if (totalSize > maxSize) {
2348+
req.destroy();
2349+
2350+
reject(new Error("Request body too large"));
2351+
2352+
return;
2353+
}
2354+
23062355
chunks.push(chunk);
23072356
});
23082357
req.on("end", () => {
@@ -2529,7 +2578,7 @@ const handleRequest = async (req: IncomingMessage, res: ServerResponse): Promise
25292578
break;
25302579

25312580
case "logout":
2532-
handleLogout(req, res);
2581+
await handleLogout(req, res);
25332582

25342583
break;
25352584

tests/server/auth.spec.ts

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { describe, expect, it } from "vitest";
77

88
import { createAccessToken, createRefreshToken, verifyToken } from "../../src/server/auth.js";
9-
import { AccessLevel, adminGroupName, worldGroupName, EntityType } from "../../src/server/auth.js";
9+
import { AccessLevel, adminGroupName, worldGroupName, EntityType, LoginAuditEvent } from "../../src/server/auth.js";
1010

1111
describe("auth — JWT", () => {
1212
it("round-trip: access token", () => {
@@ -65,3 +65,18 @@ describe("auth — Constants", () => {
6565
expect(EntityType.Feature).toBe("feature");
6666
});
6767
});
68+
69+
describe("auth — LoginAuditEvent", () => {
70+
it("has four event types with correct string values", () => {
71+
expect(LoginAuditEvent.Login).toBe("login");
72+
expect(LoginAuditEvent.GroupLogin).toBe("group_login");
73+
expect(LoginAuditEvent.Refresh).toBe("refresh");
74+
expect(LoginAuditEvent.Logout).toBe("logout");
75+
});
76+
77+
it("all enum values are unique", () => {
78+
const values = Object.values(LoginAuditEvent);
79+
80+
expect(new Set(values).size).toBe(values.length);
81+
});
82+
});

0 commit comments

Comments
 (0)