PR for #1647 - add ui:props to all material-ui widgets

[dekelb]

This PR is a fix for issue #1647

Following the comments in the issue - it seems like the best option was to use the "ui:props" inside the uiSchema (of each field), which being pushed into the "options" variable inside the widget (as part of the WidgetProps).

The reason I decided to use the "ui:props" and not just add properties directly to the "options" is that some of those properties are already being used, and this might cause issues there.

Also - those properties are relevant specifically to the Components that we take from the Theme. I noticed some requests to also support Fields/Components from other libs - using this standard we will be able to support other field-specific properties.

[dekelb]

@epicfaace any update on this one? I'm not so sure why it didn't get into v2-alpha5

[heath-freenome]

One suggestion is to pull out the required, id or onChange props using something like the Lodash omit() function. Make a helper, like the rangeSpec() utility function, called maybe uiProps(), which extracts all. of the non-override-able variables and returns just the other stuff?

[Fedorov113]

Hello, any update on this?

[epicfaace]

@dekelb just thought of another thing -- what if someone inputs dangerouslySetInnerHTML into ui:props? This is essentially a security vulnerability if someone allows users to edit uiSchemas.

I'm thinking it would be best if, for each widget, we could define a whitelist of props that could be overridden using ui:props -- this would be best for security. Do you know how such a list could be assembled? Perhaps from the type definitions for material ui widgets?

[kedano]

Hi, I'm trying to figure out how I could use the variant prop and other adjustments to my schema with material-ui.

Could it be an idea like @epicfaace mention to whitelist at least the props that has something to do with the design in the material-ui doc? https://material-ui.com/api/text-field/

[howientc]

Any progress on this PR?

[epicfaace]

We need to add a whitelist for allowed props. See how it was done with the fluent ui theme.

[ronny-rentner]

We need to add a whitelist for allowed props. See how it was done with the fluent ui theme.

Why do you need an allowlist?

Just let all props through, if they do something or not, it doesn't hurt and it will work when upstream adds new props automatically.

[ronny-rentner]

@dekelb just thought of another thing -- what if someone inputs dangerouslySetInnerHTML into ui:props? This is essentially a security vulnerability if someone allows users to edit uiSchemas.

Why would that someone not validate and escape user input correctly as you would always do it with any kind of user input?

I'm thinking it would be best if, for each widget, we could define a whitelist of props that could be overridden using ui:props -- this would be best for security. Do you know how such a list could be assembled? Perhaps from the type definitions for material ui widgets?

The list will change constantly. It should just let extra props through that it doesn't know. It's a common scheme to let pass all other props down in the hierarchy.

[ronny-rentner]

PS: Check out upstream: https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/TextField/TextField.js They also have "...other" and pass through all props.

[epicfaace]

Why would that someone not validate and escape user input correctly as you would always do it with any kind of user input?

One use case of react-jsonschema-form is allowing users to specify and save their own JSON Schemas (which I use myself in a form builder). It doesn't seem trivial to validate that input; at least, there don't exist libraries that could do that automatically (something like XSS sanitizers) and it would require you to perform the validation yourself.

The current behavior of react-jsonschema-form is that no possible JSON Schema / uiSchema can have a security impact, because there's no way to set the dangerouslyInnerHTML prop. With this change, this guarantee changes, because suddenly uiSchemas become untrusted user input and can cause XSS vulnerabilities. At the very least, this would be a backwards incompatible change.

[kedano]

Hi, any update on this? Still waiting on the whitelisting?

[wight554]

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53 It should be safe enough in this case I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that cc @epicfaace

[wight554]

Currently I can do something like this with TextWidget and I'd like to be able to do this for other widgets as well:

export const TextWidget = ({ options, ...props }: WidgetProps) => {
  const allProps: object & WhitelistedProps =
    typeof options?.props === 'object' && options?.props !== null ? options?.props : {};

  const pickedProps = pick(allProps, PROPS_WHITELIST);

  return <Widgets.TextWidget {...props} options={options} {...pickedProps} />;
};

[epicfaace]

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53 It should be safe enough in this case I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that cc @epicfaace

@wight554 sorry I don't understand what you are proposing, could you provide more details?

[wight554]

While this one is seems to be dead, can you pass rest props to all components like you do here https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/TextWidget/TextWidget.tsx#L53 It should be safe enough in this case I'd like to create own minimal wrappers for components with own props whitelist, but currently I need to rewrite all non-textwidget widgets to achieve that cc @epicfaace

@wight554 sorry I don't understand what you are proposing, could you provide more details?

The idea was to add props spreading to all widget components E.g. there's no ... props in select widget https://github.com/rjsf-team/react-jsonschema-form/blob/master/packages/material-ui/src/SelectWidget/SelectWidget.tsx#L68 like in link I've shared before

[epicfaace]

Ok. Why do you think it would be safe enough in this case?

[wight554]

Ok. Why do you think it would be safe enough in this case?

There's no way to pass these, unless you wrap component with some hoc Doing this for all will allow creating generic hoc for all components that'll check ui:props and pass it per widget

[stale[bot]]

This issue has been automatically marked as possibly close because it has not had recent activity. It will be closed if no further activity occurs. Please leave a comment if this is still an issue for you. Thank you.

[stale[bot]]

This issue was closed because of lack of recent activity. Reopen if you still need assistance.