[ambari] branch trunk updated: AMBARI-24224. Do not filter out /clusters/*/widgets*; we do check authorization later on in the resource provider based on user name (#1650)

[sm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

smolnar pushed a commit to branch trunk
in repository https://gitbox.apache.org/repos/asf/ambari.git


The following commit(s) were added to refs/heads/trunk by this push:
     new 83f7227  AMBARI-24224. Do not filter out /clusters/*/widgets*; we do check authorization later on in the resource provider based on user name (#1650)
83f7227 is described below

commit 83f7227d54654f6b8f43103ff4e6fb150cd3fc31
Author: Sandor Molnar <sm...@apache.org>
AuthorDate: Fri Jun 29 21:21:05 2018 +0200

    AMBARI-24224. Do not filter out /clusters/*/widgets*; we do check authorization later on in the resource provider based on user name (#1650)
---
 .../internal/ActiveWidgetLayoutResourceProvider.java        | 13 +++++++++++--
 .../security/authorization/AmbariAuthorizationFilter.java   |  2 ++
 .../internal/ActiveWidgetLayoutResourceProviderTest.java    | 13 ++++++++++++-
 .../authorization/AmbariAuthorizationFilterTest.java        |  5 ++++-
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java
index ab18ed4..628c2eb 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProvider.java
@@ -51,6 +51,7 @@ import org.apache.ambari.server.security.authorization.AuthorizationException;
 import org.apache.ambari.server.security.authorization.AuthorizationHelper;
 import org.apache.ambari.server.security.authorization.ResourceType;
 import org.apache.ambari.server.security.authorization.RoleAuthorization;
+import org.apache.commons.lang3.StringUtils;
 
 import com.google.gson.Gson;
 import com.google.gson.reflect.TypeToken;
@@ -152,7 +153,7 @@ public class ActiveWidgetLayoutResourceProvider extends AbstractControllerResour
         RoleAuthorization.AMBARI_MANAGE_USERS);
 
     for (Map<String, Object> propertyMap: propertyMaps) {
-      final String userName = propertyMap.get(WIDGETLAYOUT_USERNAME_PROPERTY_ID).toString();
+      final String userName = getUserName(propertyMap);
 
       // Ensure that the authenticated user has authorization to get this information
       if (!isUserAdministrator && !AuthorizationHelper.getAuthenticatedName().equalsIgnoreCase(userName)) {
@@ -223,7 +224,7 @@ public class ActiveWidgetLayoutResourceProvider extends AbstractControllerResour
 
         for (Map<String, Object> propertyMap : propertyMaps) {
           // Ensure that the authenticated user has authorization to get this information
-          String userName = propertyMap.get(WIDGETLAYOUT_USERNAME_PROPERTY_ID).toString();
+          final String userName = getUserName(propertyMap);
           if (!isUserAdministrator && !AuthorizationHelper.getAuthenticatedName().equalsIgnoreCase(userName)) {
             throw new AuthorizationException();
           }
@@ -252,6 +253,14 @@ public class ActiveWidgetLayoutResourceProvider extends AbstractControllerResour
     return getRequestStatus(null);
   }
 
+  private String getUserName(Map<String, Object> propertyMap) {
+    String userName = propertyMap.get(WIDGETLAYOUT_USERNAME_PROPERTY_ID) == null ? "" : propertyMap.get(WIDGETLAYOUT_USERNAME_PROPERTY_ID).toString();
+    if (StringUtils.isBlank(userName)) {
+      userName = AuthorizationHelper.getAuthenticatedName();
+    }
+    return userName;
+  }
+
   @Override
   public RequestStatus deleteResources(Request request, Predicate predicate)
       throws SystemException, UnsupportedPropertyException, NoSuchResourceException, NoSuchParentResourceException {
diff --git a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
index 8c84363..27eee87 100644
--- a/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
+++ b/ambari-server/src/main/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilter.java
@@ -72,6 +72,7 @@ public class AmbariAuthorizationFilter implements Filter {
   private static final String API_GROUPS_ALL_PATTERN = API_VERSION_PREFIX + "/groups.*";
   private static final String API_CLUSTERS_PATTERN = API_VERSION_PREFIX + "/clusters/(\\w+/?)?";
   private static final String API_WIDGET_LAYOUTS_PATTERN = API_VERSION_PREFIX + "/clusters/.*?/widget_layouts.*?";
+  private static final String API_WIDGET_PATTERN = API_VERSION_PREFIX + "/clusters/.*?/widgets.*";
   private static final String API_CLUSTERS_ALL_PATTERN = API_VERSION_PREFIX + "/clusters.*";
   private static final String API_VIEWS_ALL_PATTERN = API_VERSION_PREFIX + "/views.*";
   private static final String API_PERSIST_ALL_PATTERN = API_VERSION_PREFIX + "/persist.*";
@@ -342,6 +343,7 @@ public class AmbariAuthorizationFilter implements Filter {
         requestURI.matches(API_VIEWS_ALL_PATTERN) ||
         requestURI.matches(VIEWS_CONTEXT_PATH_PATTERN) ||
         requestURI.matches(API_WIDGET_LAYOUTS_PATTERN) ||
+        requestURI.matches(API_WIDGET_PATTERN) ||
         requestURI.matches(API_CLUSTER_HOSTS_ALL_PATTERN) ||
         requestURI.matches(API_CLUSTER_CONFIGURATIONS_ALL_PATTERN) ||
         requestURI.matches(API_CLUSTER_COMPONENTS_ALL_PATTERN) ||
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java
index 8bf8227..54e7d3d 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/controller/internal/ActiveWidgetLayoutResourceProviderTest.java
@@ -149,6 +149,11 @@ public class ActiveWidgetLayoutResourceProviderTest extends EasyMockSupport {
     updateResourcesTest(TestAuthenticationFactory.createClusterAdministrator("User1", 2L), "User1");
   }
 
+  @Test
+  public void testUpdateResources_NoUserName_Self() throws Exception {
+    updateResourcesTest(TestAuthenticationFactory.createClusterAdministrator("User1", 2L), "User1", false);
+  }
+
   @Test(expected = AuthorizationException.class)
   public void testUpdateResources_NonAdministrator_Other() throws Exception {
     updateResourcesTest(TestAuthenticationFactory.createClusterAdministrator("User1", 2L), "User10");
@@ -255,6 +260,10 @@ public class ActiveWidgetLayoutResourceProviderTest extends EasyMockSupport {
   }
 
   private void updateResourcesTest(Authentication authentication, String requestedUsername) throws Exception {
+    updateResourcesTest(authentication, requestedUsername, true);
+  }
+
+  private void updateResourcesTest(Authentication authentication, String requestedUsername, boolean setUserName) throws Exception {
     Injector injector = createInjector();
 
     Capture<? extends String> widgetLayoutJsonCapture = newCapture();
@@ -293,7 +302,9 @@ public class ActiveWidgetLayoutResourceProviderTest extends EasyMockSupport {
 
     HashMap<String, Object> requestProps = new HashMap<>();
     requestProps.put(ActiveWidgetLayoutResourceProvider.WIDGETLAYOUT, widgetLayouts);
-    requestProps.put(ActiveWidgetLayoutResourceProvider.WIDGETLAYOUT_USERNAME_PROPERTY_ID, requestedUsername);
+    if (setUserName) {
+      requestProps.put(ActiveWidgetLayoutResourceProvider.WIDGETLAYOUT_USERNAME_PROPERTY_ID, requestedUsername);
+    }
 
     Request request = PropertyHelper.getUpdateRequest(requestProps, null);
 
diff --git a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java
index 112bea6..b5f9dfe 100644
--- a/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java
+++ b/ambari-server/src/test/java/org/apache/ambari/server/security/authorization/AmbariAuthorizationFilterTest.java
@@ -187,10 +187,13 @@ public class AmbariAuthorizationFilterTest {
     urlTests.put("/api/v1/users/user2", "POST", true);
     urlTests.put("/api/v1/groups", "GET", true);
     urlTests.put("/api/v1/ldap_sync_events", "GET", false);
+    urlTests.put("/api/v1/clusters/c1/widgets", "GET", true);
+    urlTests.put("/api/v1/clusters/c1/widgets", "PUT", true);
+    urlTests.put("/api/v1/clusters/c1/widgets", "POST", true);
     urlTests.put("/any/other/URL", "GET", true);
     urlTests.put("/any/other/URL", "POST", false);
 
-    performGeneralDoFilterTest(TestAuthenticationFactory.createClusterAdministrator(), urlTests, false);
+    performGeneralDoFilterTest(TestAuthenticationFactory.createClusterOperator(), urlTests, false);
   }
 
   @Test