From 3dfdbc09ca0a6615b24935ad657fa66674e55a52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 9 Sep 2021 14:55:17 +0300 Subject: [PATCH] Add ability to create/revoke individual keys --- CHANGELOG.md | 1 + hc/accounts/models.py | 12 -- hc/accounts/tests/test_project.py | 43 ++++--- hc/accounts/views.py | 39 ++++-- hc/api/urls.py | 2 +- hc/front/templatetags/hc_extras.py | 5 + static/css/project.css | 4 + static/js/project.js | 12 ++ templates/accounts/project.html | 190 ++++++++++++++++++----------- templates/base.html | 1 + 10 files changed, 196 insertions(+), 113 deletions(-) create mode 100644 static/css/project.css diff --git a/CHANGELOG.md b/CHANGELOG.md index 35c554ca..f6af79b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this project will be documented in this file. ### Improvements - Add /api/v1/badges/ endpoint (#552) - Add ability to edit existing email, Signal, SMS, WhatsApp integrations +- Add new ping URL format: /{ping_key}/{slug} (#491) ### Bug Fixes - Add handling for non-latin-1 characters in webhook headers diff --git a/hc/accounts/models.py b/hc/accounts/models.py index a615db8e..7886e0ef 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -338,18 +338,6 @@ class Project(models.Model): def num_checks_available(self): return self.owner_profile.num_checks_available() - def set_api_keys(self): - def pick(nbytes=24): - while True: - candidate = token_urlsafe(nbytes) - if candidate[0] not in "-_" and candidate[-1] not in "-_": - return candidate - - self.api_key = pick() - self.api_key_readonly = pick() - self.ping_key = pick(16) - self.save() - def invite_suggestions(self): q = User.objects.filter(memberships__project__owner_id=self.owner_id) q = q.exclude(memberships__project=self) diff --git a/hc/accounts/tests/test_project.py b/hc/accounts/tests/test_project.py index aaeed31c..3445d4fd 100644 --- a/hc/accounts/tests/test_project.py +++ b/hc/accounts/tests/test_project.py @@ -23,58 +23,69 @@ class ProjectTestCase(BaseTestCase): r = self.client.get(self.url) self.assertContains(r, "Change Project Name") - def test_it_shows_api_keys(self): + def test_it_masks_keys_by_default(self): self.project.api_key_readonly = "R" * 32 + self.project.ping_key = "P" * 22 self.project.save() self.client.login(username="alice@example.org", password="password") - form = {"show_api_keys": "1"} + r = self.client.get(self.url) + self.assertEqual(r.status_code, 200) + + self.assertNotContains(r, "X" * 32) + self.assertNotContains(r, "R" * 32) + self.assertNotContains(r, "P" * 22) + + def test_it_shows_keys(self): + self.project.api_key_readonly = "R" * 32 + self.project.ping_key = "P" * 22 + self.project.save() + + self.client.login(username="alice@example.org", password="password") + + form = {"show_keys": "1"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) self.assertContains(r, "X" * 32) self.assertContains(r, "R" * 32) + self.assertContains(r, "P" * 22) self.assertContains(r, "Prometheus metrics endpoint") - def test_it_creates_api_key(self): + def test_it_creates_readonly_key(self): self.client.login(username="alice@example.org", password="password") - form = {"create_api_keys": "1"} + form = {"create_key": "api_key_readonly"} r = self.client.post(self.url, form) self.assertEqual(r.status_code, 200) self.project.refresh_from_db() - api_key = self.project.api_key - self.assertTrue(len(api_key) > 10) - self.assertFalse("b'" in api_key) + self.assertEqual(len(self.project.api_key_readonly), 32) + self.assertFalse("b'" in self.project.api_key_readonly) - def test_it_requires_rw_access_to_create_api_key(self): + def test_it_requires_rw_access_to_create_key(self): self.bobs_membership.role = "r" self.bobs_membership.save() self.client.login(username="bob@example.org", password="password") - r = self.client.post(self.url, {"create_api_keys": "1"}) + r = self.client.post(self.url, {"create_key": "api_key_readonly"}) self.assertEqual(r.status_code, 403) def test_it_revokes_api_key(self): - self.project.api_key_readonly = "R" * 32 - self.project.save() - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url, {"revoke_api_keys": "1"}) + r = self.client.post(self.url, {"revoke_key": "api_key"}) self.assertEqual(r.status_code, 200) self.project.refresh_from_db() self.assertEqual(self.project.api_key, "") - self.assertEqual(self.project.api_key_readonly, "") def test_it_requires_rw_access_to_revoke_api_key(self): self.bobs_membership.role = "r" self.bobs_membership.save() self.client.login(username="bob@example.org", password="password") - r = self.client.post(self.url, {"revoke_api_keys": "1"}) + r = self.client.post(self.url, {"revoke_key": "api_key"}) self.assertEqual(r.status_code, 403) def test_it_adds_team_member(self): @@ -348,5 +359,5 @@ class ProjectTestCase(BaseTestCase): self.bobs_membership.save() self.client.login(username="bob@example.org", password="password") - r = self.client.post(self.url, {"show_api_keys": "1"}) + r = self.client.post(self.url, {"show_keys": "1"}) self.assertEqual(r.status_code, 403) diff --git a/hc/accounts/views.py b/hc/accounts/views.py index c094f7b9..35f1b493 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -1,6 +1,6 @@ import base64 from datetime import timedelta as td -from secrets import token_bytes +from secrets import token_bytes, token_urlsafe from urllib.parse import urlparse import time import uuid @@ -139,6 +139,13 @@ def _check_2fa(request, user): return _redirect_after_login(request) +def _new_key(nbytes=24): + while True: + candidate = token_urlsafe(nbytes) + if candidate[0] not in "-_" and candidate[-1] not in "-_": + return candidate + + def login(request): form = forms.PasswordLoginForm() magic_form = forms.EmailLoginForm() @@ -320,32 +327,40 @@ def project(request, code): } if request.method == "POST": - if "create_api_keys" in request.POST: + if "create_key" in request.POST: if not rw: return HttpResponseForbidden() - project.set_api_keys() + if request.POST["create_key"] == "api_key": + project.api_key = _new_key(24) + elif request.POST["create_key"] == "api_key_readonly": + project.api_key_readonly = _new_key(24) + elif request.POST["create_key"] == "ping_key": + project.ping_key = _new_key(16) project.save() - ctx["show_api_keys"] = True - ctx["api_keys_created"] = True + ctx["key_created"] = True ctx["api_status"] = "success" - elif "revoke_api_keys" in request.POST: + ctx["show_keys"] = True + elif "revoke_key" in request.POST: if not rw: return HttpResponseForbidden() - project.api_key = "" - project.api_key_readonly = "" - project.ping_key = None + if request.POST["revoke_key"] == "api_key": + project.api_key = "" + elif request.POST["revoke_key"] == "api_key_readonly": + project.api_key_readonly = "" + elif request.POST["revoke_key"] == "ping_key": + project.ping_key = None project.save() - ctx["api_keys_revoked"] = True + ctx["key_revoked"] = True ctx["api_status"] = "info" - elif "show_api_keys" in request.POST: + elif "show_keys" in request.POST: if not rw: return HttpResponseForbidden() - ctx["show_api_keys"] = True + ctx["show_keys"] = True elif "invite_team_member" in request.POST: if not is_manager: return HttpResponseForbidden() diff --git a/hc/api/urls.py b/hc/api/urls.py index 13fb9c66..71d1ce00 100644 --- a/hc/api/urls.py +++ b/hc/api/urls.py @@ -28,7 +28,7 @@ register_converter(QuoteConverter, "quoted") register_converter(SHA1Converter, "sha1") uuid_urls = [ - path("", views.ping, name="hc-ping"), + path("", views.ping), path("fail", views.ping, {"action": "fail"}), path("start", views.ping, {"action": "start"}), path("", views.ping), diff --git a/hc/front/templatetags/hc_extras.py b/hc/front/templatetags/hc_extras.py index 2d7b7d53..18220ef4 100644 --- a/hc/front/templatetags/hc_extras.py +++ b/hc/front/templatetags/hc_extras.py @@ -227,3 +227,8 @@ def format_ping_endpoint(ping_url): assert ping_url.startswith(settings.PING_ENDPOINT) tail = ping_url[len(settings.PING_ENDPOINT) :] return mark_safe(FORMATTED_PING_ENDPOINT + escape(tail)) + + +@register.filter +def mask_key(key): + return key[:4] + "*" * len(key[4:]) diff --git a/static/css/project.css b/static/css/project.css new file mode 100644 index 00000000..cf83e57e --- /dev/null +++ b/static/css/project.css @@ -0,0 +1,4 @@ +#api-keys .not-set { + color: var(--text-muted); + font-style: italic; +} \ No newline at end of file diff --git a/static/js/project.js b/static/js/project.js index 45c865e4..7a765a94 100644 --- a/static/js/project.js +++ b/static/js/project.js @@ -30,4 +30,16 @@ $(function() { $("#transfer-confirm").prop("disabled", !this.value); }); + $("a[data-revoke-key]").click(function() { + $("#revoke-key-type").val(this.dataset.revokeKey); + $("#revoke-key-modal .name").text(this.dataset.name); + $("#revoke-key-modal").modal("show"); + }) + + $("a[data-create-key]").click(function() { + $("#create-key-type").val(this.dataset.createKey); + $("#create-key-form").submit(); + }) + + }); diff --git a/templates/accounts/project.html b/templates/accounts/project.html index 7ca8f6e7..26975531 100644 --- a/templates/accounts/project.html +++ b/templates/accounts/project.html @@ -75,86 +75,124 @@ {% endif %} + {% if rw %}

