Skip to content
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
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
from django.utils.functional import empty
from rest_framework import serializers
from rest_framework.request import Request
from rest_framework.response import Response
Expand All @@ -12,22 +11,7 @@
PlatformExternalIssueSerializer,
)
from sentry.sentry_apps.services.cell import sentry_app_cell_service
from sentry.users.models.user import User
from sentry.users.services.user.serial import serialize_rpc_user


def _extract_lazy_object(lo):
"""
Unwrap a LazyObject and return the inner object. Whatever that may be.

ProTip: This is relying on `django.utils.functional.empty`, which may
or may not be removed in the future. It's 100% undocumented.
"""
if not hasattr(lo, "_wrapped"):
return lo
if lo._wrapped is empty:
lo._setup()
return lo._wrapped
from sentry.users.services.user.serial import serialize_generic_user


class SentryAppInstallationExternalIssueActionsSerializer(serializers.Serializer):
Expand Down Expand Up @@ -57,9 +41,9 @@ def post(self, request: Request, installation) -> Response:
action = data.pop("action")
uri = data.pop("uri")

user = _extract_lazy_object(request.user)
if isinstance(user, User):
user = serialize_rpc_user(user)
rpc_user = serialize_generic_user(request.user)
if rpc_user is None:
Comment thread
geoffg-sentry marked this conversation as resolved.
return Response({"detail": "Authentication credentials were not provided."}, status=401)

result = sentry_app_cell_service.create_issue_link(
organization_id=installation.organization_id,
Expand All @@ -68,7 +52,7 @@ def post(self, request: Request, installation) -> Response:
action=action,
fields=data,
uri=uri,
user=user,
user=rpc_user,
)

if result.error:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ def delete(self, request: Request, installation, external_issue_id) -> Response:
status_code=400,
)

if not request.user.is_authenticated:
return Response({"detail": "Authentication credentials were not provided."}, status=401)

# Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2).
result = sentry_app_cell_service.delete_external_issue(
organization_id=installation.organization_id,
installation=installation,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,18 @@
except Exception:
return Response({"detail": "issueId is required, and must be an integer"}, status=400)

if not request.user.is_authenticated:
return Response({"detail": "Authentication credentials were not provided."}, status=401)

# Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2).
result = sentry_app_cell_service.create_external_issue(
Comment thread
geoffg-sentry marked this conversation as resolved.
organization_id=installation.organization_id,
installation=installation,
group_id=group_id,
web_url=data["webUrl"],
project=data["project"],
identifier=data["identifier"],
)

Check failure on line 55 in src/sentry/sentry_apps/api/endpoints/installation_external_issues.py

View check run for this annotation

@sentry/warden / warden: sentry-security

Project-level IDOR not fixed: user not passed to RPC prevents has_project_access enforcement

The authentication check at line 44-45 ensures the user is authenticated, but the `user` parameter is not passed to `create_external_issue()` RPC call. In `impl.py:202-214`, the `has_project_access()` check only runs when `user is not None`. Since `user` is not passed, any authenticated organization member can create external issues for groups in projects they don't have access to (violating open team membership restrictions). The org-level scoping at `impl.py:184` only prevents cross-organization access, not cross-project access within the same org.

if result.error:
return self.respond_rpc_sentry_app_error(result.error)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,26 @@
if not uri:
return Response({"detail": "uri query parameter is required"}, status=400)

if not request.user.is_authenticated:
return Response({"detail": "Authentication credentials were not provided."}, status=401)

project_id: int | None = None
project_id_raw = request.GET.get("projectId")
if project_id_raw:
try:
project_id = int(project_id_raw)
except (TypeError, ValueError):
return Response({"detail": "projectId must be an integer"}, status=400)

# Do not pass `user` until cells accept the new RPC arg everywhere (deploy phase 2).
result = sentry_app_cell_service.get_select_options(
organization_id=installation.organization_id,
installation=installation,
uri=request.GET.get("uri"),
project_id=int(request.GET["projectId"]) if request.GET.get("projectId") else None,
project_id=project_id,
query=request.GET.get("query"),
dependent_data=request.GET.get("dependentData"),
)

Check failure on line 48 in src/sentry/sentry_apps/api/endpoints/installation_external_requests.py

View check run for this annotation

@sentry/warden / warden: sentry-security

[FLA-EA3] Project-level IDOR not fixed: user not passed to RPC prevents has_project_access enforcement (additional location)

The authentication check at line 44-45 ensures the user is authenticated, but the `user` parameter is not passed to `create_external_issue()` RPC call. In `impl.py:202-214`, the `has_project_access()` check only runs when `user is not None`. Since `user` is not passed, any authenticated organization member can create external issues for groups in projects they don't have access to (violating open team membership restrictions). The org-level scoping at `impl.py:184` only prevents cross-organization access, not cross-project access within the same org.

if result.error:
return self.respond_rpc_sentry_app_error(result.error)
Expand Down
92 changes: 86 additions & 6 deletions src/sentry/sentry_apps/services/cell/impl.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def get_select_options(
organization_id: int,
installation: RpcSentryAppInstallation,
uri: str,
user: RpcUser | None = None,
Comment thread
geoffg-sentry marked this conversation as resolved.
project_id: int | None = None,
query: str | None = None,
dependent_data: str | None = None,
Expand All @@ -54,9 +55,32 @@ def get_select_options(

project_slug: str | None = None
if project_id is not None:
project = Project.objects.filter(id=project_id, organization_id=organization_id).first()
if project:
project_slug = project.slug
project = (
Project.objects.select_related("organization")
.filter(id=project_id, organization_id=organization_id)
.first()
)
if not project:
return RpcSelectRequesterResult(
error=RpcSentryAppError(
message="Could not find the given project for this organization.",
status_code=404,
)
)
if user is not None:
access = self._access_for_installation_user(
organization=project.organization,
installation=installation,
user=user,
)
if not access.has_project_access(project):
return RpcSelectRequesterResult(
error=RpcSentryAppError(
message="You do not have permission to access this project.",
status_code=403,
)
)
project_slug = project.slug

try:
result = SelectRequester(
Expand Down Expand Up @@ -149,12 +173,13 @@ def create_external_issue(
web_url: str,
project: str,
identifier: str,
user: RpcUser | None = None,
) -> RpcPlatformExternalIssueResult:
"""
Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issues.py @ POST
"""
try:
group = Group.objects.get(
group = Group.objects.select_related("project", "project__organization").get(
id=group_id,
project_id__in=Project.objects.filter(organization_id=organization_id),
)
Expand All @@ -166,6 +191,28 @@ def create_external_issue(
)
)

if group.project.organization_id != organization_id:
return RpcPlatformExternalIssueResult(
error=RpcSentryAppError(
message="Could not find the corresponding issue for the given issueId",
status_code=404,
)
)

if user is not None:
access = self._access_for_installation_user(
organization=group.project.organization,
installation=installation,
user=user,
)
if not access.has_project_access(group.project):
return RpcPlatformExternalIssueResult(
error=RpcSentryAppError(
message="You do not have permission to create an external issue for this issue.",
status_code=403,
)
)

try:
external_issue = ExternalIssueCreator(
install=installation,
Expand All @@ -187,14 +234,21 @@ def delete_external_issue(
organization_id: int,
installation: RpcSentryAppInstallation,
external_issue_id: int,
user: RpcUser | None = None,
) -> RpcEmptyResult:
"""
Matches: src/sentry/sentry_apps/api/endpoints/installation_external_issue_details.py @ DELETE
"""
try:
platform_external_issue = PlatformExternalIssue.objects.get(
platform_external_issue = PlatformExternalIssue.objects.select_related(
"group",
"group__project",
"group__project__organization",
"project",
"project__organization",
).get(
Comment thread
geoffg-sentry marked this conversation as resolved.
id=external_issue_id,
project__organization_id=organization_id,
group__project__organization_id=organization_id,
service_type=installation.sentry_app.slug,
)
except PlatformExternalIssue.DoesNotExist:
Expand All @@ -206,6 +260,32 @@ def delete_external_issue(
),
)

issue_project = platform_external_issue.project or platform_external_issue.group.project
if issue_project.organization_id != organization_id:
return RpcEmptyResult(
success=False,
error=RpcSentryAppError(
message="Could not find the corresponding external issue from given external_issue_id",
status_code=404,
),
)
organization = issue_project.organization

if user is not None:
access = self._access_for_installation_user(
organization=organization,
installation=installation,
user=user,
)
if not access.has_project_access(issue_project):
return RpcEmptyResult(
success=False,
error=RpcSentryAppError(
message="You do not have permission to delete this external issue.",
status_code=403,
),
)

deletions.exec_sync(platform_external_issue)

return RpcEmptyResult()
Expand Down
3 changes: 3 additions & 0 deletions src/sentry/sentry_apps/services/cell/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ def get_select_options(
organization_id: int,
installation: RpcSentryAppInstallation,
uri: str,
user: RpcUser | None = None,
project_id: int | None = None,
query: str | None = None,
dependent_data: str | None = None,
Expand Down Expand Up @@ -85,6 +86,7 @@ def create_external_issue(
web_url: str,
project: str,
identifier: str,
user: RpcUser | None = None,
) -> RpcPlatformExternalIssueResult:
"""Invokes ExternalIssueCreator to create an external issue."""
pass
Expand All @@ -97,6 +99,7 @@ def delete_external_issue(
organization_id: int,
installation: RpcSentryAppInstallation,
external_issue_id: int,
user: RpcUser | None = None,
) -> RpcEmptyResult:
"""Deletes a PlatformExternalIssue."""
pass
Expand Down
7 changes: 5 additions & 2 deletions src/sentry/users/services/user/serial.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
from collections.abc import Iterable
from typing import Any

from django.utils.functional import LazyObject
from django.utils.functional import LazyObject, empty

from sentry.db.models.manager.base_query_set import BaseQuerySet
from sentry.users.models.user import User
Expand All @@ -23,7 +23,10 @@ def serialize_generic_user(user: Any) -> RpcUser | None:
Return None if the user is anonymous (not logged in).
"""
if isinstance(user, LazyObject): # from auth middleware
user = getattr(user, "_wrapped")
lazy: Any = user
if lazy._wrapped is empty:
lazy._setup()
user = lazy._wrapped
if user is None or user.id is None:
return None
if isinstance(user, RpcUser):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,9 @@ def test_creates_external_issue(self) -> None:
def test_invalid_group_id(self) -> None:
self._set_up_sentry_app("Testin", ["event:write"])
data = self._post_data()
data["issueId"] = self.create_group(project=self.create_project()).id
other_org = self.create_organization()
other_project = self.create_project(organization=other_org)
data["issueId"] = self.create_group(project=other_project).id

response = self.client.post(
self.url, data=data, HTTP_AUTHORIZATION=f"Bearer {self.api_token.token}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,3 +93,10 @@ def test_external_request_fails(self) -> None:
url = self.url + f"?uri={self.project.id}"
response = self.client.get(url, format="json")
assert response.status_code == 500

def test_invalid_project_id_returns_400(self) -> None:
self.login_as(user=self.user)
url = self.url + "?projectId=not-an-int&uri=/get-projects&query=proj"
response = self.client.get(url, format="json")
assert response.status_code == 400
assert response.data["detail"] == "projectId must be an integer"
Loading
Loading