Skip to content

Commit

Permalink
Restrict email updates to organization owners and admins only (#5317)
Browse files Browse the repository at this point in the history
### 📣 Summary
Restrict email updates to organization owners and admins only to ensure
proper access control within multi-member organizations.



### 📖 Description
Previously, any member of an organization could update their email
address, regardless of their role. This update enforces a restriction
where only organization owners and admins can update their email
addresses. Members without these roles are now prevented from making
such changes.



### 👷 Description for instance maintainers
This update introduces a new restriction to the email update endpoint.
It checks the role of the user within the organization and ensures that
only users with the `owner` or `admin` role in organizations can update
their email. This ensures better security and role-based access control.
No changes are required for single-member organizations.
  • Loading branch information
rajpatel24 authored Dec 3, 2024
1 parent 4040ecc commit 028afe5
Show file tree
Hide file tree
Showing 2 changed files with 88 additions and 1 deletion.
14 changes: 14 additions & 0 deletions kobo/apps/accounts/serializers.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from allauth.account.models import EmailAddress
from allauth.socialaccount.models import SocialAccount
from django.utils.translation import gettext as t
from rest_framework import serializers


Expand All @@ -22,6 +23,19 @@ def create(self, validated_data):
confirm=True,
)

def validate(self, attrs):
"""
Validates that only owners or admins of the organization can update
their email
"""
user = self.context['request'].user
organization = user.organization
if organization.is_owner(user) or organization.is_admin(user):
return attrs
raise serializers.ValidationError(
{'email': t('This action is not allowed.')}
)


# https://github.com/iMerica/dj-rest-auth/blob/6b394d9d6bb1f2979ea2d31e5a1199368d5616c1/dj_rest_auth/registration/serializers.py#L22
# https://gitlab.com/glitchtip/glitchtip-backend/-/blob/master/users/serializers.py#L40
Expand Down
75 changes: 74 additions & 1 deletion kobo/apps/accounts/tests/test_email.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from django.core import mail
from django.urls import reverse
from model_bakery import baker
from rest_framework import status
from rest_framework.test import APITestCase

from kpi.utils.fuzzy_int import FuzzyInt
Expand Down Expand Up @@ -48,7 +49,7 @@ def test_new_email(self):
# Add second unconfirmed email, overrides the first
data = {'email': '[email protected]'}
# Auth, Select, Delete (many), Get or Create
queries = FuzzyInt(11, 15)
queries = FuzzyInt(11, 20)
with self.assertNumQueries(queries):
res = self.client.post(self.url_list, data, format='json')
self.assertContains(res, data['email'], status_code=201)
Expand Down Expand Up @@ -98,3 +99,75 @@ def test_new_confirm_email(self):
1,
'Expect only 1 email after confirm',
)


class EmailUpdateRestrictionTestCase(APITestCase):
"""
Test that only organization owners and admins can update their email.
"""
def setUp(self):
self.owner = baker.make(settings.AUTH_USER_MODEL)
self.admin = baker.make(settings.AUTH_USER_MODEL)
self.member = baker.make(settings.AUTH_USER_MODEL)
self.non_mmo_user = baker.make(settings.AUTH_USER_MODEL)

self.organization = self.owner.organization
self.organization.mmo_override = True
self.organization.save(update_fields=['mmo_override'])

self.organization.add_user(self.admin, is_admin=True)
self.organization.add_user(self.member)

self.url_list = reverse('emailaddress-list')

def test_that_mmo_owner_can_update_email(self):
"""
Test that the owner of the organization can update their email
"""
data = {'email': '[email protected]'}
self.client.force_login(self.owner)
res = self.client.post(self.url_list, data, format='json')

self.assertEqual(res.status_code, status.HTTP_201_CREATED)
self.assertEqual(
self.owner.emailaddress_set.filter(email=data['email']).count(), 1
)

def test_that_mmo_admin_can_update_email(self):
"""
Test that the admin of the organization can update their email
"""
data = {'email': '[email protected]'}
self.client.force_login(self.admin)
res = self.client.post(self.url_list, data, format='json')
self.assertEqual(res.status_code, status.HTTP_201_CREATED)
self.assertEqual(
self.admin.emailaddress_set.filter(email=data['email']).count(), 1
)

def test_that_mmo_member_cannot_update_email(self):
"""
Test that the member of the organization cannot update their email
"""
data = {'email': '[email protected]'}
self.client.force_login(self.member)
res = self.client.post(self.url_list, data, format='json')
self.assertEqual(res.status_code, status.HTTP_400_BAD_REQUEST)
self.assertEqual(
self.member.emailaddress_set.filter(email=data['email']).count(), 0
)

def test_that_non_mmo_user_can_update_email(self):
"""
Test that a user who is not part of MMO can update their email
"""
data = {'email': '[email protected]'}
self.client.force_login(self.non_mmo_user)
res = self.client.post(self.url_list, data, format='json')
self.assertEqual(res.status_code, status.HTTP_201_CREATED)
self.assertEqual(
self.non_mmo_user.emailaddress_set.filter(
email=data['email']
).count(),
1
)

0 comments on commit 028afe5

Please sign in to comment.