Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CORE-18169: Change User password - db worker changes #5302

Merged

Conversation

Tom-Fitzpatrick
Copy link
Contributor

@Tom-Fitzpatrick Tom-Fitzpatrick commented Dec 19, 2023

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.

Corda-api changes to support this: corda/corda-api#1418

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR title failed to match regex -> ^((CORDA|EG|ENT|INFRA|CORE|DOC|ES|DA5)-\d+)(.*)

@Tom-Fitzpatrick Tom-Fitzpatrick changed the title Tomf/core 18169/change password db worker side CORE-18169/change password db worker side Dec 19, 2023
@github-actions github-actions bot dismissed their stale review December 19, 2023 15:12

All good!

@Tom-Fitzpatrick Tom-Fitzpatrick force-pushed the tomf/CORE-18169/change-password-db-worker-side branch from 017a0b2 to 8493993 Compare December 19, 2023 16:13
@Tom-Fitzpatrick Tom-Fitzpatrick changed the title CORE-18169/change password db worker side CORE-18169: Change User password - db worker changes Dec 19, 2023
@Tom-Fitzpatrick Tom-Fitzpatrick force-pushed the tomf/CORE-18169/change-password-db-worker-side branch from 6ef05c3 to 678e4c8 Compare December 20, 2023 13:38
@corda-jenkins-ci02
Copy link
Contributor

corda-jenkins-ci02 bot commented Dec 20, 2023

Jenkins build for PR 5302 build 12

Build Successful:
Jar artifact version produced by this PR: 5.2.0.0-alpha-1703089412336
Helm chart version produced by this PR: 5.2.0-alpha.1703089412336
Helm chart pushed to: oci://corda-os-docker-dev.software.r3.com/helm-charts/pr-5302/corda

@@ -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-alpha-1703075132231
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will update this to point to the beta, once the corresponding corda-api PR is merged
corda/corda-api#1418

Copy link
Contributor

@vkolomeyko vkolomeyko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine to me, minor comment posted.

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

println("old password hash: ${user.hashedPassword}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
println("old password hash: ${user.hashedPassword}")
log.debug { "old password hash: ${user.hashedPassword}" }

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah thanks, I'll just remove this actually as it was left over from when I was debugging it

@@ -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-alpha-1703075132231
Copy link
Contributor

@vkolomeyko vkolomeyko Dec 20, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Will need to revert this change once corda-api PR is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged the corda-api PR and updated this config

Signed-off-by: TomFitzpatrick <[email protected]>
Signed-off-by: TomFitzpatrick <[email protected]>
Signed-off-by: TomFitzpatrick <[email protected]>
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@Tom-Fitzpatrick Tom-Fitzpatrick requested a review from a team December 20, 2023 15:59
@vkolomeyko vkolomeyko self-requested a review December 20, 2023 17:17
Copy link
Contributor

@vkolomeyko vkolomeyko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Tom-Fitzpatrick Tom-Fitzpatrick merged commit a0b981d into release/os/5.2 Dec 20, 2023
2 checks passed
@Tom-Fitzpatrick Tom-Fitzpatrick deleted the tomf/CORE-18169/change-password-db-worker-side branch December 20, 2023 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants