Return 403 when API key is wrong. Return 404 when resource not found. Return 405 when request method is wrong. Return 400 when request syntax is wrong.

hc/api/decorators.py

@@ -3,7 +3,8 @@ import uuid
 from functools import wraps
 
 from django.contrib.auth.models import User
-from django.http import HttpResponseBadRequest, JsonResponse
+from django.http import (HttpResponseBadRequest, HttpResponseForbidden,
+                         JsonResponse)
 from hc.lib.jsonschema import ValidationError, validate
 
 
@@ -44,7 +45,7 @@ def check_api_key(f):
         try:
             request.user = User.objects.get(profile__api_key=api_key)
         except User.DoesNotExist:
-            return make_error("wrong api_key")
+            return HttpResponseForbidden()
 
         return f(request, *args, **kwds)
 

hc/api/tests/test_badge.py

@@ -13,7 +13,7 @@ class BadgeTestCase(BaseTestCase):
 
     def test_it_rejects_bad_signature(self):
         r = self.client.get("/badge/%s/12345678/foo.svg" % self.alice.username)
-        assert r.status_code == 400
+        assert r.status_code == 404
 
     def test_it_returns_svg(self):
         sig = base64_hmac(str(self.alice.username), "foo", settings.SECRET_KEY)

hc/api/tests/test_bounce.py

@@ -33,10 +33,16 @@ class BounceTestCase(BaseTestCase):
 
         url = "/api/v1/notifications/%s/bounce" % self.n.code
         r = self.client.post(url, "foo", content_type="text/plain")
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 403)
 
     def test_it_handles_long_payload(self):
         url = "/api/v1/notifications/%s/bounce" % self.n.code
         payload = "A" * 500
         r = self.client.post(url, payload, content_type="text/plain")
         self.assertEqual(r.status_code, 200)
+
+    def test_it_handles_missing_notification(self):
+        fake_code = "07c2f548-9850-4b27-af5d-6c9dc157ec02"
+        url = "/api/v1/notifications/%s/bounce" % fake_code
+        r = self.client.post(url, "", content_type="text/plain")
+        self.assertEqual(r.status_code, 404)

hc/api/tests/test_create_check.py

@@ -95,8 +95,8 @@ class CreateCheckTestCase(BaseTestCase):
         self.assertEqual(r.json()["error"], "could not parse request body")
 
     def test_it_rejects_wrong_api_key(self):
-        self.post({"api_key": "wrong"},
+        r = self.post({"api_key": "wrong"})
-                  expected_error="wrong api_key")
+        self.assertEqual(r.status_code, 403)
 
     def test_it_rejects_small_timeout(self):
         self.post({"api_key": "abc", "timeout": 0},

hc/api/tests/test_pause.py

@@ -31,7 +31,7 @@ class PauseTestCase(BaseTestCase):
         r = self.client.post(url, "", content_type="application/json",
                              HTTP_X_API_KEY="abc")
 
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 403)
 
     def test_it_validates_uuid(self):
         url = "/api/v1/checks/not-uuid/pause"
@@ -45,4 +45,4 @@ class PauseTestCase(BaseTestCase):
         r = self.client.post(url, "", content_type="application/json",
                              HTTP_X_API_KEY="abc")
 
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 404)

hc/api/tests/test_ping.py

@@ -45,6 +45,10 @@ class PingTestCase(TestCase):
         r = self.client.get("/ping/not-uuid/")
         assert r.status_code == 400
 
