From c2f200fa02dad2b7aadaf4817d677e6fc1229734 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 29 Nov 2018 13:51:25 +0200 Subject: [PATCH] Allow simultaneous access to checks from different teams --- CHANGELOG.md | 3 +- hc/front/tests/test_details.py | 10 +++- hc/front/tests/test_log.py | 11 ++++- hc/front/tests/test_pause.py | 10 ++++ hc/front/tests/test_ping_details.py | 15 +++++- hc/front/tests/test_remove_check.py | 38 ++++++++------- hc/front/tests/test_status_single.py | 8 ++++ hc/front/tests/test_switch_channel.py | 12 ++++- hc/front/tests/test_update_name.py | 16 +++++-- hc/front/tests/test_update_timeout.py | 13 ++++- hc/front/views.py | 68 ++++++++++++--------------- templates/base.html | 7 ++- 12 files changed, 143 insertions(+), 68 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 922c4d33..ea0e6587 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Improvements - Set Pushover alert priorities for "down" and "up" events separately - Additional python usage examples +- Allow simultaneous access to checks from different teams ### Bug Fixes - Fix after-login redirects (the "?next=" query parameter) @@ -52,4 +53,4 @@ All notable changes to this project will be documented in this file. - A new "Check Details" page. - Updated django-compressor, psycopg2, pytz, requests package versions. - C# usage example. -- Checks have a "Description" field. \ No newline at end of file +- Checks have a "Description" field. diff --git a/hc/front/tests/test_details.py b/hc/front/tests/test_details.py index 78570275..c4e69740 100644 --- a/hc/front/tests/test_details.py +++ b/hc/front/tests/test_details.py @@ -29,7 +29,7 @@ class DetailsTestCase(BaseTestCase): def test_it_checks_ownership(self): self.client.login(username="charlie@example.org", password="password") r = self.client.get(self.url) - assert r.status_code == 403 + self.assertEqual(r.status_code, 404) def test_it_shows_cron_expression(self): self.check.kind = "cron" @@ -38,3 +38,11 @@ class DetailsTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get(self.url) self.assertContains(r, "Cron Expression", status_code=200) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + self.client.login(username="bob@example.org", password="password") + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) diff --git a/hc/front/tests/test_log.py b/hc/front/tests/test_log.py index b671503a..868717b3 100644 --- a/hc/front/tests/test_log.py +++ b/hc/front/tests/test_log.py @@ -52,7 +52,7 @@ class LogTestCase(BaseTestCase): url = "/checks/%s/log/" % self.check.code self.client.login(username="charlie@example.org", password="password") r = self.client.get(url) - self.assertEqual(r.status_code, 403) + self.assertEqual(r.status_code, 404) def test_it_shows_pushover_notifications(self): ch = Channel(kind="po", user=self.alice) @@ -77,3 +77,12 @@ class LogTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get(url) self.assertContains(r, "Called webhook foo/$NAME", status_code=200) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + url = "/checks/%s/log/" % self.check.code + self.client.login(username="bob@example.org", password="password") + r = self.client.get(url) + self.assertEqual(r.status_code, 200) diff --git a/hc/front/tests/test_pause.py b/hc/front/tests/test_pause.py index ba173506..5c38023b 100644 --- a/hc/front/tests/test_pause.py +++ b/hc/front/tests/test_pause.py @@ -24,3 +24,13 @@ class PauseTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.get(url) self.assertEqual(r.status_code, 405) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + url = "/checks/%s/pause/" % self.check.code + + self.client.login(username="bob@example.org", password="password") + r = self.client.post(url) + self.assertRedirects(r, "/checks/") diff --git a/hc/front/tests/test_ping_details.py b/hc/front/tests/test_ping_details.py index 539324f6..17e86751 100644 --- a/hc/front/tests/test_ping_details.py +++ b/hc/front/tests/test_ping_details.py @@ -17,7 +17,7 @@ class LastPingTestCase(BaseTestCase): def test_it_requires_user(self): check = Check.objects.create() r = self.client.get("/checks/%s/last_ping/" % check.code) - self.assertEqual(r.status_code, 403) + self.assertEqual(r.status_code, 404) def test_it_accepts_n(self): check = Check(user=self.alice) @@ -34,3 +34,16 @@ class LastPingTestCase(BaseTestCase): r = self.client.get("/checks/%s/pings/2/" % check.code) self.assertContains(r, "bar-456", status_code=200) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + check = Check(user=self.alice) + check.save() + + Ping.objects.create(owner=check, body="this is body") + + self.client.login(username="bob@example.org", password="password") + r = self.client.get("/checks/%s/last_ping/" % check.code) + self.assertEqual(r.status_code, 200) diff --git a/hc/front/tests/test_remove_check.py b/hc/front/tests/test_remove_check.py index af3a9beb..cd5042e6 100644 --- a/hc/front/tests/test_remove_check.py +++ b/hc/front/tests/test_remove_check.py @@ -9,37 +9,32 @@ class RemoveCheckTestCase(BaseTestCase): self.check = Check(user=self.alice) self.check.save() - def test_it_works(self): - url = "/checks/%s/remove/" % self.check.code + self.remove_url = "/checks/%s/remove/" % self.check.code + def test_it_works(self): self.client.login(username="alice@example.org", password="password") - r = self.client.post(url) + r = self.client.post(self.remove_url) self.assertRedirects(r, "/checks/") - assert Check.objects.count() == 0 + self.assertEqual(Check.objects.count(), 0) def test_team_access_works(self): - url = "/checks/%s/remove/" % self.check.code - # Logging in as bob, not alice. Bob has team access so this # should work. self.client.login(username="bob@example.org", password="password") - self.client.post(url) - assert Check.objects.count() == 0 + self.client.post(self.remove_url) - def test_it_handles_bad_uuid(self): - url = "/checks/not-uuid/remove/" + self.assertEqual(Check.objects.count(), 0) + def test_it_handles_bad_uuid(self): self.client.login(username="alice@example.org", password="password") - r = self.client.post(url) + r = self.client.post("/checks/not-uuid/remove/") self.assertEqual(r.status_code, 404) def test_it_checks_owner(self): - url = "/checks/%s/remove/" % self.check.code - self.client.login(username="charlie@example.org", password="password") - r = self.client.post(url) - assert r.status_code == 403 + r = self.client.post(self.remove_url) + self.assertEqual(r.status_code, 404) def test_it_handles_missing_uuid(self): # Valid UUID but there is no check for it: @@ -47,10 +42,17 @@ class RemoveCheckTestCase(BaseTestCase): self.client.login(username="alice@example.org", password="password") r = self.client.post(url) - assert r.status_code == 404 + self.assertEqual(r.status_code, 404) def test_it_rejects_get(self): - url = "/checks/%s/remove/" % self.check.code self.client.login(username="alice@example.org", password="password") - r = self.client.get(url) + r = self.client.get(self.remove_url) self.assertEqual(r.status_code, 405) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + self.client.login(username="bob@example.org", password="password") + r = self.client.post(self.remove_url) + self.assertRedirects(r, "/checks/") diff --git a/hc/front/tests/test_status_single.py b/hc/front/tests/test_status_single.py index 1fa2ce4f..c46ca98c 100644 --- a/hc/front/tests/test_status_single.py +++ b/hc/front/tests/test_status_single.py @@ -46,3 +46,11 @@ class StatusSingleTestCase(BaseTestCase): doc = r.json() self.assertFalse("events" in doc) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + self.client.login(username="bob@example.org", password="password") + r = self.client.get("/checks/%s/status/" % self.check.code) + self.assertEqual(r.status_code, 200) diff --git a/hc/front/tests/test_switch_channel.py b/hc/front/tests/test_switch_channel.py index 58fcf489..48495065 100644 --- a/hc/front/tests/test_switch_channel.py +++ b/hc/front/tests/test_switch_channel.py @@ -32,7 +32,7 @@ class SwitchChannelTestCase(BaseTestCase): def test_it_checks_ownership(self): self.client.login(username="charlie@example.org", password="password") r = self.client.post(self.url, {"state": "on"}) - self.assertEqual(r.status_code, 403) + self.assertEqual(r.status_code, 404) def test_it_checks_channels_ownership(self): cc = Check(user=self.charlie) @@ -43,4 +43,12 @@ class SwitchChannelTestCase(BaseTestCase): self.client.login(username="charlie@example.org", password="password") r = self.client.post(self.url, {"state": "on"}) - self.assertEqual(r.status_code, 403) + self.assertEqual(r.status_code, 400) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + self.client.login(username="bob@example.org", password="password") + r = self.client.post(self.url, {"state": "on"}) + self.assertEqual(r.status_code, 200) diff --git a/hc/front/tests/test_update_name.py b/hc/front/tests/test_update_name.py index 31725615..e238fff7 100644 --- a/hc/front/tests/test_update_name.py +++ b/hc/front/tests/test_update_name.py @@ -12,10 +12,8 @@ class UpdateNameTestCase(BaseTestCase): self.url = "/checks/%s/name/" % self.check.code def test_it_works(self): - payload = {"name": "Alice Was Here"} - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, data=payload) + r = self.client.post(self.url, data={"name": "Alice Was Here"}) self.assertRedirects(r, "/checks/") self.check.refresh_from_db() @@ -32,12 +30,22 @@ class UpdateNameTestCase(BaseTestCase): self.check.refresh_from_db() self.assertEqual(self.check.name, "Bob Was Here") + def test_it_allows_cross_team_access(self): + # Bob's current team is not set + self.bobs_profile.current_team = None + self.bobs_profile.save() + + # But this should still work: + self.client.login(username="bob@example.org", password="password") + r = self.client.post(self.url, data={"name": "Bob Was Here"}) + self.assertRedirects(r, "/checks/") + def test_it_checks_ownership(self): payload = {"name": "Charlie Sent This"} self.client.login(username="charlie@example.org", password="password") r = self.client.post(self.url, data=payload) - self.assertEqual(r.status_code, 403) + self.assertEqual(r.status_code, 404) def test_it_handles_bad_uuid(self): url = "/checks/not-uuid/name/" diff --git a/hc/front/tests/test_update_timeout.py b/hc/front/tests/test_update_timeout.py index c47b7a76..8d6a8e9a 100644 --- a/hc/front/tests/test_update_timeout.py +++ b/hc/front/tests/test_update_timeout.py @@ -145,10 +145,21 @@ class UpdateTimeoutTestCase(BaseTestCase): self.client.login(username="charlie@example.org", password="password") r = self.client.post(url, data=payload) - assert r.status_code == 403 + self.assertEqual(r.status_code, 404) def test_it_rejects_get(self): 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, 405) + + def test_it_allows_cross_team_access(self): + self.bobs_profile.current_team = None + self.bobs_profile.save() + + url = "/checks/%s/timeout/" % self.check.code + payload = {"kind": "simple", "timeout": 3600, "grace": 60} + + self.client.login(username="bob@example.org", password="password") + r = self.client.post(url, data=payload) + self.assertRedirects(r, "/checks/") diff --git a/hc/front/views.py b/hc/front/views.py index 72f14392..68fb1d99 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -60,6 +60,23 @@ def _tags_statuses(checks): return tags, num_down +def _get_check_for_user(request, code): + """ Return specified check if current user has access to it. """ + + if not request.user.is_authenticated: + raise Http404("not found") + + if request.user.is_superuser: + q = Check.objects + else: + q = request.profile.checks_from_all_teams() + + try: + return q.get(code=code) + except Check.DoesNotExist: + raise Http404("not found") + + @login_required def my_checks(request): if request.GET.get("sort") in VALID_SORT_VALUES: @@ -136,13 +153,11 @@ def status(request): @login_required @require_POST def switch_channel(request, code, channel_code): - check = get_object_or_404(Check, code=code) - if check.user_id != request.team.user.id: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) channel = get_object_or_404(Channel, code=channel_code) - if channel.user_id != request.team.user.id: - return HttpResponseForbidden() + if channel.user_id != check.user_id: + return HttpResponseBadRequest() if request.POST.get("state") == "on": channel.checks.add(check) @@ -228,10 +243,7 @@ def add_check(request): @require_POST @login_required def update_name(request, code): - check = get_object_or_404(Check, code=code) - if check.user_id != request.team.user.id: - return HttpResponseForbidden() - + check = _get_check_for_user(request, code) form = NameTagsForm(request.POST) if form.is_valid(): check.name = form.cleaned_data["name"] @@ -248,9 +260,7 @@ def update_name(request, code): @require_POST @login_required def update_timeout(request, code): - check = get_object_or_404(Check, code=code) - if check.user != request.team.user: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) kind = request.POST.get("kind") if kind == "simple": @@ -304,13 +314,7 @@ def cron_preview(request): def ping_details(request, code, n=None): - if not request.user.is_authenticated: - return HttpResponseForbidden() - - check = get_object_or_404(Check, code=code) - if check.user_id != request.team.user.id: - return HttpResponseForbidden() - + check = _get_check_for_user(request, code) q = Ping.objects.filter(owner=check) if n: q = q.filter(n=n) @@ -328,9 +332,7 @@ def ping_details(request, code, n=None): @require_POST @login_required def pause(request, code): - check = get_object_or_404(Check, code=code) - if check.user_id != request.team.user.id: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) check.status = "paused" check.save() @@ -344,12 +346,8 @@ def pause(request, code): @require_POST @login_required def remove_check(request, code): - check = get_object_or_404(Check, code=code) - if check.user != request.team.user: - return HttpResponseForbidden() - + check = _get_check_for_user(request, code) check.delete() - return redirect("hc-checks") @@ -371,11 +369,9 @@ def _get_events(check, limit): @login_required def log(request, code): - check = get_object_or_404(Check, code=code) - if check.user != request.team.user: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) - limit = request.team.ping_log_limit + limit = check.user.profile.ping_log_limit ctx = { "check": check, "events": _get_events(check, limit), @@ -388,11 +384,9 @@ def log(request, code): @login_required def details(request, code): - check = get_object_or_404(Check, code=code) - if check.user != request.team.user: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) - channels = Channel.objects.filter(user=request.team.user) + channels = Channel.objects.filter(user=check.user) channels = list(channels.order_by("created")) ctx = { @@ -407,9 +401,7 @@ def details(request, code): @login_required def status_single(request, code): - check = get_object_or_404(Check, code=code) - if check.user_id != request.team.user.id: - return HttpResponseForbidden() + check = _get_check_for_user(request, code) status = check.get_status() events = _get_events(check, 20) diff --git a/templates/base.html b/templates/base.html index 2e9eb677..922d3efb 100644 --- a/templates/base.html +++ b/templates/base.html @@ -115,7 +115,12 @@