diff --git a/gradle.properties b/gradle.properties index e6640dcd3ff..1e95b09fbf6 100644 --- 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 index b452c1c45a1..9b70114a9ac 100644 --- 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/server/src/main/java/org/cloudfoundry/identity/uaa/client/JdbcQueryableClientDetailsService.java b/server/src/main/java/org/cloudfoundry/identity/uaa/client/JdbcQueryableClientDetailsService.java index a9e453f4fc0..d548477254c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/client/JdbcQueryableClientDetailsService.java +++ b/server/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 implements QueryableResourceManager { @@ -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 { @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 index 244817e6495..9b92ca6549e 100644 --- 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,6 +12,10 @@ *******************************************************************************/ 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; @@ -19,8 +23,7 @@ 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 implements Queryable { @@ -82,6 +85,7 @@ public List query(String filter) { @Override public List 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 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 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 index 82475ac374b..15f9bc1acb0 100644 --- 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/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SimpleSearchQueryConverter.java b/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SimpleSearchQueryConverter.java index 39bad5d40dc..1d129e3c18c 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/resources/jdbc/SimpleSearchQueryConverter.java +++ b/server/src/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 values, Attrib } protected String comparisonClause(SCIMFilter filter, String comparator, Map 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 values, String paramPrefix) { + protected String getParamName(Map 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 index 0c53781858f..aa0a024ed14 100644 --- 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/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupExternalMembershipManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupExternalMembershipManager.java index 7f538be6428..05ecd37d34f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupExternalMembershipManager.java +++ b/server/src/main/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 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/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java index 3423467cf16..d3dddd2123f 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManager.java @@ -64,7 +64,7 @@ public class JdbcScimGroupMembershipManager extends AbstractQueryable implements ScimGroupProvisioning, ApplicationListener>, 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 { @Override diff --git a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java index 3512ba566e9..cee9e7ff439 100644 --- a/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java +++ b/server/src/main/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimUserProvisioning.java @@ -12,6 +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.Calendar; +import java.util.Date; +import java.util.GregorianCalendar; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.regex.Pattern; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.cloudfoundry.identity.uaa.audit.event.SystemDeletable; @@ -42,19 +55,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.Calendar; -import java.util.Date; -import java.util.GregorianCalendar; -import java.util.HashMap; -import java.util.List; -import java.util.Map; -import java.util.UUID; -import java.util.regex.Pattern; - import static java.sql.Types.VARCHAR; /** @@ -486,4 +486,8 @@ public int getTotalCount() { return count; } + @Override + protected void validateOrderBy(String orderBy) throws IllegalArgumentException { + super.validateOrderBy(orderBy, USER_FIELDS); + } } diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java index 8db16e44c7e..4a26ad64ed7 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/endpoints/ScimUserEndpointsTests.java @@ -12,6 +12,15 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.scim.endpoints; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.cloudfoundry.identity.uaa.approval.Approval; @@ -68,15 +77,6 @@ import org.springframework.web.HttpMediaTypeException; import org.springframework.web.servlet.View; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Map; -import java.util.Set; - import static org.hamcrest.Matchers.containsString; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; @@ -646,6 +646,27 @@ public void testInvalidFilterExpression() { assertEquals(0, results.getTotalResults()); } + @Test + public void testValidFilterExpression() { + SearchResults results = endpoints.findUsers("id", "userName eq \"d\"", "created", "ascending", 1, 100); + assertEquals(0, results.getTotalResults()); + } + + @Test + public void testInvalidOrderByExpression() { + expected.expect(ScimException.class); + expected.expectMessage(containsString("Invalid filter")); + SearchResults results = endpoints.findUsers("id", "userName eq \"d\"", "created,unknown", "ascending", 1, 100); + assertEquals(0, results.getTotalResults()); + } + + @Test + public void testValidOrderByExpression() { + endpoints.findUsers("id", "userName eq \"d\"", "1,created", "ascending", 1, 100); + endpoints.findUsers("id", "userName eq \"d\"", "1,2", "ascending", 1, 100); + endpoints.findUsers("id", "userName eq \"d\"", "username,created", "ascending", 1, 100); + } + @SuppressWarnings("unchecked") @Test public void testFindIdsByUserName() { diff --git a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManagerTests.java b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManagerTests.java index 5cb4c8c6f37..57202cc0ada 100644 --- a/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManagerTests.java +++ b/server/src/test/java/org/cloudfoundry/identity/uaa/scim/jdbc/JdbcScimGroupMembershipManagerTests.java @@ -12,8 +12,16 @@ *******************************************************************************/ package org.cloudfoundry.identity.uaa.scim.jdbc; -import org.cloudfoundry.identity.uaa.constants.OriginKeys; +import java.sql.Timestamp; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Set; + import org.cloudfoundry.identity.uaa.audit.event.EntityDeletedEvent; +import org.cloudfoundry.identity.uaa.constants.OriginKeys; import org.cloudfoundry.identity.uaa.provider.IdentityProvider; import org.cloudfoundry.identity.uaa.resources.jdbc.JdbcPagingListFactory; import org.cloudfoundry.identity.uaa.scim.ScimGroup; @@ -32,14 +40,6 @@ import org.springframework.jdbc.core.JdbcTemplate; import org.springframework.security.oauth2.common.util.RandomValueStringGenerator; -import java.sql.Timestamp; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.List; -import java.util.Set; - import static org.cloudfoundry.identity.uaa.constants.OriginKeys.LOGIN_SERVER; import static org.cloudfoundry.identity.uaa.constants.OriginKeys.UAA; import static org.hamcrest.core.Is.is; @@ -173,6 +173,16 @@ public void canQuery_Filter_Has_ZoneIn_Effect() throws Exception { IdentityZone zone = MultitenancyFixture.identityZone(id,id); IdentityZoneHolder.set(zone); assertEquals(0,dao.query("origin eq \"" + OriginKeys.UAA + "\"").size()); + IdentityZoneHolder.clear(); + assertEquals(4,dao.query("origin eq \"" + OriginKeys.UAA + "\"").size()); + assertEquals(4,dao.query("origin eq \"" + OriginKeys.UAA + "\"", "member_id", true).size()); + assertEquals(4,dao.query("origin eq \"" + OriginKeys.UAA + "\"", "1,2", true).size()); + assertEquals(4,dao.query("origin eq \"" + OriginKeys.UAA + "\"", "origin", true).size()); + } + + @Test(expected = IllegalArgumentException.class) + public void cannotQuery_Filter_Has_Unknown_Sort() throws Exception { + dao.query("origin eq \"" + OriginKeys.UAA + "\"", "unknown,origin", true); } diff --git a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java index e3e4612bc22..54f04fa5229 100644 --- a/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java +++ b/uaa/src/test/java/org/cloudfoundry/identity/uaa/mock/clients/ClientAdminEndpointsMockMvcTests.java @@ -1,5 +1,15 @@ package org.cloudfoundry.identity.uaa.mock.clients; +import java.util.Arrays; +import java.util.Collection; +import java.util.Collections; +import java.util.Date; +import java.util.HashMap; +import java.util.LinkedHashSet; +import java.util.List; +import java.util.Map; +import javax.servlet.http.HttpServletResponse; + import com.fasterxml.jackson.core.type.TypeReference; import com.google.common.collect.Iterables; import org.cloudfoundry.identity.uaa.approval.Approval; @@ -46,17 +56,6 @@ import org.springframework.test.web.servlet.request.MockHttpServletRequestBuilder; import org.springframework.util.StringUtils; -import javax.servlet.http.HttpServletResponse; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.LinkedHashSet; -import java.util.List; -import java.util.Map; - -import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.iterableWithSize; import static org.hamcrest.core.Is.is; @@ -66,7 +65,6 @@ import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; -import static org.mockito.Matchers.contains; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -1272,7 +1270,7 @@ public void testGetClientDetailsSortedByLastModified() throws Exception{ MockHttpServletRequestBuilder get = get("/oauth/clients") .header("Authorization", "Bearer " + token) - .param("sortBy", "lastModified") + .param("sortBy", "lastmodified") .param("sortOrder", "descending") .accept(APPLICATION_JSON);