Don't let SuspiciousOperation bubble up when validating channel ids in API

CHANGELOG.md

@@ -10,7 +10,7 @@ All notable changes to this project will be documented in this file.
 ### Bug Fixes
 - The "render_docs" command checks if markdown and pygments is installed (#329)
 - The team size limit is applied to the n. of distinct users across all projects (#332)
-
+- Don't let SuspiciousOperation bubble up when validating channel ids in API
 
 ## v1.13.0 - 2020-02-13
 

hc/api/tests/test_update_check.py

@@ -143,11 +143,16 @@ class UpdateCheckTestCase(BaseTestCase):
         self.assertEqual(check.channel_set.count(), 1)
 
     def test_it_rejects_bad_channel_code(self):
-        r = self.post(
-            self.check.code, {"api_key": "X" * 32, "channels": str(uuid.uuid4())}
-        )
+        r = self.post(self.check.code, {"api_key": "X" * 32, "channels": "abc"})
+        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.json()["error"], "invalid channel identifier: abc")
+
+    def test_it_rejects_missing_channel(self):
+        code = str(uuid.uuid4())
+        r = self.post(self.check.code, {"api_key": "X" * 32, "channels": code})
 
         self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.json()["error"], "invalid channel identifier: " + code)
 
         self.check.refresh_from_db()
         self.assertEqual(self.check.channel_set.count(), 0)

hc/api/views.py

@@ -2,7 +2,6 @@ from datetime import timedelta as td
 import uuid
 
 from django.conf import settings
-from django.core.exceptions import SuspiciousOperation
 from django.db import connection
 from django.http import (
     HttpResponse,
@@ -22,6 +21,10 @@ from hc.api.models import Check, Notification, Channel
 from hc.lib.badges import check_signature, get_badge_svg
 
 
+class BadChannelException(Exception):
+    pass
+
+
 @csrf_exempt
 @never_cache
 def ping(request, code, action="success"):
@@ -101,13 +104,14 @@ def _update(check, spec):
                 try:
                     chunk = uuid.UUID(chunk)
                 except ValueError:
-                    raise SuspiciousOperation("Invalid channel identifier")
+                    raise BadChannelException("invalid channel identifier: %s" % chunk)
 
                 try:
                     channel = Channel.objects.get(code=chunk)
                     channels.append(channel)
                 except Channel.DoesNotExist:
-                    raise SuspiciousOperation("Invalid channel identifier")
+                    raise BadChannelException("invalid channel identifier: %s" % chunk)
+
             check.channel_set.set(channels)
 
     return check
@@ -145,7 +149,11 @@ def create_check(request):
         check = Check(project=request.project)
         created = True
 
+    try:
         _update(check, request.json)
+    except BadChannelException as e:
+        return JsonResponse({"error": str(e)}, status=400)
+
     return JsonResponse(check.to_dict(), status=201 if created else 200)
 
 
@@ -177,7 +185,11 @@ def update(request, code):
         return HttpResponseForbidden()
 
     if request.method == "POST":
+        try:
             _update(check, request.json)
+        except BadChannelException as e:
+            return JsonResponse({"error": str(e)}, status=400)
+
         return JsonResponse(check.to_dict())
 
     elif request.method == "DELETE":