From acce0808cea56306b75978924f8d15d5532bfcf1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Tue, 25 Feb 2020 14:22:34 +0200 Subject: [PATCH] Project code in URL for the "Add Slack" page. cc: #336 --- hc/accounts/tests/test_check_token.py | 4 +- hc/accounts/tests/test_login.py | 6 +- hc/accounts/views.py | 1 + hc/front/tests/test_add_slack.py | 17 ++-- hc/front/tests/test_add_slack_btn.py | 82 +++-------------- hc/front/tests/test_add_slack_complete.py | 69 +++++++++++++++ hc/front/tests/test_add_slack_help.py | 9 ++ hc/front/urls.py | 6 +- hc/front/views.py | 102 +++++++++++++++------- templates/front/channels.html | 6 +- templates/integrations/add_slack.html | 92 +------------------ templates/integrations/add_slack_btn.html | 97 ++++++++++++++++++++ 12 files changed, 279 insertions(+), 212 deletions(-) create mode 100644 hc/front/tests/test_add_slack_complete.py create mode 100644 hc/front/tests/test_add_slack_help.py create mode 100644 templates/integrations/add_slack_btn.html diff --git a/hc/accounts/tests/test_check_token.py b/hc/accounts/tests/test_check_token.py index 5e38593e..2e0dc002 100644 --- a/hc/accounts/tests/test_check_token.py +++ b/hc/accounts/tests/test_check_token.py @@ -40,9 +40,9 @@ class CheckTokenTestCase(BaseTestCase): self.assertContains(r, "incorrect or expired") def test_it_handles_next_parameter(self): - url = "/accounts/check_token/alice/secret-token/?next=/integrations/add_slack/" + url = "/accounts/check_token/alice/secret-token/?next=" + self.channels_url r = self.client.post(url) - self.assertRedirects(r, "/integrations/add_slack/") + self.assertRedirects(r, self.channels_url) def test_it_ignores_bad_next_parameter(self): url = "/accounts/check_token/alice/secret-token/?next=/evil/" diff --git a/hc/accounts/tests/test_login.py b/hc/accounts/tests/test_login.py index 7e47a8d5..1ae2c9ae 100644 --- a/hc/accounts/tests/test_login.py +++ b/hc/accounts/tests/test_login.py @@ -24,14 +24,14 @@ class LoginTestCase(BaseTestCase): def test_it_sends_link_with_next(self): form = {"identity": "alice@example.org"} - r = self.client.post("/accounts/login/?next=/integrations/add_slack/", form) + r = self.client.post("/accounts/login/?next=" + self.channels_url, form) self.assertRedirects(r, "/accounts/login_link_sent/") self.assertIn("auto-login", r.cookies) # The check_token link should have a ?next= query parameter: self.assertEqual(len(mail.outbox), 1) body = mail.outbox[0].body - self.assertTrue("/?next=/integrations/add_slack/" in body) + self.assertTrue("/?next=" + self.channels_url in body) @override_settings(SECRET_KEY="test-secret") def test_it_rate_limits_emails(self): @@ -85,7 +85,7 @@ class LoginTestCase(BaseTestCase): form = {"action": "login", "email": "alice@example.org", "password": "password"} - samples = ["/integrations/add_slack/", "/checks/%s/details/" % check.code] + samples = [self.channels_url, "/checks/%s/details/" % check.code] for s in samples: r = self.client.post("/accounts/login/?next=%s" % s, form) diff --git a/hc/accounts/views.py b/hc/accounts/views.py index b1ceb588..fb1d1c79 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -40,6 +40,7 @@ NEXT_WHITELIST = ( "hc-details", "hc-log", "hc-channels", + "hc-p-channels", "hc-add-slack", "hc-add-pushover", ) diff --git a/hc/front/tests/test_add_slack.py b/hc/front/tests/test_add_slack.py index 18b3ec6b..7cd4e9ff 100644 --- a/hc/front/tests/test_add_slack.py +++ b/hc/front/tests/test_add_slack.py @@ -1,33 +1,32 @@ -from django.test.utils import override_settings - from hc.api.models import Channel from hc.test import BaseTestCase class AddSlackTestCase(BaseTestCase): - @override_settings(SLACK_CLIENT_ID=None) + def setUp(self): + super(AddSlackTestCase, self).setUp() + self.url = "/projects/%s/add_slack/" % self.project.code + def test_instructions_work(self): self.client.login(username="alice@example.org", password="password") - r = self.client.get("/integrations/add_slack/") + r = self.client.get(self.url) self.assertContains(r, "Integration Settings", status_code=200) - @override_settings(SLACK_CLIENT_ID=None) def test_it_works(self): form = {"value": "http://example.org"} self.client.login(username="alice@example.org", password="password") - r = self.client.post("/integrations/add_slack/", form) - self.assertRedirects(r, "/integrations/") + r = self.client.post(self.url, form) + self.assertRedirects(r, self.channels_url) c = Channel.objects.get() self.assertEqual(c.kind, "slack") self.assertEqual(c.value, "http://example.org") self.assertEqual(c.project, self.project) - @override_settings(SLACK_CLIENT_ID=None) def test_it_rejects_bad_url(self): form = {"value": "not an URL"} self.client.login(username="alice@example.org", password="password") - r = self.client.post("/integrations/add_slack/", form) + r = self.client.post(self.url, form) self.assertContains(r, "Enter a valid URL") diff --git a/hc/front/tests/test_add_slack_btn.py b/hc/front/tests/test_add_slack_btn.py index 8b724cdf..b28a6c4d 100644 --- a/hc/front/tests/test_add_slack_btn.py +++ b/hc/front/tests/test_add_slack_btn.py @@ -1,84 +1,22 @@ -import json - from django.test.utils import override_settings -from hc.api.models import Channel from hc.test import BaseTestCase -from mock import patch +@override_settings(SLACK_CLIENT_ID="fake-client-id") class AddSlackBtnTestCase(BaseTestCase): - @override_settings(SLACK_CLIENT_ID="foo") - def test_it_prepares_login_link(self): - r = self.client.get("/integrations/add_slack/") - self.assertContains(r, "Before adding Slack integration", status_code=200) + def setUp(self): + super(AddSlackBtnTestCase, self).setUp() + self.url = "/projects/%s/add_slack_btn/" % self.project.code - self.assertContains(r, "?next=/integrations/add_slack/") + def test_instructions_work(self): + self.client.login(username="alice@example.org", password="password") + r = self.client.get(self.url) + self.assertContains(r, "Setup Guide", status_code=200) - @override_settings(SLACK_CLIENT_ID="foo") def test_slack_button(self): self.client.login(username="alice@example.org", password="password") - r = self.client.get("/integrations/add_slack/") + r = self.client.get(self.url) self.assertContains(r, "slack.com/oauth/authorize", status_code=200) # There should now be a key in session - self.assertTrue("slack" in self.client.session) - - @patch("hc.front.views.requests.post") - def test_it_handles_oauth_response(self, mock_post): - session = self.client.session - session["slack"] = "foo" - session.save() - - oauth_response = { - "ok": True, - "team_name": "foo", - "incoming_webhook": {"url": "http://example.org", "channel": "bar"}, - } - - mock_post.return_value.text = json.dumps(oauth_response) - mock_post.return_value.json.return_value = oauth_response - - url = "/integrations/add_slack_btn/?code=12345678&state=foo" - - self.client.login(username="alice@example.org", password="password") - r = self.client.get(url, follow=True) - self.assertRedirects(r, "/integrations/") - self.assertContains(r, "The Slack integration has been added!") - - ch = Channel.objects.get() - self.assertEqual(ch.slack_team, "foo") - self.assertEqual(ch.slack_channel, "bar") - self.assertEqual(ch.slack_webhook_url, "http://example.org") - self.assertEqual(ch.project, self.project) - - # Session should now be clean - self.assertFalse("slack" in self.client.session) - - def test_it_avoids_csrf(self): - session = self.client.session - session["slack"] = "foo" - session.save() - - url = "/integrations/add_slack_btn/?code=12345678&state=bar" - - self.client.login(username="alice@example.org", password="password") - r = self.client.get(url) - self.assertEqual(r.status_code, 400) - - @patch("hc.front.views.requests.post") - def test_it_handles_oauth_error(self, mock_post): - session = self.client.session - session["slack"] = "foo" - session.save() - - oauth_response = {"ok": False, "error": "something went wrong"} - - mock_post.return_value.text = json.dumps(oauth_response) - mock_post.return_value.json.return_value = oauth_response - - url = "/integrations/add_slack_btn/?code=12345678&state=foo" - - self.client.login(username="alice@example.org", password="password") - r = self.client.get(url, follow=True) - self.assertRedirects(r, "/integrations/") - self.assertContains(r, "something went wrong") + self.assertTrue("add_slack" in self.client.session) diff --git a/hc/front/tests/test_add_slack_complete.py b/hc/front/tests/test_add_slack_complete.py new file mode 100644 index 00000000..6b124eae --- /dev/null +++ b/hc/front/tests/test_add_slack_complete.py @@ -0,0 +1,69 @@ +import json + +from django.test.utils import override_settings +from hc.api.models import Channel +from hc.test import BaseTestCase +from mock import patch + + +@override_settings(SLACK_CLIENT_ID="fake-client-id") +class AddSlackCompleteTestCase(BaseTestCase): + @patch("hc.front.views.requests.post") + def test_it_handles_oauth_response(self, mock_post): + session = self.client.session + session["add_slack"] = ("foo", str(self.project.code)) + session.save() + + oauth_response = { + "ok": True, + "team_name": "foo", + "incoming_webhook": {"url": "http://example.org", "channel": "bar"}, + } + + mock_post.return_value.text = json.dumps(oauth_response) + mock_post.return_value.json.return_value = oauth_response + + url = "/integrations/add_slack_btn/?code=12345678&state=foo" + + self.client.login(username="alice@example.org", password="password") + r = self.client.get(url, follow=True) + self.assertRedirects(r, self.channels_url) + self.assertContains(r, "The Slack integration has been added!") + + ch = Channel.objects.get() + self.assertEqual(ch.slack_team, "foo") + self.assertEqual(ch.slack_channel, "bar") + self.assertEqual(ch.slack_webhook_url, "http://example.org") + self.assertEqual(ch.project, self.project) + + # Session should now be clean + self.assertFalse("add_slack" in self.client.session) + + def test_it_avoids_csrf(self): + session = self.client.session + session["add_slack"] = ("foo", str(self.project.code)) + session.save() + + url = "/integrations/add_slack_btn/?code=12345678&state=bar" + + self.client.login(username="alice@example.org", password="password") + r = self.client.get(url) + self.assertEqual(r.status_code, 403) + + @patch("hc.front.views.requests.post") + def test_it_handles_oauth_error(self, mock_post): + session = self.client.session + session["add_slack"] = ("foo", str(self.project.code)) + session.save() + + oauth_response = {"ok": False, "error": "something went wrong"} + + mock_post.return_value.text = json.dumps(oauth_response) + mock_post.return_value.json.return_value = oauth_response + + url = "/integrations/add_slack_btn/?code=12345678&state=foo" + + self.client.login(username="alice@example.org", password="password") + r = self.client.get(url, follow=True) + self.assertRedirects(r, self.channels_url) + self.assertContains(r, "something went wrong") diff --git a/hc/front/tests/test_add_slack_help.py b/hc/front/tests/test_add_slack_help.py new file mode 100644 index 00000000..f9b64188 --- /dev/null +++ b/hc/front/tests/test_add_slack_help.py @@ -0,0 +1,9 @@ +from django.test.utils import override_settings +from hc.test import BaseTestCase + + +@override_settings(SLACK_CLIENT_ID="fake-client-id") +class AddSlackHelpTestCase(BaseTestCase): + def test_instructions_work(self): + r = self.client.get("/integrations/add_slack/") + self.assertContains(r, "Setup Guide", status_code=200) diff --git a/hc/front/urls.py b/hc/front/urls.py index e011d56c..f54e6fa2 100644 --- a/hc/front/urls.py +++ b/hc/front/urls.py @@ -24,8 +24,6 @@ check_urls = [ channel_urls = [ path("", views.channels, name="hc-channels"), - path("add_slack/", views.add_slack, name="hc-add-slack"), - path("add_slack_btn/", views.add_slack_btn, name="hc-add-slack-btn"), path( "add_pushbullet/", views.add_pushbullet_complete, @@ -34,6 +32,8 @@ channel_urls = [ path("add_discord/", views.add_discord_complete, name="hc-add-discord-complete"), path("add_pushover/", views.add_pushover_help), path("telegram/bot/", views.telegram_bot, name="hc-telegram-webhook"), + path("add_slack/", views.add_slack_help), + path("add_slack_btn/", views.add_slack_complete), path("add_telegram/", views.add_telegram, name="hc-add-telegram"), path("add_trello/settings/", views.trello_settings, name="hc-trello-settings"), path("/checks/", views.channel_checks, name="hc-channel-checks"), @@ -67,6 +67,8 @@ project_urls = [ path("add_pushbullet/", views.add_pushbullet, name="hc-add-pushbullet"), path("add_pushover/", views.add_pushover, name="hc-add-pushover"), path("add_shell/", views.add_shell, name="hc-add-shell"), + path("add_slack/", views.add_slack, name="hc-add-slack"), + path("add_slack_btn/", views.add_slack_btn, name="hc-add-slack-btn"), path("add_sms/", views.add_sms, name="hc-add-sms"), path("add_trello/", views.add_trello, name="hc-add-trello"), path("add_victorops/", views.add_victorops, name="hc-add-victorops"), diff --git a/hc/front/views.py b/hc/front/views.py index 30a58c74..438beb54 100644 --- a/hc/front/views.py +++ b/hc/front/views.py @@ -684,6 +684,7 @@ def channels(request, code=None): "enable_matrix": settings.MATRIX_ACCESS_TOKEN is not None, "enable_apprise": settings.APPRISE_ENABLED is True, "enable_shell": settings.SHELL_ENABLED is True, + "enable_slack_btn": settings.SLACK_CLIENT_ID is not None, "use_payments": settings.USE_PAYMENTS, } @@ -1025,76 +1026,93 @@ def add_pagerteam(request, code): return render(request, "integrations/add_pagerteam.html", ctx) -def add_slack(request): - if not settings.SLACK_CLIENT_ID and not request.user.is_authenticated: - return redirect("hc-login") +@login_required +def add_slack(request, code): + project = _get_project_for_user(request, code) if request.method == "POST": form = AddUrlForm(request.POST) if form.is_valid(): - channel = Channel(project=request.project, kind="slack") + channel = Channel(project=project, kind="slack") channel.value = form.cleaned_data["value"] channel.save() channel.assign_all_checks() - return redirect("hc-channels") + return redirect("hc-p-channels", project.code) else: form = AddUrlForm() ctx = { "page": "channels", "form": form, - "slack_client_id": settings.SLACK_CLIENT_ID, } - if request.user.is_authenticated: - ctx["project"] = request.project + return render(request, "integrations/add_slack.html", ctx) - if settings.SLACK_CLIENT_ID and request.user.is_authenticated: - ctx["state"] = _prepare_state(request, "slack") - return render(request, "integrations/add_slack.html", ctx) +def add_slack_help(request): + if not settings.SLACK_CLIENT_ID: + raise Http404("slack integration is not available") + + ctx = {"page": "channels"} + return render(request, "integrations/add_slack_btn.html", ctx) @login_required -def add_mattermost(request, code): +def add_slack_btn(request, code): + if not settings.SLACK_CLIENT_ID: + raise Http404("slack integration is not available") + project = _get_project_for_user(request, code) - if request.method == "POST": - form = AddUrlForm(request.POST) - if form.is_valid(): - channel = Channel(project=project, kind="mattermost") - channel.value = form.cleaned_data["value"] - channel.save() + state = token_urlsafe() + authorize_url = "https://slack.com/oauth/authorize?" + urlencode( + { + "scope": "incoming-webhook", + "client_id": settings.SLACK_CLIENT_ID, + "state": state, + } + ) - channel.assign_all_checks() - return redirect("hc-p-channels", project.code) - else: - form = AddUrlForm() + ctx = { + "project": project, + "page": "channels", + "authorize_url": authorize_url, + } - ctx = {"page": "channels", "form": form, "project": project} - return render(request, "integrations/add_mattermost.html", ctx) + request.session["add_slack"] = (state, str(project.code)) + return render(request, "integrations/add_slack_btn.html", ctx) @login_required -def add_slack_btn(request): - code = _get_validated_code(request, "slack") - if code is None: - return HttpResponseBadRequest() +def add_slack_complete(request): + if not settings.SLACK_CLIENT_ID: + raise Http404("slack integration is not available") + + if "add_slack" not in request.session: + return HttpResponseForbidden() + + state, code = request.session.pop("add_slack") + project = _get_project_for_user(request, code) + if request.GET.get("error") == "access_denied": + messages.warning(request, "Slack setup was cancelled.") + return redirect("hc-p-channels", project.code) + + if request.GET.get("state") != state: + return HttpResponseForbidden() result = requests.post( "https://slack.com/api/oauth.access", { "client_id": settings.SLACK_CLIENT_ID, "client_secret": settings.SLACK_CLIENT_SECRET, - "code": code, + "code": request.GET.get("code"), }, ) doc = result.json() if doc.get("ok"): - channel = Channel(kind="slack", project=request.project) - channel.user = request.project.owner + channel = Channel(kind="slack", project=project) channel.value = result.text channel.save() channel.assign_all_checks() @@ -1103,7 +1121,27 @@ def add_slack_btn(request): s = doc.get("error") messages.warning(request, "Error message from slack: %s" % s) - return redirect("hc-channels") + return redirect("hc-p-channels", project.code) + + +@login_required +def add_mattermost(request, code): + project = _get_project_for_user(request, code) + + if request.method == "POST": + form = AddUrlForm(request.POST) + if form.is_valid(): + channel = Channel(project=project, kind="mattermost") + channel.value = form.cleaned_data["value"] + channel.save() + + channel.assign_all_checks() + return redirect("hc-p-channels", project.code) + else: + form = AddUrlForm() + + ctx = {"page": "channels", "form": form, "project": project} + return render(request, "integrations/add_mattermost.html", ctx) @login_required diff --git a/templates/front/channels.html b/templates/front/channels.html index 7e3fd5e6..498abb73 100644 --- a/templates/front/channels.html +++ b/templates/front/channels.html @@ -170,7 +170,11 @@

