Skip to content
This repository has been archived by the owner on Dec 10, 2023. It is now read-only.

[JENKINS-68708] CrowdSecurityRealm: Prevent trim() calls on null strings #98

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

proski
Copy link
Contributor

@proski proski commented Aug 8, 2022

*  [JENKINS-68708] CrowdSecurityRealm: Prevent trim() calls on null strings
   
   Replace trim() with StringUtils.trimToEmpty(), it trims strings as well
   and also converts null to an empty string.
   
   Don't use trim() in the compatibility constructor. Passwords should be
   used without trimming. Secret.fromString(null) returns a valid object.
   
   Add unit tests for CrowdSecurityRealm constructors.
   
   Signed-off-by: Pavel Roskin <plroskin@gmail.com>

https://issues.jenkins.io/browse/JENKINS-68708

  • Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • Ensure that the pull request title represents the desired changelog entry
  • Please describe what you did
  • Link to relevant issues in GitHub or Jira
  • Link to relevant pull requests, esp. upstream and downstream changes
  • Ensure you have provided tests - that demonstrates feature works or fixes the issue

Manual testing performed:

  • Ran "mvn verify" successfully
  • Deployed the resulting crowd2.hpi to a Jenkins 2.346.2 server
  • Removed cookieTokenkey, group and httpProxyPassword from jenkins.yaml
  • Loaded the modified jenkins.yaml
  • Logged out and logged in (crowd2 is used for authentication)
  • Downloaded jenkins.yaml
  • cookieTokenkey, group and httpProxyPassword are not in the downloaded jenkins.yaml as exprected
  • Checked Jenkins log for any potential problems, did not find any.

Replace trim() with StringUtils.trimToEmpty(), it trims strings as well
and also converts null to an empty string.

Don't use trim() in the compatibility constructor. Passwords should be
used without trimming. Secret.fromString(null) returns a valid object.

Add unit tests for CrowdSecurityRealm constructors.

Signed-off-by: Pavel Roskin <plroskin@gmail.com>
@DuMaM DuMaM merged commit 87ae6f3 into jenkinsci:master Aug 8, 2022
@DuMaM
Copy link
Contributor

DuMaM commented Aug 9, 2022

Thanks for this pr.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants