[AMBARI-24562] Protect the ClusterConfig resource so that only author…

…ized users may have read-only access the data
apache · Aug 30, 2018 · 26dc1b3 · 26dc1b3
1 parent 4b41b0e
commit 26dc1b3
Show file tree

Hide file tree

Showing 4 changed files with 241 additions and 142 deletions.
diff --git a/.../main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java b/.../main/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProvider.java
@@ -33,6 +33,7 @@
 import java.io.PrintWriter;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.EnumSet;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedList;
@@ -64,6 +65,7 @@
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.spi.UnsupportedPropertyException;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
+import org.apache.ambari.server.security.authorization.RoleAuthorization;
 import org.apache.ambari.server.state.ClientConfigFileDefinition;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
@@ -144,6 +146,8 @@ public class ClientConfigResourceProvider extends AbstractControllerResourceProv
   ClientConfigResourceProvider(@Assisted AmbariManagementController managementController) {
     super(Resource.Type.ClientConfig, propertyIds, keyPropertyIds, managementController);
     gson = new Gson();
+
+    setRequiredGetAuthorizations(EnumSet.of(RoleAuthorization.HOST_VIEW_CONFIGS, RoleAuthorization.SERVICE_VIEW_CONFIGS, RoleAuthorization.CLUSTER_VIEW_CONFIGS));
   }
 
   // ----- ResourceProvider ------------------------------------------------
@@ -159,7 +163,7 @@ public RequestStatus createResources(Request request)
   }
 
   @Override
-  public Set<Resource> getResources(Request request, Predicate predicate)
+  public Set<Resource> getResourcesAuthorized(Request request, Predicate predicate)
           throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
 
     Set<Resource> resources = new HashSet<>();

