0.9.5 Home Stretch Part 2

MODX Staff
12,272 Posts

Ryan Thrash Reply #11, 19 years, 11 months ago

I’ve got it David.

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me

1,097 Posts

PaulGregory Reply #12, 19 years, 11 months ago

As ever, willing to proof-read any text - just PM me.

No, I don't know what OpenGeek's saying half the time either.
MODx Documentation: The Wiki | My Wiki contributions | Main MODx Documentation
Forum: Where to post threads about add-ons | Forum Rules
Like MODx? donate (and/or share your resources)
Like me? See my Amazon wishlist
MODx "Most Promising CMS" - so appropriate!

1,010 Posts

TobyL Reply #13, 19 years, 11 months ago

Quote from: PaulGregory at Oct 25, 2006, 08:48 AM

Firstly, I should just point out how nice the new menu.php file is.

This is my best attempt at making this logic clear in pseudo-code, and pointing out potential errors in logic (although most of them are unlikely to ever actually happen in practice). Hopefully we can reach consensus here on the minor changes that can be made for the 0.9.5 release.

There are IFs around some entire menus, and around some menu lines, so I’m showing the IFs on the same line as the menu item, and the IFs (or EVERYONE if there isn’t an IF) at the top of each menu. I think this makes sense. Any menu item that starts a line without an IF is visible to anyone who can see that particular menu. I’ve then pointed out potential problems after each menu.

Wouldn’t it be much easier to switch the whole permission checking around? Instead of having multiple test to see if the top menu item should be displayed and then test for each individual case I think it would be easier to test and buffer each individual menu item and defer the the main menu tests until later. Here’s a rewritten example for the security menu.

<?php
# security menu
$security_menu = '';
/* manager-users */ if($modx->hasPermission('new_user')||$modx->hasPermission('edit_user')) 
    $security_menu .= '<li><a onclick="this.blur();" href="index.php?a=75" target="main">'.$_lang["user_management_title"].'</a></li>';
/* web-users     */ if($modx->hasPermission('new_web_user')||$modx->hasPermission('edit_web_user'))
    $security_menu .= '<li><a onclick="this.blur();" href="index.php?a=99" target="main">'.$_lang["web_user_management_title"].'</a></li>';
/*roles          */ if($modx->hasPermission('new_role')||$modx->hasPermission('edit_user'))
    $security_menu .= '<li><a onclick="this.blur();" href="index.php?a=86" target="main">'.$_lang["role_management_title"].'</a></li>';
/* manager-perms */ if($modx->hasPermission('access_permissions'))
    $security_menu .= '<li><a onclick="this.blur();" href="index.php?a=40" target="main">'.$_lang["manager_permissions"].'</a></li>';
/* web-user-perms*/ if($modx->hasPermission('web_access_permissions'))
    $security_menu .= '<li><a onclick="this.blur();" href="index.php?a=91" target="main">'.$_lang["web_permissions"].'</a></li>';

#same for other menu's
?>

<!-- All the html stuff -->

<!-- Security (users) -->
<?php if( $security_menu ) { ?>
<li id="limenu2"><a href="#menu2" onclick="new NavToggle(this); return false;"><?php echo $_lang["users"]; ?></a>
<ul class="subnav" id="menu2">
<?php echo $security_menu; ?>
</ul>
</li>
<?php } ?>

<!-- Clean up -->

<?php
	unset($security_menu);
?>

An empty (main) menu not being displayed would then be a simple consequence of an empty variable instead of having multiple tests for each contained permission. Making changes to the menu order would now only require one change in the list without the need to change permission checking in 2 places. To me this seems much easier to maintain and opens up the road to "editable user-specific menus".

eForm | eForm support | eForm @ wiki

MODX Staff
12,272 Posts

Ryan Thrash Reply #14, 19 years, 11 months ago

In 1805, I made a few updates to the config tabs, including nuking the redundant "settings" word from each title. The Miscellaneous tab was actually completely mislabelled, too, so I changed it. We should probably in reality add an accurate tab language define.

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me

1,097 Posts

PaulGregory Reply #15, 19 years, 11 months ago

TobyL, yes that makes perfect logical sense: build up the menu items, and then if there are any to be shown, show that menu.
I must admit I was blinkered by detailing the current method and didn’t step back to see how it should be done!

It would also mean in David’s case that commenting out Import and Export would mean the Tools menu wouldn’t show.

That’s clearly a step in the right direction.
Next actions:
1) Compare the permissions used in the routines to those checked for in the menu (at least one is in doubt) and
a) change menu if needed
b) point out any major anomolies
2) Overhaul menu as per my notes and TobyL’s superb suggestion

