Fix auth check in InventoryViewUtils (#4869)

server/src/main/java/io/druid/server/http/DatasourcesResource.java

@@ -43,7 +43,6 @@ import io.druid.metadata.MetadataSegmentManager;
 import io.druid.query.TableDataSource;
 import io.druid.server.http.security.DatasourceResourceFilter;
 import io.druid.server.security.AuthConfig;
-import io.druid.server.security.AuthenticationResult;
 import io.druid.server.security.AuthorizerMapper;
 import io.druid.timeline.DataSegment;
 import io.druid.timeline.TimelineLookup;
@@ -110,9 +109,9 @@ public class DatasourcesResource
   {
     Response.ResponseBuilder builder = Response.ok();
     final Set<DruidDataSource> datasources = InventoryViewUtils.getSecuredDataSources(
+        req,
         serverInventoryView,
-        authorizerMapper,
+        authorizerMapper
-        (AuthenticationResult) req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)
     );
 
     if (full != null) {

server/src/main/java/io/druid/server/http/IntervalsResource.java

@@ -27,7 +27,6 @@ import io.druid.java.util.common.Intervals;
 import io.druid.java.util.common.MapUtils;
 import io.druid.java.util.common.guava.Comparators;
 import io.druid.server.security.AuthConfig;
-import io.druid.server.security.AuthenticationResult;
 import io.druid.server.security.AuthorizerMapper;
 import io.druid.timeline.DataSegment;
 import org.joda.time.Interval;
@@ -72,9 +71,9 @@ public class IntervalsResource
   {
     final Comparator<Interval> comparator = Comparators.inverse(Comparators.intervalsByStartThenEnd());
     final Set<DruidDataSource> datasources = InventoryViewUtils.getSecuredDataSources(
+        req,
         serverInventoryView,
-        authorizerMapper,
+        authorizerMapper
-        (AuthenticationResult) req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)
     );
 
     final Map<Interval, Map<String, Map<String, Object>>> retVal = Maps.newTreeMap(comparator);
@@ -104,9 +103,9 @@ public class IntervalsResource
   {
     final Interval theInterval = Intervals.of(interval.replace("_", "/"));
     final Set<DruidDataSource> datasources = InventoryViewUtils.getSecuredDataSources(
+        req,
         serverInventoryView,
-        authorizerMapper,
+        authorizerMapper
-        (AuthenticationResult) req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)
     );
 
     final Comparator<Interval> comparator = Comparators.inverse(Comparators.intervalsByStartThenEnd());

server/src/main/java/io/druid/server/http/InventoryViewUtils.java

@@ -20,7 +20,6 @@
 package io.druid.server.http;
 
 import com.google.common.base.Function;
-import com.google.common.base.Predicate;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -30,18 +29,11 @@ import io.druid.client.DruidDataSource;
 import io.druid.client.DruidServer;
 import io.druid.client.InventoryView;
 import io.druid.java.util.common.ISE;
-import io.druid.java.util.common.Pair;
+import io.druid.server.security.AuthorizationUtils;
-import io.druid.server.security.Access;
-import io.druid.server.security.Action;
-import io.druid.server.security.AuthenticationResult;
-import io.druid.server.security.Authorizer;
 import io.druid.server.security.AuthorizerMapper;
-import io.druid.server.security.Resource;
-import io.druid.server.security.ResourceType;
 
+import javax.servlet.http.HttpServletRequest;
 import java.util.Comparator;
-import java.util.HashMap;
-import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
@@ -81,42 +73,26 @@ public class InventoryViewUtils
   }
 
   public static Set<DruidDataSource> getSecuredDataSources(
+      HttpServletRequest request,
       InventoryView inventoryView,
-      final AuthorizerMapper authorizerMapper,
+      final AuthorizerMapper authorizerMapper
-      final AuthenticationResult authenticationResult
   )
   {
     if (authorizerMapper == null) {
       throw new ISE("No authorization mapper found");
     }
 
-    final Authorizer authorizer = authorizerMapper.getAuthorizer(authenticationResult.getAuthorizerName());
+    return ImmutableSet.copyOf(
-    if (authorizer == null) {
+        AuthorizationUtils.filterAuthorizedResources(
-      throw new ISE("Invalid to call a secured method with null Authorizer!!");
+            request,
-    } else {
+            getDataSources(inventoryView),
-      final Map<Pair<Resource, Action>, Access> resourceAccessMap = new HashMap<>();
+            datasource -> {
-      return ImmutableSet.copyOf(
+              return Lists.newArrayList(
-          Iterables.filter(
+                  AuthorizationUtils.DATASOURCE_READ_RA_GENERATOR.apply(datasource.getName())
-              getDataSources(inventoryView),
+              );
-              new Predicate<DruidDataSource>()
+            },
-              {
+            authorizerMapper
-                @Override
+        )
-                public boolean apply(DruidDataSource input)
+    );
-                {
-                  Resource resource = new Resource(input.getName(), ResourceType.DATASOURCE);
-                  Action action = Action.READ;
-                  Pair<Resource, Action> key = new Pair<>(resource, action);
-                  if (resourceAccessMap.containsKey(key)) {
-                    return resourceAccessMap.get(key).isAllowed();
-                  } else {
-                    Access access = authorizer.authorize(authenticationResult, key.lhs, key.rhs);
-                    resourceAccessMap.put(key, access);
-                    return access.isAllowed();
-                  }
-                }
-              }
-          )
-      );
-    }
   }
 }

