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

Decouple schema validation from core #2693

Closed
1 of 10 tasks
bollwyvl opened this issue Feb 8, 2022 · 12 comments
Closed
1 of 10 tasks

Decouple schema validation from core #2693

bollwyvl opened this issue Feb 8, 2022 · 12 comments
Assignees

Comments

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 8, 2022

Prerequisites

Description

ajv@6 has served this project well. The 6.* line (and indeed the 7.* line #2542) have reached end-of-life, and if new serious issues were discovered, may not be fixed. With the success of the decoupled, but co-maintained, themes it is perhaps time to consider co-developing multiple schema validators that meet different needs.

While client-side validation is an important part of the rjsf experience, and should be encouraged with a sensible default, picking a validator carries a number of trade-offs:

  • overall size
  • time to first load
  • time to subsequent load
  • time to validate
  • arbitrary code execution
  • JSON Schema drafts supported (the current ajv@8 drops draft4)

For example,

  • ajv@>7 makes pre-compiled standalone validators possible, which can be tree-shaken to avoid bringing the compiler
  • rjsf can be used to draw a form, with a given theme, and another technique entirely is used to validate it client-side

Proposal

  • move most of the logic in validate.js to a AJV6Validator class
    • define a new ValidationError to which an AjvError can be converted
    • move these to a new package, @rjsf/validator-ajv6
      • refactor the boilerplate of this class and errors to a NoOpValidator
        • move the base to a new @rjsf/validator-base package, as a dependency of @rjsf/core
  • add a new package @rjsf/validator-ajv8 to track features
  • add a validateFormData to the form props, somewhat like Injection of custom ajv instance via prop (+ removal of global ajv) #1286, and, depending on when it lands:
    • ... in @rjsf/core 3.*
      • use AJV6Validator.validateFormData default
    • ... in @rjsf/core 4.*
      • if available from a peerDependency on @rjsf/validator-ajv8
        • use a AJV8Validator.validateFormData
      • otherwise,
        • fall back to NoOpValidator
        • console.warn

Once the existing behavior and test suite have been

@epicfaace
Copy link
Member

I like this idea. This would also help address: #812, #1464.

@biggyspender
Copy link

Yes. I mentioned the possibility of a pluggable validator in #1464 (comment) as a means of sidestepping the need for script-src 'unsafe-eval' in a locked-down content security policy. Concerns have been raised on some of the products that I maintain that the inclusion of such a rule in the policy might be grounds for a warning when submitted for pen-testing. Would be great to see this move forward. Sadly, I don't have the time to contribute :(

@heath-freenome
Copy link
Member

@epicfaace @bollwyvl We are experiencing the unsafe-eval issue in our product and it seems like this might be the approach to take for me to fix it. I will begin digging into this for an @rjsf v5 change

@epicfaace
Copy link
Member

@heath-freenome earlier PR: #1286

@heath-freenome heath-freenome self-assigned this Apr 27, 2022
@heath-freenome
Copy link
Member

heath-freenome commented Apr 27, 2022

As it turns out, the new if-then-else condition code is now relying on the isValid() function in validate.js so implementing a NoOpValidator would probably break that feature (or at least cripple it). That said, after digging into things, I believe that refactoring the utils.js file out into its own package that @rjsf/core relies on would be necessary since the validate.js file relies on a few methods within it. Here is my updated proposal:

In a long-lived branch:

  • refactor the code utils.js into a new package, @rjsf/utils, breaking each function into its own file
    • convert each function into Typescript, moving all the types from @rjsf/core related to the utils function as well
    • implement a new ValidatorType interface that exposes the validateFormData(), toErrorList() and isValid() functions
      • NOTE: the validateFormData() function interface would NOT take additionalMetaSchemas or customFormats
    • define a new ValidationError to which an AjvError can be converted
    • update functions that use either validateFormData() or isValid() to take the ValidatorType as a parameter instead
    • update the documentation for the utils to refer to this new package and new interfaces
  • refactor the code in validate.js to an AJV6Validator class in a new package, @rjsf/validator-ajv6
    • have the package export a default ValidatorType interface implementation
    • add support to create a customized validator with additionalMetaSchemas and/or customFormats
    • add/update the validation documentation
  • in a new, breaking changes, major version, refactor @rjsf/core and all the themes to use the new @rjsf/utils package
    • replace the additionalMetaSchemas and customFormats props from Form & withTheme with a required validator prop of type ValidatorType
    • add a required validator prop of type ValidatorType on Form, putting it into a React context
      • Update Form to use validator.toErrorList()
    • add a new hook, useFormValidator() to get access to the validator
    • update all of the components that need validator for a utils function to get it from the context
    • update the index.d.ts file to import and re-export the interfaces from @rjsf/utils
      • also add/update types for the hook and FormProps, plus anything else that was changed
    • update the documentation to describe this new feature
  • add a new package @rjsf/validator-ajv8 to initially port AJV6Validator to AJV8Validator
    • update the documentation for this new validator
    • (later?) improve this to take advantage of a pre-compiled standalone validator
  • update the playground to select between validators
  • publish the new major version of @rjsf from the long-lived branch

@epicfaace
Copy link
Member

Suggested changes:

  • in an initial PR, break utils.js into separate package @rjsf/utils.
  • Default to ajv 6 validator
  • instead of publishing @rjsf/validator-ajv6, let's publish a single package and do @rjsf/validator-ajv/v6 and @rjsf/validator-ajv/v8 ...

@heath-freenome
Copy link
Member

Fixed in the v5 beta, see the 5.x migration guide

@Steffan-Ennis
Copy link

Steffan-Ennis commented Sep 20, 2022

This is really exciting stuff, I've had to monkey patch the global AJV instance in the past to make use of additional ajv key words. I'll be able to remove that code :)