I can do this in about 8 hours time, but if someone wants to do it before, be my guest.

No, I don't know what OpenGeek's saying half the time either.
MODx Documentation: The Wiki | My Wiki contributions | Main MODx Documentation
Forum: Where to post threads about add-ons | Forum Rules
Like MODx? donate (and/or share your resources)
Like me? See my Amazon wishlist
MODx "Most Promising CMS" - so appropriate!

7,075 Posts

davidm Reply #16, 19 years, 11 months ago

Quote from: rthrash at Oct 25, 2006, 09:20 AM

I’ve got it David.

You mean the password / login for Hotscripts or/and the synthetized changelog ?
To get started I’d need a clear view of what has to be emphasized.
Given all the changes that have taken place, I miss perspective right now...

Quote from: PaulGregory at Oct 25, 2006, 12:24 PM

As ever, willing to proof-read any text - just PM me.

Great

Then check out this thread : http://modxcms.com/forums/index.php/topic,8293.0.html
I have created a WriteBoard page to allow for easy editing, revisionning and collective writing

.: COO - Commerce Guys - Community Driven Innovation :.

MODx est l'outil id

MODX Staff
12,272 Posts

Ryan Thrash Reply #17, 19 years, 11 months ago

There are boatloads of "\n" characters in the language file. We need to address these as the language strings are now wrapped in single quotes. Bullet added to the list above.

update: I also noticed hard coded style information too, like <span style="color:#990000">

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me

MODX Staff
12,272 Posts

Ryan Thrash Reply #18, 19 years, 11 months ago

Quote from: PaulGregory at Oct 26, 2006, 05:36 AM

Next actions:
1) Compare the permissions used in the routines to those checked for in the menu (at least one is in doubt) and
a) change menu if needed
b) point out any major anomolies
2) Overhaul menu as per my notes and TobyL’s superb suggestion

I can do this in about 8 hours time, but if someone wants to do it before, be my guest.

Sounds like a great plan to me and I’d love to see it done properly, though user changeable menu structures should probably wait for a future release (hack away all you’d like though).

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me

MODX Staff
12,272 Posts

Ryan Thrash Reply #19, 19 years, 11 months ago

In 1806 I committed an update to the login box. ~~In the process I found a bug with a bad path to the Captcha image. Could someone test to see if it works properly now?~~ ignore me... was thinking of the path from the login.html file ... stoopid!

Also related to the login tweaks, I removed some hard coded styles in the placeholders that were being set in accesscontrol.inc.php. As such, the non Light themes probably need to have their login box styles updated.

Ryan Thrash, MODX Co-Founder
Follow me on Twitter at @rthrash or catch my occasional unofficial thoughts at thrash.me

1,010 Posts

TobyL Reply #20, 19 years, 11 months ago

A few more notes about permissions (for now or for the future):

Modules
Considering all modules can be run directly from the menu would it not make sense to hide the whole module management page from users who have no create/edit/save rights for modules? Modules like DocManager should then for instance respond by returning to the ’home’ page when the user clicks on Close Doc Manager.

If a user without sufficient rights lands on the manage module page he/she should be alerted there an then about insufficient rights instead of being alerted when trying to open a module for editing. I know that they could run the module from that page but to an ordinary user it is confusing to see a list of links that have unexpected results. To them it is not logical to click the icon first and then use the menu just to run teh module. The normal reaction would be to click on the first visible link to a module. Instead of running it they would get a ’no permissions’ message. To them this says "No permission to run"??

Any confusion can easily be avoided by not sending users (who don;t have sufficient permissions) to the manage module page in the first place and secondly (if the first is not possible) to change the behaviour so that the links on the manage modules page will run the module instead of editing them.

Roles management
I would like to see that a user can never raise or give permissions beyond the permissions they have themselves.

I have for example a user role called webManager who is capable of doing everything in MODx except anything to do with coding so editing modules and snippets are out of bounds but a webManager can create new users, assign permissions to them and even create new roles. There’s an obvious problem with being able to creating/editing roles as they could simply raise their own permissions or give anyone else permissions they themselves do not have.

Still I find it usefull to have someone who can do all the management and most of the admin on a site but whom I don’t trust to fiddle with the snippets and TV’s (for example).

I haven’t looked into the appropriate code but I think it wouldn’t be all that difficult to add the limitation that you cannot raise your own or anyone else’s permissions (using roles) beyond your own role.

eForm | eForm support | eForm @ wiki