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

Provide an API to validate block input, either client-side or server-side #4063

Closed
wsydney76 opened this issue Dec 18, 2017 · 19 comments
Closed

Provide an API to validate block input, either client-side or server-side #4063

wsydney76 opened this issue Dec 18, 2017 · 19 comments

Comments

@wsydney76
Copy link

@wsydney76 wsydney76 commented Dec 18, 2017

Issue Overview

Sometimes it is crucial to validate input values on the server side, e.g. check if an email address is valid within the company by checking against an LDAP-Directory. (Server side because "Never trust the client" rules).

I could not figure out (overlooked?) a way to do that with Gutenberg.

Steps to Reproduce

  1. Hook into rest api (pre_insert_post?) and return an error, something like
    return new WP_Error( 'rest_invalid_param', 'E-Mail not found in directory', array( 'status' => 400) );
  2. 'Updating failed' is shown, but not the specific error message.

Possible Solution

  • Document how to return an error (better a list of errors) correctly.
  • Display the error messages.

In a second step it would be helpful to trigger a JS event in the corresponding block, so that a visual feedback can be given in the right place.

PS.
Is there some kind of validation framework present/planned in Gutenberg that i did not see?

@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Jan 23, 2018

👍 I agree with the general premise that it should be possible to validate block values (either client-side or server-side) and display an error when they're invalid.

@danielbachhuber danielbachhuber changed the title Provide a method to display results of server side validation Provide an API to validate block input, either client-side or server-side Jan 23, 2018
@gziolo

This comment has been minimized.

Copy link
Member

@gziolo gziolo commented Mar 7, 2018

@adamsilverstein can you help answer this question? I'm not familiar enough with REST API integration with Gutenberg but I would assume you can use server-side hooks. The only remaining question is how to propagate that response to the client properly, i.e. how to shape the response to make sure it can be understood by Gutenberg. I think we use notices to display messages, so this might work out of the box.

@gziolo gziolo added this to To do in Extensibility via automation Mar 7, 2018
@danielbachhuber

This comment has been minimized.

Copy link
Member

@danielbachhuber danielbachhuber commented Apr 2, 2018

Worth noting: the Customizer already has a validation API like this. As Gutenberg and the Customizer will eventually need to reconcile, it might be worth cribbing from the Customizer if we can.

cc @westonruter

@westonruter

This comment has been minimized.

Copy link
Member

@westonruter westonruter commented Apr 3, 2018

