From e5bbf483609e2bfd75e9002db01a573a488b94df Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Mon, 20 Mar 2023 14:26:10 +0800 Subject: [PATCH] refactor: the acquisition method of role rules and no longer obtain aggregated by reconciler (#3425) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind improvement /area core #### What this PR does / why we need it: 修复角色模板规则改动后用户权限不更新的问题 - 权限校验的地方由原来的从角色 annotations 获取聚合 rules 改为根据角色名称查询所有依赖角色再判断 - console 端获取 UI 权限的地方由获取聚合的 ui-permissions annotation 改为根据角色名称查询所有依赖角色组合后再返回 see #3325 for more detail. 问题描述： 1. 创建一个角色比如叫 test-role 将其分配给一个新用户 2. 安装插件 [plugin-links-1.1.0.jar.zip](https://github.com/halo-dev/halo/files/10856628/plugin-links-1.1.0.jar.zip)，该插件提供了一个角色模板配置了 links 但没有配置 groups，当将此插件的查看权限分配给 test-role 后，test-role 会提示 groups 403无权限 3. 使用[plugin-links-1.1.0-after.jar.zip](https://github.com/halo-dev/halo/files/10856695/plugin-links-1.1.0-after.jar.zip) 升级插件后写了，插件已经修复了 gorups 的权限模板配置，期望 test-role 到链接管理菜单不会报 groups 403，但实际得到了 403 就是因为 test-role 角色依赖了一些其他角色但其他角色权限的改动没有办法通知到 test-role 去更新 test-role 中聚合的 dependency-rules annotation 4. 综上所述改为了直接查询 how to test it? 1. 根据上述问题描述中的步骤来测试，期望升级插件后能正确查看链接 2. 测试为一些角色分配权限后，拥有此角色的用户能正确访问那些资源不会出现403 3. 用户访问没有权限的资源会 403 #### Which issue(s) this PR fixes: Fixes #3325 #### Does this PR introduce a user-facing change? ```release-note 修复角色模板规则改动后用户权限不更新的问题 ``` --- .../core/extension/endpoint/UserEndpoint.java | 48 ++-- .../extension/reconciler/RoleReconciler.java | 56 +---- .../extension/service/DefaultRoleService.java | 24 ++ .../core/extension/service/RoleService.java | 2 + .../authorization/DefaultRuleResolver.java | 42 ++-- .../config/ExtensionConfigurationTest.java | 6 +- .../app/content/PostIntegrationTests.java | 6 + .../extension/endpoint/UserEndpointTest.java | 12 +- .../reconciler/RoleReconcilerTest.java | 131 ----------- .../service/DefaultRoleServiceTest.java | 215 ++++++++++++++++++ 10 files changed, 307 insertions(+), 235 deletions(-) delete mode 100644 src/test/java/run/halo/app/core/extension/reconciler/RoleReconcilerTest.java diff --git a/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java b/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java index 543bdbaec..f4954eed5 100644 --- a/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java +++ b/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java @@ -15,15 +15,16 @@ import io.swagger.v3.oas.annotations.media.ArraySchema; import io.swagger.v3.oas.annotations.media.Schema; import java.util.ArrayList; import java.util.Comparator; +import java.util.HashSet; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.function.Function; import java.util.function.Predicate; import java.util.regex.Matcher; import java.util.regex.Pattern; -import java.util.stream.Collectors; import lombok.RequiredArgsConstructor; import org.apache.commons.lang3.StringUtils; import org.springdoc.webflux.core.fn.SpringdocRouteBuilder; @@ -256,36 +257,33 @@ public class UserEndpoint implements CustomEndpoint { list.add(role); return list; }) - .map(roles -> { - Set uiPermissions = roles.stream() - .map(role -> role.getMetadata().getAnnotations()) - .filter(Objects::nonNull) - .map(this::mergeUiPermissions) - .flatMap(Set::stream) - .collect(Collectors.toSet()); - return new UserPermission(roles, uiPermissions); - }) + .flatMap(roles -> uiPermissions(roles) + .collectList() + .map(uiPermissions -> new UserPermission(roles, Set.copyOf(uiPermissions))) + .defaultIfEmpty(new UserPermission(roles, Set.of())) + ) .flatMap(result -> ServerResponse.ok() .contentType(MediaType.APPLICATION_JSON) .bodyValue(result) ); } - private Set mergeUiPermissions(Map annotations) { - Set result = new LinkedHashSet<>(); - String permissionsStr = annotations.get(Role.UI_PERMISSIONS_AGGREGATED_ANNO); - if (StringUtils.isNotBlank(permissionsStr)) { - result.addAll(JsonUtils.jsonToObject(permissionsStr, - new TypeReference>() { - })); - } - String uiPermissionStr = annotations.get(Role.UI_PERMISSIONS_ANNO); - if (StringUtils.isNotBlank(uiPermissionStr)) { - result.addAll(JsonUtils.jsonToObject(uiPermissionStr, - new TypeReference>() { - })); - } - return result; + private Flux uiPermissions(Set roles) { + return Flux.fromIterable(roles) + .map(role -> role.getMetadata().getName()) + .collectList() + .flatMapMany(roleNames -> roleService.listDependenciesFlux(Set.copyOf(roleNames))) + .map(role -> { + Map annotations = ExtensionUtil.nullSafeAnnotations(role); + String uiPermissionStr = annotations.get(Role.UI_PERMISSIONS_ANNO); + if (StringUtils.isBlank(uiPermissionStr)) { + return new HashSet(); + } + return JsonUtils.jsonToObject(uiPermissionStr, + new TypeReference>() { + }); + }) + .flatMapIterable(Function.identity()); } record UserPermission(@Schema(required = true) Set roles, diff --git a/src/main/java/run/halo/app/core/extension/reconciler/RoleReconciler.java b/src/main/java/run/halo/app/core/extension/reconciler/RoleReconciler.java index 54afff0c6..2df8c608a 100644 --- a/src/main/java/run/halo/app/core/extension/reconciler/RoleReconciler.java +++ b/src/main/java/run/halo/app/core/extension/reconciler/RoleReconciler.java @@ -2,23 +2,16 @@ package run.halo.app.core.extension.reconciler; import static java.util.Objects.deepEquals; -import com.fasterxml.jackson.core.type.TypeReference; -import java.util.LinkedHashMap; -import java.util.LinkedHashSet; -import java.util.List; import java.util.Map; -import java.util.Set; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.lang3.StringUtils; import org.springframework.stereotype.Component; import run.halo.app.core.extension.Role; -import run.halo.app.core.extension.service.RoleService; import run.halo.app.extension.ExtensionClient; +import run.halo.app.extension.ExtensionUtil; import run.halo.app.extension.controller.Controller; import run.halo.app.extension.controller.ControllerBuilder; import run.halo.app.extension.controller.Reconciler; import run.halo.app.extension.controller.Reconciler.Request; -import run.halo.app.infra.utils.JsonUtils; /** * Role reconcile. @@ -32,35 +25,18 @@ public class RoleReconciler implements Reconciler { private final ExtensionClient client; - private final RoleService roleService; - - public RoleReconciler(ExtensionClient client, RoleService roleService) { + public RoleReconciler(ExtensionClient client) { this.client = client; - this.roleService = roleService; } @Override public Result reconcile(Request request) { client.fetch(Role.class, request.name()) .ifPresent(role -> { - var annotations = role.getMetadata().getAnnotations(); - if (annotations == null) { - annotations = new LinkedHashMap<>(); - role.getMetadata().setAnnotations(annotations); - } - var roleDependencies = readValue(annotations.get(Role.ROLE_DEPENDENCIES_ANNO)); - var dependenciesRole = roleService.listDependencies(roleDependencies); - var dependencyRules = dependenciesRole.stream() - .map(Role::getRules) - .flatMap(List::stream) - .sorted() - .toList(); - var uiPermissions = aggregateUiPermissions(dependenciesRole); + Map annotations = ExtensionUtil.nullSafeAnnotations(role); // override dependency rules to annotations - annotations.put(Role.ROLE_DEPENDENCY_RULES, - JsonUtils.objectToJson(dependencyRules)); - annotations.put(Role.UI_PERMISSIONS_AGGREGATED_ANNO, - JsonUtils.objectToJson(uiPermissions)); + annotations.put(Role.ROLE_DEPENDENCY_RULES, "[]"); + annotations.put(Role.UI_PERMISSIONS_AGGREGATED_ANNO, "[]"); updateLabelsAndAnnotations(role); }); @@ -86,26 +62,4 @@ public class RoleReconciler implements Reconciler { client.update(freshRole); }); } - - private List aggregateUiPermissions(List dependencyRoles) { - return dependencyRoles.stream() - .filter(role -> role.getMetadata().getAnnotations() != null) - .map(role -> { - Map roleAnnotations = role.getMetadata().getAnnotations(); - return roleAnnotations.get(Role.UI_PERMISSIONS_ANNO); - }) - .map(this::readValue) - .flatMap(Set::stream) - .sorted() - .toList(); - } - - private Set readValue(String json) { - if (StringUtils.isBlank(json)) { - return new LinkedHashSet<>(); - } - return JsonUtils.jsonToObject(json, new TypeReference<>() { - }); - } - } diff --git a/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java b/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java index 1e7453009..6c5605cee 100644 --- a/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java +++ b/src/main/java/run/halo/app/core/extension/service/DefaultRoleService.java @@ -20,6 +20,7 @@ import run.halo.app.core.extension.Role; import run.halo.app.core.extension.RoleBinding; import run.halo.app.core.extension.RoleBinding.RoleRef; import run.halo.app.core.extension.RoleBinding.Subject; +import run.halo.app.extension.ExtensionUtil; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.infra.utils.JsonUtils; @@ -89,6 +90,29 @@ public class DefaultRoleService implements RoleService { return result; } + @Override + public Flux listDependenciesFlux(Set names) { + if (names == null) { + return Flux.empty(); + } + Set visited = new HashSet<>(); + return Flux.fromIterable(names) + .flatMap(name -> extensionClient.fetch(Role.class, name)) + .expand(role -> { + var name = role.getMetadata().getName(); + if (visited.contains(name)) { + return Flux.empty(); + } + visited.add(name); + var annotations = ExtensionUtil.nullSafeAnnotations(role); + var dependenciesJson = annotations.get(Role.ROLE_DEPENDENCIES_ANNO); + var dependencies = stringToList(dependenciesJson); + return Flux.fromIterable(dependencies) + .filter(dependency -> !visited.contains(dependency)) + .flatMap(dependencyName -> extensionClient.fetch(Role.class, dependencyName)); + }); + } + @Override public Flux list(Set roleNames) { return Flux.fromIterable(ObjectUtils.defaultIfNull(roleNames, Set.of())) diff --git a/src/main/java/run/halo/app/core/extension/service/RoleService.java b/src/main/java/run/halo/app/core/extension/service/RoleService.java index 52d0a8538..bfe76d9ca 100644 --- a/src/main/java/run/halo/app/core/extension/service/RoleService.java +++ b/src/main/java/run/halo/app/core/extension/service/RoleService.java @@ -25,5 +25,7 @@ public interface RoleService { List listDependencies(Set names); + Flux listDependenciesFlux(Set names); + Flux list(Set roleNames); } diff --git a/src/main/java/run/halo/app/security/authorization/DefaultRuleResolver.java b/src/main/java/run/halo/app/security/authorization/DefaultRuleResolver.java index ec52cb876..31540eda2 100644 --- a/src/main/java/run/halo/app/security/authorization/DefaultRuleResolver.java +++ b/src/main/java/run/halo/app/security/authorization/DefaultRuleResolver.java @@ -9,10 +9,11 @@ import java.util.List; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import lombok.Data; +import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.security.core.userdetails.UserDetails; import org.springframework.util.Assert; -import reactor.core.publisher.Flux; +import org.springframework.util.CollectionUtils; import reactor.core.publisher.Mono; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.service.DefaultRoleBindingService; @@ -27,6 +28,7 @@ import run.halo.app.infra.utils.JsonUtils; * @since 2.0.0 */ @Data +@Slf4j public class DefaultRuleResolver implements AuthorizationRuleResolver { private static final String AUTHENTICATED_ROLE = "authenticated"; private RoleService roleService; @@ -96,26 +98,27 @@ public class DefaultRuleResolver implements AuthorizationRuleResolver { var record = new AttributesRecord(user, requestInfo); var visitor = new AuthorizingVisitor(record); var stopVisiting = new AtomicBoolean(false); - return Flux.fromIterable(roleNames) - .flatMap(roleName -> { + return roleService.listDependenciesFlux(roleNames) + .filter(role -> !CollectionUtils.isEmpty(role.getRules())) + .doOnNext(role -> { if (stopVisiting.get()) { - return Mono.empty(); + return; } - return roleService.getMonoRole(roleName) - .onErrorResume(t -> visitor.visit(null, null, t), t -> { - //Do nothing here - return Mono.empty(); - }) - .doOnNext(role -> { - var rules = fetchRules(role); - var source = roleBindingDescriber(roleName, user.getUsername()); - for (var rule : rules) { - if (!visitor.visit(source, rule, null)) { - stopVisiting.set(true); - return; - } - } - }); + String roleName = role.getMetadata().getName(); + var rules = role.getRules(); + var source = roleBindingDescriber(roleName, user.getUsername()); + for (var rule : rules) { + if (!visitor.visit(source, rule, null)) { + stopVisiting.set(true); + return; + } + } + }) + .takeUntil(item -> stopVisiting.get()) + .onErrorResume(t -> visitor.visit(null, null, t), t -> { + log.warn("Error occurred when visiting rules", t); + //Do nothing here + return Mono.empty(); }) .then(Mono.just(visitor)); } @@ -125,6 +128,7 @@ public class DefaultRuleResolver implements AuthorizationRuleResolver { if (metadata == null || metadata.getAnnotations() == null) { return role.getRules(); } + // merge policy rules String roleDependencyRules = metadata.getAnnotations() .get(Role.ROLE_DEPENDENCY_RULES); diff --git a/src/test/java/run/halo/app/config/ExtensionConfigurationTest.java b/src/test/java/run/halo/app/config/ExtensionConfigurationTest.java index e57b305c6..e50fa118e 100644 --- a/src/test/java/run/halo/app/config/ExtensionConfigurationTest.java +++ b/src/test/java/run/halo/app/config/ExtensionConfigurationTest.java @@ -4,6 +4,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers.csrf; @@ -23,6 +24,7 @@ import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.web.reactive.server.WebTestClient; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.service.RoleService; @@ -55,9 +57,11 @@ class ExtensionConfigurationTest { .verbs("*") .build(); var role = new Role(); + role.setMetadata(new Metadata()); + role.getMetadata().setName("supper-role"); role.setRules(List.of(rule)); when(roleService.getMonoRole(anyString())).thenReturn(Mono.just(role)); - + when(roleService.listDependenciesFlux(anySet())).thenReturn(Flux.just(role)); // register scheme schemeManager.register(FakeExtension.class); diff --git a/src/test/java/run/halo/app/content/PostIntegrationTests.java b/src/test/java/run/halo/app/content/PostIntegrationTests.java index d047b0eaf..982b669a9 100644 --- a/src/test/java/run/halo/app/content/PostIntegrationTests.java +++ b/src/test/java/run/halo/app/content/PostIntegrationTests.java @@ -1,6 +1,7 @@ package run.halo.app.content; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anySet; import static org.mockito.Mockito.when; import static org.springframework.security.test.web.reactive.server.SecurityMockServerConfigurers.csrf; @@ -15,10 +16,12 @@ import org.springframework.boot.test.mock.mockito.MockBean; import org.springframework.http.MediaType; import org.springframework.security.test.context.support.WithMockUser; import org.springframework.test.web.reactive.server.WebTestClient; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.content.Post; import run.halo.app.core.extension.service.RoleService; +import run.halo.app.extension.Metadata; import run.halo.app.extension.MetadataOperator; import run.halo.app.extension.ReactiveExtensionClient; import run.halo.app.infra.utils.JsonUtils; @@ -52,8 +55,11 @@ public class PostIntegrationTests { .verbs("*") .build(); var role = new Role(); + role.setMetadata(new Metadata()); + role.getMetadata().setName("super-role"); role.setRules(List.of(rule)); when(roleService.getMonoRole("authenticated")).thenReturn(Mono.just(role)); + when(roleService.listDependenciesFlux(anySet())).thenReturn(Flux.just(role)); webTestClient = webTestClient.mutateWith(csrf()); } diff --git a/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java b/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java index 5d2f2979d..70ccabd5d 100644 --- a/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java +++ b/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java @@ -453,9 +453,7 @@ class UserEndpointTest { "metadata": { "name": "test-A", "annotations": { - "rbac.authorization.halo.run/ui-permissions": "[\\"permission-A\\"]", - "rbac.authorization.halo.run/ui-permissions-aggregated": - "[\\"permission-B\\"]" + "rbac.authorization.halo.run/ui-permissions": "[\\"permission-A\\"]" } }, "rules": [] @@ -463,6 +461,7 @@ class UserEndpointTest { """, Role.class); when(userService.listRoles(eq("fake-user"))).thenReturn( Flux.fromIterable(List.of(roleA))); + when(roleService.listDependenciesFlux(anySet())).thenReturn(Flux.just(roleA)); webClient.get().uri("/users/fake-user/permissions") .exchange() @@ -478,15 +477,12 @@ class UserEndpointTest { "name": "test-A", "annotations": { "rbac.authorization.halo.run/ui-permissions": - "[\\"permission-A\\"]", - "rbac.authorization.halo.run/ui-permissions-aggregated": - "[\\"permission-B\\"]" + "[\\"permission-A\\"]" } } }], "uiPermissions": [ - "permission-A", - "permission-B" + "permission-A" ] } """); diff --git a/src/test/java/run/halo/app/core/extension/reconciler/RoleReconcilerTest.java b/src/test/java/run/halo/app/core/extension/reconciler/RoleReconcilerTest.java deleted file mode 100644 index 0bb96c037..000000000 --- a/src/test/java/run/halo/app/core/extension/reconciler/RoleReconcilerTest.java +++ /dev/null @@ -1,131 +0,0 @@ -package run.halo.app.core.extension.reconciler; - -import static org.assertj.core.api.Assertions.assertThat; -import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; -import static org.mockito.Mockito.doNothing; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.when; - -import java.util.HashMap; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import org.json.JSONException; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; -import org.junit.jupiter.api.extension.ExtendWith; -import org.mockito.ArgumentCaptor; -import org.mockito.Mock; -import org.mockito.junit.jupiter.MockitoExtension; -import org.skyscreamer.jsonassert.JSONAssert; -import run.halo.app.core.extension.Role; -import run.halo.app.core.extension.TestRole; -import run.halo.app.core.extension.service.RoleService; -import run.halo.app.extension.ExtensionClient; -import run.halo.app.extension.controller.Reconciler; - -/** - * Tests for {@link RoleReconciler}. - * - * @author guqing - * @since 2.0.0 - */ -@ExtendWith(MockitoExtension.class) -class RoleReconcilerTest { - - @Mock - private ExtensionClient extensionClient; - - @Mock - private RoleService roleService; - - private RoleReconciler roleReconciler; - - @BeforeEach - void setUp() { - roleReconciler = new RoleReconciler(extensionClient, roleService); - } - - @Test - void reconcile() throws JSONException { - Role roleManage = TestRole.getRoleManage(); - Map manageAnnotations = new HashMap<>(); - manageAnnotations.put(Role.ROLE_DEPENDENCIES_ANNO, "[\"role-template-apple-view\"]"); - roleManage.getMetadata().setAnnotations(manageAnnotations); - - Role roleView = TestRole.getRoleView(); - - Role roleOther = TestRole.getRoleOther(); - when(roleService.listDependencies(eq(Set.of("role-template-apple-view")))) - .thenReturn(List.of(roleView, roleOther)); - - when(extensionClient.fetch(eq(Role.class), eq("role-template-apple-manage"))).thenReturn( - Optional.of(roleManage)); - - ArgumentCaptor roleCaptor = ArgumentCaptor.forClass(Role.class); - doNothing().when(extensionClient).update(roleCaptor.capture()); - - Reconciler.Request request = new Reconciler.Request("role-template-apple-manage"); - roleReconciler.reconcile(request); - String expected = """ - [ - { - "resources": ["apples"], - "verbs": ["list"] - }, - { - "resources": ["apples"], - "verbs": ["update"] - } - ] - """; - Role updateArgs = roleCaptor.getValue(); - assertThat(updateArgs).isNotNull(); - JSONAssert.assertEquals(expected, updateArgs.getMetadata().getAnnotations() - .get(Role.ROLE_DEPENDENCY_RULES), false); - } - - @Test - void reconcileUiPermission() { - Role roleManage = TestRole.getRoleManage(); - Map annotations = new LinkedHashMap<>(); - annotations.put(Role.ROLE_DEPENDENCIES_ANNO, "[\"role-template-apple-view\"]"); - annotations.put(Role.UI_PERMISSIONS_ANNO, "[\"apples:manage\"]"); - roleManage.getMetadata().setAnnotations(annotations); - - Role roleView = TestRole.getRoleView(); - Map roleViewAnnotations = new LinkedHashMap<>(); - roleViewAnnotations.put(Role.ROLE_DEPENDENCIES_ANNO, "[\"role-template-apple-other\"]"); - roleViewAnnotations.put(Role.UI_PERMISSIONS_ANNO, "[\"apples:view\"]"); - roleView.getMetadata().setAnnotations(roleViewAnnotations); - - Role roleOther = TestRole.getRoleOther(); - Map roleOtherAnnotations = new LinkedHashMap<>(); - roleOtherAnnotations.put(Role.ROLE_DEPENDENCIES_ANNO, "[\"role-template-apple-other\"]"); - roleOtherAnnotations.put(Role.UI_PERMISSIONS_ANNO, "[\"apples:foo\", \"apples:bar\"]"); - roleOther.getMetadata().setAnnotations(roleOtherAnnotations); - - when(extensionClient.fetch(eq(Role.class), eq("role-template-apple-manage"))).thenReturn( - Optional.of(roleManage)); - - when(roleService.listDependencies(any())) - .thenReturn(List.of(roleView, roleOther)); - - ArgumentCaptor roleCaptor = ArgumentCaptor.forClass(Role.class); - - roleReconciler.reconcile(new Reconciler.Request("role-template-apple-manage")); - verify(extensionClient, times(1)).update(roleCaptor.capture()); - - // assert that the user has the correct roles - Role value = roleCaptor.getValue(); - Map resultAnnotations = value.getMetadata().getAnnotations(); - assertThat(resultAnnotations).isNotNull(); - assertThat(resultAnnotations.containsKey(Role.UI_PERMISSIONS_AGGREGATED_ANNO)).isTrue(); - assertThat(resultAnnotations.get(Role.UI_PERMISSIONS_AGGREGATED_ANNO)).isEqualTo( - "[\"apples:bar\",\"apples:foo\",\"apples:view\"]"); - } -} \ No newline at end of file diff --git a/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java b/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java index 47a94450a..e6905fc65 100644 --- a/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java +++ b/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java @@ -1,8 +1,10 @@ package run.halo.app.core.extension.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.eq; +import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -12,14 +14,19 @@ import java.util.List; import java.util.Map; import java.util.Set; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; +import reactor.test.StepVerifier; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.TestRole; +import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; +import run.halo.app.infra.utils.JsonUtils; /** * Tests for {@link DefaultRoleService}. @@ -81,4 +88,212 @@ class DefaultRoleServiceTest { roleService.listDependencies(Set.of("role-template-apple-manage")); assertThat(rolesFromCycle).hasSize(3); } + + @Nested + class ListDependenciesTest { + @Test + void listDependencies() { + // prepare test data + Role role1 = createRole("role1", "role2"); + Role role2 = createRole("role2", "role3"); + Role role3 = createRole("role3"); + + Set roleNames = Set.of("role1"); + + // setup mocks + when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.just(role3)); + + // call the method under test + Flux result = roleService.listDependenciesFlux(roleNames); + + // verify the result + StepVerifier.create(result) + .expectNext(role1) + .expectNext(role2) + .expectNext(role3) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(3)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesWithCycle() { + // prepare test data + Role role1 = createRole("role1", "role2"); + Role role2 = createRole("role2", "role3"); + Role role3 = createRole("role3", "role1"); + + Set roleNames = Set.of("role1"); + + // setup mocks + when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.just(role3)); + + // call the method under test + Flux result = roleService.listDependenciesFlux(roleNames); + + // verify the result + StepVerifier.create(result) + .expectNext(role1) + .expectNext(role2) + .expectNext(role3) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(3)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesWithMiddleCycle() { + // prepare test data + // role1 -> role2 -> role3 -> role4 + // \<-----| + Role role1 = createRole("role1", "role2"); + Role role2 = createRole("role2", "role3"); + Role role3 = createRole("role3", "role2", "role4"); + Role role4 = createRole("role4"); + + Set roleNames = Set.of("role1"); + + // setup mocks + when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.just(role3)); + when(extensionClient.fetch(Role.class, "role4")).thenReturn(Mono.just(role4)); + + // call the method under test + Flux result = roleService.listDependenciesFlux(roleNames); + + // verify the result + StepVerifier.create(result) + .expectNext(role1) + .expectNext(role2) + .expectNext(role3) + .expectNext(role4) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(4)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesWithCycleAndSequence() { + // prepare test data + // role1 -> role2 -> role3 + // \->role4 \<-----| + Role role1 = createRole("role1", "role4", "role2"); + Role role2 = createRole("role2", "role3"); + Role role3 = createRole("role3", "role2"); + Role role4 = createRole("role4"); + + Set roleNames = Set.of("role1"); + + // setup mocks + when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.just(role3)); + when(extensionClient.fetch(Role.class, "role4")).thenReturn(Mono.just(role4)); + + // call the method under test + Flux result = roleService.listDependenciesFlux(roleNames); + + // verify the result + StepVerifier.create(result) + .expectNext(role1) + .expectNext(role4) + .expectNext(role2) + .expectNext(role3) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(4)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesAfterCycle() { + // prepare test data + // role1 -> role2 -> role3 + // \->role4 \<-----| + Role role1 = createRole("role1", "role4", "role2"); + Role role2 = createRole("role2", "role3"); + Role role3 = createRole("role3", "role2"); + Role role4 = createRole("role4"); + + Set roleNames = Set.of("role2"); + + // setup mocks + lenient().when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.just(role3)); + lenient().when(extensionClient.fetch(Role.class, "role4")).thenReturn(Mono.just(role4)); + + // call the method under test + Flux result = roleService.listDependenciesFlux(roleNames); + + // verify the result + StepVerifier.create(result) + .expectNext(role2) + .expectNext(role3) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(2)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesWithNullParam() { + Flux result = roleService.listDependenciesFlux(null); + // verify the result + StepVerifier.create(result) + .verifyComplete(); + + result = roleService.listDependenciesFlux(Set.of()); + StepVerifier.create(result) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(0)).fetch(eq(Role.class), anyString()); + } + + @Test + void listDependenciesAndSomeOneNotFound() { + Role role1 = createRole("role1", "role2"); + Role role2 = createRole("role2", "role3", "role4"); + Role role4 = createRole("role4"); + + Set roleNames = Set.of("role1"); + + // setup mocks + when(extensionClient.fetch(Role.class, "role1")).thenReturn(Mono.just(role1)); + when(extensionClient.fetch(Role.class, "role2")).thenReturn(Mono.just(role2)); + when(extensionClient.fetch(Role.class, "role3")).thenReturn(Mono.empty()); + when(extensionClient.fetch(Role.class, "role4")).thenReturn(Mono.just(role4)); + + Flux result = roleService.listDependenciesFlux(roleNames); + // verify the result + StepVerifier.create(result) + .expectNext(role1) + .expectNext(role2) + .expectNext(role4) + .verifyComplete(); + + // verify the mock invocations + verify(extensionClient, times(4)).fetch(eq(Role.class), anyString()); + } + + private Role createRole(String name, String... dependencies) { + Role role = new Role(); + role.setMetadata(new Metadata()); + role.getMetadata().setName(name); + + Map annotations = new HashMap<>(); + annotations.put(Role.ROLE_DEPENDENCIES_ANNO, JsonUtils.objectToJson(dependencies)); + role.getMetadata().setAnnotations(annotations); + return role; + } + } } \ No newline at end of file