diff --git a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py index 67770f09cfdc..a255031efc69 100644 --- a/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py +++ b/src/sentry/integrations/api/endpoints/organization_code_mapping_details.py @@ -12,6 +12,7 @@ OrganizationIntegrationsLoosePermission, ) from sentry.api.serializers import serialize +from sentry.api.serializers.rest_framework.base import camel_to_snake_case, convert_dict_key_case from sentry.integrations.models.repository_project_path_config import RepositoryProjectPathConfig from sentry.integrations.services.integration import integration_service @@ -44,17 +45,18 @@ def convert_args(self, request: Request, organization_id_or_slug, config_id, *ar id=config_id, organization_integration_id__in=[oi.id for oi in ois], ) - except RepositoryProjectPathConfig.DoesNotExist: + # Only set when the request wants to move the mapping to another project. + # Normalize keys the way the serializer does first, otherwise a snake_case + # `project_id` skips the access check below but still moves the mapping. + data = convert_dict_key_case(request.data, camel_to_snake_case) + if data.get("project_id"): + kwargs["new_project"] = self.get_project(kwargs["organization"], data["project_id"]) + except (RepositoryProjectPathConfig.DoesNotExist, ValueError): raise Http404 - if request.data.get("projectId"): - kwargs["new_project"] = super().get_project( - kwargs["organization"], request.data.get("projectId") - ) - return (args, kwargs) - def put(self, request: Request, config_id, organization, config, new_project) -> Response: + def put(self, request: Request, config_id, organization, config, new_project=None) -> Response: """ Update a repository project path config `````````````````` @@ -68,8 +70,10 @@ def put(self, request: Request, config_id, organization, config, new_project) -> :param string default_branch: :auth: required """ - project = config.project_repository.project - if not request.access.has_projects_access([project, new_project]): + projects = [config.project_repository.project] + if new_project is not None: + projects.append(new_project) + if not request.access.has_projects_access(projects): return self.respond(status=status.HTTP_403_FORBIDDEN) try: diff --git a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py index ed8dc00b2165..921b206fea4e 100644 --- a/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py +++ b/tests/sentry/integrations/api/endpoints/test_organization_code_mapping_details.py @@ -71,6 +71,9 @@ def test_non_project_member_permissions(self) -> None: non_member_om = self.create_member(organization=self.org, user=non_member) self.login_as(user=non_member) + response = self.client.put(self.url, {"sourceRoot": ""}) + assert response.status_code == status.HTTP_403_FORBIDDEN + response = self.make_put({"sourceRoot": "newRoot"}) assert response.status_code == status.HTTP_403_FORBIDDEN @@ -102,6 +105,40 @@ def test_basic_edit(self) -> None: assert resp.data["id"] == str(self.config.id) assert resp.data["sourceRoot"] == "newRoot" + def test_edit_rejects_missing_required_fields(self) -> None: + resp = self.client.put(self.url, {"sourceRoot": ""}) + + assert resp.status_code == status.HTTP_400_BAD_REQUEST + assert resp.data == { + "projectId": ["This field is required."], + "repositoryId": ["This field is required."], + "stackRoot": ["This field is required."], + } + + def test_edit_snake_case_project_id_is_access_checked(self) -> None: + member = self.create_user() + self.create_member( + organization=self.org, user=member, has_global_access=False, teams=[self.team] + ) + self.login_as(user=member) + + # every key is snake_case so `projectId` never appears in the body, but the + # serializer still reads it as project_id and would move the mapping + resp = self.client.put( + self.url, + { + "repository_id": self.repo.id, + "project_id": self.project2.id, + "stack_root": "/stack/root", + "source_root": "/source/root", + "default_branch": "master", + }, + ) + + assert resp.status_code == status.HTTP_403_FORBIDDEN + self.config.refresh_from_db() + assert self.config.project_repository.project_id == self.project.id + def test_basic_edit_from_member_permissions(self) -> None: self.login_as(user=self.user2) resp = self.make_put({"sourceRoot": "newRoot"})