[MGNLSSO-45] Improve the parsing of the groups property Created: 08/Feb/21  Updated: 23/Feb/21  Resolved: 23/Feb/21

Status: Closed
Project: Single Sign On
Component/s: None
Affects Version/s: None
Fix Version/s: 1.1

Type: Task Priority: Neutral
Reporter: Maxime Michel Assignee: Maxime Michel
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: PNG File Screenshot 2021-02-11 at 12.28.02.png    
Template:
Acceptance criteria:
Empty
Task DoR:
Empty

 Description   

In version 1.0.x, the usergroups property lookup was hardcoded. This was a problem for a couple of reasons:

  • usergroups is, I believe, Microsoft terminology, that we inherited from the previous version of the module; OIDC tends to favor groups (see here, here or here)
  • the fact that we had to hardcode it meant the module had no flexibility
  • for an external user, the error message was unclear when setting things up

Initial solution

I suggested to lookup the groups property in the following PR: https://git.magnolia-cms.com/projects/ENTERPRISE/repos/magnolia-sso/pull-requests/22/overview?commentId=73403

My hope was that by doing this while adding the groups OIDC claim, everything would become clearer, and work. But it didn't actually work. At least in Keycloak, adding the groups claim to the request doesn't add any data if no mapper is configured.

Actual solution

As suggested here: https://www.pac4j.org/docs/clients.html#2-compute-roles-and-permissions

Before we map the OIDC user's groups into Magnolia groups, we have to convert the OIDC user's groups into something in his OIDC user profile. (OIDC user: lives in the IDP, OIDC profile: is shared between OIDC and Magnolia during the login process)

I have:

  • opted to use OIDC profile roles instead of groups, because there is standard API for this (solving the hardcoded value problem)
  • decided to implement an AuthorizationGenerator to streamline the code as well as give us the chance to warn the user with a relevant message at every step of the way (see screenshot)
  • decided to stay with looking up the groups property. This is a convention and we would rather have users follow it (which can easily be done at least in Okta and Keycloak), rather than allow too many use cases
  • we can always revisit this in future iterations

Generated at Mon Feb 12 10:50:35 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.