ListGroups method to list all child and grandchild groups too#327
ListGroups method to list all child and grandchild groups too#327ChellaVignesh wants to merge 1 commit into
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Conventional Commits Report😢 No conventional commits found. 👉 Learn more about the conventional commits usage at Greenbone. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #327 +/- ##
==========================================
+ Coverage 57.75% 57.81% +0.06%
==========================================
Files 59 59
Lines 3082 3096 +14
==========================================
+ Hits 1780 1790 +10
- Misses 1165 1167 +2
- Partials 137 139 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
mgoetzegb
left a comment
mgoetzegb
left a comment
There was a problem hiding this comment.
Moving from Group Name to the Group Path is indeed important as group names are not guaranteed to be unique. Otherwise we can't differentiate between memberships in e.g. Groups
- GroupA/N1
- GroupA/N1/N1
- GroupB/N1
In cases where we want to compare the Group Path with the entry in the JWT token, we need to ensure then that the keycloak group protocol mapper puts the full group path in the token, rather than just the name.
| "subGroupCount":2, | ||
| "subGroups":[ | ||
| {"id":"c1","name":"Admins","path":"/Platform/Admins","subGroupCount":0,"subGroups":[]}, | ||
| {"id":"c2","name":"Users","path":"/Platform/Users","subGroupCount":0,"subGroups":[]} |
There was a problem hiding this comment.
Maybe add a subgroup to group Users to verify that we extract the subgroups recursively.
| // Group represents a Keycloak group. | ||
| type Group struct { | ||
| ID string `json:"id"` | ||
| Name string `json:"name"` |
There was a problem hiding this comment.
Maybe add a clarifying comment to the Name property, that this is the full path of the group rather than just the name.
Or maybe even rename the property, it is a breaking change, but it might make the code more clear.
| name := g.Path | ||
| if name == "" { | ||
| name = g.Name | ||
| } |
There was a problem hiding this comment.
What is the reasoning for the special case? Is there a scenario where the path is empty (and the name is not)?
From looking at the test I would assume that path is at least / + <group name> for a root level group.
2 participants
What
ListGroups method to list all child and grandchild groups too
Why
To list subgroups during creation of tag permissions
References
https://jira.greenbone.net/browse/ARTOSI-703
Checklist