From 432e592e44116587c28df996b7d67f19f5f89991 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Mon, 29 Oct 2018 21:44:34 +0200 Subject: [PATCH] Add read-only API key support --- CHANGELOG.md | 2 +- .../management/commands/createreadonlykeys.py | 20 ++++++ .../migrations/0015_auto_20181029_1858.py | 23 ++++++ hc/accounts/models.py | 6 +- hc/accounts/tests/test_profile.py | 8 ++- hc/accounts/views.py | 20 +++--- hc/api/decorators.py | 24 ++++++- hc/api/migrations/0042_auto_20181029_1522.py | 18 +++++ hc/api/tests/test_create_check.py | 7 ++ hc/api/tests/test_list_checks.py | 7 ++ hc/api/views.py | 70 +++++++++++-------- templates/accounts/profile.html | 41 ++++++----- 12 files changed, 185 insertions(+), 61 deletions(-) create mode 100644 hc/accounts/management/commands/createreadonlykeys.py create mode 100644 hc/accounts/migrations/0015_auto_20181029_1858.py create mode 100644 hc/api/migrations/0042_auto_20181029_1522.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 72fe5c73..c37fab28 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ All notable changes to this project will be documented in this file. - Add "List-Unsubscribe" header to alert and report emails - Don't send monthly reports to inactive accounts (no pings in 6 months) - Add search box in the "My Checks" page -- Refactor API key checking code +- Add read-only API key support ### Bug Fixes - During DST transition, handle ambiguous dates as pre-transition diff --git a/hc/accounts/management/commands/createreadonlykeys.py b/hc/accounts/management/commands/createreadonlykeys.py new file mode 100644 index 00000000..c7640975 --- /dev/null +++ b/hc/accounts/management/commands/createreadonlykeys.py @@ -0,0 +1,20 @@ +from base64 import urlsafe_b64encode +import os + +from django.core.management.base import BaseCommand + +from hc.accounts.models import Profile + + +class Command(BaseCommand): + help = """Create read-only API keys.""" + + def handle(self, *args, **options): + c = 0 + q = Profile.objects.filter(api_key_readonly="").exclude(api_key="") + for profile in q: + profile.api_key_readonly = urlsafe_b64encode(os.urandom(24)).decode() + profile.save() + c += 1 + + return "Done! Generated %d readonly keys." % c diff --git a/hc/accounts/migrations/0015_auto_20181029_1858.py b/hc/accounts/migrations/0015_auto_20181029_1858.py new file mode 100644 index 00000000..ae8327cc --- /dev/null +++ b/hc/accounts/migrations/0015_auto_20181029_1858.py @@ -0,0 +1,23 @@ +# Generated by Django 2.1.2 on 2018-10-29 18:58 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0014_auto_20171227_1530'), + ] + + operations = [ + migrations.AddField( + model_name='profile', + name='api_key_id', + field=models.CharField(blank=True, max_length=128), + ), + migrations.AddField( + model_name='profile', + name='api_key_readonly', + field=models.CharField(blank=True, max_length=128), + ), + ] diff --git a/hc/accounts/models.py b/hc/accounts/models.py index 6d051f01..ce588bd6 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -50,7 +50,9 @@ class Profile(models.Model): ping_log_limit = models.IntegerField(default=100) check_limit = models.IntegerField(default=20) token = models.CharField(max_length=128, blank=True) + api_key_id = models.CharField(max_length=128, blank=True) api_key = models.CharField(max_length=128, blank=True) + api_key_readonly = models.CharField(max_length=128, blank=True) current_team = models.ForeignKey("self", models.SET_NULL, null=True) bill_to = models.TextField(blank=True) last_sms_date = models.DateTimeField(null=True, blank=True) @@ -117,8 +119,10 @@ class Profile(models.Model): } emails.change_email(self.user.email, ctx) - def set_api_key(self): + def set_api_keys(self, key_id=""): + self.api_key_id = key_id self.api_key = urlsafe_b64encode(os.urandom(24)).decode() + self.api_key_readonly = urlsafe_b64encode(os.urandom(24)).decode() self.save() def checks_from_all_teams(self): diff --git a/hc/accounts/tests/test_profile.py b/hc/accounts/tests/test_profile.py index f5ea306c..387e4e97 100644 --- a/hc/accounts/tests/test_profile.py +++ b/hc/accounts/tests/test_profile.py @@ -30,7 +30,7 @@ class ProfileTestCase(BaseTestCase): def test_it_creates_api_key(self): self.client.login(username="alice@example.org", password="password") - form = {"create_api_key": "1"} + form = {"create_api_keys": "1"} r = self.client.post("/accounts/profile/", form) self.assertEqual(r.status_code, 200) @@ -40,14 +40,18 @@ class ProfileTestCase(BaseTestCase): self.assertFalse("b'" in api_key) def test_it_revokes_api_key(self): + self.profile.api_key_readonly = "R" * 32 + self.profile.save() + self.client.login(username="alice@example.org", password="password") - form = {"revoke_api_key": "1"} + form = {"revoke_api_keys": "1"} r = self.client.post("/accounts/profile/", form) assert r.status_code == 200 self.profile.refresh_from_db() self.assertEqual(self.profile.api_key, "") + self.assertEqual(self.profile.api_key_readonly, "") def test_it_sends_report(self): check = Check(name="Test Check", user=self.alice) diff --git a/hc/accounts/views.py b/hc/accounts/views.py index 3eeb51ac..c64373e0 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -153,7 +153,7 @@ def profile(request): ctx = { "page": "profile", "profile": profile, - "show_api_key": False, + "show_api_keys": False, "api_status": "default", "team_status": "default" } @@ -165,18 +165,20 @@ def profile(request): elif "set_password" in request.POST: profile.send_set_password_link() return redirect("hc-link-sent") - elif "create_api_key" in request.POST: - profile.set_api_key() - ctx["show_api_key"] = True - ctx["api_key_created"] = True + elif "create_api_keys" in request.POST: + profile.set_api_keys() + ctx["show_api_keys"] = True + ctx["api_keys_created"] = True ctx["api_status"] = "success" - elif "revoke_api_key" in request.POST: + elif "revoke_api_keys" in request.POST: + profile.api_key_id = "" profile.api_key = "" + profile.api_key_readonly = "" profile.save() - ctx["api_key_revoked"] = True + ctx["api_keys_revoked"] = True ctx["api_status"] = "info" - elif "show_api_key" in request.POST: - ctx["show_api_key"] = True + elif "show_api_keys" in request.POST: + ctx["show_api_keys"] = True elif "invite_team_member" in request.POST: if not profile.can_invite(): return HttpResponseForbidden() diff --git a/hc/api/decorators.py b/hc/api/decorators.py index 577bf48e..71dd4130 100644 --- a/hc/api/decorators.py +++ b/hc/api/decorators.py @@ -2,6 +2,7 @@ import json from functools import wraps from django.contrib.auth.models import User +from django.db.models import Q from django.http import JsonResponse from hc.lib.jsonschema import ValidationError, validate @@ -10,7 +11,7 @@ def error(msg, status=400): return JsonResponse({"error": msg}, status=status) -def check_api_key(f): +def authorize(f): @wraps(f) def wrapper(request, *args, **kwds): if "HTTP_X_API_KEY" in request.META: @@ -27,7 +28,28 @@ def check_api_key(f): return error("wrong api key", 401) return f(request, *args, **kwds) + return wrapper + + +def authorize_read(f): + @wraps(f) + def wrapper(request, *args, **kwds): + if "HTTP_X_API_KEY" in request.META: + api_key = request.META["HTTP_X_API_KEY"] + else: + api_key = str(request.json.get("api_key", "")) + if len(api_key) != 32: + return error("missing api key", 401) + + write_key_match = Q(profile__api_key=api_key) + read_key_match = Q(profile__api_key_readonly=api_key) + try: + request.user = User.objects.get(write_key_match | read_key_match) + except User.DoesNotExist: + return error("wrong api key", 401) + + return f(request, *args, **kwds) return wrapper diff --git a/hc/api/migrations/0042_auto_20181029_1522.py b/hc/api/migrations/0042_auto_20181029_1522.py new file mode 100644 index 00000000..d3322866 --- /dev/null +++ b/hc/api/migrations/0042_auto_20181029_1522.py @@ -0,0 +1,18 @@ +# Generated by Django 2.1.2 on 2018-10-29 15:22 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('api', '0041_check_desc'), + ] + + operations = [ + migrations.AlterField( + model_name='channel', + name='kind', + field=models.CharField(choices=[('email', 'Email'), ('webhook', 'Webhook'), ('hipchat', 'HipChat'), ('slack', 'Slack'), ('pd', 'PagerDuty'), ('pagertree', 'PagerTree'), ('po', 'Pushover'), ('pushbullet', 'Pushbullet'), ('opsgenie', 'OpsGenie'), ('victorops', 'VictorOps'), ('discord', 'Discord'), ('telegram', 'Telegram'), ('sms', 'SMS'), ('zendesk', 'Zendesk'), ('trello', 'Trello')], max_length=20), + ), + ] diff --git a/hc/api/tests/test_create_check.py b/hc/api/tests/test_create_check.py index f3bd03b7..00bf10e7 100644 --- a/hc/api/tests/test_create_check.py +++ b/hc/api/tests/test_create_check.py @@ -198,3 +198,10 @@ class CreateCheckTestCase(BaseTestCase): r = self.post({"api_key": "X" * 32}) self.assertEqual(r.status_code, 403) + + def test_readonly_key_does_not_work(self): + self.profile.api_key_readonly = "R" * 32 + self.profile.save() + + r = self.post({"api_key": "R" * 32, "name": "Foo"}) + self.assertEqual(r.status_code, 401) diff --git a/hc/api/tests/test_list_checks.py b/hc/api/tests/test_list_checks.py index d1557608..5ed922dc 100644 --- a/hc/api/tests/test_list_checks.py +++ b/hc/api/tests/test_list_checks.py @@ -123,3 +123,10 @@ class ListChecksTestCase(BaseTestCase): doc = r.json() self.assertTrue("checks" in doc) self.assertEqual(len(doc["checks"]), 0) + + def test_readonly_key_works(self): + self.profile.api_key_readonly = "R" * 32 + self.profile.save() + + r = self.client.get("/api/v1/checks/", HTTP_X_API_KEY="R" * 32) + self.assertEqual(r.status_code, 200) diff --git a/hc/api/views.py b/hc/api/views.py index b2277975..742f686d 100644 --- a/hc/api/views.py +++ b/hc/api/views.py @@ -11,7 +11,7 @@ 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, validate_json +from hc.api.decorators import authorize, authorize_read, validate_json from hc.api.models import Check, Notification from hc.lib.badges import check_signature, get_badge_svg @@ -87,48 +87,56 @@ def _update(check, spec): return check -@csrf_exempt -@validate_json(schemas.check) -@check_api_key -def checks(request): - if request.method == "GET": - q = Check.objects.filter(user=request.user) +@validate_json() +@authorize_read +def get_checks(request): + q = Check.objects.filter(user=request.user) - tags = set(request.GET.getlist("tag")) - for tag in tags: - # approximate filtering by tags - q = q.filter(tags__contains=tag) + tags = set(request.GET.getlist("tag")) + for tag in tags: + # approximate filtering by tags + q = q.filter(tags__contains=tag) - checks = [] - for check in q: - # precise, final filtering - if not tags or check.matches_tag_set(tags): - checks.append(check.to_dict()) + checks = [] + for check in q: + # precise, final filtering + if not tags or check.matches_tag_set(tags): + checks.append(check.to_dict()) - return JsonResponse({"checks": checks}) + return JsonResponse({"checks": checks}) - elif request.method == "POST": - created = False - check = _lookup(request.user, request.json) - if check is None: - num_checks = Check.objects.filter(user=request.user).count() - if num_checks >= request.user.profile.check_limit: - return HttpResponseForbidden() - check = Check(user=request.user) - created = True +@validate_json(schemas.check) +@authorize +def create_check(request): + created = False + check = _lookup(request.user, request.json) + if check is None: + num_checks = Check.objects.filter(user=request.user).count() + if num_checks >= request.user.profile.check_limit: + return HttpResponseForbidden() - _update(check, request.json) + check = Check(user=request.user) + created = True + + _update(check, request.json) + return JsonResponse(check.to_dict(), status=201 if created else 200) + + +@csrf_exempt +def checks(request): + if request.method == "GET": + return get_checks(request) - return JsonResponse(check.to_dict(), status=201 if created else 200) + elif request.method == "POST": + return create_check(request) - # If request is neither GET nor POST, return "405 Method not allowed" return HttpResponse(status=405) @csrf_exempt @validate_json(schemas.check) -@check_api_key +@authorize def update(request, code): check = get_object_or_404(Check, code=code) if check.user != request.user: @@ -150,7 +158,7 @@ def update(request, code): @csrf_exempt @require_POST @validate_json() -@check_api_key +@authorize def pause(request, code): check = get_object_or_404(Check, code=code) if check.user != request.user: diff --git a/templates/accounts/profile.html b/templates/accounts/profile.html index a59d5f9d..3413a9c2 100644 --- a/templates/accounts/profile.html +++ b/templates/accounts/profile.html @@ -61,8 +61,17 @@

API Access

{% if profile.api_key %} - {% if show_api_key %} - API key: {{ profile.api_key }} + {% if show_api_keys %} +

+ API key:
+ {{ profile.api_key }} +

+ {% if profile.api_key_readonly %} +

+ API key (read-only):
+ {{ profile.api_key_readonly }} +

+ {% endif %} + name="show_api_keys" + class="btn btn-default pull-right">Show API keys {% endif %} {% else %} @@ -87,21 +96,21 @@ {% csrf_token %} + name="create_api_keys" + class="btn btn-default pull-right">Create API keys {% endif %}
- {% if api_key_created %} + {% if api_keys_created %}
- API key created + API keys created
{% endif %} - {% if api_key_revoked %} + {% if api_keys_revoked %}
- API key revoked + API keys revoked
{% endif %} @@ -208,13 +217,13 @@
-

Revoke API Key?

+

Revoke API Keys?

-

You are about to revoke the current API key.

-

Afterwards, you can create a new API key, but there will +

You are about to revoke your current API keys.

+

Afterwards, you can create new API keys, but there will be no way of getting the current API - key back. + keys back.

Are you sure?

@@ -222,8 +231,8 @@ + name="revoke_api_keys" + class="btn btn-danger">Revoke API Keys