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 <i@ryanc.cc>

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.dto.OptionDTO;
 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<OptionDTO> 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<String, Object> listAllWithMapView(
-        @RequestParam(value = "key", required = false) List<String> keys) {
+        @Deprecated(since = "1.4.8", forRemoval = true)
-        if (CollectionUtils.isEmpty(keys)) {
+        @RequestParam(value = "key", required = false) List<String> keyList,
-            return optionService.listOptions();
+        @RequestParam(value = "keys", required = false) String keys) {
+        // handle for key list
+        if (!CollectionUtils.isEmpty(keyList)) {
+            return optionService.listOptions(optionFilter.filter(keyList));
         }
-
+        // handle for keys
-        return optionService.listOptions(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<String, Object> 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<Object> getBy(@PathVariable("key") String key) {
-        return BaseResponse
+        Object optionValue = optionFilter.filter(key)
-            .ok(HttpStatus.OK.getReasonPhrase(), optionService.getByKey(key).orElse(null));
+            .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<String, Object> comment() {
         List<String> 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));
     }
+
 }

src/main/java/run/halo/app/model/support/BaseResponse.java

@@ -77,7 +77,7 @@ public class BaseResponse<T> {
      * @param <T> data type
      * @return base response with data
      */
-    public static <T> BaseResponse<T> ok(@NonNull T data) {
+    public static <T> BaseResponse<T> ok(@Nullable T data) {
         return new BaseResponse<>(HttpStatus.OK.value(), HttpStatus.OK.getReasonPhrase(), data);
     }
 }

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

src/main/java/run/halo/app/service/OptionService.java

@@ -2,6 +2,7 @@ package run.halo.app.service;
 
 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<Option, Integer> {
      * @return a map of option
      */
     @NonNull
-    Map<String, Object> listOptions(@Nullable List<String> keys);
+    Map<String, Object> listOptions(@Nullable Collection<String> keys);
 
     /**
      * Lists all option dtos.
@@ -224,7 +225,7 @@ public interface OptionService extends CrudService<Option, Integer> {
 
     /**
      * Gets property value by blog property.
-     * <p>
+     *
      * Default value from property default value.
      *
      * @param property blog property must not be null

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<String> defaultPrivateOptionKeys;
+
+    private final OptionService optionService;
+
+    public OptionFilter(OptionService optionService) {
+        this.optionService = optionService;
+        this.defaultPrivateOptionKeys = getDefaultPrivateOptionKeys();
+    }
+
+    private Set<String> 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<String> 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<String> filter(Collection<String> 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<String> 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);
+    }
+}

src/main/java/run/halo/app/service/impl/OptionServiceImpl.java

@@ -2,6 +2,7 @@ package run.halo.app.service.impl;
 
 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 class OptionServiceImpl extends AbstractCrudService<Option, Integer>
     }
 
     @Override
-    public Map<String, Object> listOptions(List<String> keys) {
+    public Map<String, Object> listOptions(Collection<String> keys) {
         if (CollectionUtils.isEmpty(keys)) {
             return Collections.emptyMap();
         }

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);
+    }
+}