From 47c2c36460534e6ae84a8d28bb2409cb06265098 Mon Sep 17 00:00:00 2001 From: John Niang Date: Fri, 9 Apr 2021 13:19:15 +0800 Subject: [PATCH] Add OptionFilter for bulk option api (#1345) * Add OptionFilter for bulk option api * Add another filter method for single option * Restrict OptionController response * Remove redundant api * feat: complete private option keys. * feat: complete private option keys. Co-authored-by: Ryan Wang --- .../content/api/OptionController.java | 56 +++++-- .../halo/app/model/support/BaseResponse.java | 2 +- .../run/halo/app/model/support/HaloConst.java | 4 +- .../run/halo/app/service/OptionService.java | 5 +- .../halo/app/service/impl/OptionFilter.java | 140 ++++++++++++++++++ .../app/service/impl/OptionServiceImpl.java | 3 +- .../app/service/impl/OptionFilterTest.java | 123 +++++++++++++++ 7 files changed, 316 insertions(+), 17 deletions(-) create mode 100644 src/main/java/run/halo/app/service/impl/OptionFilter.java create mode 100644 src/test/java/run/halo/app/service/impl/OptionFilterTest.java diff --git a/src/main/java/run/halo/app/controller/content/api/OptionController.java b/src/main/java/run/halo/app/controller/content/api/OptionController.java index 32c36a14ee..8c01f4f190 100644 --- a/src/main/java/run/halo/app/controller/content/api/OptionController.java +++ b/src/main/java/run/halo/app/controller/content/api/OptionController.java @@ -1,11 +1,15 @@ package run.halo.app.controller.content.api; +import static java.util.stream.Collectors.toMap; + import io.swagger.annotations.ApiOperation; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; -import org.springframework.http.HttpStatus; +import java.util.stream.Collectors; import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; import org.springframework.web.bind.annotation.GetMapping; import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.RequestMapping; @@ -15,6 +19,7 @@ import run.halo.app.model.properties.CommentProperties; import run.halo.app.model.support.BaseResponse; import run.halo.app.service.OptionService; +import run.halo.app.service.impl.OptionFilter; /** * Content option controller. @@ -28,42 +33,69 @@ public class OptionController { private final OptionService optionService; + private final OptionFilter optionFilter; + public OptionController(OptionService optionService) { this.optionService = optionService; + optionFilter = new OptionFilter(optionService); } @GetMapping("list_view") @ApiOperation("Lists all options with list view") public List listAll() { - return optionService.listDtos(); + var options = optionService.listDtos(); + var optionMap = options.stream() + .collect(toMap(OptionDTO::getKey, option -> option)); + var keys = options.stream() + .map(OptionDTO::getKey) + .collect(Collectors.toUnmodifiableSet()); + return optionFilter.filter(keys).stream() + .map(optionMap::get) + .collect(Collectors.toUnmodifiableList()); } @GetMapping("map_view") @ApiOperation("Lists options with map view") public Map listAllWithMapView( - @RequestParam(value = "key", required = false) List keys) { - if (CollectionUtils.isEmpty(keys)) { - return optionService.listOptions(); + @Deprecated(since = "1.4.8", forRemoval = true) + @RequestParam(value = "key", required = false) List keyList, + @RequestParam(value = "keys", required = false) String keys) { + // handle for key list + if (!CollectionUtils.isEmpty(keyList)) { + return optionService.listOptions(optionFilter.filter(keyList)); } - - return optionService.listOptions(keys); + // handle for keys + if (StringUtils.hasText(keys)) { + var nameSet = Arrays.stream(keys.split(",")) + .map(String::trim) + .collect(Collectors.toUnmodifiableSet()); + var filteredNames = optionFilter.filter(nameSet); + return optionService.listOptions(filteredNames); + } + // list all + Map options = optionService.listOptions(); + return optionFilter.filter(options.keySet()).stream() + .collect(toMap(optionName -> optionName, options::get)); } @GetMapping("keys/{key}") @ApiOperation("Gets option value by option key") public BaseResponse getBy(@PathVariable("key") String key) { - return BaseResponse - .ok(HttpStatus.OK.getReasonPhrase(), optionService.getByKey(key).orElse(null)); + Object optionValue = optionFilter.filter(key) + .map(k -> optionService.getByKey(key)) + .orElse(null); + return BaseResponse.ok(optionValue); } - @GetMapping("comment") - @ApiOperation("Options for comment") + @ApiOperation("Options for comment(@deprecated, use /bulk api instead of this.)") + @Deprecated public Map comment() { List keys = new ArrayList<>(); keys.add(CommentProperties.GRAVATAR_DEFAULT.getValue()); keys.add(CommentProperties.CONTENT_PLACEHOLDER.getValue()); keys.add(CommentProperties.GRAVATAR_SOURCE.getValue()); - return optionService.listOptions(keys); + return optionService.listOptions(optionFilter.filter(keys)); } + } diff --git a/src/main/java/run/halo/app/model/support/BaseResponse.java b/src/main/java/run/halo/app/model/support/BaseResponse.java index 155136b923..c451473a0a 100644 --- a/src/main/java/run/halo/app/model/support/BaseResponse.java +++ b/src/main/java/run/halo/app/model/support/BaseResponse.java @@ -77,7 +77,7 @@ public static BaseResponse ok(@Nullable String message) { * @param data type * @return base response with data */ - public static BaseResponse ok(@NonNull T data) { + public static BaseResponse ok(@Nullable T data) { return new BaseResponse<>(HttpStatus.OK.value(), HttpStatus.OK.getReasonPhrase(), data); } } diff --git a/src/main/java/run/halo/app/model/support/HaloConst.java b/src/main/java/run/halo/app/model/support/HaloConst.java index 1ce32d4837..be55ad7e0b 100644 --- a/src/main/java/run/halo/app/model/support/HaloConst.java +++ b/src/main/java/run/halo/app/model/support/HaloConst.java @@ -156,7 +156,9 @@ public class HaloConst { /** * Options cache key. */ - public static String OPTIONS_CACHE_KEY = "options"; + public static final String OPTIONS_CACHE_KEY = "options"; + + public static final String PRIVATE_OPTION_KEY = "private_options"; static { // Set version diff --git a/src/main/java/run/halo/app/service/OptionService.java b/src/main/java/run/halo/app/service/OptionService.java index 94bbc4c0b0..e099ff28af 100755 --- a/src/main/java/run/halo/app/service/OptionService.java +++ b/src/main/java/run/halo/app/service/OptionService.java @@ -2,6 +2,7 @@ import com.qiniu.common.Zone; import com.qiniu.storage.Region; +import java.util.Collection; import java.util.List; import java.util.Locale; import java.util.Map; @@ -106,7 +107,7 @@ public interface OptionService extends CrudService { * @return a map of option */ @NonNull - Map listOptions(@Nullable List keys); + Map listOptions(@Nullable Collection keys); /** * Lists all option dtos. @@ -224,7 +225,7 @@ T getByPropertyOrDefault(@NonNull PropertyEnum property, @NonNull Class p /** * Gets property value by blog property. - *

+ * * Default value from property default value. * * @param property blog property must not be null diff --git a/src/main/java/run/halo/app/service/impl/OptionFilter.java b/src/main/java/run/halo/app/service/impl/OptionFilter.java new file mode 100644 index 0000000000..cd7fde2980 --- /dev/null +++ b/src/main/java/run/halo/app/service/impl/OptionFilter.java @@ -0,0 +1,140 @@ +package run.halo.app.service.impl; + +import java.util.Collection; +import java.util.Collections; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; +import java.util.stream.Collectors; +import org.springframework.util.CollectionUtils; +import org.springframework.util.StringUtils; +import run.halo.app.model.properties.AliOssProperties; +import run.halo.app.model.properties.ApiProperties; +import run.halo.app.model.properties.BaiduBosProperties; +import run.halo.app.model.properties.EmailProperties; +import run.halo.app.model.properties.HuaweiObsProperties; +import run.halo.app.model.properties.MinioProperties; +import run.halo.app.model.properties.QiniuOssProperties; +import run.halo.app.model.properties.SmmsProperties; +import run.halo.app.model.properties.TencentCosProperties; +import run.halo.app.model.properties.UpOssProperties; +import run.halo.app.model.support.HaloConst; +import run.halo.app.service.OptionService; + +/** + * Option filter for private options. + * + * @author johnniang + * @date 2021-04-08 + */ +public class OptionFilter { + + private final Set defaultPrivateOptionKeys; + + private final OptionService optionService; + + public OptionFilter(OptionService optionService) { + this.optionService = optionService; + this.defaultPrivateOptionKeys = getDefaultPrivateOptionKeys(); + } + + private Set getDefaultPrivateOptionKeys() { + return Set.of( + AliOssProperties.OSS_DOMAIN.getValue(), + AliOssProperties.OSS_BUCKET_NAME.getValue(), + AliOssProperties.OSS_ACCESS_KEY.getValue(), + AliOssProperties.OSS_ACCESS_SECRET.getValue(), + + ApiProperties.API_ACCESS_KEY.getValue(), + + BaiduBosProperties.BOS_DOMAIN.getValue(), + BaiduBosProperties.BOS_ENDPOINT.getValue(), + BaiduBosProperties.BOS_BUCKET_NAME.getValue(), + BaiduBosProperties.BOS_ACCESS_KEY.getValue(), + BaiduBosProperties.BOS_SECRET_KEY.getValue(), + + EmailProperties.USERNAME.getValue(), + EmailProperties.PASSWORD.getValue(), + EmailProperties.FROM_NAME.getValue(), + + HuaweiObsProperties.OSS_DOMAIN.getValue(), + HuaweiObsProperties.OSS_ENDPOINT.getValue(), + HuaweiObsProperties.OSS_BUCKET_NAME.getValue(), + HuaweiObsProperties.OSS_ACCESS_KEY.getValue(), + HuaweiObsProperties.OSS_ACCESS_SECRET.getValue(), + + MinioProperties.ENDPOINT.getValue(), + MinioProperties.BUCKET_NAME.getValue(), + MinioProperties.ACCESS_KEY.getValue(), + MinioProperties.ACCESS_SECRET.getValue(), + + QiniuOssProperties.OSS_ZONE.getValue(), + QiniuOssProperties.OSS_ACCESS_KEY.getValue(), + QiniuOssProperties.OSS_SECRET_KEY.getValue(), + QiniuOssProperties.OSS_DOMAIN.getValue(), + QiniuOssProperties.OSS_BUCKET.getValue(), + + SmmsProperties.SMMS_API_SECRET_TOKEN.getValue(), + + TencentCosProperties.COS_DOMAIN.getValue(), + TencentCosProperties.COS_REGION.getValue(), + TencentCosProperties.COS_BUCKET_NAME.getValue(), + TencentCosProperties.COS_SECRET_ID.getValue(), + TencentCosProperties.COS_SECRET_KEY.getValue(), + + UpOssProperties.OSS_PASSWORD.getValue(), + UpOssProperties.OSS_BUCKET.getValue(), + UpOssProperties.OSS_DOMAIN.getValue(), + UpOssProperties.OSS_OPERATOR.getValue() + ); + } + + private Set getConfiguredPrivateOptionKeys() { + // resolve configured private option names + return optionService.getByKey(HaloConst.PRIVATE_OPTION_KEY, String.class) + .map(privateOptions -> privateOptions.split(",")) + .map(Set::of) + .orElse(Collections.emptySet()) + .stream() + .map(String::trim) + .collect(Collectors.toUnmodifiableSet()); + } + + /** + * Filter option keys to prevent outsider from accessing private options. + * + * @param optionKeys option key collection + * @return filtered option keys + */ + public Set filter(Collection optionKeys) { + if (CollectionUtils.isEmpty(optionKeys)) { + return Collections.emptySet(); + } + + return optionKeys.stream() + .filter(Objects::nonNull) + .filter(optionKey -> !optionKey.isBlank()) + .filter(optionKey -> !defaultPrivateOptionKeys.contains(optionKey)) + .filter(optionKey -> !getConfiguredPrivateOptionKeys().contains(optionKey)) + .collect(Collectors.toUnmodifiableSet()); + } + + /** + * Filter option key to prevent outsider from accessing private option. + * + * @param optionKey option key + * @return an optional of option key + */ + public Optional filter(String optionKey) { + if (!StringUtils.hasText(optionKey)) { + return Optional.empty(); + } + if (defaultPrivateOptionKeys.contains(optionKey)) { + return Optional.empty(); + } + if (getConfiguredPrivateOptionKeys().contains(optionKey)) { + return Optional.empty(); + } + return Optional.of(optionKey); + } +} diff --git a/src/main/java/run/halo/app/service/impl/OptionServiceImpl.java b/src/main/java/run/halo/app/service/impl/OptionServiceImpl.java index 829e9f8e5f..681549fe8a 100644 --- a/src/main/java/run/halo/app/service/impl/OptionServiceImpl.java +++ b/src/main/java/run/halo/app/service/impl/OptionServiceImpl.java @@ -2,6 +2,7 @@ import com.qiniu.common.Zone; import com.qiniu.storage.Region; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.LinkedHashMap; @@ -228,7 +229,7 @@ public Map listOptions() { } @Override - public Map listOptions(List keys) { + public Map listOptions(Collection keys) { if (CollectionUtils.isEmpty(keys)) { return Collections.emptyMap(); } diff --git a/src/test/java/run/halo/app/service/impl/OptionFilterTest.java b/src/test/java/run/halo/app/service/impl/OptionFilterTest.java new file mode 100644 index 0000000000..51896b2417 --- /dev/null +++ b/src/test/java/run/halo/app/service/impl/OptionFilterTest.java @@ -0,0 +1,123 @@ +package run.halo.app.service.impl; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.BDDMockito.given; + +import java.util.Optional; +import java.util.Set; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; +import run.halo.app.model.properties.AliOssProperties; +import run.halo.app.service.OptionService; + +class OptionFilterTest { + + @Mock + OptionService optionService; + + @InjectMocks + OptionFilter optionFilter; + + @BeforeEach + void setUp() { + MockitoAnnotations.openMocks(this); + } + + @Test + void shouldFilterForDefaultPrivateOptions() { + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.empty()); + + var optionNames = Set.of(AliOssProperties.OSS_ACCESS_SECRET.getValue()); + var filteredOptionNames = optionFilter.filter(optionNames); + assertTrue(filteredOptionNames.isEmpty()); + + var filteredOptionName = + optionFilter.filter(AliOssProperties.OSS_ACCESS_SECRET.getValue()); + assertTrue(filteredOptionName.isEmpty()); + } + + @Test + void shouldFilterForConfiguredPrivateOptions() { + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of("hello")); + + var optionNames = Set.of("hello", "world"); + var filteredOptionNames = optionFilter.filter(optionNames); + var filteredOptionName = optionFilter.filter("hello"); + assertEquals(Set.of("world"), filteredOptionNames); + assertTrue(filteredOptionName.isEmpty()); + + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of("hello,world")); + + optionNames = Set.of("hello"); + filteredOptionNames = optionFilter.filter(optionNames); + filteredOptionName = optionFilter.filter("hello"); + assertTrue(filteredOptionNames.isEmpty()); + assertTrue(filteredOptionName.isEmpty()); + + optionNames = Set.of("hello", "world"); + filteredOptionNames = optionFilter.filter(optionNames); + filteredOptionName = optionFilter.filter("hello"); + assertTrue(filteredOptionNames.isEmpty()); + assertTrue(filteredOptionName.isEmpty()); + } + + @Test + void shouldFilterForConfiguredPrivateOptionsWithSpace() { + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of(" hello ")); + + var optionNames = Set.of("hello", "world"); + var filteredOptionNames = optionFilter.filter(optionNames); + var filteredOptionName = optionFilter.filter("hello"); + assertEquals(Set.of("world"), filteredOptionNames); + assertTrue(filteredOptionName.isEmpty()); + + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of(" hello , world ")); + + optionNames = Set.of("hello"); + filteredOptionNames = optionFilter.filter(optionNames); + filteredOptionName = optionFilter.filter("hello"); + assertTrue(filteredOptionNames.isEmpty()); + assertTrue(filteredOptionName.isEmpty()); + + optionNames = Set.of("hello", "world"); + filteredOptionNames = optionFilter.filter(optionNames); + filteredOptionName = optionFilter.filter("world"); + assertTrue(filteredOptionNames.isEmpty()); + assertTrue(filteredOptionName.isEmpty()); + } + + @Test + void shouldFilterForBothOfThem() { + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of("hello,world")); + + var optionNames = + Set.of("hello", "world", "halo", AliOssProperties.OSS_ACCESS_SECRET.getValue()); + var filteredOptionNames = optionFilter.filter(optionNames); + assertEquals(Set.of("halo"), filteredOptionNames); + } + + @Test + void shouldFilterNothing() { + given(optionService.getByKey(any(), eq(String.class))) + .willReturn(Optional.of(",world")); + + var optionNames = + Set.of("hello", "halo"); + var filteredOptionNames = optionFilter.filter(optionNames); + var filteredOptionName = optionFilter.filter("halo"); + assertEquals(optionNames, filteredOptionNames); + assertEquals(Optional.of("halo"), filteredOptionName); + } +} \ No newline at end of file