Removes commons lang usage by alecharp · Pull Request #693 · jenkinsci/active-directory-plugin

alecharp · 2026-01-19T15:47:43Z

Superseed #691.

This removes entirely any usage of commons lang.
The API plugin is still a transitive dependency coming from ionicons-api and configuration-as-code dependencies. Excluding it from the latter is breaking the build.

Testing done

I ran mvn verify locally.

Submitter checklist

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 demonstrate the feature works or the issue is fixed

mawinter69 · 2026-01-19T18:30:57Z

    protected UserDetails authenticate2(String username, String password) throws AuthenticationException {
        // Check if the password length is less than 14 characters
-        if(FIPS140.useCompliantAlgorithms() && StringUtils.length(password) < 14) {
+        if(FIPS140.useCompliantAlgorithms() && password.length() < 14) {


null check?

I don't think it can be null based on the call stack.

@mawinter69

Thanks @mawinter69 Co-Authored-By: Markus Winter <m.winter@sap.com>

@mawinter69

Thanks @mawinter69 Co-Authored-By: Markus Winter <m.winter@sap.com>

alecharp · 2026-01-23T09:46:28Z

e7374e3 from #677

Closes #677

gbhat618 · 2026-06-23T16:11:27Z

just checked the build - failed test seems to be a flaky (?) build on windows has already passed.
the merge conflict is a one-liner easy one..

Perhaps @alecharp could resolve this merge conflict, there by causing a new commit - thus kicking off a new build.

If all good, maybe we can merge this PR. (I am looking to the commons-lang2 removal)

Co-authored-by: James Nord <jtnord@users.noreply.github.com> Co-authored-by: Markus Winter <m.winter@sap.com>

gbhat618 · 2026-07-23T06:30:58Z

spotbugs failing the build here,

Redundant nullcheck of bindName, which is known to be non-null in hudson.plugins.active_directory.ActiveDirectoryDomain$DescriptorImpl.doValidateTest(String, String, String, String, String, TlsConfiguration, GroupLookupStrategy, boolean, boolean, boolean)

- Add null check for bindPassword before .length() in FIPS check - Remove redundant null check on bindName in doValidateTest (already guarded by early return for null/blank bindName) - Remove now-unused IOException import

gbhat618 · 2026-07-23T10:14:05Z

        this.name = name;
        // Gives exception if Password is set lees than 14 chars long in FIPS mode.
-        if(FIPS140.useCompliantAlgorithms() && StringUtils.length(bindPassword) < 14) {
+        if(FIPS140.useCompliantAlgorithms() && (bindName == null || bindPassword == null || bindPassword.length() < 14)) {


Suggested change

if(FIPS140.useCompliantAlgorithms() && (bindName == null || bindPassword == null || bindPassword.length() < 14)) {

if(FIPS140.useCompliantAlgorithms() && (bindPassword == null || bindPassword.length() < 14)) {

I contemplated about keeping the bindName == null check and making it part of the IllegalArgumentException but instead of combining it with the password check perhaps better to put another check for it.

given that bindName is only then used with fixEmpty(bindName) in this constructor code (other methods have appropriate checks), there is no need to introduce the null check again.

the fixEmpty honors null, doesn't result in NPE.

Safe to accept the suggestion removing bindName == null in here.

gbhat618 · 2026-07-23T10:19:50Z


        String bindPassword_ = Secret.toString(bindPassword);
-        if(FIPS140.useCompliantAlgorithms() && StringUtils.length(bindPassword_) < 14) {
+        if(FIPS140.useCompliantAlgorithms() && bindPassword_.length() < 14) {


Suggested change

if(FIPS140.useCompliantAlgorithms() && bindPassword_.length() < 14) {

if(FIPS140.useCompliantAlgorithms() && (bindPassword_ == null || bindPassword_.length() < 14)) {

returns 0 if null https://commons.apache.org/proper/commons-lang/apidocs/org/apache/commons/lang3/StringUtils.html#length(java.lang.CharSequence).

gbhat618 · 2026-07-23T10:20:23Z

        @RequirePOST
        public FormValidation doCheckBindPassword(@QueryParameter String bindPassword) {
-            if(FIPS140.useCompliantAlgorithms() && StringUtils.length(bindPassword) < 14) {
+            if(FIPS140.useCompliantAlgorithms() && bindPassword.length() < 14) {


Suggested change

if(FIPS140.useCompliantAlgorithms() && bindPassword.length() < 14) {

if(FIPS140.useCompliantAlgorithms() && (bindPassword == null || bindPassword.length() < 14)) {

null check required.

gbhat618 · 2026-07-23T10:24:10Z

-                    } catch (Exception e) {
-                        return FormValidation.error(e, e.getMessage());
-                    }
-                } else {


this was a dead else block, when bindName == null the code wouldn't have reached here, instead returned at L354.

gbhat618

lgtm with 2 minor comments.

jonesbusy and others added 2 commits January 15, 2026 17:39

Migrate Commons Lang from 2 to 3

511f439

Removes Commons Lang usage

4184637

alecharp mentioned this pull request Jan 19, 2026

Migrate Commons Lang from 2 to 3 #691

Closed

mawinter69 suggested changes Jan 19, 2026

View reviewed changes

alecharp and others added 2 commits January 20, 2026 11:04

Adds null checks

342a6b8

Thanks @mawinter69 Co-Authored-By: Markus Winter <m.winter@sap.com>

Adds null checks

d6757e2

Thanks @mawinter69 Co-Authored-By: Markus Winter <m.winter@sap.com>

alecharp mentioned this pull request Jan 23, 2026

Update dependency org.testcontainers:junit-jupiter to v1.21.4 #677

Open

1 task

Update dependency org.testcontainers:junit-jupiter to v1.21.4

e7374e3

alecharp requested a review from mawinter69 January 23, 2026 09:58

mawinter69 approved these changes Jan 23, 2026

View reviewed changes

jtnord added the developer label Jul 21, 2026

jtnord reviewed Jul 21, 2026

View reviewed changes

Comment thread src/main/java/hudson/plugins/active_directory/ActiveDirectoryAuthenticationProvider.java Outdated

mawinter69 suggested changes Jul 21, 2026

View reviewed changes

Comment thread src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java Outdated

Comment thread src/main/java/hudson/plugins/active_directory/ActiveDirectoryDomain.java Outdated

alecharp and others added 2 commits July 21, 2026 18:48

Apply suggestions from code review

023e5e6

Co-authored-by: James Nord <jtnord@users.noreply.github.com> Co-authored-by: Markus Winter <m.winter@sap.com>

Merge branch 'master' into chore/remove-commons-lang-usage

d37777e

gbhat618 reviewed Jul 23, 2026

View reviewed changes

fix: address review comments on commons-lang removal

370e950

- Add null check for bindPassword before .length() in FIPS check - Remove redundant null check on bindName in doValidateTest (already guarded by early return for null/blank bindName) - Remove now-unused IOException import

gbhat618 reviewed Jul 23, 2026

View reviewed changes

gbhat618 approved these changes Jul 23, 2026

View reviewed changes

	if(FIPS140.useCompliantAlgorithms() && (bindName == null \|\| bindPassword == null \|\| bindPassword.length() < 14)) {
	if(FIPS140.useCompliantAlgorithms() && (bindPassword == null \|\| bindPassword.length() < 14)) {

	if(FIPS140.useCompliantAlgorithms() && bindPassword_.length() < 14) {
	if(FIPS140.useCompliantAlgorithms() && (bindPassword_ == null \|\| bindPassword_.length() < 14)) {

Uh oh!

Conversation

alecharp commented Jan 19, 2026

Testing done

Submitter checklist

Uh oh!

Uh oh!

Uh oh!

mawinter69 Jan 19, 2026

Choose a reason for hiding this comment

Uh oh!

alecharp Jan 20, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

alecharp commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gbhat618 commented Jun 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gbhat618 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gbhat618 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gbhat618 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gbhat618 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gbhat618 Jul 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gbhat618 Jul 23, 2026

Choose a reason for hiding this comment

Uh oh!

gbhat618 left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

alecharp commented Jan 23, 2026 •

edited

Loading

gbhat618 Jul 23, 2026 •

edited

Loading