Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fixes #40: persist members and admins in group updates #104

Merged
merged 3 commits into from
Jun 23, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ <h1>
<li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/99'>#99</a>] - Fix hard-coded link to localhost for OpenAPI yaml link in docs.</li>
<li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/96'>#96</a>] - Don't require auth for readiness/liveness endpoints</li>
<li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/74'>#74</a>] - Return HTTP status code 404 instead of 500 when passing incorrect MUC service/room name</li>
<li>[<a href='https://github.com/igniterealtime/openfire-restAPI-plugin/issues/40'>#40</a>] - When creating or updating a group, use the members and admins that are provided in the input data.</li>
</ul>

<p><b>1.8.0</b> April 6, 2022</p>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,11 @@

package org.jivesoftware.openfire.plugin.rest.controller;

import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.*;

import javax.ws.rs.core.Response;

import org.jivesoftware.openfire.XMPPServer;
import org.jivesoftware.openfire.group.Group;
import org.jivesoftware.openfire.group.GroupAlreadyExistsException;
import org.jivesoftware.openfire.group.GroupManager;
Expand All @@ -30,6 +29,7 @@
import org.jivesoftware.openfire.plugin.rest.exceptions.ExceptionType;
import org.jivesoftware.openfire.plugin.rest.exceptions.ServiceException;
import org.jivesoftware.openfire.plugin.rest.utils.MUCRoomUtils;
import org.xmpp.packet.JID;

/**
* The Class GroupController.
Expand All @@ -56,7 +56,7 @@ public static GroupController getInstance() {
*/
public List<GroupEntity> getGroups() throws ServiceException {
Collection<Group> groups = GroupManager.getInstance().getGroups();
List<GroupEntity> groupEntities = new ArrayList<GroupEntity>();
List<GroupEntity> groupEntities = new ArrayList<>();
for (Group group : groups) {
GroupEntity groupEntity = new GroupEntity(group.getName(), group.getDescription());
groupEntities.add(groupEntity);
Expand Down Expand Up @@ -86,7 +86,7 @@ public GroupEntity getGroup(String groupName) throws ServiceException {
GroupEntity groupEntity = new GroupEntity(group.getName(), group.getDescription());
groupEntity.setAdmins(MUCRoomUtils.convertJIDsToStringList(group.getAdmins()));
groupEntity.setMembers(MUCRoomUtils.convertJIDsToStringList(group.getMembers()));
groupEntity.setShared(group.getProperties().get("sharedRoster.showInRoster")=="onlyGroup");
groupEntity.setShared("onlyGroup".equals(group.getProperties().get("sharedRoster.showInRoster")));

return groupEntity;
}
Expand All @@ -104,6 +104,26 @@ public Group createGroup(GroupEntity groupEntity) throws ServiceException {
Group group;
if (groupEntity != null && !groupEntity.getName().isEmpty()) {
try {
final Collection<JID> newMembers = new HashSet<>();
final Collection<JID> newAdmins = new HashSet<>();
guusdk marked this conversation as resolved.
Show resolved Hide resolved

// Input validation.
for (final String newMember : groupEntity.getMembers()) {
try {
newMembers.add(newMember.contains("@") ? new JID(newMember) : XMPPServer.getInstance().createJID(newMember, null));
guusdk marked this conversation as resolved.
Show resolved Hide resolved
} catch (IllegalArgumentException e) {
throw new ServiceException("Cannot parse a member value as a JID: " + newMember, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
}
}
for (final String newAdmin : groupEntity.getAdmins()) {
try {
newAdmins.add(newAdmin.contains("@") ? new JID(newAdmin) : XMPPServer.getInstance().createJID(newAdmin, null));
} catch (IllegalArgumentException e) {
throw new ServiceException("Cannot parse a admin value as a JID: " + newAdmin, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
}
}

// Start creating the group.
group = GroupManager.getInstance().createGroup(groupEntity.getName());
group.setDescription(groupEntity.getDescription());

Expand All @@ -112,6 +132,17 @@ public Group createGroup(GroupEntity groupEntity) throws ServiceException {

group.getProperties().put("sharedRoster.displayName", groupEntity.getName());
group.getProperties().put("sharedRoster.groupList", "");

final Collection<JID> members = group.getMembers();
for (final JID newMember : newMembers) {
// Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
members.add(newMember);
guusdk marked this conversation as resolved.
Show resolved Hide resolved
}
final Collection<JID> admins = group.getAdmins();
for (final JID newAdmin : newAdmins) {
// Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
admins.add(newAdmin);
}
} catch (GroupAlreadyExistsException e) {
throw new ServiceException("Could not create a group", groupEntity.getName(),
ExceptionType.GROUP_ALREADY_EXISTS, Response.Status.CONFLICT, e);
Expand All @@ -136,10 +167,65 @@ public Group updateGroup(String groupName, GroupEntity groupEntity) throws Servi
if (groupEntity != null && !groupEntity.getName().isEmpty()) {
if (groupName.equals(groupEntity.getName())) {
try {
final Collection<JID> newMembers = new HashSet<>();
final Collection<JID> newAdmins = new HashSet<>();

// Input validation.
group = GroupManager.getInstance().getGroup(groupName);
for (final String newMember : groupEntity.getMembers()) {
try {
newMembers.add(newMember.contains("@") ? new JID(newMember) : XMPPServer.getInstance().createJID(newMember, null));
} catch (IllegalArgumentException e) {
throw new ServiceException("Cannot parse a member value as a JID: " + newMember, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
}
}
for (final String newAdmin : groupEntity.getAdmins()) {
try {
newAdmins.add(newAdmin.contains("@") ? new JID(newAdmin) : XMPPServer.getInstance().createJID(newAdmin, null));
} catch (IllegalArgumentException e) {
throw new ServiceException("Cannot parse a admin value as a JID: " + newAdmin, groupEntity.getName(), ExceptionType.ILLEGAL_ARGUMENT_EXCEPTION, Response.Status.BAD_REQUEST, e);
}
}

group.setDescription(groupEntity.getDescription());
final String showInRoster = groupEntity.getShared() ? "onlyGroup" : "nobody";
group.getProperties().put("sharedRoster.showInRoster", showInRoster);

// Correct the member-list that already is in the group to match the desired state.
final Collection<JID> members = group.getMembers();
final Iterator<JID> membersIterator = members.iterator();
while (membersIterator.hasNext()) {
final JID oldMember = membersIterator.next();
if (newMembers.contains(oldMember)) {
// Already exists. No need to add it again.
newMembers.remove(oldMember);
} else {
// No longer exists. Remove from group.
membersIterator.remove();
}
}
for (final JID newMember : newMembers) {
// Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
members.add(newMember);
guusdk marked this conversation as resolved.
Show resolved Hide resolved
}

// Correct the admin-list that already is in the group to match the desired state.
final Collection<JID> admins = group.getAdmins();
final Iterator<JID> adminsIterator = admins.iterator();
while (adminsIterator.hasNext()) {
final JID oldAdmin = adminsIterator.next();
if (newAdmins.contains(oldAdmin)) {
// Already exists. No need to add it again.
newAdmins.remove(oldAdmin);
} else {
// No longer exists. Remove from group.
adminsIterator.remove();
}
}
for (final JID newAdmin : newAdmins) {
// Unsure if #addAll works for the specific Collection subclass that is used in the Group implementation. Let's add them one-by-one to be safe.
admins.add(newAdmin);
}
} catch (GroupNotFoundException e) {
throw new ServiceException("Could not find group", groupName, ExceptionType.GROUP_NOT_FOUND,
Response.Status.NOT_FOUND, e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,22 +52,22 @@
* The Class UserServiceController.
*/
public class UserServiceController {
private static Logger LOG = LoggerFactory.getLogger(UserServiceController.class);
private static final Logger LOG = LoggerFactory.getLogger(UserServiceController.class);

/** The Constant INSTANCE. */
private static UserServiceController INSTANCE = null;

/** The user manager. */
private UserManager userManager;
private final UserManager userManager;

/** The roster manager. */
private RosterManager rosterManager;
private final RosterManager rosterManager;

/** The server. */
private XMPPServer server;
private final XMPPServer server;

/** The lock out manager. */
private LockOutManager lockOutManager;
private final LockOutManager lockOutManager;

/**
* Gets the single instance of UserServiceController.
Expand Down Expand Up @@ -192,10 +192,15 @@ public void deleteUser(String username) throws ServiceException {
/**
* Gets the user entities.
*
* When a property key (and possibly value) is provided, then the user that is returned is one for which the
* specified property has been defined.
*
* @param userSearch
* the user search
* @param propertyValue
* @param propertyKey
* the property key (can be null)
* @param propertyValue
* the property value (can be null)
* @return the user entities
* @throws ServiceException
* the service exception
Expand Down Expand Up @@ -275,7 +280,7 @@ public RosterEntities getRosterEntities(String username) throws ServiceException
log("Get roster entities for user: " + username);
Roster roster = getUserRoster(username);

List<RosterItemEntity> rosterEntities = new ArrayList<RosterItemEntity>();
List<RosterItemEntity> rosterEntities = new ArrayList<>();
for (RosterItem rosterItem : roster.getRosterItems()) {
RosterItemEntity rosterItemEntity = new RosterItemEntity(rosterItem.getJid().toBareJID(),
rosterItem.getNickname(), rosterItem.getSubStatus().getValue());
Expand Down Expand Up @@ -318,13 +323,11 @@ public void addRosterItem(String username, RosterItemEntity rosterItemEntity) th
// Roster item does not exist. Try to add it.
}

if (roster != null) {
RosterItem rosterItem = roster.createRosterItem(jid, rosterItemEntity.getNickname(),
rosterItemEntity.getGroups(), false, true);
UserUtils.checkSubType(rosterItemEntity.getSubscriptionType());
rosterItem.setSubStatus(RosterItem.SubType.getTypeFromInt(rosterItemEntity.getSubscriptionType()));
roster.updateRosterItem(rosterItem);
}
RosterItem rosterItem = roster.createRosterItem(jid, rosterItemEntity.getNickname(),
rosterItemEntity.getGroups(), false, true);
UserUtils.checkSubType(rosterItemEntity.getSubscriptionType());
rosterItem.setSubStatus(RosterItem.SubType.getTypeFromInt(rosterItemEntity.getSubscriptionType()));
roster.updateRosterItem(rosterItem);
}

/**
Expand Down Expand Up @@ -401,9 +404,18 @@ public void deleteRosterItem(String username, String rosterJid) throws SharedGro
*/
public List<String> getUserGroups(String username) throws ServiceException {
log("Get user groups for user: " + username);
if (username.contains("@")) {
final JID jid = new JID(username);
if (jid.getDomain().equals(XMPPServer.getInstance().getServerInfo().getXMPPDomain())) {
username = jid.getNode();
} else {
// Implementing this would require us to iterate over all groups, which is a performance nightmare.
throw new ServiceException("This service cannot be used for non-local users.", username, ExceptionType.GROUP_NOT_FOUND, Response.Status.INTERNAL_SERVER_ERROR);
}
}
User user = getAndCheckUser(username);
Collection<Group> groups = GroupManager.getInstance().getGroups(user);
List<String> groupNames = new ArrayList<String>();
List<String> groupNames = new ArrayList<>();
for (Group group : groups) {
groupNames.add(group.getName());
}
Expand All @@ -424,7 +436,7 @@ public List<String> getUserGroups(String username) throws ServiceException {
public void addUserToGroups(String username, UserGroupsEntity userGroupsEntity) throws ServiceException {
if (userGroupsEntity != null) {
log("Adding user: " + username + " to groups");
Collection<Group> groups = new ArrayList<Group>();
Collection<Group> groups = new ArrayList<>();

for (String groupName : userGroupsEntity.getGroupNames()) {
Group group;
Expand All @@ -438,7 +450,7 @@ public void addUserToGroups(String username, UserGroupsEntity userGroupsEntity)
groups.add(group);
}
for (Group group : groups) {
group.getMembers().add(server.createJID(username, null));
group.getMembers().add(username.contains("@") ? new JID(username) : XMPPServer.getInstance().createJID(username, null));
}
}
}
Expand All @@ -461,7 +473,7 @@ public void addUserToGroup(String username, String groupName) throws ServiceExce
group = GroupController.getInstance().createGroup(new GroupEntity(groupName, ""));
}

group.getMembers().add(server.createJID(username, null));
group.getMembers().add(username.contains("@") ? new JID(username) : XMPPServer.getInstance().createJID(username, null));
}

/**
Expand All @@ -487,7 +499,7 @@ public void deleteUserFromGroups(String username, UserGroupsEntity userGroupsEnt
Response.Status.NOT_FOUND, e);
}
log("Removing user: " + username + " from the group: " + groupName);
group.getMembers().remove(server.createJID(username, null));
group.getMembers().remove(username.contains("@") ? new JID(username) : XMPPServer.getInstance().createJID(username, null));
}
}
}
Expand All @@ -508,7 +520,7 @@ public void deleteUserFromGroup(String username, String groupName) throws Servic
throw new ServiceException("Could not find group", groupName, ExceptionType.GROUP_NOT_FOUND,
Response.Status.NOT_FOUND, e);
}
group.getMembers().remove(server.createJID(username, null));
group.getMembers().remove(username.contains("@") ? new JID(username) : XMPPServer.getInstance().createJID(username, null));
}

/**
Expand All @@ -525,7 +537,7 @@ public void deleteUserFromGroup(String username, String groupName) throws Servic
public UserEntities getUserEntitiesByProperty(String propertyKey, String propertyValue) throws ServiceException {
log("Get user entities by property key : " + propertyKey + "and property value: " + propertyValue);
List<String> usernames = PropertyDAO.getUsernameByProperty(propertyKey, propertyValue);
List<UserEntity> users = new ArrayList<UserEntity>();
List<UserEntity> users = new ArrayList<>();
UserEntities userEntities = new UserEntities();

for (String username : usernames) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public GroupEntities getGroups() throws ServiceException
description = "Create a new user group.",
responses = {
@ApiResponse(responseCode = "201", description = "Group created."),
@ApiResponse(responseCode = "400", description = "Group or group name missing."),
@ApiResponse(responseCode = "400", description = "Group or group name missing, or invalid syntax for a property."),
@ApiResponse(responseCode = "409", description = "Group already exists.")
})
@Consumes({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
Expand Down Expand Up @@ -94,7 +94,7 @@ public GroupEntity getGroup(@Parameter(description = "The name of the group that
description = "Updates / overwrites an existing user group. Note that the name of the group cannot be changed.",
responses = {
@ApiResponse(responseCode = "200", description = "Group updated."),
@ApiResponse(responseCode = "400", description = "Group or group name missing, or name does not match existing group."),
@ApiResponse(responseCode = "400", description = "Group or group name missing, or name does not match existing group, or invalid syntax for a property."),
@ApiResponse(responseCode = "404", description = "Group with this name not found."),
})
@Consumes({ MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ public UserGroupsEntity getUserGroups(

@POST
@Operation( summary = "Add user to groups",
description = "Add a particular user to a collection of groups.",
description = "Add a particular user to a collection of groups. When a group that is provided does not exist, it will be automatically created if possible.",
responses = {
@ApiResponse(responseCode = "201", description = "The user was added to all groups."),
@ApiResponse(responseCode = "400", description = "When the username cannot be parsed into a JID.")
})
@Consumes({MediaType.APPLICATION_XML, MediaType.APPLICATION_JSON})
public Response addUserToGroups(
Expand All @@ -76,9 +77,10 @@ public Response addUserToGroups(
@POST
@Path("/{groupName}")
@Operation( summary = "Add user to group",
description = "Add a particular user to a particular group.",
description = "Add a particular user to a particular group. When the group that does not exist, it will be automatically created if possible.",
responses = {
@ApiResponse(responseCode = "201", description = "The user was added to the groups."),
@ApiResponse(responseCode = "400", description = "When the username cannot be parsed into a JID.")
})
public Response addUserToGroup(
@Parameter(description = "The username of the user that is to be added to a group.", required = true) @PathParam("username") String username,
Expand Down