Merge branch 'releases/3.4.1'

cloudfoundry · Jun 20, 2016 · ddae2a1 · ddae2a1
2 parents 23e4e93 + fb76448
commit ddae2a1
Show file tree

Hide file tree

Showing 14 changed files with 205 additions and 94 deletions.
diff --git a/gradle.properties b/gradle.properties
@@ -1 +1 @@
-version=3.4.0
+version=3.4.1
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/ClientAdminEndpoints.java
@@ -12,6 +12,17 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.client;
 
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.servlet.http.HttpServletRequest;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.approval.ApprovalStore;
@@ -65,17 +76,6 @@
 import org.springframework.web.context.request.RequestContextHolder;
 import org.springframework.web.context.request.ServletRequestAttributes;
 
-import javax.servlet.http.HttpServletRequest;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicInteger;
-
 /**
  * Controller for listing and manipulating OAuth2 clients.
  */
@@ -467,7 +467,11 @@ public SearchResults<?> listClientDetails(
                 count = clients.size();
             }
         } catch (IllegalArgumentException e) {
-            throw new UaaException("Invalid filter expression: [" + filter + "]", HttpStatus.BAD_REQUEST.value());
+            String msg = "Invalid filter expression: [" + filter + "]";
+            if (StringUtils.hasText(sortBy)) {
+                msg += " [" +sortBy+"]";
+            }
+            throw new UaaException(msg, HttpStatus.BAD_REQUEST.value());
         }
         for (ClientDetails client : UaaPagingUtils.subList(clients, startIndex, count)) {
             result.add(removeSecret(client));

diff --git a/...src/main/java/org/cloudfoundry/identity/uaa/client/JdbcQueryableClientDetailsService.java b/...src/main/java/org/cloudfoundry/identity/uaa/client/JdbcQueryableClientDetailsService.java
@@ -12,6 +12,11 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.client;
 
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.util.List;
+import java.util.Map;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.resources.QueryableResourceManager;
@@ -26,11 +31,6 @@
 import org.springframework.security.oauth2.provider.client.JdbcClientDetailsService;
 import org.springframework.util.StringUtils;
 
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.util.List;
-import java.util.Map;
-
 public class JdbcQueryableClientDetailsService extends AbstractQueryable<ClientDetails> implements
                 QueryableResourceManager<ClientDetails> {
 
@@ -101,6 +101,11 @@ public ClientDetails delete(String id, int version) {
         return client;
     }
 
+    @Override
+    protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
+        super.validateOrderBy(orderBy, CLIENT_FIELDS);
+    }
+
     private static class ClientDetailsRowMapper implements RowMapper<ClientDetails> {
 
         @Override

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/AbstractQueryable.java b/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/AbstractQueryable.java
@@ -12,15 +12,18 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.resources.jdbc;
 
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.resources.Queryable;
 import org.springframework.dao.DataAccessException;
 import org.springframework.jdbc.core.JdbcTemplate;
 import org.springframework.jdbc.core.RowMapper;
 import org.springframework.jdbc.core.namedparam.NamedParameterJdbcTemplate;
-
-import java.util.List;
+import org.springframework.util.StringUtils;
 
 public abstract class AbstractQueryable<T> implements Queryable<T> {
 
@@ -82,6 +85,7 @@ public List<T> query(String filter) {
 
     @Override
     public List<T> query(String filter, String sortBy, boolean ascending) {
+        validateOrderBy(queryConverter.map(sortBy));
         SearchQueryConverter.ProcessedFilter where = queryConverter.convert(filter, sortBy, ascending);
         logger.debug("Filtering groups with SQL: " + where);
         List<T> result;
@@ -115,6 +119,35 @@ protected String getQuerySQL(String filter, SearchQueryConverter.ProcessedFilter
     protected abstract String getBaseSqlQuery();
     protected abstract String getTableName();
 
+    protected abstract void validateOrderBy(String orderBy) throws IllegalArgumentException;
+
+    protected void validateOrderBy(String orderBy, String fields) throws IllegalArgumentException {
+        if (!StringUtils.hasText(orderBy)) {
+            return;
+        }
+        String[] input = StringUtils.commaDelimitedListToStringArray(orderBy);
+        Set<String> compare = new HashSet<>();
+        StringUtils.commaDelimitedListToSet(fields)
+                .stream()
+                .forEach(p -> compare.add(p.toLowerCase().trim()));
+        boolean allints = true;
+        for (String s : input) {
+            try {
+                Integer.parseInt(s);
+            } catch (NumberFormatException e) {
+                allints = false;
+                if (!compare.contains(s.toLowerCase().trim())) {
+                    throw new IllegalArgumentException("Invalid sort field:"+s);
+                }
+            }
+        }
+        if (allints) {
+            return;
+        }
+
+
+    }
+
     public SearchQueryConverter getQueryConverter() {
         return queryConverter;
     }

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SearchQueryConverter.java b/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SearchQueryConverter.java
@@ -12,10 +12,10 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.resources.jdbc;
 
-import org.cloudfoundry.identity.uaa.resources.AttributeNameMapper;
-
 import java.util.Map;
 
+import org.cloudfoundry.identity.uaa.resources.AttributeNameMapper;
+
 public interface SearchQueryConverter {
 
     final class ProcessedFilter {
@@ -63,4 +63,6 @@ public String toString() {
 
     ProcessedFilter convert(String filter, String sortBy, boolean ascending, AttributeNameMapper mapper);
 
+    String map(String attribute);
+
 }
diff --git a/...rc/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SimpleSearchQueryConverter.java b/...rc/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SimpleSearchQueryConverter.java
@@ -13,6 +13,13 @@
 
 package org.cloudfoundry.identity.uaa.resources.jdbc;
 
+import java.text.DateFormat;
+import java.text.ParseException;
+import java.text.SimpleDateFormat;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+
 import com.unboundid.scim.sdk.SCIMException;
 import com.unboundid.scim.sdk.SCIMFilter;
 import org.apache.commons.logging.Log;
@@ -22,13 +29,6 @@
 import org.springframework.security.oauth2.common.util.RandomValueStringGenerator;
 import org.springframework.util.StringUtils;
 
-import java.text.DateFormat;
-import java.text.ParseException;
-import java.text.SimpleDateFormat;
-import java.util.Collections;
-import java.util.HashMap;
-import java.util.Map;
-
 import static org.cloudfoundry.identity.uaa.resources.jdbc.SearchQueryConverter.ProcessedFilter.ORDER_BY;
 
 public class SimpleSearchQueryConverter implements SearchQueryConverter {
@@ -131,7 +131,7 @@ private String createFilter(SCIMFilter filter, Map<String,Object> values, Attrib
     }
 
     protected String comparisonClause(SCIMFilter filter, String comparator, Map<String, Object> values, String valuePrefix, String valueSuffix, String paramPrefix) {
-        String pName = getParamName(filter, values, paramPrefix);
+        String pName = getParamName(values, paramPrefix);
         String paramName = ":"+pName;
         if (filter.getFilterValue() == null) {
             return getAttributeName(filter, mapper) + " IS NULL";
@@ -184,7 +184,7 @@ protected String getAttributeName(SCIMFilter filter, AttributeNameMapper mapper)
         return name.replace("meta.", "");
     }
 
-    protected String getParamName(SCIMFilter filter, Map<String, Object> values, String paramPrefix) {
+    protected String getParamName(Map<String, Object> values, String paramPrefix) {
         return paramPrefix+values.size();
     }
 
@@ -197,6 +197,8 @@ protected Object getStringOrDate(String s) {
         }
     }
 
-
-
+    @Override
+    public String map(String attribute) {
+        return StringUtils.hasText(attribute) ? mapper.mapToInternal(attribute) : attribute;
+    }
 }
diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpoints.java
@@ -12,6 +12,21 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.scim.endpoints;
 
+import java.security.SecureRandom;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Random;
+import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicInteger;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.approval.Approval;
@@ -344,7 +359,11 @@ public SearchResults<?> findUsers(
                 input.add(user);
             }
         } catch (IllegalArgumentException e) {
-            throw new ScimException("Invalid filter expression: [" + filter + "]", HttpStatus.BAD_REQUEST);
+            String msg = "Invalid filter expression: [" + filter + "]";
+            if (StringUtils.hasText(sortBy)) {
+                msg += " [" +sortBy+"]";
+            }
+            throw new ScimException(msg, HttpStatus.BAD_REQUEST);
         }
 
         if (!StringUtils.hasLength(attributesCommaSeparated)) {

diff --git a/.../java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupExternalMembershipManager.java b/.../java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupExternalMembershipManager.java
@@ -12,6 +12,12 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.scim.jdbc;
 
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.List;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.resources.jdbc.AbstractQueryable;
@@ -36,12 +42,6 @@
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Timestamp;
-import java.util.List;
-
 public class JdbcScimGroupExternalMembershipManager extends AbstractQueryable<ScimGroupExternalMember>
     implements ScimGroupExternalMembershipManager {
 
@@ -320,4 +320,8 @@ protected String getBaseSqlQuery() {
             JOIN_EXTERNAL_GROUP_MAPPING_FIELDS, JOIN_GROUP_TABLE, "g.id = gm.group_id and g.identity_zone_id='"+IdentityZoneHolder.get().getId()+"'");
     }
 
+    @Override
+    protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
+        super.validateOrderBy(orderBy, EXTERNAL_GROUP_MAPPING_FIELDS);
+    }
 }
diff --git a/...src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java b/...src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java
@@ -64,7 +64,7 @@ public class JdbcScimGroupMembershipManager extends AbstractQueryable<ScimGroupM
     public static final String MEMBERSHIP_FIELDS = "group_id,member_id,member_type,authorities,added,origin";
 
     public static final String MEMBERSHIP_TABLE = "group_membership";
-    
+
     public static final String ADD_MEMBER_SQL = String.format("insert into %s ( %s ) values (?,?,?,?,?,?)", MEMBERSHIP_TABLE, MEMBERSHIP_FIELDS);
 
     public static final String UPDATE_MEMBER_SQL = String.format("update %s set authorities=? where group_id=? and member_id=?", MEMBERSHIP_TABLE);
@@ -421,6 +421,11 @@ public void setValues(PreparedStatement ps) throws SQLException {
         return groups;
     }
 
+    @Override
+    protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
+        super.validateOrderBy(orderBy, MEMBERSHIP_FIELDS);
+    }
+
     private boolean isUser(String uuid) {
         try {
             userProvisioning.retrieve(uuid);

diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupProvisioning.java
@@ -12,12 +12,19 @@
  *******************************************************************************/
 package org.cloudfoundry.identity.uaa.scim.jdbc;
 
+import java.sql.PreparedStatement;
+import java.sql.ResultSet;
+import java.sql.SQLException;
+import java.sql.Timestamp;
+import java.util.Date;
+import java.util.List;
+import java.util.UUID;
+
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent;
 import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable;
 import org.cloudfoundry.identity.uaa.resources.jdbc.AbstractQueryable;
-import org.cloudfoundry.identity.uaa.resources.jdbc.DefaultLimitSqlAdapter;
 import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory;
 import org.cloudfoundry.identity.uaa.scim.ScimGroup;
 import org.cloudfoundry.identity.uaa.scim.ScimGroupProvisioning;
@@ -38,14 +45,6 @@
 import org.springframework.util.Assert;
 import org.springframework.util.StringUtils;
 
-import java.sql.PreparedStatement;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.sql.Timestamp;
-import java.util.Date;
-import java.util.List;
-import java.util.UUID;
-
 public class JdbcScimGroupProvisioning extends AbstractQueryable<ScimGroup>
     implements ScimGroupProvisioning, ApplicationListener<EntityDeletedEvent<?>>, SystemDeletable {
 
@@ -234,6 +233,11 @@ protected void validateGroup(ScimGroup group) throws ScimResourceConstraintFaile
         }
     }
 
+    @Override
+    protected void validateOrderBy(String orderBy) throws IllegalArgumentException {
+        super.validateOrderBy(orderBy, GROUP_FIELDS);
+    }
+
     private static final class ScimGroupRowMapper implements RowMapper<ScimGroup> {
 
         @Override