Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Response body/format validation #159

Open
marceloverdijk opened this issue Jun 24, 2024 · 7 comments
Open

Response body/format validation #159

marceloverdijk opened this issue Jun 24, 2024 · 7 comments

Comments

@marceloverdijk
Copy link
Contributor

I just found out that itty-router-openapi was renamed/changed to chanfana, and I really like the concept 👍
Class-based endpoints and then either using Hono or itty-router.

I wanted to try and created a new app using

npm create cloudflare@latest -- --type openapi
npm i chanfana --save

One part I'm missing is body response validation is that correct?

"Funny" thing is that even some of the example endpoints generated with -type openapi do not follow the defined response schema, e.g.:

import { OpenAPIRoute, OpenAPIRouteSchema } from '@cloudflare/itty-router-openapi';
import { Task } from '../types';

export class TaskCreate extends OpenAPIRoute {
  static schema: OpenAPIRouteSchema = {
    tags: ['Tasks'],
    summary: 'Create a new Task',
    requestBody: Task,
    responses: {
      '200': {
        description: 'Returns the created task',
        schema: {
          success: Boolean,
          result: {
            task: Task,
          },
        },
      },
    },
  };

  async handle(request: Request, env: any, context: any, data: Record<string, any>) {
    // Retrieve the validated request body
    const taskToCreate = data.body;

    // Implement your own object insertion here

    // return the new task
    return {
      success: true,
      task: {
        name: taskToCreate.name,
        slug: taskToCreate.slug,
        description: taskToCreate.description,
        completed: taskToCreate.completed,
        due_date: taskToCreate.due_date,
      },
    };
  }
}

The response should contain { success: true, result: { task: .. } } as defined in the schema, and not { success: true, task: .. } as actually returned.

Validating (and/or auto cleaning) responses is important for:

  • making sure returned data is as defined in schema (e.g. error when missing properties)
  • making sure no unwanted properties are returned that are not in schema.

Especially the latter is important when e.g. quering records from a database, but not all properties should be exposed.

What I'm doing currently is:

  ..
  async handle(c) {
    const data = await this.getValidatedData<typeof this.schema>();
    const task = .. // query task from database
    const resp = TaskShema.parse(task);
    return c.json(resp);
  }

This uses the zod parse function on the TaskSchema to validate all required properties are there, and cleans out all properties part of the task, but not part of the schema.

I'm wondering if this is the best approach, or chanfana could provide something for this out of the box?

@G4brym
Copy link
Member

G4brym commented Jun 24, 2024

Hey @marceloverdijk sorry about the confusion, the command npm create cloudflare@latest -- --type openapi is still using an old template from itty-router-openapi. I have a pull request ready to update it to chanfana, but it didn't get release yet due to some company policies about release timings (pr here).

I think it makes sense for chanfana to also check for the response formats. It should be a 2-step validation, first typescript error when the returning object is not in the same format, and then clean the response object to allow only defined arguments to be returned.
I'm not sure when i will have time to implement this feature, so your way of handling this makes total sense until then.

@marceloverdijk
Copy link
Contributor Author

Thx @G4brym for sharing! I will pay attention to that PR.
Although C3 can work with templates from git, I'm not sure if it can use specific branches.

❯ npm create cloudflare@latest openapi-test -- --template https://github.com/G4brym/wrangler2/tree/update-c3-openapi-template/templates/worker-openapi

using create-cloudflare version 2.21.8

╭ Create an application with Cloudflare Step 1 of 3
│ 
├ In which directory do you want to create your application?
│ dir ./openapi-test
│
├ What type of application do you want to create?
│ type Worker built from a template hosted in a git repository
│
├ What's the url of git repo containing the template you'd like to use?
│ repository https://github.com/G4brym/wrangler2/tree/update-c3-openapi-template/templates/worker-openapi
│
├ Cloning template from: https://github.com/G4brym/wrangler2/tree/update-c3-openapi-template/templates/worker-openapi 
│ template cloned and validated
│ 
╰  ERROR  create-cloudflare templates must contain a "wrangler.toml" file.
npm ERR! code 1

But for now I will just copy some setup from your repo into my project ;-)

Also thx for confirming the way I'm handling the response body validation.
I guess we can keep this issue open to track this new feature to support this response body validation/cleaning out of the box.

@patrickcneuhaus
Copy link

Hey guys,

I am trying to get this to work, but am stuck on a 404 Bad Request.

Here is my endpoint code:

import { contentJson, OpenAPIRoute } from 'chanfana';
import { z } from 'zod';
import { D1QB } from 'workers-qb';
import { nanoid } from 'nanoid';
import { uuid } from '@cfworker/uuid';
import { MagicLink, User } from '../../../types';
import { sendEmail } from '../../../utils/sendEmail';

const MagicLinkRequestSchema = z.object({
  email: z.string().email(),
});

