Input validation and sanitization middleware

[funkatron]

One pain point for me in Slim 2.x was how to do input validation and sanitization. Retrieving values from the request object's ->get() and ->post() always return raw, unfiltered data. To turn it into safe(r) data, you need retrieve the items individually or as an array and pass them each through some kind of sanitization functionality, or bypass the request object entirely and do something to the superglobals directly (which may or may not be problematic depending on how Slim is internally handling that data).

I have a project I've worked on for a long time called Inspekt that sorta tries to do this to the superglobals. It wraps them inside a "cage" object, and you have to use accessor methods to retrieve data. You have to use getRaw() to get completely raw, un-sanitized input. The two big wins are, I think:

1. You're not accessing superglobals directly -- harder to make mistakes
2. You have to show intent when getting raw data -- harder to make mistakes

I think lots of people will have different approaches to how to handle input safely, and I don't think building an opinionated approach into Slim makes sense. However, I do think that it would be good if it was easier to override how request variables are accessed, both on a one-by-one basis, and as an array.

Were I designing a plugin or add-on for Slim that did this, I would want to be able to:

• eliminate the request::get() method entirely
• attach a number of input accessor methods that could either be applied to single values or Arrays (or ArrayObjects).

I have a few thoughts about how something like this might look API-wise, but I wanted to just get the idea out there first.

[codeguy]

What if the Request object supported an object that implemented some form of filterable interface. We'd inject the filter object into Slim's request object in its Pimple factory method. We would then run all values currently returned from getParam() and getParams() through the filter object if present. This would let you register the filter object on the container, too. Obviously, this stops working if you swap in an alternative PSR7 request object. Thoughts?

[codeguy]

For example, we'd register the custom request filter object like this:

$app = new \Slim\App();
$app['requestFilter'] = function ($c) {
    return new \MyRequestFilter();
};

We'd update the current Request Pimple service to look like this (note the last two lines):

$this['request'] = $this->factory(function ($c) {
    $env = $c['environment'];
    $method = $env['REQUEST_METHOD'];
    $uri = Http\Uri::createFromEnvironment($env);
    $headers = Http\Headers::createFromEnvironment($env);
    $cookies = new Collection(Http\Cookies::parseHeader($headers->get('Cookie')));
    $serverParams = new Collection($env->all());
    $body = new Http\Body(fopen('php://input', 'r'));

    $request = new Http\Request($method, $uri, $headers, $cookies, $serverParams, $body);
    $request->setFilter($c['requestFilter']);

    return $request;
});

And we'd update the Request object's getParam() method to this (note last line):

public function getParam($key)
{

    $postParams = $this->getParsedBody();
    $getParams = $this->getQueryParams();
    $result = null;
    if (is_array($postParams) && isset($postParams[$key])) {
        $result = $postParams[$key];
    } elseif (is_object($postParams) && property_exists($postParams, $key)) {
        $result = $postParams->$key;
    } elseif (isset($getParams[$key])) {
        $result = $getParams[$key];
    }

    return $this->filter ? $this->filter->process($result) ? $result;
}

[codeguy]

We could also delegate this to a service provider object like this:

$app->register(new \My\FilterProvider);

$app->get('/foo', function ($req, $res, $args) {
    $cleanValue = $this['filter']->get($req, 'paramName', FILTER_*); 
});

And pass the current request object into the service provider object and assume it does what it does and returns a clean value.

[JoeBengalen]

Why not simple create middleware for this that updates the Request object? You could use the withParsedBody(array $filteredPostData) and withQueryParams(array $filteredGetData).

[lalop]

@codeguy I think your first option can be more effective using Pimple#extends https://github.com/silexphp/Pimple/blob/master/src/Pimple/Container.php#L225

[codeguy]

@JoeBengalen That prevents access to the original raw parameter values.

@lalop Good idea :+1:

[codeguy]

@funkatron Any thoughts? I'd like to include something for input sanitization/validation in 3.0.0

[geggleto]

A wrapper around the filter_var makes sense to me. I know route parameters can have filters attached, is there any way to extend that functionality to the request body ?

[akrabat]

To my mind, filtering and validation of request parameters is related to the route, so conceptually, I'd imagine this being a either route middleware thing or if we're going to attach a requestFilter to the $app, then this would be set on a per-route basis.

[DavidePastore]

I agree with @akrabat . We could use something like Respect/Validation that has all the things you could imagine.

[codeguy]

Middleware seems like the best solution that can work on a per-app or per-route basis. The Middleware could accept an interface that lets us write adapters for various third-party components.

[codeguy]

Does anyone want to lead this effort? I'd like to have it available for 3.0.0, but since it's external middleware that's not required.

[geggleto]

@codeguy I'll take this on.

[nicgene]

This would be so helpful! Thanks @geggleto :+1:

[geggleto]

https://github.com/geggleto/vms/blob/master/src/Services/vSlim.php

@dominicbusinaro here is a kinda of beta version that I have been working on

[geggleto]

https://github.com/geggleto/svl more polished version

[SimoTod]

