diff --git a/common/models/rest-api.ts b/common/models/rest-api.ts index 49db977e4..5142961b2 100644 --- a/common/models/rest-api.ts +++ b/common/models/rest-api.ts @@ -8,6 +8,8 @@ import { OttApiRequestVoteSchema, OttApiRequestAddToQueueSchema, OttApiRequestRemoveFromQueueSchema, + OttApiRequestAccountRecoveryStartSchema, + OttApiRequestAccountRecoveryVerifySchema, } from "./zod-schemas"; import { z } from "zod"; @@ -83,12 +85,10 @@ export type OttApiResponseAddPreview = { export type OttApiRequestVote = z.infer; -export type OttApiRequestAccountRecoveryStart = { - email?: string; - username?: string; -}; +export type OttApiRequestAccountRecoveryStart = z.infer< + typeof OttApiRequestAccountRecoveryStartSchema +>; -export type OttApiRequestAccountRecoveryVerify = { - verifyKey: string; - newPassword: string; -}; +export type OttApiRequestAccountRecoveryVerify = z.infer< + typeof OttApiRequestAccountRecoveryVerifySchema +>; diff --git a/common/models/zod-schemas.ts b/common/models/zod-schemas.ts index d1bc71ce4..e44239a3f 100644 --- a/common/models/zod-schemas.ts +++ b/common/models/zod-schemas.ts @@ -41,3 +41,17 @@ export const OttApiRequestAddToQueueSchema = z.union([ export const OttApiRequestRemoveFromQueueSchema = z.object({ ...VideoIdSchema.shape, }); + +export const OttApiRequestAccountRecoveryStartSchema = z.union([ + z.object({ + email: z.string().email().min(3), + }), + z.object({ + username: z.string(), + }), +]); + +export const OttApiRequestAccountRecoveryVerifySchema = z.object({ + verifyKey: z.string(), + newPassword: z.string(), +}); diff --git a/server/tests/unit/api/accountrecovery.spec.ts b/server/tests/unit/api/accountrecovery.spec.ts index a97a469db..653eb0063 100644 --- a/server/tests/unit/api/accountrecovery.spec.ts +++ b/server/tests/unit/api/accountrecovery.spec.ts @@ -29,7 +29,7 @@ describe("Account Recovery", () => { app = (await main()).app; emailUser = await usermanager.registerUser({ - email: "email@localhost", + email: "email@localhost.com", username: "email user", password: "test1234", }); @@ -60,7 +60,7 @@ describe("Account Recovery", () => { }); it.each([ - ["email", "email@localhost"], + ["email", "email@localhost.com"], ["username", "email user"], ])( "should send a recovery email when using %s field in request", @@ -77,7 +77,7 @@ describe("Account Recovery", () => { const mailer: MockMailer = usermanager.mailer as MockMailer; expect(mailer.sentEmails).toHaveLength(1); expect(mailer.sentEmails[0]).toMatchObject({ - to: "email@localhost", + to: "email@localhost.com", }); } ); @@ -115,7 +115,7 @@ describe("Account Recovery", () => { ); it("should change the password when the verify key is valid", async () => { - await redisClient.set("accountrecovery:foo", "email@localhost"); + await redisClient.set("accountrecovery:foo", "email@localhost.com"); const body: OttApiRequestAccountRecoveryVerify = { verifyKey: "foo", @@ -139,7 +139,7 @@ describe("Account Recovery", () => { { verifyKey: "foo", newPassword: "asdf" }, { verifyKey: "bar", newPassword: "asdf1234" }, ])("should not change the password when the request is bad: %s", async (body: any) => { - await redisClient.set("accountrecovery:foo", "email@localhost"); + await redisClient.set("accountrecovery:foo", "email@localhost.com"); const resp = await request(app) .post("/api/user/recover/verify") diff --git a/server/usermanager.ts b/server/usermanager.ts index 7f54bc192..d1c5b70e8 100644 --- a/server/usermanager.ts +++ b/server/usermanager.ts @@ -34,6 +34,12 @@ import type { } from "ott-common/models/rest-api"; import { counterHttpErrors } from "./metrics"; import { OttException } from "ott-common/exceptions"; +import { + OttApiRequestAccountRecoveryStartSchema, + OttApiRequestAccountRecoveryVerifySchema, +} from "ott-common/models/zod-schemas"; +import { ZodError } from "zod"; +import { fromZodError } from "zod-validation-error"; const pwd = securePassword(); const log = getLogger("usermanager"); @@ -739,25 +745,25 @@ async function isEmailTaken(email: string): Promise { .then(room => (room ? true : false)) .catch(() => false); } + +function isEmailRequest(request: OttApiRequestAccountRecoveryStart): request is { email: string } { + return "email" in request; +} + const accountRecoveryStart: RequestHandler< unknown, OttResponseBody<{}>, OttApiRequestAccountRecoveryStart > = async (req, res) => { + const body = OttApiRequestAccountRecoveryStartSchema.parse(req.body); if (conf.get("rate_limit.enabled")) { await consumeRateLimitPoints(res, req.ip, 100); } - if (!("email" in req.body) && !("username" in req.body)) { - throw new BadApiArgumentException( - "email,username", - "Either email or username is required." - ); - } - const query = { - user: req.body.email ?? req.body.username, + user: isEmailRequest(body) ? body.email : body.username, }; + const user = await getUser(query); if (!user.email) { @@ -776,22 +782,17 @@ const accountRecoveryVerify: RequestHandler< OttResponseBody<{}>, OttApiRequestAccountRecoveryVerify > = async (req, res) => { - if (!("verifyKey" in req.body)) { - throw new BadApiArgumentException("verifyKey", "missing"); - } - if (!("newPassword" in req.body)) { - throw new BadApiArgumentException("newPassword", "missing"); - } + const body = OttApiRequestAccountRecoveryVerifySchema.parse(req.body); if (conf.get("rate_limit.enabled")) { await consumeRateLimitPoints(res, req.ip, 10); } - if (!isPasswordValid(req.body.newPassword)) { + if (!isPasswordValid(body.newPassword)) { throw new BadPasswordError(); } - const accountEmail = await redisClient.get(`accountrecovery:${req.body.verifyKey}`); + const accountEmail = await redisClient.get(`accountrecovery:${body.verifyKey}`); if (!accountEmail) { throw new InvalidVerifyKey(); } @@ -801,12 +802,12 @@ const accountRecoveryVerify: RequestHandler< throw new UserNotFound(); } - await changeUserPassword(user, req.body.newPassword, { + await changeUserPassword(user, body.newPassword, { // we already validated the password above validatePassword: false, }); - await redisClient.del(`accountrecovery:${req.body.verifyKey}`); + await redisClient.del(`accountrecovery:${body.verifyKey}`); if (req.token) { await tokens.setSessionInfo(req.token, { isLoggedIn: true, user_id: user.id }); @@ -893,6 +894,14 @@ const errorHandler: ErrorRequestHandler = (err: Error, req, res) => { success: false, error: err, }); + } else if (err instanceof ZodError) { + err = fromZodError(err); + res.status(400).json({ + success: false, + error: { + name: err.name, + }, + }); } else { log.error(`Unhandled exception: path=${req.path} ${err.name} ${err.message} ${err.stack}`); res.status(500).json({