From 012a86495f23e9cd4cc664fc3024d0849f3bd625 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Thu, 31 Aug 2017 12:16:54 +0300 Subject: [PATCH] Use client-side installation flow for installing HipChat integration. --- hc/api/models.py | 3 +- hc/api/tests/test_channel_model.py | 16 +----- hc/front/tests/test_add_hipchat.py | 45 ++++++---------- hc/front/tests/test_channels.py | 7 --- hc/front/urls.py | 1 - hc/front/views.py | 54 +++++++++---------- templates/front/channels.html | 7 +-- templates/integrations/add_hipchat.html | 47 +++------------- .../integrations/hipchat_capabilities.json | 3 +- 9 files changed, 50 insertions(+), 133 deletions(-) diff --git a/hc/api/models.py b/hc/api/models.py index 772f9095..5c4084cb 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -388,8 +388,7 @@ class Channel(models.Model): if time.time() < doc.get("expires_at", 0): return # Current access token is still valid - endpoints = requests.get(doc["capabilitiesUrl"]) - url = endpoints.json()["capabilities"]["oauth2Provider"]["tokenUrl"] + url = "https://api.hipchat.com/v2/oauth/token" auth = (doc["oauthId"], doc["oauthSecret"]) r = requests.post(url, auth=auth, data={ "grant_type": "client_credentials", diff --git a/hc/api/tests/test_channel_model.py b/hc/api/tests/test_channel_model.py index 5c228496..1e8c7451 100644 --- a/hc/api/tests/test_channel_model.py +++ b/hc/api/tests/test_channel_model.py @@ -8,28 +8,16 @@ from mock import patch class ChannelModelTestCase(BaseTestCase): @patch("hc.api.models.requests.post") - @patch("hc.api.models.requests.get") - def test_it_refreshes_hipchat_access_token(self, mock_get, mock_post): - mock_get.return_value.json.return_value = { - "capabilities": { - "oauth2Provider": {"tokenUrl": "http://example.org"} - } - } + def test_it_refreshes_hipchat_access_token(self, mock_post): mock_post.return_value.json.return_value = {"expires_in": 100} channel = Channel(kind="hipchat", user=self.alice, value=json.dumps({ "oauthId": "foo", - "oauthSecret": "bar", - "capabilitiesUrl": "http://example.org/capabilities.json" + "oauthSecret": "bar" })) channel.refresh_hipchat_access_token() - # It should fetch the remote capabilities document - mock_get.assert_called() # It should request a token using a correct tokenUrl mock_post.assert_called() - args, kwargs = mock_post.call_args - self.assertEqual(args[0], "http://example.org") - self.assertTrue("expires_at" in channel.value) diff --git a/hc/front/tests/test_add_hipchat.py b/hc/front/tests/test_add_hipchat.py index c791de75..758d6d8f 100644 --- a/hc/front/tests/test_add_hipchat.py +++ b/hc/front/tests/test_add_hipchat.py @@ -1,6 +1,3 @@ -import json - -from django.core import signing from hc.api.models import Channel from hc.test import BaseTestCase from mock import patch @@ -14,38 +11,26 @@ class AddHipChatTestCase(BaseTestCase): r = self.client.get(self.url) self.assertContains(r, "appropriate HipChat room") - def test_instructions_work_when_logged_out(self): - r = self.client.get(self.url) - self.assertContains(r, "Before adding HipChat integration, please") - - def test_it_redirects_to_addons_install(self): - self.client.login(username="alice@example.org", password="password") - r = self.client.post(self.url) - self.assertEqual(r.status_code, 302) - def test_it_returns_capabilities(self): r = self.client.get("/integrations/hipchat/capabilities/") - self.assertContains(r, "callbackUrl") + self.assertContains(r, "installedUrl") - @patch("hc.api.models.Channel.refresh_hipchat_access_token") - def test_callback_works(self, mock_refresh): - state = signing.TimestampSigner().sign("alice") - payload = json.dumps({"relayState": state, "foo": "foobar"}) + @patch("hc.front.views.Channel.refresh_hipchat_access_token") + @patch("hc.front.views.requests.get") + def test_it_adds_channel(self, mock_get, mock_refresh): + mock_get.return_value.json.return_value = { + "oauthId": "test-id" + } + mock_get.return_value.text = "{}" - r = self.client.post("/integrations/hipchat/callback/", payload, - content_type="application/json") + self.client.login(username="alice@example.org", password="password") + + s = "https://api.hipchat.com/foo" + r = self.client.post(self.url + "?installable_url=%s" % s) + self.assertEqual(r.status_code, 302) - self.assertEqual(r.status_code, 200) + self.assertTrue(mock_refresh.called) c = Channel.objects.get() self.assertEqual(c.kind, "hipchat") - self.assertTrue("foobar" in c.value) - - @patch("hc.api.models.Channel.refresh_hipchat_access_token") - def test_callback_rejects_bad_signature(self, mock_refresh): - payload = json.dumps({"relayState": "alice:bad:sig", "foo": "foobar"}) - - r = self.client.post("/integrations/hipchat/callback/", payload, - content_type="application/json") - - self.assertEqual(r.status_code, 400) + self.assertEqual(c.value, "{}") diff --git a/hc/front/tests/test_channels.py b/hc/front/tests/test_channels.py index e6f9ac73..ab194d30 100644 --- a/hc/front/tests/test_channels.py +++ b/hc/front/tests/test_channels.py @@ -47,10 +47,3 @@ class ChannelsTestCase(BaseTestCase): self.assertEqual(r.status_code, 200) self.assertContains(r, "fake-key") self.assertContains(r, "(normal priority)") - - def test_it_shows_added_message(self): - self.client.login(username="alice@example.org", password="password") - r = self.client.get("/integrations/?added=hipchat") - - self.assertEqual(r.status_code, 200) - self.assertContains(r, "The HipChat integration has been added!") diff --git a/hc/front/urls.py b/hc/front/urls.py index c9e16a9b..4b209b57 100644 --- a/hc/front/urls.py +++ b/hc/front/urls.py @@ -21,7 +21,6 @@ channel_urls = [ url(r'^add_slack_btn/$', views.add_slack_btn, name="hc-add-slack-btn"), url(r'^add_hipchat/$', views.add_hipchat, name="hc-add-hipchat"), url(r'^hipchat/capabilities/$', views.hipchat_capabilities, name="hc-hipchat-capabilities"), - url(r'^hipchat/callback/$', views.hipchat_callback, name="hc-hipchat-callback"), url(r'^add_pushbullet/$', views.add_pushbullet, name="hc-add-pushbullet"), url(r'^add_discord/$', views.add_discord, name="hc-add-discord"), url(r'^add_pushover/$', views.add_pushover, name="hc-add-pushover"), diff --git a/hc/front/views.py b/hc/front/views.py index 0fe65462..69437eb8 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -353,7 +353,6 @@ def channels(request): "enable_telegram": settings.TELEGRAM_TOKEN is not None, "enable_sms": settings.TWILIO_AUTH is not None, "enable_pd": settings.PD_VENDOR_KEY is not None, - "added": request.GET.get("added"), "use_payments": settings.USE_PAYMENTS } @@ -571,18 +570,34 @@ def add_slack_btn(request): return redirect("hc-channels") +@login_required def add_hipchat(request): - if request.method == "POST": - username = request.team.user.username - state = signing.TimestampSigner().sign(username) - capabilities = settings.SITE_ROOT + reverse("hc-hipchat-capabilities") + if "installable_url" in request.GET: + url = request.GET["installable_url"] + assert url.startswith("https://api.hipchat.com") + response = requests.get(url) + if "oauthId" not in response.json(): + messages.warning(request, "Something went wrong!") + return redirect("hc-channels") - url = "https://www.hipchat.com/addons/install?url=%s&relayState=%s" % \ - (capabilities, state) + channel = Channel(kind="hipchat") + channel.user = request.team.user + channel.value = response.text + channel.save() - return redirect(url) + channel.refresh_hipchat_access_token() + channel.assign_all_checks() + messages.success(request, "The HipChat integration has been added!") + return redirect("hc-channels") + + install_url = "https://www.hipchat.com/addons/install?" + urlencode({ + "url": settings.SITE_ROOT + reverse("hc-hipchat-capabilities") + }) - ctx = {"page": "channels"} + ctx = { + "page": "channels", + "install_url": install_url + } return render(request, "integrations/add_hipchat.html", ctx) @@ -591,27 +606,6 @@ def hipchat_capabilities(request): content_type="application/json") -@csrf_exempt -@require_POST -def hipchat_callback(request): - doc = json.loads(request.body.decode("utf-8")) - try: - signer = signing.TimestampSigner() - username = signer.unsign(doc.get("relayState"), max_age=300) - except signing.BadSignature: - return HttpResponseBadRequest() - - channel = Channel(kind="hipchat") - channel.user = User.objects.get(username=username) - channel.value = json.dumps(doc) - channel.save() - - channel.refresh_hipchat_access_token() - channel.assign_all_checks() - - return HttpResponse() - - @login_required def add_pushbullet(request): if settings.PUSHBULLET_CLIENT_ID is None: diff --git a/templates/front/channels.html b/templates/front/channels.html index 0c25d193..946093ee 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -6,16 +6,11 @@ {% block content %}
-{% if messages or added %} +{% if messages %}
{% for message in messages %}

