Skip to content

Conversation

@rhysyngsun
Copy link
Contributor

What are the relevant tickets?

Part of https://github.com/mitodl/hq/issues/5561

Description (What does it do?)

This makes some tweaks to the SCIM adaptor to handle some weird data we're getting from the scim-for-keycloak plugin I didn't previously notice:

  • The plugin is embedding JSON strings inside the JSON requests, which we have to parse because otherwise the data is being ignored.
  • The was one edge case where we weren't handling emailOptIn.
  • I had to switch over using displayName to using fullName. displayName worked on full data syncs, but if you only updated your full name on the account page it wasn't triggering a SCIM PATCH request over to Learn. There must be some special handling of it in the plugin, despite the appearance in the admin UI that you could use displayName for a custom user property.

How can this be tested?

  • You'll need to update your SCIM Remote Provider User schema configuration by removing the custom attribute setting for displayName and adding a new attribute named fullName that uses the fullName custom attribute. All other settings can be left as the default.

You should be able to go through the following flows:

  • Register as a new user:
    • Go to /realms/olapps/account/ on Keycloak, this redirects to the login pages and avoids us exercising the social auth code on Learn that can obfuscate whether SCIM worked correctly or not.
    • Create a new user, confirm your email. You should now be on the profile page.
    • Go to /admin on the Learn API and verify the user has been created and has the correct name and the email_optin setting is set to true/yes.
  • Update an existing user:
    • Go to /realms/olapps/account/ on Keycloak and login as an existing user
    • Update combinations of the fields and confirm you see these updates propagate to Learn (e.g. test w/ updating all fields, test updating them individually, etc).

@rhysyngsun rhysyngsun added the Needs Review An open Pull Request that is ready for review label Nov 4, 2024
@rhysyngsun rhysyngsun changed the title Fixes to address scim-form-keycloak weirdness Fixes to address scim-for-keycloak weirdness Nov 4, 2024
@rhysyngsun rhysyngsun force-pushed the nl/scim-keycloak-fixes-round-2 branch from f434f82 to 81f2190 Compare November 4, 2024 21:06
@cp-at-mit cp-at-mit self-assigned this Nov 5, 2024
@cp-at-mit cp-at-mit added Waiting on author and removed Needs Review An open Pull Request that is ready for review labels Nov 5, 2024
@rhysyngsun rhysyngsun merged commit cffd5d3 into main Nov 5, 2024
11 checks passed
@rhysyngsun rhysyngsun deleted the nl/scim-keycloak-fixes-round-2 branch November 5, 2024 14:21
This was referenced Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants