Skip to content

Commit

Permalink
CORE-18169: Change User password - db worker changes (#5302)
Browse files Browse the repository at this point in the history
This PR implements the db-worker side of the User password change feature in UserWriterImpl.kt

Because we can get the username from the request header, this also changes the endpoints:
{loginName}/otheruserpassword to /otheruserpassword
{loginName}/selfpassword to /selfpassword
This adds change password methods to ClusterBuilder to support future e2e tests.
  • Loading branch information
Tom-Fitzpatrick authored Dec 20, 2023
1 parent b5f873c commit a0b981d
Show file tree
Hide file tree
Showing 10 changed files with 200 additions and 122 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -108,12 +108,12 @@ class UserEndpointImpl @Activate constructor(
return userResponseDto?.convertToEndpointType() ?: throw ResourceNotFoundException("User", loginName)
}

override fun changeUserPasswordSelf(loginName: String, password: String): UserResponseType {
override fun changeUserPasswordSelf(password: String): UserResponseType {
val principal = getRestThreadLocalContext()

val userResponseDto = try {
withPermissionManager(permissionManagementService.permissionManager, logger) {
changeUserPasswordSelf(ChangeUserPasswordDto(principal, loginName, password))
changeUserPasswordSelf(ChangeUserPasswordDto(principal, principal.lowercase(), password))
}
} catch (e: IllegalArgumentException) {
throw InvalidStateChangeException(e.message ?: "New password must be different from old one.")
Expand All @@ -122,12 +122,12 @@ class UserEndpointImpl @Activate constructor(
return userResponseDto.convertToEndpointType()
}

override fun changeOtherUserPassword(loginName: String, password: String): UserResponseType {
override fun changeOtherUserPassword(username: String, password: String): UserResponseType {
val principal = getRestThreadLocalContext()

val userResponseDto = try {
withPermissionManager(permissionManagementService.permissionManager, logger) {
changeUserPasswordOther(ChangeUserPasswordDto(principal, loginName, password))
changeUserPasswordOther(ChangeUserPasswordDto(principal, username.lowercase(), password))
}
} catch (e: IllegalArgumentException) {
throw InvalidStateChangeException(e.message ?: "New password must be different from old one.")
Expand Down
2 changes: 1 addition & 1 deletion gradle.properties
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ commonsLangVersion = 3.12.0
commonsTextVersion = 1.10.0
# Corda API libs revision (change in 4th digit indicates a breaking change)
# Change to 5.2.0.xx-SNAPSHOT to pick up maven local published copy
cordaApiVersion=5.2.0.20-beta+
cordaApiVersion=5.2.0.21-beta+

disruptorVersion=3.4.4
felixConfigAdminVersion=1.9.26
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ interface UserEndpoint : RestResource {
): UserResponseType

@HttpPOST(
path = "{loginName}/selfpassword",
path = "/selfpassword",
description = "This method updates a users own password.",
responseDescription = """
A user with the following attributes:
Expand All @@ -143,8 +143,6 @@ interface UserEndpoint : RestResource {
minVersion = RestApiVersion.C5_2
)
fun changeUserPasswordSelf(
@RestPathParameter(description = "The login name of the user.")
loginName: String,
@ClientRequestBodyParameter(
description = "The new password to apply.",
required = true,
Expand All @@ -154,7 +152,7 @@ interface UserEndpoint : RestResource {
): UserResponseType

@HttpPOST(
path = "{loginName}/otheruserpassword",
path = "/otheruserpassword",
description = "This method updates another user's password, only usable by admin.",
responseDescription = """
A user with the following attributes:
Expand All @@ -175,8 +173,12 @@ interface UserEndpoint : RestResource {
minVersion = RestApiVersion.C5_2
)
fun changeOtherUserPassword(
@RestPathParameter(description = "The login name of the user who's password will be changed")
loginName: String,
@ClientRequestBodyParameter(
description = "Username for the password change.",
required = true,
name = "username"
)
username: String,
@ClientRequestBodyParameter(
description = "The new password to apply.",
required = true,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@ import net.corda.data.permissions.User
import net.corda.data.permissions.management.PermissionManagementRequest
import net.corda.data.permissions.management.PermissionManagementResponse
import net.corda.data.permissions.management.user.AddRoleToUserRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordOtherRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordSelfRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordRequest
import net.corda.data.permissions.management.user.CreateUserRequest
import net.corda.data.permissions.management.user.RemoveRoleFromUserRequest
import net.corda.libs.configuration.SmartConfig
Expand Down Expand Up @@ -77,28 +76,13 @@ class PermissionUserManagerImpl(
return cachedUser.convertToResponseDto()
}

override fun changeUserPasswordSelf(changeUserPasswordDto: ChangeUserPasswordDto): UserResponseDto {
val saltAndHash = validatePasswordAndGetHash(changeUserPasswordDto.username, changeUserPasswordDto.newPassword)

val result = sendPermissionWriteRequest<User>(
rpcSender,
writerTimeout,
PermissionManagementRequest(
changeUserPasswordDto.requestedBy,
null,
ChangeUserPasswordSelfRequest(
changeUserPasswordDto.requestedBy,
saltAndHash.salt,
saltAndHash.value,
Instant.now().plus(selfUserPasswordExpiryDays.toLong(), ChronoUnit.DAYS)
)
)
)
override fun changeUserPasswordSelf(changeUserPasswordDto: ChangeUserPasswordDto): UserResponseDto =
changeUserPassword(changeUserPasswordDto, selfUserPasswordExpiryDays)

return result.convertToResponseDto()
}
override fun changeUserPasswordOther(changeUserPasswordDto: ChangeUserPasswordDto): UserResponseDto =
changeUserPassword(changeUserPasswordDto, otherUserPasswordExpiryDays)

override fun changeUserPasswordOther(changeUserPasswordDto: ChangeUserPasswordDto): UserResponseDto {
private fun changeUserPassword(changeUserPasswordDto: ChangeUserPasswordDto, expiryDays: Int): UserResponseDto {
val saltAndHash = validatePasswordAndGetHash(changeUserPasswordDto.username, changeUserPasswordDto.newPassword)

val result = sendPermissionWriteRequest<User>(
Expand All @@ -107,12 +91,12 @@ class PermissionUserManagerImpl(
PermissionManagementRequest(
changeUserPasswordDto.requestedBy,
null,
ChangeUserPasswordOtherRequest(
ChangeUserPasswordRequest(
changeUserPasswordDto.requestedBy,
changeUserPasswordDto.username,
saltAndHash.salt,
saltAndHash.value,
Instant.now().plus(otherUserPasswordExpiryDays.toLong(), ChronoUnit.DAYS)
Instant.now().plus(expiryDays.toLong(), ChronoUnit.DAYS)
)
)
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import net.corda.data.permissions.management.role.AddPermissionToRoleRequest
import net.corda.data.permissions.management.role.CreateRoleRequest
import net.corda.data.permissions.management.role.RemovePermissionFromRoleRequest
import net.corda.data.permissions.management.user.AddRoleToUserRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordRequest
import net.corda.data.permissions.management.user.CreateUserRequest
import net.corda.data.permissions.management.user.RemoveRoleFromUserRequest
import net.corda.libs.permissions.storage.reader.PermissionStorageReader
Expand Down Expand Up @@ -49,6 +50,12 @@ class PermissionStorageWriterProcessorImpl(
permissionStorageReader.publishNewRole(avroRole)
avroRole
}
is ChangeUserPasswordRequest -> {
val avroUser = userWriter.changeUserPassword(permissionRequest, request.requestUserId)
permissionStorageReader.publishUpdatedUser(avroUser)
permissionStorageReader.reconcilePermissionSummaries()
avroUser
}
is CreatePermissionRequest -> {
val avroPermission = permissionWriter.createPermission(permissionRequest, request.requestUserId, request.virtualNodeId)
permissionStorageReader.publishNewPermission(avroPermission)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.corda.libs.permissions.storage.writer.impl.user

import net.corda.data.permissions.management.user.AddRoleToUserRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordRequest
import net.corda.data.permissions.management.user.CreateUserRequest
import net.corda.data.permissions.management.user.RemoveRoleFromUserRequest
import net.corda.data.permissions.User as AvroUser
Expand All @@ -17,6 +18,14 @@ interface UserWriter {
*/
fun createUser(request: CreateUserRequest, requestUserId: String): AvroUser

/**
* Change the password field of a User entity and return its Avro representation.
*
* @param request ChangeUserPasswordRequest containing the information of the password change request.
* @param requestUserId ID of the user who made the request.
*/
fun changeUserPassword(request: ChangeUserPasswordRequest, requestUserId: String): AvroUser

/**
* Associate a Role to a User and return its Avro representation.
*
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.corda.libs.permissions.storage.writer.impl.user.impl

import net.corda.data.permissions.management.user.AddRoleToUserRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordRequest
import net.corda.data.permissions.management.user.CreateUserRequest
import net.corda.data.permissions.management.user.RemoveRoleFromUserRequest
import net.corda.libs.permissions.storage.common.converter.toAvroUser
Expand Down Expand Up @@ -43,6 +44,37 @@ class UserWriterImpl(
}
}

override fun changeUserPassword(
request: ChangeUserPasswordRequest,
requestUserId: String
): net.corda.data.permissions.User {
log.debug { "Received request to change password for user: ${request.requestedBy}" }
return entityManagerFactory.transaction { entityManager ->

val validator = EntityValidationUtil(entityManager)
val user = validator.validateAndGetUniqueUser(request.requestedBy)

user.hashedPassword = request.hashedNewPassword
user.saltValue = request.saltValue
user.passwordExpiry = request.passwordExpiry

val updateTimestamp = Instant.now()
val auditLog = ChangeAudit(
id = UUID.randomUUID().toString(),
updateTimestamp = updateTimestamp,
actorUser = requestUserId,
changeType = RestPermissionOperation.USER_UPDATE,
details = "Password for user '${user.loginName}' changed by '$requestUserId'."
)

entityManager.merge(user)
entityManager.persist(auditLog)

log.info("Successfully changed password for user: ${user.loginName}.")
user.toAvroUser()
}
}

override fun addRoleToUser(request: AddRoleToUserRequest, requestUserId: String): AvroUser {
log.debug { "Received request to add Role ${request.roleId} to User ${request.loginName}" }
return entityManagerFactory.transaction { entityManager ->
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package net.corda.libs.permissions.storage.writer.impl.user

import net.corda.data.permissions.management.user.AddRoleToUserRequest
import net.corda.data.permissions.management.user.ChangeUserPasswordRequest
import net.corda.data.permissions.management.user.CreateUserRequest
import net.corda.data.permissions.management.user.RemoveRoleFromUserRequest
import net.corda.libs.permissions.common.exception.EntityAlreadyExistsException
Expand Down Expand Up @@ -150,6 +151,51 @@ internal class UserWriterImplTest {
assertEquals(requestUserId, audit.actorUser)
}

@Test
fun `changing user password successfully changes password`() {
// Arrange
val changeUserPasswordRequest = ChangeUserPasswordRequest().apply {
requestedBy = "existingUser"
hashedNewPassword = "newHashedPassword"
saltValue = "newSalt"
passwordExpiry = Instant.now()
}

val existingUser = User(
id = "userId",
fullName = "Existing User",
loginName = "existingUser",
enabled = true,
hashedPassword = "oldHashedPassword",
saltValue = "oldSalt",
passwordExpiry = Instant.now(),
updateTimestamp = Instant.now(),
parentGroup = mock<Group>()
)

val typedQueryMock = mock<TypedQuery<User>>()
whenever(entityManager.createQuery(any<String>(), eq(User::class.java))).thenReturn(typedQueryMock)
whenever(typedQueryMock.setParameter(eq("loginName"), eq("existingUser"))).thenReturn(typedQueryMock)
whenever(typedQueryMock.resultList).thenReturn(listOf(existingUser))

userWriter.changeUserPassword(changeUserPasswordRequest, requestUserId)

verify(entityManager).merge(existingUser)
assertEquals("newHashedPassword", existingUser.hashedPassword)
assertEquals("newSalt", existingUser.saltValue)

val auditCaptor = argumentCaptor<ChangeAudit>()
verify(entityManager).persist(auditCaptor.capture())

val capturedAudit = auditCaptor.firstValue
assertNotNull(capturedAudit)
assertEquals(RestPermissionOperation.USER_UPDATE, capturedAudit.changeType)
assertEquals("Password for user 'existingUser' changed by '$requestUserId'.", capturedAudit.details)

verify(entityTransaction).begin()
verify(entityTransaction).commit()
}

@Test
fun `add role to user fails when user does not exist`() {
whenever(entityManager.createQuery(any(), eq(User::class.java))).thenReturn(userQuery)
Expand Down
Loading

0 comments on commit a0b981d

Please sign in to comment.