Skip to content

Commit

Permalink
feat: add original password verification for password change (#5748)
Browse files Browse the repository at this point in the history 
* feat: add original password verification for password change

* chore: update properties file

* Refine ui

Signed-off-by: Ryan Wang <i@ryanc.cc>

* chore: update properties file

* fix: confirm assword

* fix: unit test case

* feat: add new api for change own password

* chore: regenerate api client

* chore: adapt to UI

* chore: enusre old password not blank

---------

Signed-off-by: Ryan Wang <i@ryanc.cc>
Co-authored-by: Ryan Wang <i@ryanc.cc>
  • Loading branch information
@guqing @ruibaby
guqing and ruibaby authored Apr 21, 2024
1 parent 36b63d6 commit f07c09b
Show file tree
Hide file tree
Showing 17 changed files with 329 additions and 41 deletions.
61 changes: 55 additions & 6 deletions api-docs/openapi/v3_0/aggregated.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4173,6 +4173,37 @@
]
}
},
"/apis/api.console.halo.run/v1alpha1/users/-/password": {
"put": {
"description": "Change own password of user.",
"operationId": "ChangeOwnPassword",
"requestBody": {
"content": {
"*/*": {
"schema": {
"$ref": "#/components/schemas/ChangeOwnPasswordRequest"
}
}
},
"required": true
},
"responses": {
"default": {
"content": {
"*/*": {
"schema": {
"$ref": "#/components/schemas/User"
}
}
},
"description": "default response"
}
},
"tags": [
"api.console.halo.run/v1alpha1/User"
]
}
},
"/apis/api.console.halo.run/v1alpha1/users/-/send-email-verification-code": {
"post": {
"description": "Send email verification code for user",
Expand Down Expand Up @@ -4329,8 +4360,8 @@
},
"/apis/api.console.halo.run/v1alpha1/users/{name}/password": {
"put": {
"description": "Change password of user.",
"operationId": "ChangePassword",
"description": "Change anyone password of user for admin.",
"operationId": "ChangeAnyonePassword",
"parameters": [
{
"description": "Name of user. If the name is equal to \u0027-\u0027, it will change the password of current user.",
Expand Down Expand Up @@ -13152,6 +13183,24 @@
}
}
},
"ChangeOwnPasswordRequest": {
"required": [
"oldPassword",
"password"
],
"type": "object",
"properties": {
"oldPassword": {
"type": "string",
"description": "Old password."
},
"password": {
"minLength": 6,
"type": "string",
"description": "New password."
}
}
},
"ChangePasswordRequest": {
"required": [
"password"
Expand Down Expand Up @@ -16913,12 +16962,12 @@
},
"visible": {
"type": "string",
"default": "PUBLIC",
"enum": [
"PUBLIC",
"INTERNAL",
"PRIVATE"
],
"default": "PUBLIC"
]
}
}
},
Expand Down Expand Up @@ -18531,12 +18580,12 @@
},
"visible": {
"type": "string",
"default": "PUBLIC",
"enum": [
"PUBLIC",
"INTERNAL",
"PRIVATE"
],
"default": "PUBLIC"
]
}
}
},
Expand Down
53 changes: 49 additions & 4 deletions application/src/main/java/run/halo/app/core/extension/endpoint/UserEndpoint.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@
import run.halo.app.infra.SystemSetting;
import run.halo.app.infra.ValidationUtils;
import run.halo.app.infra.exception.RateLimitExceededException;
import run.halo.app.infra.exception.UnsatisfiedAttributeValueException;
import run.halo.app.infra.utils.JsonUtils;
import run.halo.app.security.authentication.twofactor.TwoFactorAuthentication;

Expand Down Expand Up @@ -160,9 +161,19 @@ public RouterFunction<ServerResponse> endpoint() {
.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.")
.PUT("/users/-/password", this::changeOwnPassword,
builder -> builder.operationId("ChangeOwnPassword")
.description("Change own password of user.")
.tag(tag)
.requestBody(requestBodyBuilder()
.required(true)
.implementation(ChangeOwnPasswordRequest.class))
.response(responseBuilder()
.implementation(User.class))
)
.PUT("/users/{name}/password", this::changeAnyonePasswordForAdmin,
builder -> builder.operationId("ChangeAnyonePassword")
.description("Change anyone password of user for admin.")
.tag(tag)
.parameter(parameterBuilder().in(ParameterIn.PATH).name("name")
.description(
Expand Down Expand Up @@ -520,7 +531,7 @@ private Mono<ServerResponse> updateProfile(ServerRequest request) {
.flatMap(updatedUser -> ServerResponse.ok().bodyValue(updatedUser));
}

Mono<ServerResponse> changePassword(ServerRequest request) {
Mono<ServerResponse> changeAnyonePasswordForAdmin(ServerRequest request) {
final var nameInPath = request.pathVariable("name");
return ReactiveSecurityContextHolder.getContext()
.map(ctx -> SELF_USER.equals(nameInPath) ? ctx.getAuthentication().getName()
Expand All @@ -538,6 +549,40 @@ Mono<ServerResponse> changePassword(ServerRequest request) {
.bodyValue(updatedUser));
}

Mono<ServerResponse> changeOwnPassword(ServerRequest request) {
return ReactiveSecurityContextHolder.getContext()
.map(ctx -> ctx.getAuthentication().getName())
.flatMap(username -> request.bodyToMono(ChangeOwnPasswordRequest.class)
.switchIfEmpty(Mono.defer(() ->
Mono.error(new ServerWebInputException("Request body is empty"))))
.flatMap(changePasswordRequest -> {
var rawOldPassword = changePasswordRequest.oldPassword();
return userService.confirmPassword(username, rawOldPassword)
.filter(Boolean::booleanValue)
.switchIfEmpty(Mono.error(new UnsatisfiedAttributeValueException(
"Old password is incorrect.",
"problemDetail.user.oldPassword.notMatch",
null))
)
.thenReturn(changePasswordRequest);
})
.flatMap(changePasswordRequest -> {
var password = changePasswordRequest.password();
// encode password
return userService.updateWithRawPassword(username, password);
}))
.flatMap(updatedUser -> ServerResponse.ok()
.contentType(MediaType.APPLICATION_JSON)
.bodyValue(updatedUser));
}

record ChangeOwnPasswordRequest(
@Schema(description = "Old password.", requiredMode = REQUIRED)
String oldPassword,
@Schema(description = "New password.", requiredMode = REQUIRED, minLength = 6)
String password) {
}

record ChangePasswordRequest(
@Schema(description = "New password.", requiredMode = REQUIRED, minLength = 6)
String password) {
Expand Down
2 changes: 2 additions & 0 deletions application/src/main/java/run/halo/app/core/extension/service/UserService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,6 @@ public interface UserService {
Mono<User> signUp(User user, String password);

Mono<User> createUser(User user, Set<String> roles);

Mono<Boolean> confirmPassword(String username, String rawPassword);
}
17 changes: 17 additions & 0 deletions application/src/main/java/run/halo/app/core/extension/service/UserServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,21 @@ public Mono<User> createUser(User user, Set<String> roleNames) {
.flatMap(newUser -> grantRoles(user.getMetadata().getName(), roleNames)))
);
}

@Override
public Mono<Boolean> confirmPassword(String username, String rawPassword) {

return getUser(username)
.filter(user -> {
if (!StringUtils.hasText(user.getSpec().getPassword())) {
// If the password is not set, return true directly.
return true;
}
if (!StringUtils.hasText(rawPassword)) {
return false;
}
return passwordEncoder.matches(rawPassword, user.getSpec().getPassword());
})
.hasElement();
}
}
1 change: 1 addition & 0 deletions application/src/main/resources/config/i18n/messages.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ problemDetail.index.duplicateKey=The value of {0} already exists for unique inde
problemDetail.user.email.verify.maxAttempts=Too many verification attempts, please try again later.
problemDetail.user.password.unsatisfied=The password does not meet the specifications.
problemDetail.user.username.unsatisfied=The username does not meet the specifications.
problemDetail.user.oldPassword.notMatch=The old password does not match.
problemDetail.user.signUpFailed.disallowed=System does not allow new users to register.
problemDetail.user.duplicateName=The username {0} already exists, please rename it and retry.
problemDetail.comment.turnedOff=The comment function has been turned off.
Expand Down
1 change: 1 addition & 0 deletions application/src/main/resources/config/i18n/messages_zh.properties
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ problemDetail.index.duplicateKey=唯一索引 {1} 中的值 {0} 已存在，请
problemDetail.user.email.verify.maxAttempts=尝试次数过多，请稍候再试。
problemDetail.user.password.unsatisfied=密码不符合规范。
problemDetail.user.username.unsatisfied=用户名不符合规范。
problemDetail.user.oldPassword.notMatch=旧密码不匹配。
problemDetail.user.signUpFailed.disallowed=系统不允许注册新用户。
problemDetail.user.duplicateName=用户名 {0} 已存在，请更换用户名后重试。
problemDetail.plugin.version.unsatisfied.requires=插件要求一个最小的系统版本为 {0}, 但当前版本为 {1}。
Expand Down
10 changes: 8 additions & 2 deletions application/src/test/java/run/halo/app/core/extension/endpoint/UserEndpointTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -306,8 +306,11 @@ void shouldUpdateMyPasswordCorrectly() {
var user = new User();
when(userService.updateWithRawPassword("fake-user", "new-password"))
.thenReturn(Mono.just(user));
when(userService.confirmPassword("fake-user", "old-password"))
.thenReturn(Mono.just(true));
webClient.put().uri("/users/-/password")
.bodyValue(new UserEndpoint.ChangePasswordRequest("new-password"))
.bodyValue(
new UserEndpoint.ChangeOwnPasswordRequest("old-password", "new-password"))
.exchange()
.expectStatus().isOk()
.expectBody(User.class)
Expand All @@ -319,11 +322,14 @@ void shouldUpdateMyPasswordCorrectly() {
@Test
void shouldUpdateOtherPasswordCorrectly() {
var user = new User();
when(userService.confirmPassword("another-fake-user", "old-password"))
.thenReturn(Mono.just(true));
when(userService.updateWithRawPassword("another-fake-user", "new-password"))
.thenReturn(Mono.just(user));
webClient.put()
.uri("/users/another-fake-user/password")
.bodyValue(new UserEndpoint.ChangePasswordRequest("new-password"))
.bodyValue(
new UserEndpoint.ChangeOwnPasswordRequest("old-password", "new-password"))
.exchange()
.expectStatus().isOk()
.expectBody(User.class)
Expand Down
17 changes: 17 additions & 0 deletions application/src/test/java/run/halo/app/core/extension/service/UserServiceImplTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -485,4 +485,21 @@ User fakeSignUpUser(String name, String password) {
return user;
}
}

@Test
void confirmPasswordWhenPasswordNotSet() {
var user = new User();
user.setSpec(new User.UserSpec());
when(client.get(User.class, "fake-user")).thenReturn(Mono.just(user));
userService.confirmPassword("fake-user", "fake-password")
.as(StepVerifier::create)
.expectNext(true)
.verifyComplete();

user.getSpec().setPassword("");
userService.confirmPassword("fake-user", "fake-password")
.as(StepVerifier::create)
.expectNext(true)
.verifyComplete();
}
}
2 changes: 1 addition & 1 deletion ui/console-src/modules/system/users/components/UserPasswordChangeModal.vue
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ const handleChangePassword = async () => {
const changePasswordRequest = cloneDeep(formState.value);
delete changePasswordRequest.password_confirm;
await apiClient.user.changePassword({
await apiClient.user.changeAnyonePassword({
name: props.user?.metadata.name || "",
changePasswordRequest,
});
Expand Down
1 change: 1 addition & 0 deletions ui/packages/api-client/src/.openapi-generator/FILES
View file
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ models/category-status.ts
models/category-vo-list.ts
models/category-vo.ts
models/category.ts
models/change-own-password-request.ts
models/change-password-request.ts
models/comment-email-owner.ts
models/comment-list.ts
Expand Down
Loading

0 comments on commit f07c09b

Please sign in to comment.