fix: missing org membership creation for SCIM user update (#42603)

Author: a-lider

ee/api/scim/test/test_users_api.py

@@ -380,6 +380,48 @@ def test_put_user(self):
         assert scim_user.username == "[email protected]"
         assert scim_user.active is True
 
+    def test_put_reactivate_user(self):
+        user = User.objects.create_user(
+            email="[email protected]",
+            password=None,
+            first_name="Put",
+            last_name="Reactivate",
+            is_email_verified=True,
+        )
+        # Create then remove membership to simulate deactivated user
+        OrganizationMembership.objects.create(
+            user=user, organization=self.organization, level=OrganizationMembership.Level.MEMBER
+        )
+        SCIMProvisionedUser.objects.create(
+            user=user,
+            organization_domain=self.domain,
+            username="[email protected]",
+            identity_provider=SCIMProvisionedUser.IdentityProvider.OTHER,
+            active=True,
+        )
+        # Deactivate
+        OrganizationMembership.objects.filter(user=user, organization=self.organization).delete()
+        SCIMProvisionedUser.objects.filter(user=user, organization_domain=self.domain).update(active=False)
+        assert not OrganizationMembership.objects.filter(user=user, organization=self.organization).exists()
+
+        put_data = {
+            "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
+            "userName": "[email protected]",
+            "name": {"givenName": "Put", "familyName": "Reactivate"},
+            "emails": [{"value": "[email protected]", "primary": True}],
+            "active": True,
+        }
+
+        response = self.client.put(
+            f"/scim/v2/{self.domain.id}/Users/{user.id}", data=put_data, content_type="application/scim+json"
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["active"] is True
+        assert OrganizationMembership.objects.filter(user=user, organization=self.organization).exists()
+        scim_user = SCIMProvisionedUser.objects.get(user=user, organization_domain=self.domain)
+        assert scim_user.active is True
+
     def test_put_user_not_found(self):
         put_data = {
             "schemas": ["urn:ietf:params:scim:schemas:core:2.0:User"],
@@ -644,6 +686,41 @@ def test_patch_add_active_user_with_simple_path(self):
         assert response.status_code == status.HTTP_200_OK
         assert OrganizationMembership.objects.filter(user=user, organization=self.organization).exists()
 
+    def test_patch_replace_reactivate_user(self):
+        user = User.objects.create_user(
+            email="[email protected]", password=None, first_name="Test", is_email_verified=True
+        )
+        # Create then remove membership to simulate deactivated user
+        OrganizationMembership.objects.create(
+            user=user, organization=self.organization, level=OrganizationMembership.Level.MEMBER
+        )
+        SCIMProvisionedUser.objects.create(
+            user=user,
+            organization_domain=self.domain,
+            username="[email protected]",
+            identity_provider=SCIMProvisionedUser.IdentityProvider.OTHER,
+            active=True,
+        )
+        # Deactivate
+        OrganizationMembership.objects.filter(user=user, organization=self.organization).delete()
+        SCIMProvisionedUser.objects.filter(user=user, organization_domain=self.domain).update(active=False)
+        assert not OrganizationMembership.objects.filter(user=user, organization=self.organization).exists()
+
+        patch_data = {
+            "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+            "Operations": [{"op": "replace", "path": "active", "value": True}],
+        }
+
+        response = self.client.patch(
+            f"/scim/v2/{self.domain.id}/Users/{user.id}", data=patch_data, content_type="application/scim+json"
+        )
+
+        assert response.status_code == status.HTTP_200_OK
+        assert response.json()["active"] is True
+        assert OrganizationMembership.objects.filter(user=user, organization=self.organization).exists()
+        scim_user = SCIMProvisionedUser.objects.get(user=user, organization_domain=self.domain)
+        assert scim_user.active is True
+
     def test_patch_add_user_given_name_with_dotted_path(self):
         user = User.objects.create_user(
             email="[email protected]", password=None, first_name="", last_name="Existing", is_email_verified=True

ee/api/scim/user.py

@@ -64,6 +64,13 @@ def user_name(self) -> str:
         ).first()
         return scim_user.username if scim_user else self.obj.email
 
+    @property
+    def identity_provider(self) -> str:
+        scim_user = SCIMProvisionedUser.objects.filter(
+            user=self.obj, organization_domain=self._organization_domain
+        ).first()
+        return scim_user.identity_provider if scim_user else SCIMProvisionedUser.IdentityProvider.OTHER
+
     @property
     def active(self) -> bool:
         # A user is "active" in SCIM context if they have membership in this org
@@ -226,12 +233,18 @@ def put(self, data: dict) -> None:
                 defaults={
                     "username": user_name,
                     "active": is_active,
-                    "identity_provider": SCIMProvisionedUser.IdentityProvider.OTHER,
+                    "identity_provider": self.identity_provider,
                 },
             )
 
-            # Deactivate user if active is false
-            if not is_active:
+            if is_active:
+                # Adding org membership to reactivate the user
+                OrganizationMembership.objects.get_or_create(
+                    user=self.obj,
+                    organization=self._organization_domain.organization,
+                    defaults={"level": OrganizationMembership.Level.MEMBER},
+                )
+            else:
                 self.deactivate()
 
     def deactivate(self) -> None:
@@ -277,13 +290,19 @@ def handle_replace(self, path: AttrPath, value: Union[str, list, dict], operatio
                     self.deactivate()
                     return
                 else:
+                    OrganizationMembership.objects.get_or_create(
+                        user=self.obj,
+                        organization=self._organization_domain.organization,
+                        defaults={"level": OrganizationMembership.Level.MEMBER},
+                    )
+
                     SCIMProvisionedUser.objects.update_or_create(
                         user=self.obj,
                         organization_domain=self._organization_domain,
                         defaults={
                             "active": True,
-                            "username": self.obj.email,
-                            "identity_provider": SCIMProvisionedUser.IdentityProvider.OTHER,
+                            "username": self.user_name,
+                            "identity_provider": self.identity_provider,
                         },
                     )
 
@@ -315,7 +334,7 @@ def handle_replace(self, path: AttrPath, value: Union[str, list, dict], operatio
                     defaults={
                         "username": value,
                         "active": True,
-                        "identity_provider": SCIMProvisionedUser.IdentityProvider.OTHER,
+                        "identity_provider": self.identity_provider,
                     },
                 )
 
@@ -342,8 +361,8 @@ def handle_add(self, path: AttrPath, value: Union[str, list, dict], operation: d
                     organization_domain=self._organization_domain,
                     defaults={
                         "active": True,
-                        "username": self.obj.email,
-                        "identity_provider": SCIMProvisionedUser.IdentityProvider.OTHER,
+                        "username": self.user_name,
+                        "identity_provider": self.identity_provider,
                     },
                 )
 
@@ -377,7 +396,7 @@ def handle_add(self, path: AttrPath, value: Union[str, list, dict], operation: d
                     defaults={
                         "username": value,
                         "active": True,
-                        "identity_provider": SCIMProvisionedUser.IdentityProvider.OTHER,
+                        "identity_provider": self.identity_provider,
                     },
                 )