From 0e38acbd594d76fd4d5b775cb120fbdd3ce84810 Mon Sep 17 00:00:00 2001 From: Michael Jumper Date: Thu, 12 Feb 2015 21:29:58 -0800 Subject: [PATCH] GUAC-1101: Limit results of retrieval operations by read permissions, unless user is a sysadmin. --- .../guacamole/net/auth/mysql/MySQLUser.java | 17 ++++++ .../auth/mysql/dao/DirectoryObjectMapper.java | 57 ++++++++++++++++--- .../mysql/service/DirectoryObjectService.java | 38 +++++++++++-- .../net/auth/mysql/dao/UserMapper.xml | 31 ++++++++++ 4 files changed, 131 insertions(+), 12 deletions(-) diff --git a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/MySQLUser.java b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/MySQLUser.java index 36d159afe..c1c2cd1ef 100644 --- a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/MySQLUser.java +++ b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/MySQLUser.java @@ -29,6 +29,7 @@ import net.sourceforge.guacamole.net.auth.mysql.service.SaltService; import org.glyptodon.guacamole.GuacamoleException; import org.glyptodon.guacamole.net.auth.User; import org.glyptodon.guacamole.net.auth.permission.ObjectPermissionSet; +import org.glyptodon.guacamole.net.auth.permission.SystemPermission; import org.glyptodon.guacamole.net.auth.permission.SystemPermissionSet; import org.glyptodon.guacamole.net.auth.simple.SimpleObjectPermissionSet; import org.glyptodon.guacamole.net.auth.simple.SimpleSystemPermissionSet; @@ -126,6 +127,22 @@ public class MySQLUser implements User, DirectoryObject { } + /** + * Returns whether this user is a system administrator, and thus is not + * restricted by permissions. + * + * @return + * true if this user is a system administrator, false otherwise. + * + * @throws GuacamoleException + * If an error occurs while determining the user's system administrator + * status. + */ + public boolean isAdministrator() throws GuacamoleException { + SystemPermissionSet systemPermissionSet = getSystemPermissions(); + return systemPermissionSet.hasPermission(SystemPermission.Type.ADMINISTER); + } + @Override public SystemPermissionSet getSystemPermissions() throws GuacamoleException { diff --git a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/dao/DirectoryObjectMapper.java b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/dao/DirectoryObjectMapper.java index 3687b95c3..69ffa133a 100644 --- a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/dao/DirectoryObjectMapper.java +++ b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/dao/DirectoryObjectMapper.java @@ -24,6 +24,7 @@ package net.sourceforge.guacamole.net.auth.mysql.dao; import java.util.Collection; import java.util.Set; +import net.sourceforge.guacamole.net.auth.mysql.model.UserModel; import org.apache.ibatis.annotations.Param; /** @@ -32,23 +33,45 @@ import org.apache.ibatis.annotations.Param; * to fulfill the needs of the Directory class. * * @author Michael Jumper - * @param + * @param * The type of object contained within the directory whose objects are * mapped by this mapper. */ -public interface DirectoryObjectMapper { +public interface DirectoryObjectMapper { /** - * Selects the identifiers of all objects. + * Selects the identifiers of all objects, regardless of whether they + * are readable by any particular user. This should only be called on + * behalf of a system administrator. If identifiers are needed by a non- + * administrative user who must have explicit read rights, use + * selectReadableIdentifiers() instead. * * @return * A Set containing all identifiers of all objects. */ Set selectIdentifiers(); + /** + * Selects the identifiers of all objects that are explicitly readable by + * the given user. If identifiers are needed by a system administrator + * (who, by definition, does not need explicit read rights), use + * selectIdentifiers() instead. + * + * @param user + * The user whose permissions should determine whether an identifier + * is returned. + * + * @return + * A Set containing all identifiers of all readable objects. + */ + Set selectReadableIdentifiers(@Param("user") UserModel user); + /** * Selects all objects which have the given identifiers. If an identifier - * has no corresponding object, it will be ignored. + * has no corresponding object, it will be ignored. This should only be + * called on behalf of a system administrator. If objects are needed by a + * non-administrative user who must have explicit read rights, use + * selectReadable() instead. * * @param identifiers * The identifiers of the objects to return. @@ -56,7 +79,27 @@ public interface DirectoryObjectMapper { * @return * A Collection of all objects having the given identifiers. */ - Collection select(@Param("identifiers") Collection identifiers); + Collection select(@Param("identifiers") Collection identifiers); + + /** + * Selects all objects which have the given identifiers and are explicitly + * readably by the given user. If an identifier has no corresponding + * object, or the corresponding object is unreadable, it will be ignored. + * If objects are needed by a system administrator (who, by definition, + * does not need explicit read rights), use select() instead. + * + * @param user + * The user whose permissions should determine whether an object + * is returned. + * + * @param identifiers + * The identifiers of the objects to return. + * + * @return + * A Collection of all objects having the given identifiers. + */ + Collection selectReadable(@Param("user") UserModel user, + @Param("identifiers") Collection identifiers); /** * Inserts the given object into the database. If the object already @@ -68,7 +111,7 @@ public interface DirectoryObjectMapper { * @return * The number of rows inserted. */ - int insert(@Param("object") T object); + int insert(@Param("object") ModelType object); /** * Deletes the given object into the database. If the object does not @@ -92,6 +135,6 @@ public interface DirectoryObjectMapper { * @return * The number of rows updated. */ - int update(@Param("object") T object); + int update(@Param("object") ModelType object); } \ No newline at end of file diff --git a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/DirectoryObjectService.java b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/DirectoryObjectService.java index 933551009..a44df1e2d 100644 --- a/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/DirectoryObjectService.java +++ b/extensions/guacamole-auth-mysql/src/main/java/net/sourceforge/guacamole/net/auth/mysql/service/DirectoryObjectService.java @@ -103,9 +103,12 @@ public abstract class DirectoryObjectService objects = retrieveObjects(user, Collections.singleton(identifier)); @@ -135,16 +138,29 @@ public abstract class DirectoryObjectService retrieveObjects(AuthenticatedUser user, - Collection identifiers) { + Collection identifiers) throws GuacamoleException { // Do not query if no identifiers given if (identifiers.isEmpty()) return Collections.EMPTY_LIST; + Collection objects; + + // Bypass permission checks if the user is a system admin + if (user.getUser().isAdministrator()) + objects = getObjectMapper().select(identifiers); + + // Otherwise only return explicitly readable identifiers + else + objects = getObjectMapper().selectReadable(user.getUser().getModel(), identifiers); + // Return collection of requested objects - return getObjectInstances(getObjectMapper().select(identifiers)); + return getObjectInstances(objects); } @@ -215,9 +231,21 @@ public abstract class DirectoryObjectService getIdentifiers(AuthenticatedUser user) { - return getObjectMapper().selectIdentifiers(); + public Set getIdentifiers(AuthenticatedUser user) + throws GuacamoleException { + + // Bypass permission checks if the user is a system admin + if (user.getUser().isAdministrator()) + return getObjectMapper().selectIdentifiers(); + + // Otherwise only return explicitly readable identifiers + else + return getObjectMapper().selectReadableIdentifiers(user.getUser().getModel()); + } } diff --git a/extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/UserMapper.xml b/extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/UserMapper.xml index 695e1956b..32fa591fc 100644 --- a/extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/UserMapper.xml +++ b/extensions/guacamole-auth-mysql/src/main/resources/net/sourceforge/guacamole/net/auth/mysql/dao/UserMapper.xml @@ -40,6 +40,16 @@ FROM guacamole_user + + + + + + +