Slack

A messaging app for teams.

- Add Integration + {% if enable_slack_btn %} + Add Integration + {% else %} + Add Integration + {% endif %}
  • - - {% if slack_client_id %} -
    - {% if request.user.is_authenticated %} -

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

    - -
    - - Add to Slack - -
    - - {% else %} -

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

    - -
    - Sign In -
    - {% endif %} -
    - -

    Setup Guide

    - -
    -
    - 1 -

    - After {% if request.user.is_authenticated %}{% else %}signing in and{% endif %} - clicking on "Add to Slack", you should - be on a page that says "{% site_name %} would like access to - your Slack team". Select the team you want to add the - {% site_name %} integration app to. -

    -
    -
    - Screenshot -
    -
    - -
    -
    - 2 -

    - You should now be on a page that says "{% site_name %} would - like access to TEAM NAME". Select the channel you want to - post {% site_name %} notifications to. -

    -
    -
    - Screenshot -
    -
    - -
    -
    - 3 -

    - That is all! You will now be redirected back to - "Integrations" page on {% site_name %} and see - the new integration! -

    -
    -
    - Screenshot -
    -
    - {% else %}

    Slack

    If your team uses Slack, you can set @@ -141,7 +53,7 @@

    Integration Settings

    -
    + {% csrf_token %}
    - {% endif %} - {% endblock %} diff --git a/templates/integrations/add_slack_btn.html b/templates/integrations/add_slack_btn.html new file mode 100644 index 00000000..bc94be31 --- /dev/null +++ b/templates/integrations/add_slack_btn.html @@ -0,0 +1,97 @@ +{% extends "base.html" %} +{% load humanize static hc_extras %} + +{% block title %}Add Slack - {% site_name %}{% endblock %} + +{% block content %} +
    +
    + +
    + {% if request.user.is_authenticated %} +

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

    + +
    + + Add to Slack + +
    + + {% else %} +

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

    + +
    + Sign In +
    + {% endif %} +
    + +

    Setup Guide

    + +
    +
    + 1 +

    + After {% if request.user.is_authenticated %}{% else %}signing in and{% endif %} + clicking on "Add to Slack", you should + be on a page that says "{% site_name %} would like access to + your Slack team". Select the team you want to add the + {% site_name %} integration app to. +

    +
    +
    + Screenshot +
    +
    + +
    +
    + 2 +

    + You should now be on a page that says "{% site_name %} would + like access to TEAM NAME". Select the channel you want to + post {% site_name %} notifications to. +

    +
    +
    + Screenshot +
    +
    + +
    +
    + 3 +

    + That is all! You will now be redirected back to + "Integrations" page on {% site_name %} and see + the new integration! +

    +
    +
    + Screenshot +
    +
    + +
    +
    +{% endblock %}