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

[9.x] Add ignore param to ValidateSignature middleware #43160

Merged
merged 2 commits into from
Jul 13, 2022

Conversation

valorin
Copy link
Contributor

@valorin valorin commented Jul 13, 2022

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 skeleton here: laravel/laravel#5942

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 taylorotwell merged commit a8a4006 into laravel:9.x Jul 13, 2022
@GrahamCampbell GrahamCampbell changed the title Add ignore param to ValidateSignature middleware [9.x] Add ignore param to ValidateSignature middleware Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants