fix potential permission issue

master336 · Aug 8, 2019 · 3ce5643 · 3ce5643
1 parent 161fa85
commit 3ce5643
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 3 deletions.
diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/api/AdminServiceAPI.java
@@ -164,6 +164,10 @@ public ItemDTO loadItem(Env env, String appId, String clusterName, String namesp
           ItemDTO.class, appId, clusterName, namespaceName, key);
     }
 
+    public ItemDTO loadItemById(Env env, long itemId) {
+      return restTemplate.get(env, "items/{itemId}", ItemDTO.class, itemId);
+    }
+
     public void updateItemsByChangeSet(String appId, Env env, String clusterName, String namespace,
         ItemChangeSets changeSets) {
       restTemplate.post(env, "apps/{appId}/clusters/{clusterName}/namespaces/{namespaceName}/itemset",

diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/controller/ItemController.java
@@ -2,6 +2,7 @@
 
 import com.ctrip.framework.apollo.common.dto.ItemChangeSets;
 import com.ctrip.framework.apollo.common.dto.ItemDTO;
+import com.ctrip.framework.apollo.common.dto.NamespaceDTO;
 import com.ctrip.framework.apollo.common.exception.BadRequestException;
 import com.ctrip.framework.apollo.core.enums.ConfigFileFormat;
 import com.ctrip.framework.apollo.core.enums.Env;
@@ -12,6 +13,7 @@
 import com.ctrip.framework.apollo.portal.entity.vo.ItemDiffs;
 import com.ctrip.framework.apollo.portal.entity.vo.NamespaceIdentifier;
 import com.ctrip.framework.apollo.portal.service.ItemService;
+import com.ctrip.framework.apollo.portal.service.NamespaceService;
 import com.ctrip.framework.apollo.portal.spi.UserInfoHolder;
 import org.springframework.beans.factory.config.YamlPropertiesFactoryBean;
 import org.springframework.core.io.ByteArrayResource;
@@ -38,13 +40,16 @@
 public class ItemController {
 
   private final ItemService configService;
+  private final NamespaceService namespaceService;
   private final UserInfoHolder userInfoHolder;
   private final PermissionValidator permissionValidator;
 
-  public ItemController(final ItemService configService, final UserInfoHolder userInfoHolder, final PermissionValidator permissionValidator) {
+  public ItemController(final ItemService configService, final UserInfoHolder userInfoHolder,
+                        final PermissionValidator permissionValidator, final NamespaceService namespaceService) {
     this.configService = configService;
     this.userInfoHolder = userInfoHolder;
     this.permissionValidator = permissionValidator;
+    this.namespaceService = namespaceService;
   }
 
   @PreAuthorize(value = "@permissionValidator.hasModifyNamespacePermission(#appId, #namespaceName, #env)")
@@ -99,9 +104,14 @@ public void updateItem(@PathVariable String appId, @PathVariable String env,
   public void deleteItem(@PathVariable String appId, @PathVariable String env,
                          @PathVariable String clusterName, @PathVariable String namespaceName,
                          @PathVariable long itemId) {
-    if (itemId <= 0) {
-      throw new BadRequestException("item id invalid");
+    ItemDTO item = configService.loadItemById(Env.fromString(env), itemId);
+    NamespaceDTO namespace = namespaceService.loadNamespaceBaseInfo(appId, Env.fromString(env), clusterName, namespaceName);
+
+    // In case someone constructs an attack scenario
+    if (item.getNamespaceId() != namespace.getId()) {
+      throw new BadRequestException("Invalid request, item and namespace do not match!");
     }
+
     configService.deleteItem(Env.valueOf(env), itemId, userInfoHolder.getUser().getUserId());
   }
 

diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/FavoriteService.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/FavoriteService.java
@@ -67,6 +67,14 @@ public List<Favorite> search(String userId, String appId, Pageable page) {
       throw new BadRequestException("user id and app id can't be empty at the same time");
     }
 
+    if (!isUserIdEmpty) {
+      UserInfo loginUser = userInfoHolder.getUser();
+      //user can only search his own favorite app
+      if (!Objects.equals(loginUser.getUserId(), userId)) {
+        userId = loginUser.getUserId();
+      }
+    }
+
     //search by userId
     if (isAppIdEmpty && !isUserIdEmpty) {
       return favoriteRepository.findByUserIdOrderByPositionAscDataChangeCreatedTimeAsc(userId, page);

diff --git a/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java b/apollo-portal/src/main/java/com/ctrip/framework/apollo/portal/service/ItemService.java
@@ -114,6 +114,14 @@ public ItemDTO loadItem(Env env, String appId, String clusterName, String namesp
     return itemAPI.loadItem(env, appId, clusterName, namespaceName, key);
   }
 
+  public ItemDTO loadItemById(Env env, long itemId) {
+    ItemDTO item = itemAPI.loadItemById(env, itemId);
+    if (item == null) {
+      throw new BadRequestException("item not found for itemId " + itemId);
+    }
+    return item;
+  }
+
   public void syncItems(List<NamespaceIdentifier> comparedNamespaces, List<ItemDTO> sourceItems) {
     List<ItemDiffs> itemDiffs = compare(comparedNamespaces, sourceItems);
     for (ItemDiffs itemDiff : itemDiffs) {