@afd57
Copy link

afd57 commented Oct 20, 2022

Hello there, I upgraded rjsf/core to 5.0.0-beta.9. Still I need to add unsafe-eval for validation otherwise I am getting this error. #1973

Could you kindly clarify for me whether the rjsf/core form requires the unsafe-eval CSP?

Thank you

@heath-freenome
Copy link
Member

heath-freenome commented Oct 20, 2022 via email

@afd57
Copy link

afd57 commented Oct 20, 2022

@heath-freenome thank you for your reply. Yes, I tested both @rjsf/validator-ajv6 and @rjsf/validator-ajv8. Both required unsafe-eval.

Does it happen when you use the @rjsf/validator-ajv8?

On Wed, Oct 19, 2022 at 6:30 PM Aziz F Dagli @.> wrote: Hello there, I upgraded rjsf/core to 5.0.0-beta.9 https://github.com/rjsf-team/react-jsonschema-form/releases/tag/5.0.0-beta.9. Still I need to add unsafe-eval for validation otherwise I am getting this error. #1973 <#1973> Could you kindly clarify for me whether the rjsf/core form requires the unsafe-eval CSP? Thank you — Reply to this email directly, view it on GitHub <#2693 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMKJCZGFJ4RVBRYGRL5MAILWECOELANCNFSM5N2WP74A . You are receiving this because you modified the open/close state.Message ID: @.>
-- This e-mail is private and confidential and is for the addressee only. If misdirected, please notify us by telephone, confirming that it has been deleted from your system and any hard copies destroyed. You are strictly prohibited from using, printing, distributing or disseminating it or any information contained in it save to the intended recipient.

@lilyFromMars
Copy link

lilyFromMars commented Jan 7, 2023

@heath-freenome @afd57 as i understand, ajv will not fix CSP unsafe-eval issue, the only way is using ajv-cli to pre-compile the schema to a validation function, it will avoid run time eval problem. is there any way to plug this validation function to rjsf <Form validator={validator} />

i did this

npm install -g ajv-cli
ajv compile -s schema.json -o validate_schema.js

so i go something like this in validat_schema.js

"use strict";module.exports = validate20;module.exports.default = validate20;const schema22 = {"title":"A single-field form","type":"string"};function validate20(data, {instancePath="", parentData, parentDataProperty, rootData=data}={}){let vErrors = null;let errors = 0;if(typeof data !== "string"){validate20.errors = [{instancePath,schemaPath:"#/type",keyword:"type",params:{type: "string"},message:"must be string"}];return false;}validate20.errors = vErrors;return errors === 0;}

i am not clear on how to use this function to do the validation, thank you for the help

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