[IDP-1182] Add endpoint for updating groups_external
#355
Conversation
|
| "app_prefix": "myapp", | ||
| "groups": ["myapp-users", "myapp-managers"] |
There was a problem hiding this comment.
Why is the prefix included as a separate property as well as included in the group names themselves?
There was a problem hiding this comment.
The new sync process (yet to be written) will have a single, set prefix (per instance), so that a given copy of the new sync process can only affect external groups with that prefix.
The current need is for syncing from a Google Sheet controlled by some of the REAP admins, to enable them to add "reap-____" groups to users signing into REAP (because the new platform, Preservica, requires the use of SAML-provided groups for controlling authorization). This will let them add specific groups to specific users in specific IDPs, to control what those users are allowed to do in REAP. The app_prefix will be controlled by us, who set up and run the instance of the new sync process. The groups are controlled by the REAP admins (via the Google Sheet), so this is just another way to ensure the data they provide meets the constraints we have established for this new part of our system.
I hope that made sense. Feel free to ask more questions and/or review the parent YouTrack issue (IDP-1153) for more details.
| foreach ($appExternalGroups as $appExternalGroup) { | ||
| if (! str_starts_with($appExternalGroup, $appPrefix . '-')) { | ||
| $this->addErrors([ | ||
| 'groups_external' => sprintf( | ||
| 'The given group %s does not start with the given prefix (%s)', | ||
| json_encode($appExternalGroup), | ||
| json_encode($appPrefix) | ||
| ), | ||
| ]); | ||
| return false; | ||
| } | ||
| } |
There was a problem hiding this comment.
If the incoming group names omitted the prefix or the separate prefix property was omitted, this validation wouldn't be necessary. Doing so would remove one possible failure scenario.
There was a problem hiding this comment.
True. I considered that option, but I'd like the group names (as typed into the Google Sheet) to exactly match the group names that REAP will receive via SAML, to help make it more intuitive.
If I had them simply type in everything after the prefix (on the Google Sheet), then they would type in something like "europe" and have to know/remember that it will come to REAP via SAML as "reap-europe". It seemed better to keep that prefix as visible as possible, so that they don't set up rules for a "europe" group and then be confused when it doesn't work.
I'll be writing a new custom sync process (unless one we already have will suffice) for syncing from a Google Sheet to this new field, via this new API endpoint. |
IDP-1182 Provide a way to set
user.groups_externalAdded
Feature PR Checklist
make composershowmake psr2