diff --git a/...st/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java b/...st/java/org/apache/ambari/server/controller/internal/AlertTargetResourceProviderTest.java
@@ -270,7 +270,7 @@ public void testCreateResourcesAsAdministrator() throws Exception {
     testCreateResources(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testCreateResourcesAsClusterAdministrator() throws Exception {
     testCreateResources(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -331,7 +331,7 @@ public void testCreateResourcesWithGroupsAsAdministrator() throws Exception {
     testCreateResourcesWithGroups(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testCreateResourcesWithGroupsAsClusterAdministrator() throws Exception {
     testCreateResourcesWithGroups(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -407,7 +407,7 @@ public void testCreateGlobalTargetAsAdministrator() throws Exception {
     testCreateGlobalTarget(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testCreateGlobalTargetAsClusterAdministrator() throws Exception {
     testCreateGlobalTarget(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -473,7 +473,7 @@ public void testCreateResourceWithRecipientArrayAsAdministrator() throws Excepti
     testCreateResourceWithRecipientArray(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testCreateResourceWithRecipientArrayAsClusterAdministrator() throws Exception {
     testCreateResourceWithRecipientArray(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -537,7 +537,7 @@ public void testCreateResourceWithAlertStatesAsAdministrator() throws Exception
     testCreateResourceWithAlertStates(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testCreateResourceWithAlertStatesAsClusterAdministrator() throws Exception {
     testCreateResourceWithAlertStates(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -604,7 +604,7 @@ public void testUpdateResourcesAsAdministrator() throws Exception {
     testUpdateResources(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testUpdateResourcesAsClusterAdministrator() throws Exception {
     testUpdateResources(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -679,7 +679,7 @@ public void testUpdateResourcesWithGroupsAsAdministrator() throws Exception {
     testUpdateResourcesWithGroups(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testUpdateResourcesWithGroupsAsClusterAdministrator() throws Exception {
     testUpdateResourcesWithGroups(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -761,7 +761,7 @@ public void testDeleteResourcesAsAdministrator() throws Exception {
     testDeleteResources(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testDeleteResourcesAsClusterAdministrator() throws Exception {
     testDeleteResources(TestAuthenticationFactory.createClusterAdministrator());
   }
@@ -829,7 +829,7 @@ public void testOverwriteDirectiveAsAdministrator() throws Exception {
     testOverwriteDirective(TestAuthenticationFactory.createAdministrator());
   }
 
-  @Test(expected = AuthorizationException.class)
+  @Test
   public void testOverwriteDirectiveAsClusterAdministrator() throws Exception {
     testOverwriteDirective(TestAuthenticationFactory.createClusterAdministrator());
   }

diff --git a/...t/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java b/...t/java/org/apache/ambari/server/controller/internal/ClientConfigResourceProviderTest.java
@@ -56,6 +56,8 @@
 import org.apache.ambari.server.controller.spi.SystemException;
 import org.apache.ambari.server.controller.utilities.PredicateBuilder;
 import org.apache.ambari.server.controller.utilities.PropertyHelper;
+import org.apache.ambari.server.security.TestAuthenticationFactory;
+import org.apache.ambari.server.security.authorization.AuthorizationException;
 import org.apache.ambari.server.stack.StackManager;
 import org.apache.ambari.server.state.ClientConfigFileDefinition;
 import org.apache.ambari.server.state.Cluster;
@@ -76,20 +78,28 @@
 import org.apache.ambari.server.state.ValueAttributesInfo;
 import org.apache.ambari.server.utils.StageUtils;
 import org.easymock.EasyMock;
+import org.junit.After;
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.powermock.api.easymock.PowerMock;
 import org.powermock.api.mockito.PowerMockito;
 import org.powermock.core.classloader.annotations.PrepareForTest;
 import org.powermock.modules.junit4.PowerMockRunner;
+import org.springframework.security.core.Authentication;
+import org.springframework.security.core.context.SecurityContextHolder;
 
 /**
  * ClientConfigResourceProviderTest tests.
  */
 @RunWith(PowerMockRunner.class)
 @PrepareForTest({ClientConfigResourceProvider.class, StageUtils.class})
 public class ClientConfigResourceProviderTest {
+  @After
+  public void clearAuthentication() {
+    SecurityContextHolder.getContext().setAuthentication(null);
+  }
+
   @Test
   public void testCreateResources() throws Exception {
     Resource.Type type = Resource.Type.ClientConfig;
@@ -165,7 +175,102 @@ public void testUpdateResources() throws Exception {
   }
 
   @Test
-  public void testGetResources() throws Exception {
+  public void testGetResourcesForAdministrator() throws Exception {
+    testGetResources(TestAuthenticationFactory.createAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesForClusterAdministrator() throws Exception {
+    testGetResources(TestAuthenticationFactory.createClusterAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesForClusterOperator() throws Exception {
+    testGetResources(TestAuthenticationFactory.createClusterOperator());
+  }
+
+  @Test
+  public void testGetResourcesForServiceAdministrator() throws Exception {
+    testGetResources(TestAuthenticationFactory.createServiceAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesForServiceOperator() throws Exception {
+    testGetResources(TestAuthenticationFactory.createServiceOperator());
+  }
+
+  @Test
+  public void testGetResourcesForClusterUser() throws Exception {
+    testGetResources(TestAuthenticationFactory.createClusterUser());
+  }
+
+  @Test(expected = AuthorizationException.class)
+  public void testGetResourcesForNoRoleUser() throws Exception {
+    testGetResources(TestAuthenticationFactory.createNoRoleUser());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForAdministrator() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForClusterAdministrator() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createClusterAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForClusterOperator() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createClusterOperator());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForServiceAdministrator() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createServiceAdministrator());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForServiceOperator() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createServiceOperator());
+  }
+
+  @Test
+  public void testGetResourcesFromCommonServicesForClusterUser() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createClusterUser());
+  }
+
+  @Test(expected = AuthorizationException.class)
+  public void testGetResourcesFromCommonServicesForNoRoleUser() throws Exception {
+    testGetResourcesFromCommonServices(TestAuthenticationFactory.createNoRoleUser());
+  }
+
+  @Test
+  public void testDeleteResources() throws Exception {
+    Resource.Type type = Resource.Type.ClientConfig;
+
+    AmbariManagementController managementController = createMock(AmbariManagementController.class);
+
+    // replay
+    replay(managementController);
+
+    ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider(
+        type,
+        managementController);
+
+    Predicate predicate = new PredicateBuilder().property(
+        ClientConfigResourceProvider.COMPONENT_COMPONENT_NAME_PROPERTY_ID).equals("HDFS_CLIENT").toPredicate();
+    try {
+      provider.deleteResources(new RequestImpl(null, null, null, null), predicate);
+      Assert.fail("Expected an UnsupportedOperationException");
+    } catch (SystemException e) {
+      // expected
+    }
+
+    // verify
+    verify(managementController);
+  }
+
+  private void testGetResources(Authentication authentication) throws Exception {
     Resource.Type type = Resource.Type.ClientConfig;
 
     AmbariManagementController managementController = createNiceMock(AmbariManagementController.class);
@@ -370,6 +475,8 @@ public void testGetResources() throws Exception {
         runtime, process);
     PowerMock.replayAll();
 
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
     Set<Resource> resources = provider.getResources(request, predicate);
     assertFalse(resources.isEmpty());
     assertFalse(newFile.exists());
@@ -381,8 +488,7 @@ public void testGetResources() throws Exception {
     PowerMock.verifyAll();
   }
 
-  @Test
-  public void testGetResourcesFromCommonServices() throws Exception {
+  private void testGetResourcesFromCommonServices(Authentication authentication) throws Exception {
     Resource.Type type = Resource.Type.ClientConfig;
 
     AmbariManagementController managementController = createNiceMock(AmbariManagementController.class);
@@ -553,6 +659,8 @@ public void testGetResourcesFromCommonServices() throws Exception {
         runtime, process);
     PowerMock.replayAll();
 
+    SecurityContextHolder.getContext().setAuthentication(authentication);
+
     Set<Resource> resources = provider.getResources(request, predicate);
     assertFalse(resources.isEmpty());
 
@@ -564,30 +672,4 @@ public void testGetResourcesFromCommonServices() throws Exception {
   }
 
 
-  @Test
-  public void testDeleteResources() throws Exception {
-    Resource.Type type = Resource.Type.ClientConfig;
-
-    AmbariManagementController managementController = createMock(AmbariManagementController.class);
-
-    // replay
-    replay(managementController);
-
-    ResourceProvider provider = AbstractControllerResourceProvider.getResourceProvider(
-        type,
-        managementController);
-
-    Predicate predicate = new PredicateBuilder().property(
-        ClientConfigResourceProvider.COMPONENT_COMPONENT_NAME_PROPERTY_ID).equals("HDFS_CLIENT").toPredicate();
-    try {
-      provider.deleteResources(new RequestImpl(null, null, null, null), predicate);
-      Assert.fail("Expected an UnsupportedOperationException");
-    } catch (SystemException e) {
-      // expected
-    }
-
-    // verify
-    verify(managementController);
-  }
-
 }