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

ARTEMIS-2192 fix listener for LegacyLDAPSecuritySettingPlugin #2516

Closed
wants to merge 1 commit into from

Conversation

jbertram
Copy link
Contributor

No description provided.

if (logger.isDebugEnabled()) {
logger.debug("\tSkipping unexpected search result with " + rdns.size() + " RDNs.");
}
return;
}
// we can count on the RNDs being in order from right to left
Rdn rdn = rdns.get(0);
Rdn rdn = rdns.get(rdns.size() - 3);
Copy link
Contributor

Choose a reason for hiding this comment

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

Hands up this isnt something im super hot on, and im not entirely sure what this code is doing and what expected results or ldap setup must be followed (so please choose to ignore this)

but i do know from LDAP its possible to get duplicate rdns, so what occurs if rdns is > 3 and a dn is duplicated

e.g.

uid=john.doe,ou=People,dc=example,dc=com

dc is twice here, what do you need? also would mean you may miss the dn you do want maybe the in this case, would be uid.

Copy link
Contributor

Choose a reason for hiding this comment

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

tbh the older, hard coded rdns made a bit more sense to me, at least i know what rdn are being expected and looked for, and ordering (to handle duplicates in response) wouldnt be an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The problem with the hard-coded rdns is that the plugin won't work for anybody who doesn't use the same values. I don't see duplicates as being a problem with this code. It simply takes the first three rdns and interprets them as permission-type, destination-name, & destination-type. It doesn't care about the rdns past that.

Copy link
Contributor

Choose a reason for hiding this comment

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

fair enough

@asfgit asfgit closed this in f76a4a9 Jan 24, 2019
@jbertram jbertram deleted the ARTEMIS-2192 branch June 10, 2021 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants