Support setting role in add-member API requests

[seanh]

Fixes #9140.

Add support for an optional JSON body in add-membership API requests that lets the user set the membership's role in the add-member API request itself (rather than having to add the membership and then call the edit-membership API to change the role).

For backwards compatibility if the add-membership API is called without a body it still adds a member with the plain "member" role, as currently.

No updated API docs in this request since the backwards-compat means this is a non-breaking change. I'll send a separate PR later to update the docs for several membership APIs that've had new features added recently.

Note that the add-member API can currently only be called with an authclient. It can't be called as a user. Auth clients have permission to add any user to any group (within the auth client's authority) and with any role.

Testing

1. Log in as devdata_admin

2. Go to http://localhost:5000/groups/new and create a new group

3. Go to http://localhost:5000/admin/oauthclients/new and create an authclient with these properties:

   Authority: localhost
   Grant type: client_credentials
   Trusted: yes

4. Use the authclient to authenticate an API request to add devdata_user to the group. With no JSON body it'll create a plain "member" role:

   $ httpx http://localhost:5000/api/groups/{pubid}/members/acct:devdata_user@localhost --method POST --auth {client_id} {client_secret}
   HTTP/1.1 200 OK
   
   {
       "authority": "localhost",
       "userid": "acct:devdata_user@localhost",
       "username": "devdata_user",
       "display_name": null,
       "roles": [
           "member"
       ],
       "actions": [],
       "created": "2024-12-06T12:54:27.790391+00:00",
       "updated": "2024-12-06T12:54:27.790394+00:00"
   }
   

5. Create an API key and use it to delete the membership so we can recreate it again:

   $ httpx http://localhost:5000/api/groups/{pubid}/members/acct:devdata_user@localhost --method DELETE --headers Authorization 'Bearer {apikey}'
   

6. Now make the POST request again with a JSON body and it'll add the membership with the role from the JSON body:

   $ httpx http://localhost:5000/api/groups/{pubid}/members/acct:devdata_user@localhost --method POST --auth {client_id} {client_secret} --json '{"roles": ["owner"]}'
   

7. You can also trying posting an invalid role and you'll get an error message.

8. You can repeatedly make the same request (with or without a JSON body) and it'll keep on responding 200 OK and just returning the existing membership. But if you make a request with different roles it'll respond 409 Conflict.

[seanh]

Previously if you called this method and a matching membership already existed it would just return the existing membership. Now that I've added the roles argument it introduces a new case:

It's now possible to call this method when a membership already exists with the matching group and userid but with different roles. In this case we can't create a new membership. Nor do we want to update the existing membership (for example the API view that calls this method doesnt' want it to update existing memberships: a POST API should never update an existing resource). So raise an exception in that case.

For backwards-compatibility it will still return a pre-existing membership if it has the same roles.

[seanh]

For backwards-compatibility add-group-membership requests with no body are still allowed and they add memberships with the default role.

[seanh]

Return a 409 if there's an existing membership with different roles.

For backwards-compatibility if there's an existing membership with the same roles it'll still return 200 OK.

[marcospri]

I don't think I get why we use this kwargs here vs a more explicit roles only check, are there any other membership attributes that might create a conflict coming soon?

[seanh]

Yeah this was just worrying about the future: I'm trying to code in such a way that, if we ever do add support for another attribute besides roles, that attribute will be taken into account when deciding whether to raise ConflictError as well

Copilot reviewed 5 out of 5 changed files in this pull request and generated no suggestions.

Comments skipped due to low confidence (1)

h/services/group_members.py:140

• [nitpick] The error message could be more specific. Consider changing it to: 'The user is already a member of the group with different roles.'

raise ConflictError("The user is already a member of the group, with conflicting membership attributes")