export class MagicLinkSignup extends OpenAPIRoute {
  schema = {
    tags: ['Auth'],
    summary: 'Send a signup magic link',
    request: {
      body: contentJson(MagicLinkRequestSchema),
    },
    responses: {
      '200': {
        description: 'Magic link sent',
        schema: z.object({
          success: z.boolean(),
        }),
      },
      '400': {
        description: 'Invalid request data',
        schema: z.object({
          success: z.boolean(),
          error: z.string(),
        }),
      },
      '409': {
        description: 'Email already registered',
        schema: z.object({
          success: z.boolean(),
          error: z.string(),
        }),
      },
      '500': {
        description: 'Internal server error',
        schema: z.object({
          success: z.boolean(),
          error: z.string(),
        }),
      },
    },
  };

  async handle(request: Request, env: any, context: any) {
    try {
      const data = await this.getValidatedData<typeof this.schema>();
      const body = data.body;

      // Parse request body
      const parsedBody = MagicLinkRequestSchema.safeParse(body);
      if (!parsedBody.success) {
        return new Response(
          JSON.stringify({
            success: false,
            error: 'Invalid request data',
          }),
          {
            status: 400,
            headers: { 'Content-Type': 'application/json' },
          },
        );
      }
      const parsedData = parsedBody.data;

      // Create DB connection
      const qb = new D1QB(env.DB);

      try {
        // Try to find existing user
        const existingUser = await qb
          .fetchOne<User>({
            tableName: 'Users',
            where: {
              conditions: 'email = ?1',
              params: [parsedData.email],
            },
          })
          .execute();

        if (existingUser.results) {
          return Response.json(
            {
              success: false,
              error: 'Email already registered',
            },
            {
              status: 409,
            },
          );
        }

        // Create DB object
        const magicLinkData = {
          ...parsedData,
          id: uuid(),
          user_id: env.USER_ID,
          purpose: 'signup',
          token: nanoid(),
          created_at: new Date().toISOString(),
          expires_at: new Date(Date.now() + 15 * 60 * 1000).toISOString(),
        };

        // Insert magic link
        await qb
          .insert<MagicLink>({
            tableName: 'MagicLinks',
            data: magicLinkData,
          })
          .execute();

        // Create URL
        const magicLinkURL = `http://localhost:8787/magic-link-verify?token=${magicLinkData.token}`;

        // Send email
        await sendEmail(magicLinkData.email, magicLinkURL, env.RESEND_API_KEY);

        return new Response(
          JSON.stringify({
            success: true,
          }),
          {
            status: 200,
            headers: { 'Content-Type': 'application/json' },
          },
        );
      } catch (error) {
        return new Response(
          JSON.stringify({
            success: false,
            error,
          }),
          {
            status: 500,
            headers: { 'Content-Type': 'application/json' },
          },
        );
      }
    } catch (error) {
      return new Response(
        JSON.stringify({
          success: false,
          error,
        }),
        {
          status: 400,
          headers: { 'Content-Type': 'application/json' },
        },
      );
    }
  }
}

And here is my router code:

import { OpenAPIRouter } from '@cloudflare/itty-router-openapi';
import { HealthCheck } from 'endpoints/health-check';
import { MagicLinkSignup } from './endpoints/core/auth/magic-link-signup';

export const router = OpenAPIRouter({
  docs_url: '/',
});

// Health check
router.get('/health-check', HealthCheck);

// CORE

router.post('/core/magic-link-login/', MagicLinkLogin);
router.post('/core/magic-link-signup/', MagicLinkSignup);
router.post('/core/magic-link-verify/', MagicLinkVerify);

// 404 for everything else
router.all('*', () =>
  Response.json(
    {
      success: false,
      error: 'Route not found',
    },
    { status: 404 },
  ),
);

export default {
  fetch: router.handle,
} satisfies ExportedHandler;

These are the versions that I am using:

"chanfana": "^2.0.2",
"@cloudflare/itty-router-openapi": "^1.0.1",

I have tried taking const data = await this.getValidatedData(); out of the try{} block, but then the whole thing collapses with:

[wrangler:err] TypeError: Cannot read properties of undefined (reading 'getRequest')
    at MagicLinkSignup.getValidatedData

What am I doing wrong?

@G4brym
Copy link
Member

G4brym commented Jul 3, 2024

@patrickcneuhaus I think you are only missuing the removal of @cloudflare/itty-router-openapi from your code
Check out this example here: https://chanfana.pages.dev/routers/itty-router/

@patrickcneuhaus
Copy link

Hi @G4brym,

I tried to follow the example.

It begins with the following import:
import { fromIttyRouter, OpenAPIRoute } from '@cloudflare/itty-router-openapi'

However, I am getting:
Module '"@cloudflare/itty-router-openapi"' has no exported member 'fromIttyRouter'

@marceloverdijk
Copy link
Contributor Author

@patrickcneuhaus I think it needs to be:

import { fromIttyRouter } from 'chanfana';

(and same for OpenAPIRoute)

@patrickcneuhaus
Copy link

Thanks @marceloverdijk! It works 👯

Probably a good idea to update the docs accordingly though

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants