fixes #40: persist members and admins in group updates

[guusdk]

When creating or updating a group, the provided members and admins should not be ignored.

Note that this change will result in admins/members being removed, when an update to an existing group does not properly include the existing members and admins. Prior to this commit, no modification would have been made to the admins and members.

I've tested this as follows:

First, see if a group that will be used for testing happens to exist (it should not):

curl -X 'GET' \
  'http://localhost:9090/plugins/restapi/v1/groups/UserGroupA' \
  -H 'accept: application/xml' \
  -H 'Authorization: bOofeOWskzH3SV23'

Response status 404. The group doesn't exist, which is what was expected.

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
  <error>
    <exception>GroupNotFoundException</exception>
    <message>Could not find group</message>
    <resource>UserGroupA</resource>
  </error>

Create a new group, that has a couple of members and admins (all on a XMPP domain that is different from the XMPP domain services by the instance of Openfire):

curl -X 'POST' \
  'http://localhost:9090/plugins/restapi/v1/groups' \
  -H 'accept: */*' \
  -H 'Authorization: bOofeOWskzH3SV23' \
  -H 'Content-Type: application/xml' \
  -d '<?xml version="1.0" encoding="UTF-8"?>
<group>
  <name>UserGroupA</name>
  <description>My group of users</description>
  <shared>false</shared>
  <admins>
    <admin>[email protected]</admin>
    <admin>[email protected]</admin>
    <admin>[email protected]</admin>
  </admins>
  <members>
    <member>[email protected]</member>
    <member>[email protected]</member>
    <member>[email protected]</member>
  </members>
</group>'

Response status 201
(no body)

Retrieving the group again, to see if it was created as expected:

curl -X 'GET' \
  'http://localhost:9090/plugins/restapi/v1/groups/UserGroupA' \
  -H 'accept: application/xml' \
  -H 'Authorization: bOofeOWskzH3SV23'

Response status 200:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
  <group>
    <name>UserGroupA</name>
    <description>My group of users</description>
    <admins>
      <admin>[email protected]</admin>
      <admin>[email protected]</admin>
      <admin>[email protected]</admin>
    </admins>
    <members>
      <member>[email protected]</member>
      <member>[email protected]</member>
      <member>[email protected]</member>
    </members>
    <shared>false</shared>
  </group>

This seems to include all members and admins as expected.

Next, try to modify the group members and admins. Remove two, retain one, and add one (which covers all permutations that I could think of) in each list.

curl -X 'PUT' \
  'http://localhost:9090/plugins/restapi/v1/groups/UserGroupA' \
  -H 'accept: */*' \
  -H 'Authorization: bOofeOWskzH3SV23' \
  -H 'Content-Type: application/xml' \
  -d '<?xml version="1.0" encoding="UTF-8"?>
  <group>
    <name>UserGroupA</name>
    <description>My group of users</description>
    <admins>
      <admin>[email protected]</admin>
      <admin>[email protected]</admin>
    </admins>
    <members>
      <member>[email protected]</member>
      <member>[email protected]</member>
    </members>
    <shared>false</shared>
  </group>'

Response status 200:
(no body)

Finally, retrieve the group again, to see if the modifications were persisted:

curl -X 'GET' \
  'http://localhost:9090/plugins/restapi/v1/groups/UserGroupA' \
  -H 'accept: application/xml' \
  -H 'Authorization: bOofeOWskzH3SV23'

Response status 200:

<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
  <group>
    <name>UserGroupA</name>
    <description>My group of users</description>
    <admins>
      <admin>[email protected]</admin>
      <admin>[email protected]</admin>
    </admins>
    <members>
      <member>[email protected]</member>
      <member>[email protected]</member>
    </members>
    <shared>false</shared>
  </group>

That seems all fine by me.

[guusdk]

@Redor I think I have solved a bug, but you might have also deliberately excluded admins and members from group updates, for a reason that I don't know. Can you confirm what your intentions here were?

[Redor]

@guusdk there is no specific reason. The endpoint was just made to manage groups (add, create etc). But without directly setting the users (on creating of the group).
There is also another endpoint that allowed to add users to group(s): https://www.igniterealtime.org/projects/openfire/plugins/1.6.0/restAPI/readme.html#add-user-to-group

[guusdk]

But without directly setting the users (on creating of the group).

That leads to odd behavior, in my opinion, as the data structure that acts as input does allow for setting users. Them being ignored is something that I suspect many users find unexpected.

There is also another endpoint that allowed to add users to group(s)

I have noticed that. That endpoint is useful, but it does not allow for two things:

• Add users to a group that are not users of Openfire itself (like federated users - see Unable to add External/Remote users to a group #40)
• Add multiple users to a group with one API call. If you want to create large groups, it can become quite resource intensive to perform requests to add users to a group one by one.

Do you have any concerns with my PR? If not then I would like to merge it.

[Redor]

Do you have any concerns with my PR? If not then I would like to merge it.

No concerns. I love the PR :)

[guusdk]

\o/

Let us allow @Fishbowler some time to point out my mistakes, or forever hold his peace. ;)

[Fishbowler]

Left a bunch of questions, that might all be non-issues, but HTH.

[guusdk]

@Fishbowler want to press the big green button, if you're OK with my last few changes?

[Fishbowler]

There's a lot of deletion since the last review. Intentional?

[Fishbowler]

Other than that, all looking like sensible change

[guusdk]

I'm unsure what deletions you're referring to.

I have now rebased to the latest main branch to resolve a small merge conflict in plugin.xml.

[Fishbowler]

Don't worry. Looks like a bug in the GH mobile app that I can't reproduce this morning. I was seeing chunks of docs and changelogs deleted last night. Take my 👍

[guusdk]