Browse Source

fix(scim): support azure team PATCH remove format (#27955)

Josh Ferge 3 years ago
parent
commit
9bd0431f41

+ 21 - 14
src/sentry/scim/endpoints/teams.py

@@ -134,17 +134,8 @@ class OrganizationSCIMTeamDetails(SCIMEndpoint, TeamDetailsEndpoint):
                     data=omt.get_audit_log_data(),
                 )
 
-    def _remove_members_operation(self, request, operation, team):
-        try:
-            # grab the filter out of the brackets of the string that looks
-            # like so: members[userName eq "baz@sentry.io"]
-            parsed_filter = parse_filter_conditions(
-                re.search(r"\[(.*?)\]", operation["path"]).groups()[0]
-            )
-        except Exception:
-            # TODO: log parse error
-            raise ParseError(detail=SCIM_400_INVALID_FILTER)
-        member = OrganizationMember.objects.get(organization=team.organization, id=parsed_filter)
+    def _remove_members_operation(self, request, member_id, team):
+        member = OrganizationMember.objects.get(organization=team.organization, id=member_id)
         with transaction.atomic():
             try:
                 omt = OrganizationMemberTeam.objects.get(team=team, organizationmember=member)
@@ -193,9 +184,9 @@ class OrganizationSCIMTeamDetails(SCIMEndpoint, TeamDetailsEndpoint):
                     if op == TeamPatchOps.ADD and operation["path"] == "members":
                         self._add_members_operation(request, operation, team)
                     elif op == TeamPatchOps.REMOVE and "members" in operation["path"]:
-                        # the members op contains a filter string like so:
-                        # members[userName eq "baz@sentry.io"]
-                        self._remove_members_operation(request, operation, team)
+                        self._remove_members_operation(
+                            request, self._get_member_id_for_remove_op(operation), team
+                        )
                     elif op == TeamPatchOps.REPLACE:
                         path = operation.get("path")
 
@@ -233,3 +224,19 @@ class OrganizationSCIMTeamDetails(SCIMEndpoint, TeamDetailsEndpoint):
         # override parent's put since we don't have puts
         # in SCIM Team routes
         return self.http_method_not_allowed(request)
+
+    def _get_member_id_for_remove_op(self, operation):
+        if "value" in operation:
+            # azure sends member ids in this format under the key 'value'
+            return operation["value"][0]["value"]
+
+        try:
+            # grab the filter out of the brackets of the string that looks
+            # like so: members[value eq "123124"]
+            regex_search = re.search(r"\[(.*?)\]", operation["path"])
+            if regex_search is None:
+                raise SCIMFilterError
+            filter_path = regex_search.groups()[0]
+            return parse_filter_conditions(filter_path)
+        except SCIMFilterError:
+            raise ParseError(detail=SCIM_400_INVALID_FILTER)

+ 23 - 0
tests/sentry/api/endpoints/test_scim_team_details.py

@@ -289,3 +289,26 @@ class SCIMTeamDetailsTests(SCIMTestCase):
         assert response.status_code == 204, response.content
 
         assert Team.objects.get(id=team.id).status == TeamStatus.PENDING_DELETION
+
+    def test_remove_member_azure(self):
+        member1 = self.create_member(
+            user=self.create_user(), organization=self.organization, teams=[self.team]
+        )
+
+        url = reverse(
+            "sentry-api-0-organization-scim-team-details",
+            args=[self.organization.slug, self.team.id],
+        )
+        response = self.client.patch(
+            url,
+            {
+                "schemas": ["urn:ietf:params:scim:api:messages:2.0:PatchOp"],
+                "Operations": [
+                    {"op": "Remove", "path": "members", "value": [{"value": str(member1.id)}]}
+                ],
+            },
+        )
+        assert response.status_code == 204, response.content
+        assert not OrganizationMemberTeam.objects.filter(
+            team_id=self.team.id, organizationmember_id=member1.id
+        ).exists()