October: Build 460 breaks pages loaded in an iframe

[japomani]

After updating to Build 460, my pages, which are loaded into an iFrame of a web application, simply returned a blank page with the text: "Invalid security token"

These pages have worked in this environment for over 6 years until today when I updated the system today.

I traced the error down to this line:
https://github.com/octobercms/october/blob/882ca6019c9c1b124e6e3f052b6aec02b6fa826b/modules/backend/classes/Controller.php#L790

With some help from slack, I was able to remove the error by disabling enableCsrfProtection in config/cms.php.

Now the page displays again, BUT I have lost my CSRF protection. That is not good. While it is not common for OctoberCMS pages to be displayed in an iframe inside of a web application, I believe it should still be supported by OctoberCMS. For example, if a developer was trying to push a page into Facebook or other social media site, they would have a similar problem.

Thank you for your consideration. I hope you are able to improve CSRF, or return it to its previous configuration, so I can protect my site from CSRF.

Maybe a solution would be to add a configuration setting that would use the old approach, that way I could have at least partial protection again.

Thanks!

[daftspunk]

I suspect this may be related to #4598 that introduces a certain failure when there is no session available. Can you test the following:

• Re-enable the cms.enableCsrfProtection option
• Navigate to file modules\system\traits\SecurityController.php
• Modify Line 65

        if (!strlen($token) || !strlen(Session::token())) {
            return false;
        }

To the new code

        if (!strlen($token)) {
            return false;
        }

Please report back your findings

[japomani]

I made the change and it had no effect, still got invalid security token error

[daftspunk]

This appears to be caused by d31006ae1a1f5a709e9a100d0096a5633ab820b5

Has been rolled back in 260e1f503fe18588f099de5efc928cfe941eed77

Should be fixed in Build 461

[LukeTowers]

Temporarily re-opening this so that we remember to finish our internal discussion on it before launching 461.

[daftspunk]

This is a complex issue, the commit mentioned in the rollback kicked off a series of changes the lead to this outcome. All derived commits have been rolled back as well, including 8da798a5cd2f5f640719550eebebc03e9a9d29d1

As discussed internally we will need to look at this again the future