Mercurial > hg > release > thermostat-1.0
changeset 1340:e5cf39c66810
Prevent NPEs if no metadada is provided to WebStorageEndpoint filters.
Reviewed-by: neugens
Review-thread: http://icedtea.classpath.org/pipermail/thermostat/2013-November/008816.html
author | Severin Gehwolf <sgehwolf@redhat.com> |
---|---|
date | Tue, 19 Nov 2013 12:06:01 +0100 |
parents | 00c8e314185a |
children | fb33ffad4da1 |
files | web/server/src/main/java/com/redhat/thermostat/web/server/auth/AgentIdFilter.java web/server/src/main/java/com/redhat/thermostat/web/server/auth/VmIdFilter.java web/server/src/test/java/com/redhat/thermostat/web/server/auth/AgentIdFilterTest.java web/server/src/test/java/com/redhat/thermostat/web/server/auth/HostnameFilterTest.java web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmIdFilterTest.java web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmUsernameFilterTest.java |
diffstat | 6 files changed, 62 insertions(+), 6 deletions(-) [+] |
line wrap: on
line diff
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/auth/AgentIdFilter.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/auth/AgentIdFilter.java Tue Nov 19 12:06:01 2013 +0100 @@ -71,7 +71,7 @@ Category<T> category = desc.getCategory(); // user cannot read all agents if (category.getKey(Key.AGENT_ID.getName()) != null) { - if (metaData.hasAgentId()) { + if (metaData != null && metaData.hasAgentId()) { // if given agent ID not in granted list, return empty String agentId = Objects.requireNonNull(metaData.getAgentId()); RolePrincipal agentIdGrantRole = new RolePrincipal(AGENTS_BY_AGENT_ID_GRANT_ROLE_PREFIX + agentId);
--- a/web/server/src/main/java/com/redhat/thermostat/web/server/auth/VmIdFilter.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/main/java/com/redhat/thermostat/web/server/auth/VmIdFilter.java Tue Nov 19 12:06:01 2013 +0100 @@ -70,7 +70,7 @@ // perform filtering on vmId Category<T> category = desc.getCategory(); if (category.getKey(Key.VM_ID.getName()) != null) { - if (metaData.hasVmId()) { + if (metaData != null && metaData.hasVmId()) { String vmId = metaData.getVmId(); RolePrincipal grantedByVmId = new RolePrincipal(VMS_BY_VM_ID_GRANT_ROLE_PREFIX + vmId); if (!userRoles.contains(grantedByVmId)) {
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/auth/AgentIdFilterTest.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/auth/AgentIdFilterTest.java Tue Nov 19 12:06:01 2013 +0100 @@ -41,6 +41,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.HashSet; import java.util.Set; @@ -124,11 +125,28 @@ @Test public void addsAgentIdInQueryToParentExpression() { + performAgentIdQueryTest(new DescriptorMetadata()); + } + + /* + * We shouldn't throw NPEs if no meta data is supplied by a plug-in. It's + * treated as if it was a query with no explicit agent/writer id in the + * descriptor. + */ + @Test + public void addsAgentIdInQueryWhenMetadataNull() { + try { + performAgentIdQueryTest(null); + } catch (NullPointerException e) { + fail("Should not have thrown NPE"); + } + } + + private void performAgentIdQueryTest(DescriptorMetadata metadata) { String agentId = UUID.randomUUID().toString(); Set<BasicRole> roles = new HashSet<>(); RolePrincipal agent1Role = new RolePrincipal(AgentIdFilter.AGENTS_BY_AGENT_ID_GRANT_ROLE_PREFIX + agentId); roles.add(agent1Role); - DescriptorMetadata metadata = new DescriptorMetadata(); AgentIdFilter<FooPojo> filter = new AgentIdFilter<>(roles); ExpressionFactory factory = new ExpressionFactory(); Expression parentExpression = factory.equalTo(Key.AGENT_ID, "testKey");
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/auth/HostnameFilterTest.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/auth/HostnameFilterTest.java Tue Nov 19 12:06:01 2013 +0100 @@ -40,6 +40,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.HashSet; import java.util.Set; @@ -89,12 +90,24 @@ @Test public void addsHostnameInQueryForHostInfo() { + performHostInfoTest(new DescriptorMetadata()); + } + + @Test + public void testNullMetadata() { + try { + performHostInfoTest(null); + } catch (NullPointerException e) { + fail("Should not have thrown NPE for this test"); + } + } + + private void performHostInfoTest(DescriptorMetadata metadata) { String testHostname = "testhost.example.com"; Set<BasicRole> roles = new HashSet<>(); RolePrincipal hostnameRole = new RolePrincipal(HostnameFilter.HOSTS_BY_HOSTNAME_GRANT_ROLE_PREFIX + testHostname); roles.add(hostnameRole); - DescriptorMetadata metadata = new DescriptorMetadata(); StatementDescriptor<HostInfo> desc = new StatementDescriptor<>(HostInfoDAO.hostInfoCategory, "QUERY " + HostInfoDAO.hostInfoCategory.getName()); Set<String> hostnames = new HashSet<>();
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmIdFilterTest.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmIdFilterTest.java Tue Nov 19 12:06:01 2013 +0100 @@ -41,6 +41,7 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; import java.util.HashSet; import java.util.Set; @@ -104,11 +105,27 @@ @Test public void addsVmIdInQuery() { + performVmIdQueryTest(new DescriptorMetadata()); + } + + /* + * We should not throw a NPE when the descriptor meta data is null. That is + * optional for filtering anyway and not all plugins may supply us with one. + */ + @Test + public void addsVmIdInQueryIfMetadataNull() { + try { + performVmIdQueryTest(null); + } catch (NullPointerException e) { + fail("Should not have thrown NPE"); + } + } + + private void performVmIdQueryTest(DescriptorMetadata metadata) { String vmId = UUID.randomUUID().toString(); Set<BasicRole> roles = new HashSet<>(); RolePrincipal vmIdRole = new RolePrincipal(VmIdFilter.VMS_BY_VM_ID_GRANT_ROLE_PREFIX + vmId); roles.add(vmIdRole); - DescriptorMetadata metadata = new DescriptorMetadata(); VmIdFilter<FooPojo> filter = new VmIdFilter<>(roles); FilterResult result = filter.applyFilter(TEST_DESC_NON_NULL_VM_ID, metadata, null); assertEquals(ResultType.QUERY_EXPRESSION, result.getType());
--- a/web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmUsernameFilterTest.java Mon Nov 18 20:32:30 2013 -0700 +++ b/web/server/src/test/java/com/redhat/thermostat/web/server/auth/VmUsernameFilterTest.java Tue Nov 19 12:06:01 2013 +0100 @@ -89,12 +89,20 @@ @Test public void addsVmUsernameInQueryForVmInfo() { + performVmInfoTest(new DescriptorMetadata()); + } + + @Test + public void testMetadataNull() { + performVmInfoTest(null); + } + + private void performVmInfoTest(DescriptorMetadata metadata) { String testUsername = "fooBar"; Set<BasicRole> roles = new HashSet<>(); RolePrincipal vmUsernameRole = new RolePrincipal(VmUsernameFilter.VMS_BY_USERNAME_GRANT_ROLE_PREFIX + testUsername); roles.add(vmUsernameRole); - DescriptorMetadata metadata = new DescriptorMetadata(); StatementDescriptor<VmInfo> desc = new StatementDescriptor<>(VmInfoDAO.vmInfoCategory, "QUERY " + VmInfoDAO.vmInfoCategory.getName()); Set<String> usernames = new HashSet<>();