-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
[JENKINS-74858] Added validation for Password length in FIPS mode #9995
base: master
Are you sure you want to change the base?
Conversation
Yay, your first pull request towards Jenkins core was created successfully! Thank you so much! |
core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice if newly added tests covered the success case (long + matching password) in addition to not relying on mocks.
test/src/test/java/hudson/security/HudsonPrivateSecurityRealmFIPSTest.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have spotless issues, just execute mvn clean install -DskipTests
to see and fix them
(haven't see them clearly in the CI logs, but they seem to be related to my comments about the empty line and the wildcard)
core/src/main/java/hudson/security/HudsonPrivateSecurityRealm.java
Outdated
Show resolved
Hide resolved
test/src/test/java/hudson/security/HudsonPrivateSecurityRealmFIPSTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/hudson/security/HudsonPrivateSecurityRealmFIPSTest.java
Outdated
Show resolved
Hide resolved
test/src/test/java/hudson/security/HudsonPrivateSecurityRealmFIPSTest.java
Show resolved
Hide resolved
test/src/test/java/hudson/security/HudsonPrivateSecurityRealmFIPSTest.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
See JENKINS-74858.
Made changes to ensure that an exisiting user is now unable to alter the password to less than 14 characters in length when operating in FIPS mode.
Testing done
Automated Unit Testing
Implemented unit tests to verify FIPS compliance for password changes:
Manual Testing
Did Manual testing to verify my changes to ensure that everything is working properly.
Without FIPS mode: Shorter password is being accepted.
With FIPS mode: Shorter password is not being accepted.
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@mention
Before the changes are marked as
ready-for-merge
:Maintainer checklist