security(releases): Fix bug where any owner can be assigned to a release. (#29993)

This fixes a potential security issue with release creation. It turns out that you can create a
release with any user in the system. The main vector for abuse I see here is:

 - Create a release with an arbitrary user using their username. Could be just scraped from github,
   or other external sources.
 - Hit the release details endpoint for that release. You now have information about that user's
   account - associated emails, name, a couple of related dates, etc.

src/sentry/api/endpoints/organization_releases.py

@@ -434,7 +434,9 @@ class OrganizationReleasesEndpoint(
         :auth: required
         """
         bind_organization_context(organization)
-        serializer = ReleaseSerializerWithProjects(data=request.data)
+        serializer = ReleaseSerializerWithProjects(
+            data=request.data, context={"organization": organization}
+        )
 
         with configure_scope() as scope:
             if serializer.is_valid():

src/sentry/api/endpoints/project_releases.py

@@ -100,7 +100,9 @@ class ProjectReleasesEndpoint(ProjectEndpoint, EnvironmentMixin):
         :auth: required
         """
         bind_organization_context(project.organization)
-        serializer = ReleaseWithVersionSerializer(data=request.data)
+        serializer = ReleaseWithVersionSerializer(
+            data=request.data, context={"organization": project.organization}
+        )
 
         with configure_scope() as scope:
             if serializer.is_valid():

src/sentry/api/serializers/rest_framework/release.py

@@ -3,7 +3,7 @@ from rest_framework import serializers
 from sentry.api.fields.user import UserField
 from sentry.api.serializers.rest_framework import CommitSerializer, ListField
 from sentry.constants import COMMIT_RANGE_DELIMITER, MAX_COMMIT_LENGTH, MAX_VERSION_LENGTH
-from sentry.models import Release, ReleaseStatus
+from sentry.models import OrganizationMember, Release, ReleaseStatus
 
 
 class ReleaseHeadCommitSerializerDeprecated(serializers.Serializer):
@@ -76,3 +76,10 @@ class ReleaseWithVersionSerializer(ReleaseSerializer):
         if not Release.is_valid_version(value):
             raise serializers.ValidationError("Release with name %s is not allowed" % value)
         return value
+
+    def validate_owner(self, owner):
+        if not OrganizationMember.objects.filter(
+            organization=self.context["organization"], user=owner
+        ).exists():
+            raise serializers.ValidationError("User does not have access to this organization")
+        return owner

tests/sentry/api/endpoints/test_organization_releases.py

@@ -1274,6 +1274,7 @@ class OrganizationReleaseCreateTest(APITestCase):
         project = self.create_project(name="foo", organization=org, teams=[team])
 
         self.create_member(teams=[team], user=user, organization=org)
+        self.create_member(teams=[team], user=self.user, organization=org)
         self.login_as(user=user)
 
         url = reverse("sentry-api-0-organization-releases", kwargs={"organization_slug": org.slug})
@@ -2046,7 +2047,8 @@ class ReleaseSerializerWithProjectsTest(TestCase):
                 "headCommits": self.headCommits,
                 "refs": self.refs,
                 "projects": self.projects,
-            }
+            },
+            context={"organization": self.organization},
         )
 
         assert serializer.is_valid(), serializer.errors
@@ -2077,7 +2079,8 @@ class ReleaseSerializerWithProjectsTest(TestCase):
 
     def test_fields_not_required(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": self.version, "projects": self.projects}
+            data={"version": self.version, "projects": self.projects},
+            context={"organization": self.organization},
         )
         assert serializer.is_valid()
         result = serializer.validated_data
@@ -2086,19 +2089,22 @@ class ReleaseSerializerWithProjectsTest(TestCase):
 
     def test_do_not_allow_null_commits(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": self.version, "projects": self.projects, "commits": None}
+            data={"version": self.version, "projects": self.projects, "commits": None},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
     def test_do_not_allow_null_head_commits(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": self.version, "projects": self.projects, "headCommits": None}
+            data={"version": self.version, "projects": self.projects, "headCommits": None},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
     def test_do_not_allow_null_refs(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": self.version, "projects": self.projects, "refs": None}
+            data={"version": self.version, "projects": self.projects, "refs": None},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
@@ -2108,7 +2114,8 @@ class ReleaseSerializerWithProjectsTest(TestCase):
                 "version": self.version,
                 "projects": self.projects,
                 "ref": "a" * MAX_VERSION_LENGTH,
-            }
+            },
+            context={"organization": self.organization},
         )
         assert serializer.is_valid()
         serializer = ReleaseSerializerWithProjects(
@@ -2116,7 +2123,8 @@ class ReleaseSerializerWithProjectsTest(TestCase):
                 "version": self.version,
                 "projects": self.projects,
                 "ref": "a" * (MAX_VERSION_LENGTH + 1),
-            }
+            },
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
@@ -2126,14 +2134,16 @@ class ReleaseSerializerWithProjectsTest(TestCase):
         )
         assert serializer.is_valid()
         serializer = ReleaseSerializerWithProjects(
-            data={"version": "a" * (MAX_VERSION_LENGTH + 1), "projects": self.projects}
+            data={"version": "a" * (MAX_VERSION_LENGTH + 1), "projects": self.projects},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
     def test_version_does_not_allow_whitespace(self):
         for char in BAD_RELEASE_CHARS:
             serializer = ReleaseSerializerWithProjects(
-                data={"version": char, "projects": self.projects}
+                data={"version": char, "projects": self.projects},
+                context={"organization": self.organization},
             )
             assert not serializer.is_valid()
 
@@ -2141,21 +2151,27 @@ class ReleaseSerializerWithProjectsTest(TestCase):
         serializer = ReleaseSerializerWithProjects(data={"version": ".", "projects": self.projects})
         assert not serializer.is_valid()
         serializer = ReleaseSerializerWithProjects(
-            data={"version": "..", "projects": self.projects}
+            data={"version": "..", "projects": self.projects},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
     def test_version_does_not_allow_null_or_empty_value(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": None, "projects": self.projects}
+            data={"version": None, "projects": self.projects},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
-        serializer = ReleaseSerializerWithProjects(data={"version": "", "projects": self.projects})
+        serializer = ReleaseSerializerWithProjects(
+            data={"version": "", "projects": self.projects},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
 
     def test_version_cannot_be_latest(self):
         serializer = ReleaseSerializerWithProjects(
-            data={"version": "Latest", "projects": self.projects}
+            data={"version": "Latest", "projects": self.projects},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 

tests/sentry/api/endpoints/test_project_releases.py

@@ -4,6 +4,7 @@ import pytz
 from django.urls import reverse
 from django.utils import timezone
 from exam import fixture
+from rest_framework.exceptions import ErrorDetail
 
 from sentry.api.endpoints.project_releases import ReleaseWithVersionSerializer
 from sentry.constants import BAD_RELEASE_CHARS, MAX_VERSION_LENGTH
@@ -624,7 +625,8 @@ class ReleaseSerializerTest(TestCase):
                 "url": self.url,
                 "dateReleased": self.dateReleased,
                 "commits": self.commits,
-            }
+            },
+            context={"organization": self.organization},
         )
 
         assert serializer.is_valid()
@@ -641,46 +643,95 @@ class ReleaseSerializerTest(TestCase):
         assert result["commits"] == self.commits
 
     def test_fields_not_required(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": self.version})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": self.version},
+            context={"organization": self.organization},
+        )
         assert serializer.is_valid()
 
     def test_do_not_allow_null_commits(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": self.version, "commits": None})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": self.version, "commits": None},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
 
     def test_ref_limited_by_max_version_length(self):
         serializer = ReleaseWithVersionSerializer(
-            data={"version": self.version, "ref": "a" * MAX_VERSION_LENGTH}
+            data={"version": self.version, "ref": "a" * MAX_VERSION_LENGTH},
+            context={"organization": self.organization},
         )
         assert serializer.is_valid()
         serializer = ReleaseWithVersionSerializer(
-            data={"version": self.version, "ref": "a" * (MAX_VERSION_LENGTH + 1)}
+            data={"version": self.version, "ref": "a" * (MAX_VERSION_LENGTH + 1)},
+            context={"organization": self.organization},
         )
         assert not serializer.is_valid()
 
     def test_version_limited_by_max_version_length(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": "a" * MAX_VERSION_LENGTH})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": "a" * MAX_VERSION_LENGTH},
+            context={"organization": self.organization},
+        )
         assert serializer.is_valid()
-        serializer = ReleaseWithVersionSerializer(data={"version": "a" * (MAX_VERSION_LENGTH + 1)})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": "a" * (MAX_VERSION_LENGTH + 1)},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
 
     def test_version_does_not_allow_whitespace(self):
         for char in BAD_RELEASE_CHARS:
-            serializer = ReleaseWithVersionSerializer(data={"version": char})
+            serializer = ReleaseWithVersionSerializer(
+                data={"version": char},
+                context={"organization": self.organization},
+            )
             assert not serializer.is_valid()
 
     def test_version_does_not_allow_current_dir_path(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": "."})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": "."},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
-        serializer = ReleaseWithVersionSerializer(data={"version": ".."})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": ".."},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
 
     def test_version_does_not_allow_null_or_empty_value(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": None})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": None},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
-        serializer = ReleaseWithVersionSerializer(data={"version": ""})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": ""},
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
 
     def test_version_cannot_be_latest(self):
-        serializer = ReleaseWithVersionSerializer(data={"version": "Latest"})
+        serializer = ReleaseWithVersionSerializer(
+            data={"version": "Latest"},
+            context={"organization": self.organization},
+        )
+        assert not serializer.is_valid()
+
+    def test_owner_must_have_org_access(self):
+        serializer = ReleaseWithVersionSerializer(
+            data={
+                "version": self.version,
+                "owner": self.create_user().username,
+                "ref": self.ref,
+                "url": self.url,
+                "dateReleased": self.dateReleased,
+                "commits": self.commits,
+            },
+            context={"organization": self.organization},
+        )
         assert not serializer.is_valid()
+        assert serializer.errors == {
+            "owner": [ErrorDetail("User does not have access to this organization", "invalid")]
+        }