From 435659166c32d69cb003cf986ce09cf7e28fae93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Wed, 19 Feb 2020 11:43:42 +0200
Subject: [PATCH] Don't let SuspiciousOperation bubble up when validating
 channel ids in API

---
 CHANGELOG.md                      |  2 +-
 hc/api/tests/test_update_check.py | 11 ++++++++---
 hc/api/views.py                   | 22 +++++++++++++++++-----
 3 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 99abd110..ee33151c 100644
--- a/CHANGELOG.md
+++ b/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
 
diff --git a/hc/api/tests/test_update_check.py b/hc/api/tests/test_update_check.py
index d2a79fdd..9cf5c8f4 100644
--- a/hc/api/tests/test_update_check.py
+++ b/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)
diff --git a/hc/api/views.py b/hc/api/views.py
index ca5f5333..827ff9b5 100644
--- a/hc/api/views.py
+++ b/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
 
-    _update(check, request.json)
+    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":
-        _update(check, request.json)
+        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":