Currently the module administration pages do access check for 'administer filters' permission.

This module should provide it's own permissions. It should be at least one 'administer wysiwyg templates'. And after we complete the export/import patch (See #1371202: Add support for export/import through UI and features module), we will also need a permission for 'import wysiwyg from PHP code' with a security warning since it does an eval().

CommentFileSizeAuthor
#8 wysiwyg_template-1409360-add-permissions.patch3.61 KBrecidive
#2 wysiwyg_template-1409360-add-permissions.patch2.6 KBskwashd

Comments

guybedford’s picture

guybedford commented

Would certainly make sense to have a wysiwyg templates permission. I'm more than happy to add this, but if anyone wants to submit a patch, that is welcome as well.

Is there not a more generic permission that can be used for the exporting though?

skwashd’s picture

skwashd commented
Status: Active » Needs work
StatusFileSize
new wysiwyg_template-1409360-add-permissions.patch2.6 KB

The attached patch adds a new "administer wysiwyg template" permission. It is based on the current 7.x-2.x branch.

skwashd’s picture

skwashd commented
Status: Needs work » Needs review

fixing status

guybedford’s picture

guybedford commented
Status: Needs review » Needs work

Thanks, that seems great for the menu permissions. We still need to add the import permissions I believe. I will be looking into this when I do a full review later this month.

skwashd’s picture

skwashd commented
Status: Needs work » Needs review

I agree the import permissions are needed - when the import code lands in the tree. Right now I have a site where I want to allow users to edit templates, but I don't want to grant them access to manage filters. This provides finer grained access controls for the existing functionality.

If this is in the repo, recidive can add the import permissions when he next rerolls his import patch.

recidive’s picture

recidive commented
Status: Needs review » Reviewed & tested by the community

IMO we should get the permissions for what we already have.

I'll make sure to add a permission for import/export.

recidive’s picture

recidive commented
Status: Reviewed & tested by the community » Needs work

Patch no longer applies. Need to be rerolled adding a 'access callback' to the import menu item, since this run php code.

recidive’s picture

recidive commented
Status: Needs work » Needs review
StatusFileSize
new wysiwyg_template-1409360-add-permissions.patch3.61 KB

Ok, I re-rolled the patch adding 'import wysiwyg template' permission.

skwashd’s picture

skwashd commented
Status: Needs review » Reviewed & tested by the community

We are using this internally and it works as advertised. Can I RTBC a reroll of a patch I wrote?

recidive’s picture

recidive commented

I'll let @guybedford weigh in before committing this.

guybedford’s picture

guybedford commented

I may have suggested the import permissions without properly thinking it through... is this really necessary?

Personally I'm a fan of simple permissions - I just thought there may be deeper security implications, but I don't think there really are.

Happy to go ahead with the permissions commit, just decide if we really need the import permissions or not!

skwashd’s picture

skwashd commented

A separate permission is required as it allows execution of PHP code - which is a security consideration.

guybedford’s picture

guybedford commented

Ok yes of course, makes sense.

adriaanm’s picture

adriaanm commented

Can we get this in a new release?

guybedford’s picture

guybedford commented
Status: Reviewed & tested by the community » Closed (fixed)

Sure I've committed this into the latest release.