From 81f2190237a6255b817c40b1daf5d6fe09335d93 Mon Sep 17 00:00:00 2001 From: Nathan Levesque Date: Mon, 4 Nov 2024 15:35:15 -0500 Subject: [PATCH] Fixes to address scim-form-keycloak weirdness --- profiles/admin.py | 3 +++ profiles/forms.py | 3 +++ profiles/scim/adapters.py | 34 +++++++++++++++++++++-------- profiles/scim/views_test.py | 43 +++++++++++++++++++++++++++++++++++-- 4 files changed, 72 insertions(+), 11 deletions(-) diff --git a/profiles/admin.py b/profiles/admin.py index 38613ba64a..06211236a5 100644 --- a/profiles/admin.py +++ b/profiles/admin.py @@ -27,6 +27,9 @@ class ProfileAdmin(admin.ModelAdmin): "image_small_file", "image_medium_file", "updated_at", + "scim_id", + "scim_username", + "scim_external_id", ) diff --git a/profiles/forms.py b/profiles/forms.py index 601b62b35f..5fef0a7c1b 100644 --- a/profiles/forms.py +++ b/profiles/forms.py @@ -30,4 +30,7 @@ class Meta: "current_education", "time_commitment", "delivery", + "scim_id", + "scim_username", + "scim_external_id", ] diff --git a/profiles/scim/adapters.py b/profiles/scim/adapters.py index cda34dba1e..09adceecbb 100644 --- a/profiles/scim/adapters.py +++ b/profiles/scim/adapters.py @@ -1,9 +1,10 @@ +import json import logging from typing import Optional, Union from django.contrib.auth import get_user_model from django.db import transaction -from django_scim import constants, exceptions +from django_scim import constants from django_scim.adapters import SCIMUser from scim2_filter_parser.attr_paths import AttrPath @@ -45,6 +46,10 @@ class LearnSCIMUser(SCIMUser): ("name", "familyName", None): "last_name", } + IGNORED_PATHS = { + ("schemas", None, None), + } + @property def is_new_user(self): """_summary_ @@ -133,8 +138,8 @@ def from_dict(self, d): self.obj.profile = getattr(self.obj, "profile", Profile()) self.obj.profile.scim_username = d.get("userName") self.obj.profile.scim_external_id = d.get("externalId") - self.obj.profile.name = d.get("displayName", "") - self.obj.profile.emailOptIn = d.get("emailOptIn") == 1 + self.obj.profile.name = d.get("fullName", "") + self.obj.profile.email_optin = d.get("emailOptIn", 1) == 1 def save(self): """ @@ -176,6 +181,16 @@ def handle_add( self.obj.profile.scim_external_id = value self.obj.save() + def parse_path_and_values( + self, path: Optional[str], value: Union[str, list, dict] + ) -> list: + if not path and isinstance(value, str): + # scim-for-keycloak sends this as a noncompliant JSON-encoded string + value = json.loads(value) + value.pop("schema", None) # part of the spec, not a user prop + + return super().parse_path_and_values(path, value) + def handle_replace( self, path: Optional[AttrPath], @@ -197,14 +212,15 @@ def handle_replace( self.obj, self.ATTR_MAP.get(nested_path.first_path), nested_value ) - elif nested_path.first_path == ("name", "formatted", None): + elif nested_path.first_path == ("fullName", None, None): self.obj.profile.name = nested_value - + elif nested_path.first_path == ("emailOptIn", None, None): + self.obj.profile.email_optin = nested_value == 1 elif nested_path.first_path == ("emails", None, None): self.parse_emails(value) - - else: - msg = "Not Implemented" - raise exceptions.NotImplementedError(msg) + elif nested_path.first_path not in self.IGNORED_PATHS: + logger.debug( + "Ignoring SCIM update for path: %s", nested_path.first_path + ) self.save() diff --git a/profiles/scim/views_test.py b/profiles/scim/views_test.py index d1059b31bb..2135c8b483 100644 --- a/profiles/scim/views_test.py +++ b/profiles/scim/views_test.py @@ -26,7 +26,8 @@ def test_scim_post_user(staff_client): "familyName": "Doe", "givenName": "John", }, - "displayName": "John Smith Doe", + "fullName": "John Smith Doe", + "emailOptIn": 1, } ), ) @@ -41,6 +42,7 @@ def test_scim_post_user(staff_client): assert user.first_name == "John" assert user.last_name == "Doe" assert user.profile.name == "John Smith Doe" + assert user.profile.email_optin is True # test an update resp = staff_client.put( @@ -57,7 +59,8 @@ def test_scim_post_user(staff_client): "familyName": "Smith", "givenName": "Jimmy", }, - "displayName": "Jimmy Smith", + "fullName": "Jimmy Smith", + "emailOptIn": 0, } ), ) @@ -72,3 +75,39 @@ def test_scim_post_user(staff_client): assert user.first_name == "Jimmy" assert user.last_name == "Smith" assert user.profile.name == "Jimmy Smith" + assert user.profile.email_optin is False + + resp = staff_client.patch( + f"{reverse('scim:users')}/{user.profile.scim_id}", + content_type="application/scim+json", + data=json.dumps( + { + "schemas": [constants.SchemaURI.PATCH_OP], + "Operations": [ + { + "op": "replace", + # yes, the value we get from scim-for-keycloak is a JSON encoded string...inside JSON... + "value": json.dumps( + { + "schemas": [constants.SchemaURI.USER], + "emailOptIn": 1, + "fullName": "Billy Bob", + } + ), + } + ], + } + ), + ) + + assert resp.status_code == 200, f"Error response: {resp.content}" + + user = user_q.first() + + assert user is not None + assert user.email == "jsmith@example.com" + assert user.username == "jsmith" + assert user.first_name == "Jimmy" + assert user.last_name == "Smith" + assert user.profile.name == "Billy Bob" + assert user.profile.email_optin is True