search

Geeklog-Core / geeklog

Geeklog - The Secure CMS.
https://www.geeklog.net
25 stars 19 forks source link

User Group Assignments Cleanup #1082

Closed eSilverStrike closed 2 years ago

eSilverStrike commented 3 years ago

See forum post: https://www.geeklog.net/forum/viewtopic.php?showtopic=25830#97372 for more information.

For purposes of increasing speed can we assume a few things when it comes to certain user groups like:

Why is it necessary for GeekLog to have all groups added to the Root Group?

Anonymous too: if uid = 1, then no privilege is assumed.

Confirm this is not the same for Geeklog Super User = 2 since I believe now (or at least in older Geeklog versions) User 2 can be deleted if another user belongs to the Root Group (since there must be 1 user that does)

Example:

  1. The group 'Group Admin' is added to the Root Group.
  2. The group 'User Admin' is added to the group 'Group Admin'.
  3. The group 'User Admin' is added to the Root Group.

Why is (3) necessary?

Check forum posts for other examples...

eSilverStrike commented 2 years ago

Reviewing this improvement request and the forum post which also talks about. At first I thought maybe there could be some speed improvements here.

I am sure there would be but I am hesitant to start to make special conditions for certain groups. The current system works well now and applies to all groups equally. I do not see the need to mess with it as the cons I believe will out weight the benefit of some speed increase.

Also for the 1,2,3 example. Yes Root Group does already get all the privileges from the User Admin Group (since it belongs to the Group Admin which the User Admin Group is included) but again the rule is that Root Group belongs to all groups. If we start relying on inheritance then if the user ever deletes the include right for another group in a group then the Root Group will break (I realize this can't happen for core groups but I am thinking of plugins). This would mean we would also start having to add special cases to the Group Editor etc. so it would show the Root Group and other "special" groups properly.

@mystralkk What do you think of this improvement and my comments on it?

I am thinking of just closing the issue and marking it with a label of "suspended" I guess.

mystralkk commented 2 years ago

I support you about how to handle this issue. Making suggested changes may bring some increase in speed, but it could cause unexpected trouble. In my opinion, If we just look at speed increase, we can get it by caching the value of $_GROUPS for a limited span of time by using the Cache class.