Fixes access permissions #30
Conversation
fajo-de
commented
Jun 1, 2016
•
fajo-de
commented
- fixed access for admin only pages
- prevent users from enabling debuging and multiple logins on their own profile
- limit write access to configuration to admin
- allow users to read generic parts of the configuration
- allow access to home.php for all users
- prevent users from enabling debuging and multiple logins on their own profile - limit write access to configuration to admin - allow users to read generic parts of the configuration
|
Thanks for the PR @fajo-de - I can't merge as is, since the admin group has been renamed from "namedadmins" to "admins" which would break functionality for existing installations using LDAP authentication. Aside from this, if you update and re-submit the PR, should be OK for merging. |
|
Hi Jethro, the group has actually not been renamed. All I did was do move some permissions over to the admin group (such as global configuration, ...). When upgrading namedmanager usually nothing has to be done as users that manage accounts (members of the "admin" group) usually are also the ones that should have the right to deal with the application configuration. "namedadmins" should only be able to deal with zones and zone data. In the worst case some (few) users will have to be added to the "admin" group too prior to upgrading to make sure they still can configure the application. A more fine graned access control (user manager, config manager, zone manager [all zones or even single zone skope, zone data manager [all zones or even singe zone skope], .... the more fine graned the better ;) would be the best, but this would require many more changes and additions to the code. Cheers, |
|
OK yes not renamed, rather you've introduced the separate admin group. Unfortunately this could cause unexpected behaviour with our somewhat terrible LDAP implementation which maps the in-app groups with posixGroups inside LDAP. https://github.com/jethrocarr/namedmanager/wiki/Installation-Integration-LDAP So merging this change could suddenly see people in another group "admin" in LDAP be granted admin access to NamedManager which could be unexpected :-/ Not an easy fix, kind of need to rework the LDAP logic and do a non-compatible release. |
|
Agree in principal that finer grained ACL would be a wonderful thing. |
|
Just checked the LDAP code and yes, a major rewrite is recommended (also for the "ldaponly" part - reading the "userPassword" attribute usually is not permitted ;). There are multiple options to make the change working with LDAP support. One would be to simply rename "admin" to some group name starting with "named..." and extend the LDAP code to search in this group as well. Instead of renaming "admin" some configurable mapping could be used, so the user is free to use any LDAP group name for NamedManager by just mapping his name to the internal ones. The above is not optimal but at least causes the change no to interfere with current the installation - existing users just need to configure the mapping when upgrading. If this is a practicable way for you I would extend the change to include the mapping and the LDAP code extension; just let me know. |
