Skip to content

fix(issue-platform): Check permissions for assignees sent to the issue platform #69065

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 5 additions & 29 deletions src/sentry/api/fields/actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,7 @@
from drf_spectacular.utils import extend_schema_field
from rest_framework import serializers

from sentry.models.organizationmember import OrganizationMember
from sentry.models.team import Team
from sentry.models.user import User
from sentry.utils.actor import ActorTuple
from sentry.utils.actor import parse_and_validate_actor


@extend_schema_field(field=OpenApiTypes.STR)
Expand All @@ -20,29 +17,8 @@ def to_representation(self, value):
return value.identifier

def to_internal_value(self, data):
if not data:
return None
actor_tuple = parse_and_validate_actor(data, self.context["organization"].id)

try:
actor = ActorTuple.from_actor_identifier(data)
except Exception:
raise serializers.ValidationError(
"Could not parse actor. Format should be `type:id` where type is `team` or `user`."
)
try:
obj = actor.resolve()
except (Team.DoesNotExist, User.DoesNotExist):
raise serializers.ValidationError(f"{actor.type.__name__} does not exist")

if actor.type == Team:
if obj.organization != self.context["organization"]:
raise serializers.ValidationError("Team is not a member of this organization")
elif actor.type == User:
if not OrganizationMember.objects.filter(
organization=self.context["organization"], user_id=obj.id
).exists():
raise serializers.ValidationError("User is not a member of this organization")

if self.as_actor:
return actor.resolve_to_actor()
return actor
if self.as_actor and actor_tuple:
return actor_tuple.resolve_to_actor()
return actor_tuple
14 changes: 13 additions & 1 deletion src/sentry/issues/occurrence_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from sentry.models.organization import Organization
from sentry.models.project import Project
from sentry.utils import metrics
from sentry.utils.actor import parse_and_validate_actor

logger = logging.getLogger(__name__)

Expand Down Expand Up @@ -142,6 +143,17 @@ def _get_kwargs(payload: Mapping[str, Any]) -> Mapping[str, Any]:
try:
with metrics.timer("occurrence_ingest.duration", instance="_get_kwargs"):
metrics.distribution("occurrence.ingest.size.data", len(payload), unit="byte")
assignee_identifier = None
payload_assignee = payload.get("assignee")
if payload_assignee:
project = Project.objects.get_from_cache(id=payload["project_id"])
try:
assignee = parse_and_validate_actor(payload_assignee, project.organization_id)
if assignee:
assignee_identifier = assignee.identifier
except Exception:
logger.exception("Failed to validate assignee for occurrence")

occurrence_data = {
"id": UUID(payload["id"]).hex,
"project_id": payload["project_id"],
Expand All @@ -154,7 +166,7 @@ def _get_kwargs(payload: Mapping[str, Any]) -> Mapping[str, Any]:
"type": payload["type"],
"detection_time": payload["detection_time"],
"level": payload.get("level", DEFAULT_LEVEL),
"assignee": payload.get("assignee"),
"assignee": assignee_identifier,
}

process_occurrence_data(occurrence_data)
Expand Down
36 changes: 35 additions & 1 deletion src/sentry/utils/actor.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,12 @@

from rest_framework import serializers

from sentry.services.hybrid_cloud.user import RpcUser

if TYPE_CHECKING:
from sentry.models.actor import Actor
from sentry.models.team import Team
from sentry.models.user import User
from sentry.services.hybrid_cloud.user import RpcUser


class ActorTuple(namedtuple("Actor", "id type")):
Expand Down Expand Up @@ -164,3 +165,36 @@ def fetch_actor_by_id(cls: type[User] | type[Team], id: int) -> Team | RpcUser:
return user
else:
raise ValueError(f"Cls {cls} is not a valid actor type.")


def parse_and_validate_actor(
actor_identifier: str | None, organization_id: int
) -> ActorTuple | None:
from sentry.models.organizationmember import OrganizationMember
from sentry.models.team import Team
from sentry.models.user import User

if not actor_identifier:
return None

try:
actor = ActorTuple.from_actor_identifier(actor_identifier)
except Exception:
raise serializers.ValidationError(
"Could not parse actor. Format should be `type:id` where type is `team` or `user`."
)
try:
obj = actor.resolve()
except (Team.DoesNotExist, User.DoesNotExist):
raise serializers.ValidationError(f"{actor.type.__name__} does not exist")

if isinstance(obj, Team):
if obj.organization_id != organization_id:
raise serializers.ValidationError("Team is not a member of this organization")
elif isinstance(obj, RpcUser):
if not OrganizationMember.objects.filter(
organization_id=organization_id, user_id=obj.id
).exists():
raise serializers.ValidationError("User is not a member of this organization")

return actor
25 changes: 22 additions & 3 deletions tests/sentry/issues/test_occurrence_consumer.py
Original file line number Diff line number Diff line change
Expand Up @@ -387,8 +387,8 @@ def test_missing_event_id_in_event_data(self) -> None:

def test_project_ids_mismatch(self) -> None:
message = deepcopy(get_test_message(self.project.id))
message["project_id"] = 1
message["event"]["project_id"] = 2
message["project_id"] = self.project.id
message["event"]["project_id"] = 999999999999
with pytest.raises(InvalidEventPayloadError):
_get_kwargs(message)

Expand Down Expand Up @@ -465,6 +465,25 @@ def test_assignee(self) -> None:
kwargs = _get_kwargs(message)
assert kwargs["occurrence_data"]["assignee"] == f"user:{self.user.id}"

def test_assignee_perms(self) -> None:
message = deepcopy(get_test_message(self.project.id))
random_user = self.create_user()
message["assignee"] = f"user:{random_user.id}"
kwargs = _get_kwargs(message)
assert kwargs["occurrence_data"]["assignee"] is None

message = deepcopy(get_test_message(self.project.id))
message["assignee"] = "user:99999999999"
kwargs = _get_kwargs(message)
assert kwargs["occurrence_data"]["assignee"] is None

other_org = self.create_organization()
random_team = self.create_team(other_org)
message = deepcopy(get_test_message(self.project.id))
message["assignee"] = f"team:{random_team.id}"
kwargs = _get_kwargs(message)
assert kwargs["occurrence_data"]["assignee"] is None

def test_assignee_none(self) -> None:
kwargs = _get_kwargs(deepcopy(get_test_message(self.project.id)))
assert kwargs["occurrence_data"]["assignee"] is None
Expand All @@ -475,4 +494,4 @@ def test_assignee_none(self) -> None:
message = deepcopy(get_test_message(self.project.id))
message["assignee"] = ""
kwargs = _get_kwargs(message)
assert kwargs["occurrence_data"]["assignee"] == ""
assert kwargs["occurrence_data"]["assignee"] is None
Loading