From b63f1c0e459d6751803c2c779edc6f8ccfeb0441 Mon Sep 17 00:00:00 2001 From: Jonathan Wei Date: Fri, 2 Mar 2018 14:03:07 -0800 Subject: [PATCH] Fix authorization check in supervisor history API (#5460) --- .../supervisor/SupervisorResource.java | 2 +- .../supervisor/SupervisorResourceTest.java | 89 ++++++++++++++++++- 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/supervisor/SupervisorResource.java b/indexing-service/src/main/java/io/druid/indexing/overlord/supervisor/SupervisorResource.java index c23e028e6f1..b7cefa3fcd8 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/supervisor/SupervisorResource.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/supervisor/SupervisorResource.java @@ -235,7 +235,7 @@ public class SupervisorResource } } ); - return Response.ok(supervisorHistory).build(); + return Response.ok(authorizedSupervisorHistory).build(); } } ); diff --git a/indexing-service/src/test/java/io/druid/indexing/overlord/supervisor/SupervisorResourceTest.java b/indexing-service/src/test/java/io/druid/indexing/overlord/supervisor/SupervisorResourceTest.java index 2c34ef83a28..961f1e91069 100644 --- a/indexing-service/src/test/java/io/druid/indexing/overlord/supervisor/SupervisorResourceTest.java +++ b/indexing-service/src/test/java/io/druid/indexing/overlord/supervisor/SupervisorResourceTest.java @@ -28,9 +28,13 @@ import com.google.common.collect.Maps; import io.druid.indexing.overlord.DataSourceMetadata; import io.druid.indexing.overlord.TaskMaster; import io.druid.java.util.common.DateTimes; +import io.druid.server.security.Access; +import io.druid.server.security.Action; import io.druid.server.security.AuthConfig; -import io.druid.server.security.AuthTestUtils; 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 org.easymock.Capture; import org.easymock.EasyMock; import org.easymock.EasyMockRunner; @@ -64,7 +68,34 @@ public class SupervisorResourceTest extends EasyMockSupport @Before public void setUp() throws Exception { - supervisorResource = new SupervisorResource(taskMaster, new AuthConfig(), AuthTestUtils.TEST_AUTHORIZER_MAPPER); + supervisorResource = new SupervisorResource( + taskMaster, + new AuthConfig(), + new AuthorizerMapper(null) { + @Override + public Authorizer getAuthorizer(String name) + { + return new Authorizer() + { + @Override + public Access authorize( + AuthenticationResult authenticationResult, Resource resource, Action action + ) + { + if (authenticationResult.getIdentity().equals("druid")) { + return Access.OK; + } else { + if (resource.getName().equals("datasource2")) { + return new Access(false, "not authorized."); + } else { + return Access.OK; + } + } + } + }; + } + } + ); } @Test @@ -303,6 +334,60 @@ public class SupervisorResourceTest extends EasyMockSupport Assert.assertEquals(503, response.getStatus()); } + @Test + public void testSpecGetAllHistoryWithAuthFailureFiltering() throws Exception + { + Map> history = Maps.newHashMap(); + history.put("id1", null); + history.put("id2", null); + + EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.of(supervisorManager)).times(2); + EasyMock.expect(supervisorManager.getSupervisorHistory()).andReturn(history); + SupervisorSpec spec1 = new TestSupervisorSpec("id1", null) { + + @Override + public List getDataSources() + { + return Lists.newArrayList("datasource1"); + } + }; + SupervisorSpec spec2 = new TestSupervisorSpec("id2", null) { + + @Override + public List getDataSources() + { + return Lists.newArrayList("datasource2"); + } + }; + EasyMock.expect(supervisorManager.getSupervisorSpec("id1")).andReturn(Optional.of(spec1)).atLeastOnce(); + EasyMock.expect(supervisorManager.getSupervisorSpec("id2")).andReturn(Optional.of(spec2)).atLeastOnce(); + EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED)).andReturn(null).atLeastOnce(); + EasyMock.expect(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn( + new AuthenticationResult("wronguser", "druid", null) + ).atLeastOnce(); + request.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); + EasyMock.expectLastCall().anyTimes(); + replayAll(); + + Response response = supervisorResource.specGetAllHistory(request); + + Map> filteredHistory = Maps.newHashMap(); + filteredHistory.put("id1", null); + + Assert.assertEquals(200, response.getStatus()); + Assert.assertEquals(filteredHistory, response.getEntity()); + + resetAll(); + + EasyMock.expect(taskMaster.getSupervisorManager()).andReturn(Optional.absent()); + replayAll(); + + response = supervisorResource.specGetAllHistory(request); + verifyAll(); + + Assert.assertEquals(503, response.getStatus()); + } + @Test public void testSpecGetHistory() throws Exception {