From 66b30bb7927abfb4d23115f7a3e7eb3e3227e2bc Mon Sep 17 00:00:00 2001 From: Klaas van Schelven Date: Thu, 9 Nov 2023 23:49:52 +0100 Subject: [PATCH] Actually connect events to the correct project when the header is provided --- ingest/views.py | 23 +++++++++++----- .../0002_project_name_project_sentry_key.py | 24 +++++++++++++++++ projects/models.py | 27 ++++++++++++++++++- sentry/utils/auth.py | 11 +++----- 4 files changed, 70 insertions(+), 15 deletions(-) create mode 100644 projects/migrations/0002_project_name_project_sentry_key.py diff --git a/ingest/views.py b/ingest/views.py index 0782387..aeb84e2 100644 --- a/ingest/views.py +++ b/ingest/views.py @@ -1,6 +1,8 @@ import json # TODO consider faster APIs from urllib.parse import urlparse +from django.shortcuts import get_object_or_404 + from rest_framework import permissions, status from rest_framework.response import Response from rest_framework.views import APIView @@ -27,7 +29,7 @@ class BaseIngestAPIView(APIView): http_method_names = ["post"] @classmethod - def auth_from_request(cls, request): + def get_sentry_key_for_request(cls, request): # VENDORED FROM GlitchTip at a4f33da8d4e759d61ffe073a00f2bb3839ac65f5, with changes # KvS: I have not been able to find documentation which suggests that the below is indeed used. @@ -41,7 +43,7 @@ class BaseIngestAPIView(APIView): auth_dict = parse_auth_header(request.META[auth_key]) return auth_dict.get("sentry_key") - # KvS: this is presumably the path that is used for envelopes (and then also when + # KvS: this is presumably the path that is used for envelopes (and then also when the above are not provided) if isinstance(request.data, list): if data_first := next(iter(request.data), None): if isinstance(data_first, dict): @@ -51,6 +53,14 @@ class BaseIngestAPIView(APIView): raise exceptions.NotAuthenticated("Unable to find authentication information") + @classmethod + def get_project(cls, request, project_id): + # NOTE this gives a 404 for non-properly authorized. Is this really something we care about, i.e. do we want to + # raise NotAuthenticated? In that case we need to get the project first, and then do a constant-time-comp on the + # sentry_key + sentry_key = cls.get_sentry_key_for_request(request) + return get_object_or_404(Project, pk=project_id, sentry_key=sentry_key) + def process_event(self, event_data, request, project): event = DecompressedEvent.objects.create( project=project, @@ -67,8 +77,9 @@ class BaseIngestAPIView(APIView): class IngestEventAPIView(BaseIngestAPIView): - def post(self, request, *args, **kwargs): - project = Project.objects.first() # TODO actually parse project header + def post(self, request, project_id=None): + project = self.get_project(request, project_id) + self.process_event(request.data, request, project) return Response() @@ -76,8 +87,8 @@ class IngestEventAPIView(BaseIngestAPIView): class IngestEnvelopeAPIView(BaseIngestAPIView): parser_classes = [EnvelopeParser] - def post(self, request, *args, **kwargs): - project = Project.objects.first() # TODO actually parse project header + def post(self, request, project_id=None): + project = self.get_project(request, project_id) if len(request.data) != 3: # multi-part envelopes trigger an error too diff --git a/projects/migrations/0002_project_name_project_sentry_key.py b/projects/migrations/0002_project_name_project_sentry_key.py new file mode 100644 index 0000000..c86a53b --- /dev/null +++ b/projects/migrations/0002_project_name_project_sentry_key.py @@ -0,0 +1,24 @@ +from random import random +from django.db import migrations, models +import projects.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('projects', '0001_initial'), + ] + + operations = [ + migrations.AddField( + model_name='project', + name='name', + field=models.CharField(default=lambda: str(random()), max_length=255), + preserve_default=False, + ), + migrations.AddField( + model_name='project', + name='sentry_key', + field=models.CharField(default=projects.models.uuid4_hex, max_length=32, unique=True), + ), + ] diff --git a/projects/models.py b/projects/models.py index d908dc9..1fdf23f 100644 --- a/projects/models.py +++ b/projects/models.py @@ -1,7 +1,32 @@ +from uuid import uuid4 + from django.db import models +def uuid4_hex(): + return uuid4().hex + + class Project(models.Model): # id is implied which makes it an Integer; we would prefer a uuid but the sentry clients have int baked into the DSN # parser (we could also introduce a special field for that purpose but that's ugly too) - pass + + name = models.CharField(max_length=255, blank=False, null=False) + + # sentry_key mirrors the "public" part of the sentry DSN. As of late 2023 Sentry's docs say the this about DSNs: + # + # > DSNs are safe to keep public because they only allow submission of new events and related event data; they do + # > not allow read access to any information. + # + # The "because" in that sentence is dubious at least; however, I get why they say it, because they want to do JS and + # native apps too, and there's really no way to do those without exposing (some) endpoint. Anyway, I don't think the + # "public" key is public, and if you can help it it's always better to keep it private. + sentry_key = models.CharField(max_length=32, unique=True, null=False, default=uuid4_hex) + + # We don't implement private_key because as of late 2023 the Sentry documentation says the following: + # > The secret part of the DSN is optional and effectively deprecated. While clients will still honor it, if + # > supplied, future versions of Sentry will entirely ignore it. + # private_key = ... + + def __str__(self): + return self.name diff --git a/sentry/utils/auth.py b/sentry/utils/auth.py index 39df16b..568e0bd 100644 --- a/sentry/utils/auth.py +++ b/sentry/utils/auth.py @@ -1,12 +1,7 @@ def parse_auth_header(header): - # KvS: isn't this always either bytes or strings? I'd like to learn more (first quickly, and then formalizing for - # actual uses) - # https://github.com/getsentry/sentry/pull/12108 is the non-explanation - if isinstance(header, bytes): - print("it's bytes, probably always so") - header = header.decode("latin1") - else: - print("not bytes, already a string") + # Sentry has code in place here to parse bytes (from latin1). Based on how Django works, I'd like to think that's + # not needed. https://github.com/getsentry/sentry/pull/12108 is the non-explanation + # if isinstance(header, bytes): header = header.decode("latin1") try: _, rhs = header.split(" ", 1)