server/src/test/java/io/druid/server/http/DatasourcesResourceTest.java

@@ -118,15 +118,34 @@ public class DatasourcesResourceTest
   @Test
   public void testGetFullQueryableDataSources() throws Exception
   {
+    // first request
     EasyMock.expect(server.getDataSources()).andReturn(
         ImmutableList.of(listDataSources.get(0), listDataSources.get(1))
-    ).atLeastOnce();
+    ).once();
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
-    ).atLeastOnce();
+    ).once();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
-      new AuthenticationResult("druid", "druid")
+        new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
+
+    // second request
+    EasyMock.expect(server.getDataSources()).andReturn(
+        ImmutableList.of(listDataSources.get(0), listDataSources.get(1))
+    ).once();
+    EasyMock.expect(inventoryView.getInventory()).andReturn(
+        ImmutableList.of(server)
+    ).once();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
+        new AuthenticationResult("druid", "druid")
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
+
     EasyMock.replay(inventoryView, server, request);
     DatasourcesResource datasourcesResource = new DatasourcesResource(
         inventoryView,
@@ -155,18 +174,37 @@ public class DatasourcesResourceTest
   @Test
   public void testSecuredGetFullQueryableDataSources() throws Exception
   {
-    AuthenticationResult authenticationResult = new AuthenticationResult("druid", "druid");
+    // first request
-
-    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT))
-            .andReturn(authenticationResult)
-            .anyTimes();
     EasyMock.expect(server.getDataSources()).andReturn(
-        ImmutableList.of(listDataSources.get(0), listDataSources.get(1))
+      ImmutableList.of(listDataSources.get(0), listDataSources.get(1))
-    ).atLeastOnce();
+  ).once();
+
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
+        new AuthenticationResult("druid", "druid")
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
 
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
-    ).atLeastOnce();
+    ).once();
+
+    // second request
+    EasyMock.expect(server.getDataSources()).andReturn(
+        ImmutableList.of(listDataSources.get(0), listDataSources.get(1))
+    ).once();
+
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
+        new AuthenticationResult("druid", "druid")
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
+
+    EasyMock.expect(inventoryView.getInventory()).andReturn(
+        ImmutableList.of(server)
+    ).once();
     EasyMock.replay(inventoryView, server, request);
 
     AuthorizerMapper authMapper = new AuthorizerMapper(null) {
@@ -232,9 +270,12 @@ public class DatasourcesResourceTest
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
     ).atLeastOnce();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
         new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
 
     EasyMock.replay(inventoryView, server, request);
     DatasourcesResource datasourcesResource = new DatasourcesResource(

server/src/test/java/io/druid/server/http/IntervalsResourceTest.java

@@ -108,9 +108,12 @@ public class IntervalsResourceTest
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
     ).atLeastOnce();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
         new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
     EasyMock.replay(inventoryView, request);
 
     List<Interval> expectedIntervals = new ArrayList<>();
@@ -142,9 +145,12 @@ public class IntervalsResourceTest
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
     ).atLeastOnce();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
         new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
     EasyMock.replay(inventoryView, request);
 
     List<Interval> expectedIntervals = new ArrayList<>();
@@ -170,9 +176,12 @@ public class IntervalsResourceTest
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
     ).atLeastOnce();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
         new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
     EasyMock.replay(inventoryView, request);
 
     List<Interval> expectedIntervals = new ArrayList<>();
@@ -200,9 +209,12 @@ public class IntervalsResourceTest
     EasyMock.expect(inventoryView.getInventory()).andReturn(
         ImmutableList.of(server)
     ).atLeastOnce();
+    EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).once();
     EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(
         new AuthenticationResult("druid", "druid")
-    ).atLeastOnce();
+    ).once();
+    request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
+    EasyMock.expectLastCall().times(1);
     EasyMock.replay(inventoryView, request);
 
     IntervalsResource intervalsResource = new IntervalsResource(