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 c0b3b7e16..cc357691d 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 @@ -46,6 +46,73 @@ public class UserEndpoint implements CustomEndpoint { this.userService = userService; } + @Override + public RouterFunction endpoint() { + var tag = "api.halo.run/v1alpha1/User"; + return SpringdocRouteBuilder.route() + .GET("/users/-", this::me, builder -> builder.operationId("GetCurrentUserDetail") + .description("Get current user detail") + .tag(tag) + .response(responseBuilder().implementation(User.class))) + .POST("/users/{name}/permissions", this::grantPermission, + builder -> builder.operationId("GrantPermission") + .description("Grant permissions to user") + .tag(tag) + .parameter(parameterBuilder().in(ParameterIn.PATH).name("name") + .description("User name") + .required(true)) + .requestBody(requestBodyBuilder() + .required(true) + .implementation(GrantRequest.class)) + .response(responseBuilder().implementation(User.class))) + .GET("/users/{name}/permissions", this::getUserPermission, + builder -> builder.operationId("GetPermissions") + .description("Get permissions of user") + .tag(tag) + .parameter(parameterBuilder().in(ParameterIn.PATH).name("name") + .description("User name") + .required(true)) + .response(responseBuilder().implementation(UserPermission.class))) + .PUT("/users/{name}/password", this::changePassword, + builder -> builder.operationId("ChangePassword") + .description("Change password of user.") + .tag(tag) + .parameter(parameterBuilder().in(ParameterIn.PATH).name("name") + .description( + "Name of user. If the name is equal to '-', it will change the " + + "password of current user.") + .required(true)) + .requestBody(requestBodyBuilder() + .required(true) + .implementation(ChangePasswordRequest.class)) + .response(responseBuilder() + .implementation(User.class)) + ) + .build(); + } + + Mono changePassword(ServerRequest request) { + final var nameInPath = request.pathVariable("name"); + return ReactiveSecurityContextHolder.getContext() + .map(ctx -> "-".equals(nameInPath) ? ctx.getAuthentication().getName() : nameInPath) + .flatMap(username -> request.bodyToMono(ChangePasswordRequest.class) + .switchIfEmpty(Mono.defer(() -> + Mono.error(new ServerWebInputException("Request body is empty")))) + .flatMap(changePasswordRequest -> { + var password = changePasswordRequest.password(); + // encode password + return userService.updateWithRawPassword(username, password); + })) + .flatMap(updatedUser -> ServerResponse.ok() + .contentType(MediaType.APPLICATION_JSON) + .bodyValue(updatedUser)); + } + + record ChangePasswordRequest( + @Schema(description = "New password.", required = true, minLength = 6) + String password) { + } + @NonNull Mono me(ServerRequest request) { return ReactiveSecurityContextHolder.getContext() @@ -63,7 +130,8 @@ public class UserEndpoint implements CustomEndpoint { Mono grantPermission(ServerRequest request) { var username = request.pathVariable("name"); return request.bodyToMono(GrantRequest.class) - .switchIfEmpty(Mono.error(() -> new ServerWebInputException("Request body is empty"))) + .switchIfEmpty( + Mono.error(() -> new ServerWebInputException("Request body is empty"))) .flatMap(grant -> { // preflight check client.fetch(User.class, username) @@ -111,35 +179,6 @@ public class UserEndpoint implements CustomEndpoint { record GrantRequest(Set roles) { } - @Override - public RouterFunction endpoint() { - var tag = "api.halo.run/v1alpha1/User"; - return SpringdocRouteBuilder.route() - .GET("/users/-", this::me, builder -> builder.operationId("GetCurrentUserDetail") - .description("Get current user detail") - .tag(tag) - .response(responseBuilder().implementation(User.class))) - .POST("/users/{name}/permissions", this::grantPermission, - builder -> builder.operationId("GrantPermission") - .description("Grant permissions to user") - .tag(tag) - .parameter(parameterBuilder().in(ParameterIn.PATH).name("name") - .description("User name") - .required(true)) - .requestBody( - requestBodyBuilder().required(true).implementation(GrantRequest.class)) - .response(responseBuilder().implementation(User.class))) - .GET("/users/{name}/permissions", this::getUserPermission, - builder -> builder.operationId("GetPermissions") - .description("Get permissions of user") - .tag(tag) - .parameter(parameterBuilder().in(ParameterIn.PATH).name("name") - .description("User name") - .required(true)) - .response(responseBuilder().implementation(UserPermission.class))) - .build(); - } - @NonNull private Mono getUserPermission(ServerRequest request) { String name = request.pathVariable("name"); @@ -150,11 +189,11 @@ public class UserEndpoint implements CustomEndpoint { }) .map(roles -> { Set uiPermissions = roles.stream() - .map(role -> role.getMetadata().getAnnotations()) - .filter(Objects::nonNull) - .map(this::mergeUiPermissions) - .flatMap(Set::stream) - .collect(Collectors.toSet()); + .map(role -> role.getMetadata().getAnnotations()) + .filter(Objects::nonNull) + .map(this::mergeUiPermissions) + .flatMap(Set::stream) + .collect(Collectors.toSet()); return new UserPermission(roles, uiPermissions); }) .flatMap(result -> ServerResponse.ok() diff --git a/src/main/java/run/halo/app/core/extension/service/UserService.java b/src/main/java/run/halo/app/core/extension/service/UserService.java index 679839581..8928dbab3 100644 --- a/src/main/java/run/halo/app/core/extension/service/UserService.java +++ b/src/main/java/run/halo/app/core/extension/service/UserService.java @@ -9,7 +9,10 @@ public interface UserService { Mono getUser(String username); + @Deprecated Mono updatePassword(String username, String newPassword); + Mono updateWithRawPassword(String username, String rawPassword); + Flux listRoles(String username); } diff --git a/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java b/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java index ff86bea66..4a5f70da9 100644 --- a/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java +++ b/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java @@ -3,7 +3,10 @@ package run.halo.app.core.extension.service; import static run.halo.app.core.extension.RoleBinding.containsUser; import java.util.Objects; +import java.util.function.Predicate; +import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; +import org.springframework.util.StringUtils; import reactor.core.publisher.Flux; import reactor.core.publisher.Mono; import run.halo.app.core.extension.Role; @@ -16,8 +19,11 @@ public class UserServiceImpl implements UserService { private final ExtensionClient client; - public UserServiceImpl(ExtensionClient client) { + private final PasswordEncoder passwordEncoder; + + public UserServiceImpl(ExtensionClient client, PasswordEncoder passwordEncoder) { this.client = client; + this.passwordEncoder = passwordEncoder; } @Override @@ -35,6 +41,27 @@ public class UserServiceImpl implements UserService { .then(); } + @Override + public Mono updateWithRawPassword(String username, String rawPassword) { + return getUser(username) + .filter(Predicate.not(hasPassword().and(passwordMatches(rawPassword)))) + .flatMap(user -> { + // TODO Validate the password + user.getSpec().setPassword(passwordEncoder.encode(rawPassword)); + client.update(user); + // get the latest user + return getUser(username); + }); + } + + private Predicate hasPassword() { + return user -> StringUtils.hasText(user.getSpec().getPassword()); + } + + private Predicate passwordMatches(String rawPassword) { + return user -> passwordEncoder.matches(rawPassword, user.getSpec().getPassword()); + } + @Override public Flux listRoles(String name) { return Flux.fromStream(client.list(RoleBinding.class, containsUser(name), null) 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 6bd0df748..0e84a0736 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 @@ -1,7 +1,6 @@ package run.halo.app.core.extension.endpoint; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isA; @@ -16,6 +15,7 @@ import java.util.List; import java.util.Optional; import java.util.Set; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -26,6 +26,7 @@ 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.RoleBinding; import run.halo.app.core.extension.User; @@ -37,6 +38,7 @@ import run.halo.app.infra.utils.JsonUtils; @SpringBootTest @AutoConfigureWebTestClient +@WithMockUser(username = "fake-user", password = "fake-password", roles = "fake-super-role") class UserEndpointTest { @Autowired @@ -61,41 +63,79 @@ class UserEndpointTest { .build(); var role = new Role(); role.setRules(List.of(rule)); - when(roleService.getRole(anyString())).thenReturn(role); - // prevent from initializing the super admin. - when(client.fetch(User.class, "admin")).thenReturn(Optional.of(mock(User.class))); - } - - @Test - @WithMockUser("fake-user") - void shouldResponseErrorIfUserNotFound() { - when(client.fetch(User.class, "fake-user")).thenReturn(Optional.empty()); - webClient.get().uri("/apis/api.halo.run/v1alpha1/users/-") - .exchange() - .expectStatus().is5xxServerError(); - } - - @Test - @WithMockUser("fake-user") - void shouldGetCurrentUserDetail() { - var metadata = new Metadata(); - metadata.setName("fake-user"); - var user = new User(); - user.setMetadata(metadata); - when(client.fetch(User.class, "fake-user")).thenReturn(Optional.of(user)); - webClient.get().uri("/apis/api.halo.run/v1alpha1/users/-") - .exchange() - .expectStatus().isOk() - .expectHeader().contentType(MediaType.APPLICATION_JSON) - .expectBody(User.class) - .isEqualTo(user); + when(roleService.getRole("fake-super-role")).thenReturn(role); } @Nested + @DisplayName("GetUserDetail") + class GetUserDetailTest { + + @Test + void shouldResponseErrorIfUserNotFound() { + when(client.fetch(User.class, "fake-user")).thenReturn(Optional.empty()); + webClient.get().uri("/apis/api.halo.run/v1alpha1/users/-") + .exchange() + .expectStatus().is5xxServerError(); + } + + @Test + void shouldGetCurrentUserDetail() { + var metadata = new Metadata(); + metadata.setName("fake-user"); + var user = new User(); + user.setMetadata(metadata); + when(client.fetch(User.class, "fake-user")).thenReturn(Optional.of(user)); + webClient.get().uri("/apis/api.halo.run/v1alpha1/users/-") + .exchange() + .expectStatus().isOk() + .expectHeader().contentType(MediaType.APPLICATION_JSON) + .expectBody(User.class) + .isEqualTo(user); + } + } + + @Nested + @DisplayName("ChangePassword") + class ChangePasswordTest { + + @Test + void shouldUpdateMyPasswordCorrectly() { + var user = new User(); + when(userService.updateWithRawPassword("fake-user", "new-password")) + .thenReturn(Mono.just(user)); + webClient.put().uri("/apis/api.halo.run/v1alpha1/users/-/password") + .bodyValue(new UserEndpoint.ChangePasswordRequest("new-password")) + .exchange() + .expectStatus().isOk() + .expectBody(User.class) + .isEqualTo(user); + + verify(userService, times(1)).updateWithRawPassword("fake-user", "new-password"); + } + + @Test + void shouldUpdateOtherPasswordCorrectly() { + var user = new User(); + when(userService.updateWithRawPassword("another-fake-user", "new-password")) + .thenReturn(Mono.just(user)); + webClient.put().uri("/apis/api.halo.run/v1alpha1/users/another-fake-user/password") + .bodyValue(new UserEndpoint.ChangePasswordRequest("new-password")) + .exchange() + .expectStatus().isOk() + .expectBody(User.class) + .isEqualTo(user); + + verify(userService, times(1)).updateWithRawPassword("another-fake-user", + "new-password"); + } + + } + + @Nested + @DisplayName("GrantPermission") class GrantPermissionEndpointTest { @Test - @WithMockUser("fake-user") void shouldGetBadRequestIfRequestBodyIsEmpty() { webClient.post().uri("/apis/api.halo.run/v1alpha1/users/fake-user/permissions") .contentType(MediaType.APPLICATION_JSON) @@ -108,7 +148,6 @@ class UserEndpointTest { } @Test - @WithMockUser("fake-user") void shouldGetNotFoundIfUserNotFound() { when(client.fetch(User.class, "fake-user")).thenReturn(Optional.empty()); when(client.fetch(Role.class, "fake-role")).thenReturn(Optional.of(mock(Role.class))); @@ -124,7 +163,6 @@ class UserEndpointTest { } @Test - @WithMockUser("fake-user") void shouldGetNotFoundIfRoleNotFound() { when(client.fetch(User.class, "fake-user")).thenReturn(Optional.of(mock(User.class))); when(client.fetch(Role.class, "fake-role")).thenReturn(Optional.empty()); @@ -140,7 +178,6 @@ class UserEndpointTest { } @Test - @WithMockUser("fake-user") void shouldCreateRoleBindingIfNotExist() { when(client.fetch(User.class, "fake-user")).thenReturn(Optional.of(mock(User.class))); var role = mock(Role.class); @@ -160,7 +197,6 @@ class UserEndpointTest { } @Test - @WithMockUser("fake-user") void shouldDeleteRoleBindingIfNotProvided() { when(client.fetch(User.class, "fake-user")).thenReturn(Optional.of(mock(User.class))); var role = mock(Role.class); @@ -179,12 +215,11 @@ class UserEndpointTest { verify(client, times(1)).create(RoleBinding.create("fake-user", "fake-role")); verify(client, times(1)) .delete(argThat(binding -> binding.getMetadata().getName() - .equals(roleBinding.getMetadata().getName()))); + .equals(roleBinding.getMetadata().getName()))); verify(client, never()).update(isA(RoleBinding.class)); } @Test - @WithMockUser("fake-user") void shouldGetPermission() { Role roleA = JsonUtils.jsonToObject(""" { diff --git a/src/test/java/run/halo/app/core/extension/service/UserServiceImplTest.java b/src/test/java/run/halo/app/core/extension/service/UserServiceImplTest.java index 63cb52354..e6afe226f 100644 --- a/src/test/java/run/halo/app/core/extension/service/UserServiceImplTest.java +++ b/src/test/java/run/halo/app/core/extension/service/UserServiceImplTest.java @@ -2,20 +2,28 @@ package run.halo.app.core.extension.service; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.lenient; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.spy; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.util.List; import java.util.Optional; +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; import org.mockito.junit.jupiter.MockitoExtension; +import org.springframework.security.crypto.password.PasswordEncoder; +import reactor.core.publisher.Mono; import reactor.test.StepVerifier; import run.halo.app.core.extension.Role; import run.halo.app.core.extension.RoleBinding; @@ -30,6 +38,9 @@ class UserServiceImplTest { @Mock ExtensionClient client; + @Mock + PasswordEncoder passwordEncoder; + @InjectMocks UserServiceImpl userService; @@ -194,4 +205,88 @@ class UserServiceImplTest { JsonUtils.jsonToObject(bindB, RoleBinding.class), JsonUtils.jsonToObject(bindC, RoleBinding.class)); } + + @Nested + @DisplayName("UpdateWithRawPassword") + class UpdateWithRawPasswordTest { + + @Test + void shouldUpdatePasswordWithDifferentPassword() { + userService = spy(userService); + + doReturn( + Mono.just(createUser("fake-password")), + Mono.just(createUser("new-password"))) + .when(userService) + .getUser("fake-user"); + when(passwordEncoder.matches("new-password", "fake-password")).thenReturn(false); + StepVerifier.create(userService.updateWithRawPassword("fake-user", "new-password")) + .expectNext(createUser("new-password")) + .verifyComplete(); + + verify(passwordEncoder, times(1)).matches("new-password", "fake-password"); + verify(passwordEncoder, times(1)).encode("new-password"); + verify(userService, times(2)).getUser("fake-user"); + } + + @Test + void shouldUpdatePasswordIfNoPasswordBefore() { + userService = spy(userService); + + doReturn( + Mono.just(createUser("")), + Mono.just(createUser("new-password"))) + .when(userService) + .getUser("fake-user"); + StepVerifier.create(userService.updateWithRawPassword("fake-user", "new-password")) + .expectNext(createUser("new-password")) + .verifyComplete(); + + verify(passwordEncoder, never()).matches(anyString(), anyString()); + verify(passwordEncoder, times(1)).encode("new-password"); + verify(userService, times(2)).getUser("fake-user"); + } + + @Test + void shouldDoNothingIfPasswordNotChanged() { + userService = spy(userService); + + doReturn( + Mono.just(createUser("fake-password")), + Mono.just(createUser("new-password"))) + .when(userService) + .getUser("fake-user"); + when(passwordEncoder.matches("fake-password", "fake-password")).thenReturn(true); + + StepVerifier.create(userService.updateWithRawPassword("fake-user", "fake-password")) + .expectNextCount(0) + .verifyComplete(); + + verify(passwordEncoder, times(1)).matches("fake-password", "fake-password"); + verify(passwordEncoder, never()).encode("fake-password"); + verify(userService, times(1)).getUser("fake-user"); + } + + @Test + void shouldDoNothingIfUserNotFound() { + userService = spy(userService); + + doReturn(Mono.empty()).when(userService).getUser("fake-user"); + StepVerifier.create(userService.updateWithRawPassword("fake-user", "new-password")) + .expectNextCount(0) + .verifyComplete(); + + verify(passwordEncoder, never()).matches(anyString(), anyString()); + verify(passwordEncoder, never()).encode(anyString()); + verify(userService, times(1)).getUser(anyString()); + } + + User createUser(String password) { + var user = new User(); + user.setSpec(new User.UserSpec()); + user.getSpec().setPassword(password); + return user; + } + } + }