diff --git a/src/maasapiserver/v3/api/public/handlers/usergroups.py b/src/maasapiserver/v3/api/public/handlers/usergroups.py index 89cbf9981f..9c778bf50f 100644 --- a/src/maasapiserver/v3/api/public/handlers/usergroups.py +++ b/src/maasapiserver/v3/api/public/handlers/usergroups.py @@ -13,12 +13,19 @@ ) from maasapiserver.v3.api import services from maasapiserver.v3.api.public.models.requests.query import PaginationParams +from maasapiserver.v3.api.public.models.requests.usergroup_members import ( + UserGroupMemberRequest, +) from maasapiserver.v3.api.public.models.requests.usergroups import ( UserGroupRequest, ) from maasapiserver.v3.api.public.models.responses.base import ( OPENAPI_ETAG_HEADER, ) +from maasapiserver.v3.api.public.models.responses.usergroup_members import ( + UserGroupMemberResponse, + UserGroupMembersListResponse, +) from maasapiserver.v3.api.public.models.responses.usergroups import ( UserGroupResponse, UserGroupsListResponse, @@ -26,8 +33,19 @@ from maasapiserver.v3.auth.base import check_permissions from maasapiserver.v3.constants import V3_API_PREFIX from maasservicelayer.auth.jwt import UserRole -from maasservicelayer.exceptions.catalog import NotFoundException +from maasservicelayer.exceptions.catalog import ( + BaseExceptionDetail, + ConflictException, + NotFoundException, +) +from maasservicelayer.exceptions.constants import ( + UNIQUE_CONSTRAINT_VIOLATION_TYPE, +) from maasservicelayer.services import ServiceCollectionV3 +from maasservicelayer.services.usergroups import ( + UserAlreadyInGroup, + UserGroupNotFound, +) class UserGroupsHandler(Handler): @@ -200,3 +218,111 @@ async def delete_group( ) -> Response: await services.usergroups.delete_by_id(group_id, etag_if_match) return Response(status_code=status.HTTP_204_NO_CONTENT) + + # Membership endpoints + + @handler( + path="/groups/{group_id}/members", + methods=["GET"], + tags=TAGS, + responses={ + 200: { + "model": UserGroupMembersListResponse, + }, + 404: {"model": NotFoundBodyResponse}, + }, + response_model_exclude_none=True, + status_code=200, + dependencies=[ + Depends(check_permissions(required_roles={UserRole.USER})) + ], + ) + async def list_group_members( + self, + group_id: int, + services: ServiceCollectionV3 = Depends(services), # noqa: B008 + ) -> UserGroupMembersListResponse: + group = await services.usergroups.get_by_id(group_id) + if not group: + raise NotFoundException() + + members = await services.usergroups.list_usergroup_members(group_id) + return UserGroupMembersListResponse( + items=[ + UserGroupMemberResponse.from_model(member) + for member in members + ], + ) + + @handler( + path="/groups/{group_id}/members", + methods=["POST"], + tags=TAGS, + responses={ + 200: { + "model": UserGroupMembersListResponse, + }, + 404: {"model": NotFoundBodyResponse}, + 409: {"model": ConflictBodyResponse}, + }, + response_model_exclude_none=True, + status_code=200, + dependencies=[ + Depends(check_permissions(required_roles={UserRole.ADMIN})) + ], + ) + async def add_group_member( + self, + group_id: int, + member_request: UserGroupMemberRequest, + services: ServiceCollectionV3 = Depends(services), # noqa: B008 + ) -> UserGroupMembersListResponse: + try: + await services.usergroups.add_user_to_group_by_id( + member_request.user_id, group_id + ) + except UserGroupNotFound as err: + raise NotFoundException() from err + except UserAlreadyInGroup as err: + raise ConflictException( + details=[ + BaseExceptionDetail( + type=UNIQUE_CONSTRAINT_VIOLATION_TYPE, + message=str(err), + ) + ] + ) from err + + members = await services.usergroups.list_usergroup_members(group_id) + return UserGroupMembersListResponse( + items=[ + UserGroupMemberResponse.from_model(member) + for member in members + ], + ) + + @handler( + path="/groups/{group_id}/members/{user_id}", + methods=["DELETE"], + tags=TAGS, + responses={ + 204: {}, + 404: {"model": NotFoundBodyResponse}, + }, + status_code=204, + dependencies=[ + Depends(check_permissions(required_roles={UserRole.ADMIN})) + ], + ) + async def remove_group_member( + self, + group_id: int, + user_id: int, + services: ServiceCollectionV3 = Depends(services), # noqa: B008 + ) -> Response: + group = await services.usergroups.get_by_id(group_id) + if not group: + raise NotFoundException() + + await services.usergroups.remove_user_from_group(group_id, user_id) + return Response(status_code=status.HTTP_204_NO_CONTENT) diff --git a/src/maasapiserver/v3/api/public/models/requests/usergroup_members.py b/src/maasapiserver/v3/api/public/models/requests/usergroup_members.py new file mode 100644 index 0000000000..802b848865 --- /dev/null +++ b/src/maasapiserver/v3/api/public/models/requests/usergroup_members.py @@ -0,0 +1,8 @@ +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +from pydantic import BaseModel, Field + + +class UserGroupMemberRequest(BaseModel): + user_id: int = Field(description="The ID of the user to add to the group.") diff --git a/src/maasapiserver/v3/api/public/models/responses/usergroup_members.py b/src/maasapiserver/v3/api/public/models/responses/usergroup_members.py new file mode 100644 index 0000000000..528fcd0c70 --- /dev/null +++ b/src/maasapiserver/v3/api/public/models/responses/usergroup_members.py @@ -0,0 +1,28 @@ +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +from typing import Optional, Self + +from pydantic import BaseModel + +from maasservicelayer.models.usergroup_members import UserGroupMember + + +class UserGroupMemberResponse(BaseModel): + kind = "UserGroupMember" + user_id: int + username: str + email: str + + @classmethod + def from_model(cls, member: UserGroupMember) -> Self: + return cls( + user_id=member.id, + username=member.username, + email=member.email, + ) + + +class UserGroupMembersListResponse(BaseModel): + kind = "UserGroupMembersList" + items: list[UserGroupMemberResponse] diff --git a/src/maasopenfga/internal/migrations/00002_migrate_environments.go b/src/maasopenfga/internal/migrations/00002_migrate_environments.go index d52a38df44..13edb74cae 100644 --- a/src/maasopenfga/internal/migrations/00002_migrate_environments.go +++ b/src/maasopenfga/internal/migrations/00002_migrate_environments.go @@ -185,6 +185,7 @@ func addUsersToGroup(ctx context.Context, tx *sql.Tx, administratorGroupID int64 selectStmt, selectArgs, err := builder. Select("id", "is_superuser"). From("auth_user"). + Where(sq.Or{sq.NotEq{"username": "MAAS"}, sq.NotEq{"username": "maas-init-node"}}). // Ignore the internal users ToSql() if err != nil { return err diff --git a/src/maasserver/api/tests/test_usergroups.py b/src/maasserver/api/tests/test_usergroups.py index 2eadcb5702..e2cf26aaaf 100644 --- a/src/maasserver/api/tests/test_usergroups.py +++ b/src/maasserver/api/tests/test_usergroups.py @@ -1,5 +1,5 @@ -# Copyright 2026 Canonical Ltd. This software is licensed under the -# GNU Affero General Public License version 3 (see the file LICENSE). +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). """Tests for `UserGroup` API.""" @@ -11,6 +11,7 @@ from maasserver.auth.tests.test_auth import OpenFGAMockMixin from maasserver.testing.api import APITestCase +from maasserver.testing.factory import factory def _parse(response): @@ -135,20 +136,19 @@ def test_handler_path(self): def test_read(self): self.become_admin() - created, _ = _create_group(self.client, "read-group", "Readable") - group_id = created["id"] + group = factory.make_Usergroup() response = self.client.get( - reverse("usergroup_handler", args=[group_id]) + reverse("usergroup_handler", args=[group.id]) ) self.assertEqual( http.client.OK, response.status_code, response.content ) parsed = _parse(response) - self.assertEqual("read-group", parsed["name"]) - self.assertEqual("Readable", parsed["description"]) + self.assertEqual(group.name, parsed["name"]) + self.assertEqual(group.description, parsed["description"]) self.assertEqual( - f"/MAAS/api/2.0/groups/{group_id}/", parsed["resource_uri"] + f"/MAAS/api/2.0/groups/{group.id}/", parsed["resource_uri"] ) def test_read_404(self): @@ -160,14 +160,13 @@ def test_read_404(self): def test_update_requires_admin(self): self.become_admin() - created, _ = _create_group(self.client, "update-group") - group_id = created["id"] + group = factory.make_Usergroup() self.user.is_superuser = False self.user.save() response = self.client.put( - reverse("usergroup_handler", args=[group_id]), + reverse("usergroup_handler", args=[group.id]), {"name": "new-name"}, ) self.assertEqual( @@ -176,11 +175,10 @@ def test_update_requires_admin(self): def test_update(self): self.become_admin() - created, _ = _create_group(self.client, "old-name", "old desc") - group_id = created["id"] + group = factory.make_Usergroup() response = self.client.put( - reverse("usergroup_handler", args=[group_id]), + reverse("usergroup_handler", args=[group.id]), {"name": "new-name", "description": "new desc"}, ) self.assertEqual( @@ -192,11 +190,10 @@ def test_update(self): def test_update_name_only(self): self.become_admin() - created, _ = _create_group(self.client, "partial-name", "keep this") - group_id = created["id"] + group = factory.make_Usergroup() response = self.client.put( - reverse("usergroup_handler", args=[group_id]), + reverse("usergroup_handler", args=[group.id]), {"name": "changed-name"}, ) self.assertEqual( @@ -204,22 +201,21 @@ def test_update_name_only(self): ) parsed = _parse(response) self.assertEqual("changed-name", parsed["name"]) - self.assertEqual("keep this", parsed["description"]) + self.assertEqual(group.description, parsed["description"]) def test_update_description_only(self): self.become_admin() - created, _ = _create_group(self.client, "keep-name", "old") - group_id = created["id"] + group = factory.make_Usergroup() response = self.client.put( - reverse("usergroup_handler", args=[group_id]), + reverse("usergroup_handler", args=[group.id]), {"description": "updated"}, ) self.assertEqual( http.client.OK, response.status_code, response.content ) parsed = _parse(response) - self.assertEqual("keep-name", parsed["name"]) + self.assertEqual(group.name, parsed["name"]) self.assertEqual("updated", parsed["description"]) def test_update_404(self): @@ -233,15 +229,10 @@ def test_update_404(self): ) def test_delete_requires_admin(self): - self.become_admin() - created, _ = _create_group(self.client, "delete-group") - group_id = created["id"] - - self.user.is_superuser = False - self.user.save() + group = factory.make_Usergroup() response = self.client.delete( - reverse("usergroup_handler", args=[group_id]) + reverse("usergroup_handler", args=[group.id]) ) self.assertEqual( http.client.FORBIDDEN, response.status_code, response.content @@ -249,11 +240,10 @@ def test_delete_requires_admin(self): def test_delete(self): self.become_admin() - created, _ = _create_group(self.client, "to-delete") - group_id = created["id"] + group = factory.make_Usergroup() response = self.client.delete( - reverse("usergroup_handler", args=[group_id]) + reverse("usergroup_handler", args=[group.id]) ) self.assertEqual( http.client.NO_CONTENT, response.status_code, response.content @@ -261,7 +251,7 @@ def test_delete(self): # Verify it's gone response = self.client.get( - reverse("usergroup_handler", args=[group_id]) + reverse("usergroup_handler", args=[group.id]) ) self.assertEqual( http.client.NOT_FOUND, response.status_code, response.content @@ -276,6 +266,216 @@ def test_delete_404(self): http.client.NOT_FOUND, response.status_code, response.content ) + # Membership endpoints + def test_list_members_requires_admin(self): + group = factory.make_Usergroup() + + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) + + def test_list_members_empty(self): + self.become_admin() + group = factory.make_Usergroup() + + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + self.assertEqual([], _parse(response)) + + def test_list_members_404(self): + self.become_admin() + response = self.client.get( + reverse("usergroup_handler", args=[99999]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.NOT_FOUND, response.status_code, response.content + ) + + def test_list_members_returns_added_members(self): + self.become_admin() + group = factory.make_Usergroup() + member = factory.make_User() + + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + parsed = _parse(response) + usernames = {m["username"] for m in parsed} + self.assertIn(member.username, usernames) + + def test_add_member_requires_admin(self): + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": "someone"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) + + def test_add_member(self): + self.become_admin() + group = factory.make_Usergroup() + member = factory.make_User() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + + def test_add_member_requires_username(self): + self.become_admin() + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member"}, + ) + self.assertEqual( + http.client.BAD_REQUEST, response.status_code, response.content + ) + + def test_add_member_user_not_found(self): + self.become_admin() + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": "nonexistent-user"}, + ) + self.assertEqual( + http.client.NOT_FOUND, response.status_code, response.content + ) + + def test_add_member_group_not_found(self): + self.become_admin() + member = factory.make_User() + + response = self.client.post( + reverse("usergroup_handler", args=[99999]), + {"op": "add_member", "username": member.username}, + ) + self.assertEqual( + http.client.NOT_FOUND, response.status_code, response.content + ) + + def test_add_member_already_in_group(self): + self.become_admin() + group = factory.make_Usergroup() + member = factory.make_User() + + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + self.assertEqual( + http.client.CONFLICT, response.status_code, response.content + ) + + def test_remove_member_requires_admin(self): + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": "someone"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) + + def test_remove_member(self): + self.become_admin() + group = factory.make_Usergroup() + member = factory.make_User() + + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": member.username}, + ) + self.assertEqual( + http.client.NO_CONTENT, response.status_code, response.content + ) + + def test_remove_member_requires_username(self): + self.become_admin() + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member"}, + ) + self.assertEqual( + http.client.BAD_REQUEST, response.status_code, response.content + ) + + def test_remove_member_user_not_found(self): + self.become_admin() + group = factory.make_Usergroup() + + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": "nonexistent-user"}, + ) + self.assertEqual( + http.client.NOT_FOUND, response.status_code, response.content + ) + + def test_remove_member_not_in_group_after_removal(self): + self.become_admin() + group = factory.make_Usergroup() + member = factory.make_User() + + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": member.username}, + ) + + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + parsed = _parse(response) + usernames = {m["username"] for m in parsed} + self.assertNotIn(member.username, usernames) + class TestUserGroupsOpenFGAIntegration(OpenFGAMockMixin, APITestCase.ForUser): def _create_group_as_admin(self, name, description="test"): @@ -393,3 +593,87 @@ def test_delete_denied_without_edit_permission(self): self.assertEqual( http.client.FORBIDDEN, response.status_code, response.content ) + + def test_list_members_requires_can_view_identities(self): + group = factory.make_Usergroup() + self.openfga_client.can_view_identities.return_value = True + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + self.openfga_client.can_view_identities.assert_called_once_with( + self.user + ) + + def test_list_members_denied_without_view_permission(self): + group = factory.make_Usergroup() + self.openfga_client.can_view_identities.return_value = False + response = self.client.get( + reverse("usergroup_handler", args=[group.id]), + {"op": "list_members"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) + + def test_add_member_requires_can_edit_identities(self): + group = factory.make_Usergroup() + self.openfga_client.can_edit_identities.return_value = True + member = factory.make_User() + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + self.assertEqual( + http.client.OK, response.status_code, response.content + ) + self.openfga_client.can_edit_identities.assert_called_once_with( + self.user + ) + + def test_add_member_denied_without_edit_permission(self): + group = factory.make_Usergroup() + self.openfga_client.can_edit_identities.return_value = False + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": "someone"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) + + def test_remove_member_requires_can_edit_identities(self): + group = factory.make_Usergroup() + self.openfga_client.can_edit_identities.return_value = True + member = factory.make_User() + + self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "add_member", "username": member.username}, + ) + self.openfga_client.reset_mock() + self.openfga_client.can_edit_identities.return_value = True + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": member.username}, + ) + self.assertEqual( + http.client.NO_CONTENT, response.status_code, response.content + ) + self.openfga_client.can_edit_identities.assert_called_once_with( + self.user + ) + + def test_remove_member_denied_without_edit_permission(self): + group = factory.make_Usergroup() + self.openfga_client.can_edit_identities.return_value = False + response = self.client.post( + reverse("usergroup_handler", args=[group.id]), + {"op": "remove_member", "username": "someone"}, + ) + self.assertEqual( + http.client.FORBIDDEN, response.status_code, response.content + ) diff --git a/src/maasserver/api/usergroups.py b/src/maasserver/api/usergroups.py index f97ae30628..cac4f0068c 100644 --- a/src/maasserver/api/usergroups.py +++ b/src/maasserver/api/usergroups.py @@ -5,10 +5,22 @@ from piston3.utils import rc -from maasserver.api.support import check_permission, OperationsHandler -from maasserver.exceptions import MAASAPIBadRequest, MAASAPINotFound +from maasserver.api.support import ( + check_permission, + operation, + OperationsHandler, +) +from maasserver.exceptions import ( + MAASAPIBadRequest, + MAASAPINotFound, + UserAlreadyGroupMemberConflict, +) from maasserver.sqlalchemy import service_layer from maasservicelayer.builders.usergroups import UserGroupBuilder +from maasservicelayer.services.usergroups import ( + UserAlreadyInGroup, + UserGroupNotFound, +) DISPLAYED_USERGROUP_FIELDS = ("id", "name", "description") @@ -97,6 +109,101 @@ def delete(self, request, id): service_layer.services.usergroups.delete_by_id(int(id)) return rc.DELETED + @operation(idempotent=True) + @check_permission("can_view_identities") + def list_members(self, request, id): + """@description Lists members of a user group. + @param (url-string) "{id}" [required=true] A group ID. + + @success (http-status-code) "server_success" 200 + @success (json) "content_success" A JSON list of group members. + + @error (http-status-code) "404" 404 + @error (content) "notfound" The group is not found. + """ + group = service_layer.services.usergroups.get_by_id(int(id)) + if group is None: + raise MAASAPINotFound(f"UserGroup with id {id} not found.") + + members = service_layer.services.usergroups.list_usergroup_members( + int(id) + ) + return [ + { + "username": member.username, + "email": member.email, + } + for member in members + ] + + @operation(idempotent=False) + @check_permission("can_edit_identities") + def add_member(self, request, id): + """@description Adds a user to a user group. + @param (url-string) "{id}" [required=true] A group ID. + @param (string) "username" [required=true] The username to add. + + @success (http-status-code) "server_success" 200 + @success (json) "content_success" The updated list of group members. + + @error (http-status-code) "400" 400 + @error (content) "badrequest" username is required. + @error (http-status-code) "404" 404 + @error (content) "notfound" The group or user is not found. + @error (http-status-code) "409" 409 + @error (content) "conflict" The user is already a member of the group. + + """ + username = request.data.get("username") + if not username: + raise MAASAPIBadRequest("username is required.") + + user = service_layer.services.users.get_by_username(username) + if user is None: + raise MAASAPINotFound(f"User '{username}' not found.") + + try: + service_layer.services.usergroups.add_user_to_group_by_id( + user.id, int(id) + ) + except UserGroupNotFound as err: + raise MAASAPINotFound( + f"UserGroup with id {id} not found." + ) from err + except UserAlreadyInGroup as err: + raise UserAlreadyGroupMemberConflict( + f"User `{user.username}` is already a member of the group with ID `{id}`." + ) from err + + return rc.ALL_OK + + @operation(idempotent=False) + @check_permission("can_edit_identities") + def remove_member(self, request, id): + """@description Removes a user from a user group. + @param (url-string) "{id}" [required=true] A group ID. + @param (string) "username" [required=true] The username to remove. + + @success (http-status-code) "server_success" 204 + + @error (http-status-code) "400" 400 + @error (content) "badrequest" username is required. + @error (http-status-code) "404" 404 + @error (content) "notfound" The group or user is not found. + """ + username = request.data.get("username") + if not username: + raise MAASAPIBadRequest("username is required.") + + user = service_layer.services.users.get_by_username(username) + if user is None: + raise MAASAPINotFound(f"User '{username}' not found.") + + service_layer.services.usergroups.remove_user_from_group( + int(id), user.id + ) + return rc.DELETED + class UserGroupsHandler(OperationsHandler): """Manage user groups.""" diff --git a/src/maasserver/exceptions.py b/src/maasserver/exceptions.py index 32441b7026..7a27bd2a06 100644 --- a/src/maasserver/exceptions.py +++ b/src/maasserver/exceptions.py @@ -1,4 +1,4 @@ -# Copyright 2012-2017 Canonical Ltd. This software is licensed under the +# Copyright 2012-2026 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Exceptions.""" @@ -255,3 +255,9 @@ class NetworkingResetProblem(MAASException): class MAASBadDeprecation(MAASException): """Raised when an API endpoint or operation has it's deprecation incorrectly assigned.""" + + +class UserAlreadyGroupMemberConflict(MAASAPIException): + """Raised when a user is already member of a UserGroup.""" + + api_error = int(http.client.CONFLICT) diff --git a/src/maasserver/management/commands/dbupgrade.py b/src/maasserver/management/commands/dbupgrade.py index dcc5f11aa5..024fbf777d 100644 --- a/src/maasserver/management/commands/dbupgrade.py +++ b/src/maasserver/management/commands/dbupgrade.py @@ -1,4 +1,4 @@ -# Copyright 2015-2016 Canonical Ltd. This software is licensed under the +# Copyright 2015-2026 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """ @@ -316,6 +316,14 @@ def handle(self, *args, **options): verbosity=options.get("verbosity"), ) + # When we execute the unit tests we don't have OpenFGA built binaries available at the location where the migrator + # expects them, so we let the unit tests specify where to find them. We have to run the openfga built-in migrations before the alembic ones because the alembic migrations depend on some of the database structures created by the openfga built-in migrations. + openfga_path = options.get("openfga_path") + openfga_dsn = self._build_postgres_dsn( + conn.get_connection_params(), "postgres", search_path="openfga" + ) + self._openfga_migration(openfga_path, database, openfga_dsn) + # Run alembic migrations alembic_ini_path = str( files("maasservicelayer.db.alembic") / "alembic.ini" @@ -338,14 +346,6 @@ def handle(self, *args, **options): self._temporal_migration(database) - # When we execute the unit tests we don't have OpenFGA built binaries available at the location where the migrator - # expects them, so we let the unit tests specify where to find them. - openfga_path = options.get("openfga_path") - openfga_dsn = self._build_postgres_dsn( - conn.get_connection_params(), "postgres", search_path="openfga" - ) - self._openfga_migration(openfga_path, database, openfga_dsn) - openfga_app_dsn = self._build_postgres_dsn( conn.get_connection_params(), "postgres" ) diff --git a/src/maasserver/models/signals/tests/test_users.py b/src/maasserver/models/signals/tests/test_users.py index 1010d4e987..a84c214653 100644 --- a/src/maasserver/models/signals/tests/test_users.py +++ b/src/maasserver/models/signals/tests/test_users.py @@ -8,8 +8,10 @@ from maasserver.sqlalchemy import service_layer from maasserver.testing.factory import factory from maasserver.testing.testcase import MAASServerTestCase +from maasserver.worker_user import get_worker_user from maasservicelayer.db.filters import QuerySpec from maasservicelayer.db.repositories.usergroups import UserGroupsClauseFactory +from metadataserver.nodeinituser import get_node_init_user class TestUserUsername(MAASServerTestCase): @@ -57,6 +59,25 @@ def test_save_creates_openfga_tuple(self): self.assertEqual("member", openfga_tuple[2]) +class TestPostSaveUserSystemUsersIgnoredSignal(MAASServerTestCase): + scenarios = ( + ("MAAS", {"user_factory": get_worker_user}), + ("maas-init-node", {"user_factory": get_node_init_user}), + ) + + def test_save_user_is_ignored(self): + user = self.user_factory() + + with connection.cursor() as cursor: + cursor.execute( + "SELECT object_type, object_id, relation FROM openfga.tuple WHERE _user = 'user:%s'", + [user.id], + ) + openfga_tuple = cursor.fetchone() + + self.assertIsNone(openfga_tuple) + + class TestPostDeleteUserSignal(MAASServerTestCase): def test_delete_removes_openfga_tuple(self): user = factory.make_User() diff --git a/src/maasserver/models/signals/users.py b/src/maasserver/models/signals/users.py index 88f55c8568..7f1534ddf2 100644 --- a/src/maasserver/models/signals/users.py +++ b/src/maasserver/models/signals/users.py @@ -7,6 +7,7 @@ from django.db.models.signals import post_delete, post_save, pre_delete from maasserver.models import Event +from maasserver.models.user import SYSTEM_USERS from maasserver.sqlalchemy import service_layer from maasserver.utils.signals import SignalsManager @@ -25,11 +26,14 @@ def pre_delete_set_event_username(sender, instance, **kwargs): def post_created_user(sender, instance, created, **kwargs): if created: - # Guarantee backwards compatibility and assign users to pre-defined groups (users/administrators) - service_layer.services.usergroups.add_user_to_group( - instance.id, - "Administrators" if instance.is_superuser else "Users", - ) + if ( + instance.username not in SYSTEM_USERS + ): # Do not add system users to groups. + # Guarantee backwards compatibility and assign users to pre-defined groups (users/administrators) + service_layer.services.usergroups.add_user_to_group( + instance.id, + "Administrators" if instance.is_superuser else "Users", + ) def post_delete_user(sender, instance, **kwargs): diff --git a/src/maasserver/testing/factory.py b/src/maasserver/testing/factory.py index 108ba94656..ac6315eac8 100644 --- a/src/maasserver/testing/factory.py +++ b/src/maasserver/testing/factory.py @@ -1,4 +1,4 @@ -# Copyright 2012-2025 Canonical Ltd. This software is licensed under the +# Copyright 2012-2026 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). @@ -119,6 +119,7 @@ from maasserver.models.virtualmachine import VirtualMachine, VirtualMachineDisk from maasserver.node_status import NODE_TRANSITIONS from maasserver.sessiontimeout import SessionStore +from maasserver.sqlalchemy import service_layer from maasserver.storage_layouts import MIN_BOOT_PARTITION_SIZE from maasserver.testing import get_data from maasserver.testing.testclient import MAASSensibleRequestFactory @@ -126,6 +127,8 @@ from maasserver.utils.orm import get_one, post_commit_hooks, reload_object from maasserver.utils.osystems import get_release_from_distro_info from maasserver.worker_user import get_worker_user +from maasservicelayer.builders.usergroups import UserGroupBuilder +from maasservicelayer.models.usergroups import UserGroup from maasservicelayer.utils.image_local_files import SyncLocalBootResourceFile import maastesting.factory from maastesting.factory import TooManyRandomRetries @@ -1440,6 +1443,15 @@ def _make_session(self, user): session.save() return session + def make_Usergroup(self, name=None, description=None) -> UserGroup: + if name is None: + name = self.make_name("group") + if description is None: + description = self.make_string() + builder = UserGroupBuilder(name=name, description=description) + group = service_layer.services.usergroups.create(builder) + return group + def make_ResourcePool(self, name=None, description=None, nodes=None): if name is None: name = self.make_name("resourcepool") diff --git a/src/maasserver/testing/initial.maas_test.sql b/src/maasserver/testing/initial.maas_test.sql index 9244b65cae..6c789517db 100644 --- a/src/maasserver/testing/initial.maas_test.sql +++ b/src/maasserver/testing/initial.maas_test.sql @@ -7471,6 +7471,20 @@ ALTER TABLE public.maasserver_usergroup ALTER COLUMN id ADD GENERATED BY DEFAULT ); +-- +-- Name: maasserver_usergroup_members_view; Type: VIEW; Schema: public; Owner: - +-- + +CREATE VIEW public.maasserver_usergroup_members_view AS + SELECT (t.object_id)::integer AS group_id, + u.id, + u.username, + u.email + FROM (openfga.tuple t + JOIN public.auth_user u ON ((u.id = (split_part(t._user, ':'::text, 2))::integer))) + WHERE ((t.object_type = 'group'::text) AND (t.relation = 'member'::text) AND (t.user_type = 'user'::text)); + + -- -- Name: maasserver_userprofile; Type: TABLE; Schema: public; Owner: - -- @@ -8581,7 +8595,7 @@ COPY openfga.tuple (store, object_type, object_id, relation, _user, user_type, u -- COPY public.alembic_version (version_num) FROM stdin; -0019 +0020 \. diff --git a/src/maasservicelayer/db/alembic/versions/0020_create_maasserver_user_group_membership.py b/src/maasservicelayer/db/alembic/versions/0020_create_maasserver_user_group_membership.py new file mode 100644 index 0000000000..b712acdfdf --- /dev/null +++ b/src/maasservicelayer/db/alembic/versions/0020_create_maasserver_user_group_membership.py @@ -0,0 +1,45 @@ +"""Create maasserver_usergroup_members_view table + +Revision ID: 0020 +Revises: 0019 +Create Date: 2026-03-03 16:34:42.720984+00:00 +""" + +from textwrap import dedent +from typing import Sequence + +from alembic import op + +# revision identifiers, used by Alembic. +revision: str = "0020" +down_revision: str | None = "0019" +branch_labels: str | Sequence[str] | None = None +depends_on: str | Sequence[str] | None = None + + +def upgrade() -> None: + sql = dedent("""\ + SELECT + (t.object_id)::integer AS group_id, + u.id as id, + u.username as username, + u.email as email + FROM + openfga.tuple t + JOIN + auth_user u + ON u.id = split_part(t._user, ':', 2)::integer + WHERE + t.object_type = 'group' + AND t.relation = 'member' + AND t.user_type = 'user' + """) + + op.execute( + f"""CREATE OR REPLACE VIEW maasserver_usergroup_members_view AS ({sql});""" + ) + + +def downgrade() -> None: + # We do not support migration downgrade + pass diff --git a/src/maasservicelayer/db/alembic/versions/__init__.py b/src/maasservicelayer/db/alembic/versions/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/maasservicelayer/db/repositories/usergroups_members.py b/src/maasservicelayer/db/repositories/usergroups_members.py new file mode 100644 index 0000000000..8e9523b325 --- /dev/null +++ b/src/maasservicelayer/db/repositories/usergroups_members.py @@ -0,0 +1,34 @@ +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +from typing import Type + +from sqlalchemy import Table +from sqlalchemy.sql.operators import eq + +from maasservicelayer.db.filters import Clause, ClauseFactory +from maasservicelayer.db.repositories.base import ReadOnlyRepository +from maasservicelayer.db.tables import UserGroupMembersView +from maasservicelayer.models.usergroup_members import UserGroupMember + + +class UserGroupMembersClauseFactory(ClauseFactory): + @classmethod + def with_group_id(cls, group_id: int) -> Clause: + return Clause(condition=eq(UserGroupMembersView.c.group_id, group_id)) + + @classmethod + def with_username(cls, username: str) -> Clause: + return Clause(condition=eq(UserGroupMembersView.c.username, username)) + + @classmethod + def with_id(cls, user_id: int) -> Clause: + return Clause(condition=eq(UserGroupMembersView.c.id, user_id)) + + +class UserGroupMembersRepository(ReadOnlyRepository[UserGroupMember]): + def get_repository_table(self) -> Table: + return UserGroupMembersView + + def get_model_factory(self) -> Type[UserGroupMember]: + return UserGroupMember diff --git a/src/maasservicelayer/db/tables.py b/src/maasservicelayer/db/tables.py index 1992b439c2..e0df194327 100644 --- a/src/maasservicelayer/db/tables.py +++ b/src/maasservicelayer/db/tables.py @@ -419,6 +419,7 @@ "boot_source_id", ), ) + BootSourceSelectionStatusView = Table( "maasserver_bootsourceselectionstatus_view", METADATA, @@ -2353,6 +2354,15 @@ ), ) +UserGroupMembersView = Table( + "maasserver_usergroup_members_view", + METADATA, + Column("id", Integer, Identity(), primary_key=True), + Column("group_id", String, primary_key=True), + Column("username", String(150), nullable=False, unique=True), + Column("email", String(254), nullable=True, unique=True), +) + UserProfileTable = Table( "maasserver_userprofile", METADATA, diff --git a/src/maasservicelayer/models/usergroup_members.py b/src/maasservicelayer/models/usergroup_members.py new file mode 100644 index 0000000000..7cd294d073 --- /dev/null +++ b/src/maasservicelayer/models/usergroup_members.py @@ -0,0 +1,10 @@ +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +from maasservicelayer.models.base import MaasBaseModel + + +class UserGroupMember(MaasBaseModel): + group_id: int + username: str + email: str diff --git a/src/maasservicelayer/services/__init__.py b/src/maasservicelayer/services/__init__.py index 394bc7ca46..268bf6e4cf 100644 --- a/src/maasservicelayer/services/__init__.py +++ b/src/maasservicelayer/services/__init__.py @@ -112,6 +112,9 @@ ) from maasservicelayer.db.repositories.ui_subnets import UISubnetsRepository from maasservicelayer.db.repositories.usergroups import UserGroupsRepository +from maasservicelayer.db.repositories.usergroups_members import ( + UserGroupMembersRepository, +) from maasservicelayer.db.repositories.users import UsersRepository from maasservicelayer.db.repositories.vlans import VlansRepository from maasservicelayer.db.repositories.vmcluster import VmClustersRepository @@ -494,6 +497,7 @@ async def produce( services.usergroups = UserGroupsService( context=context, usergroups_repository=UserGroupsRepository(context), + usergroup_members_repository=UserGroupMembersRepository(context), openfga_tuples_service=services.openfga_tuples, ) services.machines = MachinesService( diff --git a/src/maasservicelayer/services/openfga_tuples.py b/src/maasservicelayer/services/openfga_tuples.py index 520b26b8c1..09a2da5f2e 100644 --- a/src/maasservicelayer/services/openfga_tuples.py +++ b/src/maasservicelayer/services/openfga_tuples.py @@ -93,3 +93,19 @@ async def delete_group(self, group_id: int) -> None: ) ) await self.delete_many(membership_query) + + async def remove_user_from_group( # TODO @r00ta: test + self, group_id: int, user_id: int + ) -> None: + query = QuerySpec( + where=OpenFGATuplesClauseFactory.and_clauses( + [ + OpenFGATuplesClauseFactory.with_user(f"user:{user_id}"), + OpenFGATuplesClauseFactory.with_relation("member"), + OpenFGATuplesClauseFactory.with_object_type("group"), + OpenFGATuplesClauseFactory.with_object_type("group"), + OpenFGATuplesClauseFactory.with_object_id(str(group_id)), + ] + ) + ) + await self.delete_many(query) diff --git a/src/maasservicelayer/services/usergroups.py b/src/maasservicelayer/services/usergroups.py index 855302c73b..e2eda87ecf 100644 --- a/src/maasservicelayer/services/usergroups.py +++ b/src/maasservicelayer/services/usergroups.py @@ -11,6 +11,11 @@ UserGroupsClauseFactory, UserGroupsRepository, ) +from maasservicelayer.db.repositories.usergroups_members import ( + UserGroupMembersClauseFactory, + UserGroupMembersRepository, +) +from maasservicelayer.models.usergroup_members import UserGroupMember from maasservicelayer.models.usergroups import UserGroup from maasservicelayer.services.base import BaseService from maasservicelayer.services.openfga_tuples import OpenFGATupleService @@ -20,6 +25,10 @@ class UserGroupNotFound(Exception): """Raised when a user group is not found in the database.""" +class UserAlreadyInGroup(Exception): + """Raised when a user is already a member of the group.""" + + class UserGroupsService( BaseService[UserGroup, UserGroupsRepository, UserGroupBuilder] ): @@ -29,11 +38,38 @@ def __init__( self, context: Context, usergroups_repository: UserGroupsRepository, + usergroup_members_repository: UserGroupMembersRepository, openfga_tuples_service: OpenFGATupleService, ): super().__init__(context, usergroups_repository) + self.usergroup_members_repository = usergroup_members_repository self.openfga_tuples_service = openfga_tuples_service + async def post_delete_hook(self, resource: UserGroup) -> None: + await self.openfga_tuples_service.delete_group(resource.id) + + async def post_delete_many_hook(self, resources: List[UserGroup]) -> None: + raise NotImplementedError( + "Deleting multiple user groups is not supported." + ) + + # User management methods. + async def _check_already_member(self, user_id: int, group_id: int) -> None: + exists = await self.usergroup_members_repository.exists( + QuerySpec( + where=UserGroupMembersClauseFactory.and_clauses( + [ + UserGroupMembersClauseFactory.with_group_id(group_id), + UserGroupMembersClauseFactory.with_id(user_id), + ] + ) + ) + ) + if exists: + raise UserAlreadyInGroup( + f"User {user_id} is already a member of group {group_id}." + ) + async def add_user_to_group(self, user_id: int, group_name: str): group = await self.get_one( QuerySpec(where=UserGroupsClauseFactory.with_name(group_name)) @@ -41,14 +77,31 @@ async def add_user_to_group(self, user_id: int, group_name: str): if group is None: raise UserGroupNotFound() + await self._check_already_member(user_id, group.id) await self.openfga_tuples_service.create( OpenFGATupleBuilder.build_user_member_group(user_id, group.id) ) - async def post_delete_hook(self, resource: UserGroup) -> None: - await self.openfga_tuples_service.delete_group(resource.id) + async def add_user_to_group_by_id(self, user_id: int, group_id: int): + group = await self.get_by_id(group_id) + if group is None: + raise UserGroupNotFound() - async def post_delete_many_hook(self, resources: List[UserGroup]) -> None: - raise NotImplementedError( - "Deleting multiple user groups is not supported." + await self._check_already_member(user_id, group_id) + await self.openfga_tuples_service.create( + OpenFGATupleBuilder.build_user_member_group(user_id, group.id) + ) + + async def list_usergroup_members( + self, group_id: int + ) -> List[UserGroupMember]: + return await self.usergroup_members_repository.list_all( + QuerySpec( + where=UserGroupMembersClauseFactory.with_group_id(group_id) + ) + ) + + async def remove_user_from_group(self, group_id: int, user_id: int): + await self.openfga_tuples_service.remove_user_from_group( + group_id, user_id ) diff --git a/src/tests/fixtures/factories/user.py b/src/tests/fixtures/factories/user.py index dcd9c93317..2b97a65a3a 100644 --- a/src/tests/fixtures/factories/user.py +++ b/src/tests/fixtures/factories/user.py @@ -1,4 +1,4 @@ -# Copyright 2024-2025 Canonical Ltd. This software is licensed under the +# Copyright 2024-2026 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). from datetime import datetime, timedelta @@ -23,6 +23,7 @@ async def create_test_user(fixture: Fixture, **extra_details: Any) -> User: "is_superuser": False, "first_name": "first", "last_name": "last", + "email": "mail@example.com", "is_staff": False, "is_active": True, "date_joined": date_joined, diff --git a/src/tests/maasapiserver/v3/api/public/handlers/test_usergroups.py b/src/tests/maasapiserver/v3/api/public/handlers/test_usergroups.py index 20d9637e56..16bc636d74 100644 --- a/src/tests/maasapiserver/v3/api/public/handlers/test_usergroups.py +++ b/src/tests/maasapiserver/v3/api/public/handlers/test_usergroups.py @@ -8,9 +8,15 @@ import pytest from maasapiserver.common.api.models.responses.errors import ErrorBodyResponse +from maasapiserver.v3.api.public.models.requests.usergroup_members import ( + UserGroupMemberRequest, +) from maasapiserver.v3.api.public.models.requests.usergroups import ( UserGroupRequest, ) +from maasapiserver.v3.api.public.models.responses.usergroup_members import ( + UserGroupMembersListResponse, +) from maasapiserver.v3.api.public.models.responses.usergroups import ( UserGroupResponse, UserGroupsListResponse, @@ -26,9 +32,14 @@ UNIQUE_CONSTRAINT_VIOLATION_TYPE, ) from maasservicelayer.models.base import ListResult +from maasservicelayer.models.usergroup_members import UserGroupMember from maasservicelayer.models.usergroups import UserGroup from maasservicelayer.services import ServiceCollectionV3 -from maasservicelayer.services.usergroups import UserGroupsService +from maasservicelayer.services.usergroups import ( + UserAlreadyInGroup, + UserGroupNotFound, + UserGroupsService, +) from maasservicelayer.utils.date import utcnow from tests.maasapiserver.v3.api.public.handlers.base import ( ApiCommonTests, @@ -52,6 +63,7 @@ def user_endpoints(self) -> list[Endpoint]: return [ Endpoint(method="GET", path=f"{self.BASE_PATH}"), Endpoint(method="GET", path=f"{self.BASE_PATH}/1"), + Endpoint(method="GET", path=f"{self.BASE_PATH}/1/members"), ] @pytest.fixture @@ -60,6 +72,8 @@ def admin_endpoints(self) -> list[Endpoint]: Endpoint(method="POST", path=f"{self.BASE_PATH}"), Endpoint(method="PUT", path=f"{self.BASE_PATH}/1"), Endpoint(method="DELETE", path=f"{self.BASE_PATH}/1"), + Endpoint(method="POST", path=f"{self.BASE_PATH}/1/members"), + Endpoint(method="DELETE", path=f"{self.BASE_PATH}/1/members/10"), ] # GET /groups @@ -255,3 +269,144 @@ async def test_delete_with_etag( headers={"if-match": "correct"}, ) assert response.status_code == 204 + + # GET /groups/{group_id}/members + async def test_list_members( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_user: AsyncClient, + ) -> None: + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.get_by_id.return_value = TEST_GROUP + services_mock.usergroups.list_usergroup_members.return_value = [ + UserGroupMember( + id=10, group_id=1, username="user1", email="u1@test.com" + ), + UserGroupMember( + id=20, group_id=1, username="user2", email="u2@test.com" + ), + ] + + response = await mocked_api_client_user.get( + f"{self.BASE_PATH}/{TEST_GROUP.id}/members" + ) + assert response.status_code == 200 + members_response = UserGroupMembersListResponse(**response.json()) + assert len(members_response.items) == 2 + assert members_response.items[0].username == "user1" + assert members_response.items[1].username == "user2" + + async def test_list_members_empty( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_user: AsyncClient, + ) -> None: + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.get_by_id.return_value = TEST_GROUP + services_mock.usergroups.list_usergroup_members.return_value = [] + + response = await mocked_api_client_user.get( + f"{self.BASE_PATH}/{TEST_GROUP.id}/members" + ) + assert response.status_code == 200 + members_response = UserGroupMembersListResponse(**response.json()) + assert len(members_response.items) == 0 + + async def test_list_members_404( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_user: AsyncClient, + ) -> None: + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.get_by_id.return_value = None + + response = await mocked_api_client_user.get( + f"{self.BASE_PATH}/999/members" + ) + assert response.status_code == 404 + + # POST /groups/{group_id}/members + async def test_add_member( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_admin: AsyncClient, + ) -> None: + member_request = UserGroupMemberRequest(user_id=10) + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.add_user_to_group_by_id.return_value = None + services_mock.usergroups.list_usergroup_members.return_value = [ + UserGroupMember( + id=10, group_id=1, username="user1", email="u1@test.com" + ), + ] + + response = await mocked_api_client_admin.post( + f"{self.BASE_PATH}/{TEST_GROUP.id}/members", + json=jsonable_encoder(member_request), + ) + assert response.status_code == 200 + members_response = UserGroupMembersListResponse(**response.json()) + assert len(members_response.items) == 1 + assert members_response.items[0].user_id == 10 + + async def test_add_member_group_not_found( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_admin: AsyncClient, + ) -> None: + member_request = UserGroupMemberRequest(user_id=10) + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.add_user_to_group_by_id.side_effect = ( + UserGroupNotFound() + ) + + response = await mocked_api_client_admin.post( + f"{self.BASE_PATH}/999/members", + json=jsonable_encoder(member_request), + ) + assert response.status_code == 404 + + async def test_add_member_already_in_group( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_admin: AsyncClient, + ) -> None: + member_request = UserGroupMemberRequest(user_id=10) + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.add_user_to_group_by_id.side_effect = ( + UserAlreadyInGroup(10, 1) + ) + + response = await mocked_api_client_admin.post( + f"{self.BASE_PATH}/1/members", + json=jsonable_encoder(member_request), + ) + assert response.status_code == 409 + + # DELETE /groups/{group_id}/members/{user_id} + async def test_remove_member( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_admin: AsyncClient, + ) -> None: + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.get_by_id.return_value = TEST_GROUP + services_mock.usergroups.remove_user_from_group.return_value = None + + response = await mocked_api_client_admin.delete( + f"{self.BASE_PATH}/{TEST_GROUP.id}/members/10" + ) + assert response.status_code == 204 + + async def test_remove_member_group_not_found( + self, + services_mock: ServiceCollectionV3, + mocked_api_client_admin: AsyncClient, + ) -> None: + services_mock.usergroups = Mock(UserGroupsService) + services_mock.usergroups.get_by_id.return_value = None + + response = await mocked_api_client_admin.delete( + f"{self.BASE_PATH}/999/members/10" + ) + assert response.status_code == 404 diff --git a/src/tests/maasservicelayer/db/repositories/test_usergroups_members.py b/src/tests/maasservicelayer/db/repositories/test_usergroups_members.py new file mode 100644 index 0000000000..02f04e1893 --- /dev/null +++ b/src/tests/maasservicelayer/db/repositories/test_usergroups_members.py @@ -0,0 +1,92 @@ +# Copyright 2026 Canonical Ltd. This software is licensed under the +# GNU Affero General Public License version 3 (see the file LICENSE). + +import pytest +from sqlalchemy.ext.asyncio import AsyncConnection + +from maasservicelayer.context import Context +from maasservicelayer.db.repositories.usergroups_members import ( + UserGroupMembersClauseFactory, + UserGroupMembersRepository, +) +from maasservicelayer.models.usergroup_members import UserGroupMember +from tests.fixtures.factories.openfga_tuples import create_openfga_tuple +from tests.fixtures.factories.user import create_test_user +from tests.maasapiserver.fixtures.db import Fixture +from tests.maasservicelayer.db.repositories.base import ( + ReadOnlyRepositoryCommonTests, +) + + +class TestUserGroupMembersClauseFactory: + def test_with_group_id(self): + clause = UserGroupMembersClauseFactory.with_group_id(100) + assert str( + clause.condition.compile(compile_kwargs={"literal_binds": True}) + ) == ("maasserver_usergroup_members_view.group_id = 100") + + def test_with_username(self): + clause = UserGroupMembersClauseFactory.with_username("foo") + assert str( + clause.condition.compile(compile_kwargs={"literal_binds": True}) + ) == ("maasserver_usergroup_members_view.username = 'foo'") + + def test_with_id(self): + clause = UserGroupMembersClauseFactory.with_id(100) + assert str( + clause.condition.compile(compile_kwargs={"literal_binds": True}) + ) == ("maasserver_usergroup_members_view.id = 100") + + +class TestUserGroupMembersRepository( + ReadOnlyRepositoryCommonTests[UserGroupMember] +): + @pytest.fixture + def repository_instance( + self, db_connection: AsyncConnection + ) -> UserGroupMembersRepository: + return UserGroupMembersRepository(Context(connection=db_connection)) + + @pytest.fixture + async def _setup_test_list( + self, fixture: Fixture, num_objects: int + ) -> list[UserGroupMember]: + created_members = [] + for i in range(num_objects): + user = await create_test_user( + fixture, username=f"user-{i}", email=f"{i}@example.com" + ) + await create_openfga_tuple( + fixture, + f"user:{user.id}", + "user", + "member", + "group", + "0", + ) + created_members.append( + UserGroupMember( + id=user.id, + group_id=0, + username=user.username, + email=user.email, + ) + ) + return created_members + + @pytest.fixture + async def created_instance(self, fixture: Fixture) -> UserGroupMember: + user = await create_test_user( + fixture, username="user-0", email="0@example.com" + ) + await create_openfga_tuple( + fixture, + f"user:{user.id}", + "user", + "member", + "group", + "0", + ) + return UserGroupMember( + id=user.id, group_id=0, username="user-0", email="0@example.com" + ) diff --git a/src/tests/maasservicelayer/services/test_usergroups.py b/src/tests/maasservicelayer/services/test_usergroups.py index 6b37aaf398..26164f360d 100644 --- a/src/tests/maasservicelayer/services/test_usergroups.py +++ b/src/tests/maasservicelayer/services/test_usergroups.py @@ -15,13 +15,18 @@ UserGroupsClauseFactory, UserGroupsRepository, ) +from maasservicelayer.db.repositories.usergroups_members import ( + UserGroupMembersRepository, +) from maasservicelayer.db.tables import OpenFGATupleTable from maasservicelayer.models.base import MaasBaseModel +from maasservicelayer.models.usergroup_members import UserGroupMember from maasservicelayer.models.usergroups import UserGroup from maasservicelayer.services import ServiceCollectionV3 from maasservicelayer.services.base import BaseService from maasservicelayer.services.openfga_tuples import OpenFGATupleService from maasservicelayer.services.usergroups import ( + UserAlreadyInGroup, UserGroupNotFound, UserGroupsService, ) @@ -48,6 +53,7 @@ def service_instance(self) -> BaseService: return UserGroupsService( context=Context(), usergroups_repository=Mock(UserGroupsRepository), + usergroup_members_repository=Mock(UserGroupMembersRepository), openfga_tuples_service=Mock(OpenFGATupleService), ) @@ -67,20 +73,45 @@ class TestUserGroupsService: async def test_add_user_to_group(self) -> None: usergroups_repository = Mock(UserGroupsRepository) usergroups_repository.get_one.return_value = TEST_GROUP + usergroup_members_repository = Mock(UserGroupMembersRepository) + usergroup_members_repository.exists.return_value = False openfga_tuples_service = Mock(OpenFGATupleService) service = UserGroupsService( context=Context(), usergroups_repository=usergroups_repository, + usergroup_members_repository=usergroup_members_repository, openfga_tuples_service=openfga_tuples_service, ) await service.add_user_to_group(1, TEST_GROUP.name) usergroups_repository.get_one.assert_awaited_once() + usergroup_members_repository.exists.assert_awaited_once() openfga_tuples_service.create.assert_awaited_once_with( OpenFGATupleBuilder.build_user_member_group(1, TEST_GROUP.id) ) + async def test_add_user_to_group_raises_if_already_member(self) -> None: + usergroups_repository = Mock(UserGroupsRepository) + usergroups_repository.get_one.return_value = TEST_GROUP + usergroup_members_repository = Mock(UserGroupMembersRepository) + usergroup_members_repository.exists.return_value = True + openfga_tuples_service = Mock(OpenFGATupleService) + + service = UserGroupsService( + context=Context(), + usergroups_repository=usergroups_repository, + usergroup_members_repository=usergroup_members_repository, + openfga_tuples_service=openfga_tuples_service, + ) + + with pytest.raises(UserAlreadyInGroup): + await service.add_user_to_group(1, TEST_GROUP.name) + + usergroups_repository.get_one.assert_awaited_once() + usergroup_members_repository.exists.assert_awaited_once() + openfga_tuples_service.create.assert_not_awaited() + async def test_delete_by_id(self) -> None: usergroups_repository = Mock(UserGroupsRepository) usergroups_repository.delete_by_id.return_value = TEST_GROUP @@ -90,6 +121,7 @@ async def test_delete_by_id(self) -> None: service = UserGroupsService( context=Context(), usergroups_repository=usergroups_repository, + usergroup_members_repository=Mock(UserGroupMembersRepository), openfga_tuples_service=openfga_tuples_service, ) @@ -101,6 +133,105 @@ async def test_delete_by_id(self) -> None: TEST_GROUP.id ) + async def test_add_user_to_group_by_id(self) -> None: + usergroups_repository = Mock(UserGroupsRepository) + usergroups_repository.get_by_id.return_value = TEST_GROUP + usergroup_members_repository = Mock(UserGroupMembersRepository) + usergroup_members_repository.exists.return_value = False + openfga_tuples_service = Mock(OpenFGATupleService) + + service = UserGroupsService( + context=Context(), + usergroups_repository=usergroups_repository, + usergroup_members_repository=usergroup_members_repository, + openfga_tuples_service=openfga_tuples_service, + ) + + await service.add_user_to_group_by_id(1, TEST_GROUP.id) + usergroups_repository.get_by_id.assert_awaited_once_with( + id=TEST_GROUP.id + ) + usergroup_members_repository.exists.assert_awaited_once() + openfga_tuples_service.create.assert_awaited_once_with( + OpenFGATupleBuilder.build_user_member_group(1, TEST_GROUP.id) + ) + + async def test_add_user_to_group_by_id_raises_if_already_member( + self, + ) -> None: + usergroups_repository = Mock(UserGroupsRepository) + usergroups_repository.get_by_id.return_value = TEST_GROUP + usergroup_members_repository = Mock(UserGroupMembersRepository) + usergroup_members_repository.exists.return_value = True + openfga_tuples_service = Mock(OpenFGATupleService) + + service = UserGroupsService( + context=Context(), + usergroups_repository=usergroups_repository, + usergroup_members_repository=usergroup_members_repository, + openfga_tuples_service=openfga_tuples_service, + ) + + with pytest.raises(UserAlreadyInGroup): + await service.add_user_to_group_by_id(1, TEST_GROUP.id) + usergroups_repository.get_by_id.assert_awaited_once_with( + id=TEST_GROUP.id + ) + usergroup_members_repository.exists.assert_awaited_once() + openfga_tuples_service.create.assert_not_awaited() + + async def test_add_user_to_group_by_id_not_found(self) -> None: + usergroups_repository = Mock(UserGroupsRepository) + usergroups_repository.get_by_id.return_value = None + + service = UserGroupsService( + context=Context(), + usergroups_repository=usergroups_repository, + usergroup_members_repository=Mock(UserGroupMembersRepository), + openfga_tuples_service=Mock(OpenFGATupleService), + ) + + with pytest.raises(UserGroupNotFound): + await service.add_user_to_group_by_id(1, 999) + + async def test_list_usergroup_members(self) -> None: + members = [ + UserGroupMember( + id=10, group_id=1, username="user1", email="u1@test.com" + ), + UserGroupMember( + id=20, group_id=1, username="user2", email="u2@test.com" + ), + ] + usergroup_members_repository = Mock(UserGroupMembersRepository) + usergroup_members_repository.list_all.return_value = members + + service = UserGroupsService( + context=Context(), + usergroups_repository=Mock(UserGroupsRepository), + usergroup_members_repository=usergroup_members_repository, + openfga_tuples_service=Mock(OpenFGATupleService), + ) + + result = await service.list_usergroup_members(1) + assert result == members + usergroup_members_repository.list_all.assert_awaited_once() + + async def test_remove_user_from_group(self) -> None: + openfga_tuples_service = Mock(OpenFGATupleService) + + service = UserGroupsService( + context=Context(), + usergroups_repository=Mock(UserGroupsRepository), + usergroup_members_repository=Mock(UserGroupMembersRepository), + openfga_tuples_service=openfga_tuples_service, + ) + + await service.remove_user_from_group(1, 10) + openfga_tuples_service.remove_user_from_group.assert_awaited_once_with( + 1, 10 + ) + @pytest.mark.asyncio class TestIntegrationUserGroupsService: @@ -240,3 +371,118 @@ async def test_default_administrator_group_exists( ) ) assert exists is True + + async def test_list_usergroup_members( + self, + fixture: Fixture, + services: ServiceCollectionV3, + ) -> None: + group = await create_test_usergroup( + fixture, name="members-group", description="test" + ) + user1 = await create_test_user( + fixture, username="member1", email="mail1@example.com" + ) + user2 = await create_test_user( + fixture, username="member2", email="mail2@example.com" + ) + + await create_openfga_tuple( + fixture, + f"user:{user1.id}", + "user", + "member", + "group", + str(group.id), + ) + await create_openfga_tuple( + fixture, + f"user:{user2.id}", + "user", + "member", + "group", + str(group.id), + ) + + members = await services.usergroups.list_usergroup_members(group.id) + assert len(members) == 2 + usernames = {m.username for m in members} + assert usernames == {"member1", "member2"} + + async def test_list_usergroup_members_empty( + self, + fixture: Fixture, + services: ServiceCollectionV3, + ) -> None: + group = await create_test_usergroup( + fixture, name="empty-group", description="test" + ) + members = await services.usergroups.list_usergroup_members(group.id) + assert len(members) == 0 + + async def test_remove_user_from_group( + self, + fixture: Fixture, + services: ServiceCollectionV3, + ) -> None: + group = await create_test_usergroup( + fixture, name="remove-group", description="test" + ) + user = await create_test_user(fixture, username="to-remove") + + await create_openfga_tuple( + fixture, + f"user:{user.id}", + "user", + "member", + "group", + str(group.id), + ) + + members_before = await services.usergroups.list_usergroup_members( + group.id + ) + assert len(members_before) == 1 + + await services.usergroups.remove_user_from_group(group.id, user.id) + + membership_tuples = await fixture.get( + OpenFGATupleTable.fullname, + and_( + eq(OpenFGATupleTable.c._user, f"user:{user.id}"), + eq(OpenFGATupleTable.c.relation, "member"), + eq(OpenFGATupleTable.c.object_type, "group"), + eq(OpenFGATupleTable.c.object_id, str(group.id)), + ), + ) + assert len(membership_tuples) == 0 + + async def test_add_user_to_group_by_id( + self, + fixture: Fixture, + services: ServiceCollectionV3, + ) -> None: + group = await create_test_usergroup( + fixture, name="add-by-id-group", description="test" + ) + user = await create_test_user(fixture) + + await services.usergroups.add_user_to_group_by_id(user.id, group.id) + + membership_tuples = await fixture.get( + OpenFGATupleTable.fullname, + and_( + eq(OpenFGATupleTable.c._user, f"user:{user.id}"), + eq(OpenFGATupleTable.c.relation, "member"), + eq(OpenFGATupleTable.c.object_type, "group"), + eq(OpenFGATupleTable.c.object_id, str(group.id)), + ), + ) + assert len(membership_tuples) == 1 + + async def test_add_user_to_group_by_id_not_found( + self, + services: ServiceCollectionV3, + ) -> None: + with pytest.raises(UserGroupNotFound): + await services.usergroups.add_user_to_group_by_id(1, 99999)