And the Customizer's validation API is designed to feel like the REST API's validation API. With the move to defining all block attributes server-side and having them exported to the client (see #2751) then this means we can amend the attributes schema with things like sanitize_callback and validate_callback to use during validation. The REST API endpoint to insert/update a post could run through the block validate/sanitize callbacks prior to saving the post content.

@gziolo

This comment has been minimized.

Copy link
Member

@gziolo gziolo commented Apr 3, 2018

@westonruter, I started working on moving the definition of all block attributes server-side in #5802. The idea is to have also a filter on the server which allows plugins to add some automated changes to all blocks. I was thinking about adding validation step just before all those definitions are exposed to the client. If you have some insights how this should be implemented, please leave a comment on #5802. In general, this blocks definition should be exposed as new REST API endpoint in the near future. So all that you shared aligns with what we are exploring at the moment.

@adamsilverstein

This comment has been minimized.

Copy link
Contributor

@adamsilverstein adamsilverstein commented Apr 3, 2018

@gziolo

can you help answer this question? I'm not familiar enough with REST API integration with
Gutenberg but I would assume you can use server-side hooks.

yes, the current REST API endpoints all include hooks at multiple points - before the query, on the response, etc. as long as you can identify the request, you can add whatever filter you want.

@westonruter

This comment has been minimized.

Copy link
Member

@westonruter westonruter commented Apr 8, 2018

@gziolo @adamsilverstein @danielbachhuber Something else that comes to mind in relation to block validation is allowing pre-parsed blocks to be exposed in the REST API response. I think this was previously talked about as /wp/v2/posts/:id/blocks or something. But that seems to be disconnected from the fact that the blocks are (for now) mostly managed as part of content. So what if just as we have content.raw and content.rendered we also had content.blocks? This could be an array data structure of blocks (and their nested blocks). A client could then manipulate the blocks in their parsed representations and then PUT back the modified content.blocks data to update the post. This is where the validate_callback and sanitize_callback functions could be called on the blocks, in the rest_pre_insert_{$this->post_type} filter. If there is a validation error the filter could return a WP_Error to cause prepare_item_for_database to return an error and thus cause update_item to be rejected.

@adamsilverstein

This comment has been minimized.

Copy link
Contributor

@adamsilverstein adamsilverstein commented Apr 9, 2018

what if just as we have content.raw and content.rendered we also had content.blocks?

@westonruter what i was proposing as a read only form in #2649... I like your idea of making it writable as well.

@gziolo

This comment has been minimized.

Copy link
Member

@gziolo gziolo commented Apr 9, 2018

So what if just as we have content.raw and content.rendered we also had content.blocks?

@mtias and @mcsf, I think this is something you should respond to. I personally don't feel like we should reimplement all the logic we already have on the client, but I don't know the bigger picture and in particular, I'm not aware of any use case where it could be used.

@danielbachhuber danielbachhuber mentioned this issue Apr 10, 2018
0 of 2 tasks complete
@danielbachhuber danielbachhuber mentioned this issue Apr 24, 2018
3 of 3 tasks complete
@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 8, 2018

My gut tells me we should be cautious with what we implement on the server, for a couple of reasons:

  • The client is the default place for editing logic, as is the case for core blocks and their business rules, and it's also where serialization is supposed to happen.
    • Now, I know the client will not always be the adequate solution for some — this is the same debate as for ServerSideRender (see #5602) — but by optimizing and providing out-of-the-box tooling for certain server-side scenarios we create impetus for those flows.
  • The client has good hook coverage, making validation tasks easy to implement, as can be seen with color contrast in wp:paragraph. It lends itself to a better user experience for a number of reasons (as cited for ServerSideRender, namely no-latency interactivity).
  • This doesn't mean that we should forbid or work against server-side validation by any means. I can't stress that enough. But:
    • Server-side validation is technically possible without framework changes: see Reusable Blocks (wp:block) and the way they reuse WordPress tools (CPT, REST endpoint) to potentially retain control over every datum. Yes, this requires some added boilerplate, but that may be justified for more ambitious blocks that need to validate and guard a lot of information.
    • I'd rather we leave some of this unanswered for now as Gutenberg lands in core and the plugin ecosystem adapts, and we let patterns emerge and figure out how to best deal with server-side validation.
  • That said, we can and should make sure we have basic tools in place for working with blocks — for this, see PHP APIs Overview #8352.
@mtias mtias mentioned this issue Aug 8, 2018
6 of 16 tasks complete
@wsydney76

This comment has been minimized.

Copy link
Author

@wsydney76 wsydney76 commented Aug 8, 2018

Just some quotes:

The bottom line is that anything client side can be manipulated (disabled/completely got rid of/bypassed or modified) by the end-user. So any sort of "client side validation" is totally useless from a security perspective. You must always validate things server side.

And you must do it in the same request that would finally save the data, not in a seperate ajax request before.

Client side validation is purely about user experience

From https://security.stackexchange.com/questions/169771/is-html5-input-pattern-validation-sufficient-or-even-relevant-for-client-side

@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 8, 2018

So any sort of "client side validation" is totally useless from a security perspective.

Yes, though this is about more than security (e.g. validation is also about correctness or about successfully guiding the user). If you depend on block-level or field-level validation even for privileged users (can_edit_post, etc.) for security, then you probably have a larger issue at hand. The same goes with user content up predating Gutenberg.

In any case, all content should always be sanitized and escaped regardless of block adoption.

not in a seperate ajax request before.

My mentioning endpoints and PHP APIs didn't imply relying on ad hoc or post hoc server requests to "fix" saved data — that would be bad. I meant either PHP-side APIs to be used in server-side hook callbacks, or an altogether separate transaction for saving certain blocks (like Reusable Blocks).

@wsydney76

This comment has been minimized.

Copy link
Author

@wsydney76 wsydney76 commented Aug 9, 2018

Yes, though this is about more than security (e.g. validation is also about correctness or about successfully guiding the user)

Sure, but imho the integrity of data is as important. E.g. the integrity of an email address may be crucial because secret internal infos will be sent to it. So there should be no way to manipulate it. Should have mentioned that in my reply, sorry.

@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 9, 2018

the integrity of data is as important. E.g. the integrity of an email address may be crucial because secret internal infos will be sent to it. So there should be no way to manipulate it.

That's fair.

I'm still inclined to say that, for now, developers (and third-party solutions which are likely to emerge) should be left to solve this as most fitting for their use cases, provided we ship the basic tooling as mentioned in my original comment. What are your thoughts on that, @wsydney76?

@wsydney76

This comment has been minimized.

Copy link
Author

@wsydney76 wsydney76 commented Aug 9, 2018

@mcsf I would like to see a "how to do validations in Gutenberg" - guide.

For server side validations (a bit high level, as i am not so up-to-date any more):

  1. How to hook into the request
  2. How to extract the data. You may need all the posts data, not only the current block.
  3. How to stop the request and specify error messages (plural) that will be shown in Gutenberg.
  4. (optional, but very welcome) How to display the error messages near the field.

I think most developers would finally like to see an approach that is so close as possible to well known MVC-patterns e.g.

public function getElementValidationRules(): array
{
    $rules = parent::getElementValidationRules(); // the default rules from parent email field
    $rules[] = ['validateEmail'];
    return ['validateEmail'];
}

public function validateEmail($element){

    $fieldname = $this->handle;

    $email = $element->$fieldname;
    $service = addonsPlugin::getInstance()->myHelpers;

    if ( ! $service->validateEmail($email) ) {
        $element->addError($fieldname , \Craft::t('addons', 'Please use a valid corporate email'));            
}

resulting in
image

but i definitely understand that a more low level approach is more appropriate right now.

Thanks for your effort.

PS.
Having read the announcement of ACF blocks yesterday and knowing that ACF heavily relies on validations (including server side), maybe the two of you could join forces?

@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 27, 2018

@wsydney76, thanks for your reply and for your patience. I don't have a straight answer before sharing some open questions I still have. Below are some thoughts on my mind, followed by my best approach at answering your doubts. Sorry for the long reply, but I hope it can be helpful in the end.

Questions

  1. Is it a requirement that a validation service ($ldap_directory->validate_address) should run on the server and not on the client? Why? I ask because by addressing this on the client (whether locally or, in this case, via remote call to your LDAP service) we can provide a more immediate and granular experience that would satisfy one of the definitions of "validation". This pattern is the one we see with <ContrastChecker /> (see gif), and one that I'd like to see more widely used.

Sure, but imho the integrity of data is as important. E.g. the integrity of an email address may be crucial because secret internal infos will be sent to it.

This brings me to:

  1. a) What is the threat model here? What would constitute a breach of security, and how is WordPress's basic capabilities model inadequate? Is the page we're editing to be edited by other users that are not trustworthy?

  2. b) When and how will the provided fields be used aside from display? You suggested e-mail delivery, which makes sense, but we would always be expected to sanitize our data anyway, e.g.

$address = sanitize_email( $user_input->email_address );

$address = $ldap_directory->validate_address( $address );
if ( is_wp_error( $address ) ) return $address;

$mailer->send_to( $address, $subject, $body );
  1. Before Gutenberg, what would a solution for this have entailed? More specifically:
  • What would the interface and interaction abstractions have been?
  • What would the persistence and control abstractions have been? Would they have been like WordPress meta in any way?

Maybe a solution

Blocks are pretty versatile in the ability to deal with both more visual and more behavioral information, but also in the ability to deal with information of varying scope: block attributes are by default local to each block, but they can be local to the post thanks to meta attributes — see meta paragraph in docs).

