Allow data containing groups from SSO server to be a List of Maps in addition to a List of Strings. #198
Conversation
addition to a List of Strings. In cases where groups is a List of Maps, give option to specify which key in the Map holds the group name, defaulting to a key "name"
|
[michael-doubez] I missed your comment on my older PR. I reconciled my change with the latest changes this week so that I could continue using my forked version. If you still think this is not the right approach, I understand. I am willing to retry using a different method if you prefer. Thanks. |
Codecov Report
@@ Coverage Diff @@
## master #198 +/- ##
============================================
- Coverage 73.27% 72.61% -0.67%
- Complexity 182 186 +4
============================================
Files 9 9
Lines 696 723 +27
Branches 110 120 +10
============================================
+ Hits 510 525 +15
- Misses 132 140 +8
- Partials 54 58 +4
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
@bsmoyers Thank you for your the PR. I had initially planned to use JMESPath (see #149) but at his time, I have not found a suitable java library for that so I guess we can at least stay somewhat compatible with it. Instead of introducing yet-another-parameter, I'd rather implement it in the JMESPath way: I expect, your current additional parameter can be generated by splitting the current group name with this specific syntax. If you think I am asking for too much, I am willing to contribute on your PR. |
|
@michael-doubez Thanks for the update. That sounds fine with me, and it will make it much simpler to not add the additional field. Can you confirm that you want the existing group field to handle a top level groups field "groups" with a nested field "name" would be input in the text box as "groups[].name" |
|
Yes. That would be be fine. If people need it we 'll be able to improve on that and not break the format. |
… nested group field name for case where SSO server returns groups as an array of maps.
| @@ -136,6 +137,7 @@ public static enum TokenAuthMethod { client_secret_basic, client_secret_post }; | |||
| private String fullNameFieldName = null; | |||
| private String emailFieldName = null; | |||
| private String groupsFieldName = null; | |||
| private String nestedGroupFieldName = "name"; | |||
There was a problem hiding this comment.
I would put the parsing of groupsFieldName in the setter (setGroupsFieldName) and use transient variables to be used latter on.
There was a problem hiding this comment.
updated in latest changes
| // if groupsFieldName contains []., then groupsFieldName | ||
| // is first portion, and nestedGroupFieldName is | ||
| // second portion | ||
| String[] parts = groupsFieldName.split("\\[\\]\\."); |
There was a problem hiding this comment.
use limit to limit splitting to 2 String[] split(String regex, int limit) - granted there should be only one and warning should be generated if format is not correct.
Note: a method doCheckGroupsFieldName should be added to check field format for GUI users.
If you don't do it, I'll make it is another PR.
There was a problem hiding this comment.
updated in latest changes
There was a problem hiding this comment.
added doCheckGroupsFieldName() to check format
| // second portion | ||
| String[] parts = groupsFieldName.split("\\[\\]\\."); | ||
| // don't modify groupsFieldName | ||
| String localGroupsFieldName = parts.length > 1 ? parts[0] : groupsFieldName; |
There was a problem hiding this comment.
If there is no split, parts[0] will be the content of groupsFieldName, conditional is not required
There was a problem hiding this comment.
updated in latest changes
| String[] parts = groupsFieldName.split("\\[\\]\\."); | ||
| // don't modify groupsFieldName | ||
| String localGroupsFieldName = parts.length > 1 ? parts[0] : groupsFieldName; | ||
| nestedGroupFieldName = parts.length > 1 ? parts[1] : nestedGroupFieldName; |
There was a problem hiding this comment.
nestedGroupField shoud be null if not defined
There was a problem hiding this comment.
updated in latest changes
| } else if (group instanceof ArrayMap) { | ||
| // if its a Map, we use the nestedGroupFieldName to grab the groups | ||
| Map<String, String> groupMap = (Map<String, String>) group; | ||
| if (groupMap.keySet().contains(nestedGroupFieldName)) { |
There was a problem hiding this comment.
Test nestedGroupFieldName not null (with new logic)
There was a problem hiding this comment.
updated in latest changes
|
working on updating, may be a little while before I can get to it. |
|
Thanks for the feature. I have scheduled a release in the coming week (or the week after). |
|
thanks! |
Allow data containing groups from SSO server to be a List of Maps in addition to a List of Strings. In cases where groups is a List of Maps, give option to specify which key in the Map holds the group name, defaulting to a key "name".
Oracle IDCS is the product we needed to integrate with in this case. I don't currently know if any other providers need this change.