craftcms / cms

Build bespoke content experiences with Craft.
https://craftcms.com
Other
3.28k stars 635 forks source link

[4.x]: craft.app.request.url includes token #12458

Closed joepagan closed 1 year ago

joepagan commented 1 year ago

What happened?

Description

Hi, on an app we had been using craft.app.request.url for the returnUrl parameter in a freeform render tag. After a form submit this would redirect the user onwards to the same url with a success QS param to show dynamic content (a thanks message on the same page).

However, guest users (not admins), would get redirect to a URL with a token parameter, and were instead met with a blank page. Maybe Craft thinks the page is a "preview" page because this param existed in the URL? In this case it was loading a completely blank template, as the page was enabled/published. Maybe this param is not allowed and saved for something else?

Some tests reveal that is was not freeform guilty of adding this param but craft.app.request.url instead.

Not sure if we may have done something to make it behave this way? Or we have missed something? This site has recently been migrated to craft 4.3.3 from ~3.7.

Some existing issues related to token describing behaviour and how to handle it, but not the same issue: https://github.com/craftcms/cms/discussions/8914 https://github.com/craftcms/cms/issues/5698

Steps to reproduce

<input type="hidden" name="_debugRequestUrl" value="{{ craft.app.request.url }}">
<input type="hidden" name="_debugOverrideUrl" value="{{ '?' in craft.app.request.url ? craft.app.request.url ~ "&success=1" : craft.app.request.url ~ "?success=1" }}">
<input type="hidden" name="_debugReturnUrl" value="{{ form.getReturnUrl() }}">
{% set bag = form.getPropertyBag() %}
<input type="hidden" name="_debugBagUrl" value="{{ bag.get('returnUrl') }}">
<input type="hidden" name="_debugBagHashedUrl" value="{{ craft.app.security.hashData(bag.get('returnUrl')) }}">

Results in:

<input type="hidden" name="_debugRequestUrl" value="/contact?token=A3ouEXansCHsN1VpygFYcX5eX8sBNYqf">
<input type="hidden" name="_debugOverrideUrl" value="/contact?token=A3ouEXansCHsN1VpygFYcX5eX8sBNYqf&amp;success=1">
<input type="hidden" name="_debugReturnUrl" value="">
<input type="hidden" name="_debugBagUrl" value="/contact?token=A3ouEXansCHsN1VpygFYcX5eX8sBNYqf&amp;success=1">
<input type="hidden" name="_debugBagHashedUrl" value="074d585a02d2e2fdceff32a803f97efee32893d95206b9aeb93c71cfee7c06e0/contact?token=A3ouEXansCHsN1VpygFYcX5eX8sBNYqf&amp;success=1">

Expected behavior

no token param in craft.app.request.url

Actual behavior

token param in craft.app.request.url

Craft CMS version

"craftcms/cms": "4.3.3",

PHP version

8.1

Operating system and version

8.1-fpm-alpine

Database type and version

aurora (mysql 5.7)

Image driver and version

No response

Installed plugins and versions

"php": "^7.2||^8.0", "ext-curl": "", "ext-dom": "", "ext-json": "", "ext-simplexml": "", "ext-soap": "*", "composer/composer": "^2.0.10", "craftcms/aws-s3": "2.0.1", "craftcms/cms": "4.3.3", "craftcms/redactor": "3.0.2", "extphp/xml-to-json": "^0.2", "facebook/php-business-sdk": "^10.0", "monolog/monolog": "^2.3", "nystudio107/craft-retour": "4.1.6", "nystudio107/craft-seomatic": "4.0.15", "nystudio107/craft-templatecomments": "^4.0", "nystudio107/craft-vite": "^4.0.0", "putyourlightson/craft-blitz": "4.2.3", "samdark/yii2-psr-log-target": "^1.1", "solspace/craft-freeform": "4.0.8", "spicyweb/craft-neo": "3.5.6", "topshelfcraft/wordsmith": "4.0.1", "verbb/image-resizer": "3.0.4", "verbb/navigation": "2.0.13", "vlucas/phpdotenv": "^3.4.0", "yiisoft/yii2-redis": "^2.0"

i-just commented 1 year ago

Hi, I’m having trouble replicating this behaviour. Can you please confirm that there’s no token param in the address of the page that is displaying this form? Is the entry enabled and with a live status?

If you were to display or dump the {{ craft.app.request.url }} on any other page, are you getting the URL with the token too?

Also, have you tried to replicate this behaviour with the blitz plugin turned off?

Finally, though this is not a fix for what you’re describing, I just wanted to mention that if you’re using Freeform Pro, you can set it to redirect to the same page with a content-managed success message without having to manipulate the return URL yourself. It might not suit your case, but I thought I’d mention it just in case.

joepagan commented 1 year ago

Hey, thanks for getting back!

I can confirm no token param in the address bar is present when I see the token param in the DOM using craft.app.request.url

If you were to display or dump the {{ craft.app.request.url }} on any other page, are you getting the URL with the token too?

I'll need to redeploy the above and get back to you.

Also, have you tried to replicate this behaviour with the blitz plugin turned off?

No, but I can try this and get back to you, though I have requested a cache busted page (with a random query string) incognito mode and still witnessed the token in the dumps referenced above. For added context, this site is going through cloudflare.

Finally, though this is not a fix for what you’re describing, I just wanted to mention that if you’re using Freeform Pro, you can set it to redirect to the same page with a content-managed success message without having to manipulate the return URL yourself. It might not suit your case, but I thought I’d mention it just in case.

Thanks for the suggestion but we do need to manipulate the URL to add some QS params for tracking purposes. We ARE sorted by grabbing the current URL with JS and using that instead, however, this seemed important enough to log an issue about as I suspect this might be happening in other scenarios and will likely kick me in the shins later down the line somewhere else!

i-just commented 1 year ago

however, this seemed important enough to log an issue about as I suspect this might be happening in other scenarios and will likely kick me in the shins later down the line somewhere else!

Absolutely. As I’m having a hard time replicating this on a fresh Craft 4 install, would you be able to send your database dump, composer.json and composer.lock to support@craftcms.com so that I can dig deeper into what’s going on?

brandonkelly commented 1 year ago

I have a feeling this is a template caching issue. If there are any {% cache %} tags on the page, the cache was probably generated for a request that had a token, which would have resulted in the tokens getting included in any generated URLs in the cache.

I’ve just updated template caching to not be enabled for tokenized requests (8e1018e73cd10bc279e9ecd25d70a33ec7bf21e6). If the theory is correct, that will fix this going forward. Sound plausible @joepagan?

brandonkelly commented 1 year ago

Craft 3.7.63 and 4.3.6 are out with that change to template caching. Going to assume that was the culprit here and close the issue, but feel free to reply if there’s anything else to add.

joepagan commented 1 year ago

Sorry for the late reply on this issue had some time off.

Unfortunately, template caching is disabled on this app as we use full page caching with blitz/cloudflare and template caching causes issues there.

This can only be replicated in remote production and I was unable to reproduce on a developer environment, I cannot replicate on remote UAT environment (which has cloudflare & blitz enabled too). This is pretty odd, considering UAT is almost a carbon copy of prod with the exception of:

I am blocked deploying to this app at the moment so I cannot provide the same twig variables debugging currently but when looking at the freeform autogenerated input formReturnUrl (where we currently pass craft.app.request.url to it), they partially hash it and looks like this which appears to contain the token still. Here are some results from 2 forms where I enabled/disabled blitz & purged cloudflare on their respective URLs:

Where blitz is enabled on form 1:

<input type="hidden" name="formReturnUrl" value="b4ac31072816b5c7a1013bf86a8db0b3d871e701acd8019c16b4fa35354709ed/brochure-request?token=HEVqr78t48M15uYK3wyiydmiSKQpOSen&amp;success=1">

Where blitz is enabled on form 2:

<input type="hidden" name="formReturnUrl" value="023ac619cb393a73bf98e87ebe28d03e5c4ede966e1605f84c28bb2514dd9c83/contact?token=hT9Gs8VNmopsWZw7J6F3gaJ1P07OfaHL&amp;success=1">

Where blitz is disabled on form 1:

<input type="hidden" name="formReturnUrl" value="9e3428595093a6eb06a74af064aa731426304665d9a5463707fa23b86d4d207b/brochure-request?success=1">

Where blitz is disabled on form 2:

<input type="hidden" name="formReturnUrl" value="d6c490fbae7b223c8aca25f1d7dcebba17eb00dd6c597ebb78a819490837bcb0/contact?success=1">

Would it still be beneficial for me to get a database dump & provide composer files to you now I have provided this info?

Do you think this is an issue better placed on the blitz repo (though seems strange something they are doing could manipulate the outcome of craft.request.app.url).

Hope that helps, let me know if you want me to perform any follow up tests

brandonkelly commented 1 year ago

The same issue could equally affect Blitz, if caches aren’t pre-warmed, so I’d suggest reporting it over there. You can point them to this issue and 8e1018e73cd10bc279e9ecd25d70a33ec7bf21e6 for inspiration.