@geggleto Any news about the component? I think you shouldn't focus on write a new validation component. You could write an interface for adapters, pick a third-party validation library (auraphp/Aura.Filter, Respect/Validation, illuminate/validation, vlucas/valitron, ecc), write the adapter and than write a middleware that use interface methods.

[akrabat]

I'm not sure what's required here.

@JoeBengalen's comment of:

Why not simple create middleware for this that updates the Request object? You could use the withParsedBody(array $filteredPostData) and withQueryParams(array $filteredGetData).

summed it up for me.

Do we need a section in the docs?

[designermonkey]

I've been working with the ADR pattern lately, and the validation happens as part of the process. The pattern is quite abstract and examples in the wild take a request object and return an array, which is entirely up to the developer to implement.

My take on that is to return an instance of the request that contains all the filtered input. You can then have access to the original request, and a filtered version.

[geggleto]

Slim currently doesn't have a way to filter the raw data currently as the request object is created during bootstrapping where in some instances the request body is parsed automatically.

https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Request.php#L147-L152

The only way around this is to override the way Slim get's the request object. This seems like a lot of work and likely should be a lot easier to do.

[designermonkey]

What I meant was to provide the initial request to a middleware

$container['requestFilter'] = function ($c) {
    return new RequestFilter($c['request']);
};

then a filtered request object can be used at any time, while the initial one is still available. This is the benefit of having the immutable nature of the request.

$initialRequest = $container['request'];
$filteredRequest = $container['requestFilter']->getFiltered();

[geggleto]

This is why I kind of wanted Slim to have factories... we could extend the basic RequestFactory to have FilteredRequestFactory which returns a filtered request object... Can't do this in the 3.0 branch though as it would likely be a BC break.

in your example the filtered request won't propagate through the system unless you overwrite it. $container['request'] = $container['requestFilter']->getFiltered();

[akrabat]

I don't see how you would know which filter you wanted until routing had happened. If you do it in middleware, then you could put the filtered request into an attribute.

[geggleto]

When I think of a filter, it would be pretty all encompassing. like removing a sql injection attempt or an attempted xss.

[designermonkey]

Yeah, the two approaches are whitelist and blacklist... Blacklist on input, like XSS, then whitelist on insertion into a DB, so an input filter is like @geggleto says, it encompases all of a request and should be standard to all input sources.

[geggleto]

@akrabat once parsed the body cannot be reparsed

https://github.com/slimphp/Slim/blob/3.x/Slim/Http/Request.php#L936-L941

And unfortunately if you are accepting a POST request from a HTML form the body will get parsed in the bootstrapping of the app, meaning that you can't filter it, unless you recreate the request object itself in your middleware.

[akrabat]

Why doesn't $request = $request->withParsedBody($newData); work?

[designermonkey]

I was just about to suggest that.

The middleware would getParsedBody, do it's processing, then set it withParsedBody into a filtered request object being made available.

[geggleto]

I don't see a way to current get the raw body data, so we are filtering over the parsed data already?

[designermonkey]

@geggleto can you elaborate on what the problem with using the parsed body is?

You're right, the raw body is readable only once, and is read into a StreamInterface object, but I don't see the issue of doing XSS filtering and the like on a pre-parsed dataset.

[designermonkey]

In my codeigniter days, we just filtered on the $_POST array, which is no different in theory to what I'm proposing here. We have the initial Request object, fully parsed, and then we want to create a filtered version for use in the app. We would still have the initial version to compare against in case we want to notify anyone that the input was changed or similar.

[geggleto]

I mainly dislike doing the same thing more than once... it makes most sense to me to be able to filter the body before anything is done. But given slim's current architecture I don't see how that is possible.

[akrabat]

Just create your own Request object and set it in the container before you call run().

[ghost]

Getting too many mails on this topics ... why discussing so much on this topic and not do the Slim behaviour analysis first and say something useful.

What you need to do is extend Route.php ... there you have access to all arguments ... add a function for example called setValidation(arrayRules) ... this way you have arguments and rules access for specific route.

Then there is function called __invoke(ServerRequestInterface $request, ResponseInterface $response) .... in this function on the beginning initialize your validation library or whatever and if validation passes you let this function continue work else you return your own custom validation response ....

[akrabat]

Many solutions to this - all of which are outside the scope of the code Slim code as the solution is either via replacing the Request object in the DI container or via middleware.

[DavidePastore]

For validation, a nice simple solution could be this one: https://github.com/DavidePastore/Slim-Validation If you have questions and want to know more, just ask on that repo. :stars: :smile_cat:

[micoboyet]

@DavidePastore Hi, how do you pass the request parameters into the validators?

[micoboyet]

@DavidePastore Hi, I already know how to pass the parameter but how can we make the parameters more flexible using two dimensional arrays?

For example on my payload, i have,

params['user']['first_name'] params['user']['last_name']

In backend I access the parameters like this :

$params = $request->getParsedBody();

and I am getting error on

Call to a member function assert() on array in davidepastore\slim-validation\src\Validation.php line 67.

I hope that it also supports two dimensional arrays. Btw the validation is great and it really works well using single associative array.

[silentworks]

@micoboyet you are probably better off opening a issue on the actual repo @DavidePastore linked to, easier to keep track of your conversation there as this issue is closed atm.