Skip to content

Commit 0ea8018

Browse files
CSRF and XSS protection (#59)
* XSS protections Customize content-security-policy header Verify the origin or referer on state changes, failing if not present * Create CSRF token per session Transmit in /user/me Send back in X-CSRF-Token * Fix lint * Revert create app form changes It looks like these were meant for a separate PR? --------- Co-authored-by: FluxCapacitor2 <31071265+FluxCapacitor2@users.noreply.github.com>
1 parent 2cf6f8a commit 0ea8018

8 files changed

Lines changed: 119 additions & 13 deletions

File tree

backend/package-lock.json

Lines changed: 31 additions & 6 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

backend/src/handlers/getUser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ export const getUserHandler: HandlerMap["getUser"] = async (
88
res,
99
) => {
1010
const user = await getUserService.getUser(req.user.id);
11-
return json(200, res, user);
11+
return json(200, res, { ...user, csrfToken: req.user.csrfToken });
1212
};

backend/src/handlers/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ export type AuthenticatedRequest = ExpressRequest & {
5959
id: number;
6060
email?: string;
6161
name?: string;
62+
csrfToken?: string;
6263
};
6364
};
6465

backend/src/index.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,17 @@ app.use(
4747
);
4848

4949
// Sets several security-related HTTP headers
50-
app.use(helmet());
50+
app.use(
51+
helmet({
52+
contentSecurityPolicy: {
53+
directives: {
54+
"script-src": ["'self'"], // blocks inline <script></script>
55+
"connect-src": ["'self'"],
56+
"frame-ancestors": ["'none'"],
57+
},
58+
},
59+
}),
60+
);
5161

5262
app.use(cookieParser());
5363

backend/src/server/auth.ts

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
import { SpanStatusCode, trace } from "@opentelemetry/api";
2-
import express from "express";
2+
import express, { type Request } from "express";
3+
import crypto from "node:crypto";
34
import type { operations } from "../generated/openapi.ts";
45
import type { AuthenticatedRequest } from "../handlers/index.ts";
6+
import { env } from "../lib/env.ts";
57
import { logger } from "../logger.ts";
68
import {
79
InvalidIDPError,
@@ -33,10 +35,13 @@ router.get("/oauth_callback", async (req, res) => {
3335
req.session.nonce,
3436
);
3537

38+
const csrfToken = crypto.randomBytes(32).toString("hex");
39+
3640
req.session.user = {
3741
id: user.id,
3842
name: user.name,
3943
email: user.email,
44+
csrfToken,
4045
};
4146
return res.redirect("/dashboard");
4247
} catch (err) {
@@ -84,8 +89,14 @@ export const ALLOWED_ANONYMOUS_OPERATIONS: (keyof operations)[] = [
8489
"acmeChallenge",
8590
];
8691

92+
const isAllowedAnonymousRoute = (req: Request) => {
93+
return ALLOWED_ANONYMOUS_ROUTES.some(
94+
(path) => req.path === path || req.path.startsWith(`${path}/`),
95+
);
96+
};
97+
8798
router.use((req, res, next) => {
88-
if (ALLOWED_ANONYMOUS_ROUTES.some((path) => req.url.startsWith(path))) {
99+
if (isAllowedAnonymousRoute(req)) {
89100
next();
90101
return;
91102
}
@@ -102,4 +113,41 @@ router.use((req, res, next) => {
102113
next();
103114
});
104115

116+
const UNSAFE_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
117+
118+
const validateOrigin = (req: Request) => {
119+
for (const value of [req.get("origin"), req.get("referer")]) {
120+
let source: string;
121+
try {
122+
source = new URL(value).origin;
123+
} catch {
124+
continue;
125+
}
126+
127+
if (source === env.BASE_URL) {
128+
return true;
129+
}
130+
}
131+
return false;
132+
};
133+
134+
router.use((req, res, next) => {
135+
if (isAllowedAnonymousRoute(req) || !UNSAFE_METHODS.has(req.method)) {
136+
next();
137+
return;
138+
}
139+
140+
if (!validateOrigin(req)) {
141+
res.status(401).json({ code: 401, message: "Unauthorized" });
142+
return;
143+
}
144+
145+
if (req.session["user"].csrfToken !== req.headers["x-csrf-token"]) {
146+
res.status(401).json({ code: 401, message: "Unauthorized" });
147+
return;
148+
}
149+
150+
next();
151+
});
152+
105153
export default router;

backend/src/types.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ declare module "express-session" {
124124
id: number;
125125
name: string;
126126
email?: string;
127+
csrfToken?: string;
127128
};
128129

129130
code_verifier?: string;

frontend/src/lib/api.ts

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import {
1010
import createFetchClient from "openapi-fetch";
1111
import createClient from "openapi-react-query";
1212
import { toast } from "sonner";
13-
import { type paths } from "../generated/openapi";
13+
import { type components, type paths } from "../generated/openapi";
1414

1515
const acceptJson = new Headers();
1616
acceptJson.set("Accept", "application/json");
@@ -20,8 +20,6 @@ const fetchClient = createFetchClient<paths>({
2020
headers: acceptJson,
2121
});
2222

23-
export const api = createClient(fetchClient);
24-
2523
/**
2624
* When the user visits one of these pages, they won't be redirected to the sign-in page if they're logged out.
2725
*/
@@ -60,3 +58,23 @@ export const queryClient = new QueryClient({
6058
queryCache: new QueryCache({ onError }),
6159
mutationCache: new MutationCache({ onError }),
6260
});
61+
62+
const MUTATING_METHODS = new Set(["POST", "PUT", "PATCH", "DELETE"]);
63+
const USER_ME_QUERY_KEY = ["get", "/user/me", {}] as const;
64+
65+
fetchClient.use({
66+
onRequest({ request }) {
67+
if (MUTATING_METHODS.has(request.method)) {
68+
const user =
69+
queryClient.getQueryData<components["schemas"]["User"]>(
70+
USER_ME_QUERY_KEY,
71+
);
72+
if (user?.csrfToken) {
73+
request.headers.set("X-CSRF-Token", user.csrfToken);
74+
}
75+
}
76+
return request;
77+
},
78+
});
79+
80+
export const api = createClient(fetchClient);

openapi/openapi.yaml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2113,6 +2113,8 @@ components:
21132113
nullable: true
21142114
name:
21152115
type: string
2116+
csrfToken:
2117+
type: string
21162118
orgs:
21172119
type: array
21182120
items:
@@ -2134,6 +2136,7 @@ components:
21342136
- email
21352137
- name
21362138
- orgs
2139+
- csrfToken
21372140
UnassignedInstallation:
21382141
type: object
21392142
properties:

0 commit comments

Comments
 (0)