Add ValidateSignature middleware for ignore params

[valorin]

This provides a work around to an interesting problem with Signed URLs that I encountered and discussed here: https://twitter.com/valorin/status/1547053470389112834.

I've created a PR for the framework here: https://github.com/laravel/framework/pull/43160

Problem

The problem is that some systems, such as mailing list providers and Facebook will add their own tracking parameters onto URLs. Such as the UTM tracking parameters (utm_*), and Facebook's fbclid parameter, which it adds to any links clicked for tracking purposes.

Since these are added as query parameters, they break the signature and prevent the URL from being validated.

Solution

My solution is to replicate the design used in other core middleware (i.e. EncryptCookies, PreventRequestsDuringMaintenance, TrimStrings, & VerifyCsrfToken) and include a configurable $ignore parameter, which can be used to specify the query parameters to be ignored when verifying the signature.

class ValidateSignature extends Middleware
{
    /**
     * The names of the parameters that should be ignored.
     *
     * @var array<int, string>
     */
    protected $ignore = [
        'utm_campaign',
        'utm_source',
        'utm_medium',
        'utm_content',
        'utm_term',
        'fbclid',
    ];
}

Questions...

The big question is if common tracking query parameters should be ignored by default.

Signed URL issues are hard to debug, such as when they are shared on Facebook, so having these default ignored would be beneficial in most cases. However it would break signed URLs that legitimately include those parameters. Although the argument could be made that you'd need to update the middleware to introduce these parameters, so it shouldn't break existing signed URLs.

I've defaulted them commented out for now to be safe, but enabled by default should be considered.

[taylorotwell]

Wouldn't the app HTTP kernel need to be updated as well?

[valorin]

Oops, yes, it does! I'll fix now. 😄

[valorin]

Oops, forgot to switch this from Draft when I fixed the kernel.

[jonnott]

@taylorotwell @valorin While I can understand the usefulness of adding the $ignore property into the core framework middleware, I'm not sure how adding yet another 'stock' piece of app-level middleware into the skeleton app helps anything/anyone? If developers want to make use of the $ignore parameter, can they not just extend the core middleware into their app codebase themselves, rather than dropping this on all (future) Laravel codebases?

[valorin]

I like the discoverability of an empty array in the code, rather than having to dig into the code or documentation to discover overrides, but it is a valid point. 🤔

@jonnott Would shifting this (and other string-array middleware config) into a config file or a service provider solve the problem, do you think? Something like config/middleware.php and have the various ignore arrays in there.

[jonnott]

@valorin I feel the best solution is to ship the skeleton with only middlewares that serve the 98% common use case (i.e. middlewares that 98% of apps will actually need), and then have some very good documentation covering other available core framework middlewares, how they work, and how to extend them into your app if they're needed.