From 403fea94bb787d845aa07f927f9dd9fa84fbc56b Mon Sep 17 00:00:00 2001 From: Quentin BEY Date: Tue, 11 Mar 2025 15:14:18 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(teams)=20allow=20broadly=20available?= =?UTF-8?q?=20teams?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds `is_visible_all_services` field to `Teams` to make them visible to all service providers --- CHANGELOG.md | 1 + src/backend/core/api/client/serializers.py | 2 + .../core/api/resource_server/serializers.py | 1 + .../core/api/resource_server/viewsets.py | 3 +- .../0011_team_is_visible_all_services.py | 18 +++++++ src/backend/core/models.py | 5 ++ .../resource_server_api/teams/test_create.py | 44 +++++++++++++++ .../resource_server_api/teams/test_list.py | 49 +++++++++++++++++ .../teams/test_retrieve.py | 26 +++++++++ .../resource_server_api/teams/test_update.py | 54 +++++++++++++++++++ .../tests/teams/test_core_api_teams_create.py | 30 +++++++++++ .../tests/teams/test_core_api_teams_list.py | 4 ++ .../teams/test_core_api_teams_retrieve.py | 2 + .../tests/teams/test_core_api_teams_update.py | 21 ++++++++ 14 files changed, 259 insertions(+), 1 deletion(-) create mode 100644 src/backend/core/migrations/0011_team_is_visible_all_services.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eec689..5f3935e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to ### Added +- ✨(teams) allow broadly available teams #796 - ✨(teams) update and enhance team invitation email - ✨(api) define dimail timeout as a setting - ✨(frontend) feature modal add new access role to domain diff --git a/src/backend/core/api/client/serializers.py b/src/backend/core/api/client/serializers.py index 2d3864f..1cc620d 100644 --- a/src/backend/core/api/client/serializers.py +++ b/src/backend/core/api/client/serializers.py @@ -251,6 +251,7 @@ class TeamSerializer(serializers.ModelSerializer): """Serialize teams.""" abilities = serializers.SerializerMethodField(read_only=True) + is_visible_all_services = serializers.BooleanField(required=False, default=True) service_providers = serializers.PrimaryKeyRelatedField( queryset=ServiceProvider.objects.all(), many=True, required=False ) @@ -263,6 +264,7 @@ class TeamSerializer(serializers.ModelSerializer): "accesses", "created_at", "depth", + "is_visible_all_services", "name", "numchild", "path", diff --git a/src/backend/core/api/resource_server/serializers.py b/src/backend/core/api/resource_server/serializers.py index de756f9..aee11c9 100644 --- a/src/backend/core/api/resource_server/serializers.py +++ b/src/backend/core/api/resource_server/serializers.py @@ -14,6 +14,7 @@ class TeamSerializer(serializers.ModelSerializer): "id", "created_at", "depth", + "is_visible_all_services", "name", "numchild", "path", diff --git a/src/backend/core/api/resource_server/viewsets.py b/src/backend/core/api/resource_server/viewsets.py index 13b4f31..b20ac31 100644 --- a/src/backend/core/api/resource_server/viewsets.py +++ b/src/backend/core/api/resource_server/viewsets.py @@ -103,7 +103,8 @@ class TeamViewSet( # pylint: disable=too-many-ancestors for d in depth_path ), ), - service_providers__audience_id=service_provider_audience, + Q(service_providers__audience_id=service_provider_audience) + | Q(is_visible_all_services=True), ) # Abilities are computed based on logged-in user's role for the team # and if the user does not have access, it's ok to consider them as a member diff --git a/src/backend/core/migrations/0011_team_is_visible_all_services.py b/src/backend/core/migrations/0011_team_is_visible_all_services.py new file mode 100644 index 0000000..d4caf48 --- /dev/null +++ b/src/backend/core/migrations/0011_team_is_visible_all_services.py @@ -0,0 +1,18 @@ +# Generated by Django 5.1.7 on 2025-03-11 13:34 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('core', '0010_team_depth_team_numchild_team_path_and_more'), + ] + + operations = [ + migrations.AddField( + model_name='team', + name='is_visible_all_services', + field=models.BooleanField(default=False, help_text='Whether this team is visible to all service providers.', verbose_name='is visible for all SP'), + ), + ] diff --git a/src/backend/core/models.py b/src/backend/core/models.py index caa1fd3..b00cfab 100644 --- a/src/backend/core/models.py +++ b/src/backend/core/models.py @@ -721,6 +721,11 @@ class Team(MP_Node, BaseModel): related_name="teams", blank=True, ) + is_visible_all_services = models.BooleanField( + _("is visible for all SP"), + default=False, + help_text=_("Whether this team is visible to all service providers."), + ) objects = TeamManager() diff --git a/src/backend/core/tests/resource_server_api/teams/test_create.py b/src/backend/core/tests/resource_server_api/teams/test_create.py index 21a3b0b..93ca709 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_create.py +++ b/src/backend/core/tests/resource_server_api/teams/test_create.py @@ -59,6 +59,7 @@ def test_api_teams_create_authenticated_new_service_provider( assert response.json() == { "created_at": team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "id": str(team.pk), + "is_visible_all_services": False, "depth": team.depth, "name": "my team", "numchild": team.numchild, @@ -164,3 +165,46 @@ def test_api_teams_create_cannot_override_service_provider( team = Team.objects.get() assert team.name == "my team" assert team.service_providers.get().audience_id == service_provider.audience_id + + +def test_api_teams_create_is_visible_all_services_team( + client, force_login_via_resource_server +): + """ + Authenticated users should be able to create teams visible in all service providers. + """ + user = UserFactory() + service_provider = ServiceProviderFactory() + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.post( + "/resource-server/v1.0/teams/", + { + "name": "my team", + "is_visible_all_services": True, + }, + format="json", + ) + + assert response.status_code == HTTP_201_CREATED + team = Team.objects.get() + assert team.is_visible_all_services is True + + +def test_api_teams_create_restricted_team(client, force_login_via_resource_server): + """Authenticated users should be able to create teams restricted to some service providers.""" + user = UserFactory() + service_provider = ServiceProviderFactory() + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.post( + "/resource-server/v1.0/teams/", + { + "name": "my team", + }, + format="json", + ) + + assert response.status_code == HTTP_201_CREATED + team = Team.objects.get() + assert team.is_visible_all_services is False diff --git a/src/backend/core/tests/resource_server_api/teams/test_list.py b/src/backend/core/tests/resource_server_api/teams/test_list.py index b6ca255..099aca5 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_list.py +++ b/src/backend/core/tests/resource_server_api/teams/test_list.py @@ -82,6 +82,7 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals "created_at": team_1.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": team_1.depth, "id": str(team_1.pk), + "is_visible_all_services": False, "name": team_1.name, "numchild": team_1.numchild, "path": team_1.path, @@ -91,6 +92,7 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals "created_at": team_2.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": team_2.depth, "id": str(team_2.pk), + "is_visible_all_services": False, "name": team_2.name, "numchild": team_2.numchild, "path": team_2.path, @@ -100,6 +102,7 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals "created_at": team_3.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": team_3.depth, "id": str(team_3.pk), + "is_visible_all_services": False, "name": team_3.name, "numchild": team_3.numchild, "path": team_3.path, @@ -109,6 +112,7 @@ def test_api_teams_list_authenticated( # pylint: disable=too-many-locals "created_at": team_4.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": team_4.depth, "id": str(team_4.pk), + "is_visible_all_services": False, "name": team_4.name, "numchild": team_4.numchild, "path": team_4.path, @@ -273,3 +277,48 @@ def test_api_teams_list_with_parent_teams_other_organization( team_ids = [team["id"] for team in response_data["results"]] assert len(team_ids) == 1 assert set(team_ids) == {str(second_team.id)} + + +def test_api_teams_list_is_visible_all_services_teams( + client, force_login_via_resource_server +): + """ + Authenticated users should be able to see teams + if they are associated with another requesting service provider. + """ + user = factories.UserFactory() + service_provider = factories.ServiceProviderFactory() + other_service_provider = factories.ServiceProviderFactory() + + # Create a public team visible to the requesting service provider + service_team = factories.TeamFactory( + is_visible_all_services=False, + service_providers=[service_provider], + ) + factories.TeamAccessFactory(user=user, team=service_team, role="member") + + # Create a public team visible to another service provider (should be listed) + other_public_team = factories.TeamFactory( + is_visible_all_services=True, + service_providers=[other_service_provider], + ) + factories.TeamAccessFactory(user=user, team=other_public_team, role="member") + + # Create a public team visible to the requesting service provider (should not be listed) + private_team = factories.TeamFactory( + is_visible_all_services=False, + service_providers=[other_service_provider], + ) + factories.TeamAccessFactory(user=user, team=private_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get("/resource-server/v1.0/teams/") + + assert response.status_code == HTTP_200_OK + response_data = response.json() + assert response_data["count"] == 2 + + team_ids = [team["id"] for team in response_data["results"]] + assert len(team_ids) == 2 + assert str(service_team.id) in team_ids + assert str(other_public_team.id) in team_ids diff --git a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py index c162945..2734f01 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_retrieve.py +++ b/src/backend/core/tests/resource_server_api/teams/test_retrieve.py @@ -69,6 +69,7 @@ def test_api_teams_retrieve_authenticated_related( "created_at": team.created_at.isoformat().replace("+00:00", "Z"), "depth": 1, "id": str(team.id), + "is_visible_all_services": False, "name": team.name, "numchild": 0, "path": team.path, @@ -143,6 +144,7 @@ def test_api_teams_retrieve_authenticated_related_parent_same_organization( "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), "depth": 2, "id": str(first_team.pk), + "is_visible_all_services": False, "name": first_team.name, "numchild": 1, "path": first_team.path, @@ -229,3 +231,27 @@ def test_api_teams_retrieve_authenticated_related_child_same_organization( assert response.status_code == status.HTTP_404_NOT_FOUND assert response.json() == {"detail": "No Team matches the given query."} + + +def test_api_teams_retrieve_is_visible_all_services_team( + client, force_login_via_resource_server +): + """ + Authenticated users should be able to retrieve teams even + if associated with the another requesting service provider. + """ + user = factories.UserFactory() + service_provider = factories.ServiceProviderFactory() + other_service_provider = factories.ServiceProviderFactory() + + public_team = factories.TeamFactory( + is_visible_all_services=True, + service_providers=[other_service_provider], + ) + TeamAccessFactory(user=user, team=public_team, role="member") + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.get(f"/resource-server/v1.0/teams/{public_team.id}/") + + assert response.status_code == status.HTTP_200_OK + assert response.json()["id"] == str(public_team.id) diff --git a/src/backend/core/tests/resource_server_api/teams/test_update.py b/src/backend/core/tests/resource_server_api/teams/test_update.py index 467c95a..c46b09c 100644 --- a/src/backend/core/tests/resource_server_api/teams/test_update.py +++ b/src/backend/core/tests/resource_server_api/teams/test_update.py @@ -336,3 +336,57 @@ def test_api_teams_update_child_team(client, force_login_via_resource_server, ro second_team.refresh_from_db() assert second_team.name == "Second" + + +def test_api_teams_update_is_visible_all_services_status( + client, force_login_via_resource_server +): + """Team administrators should be able to change the visibility status of their team.""" + user = factories.UserFactory() + service_provider = factories.ServiceProviderFactory() + team = factories.TeamFactory( + users=[(user, "administrator")], + service_providers=[service_provider], + is_visible_all_services=True, + ) + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.patch( + f"/resource-server/v1.0/teams/{team.id}/", + { + "is_visible_all_services": False, + }, + content_type="application/json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_200_OK + team.refresh_from_db() + assert team.is_visible_all_services is False + + +def test_api_teams_update_public_status_as_member( + client, force_login_via_resource_server +): + """Team members should not be able to change the visibility status of their team.""" + user = factories.UserFactory() + service_provider = factories.ServiceProviderFactory() + team = factories.TeamFactory( + users=[(user, "member")], + service_providers=[service_provider], + is_visible_all_services=True, + ) + + with force_login_via_resource_server(client, user, service_provider.audience_id): + response = client.patch( + f"/resource-server/v1.0/teams/{team.id}/", + { + "is_visible_all_services": False, + }, + content_type="application/json", + HTTP_AUTHORIZATION="Bearer b64untestedbearertoken", + ) + + assert response.status_code == HTTP_403_FORBIDDEN + team.refresh_from_db() + assert team.is_visible_all_services is True diff --git a/src/backend/core/tests/teams/test_core_api_teams_create.py b/src/backend/core/tests/teams/test_core_api_teams_create.py index 3d0b670..5872c2c 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_create.py +++ b/src/backend/core/tests/teams/test_core_api_teams_create.py @@ -61,6 +61,7 @@ def test_api_teams_create_authenticated(settings): assert team.name == "my team" assert team.organization == organization assert team.accesses.filter(role="owner", user=user).exists() + assert team.is_visible_all_services is True def test_api_teams_create_authenticated_feature_disabled(settings): @@ -120,3 +121,32 @@ def test_api_teams_create_cannot_override_organization(): assert team.name == "my team" assert team.organization == organization assert team.accesses.filter(role="owner", user=user).exists() + assert team.is_visible_all_services is True + + +def test_api_teams_create_not_is_visible_all_services(): + """ + Authenticated users should be able to create teams and + make is restricted to the services which can view it. + """ + organization = OrganizationFactory(with_registration_id=True) + user = UserFactory(organization=organization) + + client = APIClient() + client.force_login(user) + + response = client.post( + "/api/v1.0/teams/", + { + "name": "my team", + "is_visible_all_services": False, + }, + format="json", + ) + + assert response.status_code == HTTP_201_CREATED + team = Team.objects.get() + assert team.name == "my team" + assert team.organization == organization + assert team.accesses.filter(role="owner", user=user).exists() + assert team.is_visible_all_services is False diff --git a/src/backend/core/tests/teams/test_core_api_teams_list.py b/src/backend/core/tests/teams/test_core_api_teams_list.py index 8692502..4fb5acc 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_list.py +++ b/src/backend/core/tests/teams/test_core_api_teams_list.py @@ -192,6 +192,7 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": 3, "id": str(second_team.pk), + "is_visible_all_services": False, "name": "Second", "numchild": 1, "path": second_team.path, @@ -211,6 +212,7 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti "created_at": first_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": 2, "id": str(first_team.pk), + "is_visible_all_services": False, "name": "First", "numchild": 1, "path": first_team.path, @@ -230,6 +232,7 @@ def test_api_teams_list_authenticated_team_tree(client, role, local_team_abiliti "created_at": root_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": 1, "id": str(root_team.pk), + "is_visible_all_services": False, "name": "Root", "numchild": 1, "path": root_team.path, @@ -316,6 +319,7 @@ def test_api_teams_list_authenticated_team_different_organization( "created_at": second_team.created_at.strftime("%Y-%m-%dT%H:%M:%S.%fZ"), "depth": 3, "id": str(second_team.pk), + "is_visible_all_services": False, "name": "Second", "numchild": 1, "path": second_team.path, diff --git a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py index 08128f7..206cb2c 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_retrieve.py +++ b/src/backend/core/tests/teams/test_core_api_teams_retrieve.py @@ -71,6 +71,7 @@ def test_api_teams_retrieve_authenticated_related(): "created_at": team.created_at.isoformat().replace("+00:00", "Z"), "depth": 1, "id": str(team.id), + "is_visible_all_services": False, "name": team.name, "numchild": 0, "path": team.path, @@ -112,6 +113,7 @@ def test_api_teams_retrieve_authenticated_related_parent(client, role): "created_at": first_team.created_at.isoformat().replace("+00:00", "Z"), "depth": 2, "id": str(first_team.pk), + "is_visible_all_services": False, "name": first_team.name, "numchild": 1, "path": first_team.path, diff --git a/src/backend/core/tests/teams/test_core_api_teams_update.py b/src/backend/core/tests/teams/test_core_api_teams_update.py index 8f23be4..7cadcc7 100644 --- a/src/backend/core/tests/teams/test_core_api_teams_update.py +++ b/src/backend/core/tests/teams/test_core_api_teams_update.py @@ -128,6 +128,27 @@ def test_api_teams_update_authenticated_administrators(): assert value == new_values[key] +def test_api_teams_update_is_visible_all_services(): + """Administrators of a team should be allowed to update the visibility to all services.""" + user = factories.UserFactory() + + client = APIClient() + client.force_login(user) + + team = factories.TeamFactory( + users=[(user, "administrator")], is_visible_all_services=False + ) + + response = client.patch( + f"/api/v1.0/teams/{team.id!s}/", + {"is_visible_all_services": True}, + format="json", + ) + assert response.status_code == HTTP_200_OK + team.refresh_from_db() + assert team.is_visible_all_services is True + + def test_api_teams_update_authenticated_owners(): """Administrators of a team should be allowed to update it, apart from read-only fields."""