From bd4cc0c72d5d070f5a5f8951a53aac384904dfc9 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Mon, 27 Mar 2023 17:48:01 +0800 Subject: [PATCH] feat: support aggregate several roles into one combined role (#3568) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #### What type of PR is this? /kind feature /milestone 2.4.x /area core #### What this PR does / why we need it: 支持聚合多个角色到一个角色 see #3560 for more details. how to test it? 创建一个测试角色和和一个 RoleBinding 将此角色的绑定到其他角色，在不修改用户权限的情况下，用户将拥有新创建的测试角色的权限。 #### Which issue(s) this PR fixes: Fixes #3560 #### Does this PR introduce a user-facing change? ```release-note 支持聚合多个角色到一个角色 ``` --- .../run/halo/app/core/extension/Role.java | 2 + .../extension/service/DefaultRoleService.java | 37 ++++++++++- .../authorization/RbacRequestEvaluation.java | 29 ++++++++- .../service/DefaultRoleServiceTest.java | 65 ++++++++++++++++++- .../RbacRequestEvaluationTest.java | 47 ++++++++++++++ 5 files changed, 176 insertions(+), 4 deletions(-) create mode 100644 application/src/test/java/run/halo/app/security/authorization/RbacRequestEvaluationTest.java diff --git a/api/src/main/java/run/halo/app/core/extension/Role.java b/api/src/main/java/run/halo/app/core/extension/Role.java index 0cc0d31b1..6faedcada 100644 --- a/api/src/main/java/run/halo/app/core/extension/Role.java +++ b/api/src/main/java/run/halo/app/core/extension/Role.java @@ -30,6 +30,8 @@ import run.halo.app.extension.GVK; public class Role extends AbstractExtension { public static final String ROLE_DEPENDENCY_RULES = "rbac.authorization.halo.run/dependency-rules"; + public static final String ROLE_AGGREGATE_LABEL_PREFIX = + "rbac.authorization.halo.run/aggregate-to-"; public static final String ROLE_DEPENDENCIES_ANNO = "rbac.authorization.halo.run/dependencies"; public static final String UI_PERMISSIONS_ANNO = "rbac.authorization.halo.run/ui-permissions"; 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 28ee304a6..528130a8c 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 @@ -1,14 +1,18 @@ package run.halo.app.core.extension.service; +import static run.halo.app.extension.MetadataUtil.nullSafeLabels; + import com.fasterxml.jackson.core.type.TypeReference; import java.util.ArrayDeque; import java.util.ArrayList; import java.util.Collections; +import java.util.Comparator; import java.util.Deque; import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.ObjectUtils; import org.apache.commons.lang3.StringUtils; @@ -110,7 +114,38 @@ public class DefaultRoleService implements RoleService { return Flux.fromIterable(dependencies) .filter(dependency -> !visited.contains(dependency)) .flatMap(dependencyName -> extensionClient.fetch(Role.class, dependencyName)); - }); + }) + .flatMap(role -> Flux.just(role) + .mergeWith(listAggregatedRoles(role.getMetadata().getName())) + ); + } + + Flux listAggregatedRoles(String roleName) { + return extensionClient.list(Role.class, + role -> Boolean.parseBoolean(nullSafeLabels(role) + .get(Role.ROLE_AGGREGATE_LABEL_PREFIX + roleName) + ), + Comparator.comparing(item -> item.getMetadata().getCreationTimestamp())); + } + + Predicate getRoleBindingPredicate(Subject targetSubject) { + return roleBinding -> { + List subjects = roleBinding.getSubjects(); + for (Subject subject : subjects) { + return matchSubject(targetSubject, subject); + } + return false; + }; + } + + private static boolean matchSubject(Subject targetSubject, Subject subject) { + if (targetSubject == null || subject == null) { + return false; + } + return StringUtils.equals(targetSubject.getKind(), subject.getKind()) + && StringUtils.equals(targetSubject.getName(), subject.getName()) + && StringUtils.defaultString(targetSubject.getApiGroup()) + .equals(StringUtils.defaultString(subject.getApiGroup())); } @Override diff --git a/application/src/main/java/run/halo/app/security/authorization/RbacRequestEvaluation.java b/application/src/main/java/run/halo/app/security/authorization/RbacRequestEvaluation.java index f810a44c4..3cf832cfc 100644 --- a/application/src/main/java/run/halo/app/security/authorization/RbacRequestEvaluation.java +++ b/application/src/main/java/run/halo/app/security/authorization/RbacRequestEvaluation.java @@ -109,14 +109,39 @@ public class RbacRequestEvaluation { if (ArrayUtils.isEmpty(rule.getResourceNames())) { return true; } + String[] requestedNameParts = ArrayUtils.nullToEmpty(StringUtils.split(requestedName, "/")); for (String ruleName : rule.getResourceNames()) { - if (Objects.equals(ruleName, requestedName)) { - return true; + String[] patternParts = StringUtils.split(ruleName, "/"); + + for (int i = 0; i < patternParts.length; i++) { + String patternPart = patternParts[i]; + String textPart = StringUtils.EMPTY; + if (requestedNameParts.length > i) { + textPart = requestedNameParts[i]; + } + + if (!matchPart(patternPart, textPart)) { + return false; + } } + + return true; } return false; } + private static boolean matchPart(String patternPart, String textPart) { + if (patternPart.equals("*")) { + return true; + } else if (patternPart.startsWith("*")) { + return textPart.endsWith(patternPart.substring(1)); + } else if (patternPart.endsWith("*")) { + return textPart.startsWith(patternPart.substring(0, patternPart.length() - 1)); + } else { + return patternPart.equals(textPart); + } + } + protected boolean nonResourceURLMatches(Role.PolicyRule rule, String requestedURL) { for (String ruleURL : rule.getNonResourceURLs()) { if (Objects.equals(ruleURL, WildCard.NonResourceAll)) { diff --git a/application/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java b/application/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java index e6905fc65..a9b622921 100644 --- a/application/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java +++ b/application/src/test/java/run/halo/app/core/extension/service/DefaultRoleServiceTest.java @@ -1,6 +1,7 @@ package run.halo.app.core.extension.service; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.same; import static org.mockito.Mockito.eq; @@ -13,6 +14,9 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.function.Predicate; +import java.util.stream.Stream; +import org.assertj.core.api.AssertionsForInterfaceTypes; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; @@ -23,6 +27,7 @@ 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.RoleBinding; import run.halo.app.core.extension.TestRole; import run.halo.app.extension.Metadata; import run.halo.app.extension.ReactiveExtensionClient; @@ -104,6 +109,8 @@ class DefaultRoleServiceTest { 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.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); // call the method under test Flux result = roleService.listDependenciesFlux(roleNames); @@ -132,6 +139,8 @@ class DefaultRoleServiceTest { 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.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); // call the method under test Flux result = roleService.listDependenciesFlux(roleNames); @@ -164,6 +173,8 @@ class DefaultRoleServiceTest { 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)); + when(extensionClient.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); // call the method under test Flux result = roleService.listDependenciesFlux(roleNames); @@ -197,6 +208,8 @@ class DefaultRoleServiceTest { 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)); + when(extensionClient.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); // call the method under test Flux result = roleService.listDependenciesFlux(roleNames); @@ -230,6 +243,8 @@ class DefaultRoleServiceTest { 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)); + when(extensionClient.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); // call the method under test Flux result = roleService.listDependenciesFlux(roleNames); @@ -272,6 +287,8 @@ class DefaultRoleServiceTest { 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)); + when(extensionClient.list(eq(Role.class), any(), any())) + .thenReturn(Flux.empty()); Flux result = roleService.listDependenciesFlux(roleNames); // verify the result @@ -285,6 +302,52 @@ class DefaultRoleServiceTest { verify(extensionClient, times(4)).fetch(eq(Role.class), anyString()); } + @Test + void testSubjectMatch() { + RoleBinding fakeAuthenticatedBinding = + createRoleBinding("authenticated-fake-binding", "fake", "authenticated"); + RoleBinding fakeEditorBinding = + createRoleBinding("editor-fake-binding", "fake", "editor"); + RoleBinding fakeAnonymousBinding = + createRoleBinding("test-anonymous-binding", "test", "anonymous"); + + RoleBinding.Subject subject = new RoleBinding.Subject(); + subject.setName("authenticated"); + subject.setKind(Role.KIND); + subject.setApiGroup(Role.GROUP); + + Predicate predicate = roleService.getRoleBindingPredicate(subject); + List result = + Stream.of(fakeAuthenticatedBinding, fakeEditorBinding, fakeAnonymousBinding) + .filter(predicate) + .toList(); + AssertionsForInterfaceTypes.assertThat(result) + .containsExactly(fakeAuthenticatedBinding); + + subject.setName("editor"); + predicate = roleService.getRoleBindingPredicate(subject); + result = + Stream.of(fakeAuthenticatedBinding, fakeEditorBinding, fakeAnonymousBinding) + .filter(predicate) + .toList(); + AssertionsForInterfaceTypes.assertThat(result).containsExactly(fakeEditorBinding); + } + + RoleBinding createRoleBinding(String name, String refName, String subjectName) { + RoleBinding roleBinding = new RoleBinding(); + roleBinding.setMetadata(new Metadata()); + roleBinding.getMetadata().setName(name); + roleBinding.setRoleRef(new RoleBinding.RoleRef()); + roleBinding.getRoleRef().setKind(Role.KIND); + roleBinding.getRoleRef().setApiGroup(Role.GROUP); + roleBinding.getRoleRef().setName(refName); + roleBinding.setSubjects(List.of(new RoleBinding.Subject())); + roleBinding.getSubjects().get(0).setKind(Role.KIND); + roleBinding.getSubjects().get(0).setName(subjectName); + roleBinding.getSubjects().get(0).setApiGroup(Role.GROUP); + return roleBinding; + } + private Role createRole(String name, String... dependencies) { Role role = new Role(); role.setMetadata(new Metadata()); @@ -296,4 +359,4 @@ class DefaultRoleServiceTest { return role; } } -} \ No newline at end of file +} diff --git a/application/src/test/java/run/halo/app/security/authorization/RbacRequestEvaluationTest.java b/application/src/test/java/run/halo/app/security/authorization/RbacRequestEvaluationTest.java new file mode 100644 index 000000000..efbfe0640 --- /dev/null +++ b/application/src/test/java/run/halo/app/security/authorization/RbacRequestEvaluationTest.java @@ -0,0 +1,47 @@ +package run.halo.app.security.authorization; + +import static org.assertj.core.api.AssertionsForClassTypes.assertThat; + +import org.junit.jupiter.api.Test; +import run.halo.app.core.extension.Role; + +/** + * Tests for {@link RbacRequestEvaluation}. + * + * @author guqing + * @since 2.4.0 + */ +class RbacRequestEvaluationTest { + + @Test + void resourceNameMatches() { + RbacRequestEvaluation rbacRequestEvaluation = new RbacRequestEvaluation(); + assertThat(matchResourceName(rbacRequestEvaluation, "", "fake/test")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "", "fake")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "", "")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "*", null)).isTrue(); + + assertThat(matchResourceName(rbacRequestEvaluation, "*/test", "fake/test")).isTrue(); + + assertThat(matchResourceName(rbacRequestEvaluation, "*/test", "hello/test")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "*/test", "hello/fake")).isFalse(); + + assertThat(matchResourceName(rbacRequestEvaluation, "test/*", "hello/fake")).isFalse(); + assertThat(matchResourceName(rbacRequestEvaluation, "test/*", "test/fake")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "test/*", "test")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "test/*", "hello")).isFalse(); + + assertThat(matchResourceName(rbacRequestEvaluation, "*/*", "test/fake")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "*/*", "test")).isTrue(); + + assertThat(matchResourceName(rbacRequestEvaluation, "*", "test")).isTrue(); + assertThat(matchResourceName(rbacRequestEvaluation, "*", "hello")).isTrue(); + } + + boolean matchResourceName(RbacRequestEvaluation rbacRequestEvaluation, String rule, + String requestedName) { + return rbacRequestEvaluation.resourceNameMatches(new Role.PolicyRule.Builder() + .resourceNames(rule) + .build(), requestedName); + } +}