Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix potential permission issue #2496

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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)")
Expand Down Expand Up @@ -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());
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down