CUNY Academic Commons

The plugin won't work as-is on the Commons. Its syntax requires authors to use pseudo-HTML tags, like <Loop>. But in our Multisite configuration, non-super-admins are limited in the markup that they are permitted in their posts. As such, when you try to use the plugin's tags, they're stripped by WordPress before being saved to the database.

The plugin looks useful and the concept is very clever, so I think it might be worth a small amount of time to figure out whether we can work around the limitation. Ray, I was hoping you could spend a few minutes to look it over. I'm thinking that we may be able to add the tags defined at https://plugins.trac.wordpress.org/browser/tangible-loops-and-logic/tags/2.3.9/vendor/tangible/template/tags/index.php to the list of tags allowed by kses. The tricky bit here is likely to be that we need to define attribute for each of these tags, and I'm not sure whether it'd be easy to come up with a clear list of these attributes.

Before working too much on this, I'd be curious to hear Ray's thoughts about the security implications of this kind of plugin. The "logic" parts - loops etc - seem like they're probably fairly innocuous, but arbitrary script elements seem particularly problematic to me.

If you agree that it's worthwhile, could you spend a small amount of time to see whether it's feasible to make Commons interoperability with an hour or two of work? If it seems like you're going to hit snags, I think we'll have to say no. (I've put into the 1.19.6 milestone but this can definitely be bumped if necessary.)

I did some basic testing and the plugin wouldn't require any major modifications on our end for the most part.

As long as users input their templates directly into the "Tangible > Templates" admin page and uses either the provided [template id=XXX] shortcode or the "Tangible Templates" block in their post, they should be good to go. I haven't tested the plugin's integration with ACF, but I assume that will also work.

The KSES problem only occurs if the Tangible syntax is used directly into the post's Code Editor. Using the template is easier to manage for the user, so we should be okay here.

but arbitrary script elements seem particularly problematic to me.

This is the big problem. I entered a <script src="XXX"></script> line into a Tangible Template and when rendered on the page, the JS loaded in the footer. It's possible to unhook the Tangible footer routine: https://plugins.trac.wordpress.org/browser/tangible-loops-and-logic/tags/2.3.9/vendor/tangible/template/actions/index.php#L29 on the 'tangible_templates_ready' hook and run our own without rendering the scripts call:

/**
 * Do not allow scripts to load for Tangible Loops and Logic.
 */
add_action( 'tangible_templates_ready', function( $html ) {
    // Remove default footer hooks.
    remove_action( 'wp_footer', $html->footer_action, 99 );
    remove_action( 'admin_footer', $html->footer_action, 99 );

    // Redo footer actions, but only allow styles.
    $html->footer_action = function() use ($html) {
        $html->load_all_enqueued_styles();
        $html->set_action_done( 'foot' );
    };

    // Add back footer hooks.
    add_action( 'wp_footer', $html->footer_action, 99 );
    add_action( 'admin_footer', $html->footer_action, 99 );
} );

I've added the fix for this here: https://github.com/cuny-academic-commons/cac/commit/f7929d245f3ae2606e881eba508f76a1d0c6ce11

One other thing is we will want to remove the "Tangible > Scripts" menu item and the "Script" tab from the Template and Layout admin pages to prevent confusion since the JS will no longer render on the frontend. See attached screenshot: https://redmine.gc.cuny.edu/attachments/21456/Scripts.png. I've just done this here: https://github.com/cuny-academic-commons/cac/commit/9ba9179aff4d74726b8ffc3b676437075ef9b5d8

I also tried to write some JS in the "Style" tab and Tangible prevented the JS from rendering on the frontend, so we should be okay with allowing the plugin to run on the Commons.

Awesome, Ray! Thank you for your great work! I had been looking only at the editor, and didn't notice that there was a different way to create templates.

Just wondering -- are these changes specific to the CAC or would it be useful to share them with the plugin author as a kind of safe-for-multisite version of the plugin?

I didn't see a GitHub repo for the plugin, but maybe I missed it. In any case, these improvements wouldn't be applicable to the public plugin, but Ray could probably make some suggestions that would address the same underlying issues - such as a toggle that disables script support, or improved allowedtags filtering so that you don't need unfiltered_html to use the plugin in Multisite.

Sorry - should I let the user know anything at this point?

The plugin will be available for use after the 1.19.6 release, on Tuesday March 22.

Crud.

This isn't great, but I've added some code to bypass KSES only for the 'tangible_template' and 'tangible_layout' post types: https://github.com/cuny-academic-commons/cac/commit/42aa542d5de317df557ae27d06f01581f73ffdbd. Coupled with the javascript mods I did above, this should be okay. However, it is still possible to write inline JS to an attribute like onmouseover="alert('hi')". Because of this, we might want to limit activation of the plugin only to the gcenglish site. What do you think, Boone?

Just to be clear about this patch, it looks like you are bypassing kses, but you had to polyfill force_balance_tags() in a way that avoids problems with upper-case tags. Is this correct?

Removing KSES and allowing JS, even just for this one site and these post types, feels pretty bad to me. There's the direct security risks of an editor knowingly or unknowingly using bad scripts. And there's the long-term maintenance burden - if Tangible changes the way the plugin works, we may have to revisit the fix.

I'm leaning toward a reverse of course, and telling the requester that we can't install the plugin for security reasons. Matt, if you're reading this, can you weigh in? I hate to be in the situation where we've told a user that we can accommodate a special request and then have to rescind it, but this may be one of those cases.

Ray, could you please pull the plugin and your mods from the codebase?

Done in https://github.com/cuny-academic-commons/cac/commit/a9c244cfdee1eb86bf02d3c8471b59ffc92a1bdf.

I was also emailing back-and-forth with one of the developers of Loops & Logic and he said that their team does not plan on implementing overrides for the default 'unfiltered_html' behavior. So this is a no-go as far as looking at adding Loops & Logic in the future.