Launchpad MP (501314) - r00ta/openfga-users-to-groups-endpoints by r00tabot · Pull Request #437 · SpaghettiHub/maas

r00tabot · 2026-03-04T00:16:04Z

This is autogenerated by maas.r00ta.com. Enjoy!

Commit message: feat: add v2 and v3 API usergroup membership endpoints

Details:

openfga built-in migrations are executed before the alembic migrations, because we added an alembic migration that creates a view on the openfga.tuple table.
A new view maasserver_usergroup_members_view has been added. It reads data from the openfga.tuple table and joins it with the auth_user table, so to return the list of users and their groups.
V2 endpoints to list, add and remove users to groups
V3 endpoints to list, add and remove users to groups
Avoid adding system users to groups.

Copilot

Pull request overview

Adds user-group membership management endpoints (v2 + v3) backed by OpenFGA tuples, introducing a DB view + repository/model to list group members and preventing system users from being auto-added to groups. Also adjusts migration ordering so OpenFGA built-in migrations run before Alembic.

Changes:

Add maasserver_usergroup_members_view (based on openfga.tuple joined to auth_user) plus service-layer model/repository to query group members.
Add membership operations (list/add/remove) to the service layer and expose them via v2 + v3 APIs.
Update migrations/test DB bootstrap and add/extend tests; prevent system users from being assigned to groups.

Reviewed changes

Copilot reviewed 24 out of 25 changed files in this pull request and generated 9 comments.

Show a summary per file

File	Description
src/maasservicelayer/services/usergroups.py	Adds membership methods and duplicate-membership guard.
src/maasservicelayer/services/openfga_tuples.py	Adds tuple deletion helper for removing a user from a group.
src/maasservicelayer/db/repositories/usergroups_members.py	New read-only repository + clause factory for the members view.
src/maasservicelayer/models/usergroup_members.py	New model representing a user’s membership in a group.
src/maasservicelayer/db/tables.py	Registers the `maasserver_usergroup_members_view` table/view metadata.
src/maasservicelayer/db/alembic/versions/0020_create_maasserver_user_group_membership.py	Creates the DB view used for membership listing.
src/maasservicelayer/services/init.py	Wires the new repository into the service collection.
src/maasapiserver/v3/api/public/handlers/usergroups.py	Adds v3 membership endpoints (GET/POST/DELETE).
src/maasapiserver/v3/api/public/models/requests/usergroup_members.py	Adds v3 request model for adding a member by `user_id`.
src/maasapiserver/v3/api/public/models/responses/usergroup_members.py	Adds v3 response models for membership listing.
src/maasserver/api/usergroups.py	Adds v2 membership operations (`list_members`, `add_member`, `remove_member`).
src/maasserver/exceptions.py	Adds a v2 API conflict exception for “already a member”.
src/maasserver/models/signals/users.py	Skips auto-adding system users to default groups.
src/maasserver/models/signals/tests/test_users.py	Tests that system users are ignored by the post-save hook.
src/maasserver/management/commands/dbupgrade.py	Runs OpenFGA migrations before Alembic (to support the view).
src/maasserver/testing/initial.maas_test.sql	Updates test DB bootstrap with the new view + Alembic version bump.
src/maasserver/testing/factory.py	Adds `make_Usergroup()` factory using the service layer.
src/maasopenfga/internal/migrations/00002_migrate_environments.go	Updates OpenFGA migration logic to ignore internal users.
src/tests/fixtures/factories/user.py	Adjusts test user factory defaults.
src/tests/maasservicelayer/services/test_usergroups.py	Adds unit/integration tests for membership operations.
src/tests/maasservicelayer/services/test_openfga_tuples.py	Adds test coverage for removing user-group membership tuples.
src/tests/maasservicelayer/db/repositories/test_usergroups_members.py	Adds repository tests for members view querying.
src/tests/maasapiserver/v3/api/public/handlers/test_usergroups.py	Adds handler tests for v3 membership endpoints.
src/maasserver/api/tests/test_usergroups.py	Adds v2 API tests for membership operations + permissions.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-04T00:23:04Z

src/maasservicelayer/services/openfga_tuples.py

+                    OpenFGATuplesClauseFactory.with_user(f"user:{user_id}"),
+                    OpenFGATuplesClauseFactory.with_relation("member"),
+                    OpenFGATuplesClauseFactory.with_object_type("group"),
+                    OpenFGATuplesClauseFactory.with_object_type("group"),


remove_user_from_group builds a filter list that includes with_object_type("group") twice. This is redundant at best, and if one of these was intended to be a different predicate it will currently delete the wrong set of tuples. Remove the duplicate filter (or replace it with the intended condition) so the query precisely targets the membership tuple.

Suggested change

OpenFGATuplesClauseFactory.with_object_type("group"),

Copilot · 2026-03-04T00:23:04Z

src/maasservicelayer/db/tables.py

+    "maasserver_usergroup_members_view",
+    METADATA,
+    Column("id", Integer, Identity(), primary_key=True),
+    Column("group_id", String, primary_key=True),


UserGroupMembersView.c.group_id is declared as String, but the view definition casts t.object_id to integer and the clause factory passes an int. In Postgres this can cause operator resolution errors (varchar = integer). Define group_id as an Integer/BigInteger to match the view output and how it’s queried.

Suggested change