It seems to me that, assuming that we could have client-side handling of the validation (read: the validation could be provided by a remote service, but the client mediates the request), the rest would nicely be accomplished with meta-based block attributes:

  1. Client warns the user if the provided address is invalid, as judged by the LDAP directory.
  2. The address is saved as a block's address attribute, which maps to my_address in post meta.
  3. Since we're dealing with meta, server-side hooks are available to filter any incoming data ('updated_post_meta'), thereby guaranteeing integrity.
  • Since we're consulting an external service for the address validation, we didn't need to duplicate business logic between client and server.
  • This could be an occasion to return error messages from the server, but we'd need to build the tooling to send them on one end and receive and display them on the other, which I know is not ideal. We could also take shortcuts with less robust solutions, like saving a separate meta value on the server (my_address_validation_error) if the address is invalid — upon saving, this new value would be added to the POST response sent back to the client, possibly triggering an error notice.
  1. In any case, saving the post on the client should happen without no hiccups, since we're sending to the server what we've already validated. We should only expect failure here if the user (or something on the client) is tampering with the post data or if some network failure broke the validation step.

Having read the announcement of ACF blocks yesterday and knowing that ACF heavily relies on validations (including server side), maybe the two of you could join forces?

From the beginning, Gutenberg wanted to bring users and devs a world of possibilities in terms of block development, hence the rollout of the JS APIs — particularly, the functional nature of edit and save — as the primary extensibility methods. That said, the idea is definitely on the table of providing good defaults in the form of a server-side-capable block description language with automated data binding and rendering is definitely on the table. That is, in the future we may be able to call register_block_type on the server and pass it a deep object that will render on the server as an interactive block that handles user data.

However, that falls well outside the scope of 5.0, which is also why I'm happy to see efforts from ACF and any other player to provide specialized interfaces mindful of their users' specific needs. However, I don't expect Gutenberg to join efforts with any particular plugin, for two main reasons:

  • The WordPress philosophy's principle of "decisions, not options", but especially the principle of "clean, lean and mean".
  • I think Gutenberg has a lot more room to learn and adapt if we leave these specialized solutions out of core and let the plugin ecosystem develop an array of competing solutions. Over time, these will definitely inform the course of WordPress core, but until then settling on a specific solution would be restricting our options and tying WordPress to an untested interface.
@wsydney76

This comment has been minimized.

Copy link
Author

@wsydney76 wsydney76 commented Aug 28, 2018

@mcsf Thanks for your answer,

for your patience.

No problem, i opened this and some other issues back in december when we (some companies and agencies) were evaluating the (continued) use of WordPress and Gutenberg as a platform for more advanced and sophisticated applictions, where validation is a crucial part. Since then all of our clients figured out that WP in it's between-the-worlds state does not fulfill their requirements of having a proven, stable, comprohensive solution and switched to other CMS's.

Don't ge me wrong, that does not mean that WP/GB is a bad product, not at all, it's simple not the right choice for every task. (And no, you can't sell this classic editor thing to companies).

As i am no longer involved with GB right now, please understand that i am not able to answer your questions in every detail. Sorry if i understood something wrong.

Is it a requirement that a validation service .. should run on the server and not on the client?

Yes.

Server side: Mandatory. Assures the integrity of data. Client request's can always be manipulated and come from other sources than GB.

Client side: Optional. Provide a better User Experience.

What is the threat model here?

That malicious people are always cleverer then i am and will find ways that i don't even think of.

but we would always be expected to sanitize our data anyway

Yes, that is an additional level but used as the only validation it does not give a user feedback and you don't want to have unchecked data in your db.

Before Gutenberg, what would a solution for this have entailed?

Something like described in #3964 .

but they can be local to the post thanks to meta attributes

Yes, always assumig the data is stored as meta attributes. Accessing data local to a block is ... (insert your own wording here).

Since we're consulting an external service for the address validation, we didn't need to duplicate business logic between client and server.

Yes, the mandatory server side validations could be exposed to a client via a service provided by our application, which may encapsulate a call to a corporate ldap service. The client should not contact an external service directly. It may not even be allowed to do so. I am no security expert and cannot say if methods like hashing a value for the final submission to protect it from being tampered is secure enough.

but we'd need to build the tooling to send them on one end and receive and display them on the other,

Yes, i think there are some issues around something like a notification api ?

So with that i am out of the game, and want to thank everybody for their work.

PS.
Personally i would be fine with closing this issue, but i am very sure this topic will continue to pop up, so you may want to leave it open.

@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 30, 2018

Since then all of our clients figured out that WP in it's between-the-worlds state does not fulfill their requirements of having a proven, stable, comprohensive solution and switched to other CMS's.
Don't ge me wrong, that does not mean that WP/GB is a bad product, not at all, it's simple not the right choice for every task. (And no, you can't sell this classic editor thing to companies).

No worries, thanks for being upfront about it.

Yes, i think there are some issues around something like a notification api ?

Yes, #5975.

Personally i would be fine with closing this issue, but i am very sure this topic will continue to pop up, so you may want to leave it open.

Thanks. My idea is to close this one, since its scope is more ambitious, and open a more focused issue suggesting some documentation around possible practices to guide developers around server-side block operation.

@mcsf

This comment has been minimized.

Copy link
Contributor

@mcsf mcsf commented Aug 30, 2018

Closing per #9463.

@mcsf mcsf closed this Aug 30, 2018
Extensibility automation moved this from To Do to Done Aug 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Extensibility
  
Done
6 participants
You can’t perform that action at this time.