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

Bad Permission Checks #376

Closed
exponentcms opened this issue Apr 27, 2020 · 6 comments
Closed

Bad Permission Checks #376

exponentcms opened this issue Apr 27, 2020 · 6 comments
Assignees

Comments

@exponentcms
Copy link
Owner

I have a user who is a member and admin of a group called editors. On a page I have a container that has a text module in it. The Group has full permissions to the text module and no permissions to the container.

Referring to file: https://github.com/exponentcms/exponent-cms/blob/master/framework/modules-1/containermodule/views/Default.tpl

The permission check fails on a opening div but later passes on the matching closing div. This cause an extra closing div that wreaks havoc on a layout.

First of all the permission checks are do not match. Line 18 does not match the the check on Line 100
Line 18:
@@@
{if ($permissions.administrate == 1 || $permissions.edit_module == 1 || $permissions.delete_module == 1 || $permissions.add_module == 1 || $container->permissions.administrate == 1 || $container->permissions.edit_module == 1 || $container->permissions.delete_module == 1)}
@@@
Line 100:
@@@
{if ($permissions.administrate == 1 || $permissions.edit_module == 1 || $permissions.delete_module == 1 || $permissions.add_module == 1 || $container->permissions.administrate == 1 || $container->permissions.edit_module == 1 || $container->permissions.delete_module == 1 || $container->permissions.configure || $container->permissions.configure == 1)}
@@@

Line 70 and 87 are opening/closing tag pairs.
However Line 58 is just {permissions} while Line 85 includes a level. As well, the {if} on Line 59 does not match the {if} on Line 86.

These opening and closing must match, but there is a larger issue as well.

The initial check on Line 18 is looking for $container->permission.bla-bla-bla. However $container is not set until the {foreach} loop on line 52. Its closing tag on line 100 also checks for $container permissions. But since the var is not set (to the last container in the loop) it might pass when. Thus the extra closing div wreaking havoc.

To fix my layout I added the following at the very top.
@@@
{foreach key=key name=c from=$containers item=container}

{/foreach}
@@@
This should count as a horrible, horrible hack and a better way that checks the permissions of the specific container explicitly rather than whatever was the last container in the foreach.

@exponentcms
Copy link
Owner Author

FWIW, because we can have nested containers...the $container variable at the top of the file is the 'current' container when the Default template is pulling in/including (line 55) a copy of itself.

@exponentcms
Copy link
Owner Author

[bulk edit]

@exponentcms
Copy link
Owner Author

[bulk edit]

@exponentcms
Copy link
Owner Author

This may have been alleviated somewhat by a recent push with a cleanup of the container templates

@exponentcms
Copy link
Owner Author

should no longer be an issue.

@exponentcms
Copy link
Owner Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants