🔒️(users) restrict listable users to same organization

This is a quick fix to a security issue. Previously, any user could
list all users. Now /users/ endpoint only lists users from same
organization.
This commit is contained in:
Marie PUPO JEAMMET
2025-03-27 15:09:28 +01:00
committed by Marie
parent a009f3ccb7
commit b4de7fda92
5 changed files with 93 additions and 30 deletions

View File

@@ -22,6 +22,7 @@ and this project adheres to
### Changed ### Changed
- ♻️(plugins) rewrite plugin system as django app #844 - ♻️(plugins) rewrite plugin system as django app #844
- 🔒️(users) restrict listable users to same organization #846
### Fixed ### Fixed

View File

@@ -50,6 +50,9 @@
"firstName": "John", "firstName": "John",
"lastName": "Doe", "lastName": "Doe",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -81,6 +84,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Chromium", "lastName": "Chromium",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -95,6 +101,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Webkit", "lastName": "Webkit",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -109,6 +118,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Firefox", "lastName": "Firefox",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -123,6 +135,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Member", "lastName": "Group Member",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -137,6 +152,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Administrator", "lastName": "Group Administrator",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -151,6 +169,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Owner", "lastName": "Group Owner",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -165,6 +186,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Mailbox Member", "lastName": "Mailbox Member",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -179,6 +203,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Mailbox Administrator", "lastName": "Mailbox Administrator",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -193,6 +220,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Mailbox Owner", "lastName": "Mailbox Owner",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -207,6 +237,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Member Mailbox Member", "lastName": "Group Member Mailbox Member",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -221,6 +254,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Member Mailbox Administrator", "lastName": "Group Member Mailbox Administrator",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -235,6 +271,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Member Mailbox Owner", "lastName": "Group Member Mailbox Owner",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -249,6 +288,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Administrator Mailbox Member", "lastName": "Group Administrator Mailbox Member",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -263,6 +305,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Administrator Mailbox Administrator", "lastName": "Group Administrator Mailbox Administrator",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -277,6 +322,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Administrator Mailbox Owner", "lastName": "Group Administrator Mailbox Owner",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -291,6 +339,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Owner Mailbox Member", "lastName": "Group Owner Mailbox Member",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -305,6 +356,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Group Owner Mailbox Administrator", "lastName": "Group Owner Mailbox Administrator",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",
@@ -319,6 +373,9 @@
"firstName": "E2E", "firstName": "E2E",
"lastName": "Mailbox Owner", "lastName": "Mailbox Owner",
"enabled": true, "enabled": true,
"attributes": {
"siret": "13002526500013"
},
"credentials": [ "credentials": [
{ {
"type": "password", "type": "password",

View File

@@ -263,9 +263,10 @@ class UserViewSet(
queryset = self.queryset queryset = self.queryset
if self.action == "list": if self.action == "list":
# Exclude inactive contacts # Filter active users
# and users from same organization
queryset = queryset.filter( queryset = queryset.filter(
is_active=True, is_active=True, organization_id=self.request.user.organization_id
) )
# Exclude all users already in the given team # Exclude all users already in the given team

View File

@@ -31,26 +31,29 @@ def test_api_users_list_anonymous():
def test_api_users_list_authenticated(): def test_api_users_list_authenticated():
""" """
Authenticated users should be able to list all users. Authenticated users should be able to list users from their organization.
""" """
user = factories.UserFactory() organization = factories.OrganizationFactory(with_registration_id=True)
user = factories.UserFactory(organization=organization)
client = APIClient() client = APIClient()
client.force_login(user) client.force_login(user)
factories.UserFactory.create_batch(2) factories.UserFactory(organization=organization)
factories.UserFactory.create_batch(2) # 2 users outside organization
response = client.get( response = client.get(
"/api/v1.0/users/", "/api/v1.0/users/",
) )
assert response.status_code == HTTP_200_OK assert response.status_code == HTTP_200_OK
assert len(response.json()["results"]) == 3 assert len(response.json()["results"]) == 2
def test_api_users_list_authenticated_response_content( def test_api_users_list_authenticated_response_content(
client, django_assert_num_queries client, django_assert_num_queries
): ):
""" """
Authenticated users should be able to list all users with the expected output. Authenticated users should be able to list all users from their organization
with the expected output.
""" """
user_organization = factories.OrganizationFactory( user_organization = factories.OrganizationFactory(
with_registration_id=True, name="HAL 9000" with_registration_id=True, name="HAL 9000"
@@ -67,7 +70,7 @@ def test_api_users_list_authenticated_response_content(
other_user_organization = factories.OrganizationFactory( other_user_organization = factories.OrganizationFactory(
with_registration_id=True, name="Corp. Inc." with_registration_id=True, name="Corp. Inc."
) )
other_user = factories.UserFactory( factories.UserFactory(
organization=other_user_organization, organization=other_user_organization,
email="sara83@example.com", email="sara83@example.com",
name="Christopher Thompson", name="Christopher Thompson",
@@ -85,23 +88,10 @@ def test_api_users_list_authenticated_response_content(
.first() .first()
) )
assert edited_json == { assert edited_json == {
"count": 2, "count": 1,
"next": None, "next": None,
"previous": None, "previous": None,
"results": [ "results": [
{
"email": "sara83@example.com",
"id": str(other_user.pk),
"is_device": False,
"is_staff": False,
"language": "fr-fr",
"name": "Christopher Thompson",
"organization": {
"id": str(other_user.organization.pk),
"name": "Corp. Inc.",
},
"timezone": "UTC",
},
{ {
"email": "kylefields@example.net", "email": "kylefields@example.net",
"id": str(user.pk), "id": str(user.pk),
@@ -124,13 +114,17 @@ def test_api_users_authenticated_list_by_email():
Authenticated users should be able to search users with a case-insensitive and Authenticated users should be able to search users with a case-insensitive and
partial query on the email. partial query on the email.
""" """
user = factories.UserFactory(email="tester@ministry.fr", name="john doe") user = factories.UserFactory(
dave = factories.UserFactory(email="david.bowman@work.com", name=None) email="tester@ministry.fr", name="john doe", with_organization=True
)
dave = factories.UserFactory(
email="david.bowman@work.com", name=None, organization=user.organization
)
nicole = factories.UserFactory( nicole = factories.UserFactory(
email="nicole_foole@work.com", name=None, with_organization=True email="nicole_foole@work.com", name=None, organization=user.organization
) )
frank = factories.UserFactory( frank = factories.UserFactory(
email="frank_poole@work.com", name=None, with_organization=True email="frank_poole@work.com", name=None, organization=user.organization
) )
factories.UserFactory(email="heywood_floyd@work.com", name=None) factories.UserFactory(email="heywood_floyd@work.com", name=None)
@@ -203,13 +197,17 @@ def test_api_users_authenticated_list_by_name():
Authenticated users should be able to search users with a case-insensitive and Authenticated users should be able to search users with a case-insensitive and
partial query on the name. partial query on the name.
""" """
user = factories.UserFactory(email="tester@ministry.fr", name="john doe") user = factories.UserFactory(
dave = factories.UserFactory(name="Dave bowman", email=None) email="tester@ministry.fr", name="john doe", with_organization=True
)
dave = factories.UserFactory(
name="Dave bowman", email=None, organization=user.organization
)
nicole = factories.UserFactory( nicole = factories.UserFactory(
name="nicole foole", email=None, with_organization=True name="nicole foole", email=None, organization=user.organization
) )
frank = factories.UserFactory( frank = factories.UserFactory(
name="frank poolé", email=None, with_organization=True name="frank poolé", email=None, organization=user.organization
) )
factories.UserFactory(name="heywood floyd", email=None) factories.UserFactory(name="heywood floyd", email=None)

View File

@@ -201,6 +201,10 @@ def create_demo(stdout): # pylint: disable=too-many-locals
) )
# this is a quick fix to fix e2e tests # this is a quick fix to fix e2e tests
# tests needs some no random data # tests needs some no random data
organization, _created = models.Organization.objects.get_or_create(
name="13002526500013",
registration_id_list=["13002526500013"],
)
queue.push( queue.push(
models.User( models.User(
sub=uuid4(), sub=uuid4(),
@@ -210,6 +214,7 @@ def create_demo(stdout): # pylint: disable=too-many-locals
is_superuser=False, is_superuser=False,
is_active=True, is_active=True,
is_staff=False, is_staff=False,
organization=organization,
language=random.choice(settings.LANGUAGES)[0], language=random.choice(settings.LANGUAGES)[0],
) )
) )
@@ -222,6 +227,7 @@ def create_demo(stdout): # pylint: disable=too-many-locals
is_superuser=False, is_superuser=False,
is_active=True, is_active=True,
is_staff=False, is_staff=False,
organization=organization,
language=random.choice(settings.LANGUAGES)[0], language=random.choice(settings.LANGUAGES)[0],
) )
) )