From b6222f48a4f2efdf4c24027ce33f12585844f874 Mon Sep 17 00:00:00 2001 From: John Niang Date: Tue, 27 Aug 2024 15:09:18 +0800 Subject: [PATCH] Fix the incorrect list options builder while listing aggregated roles (#6530) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind bug /area core /milestone 2.19.0 #### What this PR does / why we need it: This PR corrects list options builder for listing aggregated roles, because I wrongly used the label selector in . #### Special notes for your reviewer: 1. Try to install the plugin 2. Enable the plugin and enable setting `匿名评论需要验证码` 3. **Anonymous** request any of posts with comment enabled 4. Check the captcha in comment area #### Does this PR introduce a user-facing change? ```release-note 修复可能无法正常访问插件提供的接口的问题 ``` --- .../extension/service/DefaultRoleService.java | 13 ++-- .../run/halo/app/infra/SchemeInitializer.java | 19 +++++- .../authorization/AuthorizationTest.java | 61 +++++++++++++++---- 3 files changed, 71 insertions(+), 22 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java b/application/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java index 6351643c3..11b042aae 100644 --- a/application/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java +++ b/application/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java @@ -190,15 +190,12 @@ public class DefaultRoleService implements RoleService { if (CollectionUtils.isEmpty(roleNames)) { return Flux.empty(); } - var listOptionsBuilder = Optional.ofNullable(additionalListOptions) + var listOptions = Optional.ofNullable(additionalListOptions) .map(ListOptions::builder) - .orElseGet(ListOptions::builder); - roleNames.stream() - .map(roleName -> Role.ROLE_AGGREGATE_LABEL_PREFIX + roleName) - .forEach( - label -> listOptionsBuilder.labelSelector().eq(label, Boolean.TRUE.toString()) - ); - return client.listAll(Role.class, listOptionsBuilder.build(), ExtensionUtil.defaultSort()); + .orElseGet(ListOptions::builder) + .andQuery(QueryFactory.in("labels.aggregateToRoles", roleNames)) + .build(); + return client.listAll(Role.class, listOptions, ExtensionUtil.defaultSort()); } Predicate getRoleBindingPredicate(Subject targetSubject) { diff --git a/application/src/main/java/run/halo/app/infra/SchemeInitializer.java b/application/src/main/java/run/halo/app/infra/SchemeInitializer.java index 0825ce447..80292bf2e 100644 --- a/application/src/main/java/run/halo/app/infra/SchemeInitializer.java +++ b/application/src/main/java/run/halo/app/infra/SchemeInitializer.java @@ -3,6 +3,7 @@ package run.halo.app.infra; import static org.apache.commons.lang3.BooleanUtils.isTrue; import static org.apache.commons.lang3.BooleanUtils.toStringTrueFalse; import static org.apache.commons.lang3.ObjectUtils.defaultIfNull; +import static run.halo.app.core.extension.Role.ROLE_AGGREGATE_LABEL_PREFIX; import static run.halo.app.extension.index.IndexAttributeFactory.multiValueAttribute; import static run.halo.app.extension.index.IndexAttributeFactory.simpleAttribute; @@ -75,7 +76,23 @@ public class SchemeInitializer implements ApplicationListener { + is.add(new IndexSpec() + .setName("labels.aggregateToRoles") + .setIndexFunc(multiValueAttribute(Role.class, + role -> Optional.ofNullable(role.getMetadata().getLabels()) + .map(labels -> labels.keySet() + .stream() + .filter(key -> key.startsWith(ROLE_AGGREGATE_LABEL_PREFIX)) + .filter(key -> Boolean.parseBoolean(labels.get(key))) + .map( + key -> StringUtils.removeStart(key, ROLE_AGGREGATE_LABEL_PREFIX) + ) + .collect(Collectors.toSet()) + ) + .orElseGet(Set::of))) + ); + }); // plugin.halo.run schemeManager.register(Plugin.class); diff --git a/application/src/test/java/run/halo/app/security/authorization/AuthorizationTest.java b/application/src/test/java/run/halo/app/security/authorization/AuthorizationTest.java index 9ae0ef24d..23d2e7998 100644 --- a/application/src/test/java/run/halo/app/security/authorization/AuthorizationTest.java +++ b/application/src/test/java/run/halo/app/security/authorization/AuthorizationTest.java @@ -6,19 +6,18 @@ import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers.csrf; -import static org.springframework.web.reactive.function.server.RequestPredicates.GET; -import static org.springframework.web.reactive.function.server.RequestPredicates.PUT; -import static org.springframework.web.reactive.function.server.RequestPredicates.accept; -import static org.springframework.web.reactive.function.server.RouterFunctions.route; +import static run.halo.app.core.extension.Role.ROLE_AGGREGATE_LABEL_PREFIX; import java.util.ArrayList; +import java.util.HashMap; +import java.util.Map; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.reactive.AutoConfigureWebTestClient; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.boot.test.context.TestConfiguration; -import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.boot.test.mock.mockito.SpyBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Import; import org.springframework.http.MediaType; @@ -26,8 +25,10 @@ import org.springframework.lang.NonNull; import org.springframework.security.core.userdetails.ReactiveUserDetailsPasswordService; import org.springframework.security.core.userdetails.ReactiveUserDetailsService; import org.springframework.security.test.context.support.WithMockUser; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.web.reactive.server.WebTestClient; import org.springframework.web.reactive.function.server.RouterFunction; +import org.springframework.web.reactive.function.server.RouterFunctions; import org.springframework.web.reactive.function.server.ServerRequest; import org.springframework.web.reactive.function.server.ServerResponse; import reactor.core.publisher.Flux; @@ -35,26 +36,31 @@ import reactor.core.publisher.Mono; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.Role.PolicyRule; import run.halo.app.core.extension.service.RoleService; +import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.Metadata; import run.halo.app.infra.AnonymousUserConst; @SpringBootTest @AutoConfigureWebTestClient +@DirtiesContext @Import(AuthorizationTest.TestConfig.class) class AuthorizationTest { @Autowired WebTestClient webClient; - @MockBean + @SpyBean ReactiveUserDetailsService userDetailsService; - @MockBean + @SpyBean ReactiveUserDetailsPasswordService userDetailsPasswordService; - @MockBean + @SpyBean RoleService roleService; + @Autowired + ExtensionClient client; + @BeforeEach void setUp() { webClient = webClient.mutateWith(csrf()); @@ -122,16 +128,45 @@ class AuthorizationTest { verify(roleService).listDependenciesFlux(anySet()); } + @Test + void anonymousUserShouldAccessResourcesByAggregatedRoles() { + // create a role + var role = new Role(); + role.setMetadata(new Metadata()); + role.getMetadata().setName("fake-role-with-aggregate-to-anonymous"); + role.getMetadata().setLabels(new HashMap<>(Map.of( + ROLE_AGGREGATE_LABEL_PREFIX + AnonymousUserConst.Role, "true" + ))); + role.setRules(new ArrayList<>()); + var policyRule = new PolicyRule.Builder() + .apiGroups("fake.halo.run") + .verbs("list") + .resources("fakes") + .build(); + role.getRules().add(policyRule); + client.create(role); + + webClient.get().uri("/apis/fake.halo.run/v1/fakes").exchange() + .expectStatus() + .isOk(); + } + @TestConfiguration static class TestConfig { @Bean public RouterFunction postRoute() { - return route( - GET("/apis/fake.halo.run/v1/posts").and(accept(MediaType.APPLICATION_JSON)), - this::queryPosts).andRoute( - PUT("/apis/fake.halo.run/v1/posts/{name}").and(accept(MediaType.APPLICATION_JSON)), - this::updatePost); + return RouterFunctions.route() + .GET("/apis/fake.halo.run/v1/posts", request -> ServerResponse.ok() + .contentType(MediaType.TEXT_PLAIN) + .bodyValue("returned posts") + ) + .PUT("/apis/fake.halo.run/v1/posts/{name}", request -> ServerResponse.ok() + .contentType(MediaType.TEXT_PLAIN) + .bodyValue("updated post " + request.pathVariable("name")) + ) + .GET("/apis/fake.halo.run/v1/fakes", request -> ServerResponse.ok().build()) + .build(); } @NonNull