API Access

- {% if project.api_key %} - {% if show_api_keys %} -

- API key:
- 

{{ project.api_key }}
-

- {% if project.api_key_readonly %} -

- API key (read-only):
- 

{{ project.api_key_readonly }}
-

- {% endif %} - {% if project.ping_key %} -

- Ping key:
- 

{{ project.ping_key }}
-

- {% endif %} -

See also:

-
    -
  • API documentation
    • + + + + + + + + + + + + + + + + +
    API key + {% if project.api_key %} + {% if show_keys %} + {{ project.api_key }} + {% else %} + {{ project.api_key|mask_key }} + {% endif %} + {% else %} + not set + {% endif %} + + {% if project.api_key %} + Revoke + {% else %} + Create + {% endif %} +
    API key (read-only) {% if project.api_key_readonly %} - {% if enable_prometheus %} -
  • - Prometheus metrics endpoint -
    • + {% if show_keys %} + {{ project.api_key_readonly }} + {% else %} + {{ project.api_key_readonly|mask_key }} {% endif %} -
  • - Read-only dashboard - (security considerations) -
    • + {% else %} + not set {% endif %} - - - - {% else %} -
    - - API access is enabled. - {% csrf_token %} - - {% if rw %} - +
    		+ {% if project.api_key_readonly %} + Revoke + {% else %} + Create {% endif %} - +
    Ping key + {% if project.ping_key %} + {% if show_keys %} + {{ project.ping_key }} + {% else %} + {{ project.ping_key|mask_key }} + {% endif %} + {% else %} + not set + {% endif %} + + {% if project.ping_key %} + Revoke + {% else %} + Create + {% endif %} +
    + +

    See also:

    + + + {% if not show_keys %} + {% if project.api_key or project.api_key_readonly or project.ping_key %} +
    + {% csrf_token %} + +
    + {% endif %} {% endif %}
- {% if api_keys_created %} + {% if key_created %}
- API keys created + Key created
{% endif %} - {% if api_keys_revoked %} + {% if key_revoked %}
- API keys revoked + Key revoked
{% endif %}
+ {% endif %} {% with invite_suggestions=project.invite_suggestions %}
@@ -320,29 +358,37 @@
-