Skip to content

Commit

Permalink
fix(roles): Allow team admins to download debug files (#78279)
Browse files Browse the repository at this point in the history
There's a lot of test refactoring b/c it was super messy before, so I'll
point out where the new testing happens

Closes #78229
  • Loading branch information
schew2381 committed Sep 27, 2024
1 parent 540274c commit 09cb65f
Show file tree
Hide file tree
Showing 3 changed files with 215 additions and 363 deletions.
40 changes: 23 additions & 17 deletions src/sentry/api/endpoints/debug_files.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
from symbolic.debuginfo import normalize_debug_id
from symbolic.exceptions import SymbolicError

from sentry import ratelimits, roles
from sentry import ratelimits
from sentry.api.api_owners import ApiOwner
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
Expand All @@ -39,6 +39,7 @@
from sentry.models.project import Project
from sentry.models.release import Release, get_artifact_counts
from sentry.models.releasefile import ReleaseFile
from sentry.roles import organization_roles
from sentry.tasks.assemble import (
AssembleTask,
ChunkFileState,
Expand All @@ -53,15 +54,15 @@
_release_suffix = re.compile(r"^(.*)\s+\(([^)]+)\)\s*$")


def upload_from_request(request, project):
def upload_from_request(request: Request, project: Project):
if "file" not in request.data:
return Response({"detail": "Missing uploaded file"}, status=400)
fileobj = request.data["file"]
files = create_files_from_dif_zip(fileobj, project=project)
return Response(serialize(files, request.user), status=201)


def has_download_permission(request, project):
def has_download_permission(request: Request, project: Project):
if is_system_auth(request.auth) or is_active_superuser(request):
return True

Expand All @@ -72,7 +73,7 @@ def has_download_permission(request, project):
required_role = organization.get_option("sentry:debug_files_role") or DEBUG_FILES_ROLE_DEFAULT

if request.user.is_sentry_app:
if roles.get(required_role).priority > roles.get("member").priority:
if organization_roles.can_manage("member", required_role):
return request.access.has_scope("project:write")
else:
return request.access.has_scope("project:read")
Expand All @@ -86,7 +87,12 @@ def has_download_permission(request, project):
except OrganizationMember.DoesNotExist:
return False

return roles.get(current_role).priority >= roles.get(required_role).priority
if organization_roles.can_manage(current_role, required_role):
return True

# There's an edge case where a team admin is an org member but the required
# role is org admin. In that case, the team admin should be able to download.
return required_role == "admin" and request.access.has_project_scope(project, "project:write")


def _has_delete_permission(access: Access, project: Project) -> bool:
Expand All @@ -104,7 +110,7 @@ class ProguardArtifactReleasesEndpoint(ProjectEndpoint):
}
permission_classes = (ProjectReleasePermission,)

def post(self, request: Request, project) -> Response:
def post(self, request: Request, project: Project) -> Response:
release_name = request.data.get("release_name")
proguard_uuid = request.data.get("proguard_uuid")

Expand Down Expand Up @@ -153,7 +159,7 @@ def post(self, request: Request, project) -> Response:
status=status.HTTP_409_CONFLICT,
)

def get(self, request: Request, project) -> Response:
def get(self, request: Request, project: Project) -> Response:
"""
List a Project's Proguard Associated Releases
````````````````````````````````````````
Expand Down Expand Up @@ -189,7 +195,7 @@ class DebugFilesEndpoint(ProjectEndpoint):
}
permission_classes = (ProjectReleasePermission,)

def download(self, debug_file_id, project):
def download(self, debug_file_id, project: Project):
rate_limited = ratelimits.backend.is_limited(
project=project,
key=f"rl:DSymFilesEndpoint:download:{debug_file_id}:{project.id}",
Expand Down Expand Up @@ -223,7 +229,7 @@ def download(self, debug_file_id, project):
except OSError:
raise Http404

def get(self, request: Request, project) -> Response:
def get(self, request: Request, project: Project) -> Response:
"""
List a Project's Debug Information Files
````````````````````````````````````````
Expand All @@ -240,7 +246,7 @@ def get(self, request: Request, project) -> Response:
:auth: required
"""
download_requested = request.GET.get("id") is not None
if download_requested and (has_download_permission(request, project)):
if download_requested and has_download_permission(request, project):
return self.download(request.GET.get("id"), project)
elif download_requested:
return Response(status=403)
Expand Down Expand Up @@ -335,7 +341,7 @@ def delete(self, request: Request, project: Project) -> Response:

return Response(status=404)

def post(self, request: Request, project) -> Response:
def post(self, request: Request, project: Project) -> Response:
"""
Upload a New File
`````````````````
Expand Down Expand Up @@ -367,7 +373,7 @@ class UnknownDebugFilesEndpoint(ProjectEndpoint):
}
permission_classes = (ProjectReleasePermission,)

def get(self, request: Request, project) -> Response:
def get(self, request: Request, project: Project) -> Response:
checksums = request.GET.getlist("checksums")
missing = ProjectDebugFile.objects.find_missing(checksums, project=project)
return Response({"missing": missing})
Expand All @@ -382,7 +388,7 @@ class AssociateDSymFilesEndpoint(ProjectEndpoint):
permission_classes = (ProjectReleasePermission,)

# Legacy endpoint, kept for backwards compatibility
def post(self, request: Request, project) -> Response:
def post(self, request: Request, project: Project) -> Response:
return Response({"associatedDsymFiles": []})


Expand All @@ -394,7 +400,7 @@ class DifAssembleEndpoint(ProjectEndpoint):
}
permission_classes = (ProjectReleasePermission,)

def post(self, request: Request, project) -> Response:
def post(self, request: Request, project: Project) -> Response:
"""
Assemble one or multiple chunks (FileBlob) into debug files
````````````````````````````````````````````````````````````
Expand Down Expand Up @@ -517,7 +523,7 @@ class SourceMapsEndpoint(ProjectEndpoint):
}
permission_classes = (ProjectReleasePermission,)

def get(self, request: Request, project) -> Response:
def get(self, request: Request, project: Project) -> Response:
"""
List a Project's Source Map Archives
````````````````````````````````````
Expand Down Expand Up @@ -549,7 +555,7 @@ def get(self, request: Request, project) -> Response:

queryset = queryset.filter(query_q)

def expose_release(release, count):
def expose_release(release, count: int):
return {
"type": "release",
"id": release["id"],
Expand Down Expand Up @@ -581,7 +587,7 @@ def serialize_results(results):
on_results=serialize_results,
)

def delete(self, request: Request, project) -> Response:
def delete(self, request: Request, project: Project) -> Response:
"""
Delete an Archive
```````````````````````````````````````````````````
Expand Down
2 changes: 1 addition & 1 deletion src/sentry/roles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,5 @@
get_choices = default_manager.get_choices
get_default = default_manager.get_default
get_top_dog = default_manager.get_top_dog
with_scope = default_manager.with_scope
with_any_scope = default_manager.with_any_scope
with_scope = default_manager.with_scope
Loading

0 comments on commit 09cb65f

Please sign in to comment.