+    def test_it_handles_missing_check(self):
+        r = self.client.get("/ping/07c2f548-9850-4b27-af5d-6c9dc157ec02/")
+        assert r.status_code == 404
+
     def test_it_handles_120_char_ua(self):
         ua = ("Mozilla/5.0 (Macintosh; Intel Mac OS X 10_10_4) "
               "AppleWebKit/537.36 (KHTML, like Gecko) "

hc/api/tests/test_update_check.py

@@ -74,7 +74,14 @@ class UpdateCheckTestCase(BaseTestCase):
     def test_it_handles_missing_check(self):
         made_up_code = "07c2f548-9850-4b27-af5d-6c9dc157ec02"
         r = self.post(made_up_code, {"api_key": "abc"})
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 404)
+
+    def test_it_validates_ownership(self):
+        check = Check(user=self.bob, status="up")
+        check.save()
+
+        r = self.post(check.code, {"api_key": "abc"})
+        self.assertEqual(r.status_code, 403)
 
     def test_it_updates_cron_to_simple(self):
         self.check.kind = "cron"

hc/api/views.py

@@ -1,10 +1,13 @@
 from datetime import timedelta as td
 
 from django.db.models import F
-from django.http import HttpResponse, HttpResponseBadRequest, JsonResponse
+from django.http import (HttpResponse, HttpResponseForbidden,
+                         HttpResponseNotFound, JsonResponse)
+from django.shortcuts import get_object_or_404
 from django.utils import timezone
 from django.views.decorators.cache import never_cache
 from django.views.decorators.csrf import csrf_exempt
+from django.views.decorators.http import require_POST
 
 from hc.api import schemas
 from hc.api.decorators import check_api_key, uuid_or_400, validate_json
@@ -16,10 +19,7 @@ from hc.lib.badges import check_signature, get_badge_svg
 @uuid_or_400
 @never_cache
 def ping(request, code):
-    try:
+    check = get_object_or_404(Check, code=code)
-        check = Check.objects.get(code=code)
-    except Check.DoesNotExist:
-        return HttpResponseBadRequest()
 
     check.n_pings = F("n_pings") + 1
     check.last_ping = timezone.now()
@@ -122,34 +122,27 @@ def checks(request):
 
 
 @csrf_exempt
+@require_POST
 @uuid_or_400
 @check_api_key
 @validate_json(schemas.check)
 def update(request, code):
-    if request.method != "POST":
+    check = get_object_or_404(Check, code=code)
-        return HttpResponse(status=405)  # method not allowed
+    if check.user != request.user:
-
+        return HttpResponseForbidden()
-    try:
-        check = Check.objects.get(code=code, user=request.user)
-    except Check.DoesNotExist:
-        return HttpResponseBadRequest()
 
     _update(check, request.json)
     return JsonResponse(check.to_dict(), status=200)
 
 
 @csrf_exempt
+@require_POST
 @uuid_or_400
 @check_api_key
 def pause(request, code):
-    if request.method != "POST":
+    check = get_object_or_404(Check, code=code)
-        # Method not allowed
+    if check.user != request.user:
-        return HttpResponse(status=405)
+        return HttpResponseForbidden()
-
-    try:
-        check = Check.objects.get(code=code, user=request.user)
-    except Check.DoesNotExist:
-        return HttpResponseBadRequest()
 
     check.status = "paused"
     check.save()
@@ -159,7 +152,7 @@ def pause(request, code):
 @never_cache
 def badge(request, username, signature, tag):
     if not check_signature(username, tag, signature):
-        return HttpResponseBadRequest()
+        return HttpResponseNotFound()
 
     status = "up"
     q = Check.objects.filter(user__username=username, tags__contains=tag)
@@ -181,15 +174,12 @@ def badge(request, username, signature, tag):
 @csrf_exempt
 @uuid_or_400
 def bounce(request, code):
-    try:
+    notification = get_object_or_404(Notification, code=code)
-        notification = Notification.objects.get(code=code)
-    except Notification.DoesNotExist:
-        return HttpResponseBadRequest()
 
     # If webhook is more than 10 minutes late, don't accept it:
     td = timezone.now() - notification.created
     if td.total_seconds() > 600:
-        return HttpResponseBadRequest()
+        return HttpResponseForbidden()
 
     notification.error = request.body[:200]
     notification.save()

hc/front/tests/test_add_check.py

@@ -24,4 +24,4 @@ class AddCheckTestCase(BaseTestCase):
         url = "/checks/add/"
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_cron_preview.py

@@ -29,4 +29,4 @@ class CronPreviewTestCase(BaseTestCase):
 
     def test_it_rejects_get(self):
         r = self.client.get("/checks/cron_preview/", {})
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_pause.py

@@ -23,4 +23,4 @@ class PauseTestCase(BaseTestCase):
         url = "/checks/%s/pause/" % self.check.code
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_remove_channel.py

@@ -52,4 +52,4 @@ class RemoveChannelTestCase(BaseTestCase):
         url = "/integrations/%s/remove/" % self.channel.code
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_remove_check.py

@@ -53,4 +53,4 @@ class RemoveCheckTestCase(BaseTestCase):
         url = "/checks/%s/remove/" % self.check.code
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_update_name.py

@@ -71,4 +71,4 @@ class UpdateNameTestCase(BaseTestCase):
         url = "/checks/%s/name/" % self.check.code
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/tests/test_update_timeout.py

@@ -125,4 +125,4 @@ class UpdateTimeoutTestCase(BaseTestCase):
         url = "/checks/%s/timeout/" % self.check.code
         self.client.login(username="alice@example.org", password="password")
         r = self.client.get(url)
-        self.assertEqual(r.status_code, 400)
+        self.assertEqual(r.status_code, 405)

hc/front/views.py

@@ -14,6 +14,7 @@ from django.urls import reverse
 from django.utils import timezone
 from django.utils.crypto import get_random_string
 from django.views.decorators.csrf import csrf_exempt
+from django.views.decorators.http import require_POST
 from django.utils.six.moves.urllib.parse import urlencode
 from hc.api.decorators import uuid_or_400
 from hc.api.models import (DEFAULT_GRACE, DEFAULT_TIMEOUT, Channel, Check,
@@ -131,11 +132,9 @@ def about(request):
     return render(request, "front/about.html", {"page": "about"})
 
 
+@require_POST
 @login_required
 def add_check(request):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     check = Check(user=request.team.user)
     check.save()
 
@@ -144,12 +143,10 @@ def add_check(request):
     return redirect("hc-checks")
 
 
+@require_POST
 @login_required
 @uuid_or_400
 def update_name(request, code):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     check = get_object_or_404(Check, code=code)
     if check.user_id != request.team.user.id:
         return HttpResponseForbidden()
@@ -163,12 +160,10 @@ def update_name(request, code):
     return redirect("hc-checks")
 
 
+@require_POST
 @login_required
 @uuid_or_400
 def update_timeout(request, code):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     check = get_object_or_404(Check, code=code)
     if check.user != request.team.user:
         return HttpResponseForbidden()
@@ -200,10 +195,8 @@ def update_timeout(request, code):
 
 
 @csrf_exempt
+@require_POST
 def cron_preview(request):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     schedule = request.POST.get("schedule")
     tz = request.POST.get("tz")
     ctx = {"tz": tz, "dates": []}
@@ -223,12 +216,10 @@ def cron_preview(request):
     return render(request, "front/cron_preview.html", ctx)
 
 
+@require_POST
 @login_required
 @uuid_or_400
 def pause(request, code):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     check = get_object_or_404(Check, code=code)
     if check.user_id != request.team.user.id:
         return HttpResponseForbidden()
@@ -239,12 +230,10 @@ def pause(request, code):
     return redirect("hc-checks")
 
 
+@require_POST
 @login_required
 @uuid_or_400
 def remove_check(request, code):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     check = get_object_or_404(Check, code=code)
     if check.user != request.team.user:
         return HttpResponseForbidden()
@@ -375,12 +364,10 @@ def unsubscribe_email(request, code, token):
     return render(request, "front/unsubscribe_success.html")
 
 
+@require_POST
 @login_required
 @uuid_or_400
 def remove_channel(request, code):
-    if request.method != "POST":
-        return HttpResponseBadRequest()
-
     # user may refresh the page during POST and cause two deletion attempts
     channel = Channel.objects.filter(code=code).first()
     if channel: