Skip to content

Commit 21e0c69

Browse files
Antony1060Antony1060
authored
Fix some security issues (#131)
* Fix some security issues * Fix type
1 parent fb991e2 commit 21e0c69

8 files changed

Lines changed: 130 additions & 56 deletions

File tree

‎apps/backend/src/globals.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ type GlobalsType = {
1212
dbDatacenter: string;
1313
evaluatorEndpoint: string;
1414
redisUrl: string;
15-
oauthAllowedDomains: string[];
15+
oauthClientId: string;
1616
defaultOrganisationName: string;
1717
influxUrl: string;
1818
influxToken: string;
@@ -71,9 +71,7 @@ export const Globals: GlobalsType = {
7171
evaluatorEndpoint:
7272
process.env.EVALUATOR_ENDPOINT ?? "https://kontestis-evaluator-y7a5esl5qq-oa.a.run.app",
7373
redisUrl: process.env.REDIS_URL ?? "redis://localhost:6379",
74-
oauthAllowedDomains: process.env.OAUTH_ALLOWED_DOMAINS
75-
? process.env.OAUTH_ALLOWED_DOMAINS.split(",").filter(Boolean)
76-
: [],
74+
oauthClientId: process.env.OAUTH_CLIENT_ID ?? "",
7775
defaultOrganisationName: process.env.DEFAULT_ORGANISATION_NAME ?? "Kontestis",
7876
influxUrl: process.env.INFLUXDB_URL ?? "http://localhost:8086",
7977
influxToken: process.env.INFLUXDB_TOKEN ?? "devtoken",

‎apps/backend/src/lib/google.ts‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import { generateSnowflake } from "./snowflake";
1212

1313
type VerifyTokenResponse = {
1414
sub: string;
15-
hd: string;
15+
aud: string;
1616
email: string;
1717
email_verified: string;
1818
name: string;
@@ -43,19 +43,23 @@ const googleServiceTokenCache: TokenCache = {
4343
};
4444

4545
export const verifyToken = async (token: string): Promise<NiceTokenResponse> => {
46-
const niceGoogleResponse = await axios
47-
.get<VerifyTokenResponse>("https://oauth2.googleapis.com/tokeninfo", {
46+
const { data } = await axios.get<VerifyTokenResponse>(
47+
"https://oauth2.googleapis.com/tokeninfo",
48+
{
4849
params: { id_token: token },
49-
})
50-
.then(({ data }) => ({
51-
...R.omit(data, ["sub", "picture"]),
52-
id: data.sub,
53-
picture_url: data.picture,
54-
}));
50+
}
51+
);
52+
53+
if (!Globals.oauthClientId || data.aud !== Globals.oauthClientId)
54+
throw new Error("invalid token audience");
5555

56-
if (niceGoogleResponse.email_verified !== "true") throw new Error("email not verified");
56+
if (data.email_verified !== "true") throw new Error("email not verified");
5757

58-
return niceGoogleResponse;
58+
return {
59+
...R.omit(data, ["sub", "picture"]),
60+
id: data.sub,
61+
picture_url: data.picture,
62+
};
5963
};
6064

6165
export const processUserFromTokenData = async (tokenData: NiceTokenResponse): Promise<User> => {

‎apps/backend/src/lib/mail.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,17 +14,29 @@ const transporter = createTransport({
1414
},
1515
});
1616

17+
const escapeHtml = (value: string): string =>
18+
value
19+
.replaceAll("&", "&amp;")
20+
.replaceAll("<", "&lt;")
21+
.replaceAll(">", "&gt;")
22+
// eslint-disable-next-line quotes
23+
.replaceAll('"', "&quot;")
24+
.replaceAll("'", "&#39;");
25+
1726
export const sendRegistrationMail = async (user: User, code: string) => {
1827
Logger.debug("Sending verification email to: " + user.email);
1928

2029
const subject = "Kontestis - E-mail verification";
30+
const confirmationUrl = `${Globals.backendUrl}/api/auth/managed/confirm/${user.id}/${code}`;
2131
const text = `Hello ${user.full_name},
2232
23-
Please verify your email by clicking on the following link: ${Globals.backendUrl}/api/auth/managed/confirm/${user.id}/${code}`;
33+
Please verify your email by clicking on the following link: ${confirmationUrl}`;
34+
35+
const escapedConfirmationUrl = escapeHtml(confirmationUrl);
2436

25-
const html = `Hello ${user.full_name},
37+
const html = `Hello ${escapeHtml(user.full_name)},
2638
27-
Please verify your email by clicking on the following link: <a href="${Globals.backendUrl}/api/auth/managed/confirm/${user.id}/${code}">${Globals.backendUrl}/api/auth/managed/confirm/${user.id}/${code}</a>`;
39+
Please verify your email by clicking on the following link: <a href="${escapedConfirmationUrl}">${escapedConfirmationUrl}</a>`;
2840

2941
await transporter
3042
.sendMail({
@@ -74,7 +86,7 @@ export const sendMail = async (
7486
</p>
7587
<br/>
7688
<div>
77-
<p>${text.replaceAll("\n", "<br/>")}</p>
89+
<p>${escapeHtml(text).replaceAll("\n", "<br/>")}</p>
7890
</div>
7991
`,
8092
});

‎apps/backend/src/routes/auth/ManagedHandler.ts‎

Lines changed: 37 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { Redis } from "../../redis/Redis";
1717
import { RedisKeys } from "../../redis/RedisKeys";
1818
import { randomSequence } from "../../utils/random";
1919
import { respond } from "../../utils/response";
20+
import { isHttpUrl } from "../../utils/url";
2021

2122
const ManagedHandler = Router();
2223

@@ -35,44 +36,50 @@ const LoginSchema = Type.Object({
3536
password: Type.String({ minLength: 4, maxLength: 1 << 10 }),
3637
});
3738

38-
ManagedHandler.post("/login", useValidation(LoginSchema, { body: true }), async (req, res) => {
39-
const managedUser = await Database.selectOneFrom("managed_users", "*", {
40-
email: req.body.email,
41-
});
39+
ManagedHandler.post(
40+
"/login",
41+
useCaptchaSchema,
42+
useCaptcha,
43+
useValidation(LoginSchema, { body: true }),
44+
async (req, res) => {
45+
const managedUser = await Database.selectOneFrom("managed_users", "*", {
46+
email: req.body.email,
47+
});
4248

43-
if (!managedUser) throw new SafeError(StatusCodes.UNAUTHORIZED);
49+
if (!managedUser) throw new SafeError(StatusCodes.UNAUTHORIZED);
4450

45-
const verifyResult = await verify(managedUser.password, req.body.password);
51+
const verifyResult = await verify(managedUser.password, req.body.password);
4652

47-
if (!verifyResult) throw new SafeError(StatusCodes.UNAUTHORIZED);
53+
if (!verifyResult) throw new SafeError(StatusCodes.UNAUTHORIZED);
4854

49-
const user = await Database.selectOneFrom("users", "*", {
50-
id: managedUser.id,
51-
});
55+
const user = await Database.selectOneFrom("users", "*", {
56+
id: managedUser.id,
57+
});
5258

53-
if (!user) throw new SafeError(StatusCodes.INTERNAL_SERVER_ERROR);
59+
if (!user) throw new SafeError(StatusCodes.INTERNAL_SERVER_ERROR);
5460

55-
if (!managedUser.confirmed_at) {
56-
const confirmationCode = await Redis.get(
57-
RedisKeys.MANAGED_USER_CONFIRMATION_CODE(managedUser.id)
58-
);
61+
if (!managedUser.confirmed_at) {
62+
const confirmationCode = await Redis.get(
63+
RedisKeys.MANAGED_USER_CONFIRMATION_CODE(managedUser.id)
64+
);
5965

60-
if (confirmationCode !== null) {
61-
throw new SafeError(StatusCodes.UNPROCESSABLE_ENTITY);
62-
}
66+
if (confirmationCode !== null) {
67+
throw new SafeError(StatusCodes.UNPROCESSABLE_ENTITY);
68+
}
6369

64-
await processEmailVerification(user);
70+
await processEmailVerification(user);
6571

66-
throw new SafeError(StatusCodes.UNPROCESSABLE_ENTITY, "verification-repeat");
67-
}
72+
throw new SafeError(StatusCodes.UNPROCESSABLE_ENTITY, "verification-repeat");
73+
}
6874

69-
await processLogin(user, {
70-
newLogin: false,
71-
confirm: true,
72-
});
75+
await processLogin(user, {
76+
newLogin: false,
77+
confirm: true,
78+
});
7379

74-
return respond(res, StatusCodes.OK, { token: generateJwt(user.id, "managed", {}) });
75-
});
80+
return respond(res, StatusCodes.OK, { token: generateJwt(user.id, "managed", {}) });
81+
}
82+
);
7683

7784
const RegisterSchema = Type.Object({
7885
email: Type.RegEx(/^[^@]+@[^@]+\.[^@]+$/),
@@ -87,6 +94,9 @@ ManagedHandler.post(
8794
useCaptcha,
8895
useValidation(RegisterSchema, { body: true }),
8996
async (req, res) => {
97+
if (req.body.picture_url && !isHttpUrl(req.body.picture_url))
98+
throw new SafeError(StatusCodes.BAD_REQUEST);
99+
90100
const existingUser = await Database.selectOneFrom("users", ["id"], {
91101
email: req.body.email.toLowerCase(),
92102
});

‎apps/backend/src/utils/url.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
export const isHttpUrl = (url: string): boolean => {
2+
try {
3+
const parsedUrl = new URL(url);
4+
5+
return parsedUrl.protocol === "http:" || parsedUrl.protocol === "https:";
6+
} catch {
7+
return false;
8+
}
9+
};

‎apps/frontend/src/hooks/auth/useLogin.ts‎

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,25 @@ import { useMutation } from "react-query";
33
import { http, MutationHandler, wrapAxios } from "../../api/http";
44

55
type LoginVariables = {
6-
email: string;
7-
password: string;
6+
data: {
7+
email: string;
8+
password: string;
9+
};
10+
captcha_token: string;
811
};
912

1013
type LoginData = {
1114
token: string;
1215
};
1316

1417
export const useLogin: MutationHandler<LoginVariables, LoginData> = (options) =>
15-
useMutation((variables) => wrapAxios(http.post("/auth/managed/login", variables)), options);
18+
useMutation(
19+
({ data, captcha_token }) =>
20+
wrapAxios(
21+
http.post(
22+
`/auth/managed/login?captcha_token=${encodeURIComponent(captcha_token)}`,
23+
data
24+
)
25+
),
26+
options
27+
);

‎apps/frontend/src/pages/auth/LoginPage.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { Link } from "react-router-dom";
66
import { http, ServerData } from "../../api/http";
77
import { AaiEduButton } from "../../components/AaiEduButton";
88
import { TitledSection } from "../../components/TitledSection";
9+
import { withCaptcha } from "../../hoc/withCaptcha";
910
import { useTranslation } from "../../hooks/useTranslation";
1011
import { useTokenStore } from "../../state/token";
1112
import { ManagedLoginForm } from "./ManagedLoginForm";
@@ -112,8 +113,10 @@ const LoginBase: FC = () => {
112113
);
113114
};
114115

116+
const LoginBaseWithCaptcha = withCaptcha(LoginBase);
117+
115118
export const LoginPage: FC = () => (
116119
<GoogleOAuthProvider clientId={import.meta.env.VITE_OAUTH_CLIENT_ID}>
117-
<LoginBase />
120+
<LoginBaseWithCaptcha />
118121
</GoogleOAuthProvider>
119122
);

‎apps/frontend/src/pages/auth/ManagedLoginForm.tsx‎

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { zodResolver } from "@hookform/resolvers/zod";
22
import React, { FC } from "react";
3+
import { useGoogleReCaptcha } from "react-google-recaptcha-v3";
34
import { useForm } from "react-hook-form";
45
import { Link } from "react-router-dom";
56
import { theme } from "twin.macro";
@@ -22,14 +23,28 @@ type Properties = {
2223

2324
export const ManagedLoginForm: FC<Properties> = ({ onError, onEmailResent }) => {
2425
const { setToken } = useTokenStore();
26+
const { executeRecaptcha } = useGoogleReCaptcha();
2527

2628
const loginMutation = useLogin({
2729
onError: (error) => {
28-
if (error.status === 401) onError("Invalid email or password");
29-
else if (error.status === 422) {
30-
if (error.message === "verification-repeat") onEmailResent();
31-
else onError("Email not verified");
32-
} else onError("Something went wrong");
30+
switch (error.status) {
31+
case 401: {
32+
onError("Invalid email or password");
33+
break;
34+
}
35+
case 403: {
36+
onError("Captcha failed!");
37+
break;
38+
}
39+
case 422: {
40+
if (error.message === "verification-repeat") onEmailResent();
41+
else onError("Email not verified");
42+
43+
break;
44+
}
45+
default:
46+
onError("Something went wrong");
47+
}
3348
},
3449
onSuccess: (data) => {
3550
setToken(data.token);
@@ -45,7 +60,18 @@ export const ManagedLoginForm: FC<Properties> = ({ onError, onEmailResent }) =>
4560
});
4661

4762
const onSubmit = handleSubmit((data) => {
48-
loginMutation.mutate(data);
63+
// eslint-disable-next-line unicorn/no-useless-undefined
64+
onError(undefined);
65+
66+
if (!executeRecaptcha) {
67+
onError("Failed to load captcha");
68+
69+
return;
70+
}
71+
72+
executeRecaptcha()
73+
.then((token) => loginMutation.mutate({ data, captcha_token: token }))
74+
.catch(() => onError("Failed to load captcha"));
4975
});
5076

5177
return (

0 commit comments

Comments
 (0)