GUACAMOLE-926: Remove patch update functionality. It's needed for batch import, and it's a can of worms.

This commit is contained in:
James Muehlner
2023-02-07 00:31:06 +00:00
parent cbb44efb2b
commit 314adf6c23
14 changed files with 39 additions and 184 deletions

View File

@@ -166,14 +166,6 @@ public class ActiveConnectionService
} }
@Override
public void updateExternalObject(ModeledAuthenticatedUser user, ActiveConnection object) throws GuacamoleException {
// Updating active connections is not implemented
throw new GuacamoleSecurityException("Permission denied.");
}
/** /**
* Retrieve the permission set for the specified user that relates * Retrieve the permission set for the specified user that relates
* to access to active connections. * to access to active connections.

View File

@@ -116,24 +116,6 @@ public interface DirectoryObjectService<InternalType, ExternalType> {
void deleteObject(ModeledAuthenticatedUser user, String identifier) void deleteObject(ModeledAuthenticatedUser user, String identifier)
throws GuacamoleException; throws GuacamoleException;
/**
* Updates the object corresponding to the given external representation,
* applying any changes that have been made. If no such object exists,
* this function has no effect.
*
* @param user
* The user updating the object.
*
* @param object
* The external object to apply updates from.
*
* @throws GuacamoleException
* If the user lacks permission to update the object, or an error
* occurs while updating the object.
*/
void updateExternalObject(ModeledAuthenticatedUser user, ExternalType object)
throws GuacamoleException;
/** /**
* Updates the given object, applying any changes that have been made. If * Updates the given object, applying any changes that have been made. If
* no such object exists, this function has no effect. * no such object exists, this function has no effect.

View File

@@ -511,20 +511,6 @@ public abstract class ModeledDirectoryObjectService<InternalType extends Modeled
} }
@Override
@Transactional
public void updateExternalObject(ModeledAuthenticatedUser user, ExternalType object)
throws GuacamoleException {
// Convert to the internal type
InternalType internalObject = getObjectInstance(
user, getModelInstance(user, object));
// Delegate to the standard internal update functionality
updateObject(user, internalObject);
}
@Override @Override
public Set<String> getIdentifiers(ModeledAuthenticatedUser user) public Set<String> getIdentifiers(ModeledAuthenticatedUser user)
throws GuacamoleException { throws GuacamoleException {

View File

@@ -21,8 +21,6 @@ package org.apache.guacamole.auth.jdbc.base;
import java.util.Collection; import java.util.Collection;
import org.apache.guacamole.GuacamoleException;
/** /**
* Object representation of a Guacamole object, such as a user or connection, * Object representation of a Guacamole object, such as a user or connection,
* as represented in the database. * as represented in the database.
@@ -77,7 +75,7 @@ public abstract class ObjectModel {
* *
* @return * @return
* The ID of this object in the database, or null if this object was * The ID of this object in the database, or null if this object was
* not retrieved from or intended to update the database. * not retrieved from the database.
*/ */
public Integer getObjectID() { public Integer getObjectID() {
return objectID; return objectID;
@@ -93,31 +91,6 @@ public abstract class ObjectModel {
this.objectID = objectID; this.objectID = objectID;
} }
/**
* Given a text identifier, attempt to convert to an integer database ID.
* If the identifier is valid, the database ID will be set to this value.
* Otherwise, a GuacamoleException will be thrown.
*
* @param identifier
* The identifier to convert to an integer and set on the database
* model, if valid.
*
* @throws GuacamoleException
* If the provided identifier is not a valid integer.
*/
public void setObjectID(String identifier) throws GuacamoleException {
// Try to convert the provided identifier to an integer ID
try {
setObjectID(Integer.parseInt(identifier));
}
catch (NumberFormatException e) {
throw new GuacamoleException(
"Database identifiers must be integers.");
}
}
/** /**
* Returns a map of attribute name/value pairs for all attributes associated * Returns a map of attribute name/value pairs for all attributes associated
* with this model which do not have explicit mappings to actual model * with this model which do not have explicit mappings to actual model

View File

@@ -68,18 +68,8 @@ public class ConnectionDirectory extends JDBCDirectory<Connection> {
@Override @Override
@Transactional @Transactional
public void update(Connection object) throws GuacamoleException { public void update(Connection object) throws GuacamoleException {
ModeledConnection connection = (ModeledConnection) object;
// If the provided connection is already an internal type, update connectionService.updateObject(getCurrentUser(), connection);
// using the internal method
if (object instanceof ModeledConnection)
connectionService.updateObject(
getCurrentUser(), (ModeledConnection) object);
// If the type is not already the expected internal type, use the
// external update method
else
connectionService.updateExternalObject(getCurrentUser(), object);
} }
@Override @Override

View File

@@ -387,4 +387,9 @@ public class ConnectionModel extends ChildObjectModel {
} }
@Override
public void setIdentifier(String identifier) {
throw new UnsupportedOperationException("Connection identifiers are derived from IDs. They cannot be set.");
}
} }

View File

@@ -110,19 +110,14 @@ public class ConnectionService extends ModeledChildDirectoryObjectService<Modele
@Override @Override
protected ConnectionModel getModelInstance(ModeledAuthenticatedUser currentUser, protected ConnectionModel getModelInstance(ModeledAuthenticatedUser currentUser,
final Connection object) throws GuacamoleException { final Connection object) {
// Create new ModeledConnection backed by blank model // Create new ModeledConnection backed by blank model
ConnectionModel model = new ConnectionModel(); ConnectionModel model = new ConnectionModel();
ModeledConnection connection = getObjectInstance(currentUser, model); ModeledConnection connection = getObjectInstance(currentUser, model);
// If the provided connection has an identifier, set it on the model
if (object.getIdentifier() != null)
model.setObjectID(object.getIdentifier());
// Set model contents through ModeledConnection, copying the provided connection // Set model contents through ModeledConnection, copying the provided connection
connection.setParentIdentifier(object.getParentIdentifier()); connection.setParentIdentifier(object.getParentIdentifier());
connection.setIdentifier(object.getIdentifier());
connection.setName(object.getName()); connection.setName(object.getName());
connection.setConfiguration(object.getConfiguration()); connection.setConfiguration(object.getConfiguration());
connection.setAttributes(object.getAttributes()); connection.setAttributes(object.getAttributes());

View File

@@ -266,4 +266,10 @@ public class ConnectionGroupModel extends ChildObjectModel {
return id.toString(); return id.toString();
} }
@Override
public void setIdentifier(String identifier) {
throw new UnsupportedOperationException("Connection group identifiers are derived from IDs. They cannot be set.");
}
} }

View File

@@ -92,19 +92,14 @@ public class ConnectionGroupService extends ModeledChildDirectoryObjectService<M
@Override @Override
protected ConnectionGroupModel getModelInstance(ModeledAuthenticatedUser currentUser, protected ConnectionGroupModel getModelInstance(ModeledAuthenticatedUser currentUser,
final ConnectionGroup object) throws GuacamoleException { final ConnectionGroup object) {
// Create new ModeledConnectionGroup backed by blank model // Create new ModeledConnectionGroup backed by blank model
ConnectionGroupModel model = new ConnectionGroupModel(); ConnectionGroupModel model = new ConnectionGroupModel();
ModeledConnectionGroup connectionGroup = getObjectInstance(currentUser, model); ModeledConnectionGroup connectionGroup = getObjectInstance(currentUser, model);
// If the provided connection has an identifier, set it on the model
if (object.getIdentifier() != null)
model.setObjectID(object.getIdentifier());
// Set model contents through ModeledConnectionGroup, copying the provided connection group // Set model contents through ModeledConnectionGroup, copying the provided connection group
connectionGroup.setParentIdentifier(object.getParentIdentifier()); connectionGroup.setParentIdentifier(object.getParentIdentifier());
connectionGroup.setIdentifier(object.getIdentifier());
connectionGroup.setName(object.getName()); connectionGroup.setName(object.getName());
connectionGroup.setType(object.getType()); connectionGroup.setType(object.getType());
connectionGroup.setAttributes(object.getAttributes()); connectionGroup.setAttributes(object.getAttributes());

View File

@@ -71,4 +71,10 @@ public class SharingProfileModel extends ChildObjectModel {
} }
@Override
public void setIdentifier(String identifier) {
throw new UnsupportedOperationException("Sharing profile identifiers "
+ "are derived from IDs. They cannot be set.");
}
} }

View File

@@ -90,20 +90,15 @@ public class SharingProfileService
@Override @Override
protected SharingProfileModel getModelInstance(ModeledAuthenticatedUser currentUser, protected SharingProfileModel getModelInstance(ModeledAuthenticatedUser currentUser,
final SharingProfile object) throws GuacamoleException { final SharingProfile object) {
// Create new ModeledSharingProfile backed by blank model // Create new ModeledSharingProfile backed by blank model
SharingProfileModel model = new SharingProfileModel(); SharingProfileModel model = new SharingProfileModel();
ModeledSharingProfile sharingProfile = getObjectInstance(currentUser, model); ModeledSharingProfile sharingProfile = getObjectInstance(currentUser, model);
// If the provided connection has an identifier, set it on the model
if (object.getIdentifier() != null)
model.setObjectID(object.getIdentifier());
// Set model contents through ModeledSharingProfile, copying the // Set model contents through ModeledSharingProfile, copying the
// provided sharing profile // provided sharing profile
sharingProfile.setPrimaryConnectionIdentifier(object.getPrimaryConnectionIdentifier()); sharingProfile.setPrimaryConnectionIdentifier(object.getPrimaryConnectionIdentifier());
sharingProfile.setIdentifier(object.getIdentifier());
sharingProfile.setName(object.getName()); sharingProfile.setName(object.getName());
sharingProfile.setParameters(object.getParameters()); sharingProfile.setParameters(object.getParameters());
sharingProfile.setAttributes(object.getAttributes()); sharingProfile.setAttributes(object.getAttributes());

View File

@@ -202,10 +202,6 @@ public class UserService extends ModeledDirectoryObjectService<ModeledUser, User
UserModel model = new UserModel(); UserModel model = new UserModel();
ModeledUser user = getObjectInstance(currentUser, model); ModeledUser user = getObjectInstance(currentUser, model);
// If the provided connection has an identifier, set it on the model
if (object.getIdentifier() != null)
model.setObjectID(object.getIdentifier());
// Set model contents through ModeledUser, copying the provided user // Set model contents through ModeledUser, copying the provided user
user.setIdentifier(object.getIdentifier()); user.setIdentifier(object.getIdentifier());
user.setPassword(object.getPassword()); user.setPassword(object.getPassword());

View File

@@ -107,10 +107,6 @@ public class UserGroupService extends ModeledDirectoryObjectService<ModeledUserG
UserGroupModel model = new UserGroupModel(); UserGroupModel model = new UserGroupModel();
ModeledUserGroup group = getObjectInstance(currentUser, model); ModeledUserGroup group = getObjectInstance(currentUser, model);
// If the provided connection has an identifier, set it on the model
if (object.getIdentifier() != null)
model.setObjectID(object.getIdentifier());
// Set model contents through ModeledUser, copying the provided group // Set model contents through ModeledUser, copying the provided group
group.setIdentifier(object.getIdentifier()); group.setIdentifier(object.getIdentifier());
group.setAttributes(object.getAttributes()); group.setAttributes(object.getAttributes());

View File

@@ -417,12 +417,12 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
/** /**
* Applies the given object patches, updating the underlying directory * Applies the given object patches, updating the underlying directory
* accordingly. This operation supports addition, update, and removal of * accordingly. This operation supports addition and removal of objects
* objects through the "add", "replace", and "remove" patch operation. * through the "add" and "remove" patch operation. The path of each patch
* The path of each patch operation is of the form "/ID" where ID is the * operation is of the form "/ID" where ID is the identifier of the object
* identifier of the object being modified. In the case of object creation, * being modified. In the case of object creation, the identifier is
* the identifier is ignored, as the identifier will be automatically * ignored, as the identifier will be automatically provided. This operation
* provided. This operation is atomic. * is atomic.
* *
* @param patches * @param patches
* The patches to apply for this request. * The patches to apply for this request.
@@ -461,9 +461,8 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
"The patch cannot be executed."); "The patch cannot be executed.");
// Keep a list of all objects that have been successfully // Keep a list of all objects that have been successfully
// added, updated, or removed // added or removed
Collection<InternalType> addedObjects = new ArrayList<>(); Collection<InternalType> addedObjects = new ArrayList<>();
Collection<InternalType> updatedObjects = new ArrayList<>();
Collection<String> removedIdentifiers = new ArrayList<>(); Collection<String> removedIdentifiers = new ArrayList<>();
// A list of all responses associated with the successful // A list of all responses associated with the successful
@@ -527,64 +526,6 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
} }
else if (patch.getOp() == APIPatch.Operation.replace) {
// Filter/sanitize object contents
InternalType internal = filterAndTranslate(patch.getValue());
try {
// Set the model to the supplied identifier from the PATH
String identifier = path.substring(1);
if (identifier == null)
throw new GuacamoleClientException(
"An identifier is required when updating.");
// If there's an identiifer provided in the value,
// make sure it matches the one from the path
String valueId = internal.getIdentifier();
if (valueId != null && !valueId.equals(identifier))
throw new GuacamoleClientException(
"Identifier mismatch between path and value.");
internal.setIdentifier(identifier);
// Attempt to update the object
directory.update(internal);
// Add the object to the list if the update was successful
updatedObjects.add(internal);
// Add a success outcome describing the object update
APIPatchOutcome response = new APIPatchOutcome(
patch.getOp(), internal.getIdentifier(), path);
patchOutcomes.add(response);
creationSuccesses.add(response);
}
catch (GuacamoleException | RuntimeException | Error e) {
failed = true;
fireDirectoryFailureEvent(
DirectoryEvent.Operation.UPDATE,
internal.getIdentifier(), internal, e);
/*
* If the failure represents an understood issue,
* create a failure outcome for this failed patch.
*/
if (e instanceof GuacamoleException)
patchOutcomes.add(new APIPatchError(
patch.getOp(), internal.getIdentifier(), path,
((GuacamoleException) e).getMessage()));
// If an unexpected failure occurs, fall through to the
// standard API error handling
else
throw e;
}
}
// Append each identifier to the list, to be removed atomically // Append each identifier to the list, to be removed atomically
else if (patch.getOp() == APIPatch.Operation.remove) { else if (patch.getOp() == APIPatch.Operation.remove) {
@@ -626,6 +567,14 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
} }
} }
else {
throw new GuacamoleUnsupportedException(
"Unsupported patch operation \""
+ patch.getOp() + "\". "
+ "Only add and remove are supported.");
}
} }
// If any operation failed // If any operation failed
@@ -655,24 +604,13 @@ public abstract class DirectoryResource<InternalType extends Identifiable, Exter
} }
// Fire directory success events for each updated object
Iterator<InternalType> updatedIterator = updatedObjects.iterator();
while (updatedIterator.hasNext()) {
InternalType internal = updatedIterator.next();
fireDirectorySuccessEvent(
DirectoryEvent.Operation.UPDATE,
internal.getIdentifier(), internal);
}
// Fire directory success events for each removed object // Fire directory success events for each removed object
Iterator<String> removedIterator = removedIdentifiers.iterator(); Iterator<String> removedIterator = removedIdentifiers.iterator();
while (removedIterator.hasNext()) { while (removedIterator.hasNext()) {
String identifier = removedIterator.next(); String identifier = removedIterator.next();
fireDirectorySuccessEvent( fireDirectorySuccessEvent(
DirectoryEvent.Operation.UPDATE, DirectoryEvent.Operation.REMOVE,
identifier, null); identifier, null);
} }