From 602b783506556d71c1764f9d35f08289677e5b16 Mon Sep 17 00:00:00 2001 From: guqing <38999863+guqing@users.noreply.github.com> Date: Mon, 17 Apr 2023 21:46:38 +0800 Subject: [PATCH] refactor: allow users to modify their own annotations in metadata (#3739) 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 /milestone 2.5.x #### What this PR does / why we need it: 允许用户修改自己的元数据信息 how to test it 使用 API 修改元数据 `PUT localhost:8090/apis/api.console.halo.run/v1alpha1/users/-` 1. 修改 annotations 中的 `"rbac.authorization.halo.run/role-names": "[\"super-role\",\"fake-role\"]"` 会被复原 2. 修改其他的 annotations 能正确修改，也能增加新的 annotation #### Which issue(s) this PR fixes: Fixes #3544 #### Does this PR introduce a user-facing change? ```release-note 允许用户修改自己的元数据信息 ``` --- .../core/extension/endpoint/UserEndpoint.java | 8 +-- .../extension/reconciler/UserReconciler.java | 47 +++++++++++++++- .../reconciler/UserReconcilerTest.java | 54 ++++++++++++++++--- 3 files changed, 97 insertions(+), 12 deletions(-) diff --git a/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java b/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java index 88321ee8d..e8271fbed 100644 --- a/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java +++ b/application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java @@ -148,12 +148,14 @@ public class UserEndpoint implements CustomEndpoint { .map(Authentication::getName) .flatMap(currentUserName -> client.get(User.class, currentUserName)) .flatMap(currentUser -> request.bodyToMono(User.class) - .filter(user -> - Objects.equals(user.getMetadata().getName(), - currentUser.getMetadata().getName())) + .filter(user -> user.getMetadata() != null + && Objects.equals(user.getMetadata().getName(), + currentUser.getMetadata().getName()) + ) .switchIfEmpty( Mono.error(() -> new ServerWebInputException("Username didn't match."))) .map(user -> { + currentUser.getMetadata().setAnnotations(user.getMetadata().getAnnotations()); var spec = currentUser.getSpec(); var newSpec = user.getSpec(); spec.setAvatar(newSpec.getAvatar()); diff --git a/application/src/main/java/run/halo/app/core/extension/reconciler/UserReconciler.java b/application/src/main/java/run/halo/app/core/extension/reconciler/UserReconciler.java index 5e540d85a..222078bd1 100644 --- a/application/src/main/java/run/halo/app/core/extension/reconciler/UserReconciler.java +++ b/application/src/main/java/run/halo/app/core/extension/reconciler/UserReconciler.java @@ -1,31 +1,42 @@ package run.halo.app.core.extension.reconciler; +import static run.halo.app.core.extension.User.GROUP; +import static run.halo.app.core.extension.User.KIND; + import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; -import lombok.AllArgsConstructor; +import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.apache.commons.lang3.StringUtils; import org.springframework.retry.support.RetryTemplate; import org.springframework.stereotype.Component; +import run.halo.app.core.extension.Role; +import run.halo.app.core.extension.RoleBinding; import run.halo.app.core.extension.User; import run.halo.app.core.extension.UserConnection; +import run.halo.app.core.extension.service.RoleService; import run.halo.app.extension.ExtensionClient; +import run.halo.app.extension.GroupKind; +import run.halo.app.extension.MetadataUtil; 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.AnonymousUserConst; import run.halo.app.infra.ExternalUrlSupplier; +import run.halo.app.infra.utils.JsonUtils; import run.halo.app.infra.utils.PathUtils; @Slf4j @Component -@AllArgsConstructor +@RequiredArgsConstructor public class UserReconciler implements Reconciler { private static final String FINALIZER_NAME = "user-protection"; private final ExtensionClient client; private final ExternalUrlSupplier externalUrlSupplier; + private final RoleService roleService; private final RetryTemplate retryTemplate = RetryTemplate.builder() .maxAttempts(20) .fixedBackoff(300) @@ -41,11 +52,43 @@ public class UserReconciler implements Reconciler { } addFinalizerIfNecessary(user); + ensureRoleNamesAnno(request.name()); updatePermalink(request.name()); }); return new Result(false, null); } + private void ensureRoleNamesAnno(String name) { + client.fetch(User.class, name).ifPresent(user -> { + Map annotations = MetadataUtil.nullSafeAnnotations(user); + Map oldAnnotations = Map.copyOf(annotations); + + List roleNames = listRoleNamesRef(name); + annotations.put(User.ROLE_NAMES_ANNO, JsonUtils.objectToJson(roleNames)); + + if (!oldAnnotations.equals(annotations)) { + client.update(user); + } + }); + } + + List listRoleNamesRef(String username) { + var subject = new RoleBinding.Subject(KIND, username, GROUP); + return roleService.listRoleRefs(subject) + .filter(this::isRoleRef) + .map(RoleBinding.RoleRef::getName) + .distinct() + .collectList() + .blockOptional() + .orElse(List.of()); + } + + private boolean isRoleRef(RoleBinding.RoleRef roleRef) { + var roleGvk = new Role().groupVersionKind(); + var gk = new GroupKind(roleRef.getApiGroup(), roleRef.getKind()); + return gk.equals(roleGvk.groupKind()); + } + private void updatePermalink(String name) { client.fetch(User.class, name).ifPresent(user -> { if (AnonymousUserConst.isAnonymousUser(name)) { diff --git a/application/src/test/java/run/halo/app/core/extension/reconciler/UserReconcilerTest.java b/application/src/test/java/run/halo/app/core/extension/reconciler/UserReconcilerTest.java index 57d58f5ff..19bf539f0 100644 --- a/application/src/test/java/run/halo/app/core/extension/reconciler/UserReconcilerTest.java +++ b/application/src/test/java/run/halo/app/core/extension/reconciler/UserReconcilerTest.java @@ -3,22 +3,29 @@ 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.lenient; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import static run.halo.app.core.extension.User.GROUP; +import static run.halo.app.core.extension.User.KIND; import java.net.URI; import java.net.URISyntaxException; import java.util.Optional; import java.util.Set; +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.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; -import org.springframework.context.ApplicationEventPublisher; +import reactor.core.publisher.Flux; +import run.halo.app.core.extension.Role; +import run.halo.app.core.extension.RoleBinding; import run.halo.app.core.extension.User; +import run.halo.app.core.extension.service.RoleService; import run.halo.app.extension.ExtensionClient; import run.halo.app.extension.Metadata; import run.halo.app.extension.controller.Reconciler; @@ -33,18 +40,23 @@ import run.halo.app.infra.ExternalUrlSupplier; */ @ExtendWith(MockitoExtension.class) class UserReconcilerTest { - @Mock - private ApplicationEventPublisher eventPublisher; - @Mock private ExternalUrlSupplier externalUrlSupplier; @Mock private ExtensionClient client; + @Mock + private RoleService roleService; + @InjectMocks private UserReconciler userReconciler; + @BeforeEach + void setUp() { + lenient().when(roleService.listRoleRefs(any())).thenReturn(Flux.empty()); + } + @Test void permalinkForFakeUser() throws URISyntaxException { when(externalUrlSupplier.get()).thenReturn(new URI("http://localhost:8090")); @@ -52,10 +64,10 @@ class UserReconcilerTest { when(client.fetch(eq(User.class), eq("fake-user"))) .thenReturn(Optional.of(user("fake-user"))); userReconciler.reconcile(new Reconciler.Request("fake-user")); - verify(client, times(1)).update(any(User.class)); + verify(client, times(2)).update(any(User.class)); ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); - verify(client, times(1)).update(captor.capture()); + verify(client, times(2)).update(captor.capture()); assertThat(captor.getValue().getStatus().getPermalink()) .isEqualTo("http://localhost:8090/authors/fake-user"); } @@ -65,7 +77,35 @@ class UserReconcilerTest { when(client.fetch(eq(User.class), eq(AnonymousUserConst.PRINCIPAL))) .thenReturn(Optional.of(user(AnonymousUserConst.PRINCIPAL))); userReconciler.reconcile(new Reconciler.Request(AnonymousUserConst.PRINCIPAL)); - verify(client, times(0)).update(any(User.class)); + verify(client, times(1)).update(any(User.class)); + } + + @Test + void ensureRoleNamesAnno() { + RoleBinding.RoleRef roleRef = new RoleBinding.RoleRef(); + roleRef.setName("fake-role"); + roleRef.setKind(Role.KIND); + + roleRef.setApiGroup(Role.GROUP); + RoleBinding.RoleRef notworkRef = new RoleBinding.RoleRef(); + notworkRef.setName("super-role"); + notworkRef.setKind("Fake"); + notworkRef.setApiGroup("fake.halo.run"); + + RoleBinding.Subject subject = new RoleBinding.Subject(KIND, "fake-user", GROUP); + when(roleService.listRoleRefs(eq(subject))).thenReturn(Flux.just(roleRef, notworkRef)); + + when(client.fetch(eq(User.class), eq("fake-user"))) + .thenReturn(Optional.of(user("fake-user"))); + + when(externalUrlSupplier.get()).thenReturn(URI.create("/")); + + userReconciler.reconcile(new Reconciler.Request("fake-user")); + ArgumentCaptor captor = ArgumentCaptor.forClass(User.class); + verify(client, times(2)).update(captor.capture()); + User user = captor.getAllValues().get(1); + assertThat(user.getMetadata().getAnnotations().get(User.ROLE_NAMES_ANNO)) + .isEqualTo("[\"fake-role\"]"); } User user(String name) {