Column("group_id", String, primary_key=True),

Column("group_id", Integer, primary_key=True),

Copilot · 2026-03-04T00:23:04Z

src/maasopenfga/internal/migrations/00002_migrate_environments.go

 	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


The Where(sq.Or{sq.NotEq{"username": "MAAS"}, sq.NotEq{"username": "maas-init-node"}}) condition is logically always true (every row is != at least one of those values), so internal users will not be filtered out. Use an AND of the two NotEq conditions or sq.NotEq{"username": []string{"MAAS","maas-init-node"}}/sq.NotIn to exclude both usernames correctly.

Suggested change

Where(sq.Or{sq.NotEq{"username": "MAAS"}, sq.NotEq{"username": "maas-init-node"}}). // Ignore the internal users

Where(sq.NotEq{"username": []string{"MAAS", "maas-init-node"}}). // Ignore the internal users

Copilot · 2026-03-04T00:23:05Z

src/maasservicelayer/models/usergroup_members.py

+class UserGroupMember(MaasBaseModel):
+    group_id: int
+    username: str
+    email: str


UserGroupMember.email is declared as str, but auth_user.email (and the view column) is nullable in this codebase. If a user has NULL email, mapping the row into this model will fail validation. Make email optional (str | None) to match the database schema.

Suggested change

email: str

email: str | None

Copilot · 2026-03-04T00:23:05Z

src/maasapiserver/v3/api/public/models/responses/usergroup_members.py

+    kind = "UserGroupMember"
+    user_id: int
+    username: str
+    email: str


UserGroupMemberResponse.email is typed as str, but member emails can be null (auth_user.email is nullable). This will cause response validation errors when serializing members without an email. Change the field to str | None (and keep from_model passing through the value).

Suggested change

email: str

email: str | None

Copilot · 2026-03-04T00:23:05Z

src/maasserver/api/usergroups.py

+        """@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


The add_member docstring advertises returning “The updated list of group members” (200 JSON), but the implementation returns rc.ALL_OK without any member list. Either update the handler to return the updated members (e.g. call list_usergroup_members and serialize) or adjust the API docs to match the actual response body.

Copilot · 2026-03-04T00:23:06Z

src/maasservicelayer/db/alembic/versions/0020_create_maasserver_user_group_membership.py

+def upgrade() -> None:
+    sql = dedent("""\
+        SELECT
+            (t.object_id)::integer AS group_id,


The view casts t.object_id to integer for group_id, but maasserver_usergroup.id is a BIGINT (see migration 0019). To avoid overflow/mismatch as IDs grow, cast to bigint and model the column as BigInteger in SQLAlchemy.

Suggested change

(t.object_id)::integer AS group_id,

(t.object_id)::bigint AS group_id,

Copilot · 2026-03-04T00:23:06Z

src/maasservicelayer/db/tables.py

+    Column("username", String(150), nullable=False, unique=True),
+    Column("email", String(254), nullable=True, unique=True),


UserGroupMembersView marks username and email as unique=True, but this is a membership view (one user can appear in multiple rows if they’re in multiple groups), so these columns are not unique at the view level. Keeping these as unique is misleading and can break any code that relies on SQLAlchemy’s uniqueness metadata. Drop the unique=True flags for these view columns.

Suggested change

Column("username", String(150), nullable=False, unique=True),

Column("email", String(254), nullable=True, unique=True),

Column("username", String(150), nullable=False),

Column("email", String(254), nullable=True),

Copilot · 2026-03-04T00:23:06Z

src/tests/fixtures/factories/user.py

    user = {
        "username": "myusername",
        "password": "pbkdf2_sha256$260000$f1nMJPH4Z5Wc8QxkTsZ1p6$ylZBpgGE3FNlP2zOU21cYiLtvxwtkglsPKUETtXhzDw=",  # hash('test')
        "is_superuser": False,
        "first_name": "first",
        "last_name": "last",
+        "email": "mail@example.com",
        "is_staff": False,


create_test_user now always sets email to a constant value. In this codebase auth_user.email is unique, so any test creating more than one user without overriding email will start failing with unique-constraint violations. Generate a unique email by default (e.g., derived from the username or a random suffix) instead of a constant.

enjoy this from r00ta

fcb902b

r00ta requested a review from Copilot March 4, 2026 00:16

Copilot started reviewing on behalf of r00ta March 4, 2026 00:17 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Launchpad MP (501314) - r00ta/openfga-users-to-groups-endpoints#437

Launchpad MP (501314) - r00ta/openfga-users-to-groups-endpoints#437
r00tabot wants to merge 1 commit intoSpaghettiHub:masterfrom
r00tabot:36c8d592-90bc-489a-9b62-8d22e79914f7

r00tabot commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	Column("group_id", String, primary_key=True),
	Column("group_id", Integer, primary_key=True),

	Where(sq.Or{sq.NotEq{"username": "MAAS"}, sq.NotEq{"username": "maas-init-node"}}). // Ignore the internal users
	Where(sq.NotEq{"username": []string{"MAAS", "maas-init-node"}}). // Ignore the internal users

	(t.object_id)::integer AS group_id,
	(t.object_id)::bigint AS group_id,

		Column("username", String(150), nullable=False, unique=True),
		Column("email", String(254), nullable=True, unique=True),

Conversation

r00tabot commented Mar 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants