Relationship Graph

Relationship Graph
related to related to child of child of duplicate of duplicate of

View Issue Details

This bug affects 1 person(s).
 18
IDProjectCategoryView StatusLast Update
18281Bug reportsUser / Groups / Rolespublic2023-07-31 12:32
Reporterp_teichmann Assigned Totibor.pacalat  
PrioritynoneSeverityminor 
Status closedResolutionfixed 
Product Version5.3.x 
Summary18281: Users in group are not deleted
Description

When you click the menu entry “user groups“ in the top menu configuration you get an error (page is not loading) Screenshot attached

  1. The reason is (maybe), that when deleting a usergroup the entry in table user_in_groups is not deleted, but it should.
  2. Simply set debug=0 and it works again
Steps To Reproduce

Steps to reproduce

Case 1.
Create a new user A with user management permissions.
With that user, create a group with a member (any member).
Delete the user A
Try to see the list of user groups.
Try to see the members of the group.

Case 2.
Create a new user AB .
Create a group having B as member.
Delete the user B
Try to see the list of user groups.
Try to see the members of the group.

Expected result

All good. No dumps.

Actual result

Dump

TagsNo tags attached.
Attached Files
Bug heat18
Complete LimeSurvey version number (& build)5
I will donate to the project if issue is resolvedNo
Browser
Database type & versionMySQL
Server OS (if known)
Webserver software & version (if known)
PHP Version7.2

Relationships

has duplicate 18400 closedgabrieljenik After I delte a User there will still be an entry in lime_user_in_groups 
related to 18294 closedgabrieljenik User can not see group created and user in group created 
related to 18936 resolvedtibor.pacalat User count in group is not OK after deleting a user. 
child of 18289 confirmed User with group creation allowed can not see is own group 

Activities

gabrieljenik

gabrieljenik

2022-07-27 22:32

manager   ~71204

Can you provide more steps for rerproducing?
I think you had some kind of orphan records or broken relationships..

gabrieljenik

gabrieljenik

2022-08-04 15:26

manager   ~71322

Last edited: 2022-08-04 15:28

So the problem happens when you create a usergrp as a user and then delete the user, when you then go to the usergroups page the error is visible. The main problem is that the users dont get deleted at all from the tables "user_groups" and "user_in_groups".

gabrieljenik

gabrieljenik

2022-08-09 19:28

manager   ~71391

Part of the problems had to do with the fact that the code assumed that if you were the owner you were in the group. In LTS it is like this, the owner is always in the group. But in master no.

