From 2346ac3e8040dd7e20c81e5fa0f082b8573e6fb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C4=93teris=20Caune?= Date: Wed, 19 Aug 2020 12:07:48 +0300 Subject: [PATCH] Bugfix: don't allow duplicate team memberships --- CHANGELOG.md | 5 +++-- .../migrations/0032_auto_20200819_0757.py | 17 ++++++++++++++++ hc/accounts/models.py | 14 +++++++++++++ hc/accounts/tests/test_project.py | 20 +++++++++++++++++++ hc/accounts/views.py | 9 ++++++--- templates/accounts/project.html | 6 ++++++ 6 files changed, 66 insertions(+), 5 deletions(-) create mode 100644 hc/accounts/migrations/0032_auto_20200819_0757.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f26e5f8..c63dd1d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,9 @@ All notable changes to this project will be documented in this file. - Host a read-only dashboard (from github.com/healthchecks/dashboard/) ## Bug Fixes -- Handle excessively long email addresses in the signup form. -- Handle excessively long email addresses in the team member invite form. +- Handle excessively long email addresses in the signup form +- Handle excessively long email addresses in the team member invite form +- Don't allow duplicate team memberships ## v1.16.0 - 2020-08-04 diff --git a/hc/accounts/migrations/0032_auto_20200819_0757.py b/hc/accounts/migrations/0032_auto_20200819_0757.py new file mode 100644 index 00000000..f921c783 --- /dev/null +++ b/hc/accounts/migrations/0032_auto_20200819_0757.py @@ -0,0 +1,17 @@ +# Generated by Django 3.1 on 2020-08-19 07:57 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('accounts', '0031_auto_20200803_1413'), + ] + + operations = [ + migrations.AddConstraint( + model_name='member', + constraint=models.UniqueConstraint(fields=('user', 'project'), name='accounts_member_no_duplicates'), + ), + ] diff --git a/hc/accounts/models.py b/hc/accounts/models.py index bd7cd909..9b3efa90 100644 --- a/hc/accounts/models.py +++ b/hc/accounts/models.py @@ -319,9 +319,16 @@ class Project(models.Model): return used < self.owner_profile.team_limit def invite(self, user): + if Member.objects.filter(user=user, project=self).exists(): + return False + + if self.owner_id == user.id: + return False + Member.objects.create(user=user, project=self) checks_url = reverse("hc-checks", args=[self.code]) user.profile.send_instant_login_link(self, redirect_url=checks_url) + return True def set_next_nag_date(self): """ Set next_nag_date on profiles of all members of this project. """ @@ -373,5 +380,12 @@ class Member(models.Model): project = models.ForeignKey(Project, models.CASCADE) transfer_request_date = models.DateTimeField(null=True, blank=True) + class Meta: + constraints = [ + models.UniqueConstraint( + fields=["user", "project"], name="accounts_member_no_duplicates" + ) + ] + def can_accept(self): return self.user.profile.can_accept(self.project) diff --git a/hc/accounts/tests/test_project.py b/hc/accounts/tests/test_project.py index bef114f8..e7c5aac7 100644 --- a/hc/accounts/tests/test_project.py +++ b/hc/accounts/tests/test_project.py @@ -108,6 +108,26 @@ class ProjectTestCase(BaseTestCase): q = TokenBucket.objects.filter(value="invite-%d" % self.alice.id) self.assertFalse(q.exists()) + def test_it_rejects_duplicate_membership(self): + self.client.login(username="alice@example.org", password="password") + + form = {"invite_team_member": "1", "email": "bob@example.org"} + r = self.client.post(self.url, form) + self.assertContains(r, "bob@example.org is already a member") + + # The number of memberships should have not increased + self.assertEqual(self.project.member_set.count(), 1) + + def test_it_rejects_owner_as_a_member(self): + self.client.login(username="alice@example.org", password="password") + + form = {"invite_team_member": "1", "email": "alice@example.org"} + r = self.client.post(self.url, form) + self.assertContains(r, "alice@example.org is already a member") + + # The number of memberships should have not increased + self.assertEqual(self.project.member_set.count(), 1) + def test_it_rejects_too_long_email_addresses(self): self.client.login(username="alice@example.org", password="password") diff --git a/hc/accounts/views.py b/hc/accounts/views.py index 47b083b6..e88d8954 100644 --- a/hc/accounts/views.py +++ b/hc/accounts/views.py @@ -304,9 +304,12 @@ def project(request, code): except User.DoesNotExist: user = _make_user(email, with_project=False) - project.invite(user) - ctx["team_member_invited"] = email - ctx["team_status"] = "success" + if project.invite(user): + ctx["team_member_invited"] = email + ctx["team_status"] = "success" + else: + ctx["team_member_duplicate"] = email + ctx["team_status"] = "info" elif "remove_team_member" in request.POST: if not is_owner: diff --git a/templates/accounts/project.html b/templates/accounts/project.html index 2e07b372..d9ff5235 100644 --- a/templates/accounts/project.html +++ b/templates/accounts/project.html @@ -228,6 +228,12 @@ {% endif %} + {% if team_member_duplicate %} +
+ {{ team_member_duplicate }} is already a member +
+ {% endif %} + {% if team_member_removed %}
{{ team_member_removed }} removed from team