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<>();