Also done:

  • Transfer to user 1 if the owner user is deleted
  • If the owner of a group no longer exists, instead of the name it shows "(Deleted user)" in the table. (shouldn't happen, but still)
  • Fixed inconsistencies in the actionViewGroup(). Things that were set inside IF statements and were always used.

https://github.com/LimeSurvey/LimeSurvey/pull/2565

DenisChenu

DenisChenu

2022-08-23 16:00

developer   ~71519

Last edited: 2022-08-23 16:00

My opinion : we must have a clean decision about $userGroup->owner_id == Yii::app()->user->id on the right part
We don't have this control in 3.X

I think we must have UserGroup->hasPermission with something like this

    /* superadmin can do whole */
    if(Permission::model()->hasPermission(0, 'global', 'superadmin', 'read', $iUserID)) {
        return true;
    }
    /* usercontrolSameGroupPolicy : need to be in group */
    if (App()->getConfig('usercontrolSameGroupPolicy') && !$this->hasUser($iUserID)) {
        return false;
    }
    /* Check global right only (no single rights) */
    return Permission::model()->hasPermission(0, 'global', 'usergroups', $sCRUD, $iUserID);

When decision is done about owner : we can easily add it here.

gabrieljenik

gabrieljenik

2022-08-23 17:17

manager   ~71520

I think we must have UserGroup->hasPermission with something like this

So, you wouldn't care about who owns the group, right?

DenisChenu

DenisChenu

2022-08-24 08:16

developer   ~71522

Currently no, be cause we don't care in 3.X and in current 5.X.
owner_id are not used in UserGroup permission since years … do not add it without discussion (and API update)

Then : need decision about usage of owner_id before adding it.

See the last sentence

When decision is done about owner : we can easily add it here.

gabrieljenik

gabrieljenik

2022-08-24 14:30

manager   ~71525

owner_id are not used in UserGroup permission since years

Not sure if we are talking about the same thing... I see it is being used a lot...

https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/UserGroup.php#L191
https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/UserGroup.php#L232

https://github.com/LimeSurvey/LimeSurvey/blob/master/application/controllers/UserGroupController.php#L178
https://github.com/LimeSurvey/LimeSurvey/blob/master/application/controllers/UserGroupController.php#L190
https://github.com/LimeSurvey/LimeSurvey/blob/master/application/controllers/UserGroupController.php#L347
....

Are we talking about the same stuff?

do not add it without discussion (and API update)

No worries, I am not saying I will, I will not... Why would I?

DenisChenu

DenisChenu

2022-08-27 19:49

developer   ~71544

Not sure if we are talking about the same thing... I see it is being used a lot...

Seems to didn't work with specific permission then …

Time to update to real Permission system …
https://github.com/LimeSurvey/LimeSurvey/blob/master/application/models/Traits/PermissionTrait.php

gabrieljenik

gabrieljenik

2022-08-29 15:17

manager   ~71555

Look forward for decisions and directions ...

c_schmitz

c_schmitz

2023-05-30 14:50

administrator   ~75255

For LS5:
The group owner should be member of the group.
For LS6:
The group owner can be a group member, but doesn't have to be. However, the group owner should always be able to see/edit group members.

gabrieljenik

gabrieljenik

2023-07-07 23:35

manager   ~75989

@tibor.pacalat if you could test it one more time before merging would be great as this seems sensitive

tibor.pacalat

tibor.pacalat

2023-07-10 13:26

administrator   ~76001

Last edited: 2023-07-10 13:27

@gabrieljenik when I include owner in the group (let's say 2 members and owner), and then delete the owner, members count doesn't account for that and shows 3 still.

Also, I noticed there is no way to change owner of the group once the owner has been deleted (except from the DB).

gabrieljenik

gabrieljenik

2023-07-10 14:37

manager   ~76003

when I include owner in the group (let's say 2 members and owner), and then delete the owner, members count doesn't account for that and shows 3 still.

That's treated on 18936
We can wait and merge both at the same time if prefer.

tibor.pacalat

tibor.pacalat

2023-07-10 15:04

administrator   ~76010

No, that is fine, I merged it.

gabrieljenik

gabrieljenik

2023-07-10 15:10

manager   ~76012

v5 is pending. Will add PR soon

guest

guest

2023-07-10 15:18

viewer   ~76014

Fix committed to master branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=34954

gabrieljenik

gabrieljenik

2023-07-12 15:18

manager   ~76060

v5: https://github.com/LimeSurvey/LimeSurvey/pull/3284

LimeBot

LimeBot

2023-07-17 10:07

administrator   ~76107

Fixed in Release 6.1.8+230717

gabrieljenik

gabrieljenik

2023-07-18 13:49

manager   ~76129

Please, if possible give it a quick test as this may be sensitive.

guest

guest

2023-07-18 14:03

viewer   ~76131

Fix committed to 5.x branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=35185

guest

guest

2023-07-18 14:03

viewer   ~76132

Fix committed to 5.x branch: http://bugs.limesurvey.org/plugin.php?page=Source/view&id=35184

tibor.pacalat

tibor.pacalat

2023-07-18 14:04

administrator   ~76133

Tested fix for 5.x and merged.

LimeBot

LimeBot

2023-07-31 12:32

administrator   ~76363

Fixed in Release 5.6.32+230731

Related Changesets

LimeSurvey: master 5692fcba

2023-07-10 15:04:05

Gabriel Jenik


Committer: GitHub Details Diff
Fixed issue 18281: Users in group are not deleted (#2565)

Co-authored-by: encuestabizdevgit <devgit@encuesta.biz>
Co-authored-by: lapiudevgit <devgit@lapiu.biz>
Affected Issues
18281
mod - application/controllers/UserGroupController.php Diff File
mod - application/controllers/UserManagementController.php Diff File
mod - application/models/UserGroup.php Diff File

LimeSurvey: 5.x ad62e137

2023-07-18 14:03:43

Gabriel Jenik


Committer: GitHub Details Diff
Fixed issue 18281: Users in group are not deleted (#3284)

Co-authored-by: encuestabizdevgit <devgit@encuesta.biz>
Co-authored-by: lapiudevgit <devgit@lapiu.biz>
Affected Issues
18281
mod - application/controllers/UserGroupController.php Diff File
mod - application/controllers/UserManagementController.php Diff File
mod - application/models/UserGroup.php Diff File
mod - application/views/userGroup/usergroups_view.php Diff File

LimeSurvey: 5.x ad62e137

2023-07-18 14:03:43

Gabriel Jenik


Committer: GitHub Details Diff
Fixed issue 18281: Users in group are not deleted (#3284)

Co-authored-by: encuestabizdevgit <devgit@encuesta.biz>
Co-authored-by: lapiudevgit <devgit@lapiu.biz>
Affected Issues
18281
mod - application/controllers/UserGroupController.php Diff File
mod - application/controllers/UserManagementController.php Diff File
mod - application/models/UserGroup.php Diff File
mod - application/views/userGroup/usergroups_view.php Diff File

Issue History

Date Modified Username Field Change
2022-07-27 11:04 p_teichmann New Issue
2022-07-27 11:04 p_teichmann File Added: 58aefca2-a89d-4515-b277-d4cfdd7b17a9.png
2022-07-27 11:05 p_teichmann Assigned To => gabrieljenik
2022-07-27 11:05 p_teichmann Status new => assigned
2022-07-27 22:32 gabrieljenik Note Added: 71204
2022-07-27 22:32 gabrieljenik Bug heat 0 => 2
2022-07-27 22:32 gabrieljenik Status assigned => feedback
2022-08-04 15:26 gabrieljenik Status feedback => assigned
2022-08-04 15:26 gabrieljenik Note Added: 71322
2022-08-04 15:28 gabrieljenik Note Edited: 71322
2022-08-09 19:28 gabrieljenik Assigned To gabrieljenik => DenisChenu
2022-08-09 19:28 gabrieljenik Status assigned => ready for code review
2022-08-09 19:28 gabrieljenik Note Added: 71391
2022-08-23 15:49 DenisChenu Relationship added child of 18289
2022-08-23 16:00 DenisChenu Note Added: 71519
2022-08-23 16:00 DenisChenu Bug heat 2 => 4
2022-08-23 16:00 DenisChenu Note Edited: 71519
2022-08-23 16:30 DenisChenu Assigned To DenisChenu => gabrieljenik
2022-08-23 16:30 DenisChenu Status ready for code review => in code review
2022-08-23 17:17 gabrieljenik Note Added: 71520
2022-08-24 08:16 DenisChenu Note Added: 71522
2022-08-24 14:30 gabrieljenik Note Added: 71525
2022-08-27 19:49 DenisChenu Note Added: 71544
2022-08-29 15:17 gabrieljenik Assigned To gabrieljenik => c_schmitz
2022-08-29 15:17 gabrieljenik Status in code review => feedback
2022-08-29 15:17 gabrieljenik Note Added: 71555
2022-09-09 16:24 DenisChenu Relationship added related to 18294
2022-10-17 22:10 gabrieljenik Relationship added has duplicate 18400
2022-10-17 22:10 gabrieljenik Bug heat 4 => 10
2023-05-30 14:50 c_schmitz Note Added: 75255
2023-05-30 14:50 c_schmitz Bug heat 10 => 12
2023-06-22 21:30 gabrieljenik Steps to Reproduce Updated
2023-06-26 16:49 gabrieljenik Assigned To c_schmitz => gabrieljenik
2023-06-26 16:49 gabrieljenik Status feedback => ready for testing
2023-07-03 22:45 gabrieljenik Issue cloned: 18936
2023-07-03 22:45 gabrieljenik Relationship added related to 18936
2023-07-07 23:34 gabrieljenik Assigned To gabrieljenik => tibor.pacalat
2023-07-07 23:34 gabrieljenik Status ready for testing => ready for merge
2023-07-07 23:35 gabrieljenik Note Added: 75989
2023-07-10 13:26 tibor.pacalat Note Added: 76001
2023-07-10 13:26 tibor.pacalat File Added: Screenshot 2023-07-10 at 13.24.23.png
2023-07-10 13:26 tibor.pacalat Bug heat 12 => 14
2023-07-10 13:27 tibor.pacalat Note Edited: 76001
2023-07-10 13:27 tibor.pacalat Note Edited: 76001
2023-07-10 14:37 gabrieljenik Note Added: 76003
2023-07-10 15:04 tibor.pacalat Note Added: 76010
2023-07-10 15:04 tibor.pacalat Status ready for merge => resolved
2023-07-10 15:04 tibor.pacalat Resolution open => fixed
2023-07-10 15:10 gabrieljenik Assigned To tibor.pacalat => gabrieljenik
2023-07-10 15:10 gabrieljenik Status resolved => assigned
2023-07-10 15:10 gabrieljenik Note Added: 76012
2023-07-10 15:18 Changeset attached => LimeSurvey master 5692fcba
2023-07-10 15:18 guest Note Added: 76014
2023-07-10 15:18 guest Bug heat 14 => 16
2023-07-12 15:18 gabrieljenik Assigned To gabrieljenik => DenisChenu
2023-07-12 15:18 gabrieljenik Status assigned => ready for code review
2023-07-12 15:18 gabrieljenik Note Added: 76060
2023-07-13 11:58 DenisChenu Assigned To DenisChenu =>
2023-07-13 11:58 DenisChenu Status ready for code review => ready for testing
2023-07-17 10:07 LimeBot Note Added: 76107
2023-07-17 10:07 LimeBot Status ready for testing => closed
2023-07-17 10:07 LimeBot Bug heat 16 => 18
2023-07-17 17:30 gabrieljenik Status closed => ready for testing
2023-07-18 13:49 gabrieljenik Assigned To => tibor.pacalat
2023-07-18 13:49 gabrieljenik Status ready for testing => ready for merge
2023-07-18 13:49 gabrieljenik Note Added: 76129
2023-07-18 14:03 Changeset attached => LimeSurvey 5.x ad62e137
2023-07-18 14:03 guest Note Added: 76131
2023-07-18 14:03 Changeset attached => LimeSurvey 5.x ad62e137
2023-07-18 14:03 guest Note Added: 76132
2023-07-18 14:04 tibor.pacalat Status ready for merge => resolved
2023-07-18 14:04 tibor.pacalat Note Added: 76133
2023-07-31 12:32 LimeBot Note Added: 76363
2023-07-31 12:32 LimeBot Status resolved => closed