From 1b7b71eaec6cf1df21578286e9c6b1e549126645 Mon Sep 17 00:00:00 2001 From: Jesse Wierzbinski Date: Thu, 25 Apr 2024 08:50:30 -1000 Subject: [PATCH] refactor(api): :art: Refactor request parser --- packages/request-parser/index.ts | 97 +++++++++---------- .../tests/request-parser.test.ts | 22 ++--- packages/server-handler/index.ts | 1 + server/api/api/auth/login/index.ts | 49 ++++++---- tests/oauth.test.ts | 2 + 5 files changed, 92 insertions(+), 79 deletions(-) diff --git a/packages/request-parser/index.ts b/packages/request-parser/index.ts index cbae659c..9559978b 100644 --- a/packages/request-parser/index.ts +++ b/packages/request-parser/index.ts @@ -16,19 +16,24 @@ export class RequestParser { * @throws Error if body is invalid */ async toObject() { - try { - switch (await this.determineContentType()) { - case "application/json": - return this.parseJson(); - case "application/x-www-form-urlencoded": - return this.parseFormUrlencoded(); - case "multipart/form-data": - return this.parseFormData(); - default: - return this.parseQuery(); - } - } catch { - return {} as T; + switch (await this.determineContentType()) { + case "application/json": + return { + ...(await this.parseJson()), + ...this.parseQuery(), + }; + case "application/x-www-form-urlencoded": + return { + ...(await this.parseFormUrlencoded()), + ...this.parseQuery(), + }; + case "multipart/form-data": + return { + ...(await this.parseFormData()), + ...this.parseQuery(), + }; + default: + return { ...this.parseQuery() } as T; } } @@ -57,7 +62,7 @@ export class RequestParser { // Check if body is valid JSON try { - await this.request.json(); + await this.request.clone().json(); return "application/json"; } catch { // This is not JSON @@ -65,7 +70,7 @@ export class RequestParser { // Check if body is valid FormData try { - await this.request.formData(); + await this.request.clone().formData(); return "multipart/form-data"; } catch { // This is not FormData @@ -90,44 +95,38 @@ export class RequestParser { * @throws Error if body is invalid */ private async parseFormData(): Promise> { - const formData = await this.request.formData(); + const formData = await this.request.clone().formData(); const result: Partial = {}; - // Check if there are any files in the FormData - if ( - Array.from(formData.values()).some((value) => value instanceof Blob) - ) { - for (const [key, value] of formData.entries()) { - if (value instanceof Blob) { - result[key as keyof T] = value as T[keyof T]; - } else if (key.endsWith("[]")) { - const arrayKey = key.slice(0, -2) as keyof T; - if (!result[arrayKey]) { - result[arrayKey] = [] as T[keyof T]; - } - - (result[arrayKey] as FormDataEntryValue[]).push(value); - } else { - result[key as keyof T] = value as T[keyof T]; - } + // Extract the files from the FormData + for (const [key, value] of formData.entries()) { + if (value instanceof Blob) { + result[key as keyof T] = value as T[keyof T]; } - } else { - // Convert to URLSearchParams and parse as query - const searchParams = new URLSearchParams([ - ...formData.entries(), - ] as [string, string][]); - - const parsed = parse(searchParams.toString(), { - parseArrays: true, - interpretNumericEntities: true, - }); - - return castBooleanObject( - parsed as PossiblyRecursiveObject, - ) as Partial; } - return result; + const formDataWithoutFiles = new FormData(); + for (const [key, value] of formData.entries()) { + if (!(value instanceof Blob)) { + formDataWithoutFiles.append(key, value); + } + } + + // Convert to URLSearchParams and parse as query + const searchParams = new URLSearchParams([ + ...formDataWithoutFiles.entries(), + ] as [string, string][]); + + const parsed = parse(searchParams.toString(), { + parseArrays: true, + interpretNumericEntities: true, + }); + + const casted = castBooleanObject( + parsed as PossiblyRecursiveObject, + ) as Partial; + + return { ...result, ...casted }; } /** @@ -163,7 +162,7 @@ export class RequestParser { * @throws Error if body is invalid * @returns JavaScript object of type T */ - private parseQuery(): Partial { + parseQuery(): Partial { const parsed = parse( new URL(this.request.url).searchParams.toString(), { diff --git a/packages/request-parser/tests/request-parser.test.ts b/packages/request-parser/tests/request-parser.test.ts index e724b9da..e0eb2a61 100644 --- a/packages/request-parser/tests/request-parser.test.ts +++ b/packages/request-parser/tests/request-parser.test.ts @@ -7,7 +7,7 @@ describe("RequestParser", () => { const request = new Request( "http://localhost?param1=value1¶m2=value2", ); - const result = await new RequestParser(request).toObject<{ + const result = await new RequestParser(request).parseQuery<{ param1: string; param2: string; }>(); @@ -18,30 +18,30 @@ describe("RequestParser", () => { const request = new Request( "http://localhost?test[]=value1&test[]=value2", ); - const result = await new RequestParser(request).toObject<{ + const result = await new RequestParser(request).parseQuery<{ test: string[]; }>(); - expect(result.test).toEqual(["value1", "value2"]); + expect(result?.test).toEqual(["value1", "value2"]); }); test("With Array of objects", async () => { const request = new Request( "http://localhost?test[][key]=value1&test[][value]=value2", ); - const result = await new RequestParser(request).toObject<{ + const result = await new RequestParser(request).parseQuery<{ test: { key: string; value: string }[]; }>(); - expect(result.test).toEqual([{ key: "value1", value: "value2" }]); + expect(result?.test).toEqual([{ key: "value1", value: "value2" }]); }); test("With Array of multiple objects", async () => { const request = new Request( "http://localhost?test[][key]=value1&test[][value]=value2&test[][key]=value3&test[][value]=value4", ); - const result = await new RequestParser(request).toObject<{ + const result = await new RequestParser(request).parseQuery<{ test: { key: string[]; value: string[] }[]; }>(); - expect(result.test).toEqual([ + expect(result?.test).toEqual([ { key: ["value1", "value3"], value: ["value2", "value4"] }, ]); }); @@ -50,7 +50,7 @@ describe("RequestParser", () => { const request = new Request( "http://localhost?param1=value1¶m2=value2&test[]=value1&test[]=value2", ); - const result = await new RequestParser(request).toObject<{ + const result = await new RequestParser(request).parseQuery<{ param1: string; param2: string; test: string[]; @@ -116,8 +116,8 @@ describe("RequestParser", () => { const result = await new RequestParser(request).toObject<{ file: File; }>(); - expect(result.file).toBeInstanceOf(File); - expect(await result.file?.text()).toEqual("content"); + expect(result?.file).toBeInstanceOf(File); + expect(await result?.file?.text()).toEqual("content"); }); test("With Array", async () => { @@ -131,7 +131,7 @@ describe("RequestParser", () => { const result = await new RequestParser(request).toObject<{ test: string[]; }>(); - expect(result.test).toEqual(["value1", "value2"]); + expect(result?.test).toEqual(["value1", "value2"]); }); test("With all three at once", async () => { diff --git a/packages/server-handler/index.ts b/packages/server-handler/index.ts index d68fd99b..998ea68f 100644 --- a/packages/server-handler/index.ts +++ b/packages/server-handler/index.ts @@ -110,6 +110,7 @@ export const processRoute = async ( const parsedRequest = await new RequestParser(request.clone()) .toObject() .catch(async (err) => { + console.log(err); await logger.logError( LogLevel.ERROR, "Server.RouteRequestParser", diff --git a/server/api/api/auth/login/index.ts b/server/api/api/auth/login/index.ts index fc2cd952..f5c9b6e2 100644 --- a/server/api/api/auth/login/index.ts +++ b/server/api/api/auth/login/index.ts @@ -26,6 +26,31 @@ export const meta = applyConfig({ export const schema = z.object({ email: z.string().email(), password: z.string().min(2).max(100), + scope: z.string().optional(), + redirect_uri: z.string().url().optional(), + response_type: z.enum([ + "code", + "token", + "none", + "id_token", + "code id_token", + "code token", + "token id_token", + "code token id_token", + ]), + client_id: z.string(), + state: z.string().optional(), + code_challenge: z.string().optional(), + code_challenge_method: z.enum(["plain", "S256"]).optional(), + prompt: z + .enum(["none", "login", "consent", "select_account"]) + .optional() + .default("none"), + max_age: z + .number() + .int() + .optional() + .default(60 * 60 * 24 * 7), }); export const querySchema = z.object({ @@ -91,22 +116,7 @@ export default apiRoute(async (req, matchedRoute, extraData) => { "Invalid email or password", ); - const parsedQuery = await new RequestParser( - new Request(req.url), - ).toObject(); - - if (!parsedQuery) { - return errorResponse("Invalid query", 400); - } - - const parsingResult = querySchema.safeParse(parsedQuery); - - if (parsingResult && !parsingResult.success) { - // Return a 422 error with the first error message - return errorResponse(fromZodError(parsingResult.error).toString(), 422); - } - - const { client_id } = parsingResult.data; + const { client_id } = extraData.parsedRequest; // Try and import the key const privateKey = await crypto.subtle.importKey( @@ -145,9 +155,10 @@ export default apiRoute(async (req, matchedRoute, extraData) => { if (application.website) searchParams.append("website", application.website); - // Add all data that is not undefined - for (const [key, value] of Object.entries(parsingResult.data)) { - if (value !== undefined) searchParams.append(key, String(value)); + // Add all data that is not undefined except email and password + for (const [key, value] of Object.entries(extraData.parsedRequest)) { + if (key !== "email" && key !== "password" && value !== undefined) + searchParams.append(key, value); } // Redirect to OAuth authorize with JWT diff --git a/tests/oauth.test.ts b/tests/oauth.test.ts index 2f278494..a59e5ebf 100644 --- a/tests/oauth.test.ts +++ b/tests/oauth.test.ts @@ -77,6 +77,8 @@ describe("POST /api/auth/login/", () => { ), ); + console.log(await response.text()); + expect(response.status).toBe(302); expect(response.headers.get("location")).toBeDefined(); const locationHeader = new URL(