October: Permission cms.manage_theme_options not usable without cms.manage_themes

Created on 14 Dec 2017  路  10Comments  路  Source: octobercms/october

Expected behavior

When a backend user has the permission to Configure customization options for the active theme but not Activate, deactivate and configure CMS themes navigating to CMS -> Front-end theme should open the active theme's update page directly.

Actual behavior

The page returns an "ACCESS DENIED" message.

Reproduce steps
  1. Create a new client backend account.
  2. Set the permission Configure customization options for the active theme to allow but Activate, deactivate and configure CMS themes as deny.
  3. Login thro that account and navigate to Settings -> CMS and click on Front-end theme.
October build

430

I tried removing this piece of code in modules/cms/controllers/Themes.php

/**
 * @var array Permissions required to view this page.
 */
public $requiredPermissions = ['cms.manage_themes'];

And the problem seems to have been resolved i.e. I'm able to now navigate to the Front-end theme page from the backend in client account and it directly opens the theme's update page. And when I do the same from admin account it shows me all the theme listings. I also tried enabling the permission to Activate, deactivate and configure CMS themes in client account with this code removed and navigated to the page to see that I can indeed see the theme listing as expected. So, I believe that removing this code fixes this issue but I am not certain if removing it would affect any other part of the web app and hence didn't make any PR.

Medium Completed Bug

All 10 comments

@SaifurRahmanMohsin this should be handled by https://github.com/octobercms/october/blob/master/modules/cms/controllers/Themes.php#L48-L55, are you saying that it doesn't?

That's correct, that event doesn't work as expected. When I first created the client account with the customization permissions (ALLOW) and configure CMS themes (DENY) it actually showed me the "ACCESS DENIED" message rather than redirecting. I'm guessing that the ACCESS DENIED page returns before the event code returns the redirect and that's why this happens.

AH! I've got it now :) It was a necessary change but one that caused issues in this case: https://github.com/octobercms/october/commit/c7a3354dfd5fcad9a435dae0f15db4ace80de4ef#diff-6cdbb280344f40eebe758cf8e8e5f7d9.

We're going to have to look at options for hooking in before the authentication system kicks users out, whether that's in the form of a new event or if it's by putting the authentication code in the page.beforeDisplay event and then allowing other bound events to that event to run before it if they have been specially marked as such. Thoughts?

Basically the difference between

// request needs to be kicked out
if ($response = $this->fireSystemEvent('backend.page.invalidAuth', $params)) {
    return $response;
}

and

$this->bindEvent('page.beforeDisplay', function ($action, $params) {
    // check if request needs to be kicked out
});

^ being prevented by other code hooking before it with $this->bindEvent('page.beforeDisplay', $function, 9999);

Just ran into this problem ourselves. Wanting to give clients the ability to customise the active theme but certainly not to delete/add/edit properties of other themes.

Removing the line in @SaifurRahmanMohsin original post is the only way we've found to get round this. Additionally, when the client is done customising theme options the client can "save" or "save and close"... the "save and close" option saves but doesn't close.

Can see this has been tagged with a medium priority from over a year ago, so any suggestions we can override the permissions setting without having to hack the core by removing the required permissions line in the meantime?

Thanks in advance!

@gavinworks you could aways submit a PR to address the issue, or make a contribution to have a core developer work on it.

@LukeTowers oooo I like the idea of a contribution! How would that work exactly?

@gavinworks create PR and provide a solution. As easy as that :)

@w20k Luke said submit a PR... ORRR "make a contribution to have a core developer work on it." Sounds like he is alluding to my making a financial contribution to have a core developer work on it. This is the bit I'm interested in. Obviously if I was a talented enough developer I'd fix myself and submit a PR ;)

@LukeTowers could you perhaps confirm if I've understood this correctly?

@gavinworks I think you got it right, didn't read the whole thread! You could talk to @LukeTowers directly via slack or email, or via other communication channels 馃槈

@w20k no problem! I didn't realise you were on slack. I've DM'd him on there now :)

Was this page helpful?
0 / 5 - 0 ratings