From patchwork Tue Oct 30 11:19:02 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Stephen Finucane X-Patchwork-Id: 990743 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [203.11.71.2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 42kq1v1TbWz9s89 for ; Tue, 30 Oct 2018 22:27:15 +1100 (AEDT) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="XfX7Rgxa"; dkim-atps=neutral Received: from lists.ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 42kq1t2h8szDqfS for ; Tue, 30 Oct 2018 22:27:14 +1100 (AEDT) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="XfX7Rgxa"; dkim-atps=neutral X-Original-To: patchwork@lists.ozlabs.org Delivered-To: patchwork@lists.ozlabs.org Authentication-Results: lists.ozlabs.org; spf=none (mailfrom) smtp.mailfrom=that.guru (client-ip=185.234.75.10; helo=relay003.mxroute.com; envelope-from=stephen@that.guru; receiver=) Authentication-Results: lists.ozlabs.org; dmarc=none (p=none dis=none) header.from=that.guru Authentication-Results: lists.ozlabs.org; dkim=fail reason="key not found in DNS" (0-bit key; unprotected) header.d=that.guru header.i=@that.guru header.b="XfX7Rgxa"; dkim-atps=neutral Received: from relay003.mxroute.com (relay003.mxroute.com [185.234.75.10]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 42kpsf4VhfzDsG1 for ; Tue, 30 Oct 2018 22:20:06 +1100 (AEDT) Received: from relay-ext2.mxrelay.co (relay-ext2.mxrelay.co [159.100.240.208]) (using TLSv1.2 with cipher ADH-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by relay004.mxroute.com (Postfix) with ESMTPS id 0CFDE3F37D for ; Tue, 30 Oct 2018 11:19:34 +0000 (UTC) Received: from filter001.mxrelay.co (filter001.mxrelay.co [64.52.23.203]) by relay-ext2.mxrelay.co (Postfix) with ESMTP id 2EFF43FC3F for ; Tue, 30 Oct 2018 11:19:27 +0000 (UTC) Received: from one.mxroute.com (one.mxroute.com [195.201.59.211]) by filter001.mxrelay.co (Postfix) with ESMTPS id 131D510007F for ; Tue, 30 Oct 2018 11:19:26 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=that.guru; s=default; h=References:In-Reply-To:Message-Id:Date:Subject:Cc:To:From: Sender:Reply-To:MIME-Version:Content-Type:Content-Transfer-Encoding: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:List-Id:List-Help:List-Unsubscribe: List-Subscribe:List-Post:List-Owner:List-Archive; bh=MJ/2SYayCNny8YPr+l5gqjrgZEw1TdcjxoTxd3U3QIk=; b=XfX7Rgxaq8MKxxh18sE8Kx6IqQ XIe9JApkjfzu1lvC78hSFBaLFgJFy8VlO/q0yPOD1P6FXnOqqm5vL0+K+Xu4iinFCQMym4RQ2h1n4 RXLAhEzkZfVnNfmi4S8Nb1hx+hHCNWvz0C91D6hq9VcUhQN8hYqe7yKNun8i9/0l3pBCcuODdJUaP hxkQEWIT6jEaFOUfqO6wrj3XbTVTJdZY/EaMx4hajAAshC2t0rnGQ9/58C+9TomMegpyOFLg/HyDP vj1dYNlSbUv2JVWCDhphAoD/ZDcmsJaABT6Kqc/bcA8vc9nogl+sASgoJ+lXKl4p+jofbkGkCstgi 1vebx4ag==; From: Stephen Finucane To: patchwork@lists.ozlabs.org Subject: [PATCH 03/17] REST: Ensure patch exists for check creation Date: Tue, 30 Oct 2018 11:19:02 +0000 Message-Id: <20181030111916.7342-4-stephen@that.guru> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20181030111916.7342-1-stephen@that.guru> References: <20181030111916.7342-1-stephen@that.guru> X-AuthUser: stephen@that.guru X-BeenThere: patchwork@lists.ozlabs.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Patchwork development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: patchwork-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Patchwork" Signed-off-by: Stephen Finucane Closes: #226 --- patchwork/api/check.py | 8 ++++- patchwork/tests/api/test_check.py | 31 ++++++++++++++++++- .../notes/issue-226-27ea72266d3ee9ac.yaml | 7 +++++ 3 files changed, 44 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml diff --git a/patchwork/api/check.py b/patchwork/api/check.py index 4771455f..0e35bd45 100644 --- a/patchwork/api/check.py +++ b/patchwork/api/check.py @@ -3,6 +3,8 @@ # # SPDX-License-Identifier: GPL-2.0-or-later +from django.http import Http404 +from django.shortcuts import get_object_or_404 from rest_framework.exceptions import PermissionDenied from rest_framework.generics import ListCreateAPIView from rest_framework.generics import RetrieveAPIView @@ -70,6 +72,10 @@ class CheckMixin(object): def get_queryset(self): patch_id = self.kwargs['patch_id'] + + if not Patch.objects.filter(pk=self.kwargs['patch_id']).exists(): + raise Http404 + return Check.objects.prefetch_related('user').filter(patch=patch_id) @@ -86,7 +92,7 @@ class CheckListCreate(CheckMixin, ListCreateAPIView): ordering = 'id' def create(self, request, patch_id, *args, **kwargs): - p = Patch.objects.get(id=patch_id) + p = get_object_or_404(Patch, id=patch_id) if not p.is_editable(request.user): raise PermissionDenied() request.patch = p diff --git a/patchwork/tests/api/test_check.py b/patchwork/tests/api/test_check.py index 783a6154..06255957 100644 --- a/patchwork/tests/api/test_check.py +++ b/patchwork/tests/api/test_check.py @@ -77,6 +77,12 @@ class TestCheckAPI(APITestCase): resp = self.client.get(self.api_url(), {'user': 'otheruser'}) self.assertEqual(0, len(resp.data)) + def test_list_invalid_patch(self): + """Ensure we get a 404 for a non-existent patch.""" + resp = self.client.get( + reverse('api-check-list', kwargs={'pk': '99999'})) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + def test_detail(self): """Validate we can get a specific check.""" check = self._create_check() @@ -99,12 +105,21 @@ class TestCheckAPI(APITestCase): self.assertEqual(1, Check.objects.all().count()) self.assertSerialized(Check.objects.first(), resp.data) + def test_create_no_permissions(self): + """Ensure creations are rejected by standard users.""" + check = { + 'state': 'success', + 'target_url': 'http://t.co', + 'description': 'description', + 'context': 'context', + } + user = create_user() self.client.force_authenticate(user=user) resp = self.client.post(self.api_url(), check) self.assertEqual(status.HTTP_403_FORBIDDEN, resp.status_code) - def test_create_invalid(self): + def test_create_invalid_state(self): """Ensure we handle invalid check states.""" check = { 'state': 'this-is-not-a-valid-state', @@ -118,6 +133,20 @@ class TestCheckAPI(APITestCase): self.assertEqual(status.HTTP_400_BAD_REQUEST, resp.status_code) self.assertEqual(0, Check.objects.all().count()) + def test_create_invalid_patch(self): + """Ensure we handle non-existent patches.""" + check = { + 'state': 'success', + 'target_url': 'http://t.co', + 'description': 'description', + 'context': 'context', + } + + self.client.force_authenticate(user=self.user) + resp = self.client.post( + reverse('api-check-list', kwargs={'pk': '99999'}), check) + self.assertEqual(status.HTTP_404_NOT_FOUND, resp.status_code) + def test_update_delete(self): """Ensure updates and deletes aren't allowed""" check = self._create_check() diff --git a/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml b/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml new file mode 100644 index 00000000..8f891e04 --- /dev/null +++ b/releasenotes/notes/issue-226-27ea72266d3ee9ac.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Showing checks for a non-existant patch was returning an empty response + instead of a HTTP 404. Similarly, attempting to create a new check against + this patch would result in a HTTP 5xx error instead of a HTTP 404. Both + issues are now resolved.