Coldbox and VueJS untangled

cbValidation: validating a model or the request collection?

Recently I was coding a fluent API based on this sample code which was presented at ITB 2020 by Gavin Pickin. When I was testing I discovered I could overwrite existing records when trying to insert new ones, which sounds like a huge security vulnerability. But before blaming Gavin for this let me confess I changed the code a little, just enough to create this security hole. So this exercise showed me the following:

  • never ever populate a model automatically from the request collection without realizing what your customers can insert.
  • validating your request collection before populating your model has it advantages.

Actually I already knew this. But I learnt a few new things about cbvalidation and this fluent API code, so let me explain.

At ITB 2020 I presented about cbvalidation and people were asking questions if we should only validate the model or the request collections. I was validating models most of the time, many people are validating the request scope and others are doing both. Let me show you some code by Gavin:

function create( event, rc, prc ) {
  var validationResults = validateOrFail(
    target = rc,
    constraints = rantService.getConstraints()
  );
  userService.existsOrFail( rc.userID );
  var result = rantService
    .new( validationResults )
    .validateOrFail( rantService.getConstraints() )
    .save();
  prc.response.setData( { "rantID": result.getID() } );
  prc.response.addMessage( "Rant created" );
}

In the highlighted code you will see there is a .new() method wich creates a new Rant instance and in the same chain a .save(). This .save() call is defined in some baseEntity and used both for updating an existing record and inserting new ones.

function save() {
  if ( isLoaded() ) {
    return wirebox.getInstance( "#getServiceName()#" ).update( this );
  } else {
    return wirebox.getInstance( "#getServiceName()#" ).create( this );
  }
}

If my object already has a valid ID ( isLoaded()) in this code will be true and it will be an update ; if not if will be an insert. So if I am inserting new data, but provide a valid Id (by accident, in my case) it will replace an existing record by the new data. It is obvious we should prevent our new object to be populated with id’s.
Population is happening in the .new( someStruct) call. Behind the scenes .new() is calling populate, which has many parameters including some unused parameters to include or exclude fields. So it would make sense to add some extra include and exclude parameters to our .new() function, so we can exclude the id. Our base entity has many more properties which should not be overridden by a user, such as pk,entityName,serviceName,moduleName and entityService. I modified my new method in my BaseEntityService so it looks like this

function new( struct data = {}, string exclude = getPrimaryKey() ) {
  var modelName = getEntityName() & ( getModuleName().len() ? "@#getModuleName()#" : "" );
  return populator.populateFromStruct( 
    target = wireBox.getInstance( modelName ), 
    memento = arguments.data, 
    exclude= listAppend( "pk,entityName,serviceName,moduleName,entityService", arguments.exclude 
    )
  );
}

So now I cannot add any of these special keys anymore from my request collection. (I also changed my populator in a simular way for the update call).
Is this necessary? That’s where this post is about: the advantages and disadvantages of validation a request scope vs validation a model.

Let’s go back to our create handler:

function create( event, rc, prc ) {
  var validationResults = validateOrFail(
    target = rc,
    constraints = rantService.getConstraints()
  );
  userService.existsOrFail( rc.userID );
  var result = rantService
    .new( validationResults )
//......

Before populating your model the rc will be validated. And what’s important: ONLY the VALIDATED fields will be returned in a struct which is called validationResult. So by feeding my .new()method with a validationResult struct instead of an unfiltered request collection you have protected your input fields. There’s no way others can pollute your id, entityName and other fields as long as you don’t have them in your validation constraints.

So the flow here is: validate your request collection against some constraints and only your constrained fields will be returned. These fields will be fed to your new or populate method, so you don’t need no additional excludes there. Finally your populated object is validated again (if desired) and saved.

Are there more advantages or disadvantages to this approach? Of course! But most of them are quite easy to resolve.

  • fields without constraints will not show up in your validationResults so they will not populate your object. This is easy to resolve by adding myField ={ required=false} to your constraints.
  • you can implicitly validate constraints in your model, without specifying them. For request collection validation you have to specify your constraints again. By creating a getConstraints() method in your base service you can retrieve your constraints from your entity so you have the same source of truth for both request collection and your model. You can even specify multiple constraints in your model and call them by name (default name: constraints).
  • request validation is not the same as model validation. Many constraints are the same but you can exclude any field from request validation or model validation by using the excludeFields and includeFields of validate() and validateOrFail().
  • sometimes you want to validate your fields BEFORE you insert them into your model. A simple example for this: I have some foreign keys defined as guid in my model. When I try to feed invalid guid’s to my model my code will already throw an error before the validation. Unless I validate it already in my request collection.
  • some constraints will fail in request collection validation. Samples: method validators (because there is no object with this method), some UDF validators (if you are calling all kind of dependencies which are in your model, but not in your handler). This is all true. If you standardize on both request validation and model validation it is almost trivial to split your constraints in two sets: one which can be done in your request collection and one which has business rules in it so it can only be done in your model. You could decide if you could validate ALL rules again in your model or only the ones which were not hit in your request collection.
  • validating twice means performance loss. Most validations are fast, so if your traffic volume is not high you shouldn’t worry about that. If some of your validations are slow you migh consider skipping them in model or request collection (but not both…)

As you can see there are not much reasons why you should skip request collection validation. Still I was doing that, because model validation seemed good enough. And I loved my Customvalidators. I always thought I couldn’t check uniqueness of a record in my request collection, something which is very easy in a cborm entity.
In a next post I will show you how you can create your own unique record validator which works fine not only in model validation, but also in request collection validation.

1 Comment

  1. Dave Levin

    This is awesome! I was thinking the same thing during an Ortus training when I saw the example populate a model with the RC and then validating it.

    The way I’ve addressed the vulnerability is to always use the `include` argument to specify which fields from the RC are allowed to be set.
    “`
    populateModel(
    model = prc.user,
    include = “firstName,lastName,emailAddress”
    );
    “`

Leave a Reply

Your email address will not be published. Required fields are marked *

This site uses Akismet to reduce spam. Learn how your comment data is processed.

© 2020 ShiftInsert.nl

Theme by Anders NorenUp ↑