cdimascio / express-openapi-validator

🦋 Auto-validates api requests, responses, and securities using ExpressJS and an OpenAPI 3.x specification
MIT License
887 stars 204 forks source link

Feature: Extendable fileUploader #248

Open dotMortis opened 4 years ago

dotMortis commented 4 years ago

Hello again :)

I'm going to use multer to upload files, but there are some specific issues with your file uploader. I want to handle the file upload with multer and then the file should be uploaded to an AWS S3 Bucket.

When I use your fileUploader, the file is only saved in my local folder. It would be really nice if I could add a middleware to your fileUploader so that I can process the new File after multer.

Another option is to set the fileUploader to false and add my own middleware logic. However, if the fileUploader is set false, your validator will no longer validate my openapi definitions for files.

Is there a workaround or am I missing something?

Thank you!

dotMortis commented 4 years ago

This is wrong. It's more like your validator can't validate the body anymore. It will fail if the fileUploader is false and the body property "image" is required even if the correct body is sent.

    requestBody:
        content:
          multipart/form-data:
            schema:
              description: default
              type: object
              additionalProperties: false
              required:
                -  image
              properties:
                image:
                  type: string
                  format: binary

In addition, middleware should be able to be attached to the beginning and end of the fileUploader so that authorization logic can be added.

cdimascio commented 3 years ago

@Haruki-Mortis apologies for the late reply. this is certainly something that can use improvement. we'll need to provide a way to handle the validation, then call next to enable the implementor to provide a custom file upload implementation. i will think on this. let me know if you have suggestions. ultimately, it means improving how the multipart middleware works

PRs are welcome, if you want to take a stab at it

dotMortis commented 3 years ago

@cdimascio sry for the late reply, too.

How about something like this (just a quick example):

types.ts

// extended fileuploader options
export type FileUploaderOptions = {
  multer: multer.Options;
  preMiddleware?: FileUploaderMiddlewareOptions[];
  postMiddleware?: FileUploaderMiddlewareOptions[];
};

// define a middlware
// optional for specific path(s)
export type FileUploaderMiddlewareOptions = {
  middleware: OpenApiRequestHandler;
  onPath?: RegExp | string;
};

export interface OpenApiValidatorOpts {
  apiSpec: OpenAPIV3.Document | string;
  validateResponses?: boolean | ValidateResponseOpts;
  validateRequests?: boolean | ValidateRequestOpts;
  validateSecurity?: boolean | ValidateSecurityOpts;
  ignorePaths?: RegExp;
  securityHandlers?: SecurityHandlers;
  coerceTypes?: boolean | 'array';
  unknownFormats?: true | string[] | 'ignore';
  fileUploader?: boolean | FileUploaderOptions; // Update this one
  multerOpts?: multer.Options;
  $refParser?: {
    mode: 'bundle' | 'dereference';
  };
  operationHandlers?: false | string | OperationHandlerOptions;
  validateFormats?: false | 'fast' | 'full';
}

openapi.multipart.ts

// add some validation logic to middlewares
export function toMultipartMiddleware(
  middlewareOpts: FileUploaderMiddlewareOptions[],
  req: OpenApiRequest,
  pContext: Promise<OpenApiContext>,
): OpenApiRequestHandler[] {
  return middlewareOpts?.map<OpenApiRequestHandler>(
    (middlewareOpts: FileUploaderMiddlewareOptions) => (
      req: OpenApiRequest,
      res: Response,
      next: NextFunction,
    ) =>
      pContext
        .then(() => {
          // default isOnPath should be true
          let isOnPath = true;
          // if onPath is set validate the 
          if (middlewareOpts.onPath) {
            // get current requested path
            const currentPath = (<OpenApiRequestMetadata>req.openapi)
                .openApiRoute;
            //validate the path
            isOnPath =
              typeof middlewareOpts.onPath === 'string'
                ? currentPath.includes(middlewareOpts.onPath)
                : currentPath.match(middlewareOpts.onPath)
                ? true
                : false;
          }

          if (isMultipart(req) && isOnPath) {
            //call the pre/postmiddleware if all requirements are valid
            return middlewareOpts.middleware(req, res, next);
          } else {
            // else just call next
            next();
          }
        })
        .catch(next),
  ) || [];
}

openapi.validator.ts

export class OpenApiValidator {
  ...
  constructor {
    ...
    if (options.fileUploader == null) options.fileUploader = true
    ...
  }
  ...
  installMiddleware(spec: Promise<Spec>): OpenApiRequestHandler[] {
    ...
    if (this.options.fileUploader) {
      // multipart middleware
      const {
        multer,
        preMiddleware = [],
        postMiddleware = [],
      }: FileUploaderOptions =
        typeof this.options.fileUploader === 'boolean'
          ? undefined
          : this.options.fileUploader;

      // install premiddlwares
      middlewares.push(...toMultipartMiddleware(preMiddleware, pContext));

      // install multer
      let fumw;
      middlewares.push((req, res, next) =>
        pContext
          .then((context) => {
            fumw = fumw || this.multipartMiddleware(context);
            return fumw(req, res, next);
          })
          .catch(next),
      );

      // install postmiddlewares
      middlewares.push(...toMultipartMiddleware(postMiddleware, pContext));
    }
    ...
  }
  ...
  private multipartMiddleware(context: OpenApiContext) {
    return middlewares.multipart(context, {
      // get multer options if set
      multerOpts: this.options.fileUploader['multer'],
      unknownFormats: this.options.unknownFormats,
    });
  }
  ...
}
cdimascio commented 3 years ago

@Haruki-Mortis Thanks so much for this proposal. Would you mind putting together a PR with your idea? It will be great to expand the capability here.

Note: I generally try to apply this rule of thumb: "always use a standard express mechanism when possible. add a proprietary mechanism only when no other option exists".

With the rule of thumb in mind, here are some comments:

In terms of interface, can we get closer to the following?

export type FileUploaderOptions = {
  multer: multer.Options;
  handler: (req: Request, res: Response, next: NextFunction) => void 
};
dotMortis commented 3 years ago

I'm working on it this week.