{{ message }}

{% endfor %} - {% if added == "hipchat" %} -

- The HipChat integration has been added! -

- {% endif %}
{% endif %} diff --git a/templates/integrations/add_hipchat.html b/templates/integrations/add_hipchat.html index 3ef2c213..c59151e2 100644 --- a/templates/integrations/add_hipchat.html +++ b/templates/integrations/add_hipchat.html @@ -10,51 +10,17 @@

HipChat

- {% if request.user.is_authenticated %}

If your team uses HipChat, you can set up {% site_name %} to post status updates directly to an appropriate HipChat room.

-
- {% csrf_token %} - -
-
- - {% else %} -

- {% site_name %} is a free and - open source - service for monitoring your cron jobs, background processes and - scheduled tasks. Before adding HipChat integration, please log into - {% site_name %}:

- -
-
{% csrf_token %} - -
-
-
@
- -
-
-
- -
-
+ + HipChat + Connect HipChat +
- {% endif %}

Setup Guide

@@ -63,8 +29,8 @@
1

- After {% if request.user.is_authenticated %}{% else %}logging in and{% endif %} - clicking on "Install HipChat Integration", you will be + After + clicking on "Connect HipChat", you will be asked to log into HipChat.

@@ -125,7 +91,6 @@ src="{% static 'img/integrations/setup_hipchat_4.png' %}">
- {% endblock %} diff --git a/templates/integrations/hipchat_capabilities.json b/templates/integrations/hipchat_capabilities.json index cceabd84..b462f7fd 100644 --- a/templates/integrations/hipchat_capabilities.json +++ b/templates/integrations/hipchat_capabilities.json @@ -11,8 +11,7 @@ "installable": { "allowGlobal": false, "allowRoom": true, - "callbackUrl": "{% site_root %}{% url 'hc-hipchat-callback'%}", - "installedUrl": "{% site_root %}{% url 'hc-channels'%}?added=hipchat" + "installedUrl": "{% site_root %}{% url 'hc-add-hipchat'%}" }, "hipchatApiConsumer": { "avatar": "{% site_root %}{% static 'img/logo-512-green.png' %}",