[MAGNOLIA-6615] UserManager#getUsersWithGroup(groupName, transitive) is b0rked, never worked Created: 24/Mar/16 Updated: 21/Dec/16 Resolved: 07/Apr/16 |
|
| Status: | Closed |
| Project: | Magnolia |
| Component/s: | security |
| Affects Version/s: | 5.3.7, 5.4.5 |
| Fix Version/s: | 5.3.15, 5.4.6 |
| Type: | Bug | Priority: | Major |
| Reporter: | Mikaël Geljić | Assignee: | Oanh Thai Hoang |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | usermanager | ||
| Remaining Estimate: | 0d | ||
| Time Spent: | 5d 5h | ||
| Original Estimate: | 4d | ||
| Attachments: |
|
||||||||||||||||||||||||||||||||
| Issue Links: |
|
||||||||||||||||||||||||||||||||
| Template: |
|
||||||||||||||||||||||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||||||||||||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||||||||||||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||||||||||||||||||||||
| Release notes required: |
Yes
|
||||||||||||||||||||||||||||||||
| Date of First Response: | |||||||||||||||||||||||||||||||||
| Sprint: | Saigon 39 | ||||||||||||||||||||||||||||||||
| Story Points: | 8 | ||||||||||||||||||||||||||||||||
| Description |
|
UserManager#getUsersWithGroup(groupName, transitive) is just b0rked and most likely never worked properly... Considering user/group setup from_templating-samples_, as follows:
UserManager#getUsersWithGroup("employees", true) doesn't return david (wrong) Concept: Implementing only part https://wiki.magnolia-cms.com/display/DEV/Concept+-+Improvements+in+the+security+package#Concept-Improvementsinthesecuritypackage-1a.super-groupsvs.sub-groups |
| Comments |
| Comment by Jan Haderka [ 24/Mar/16 ] |
|
Was it actually supposed to find all the groups, including inherited ones or only directly assigned groups? While I have no idea what the intention behind the method was, it would be worth investigating its use and history of changes of the class, before assuming it is broken. It might be just incorrectly named. |
| Comment by Mikaël Geljić [ 24/Mar/16 ] |
|
Linked the original ticket where this was implemented (fairly "recent" fwiw). This is indeed expected to return groups transitively, as opposed to UserManager#getUsersWithGroup(String), where only direct members of a group are returned. Current implementation is transitive, but the wrong way around, i.e. #getUsersWithGroup("developers", true) would return all employees, even those who are non-developers. And again, comparing to impl of User#getAllGroups, as shown in screenshot, this is clearly at least inconsistent. |
| Comment by Jan Haderka [ 29/Mar/16 ] |
|
Cool. Thx for checking it out. |