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

Fix group membership lookup if DN contains whitespace #2484

Merged
merged 3 commits into from Jul 15, 2016

Conversation

Projects
None yet
2 participants
@bernd
Member

bernd commented Jul 14, 2016

Normalize the DN before comparing the strings to avoid mapping problems if the DNs have different whitespace formatting.

Fixes #1790

Fix group membership lookup if DN contains whitespace
Normalize the DN before comparing the strings to avoid mapping problems
if the DNs have different whitespace formatting.

Fixes #1790

@bernd bernd added this to the 2.1.0 milestone Jul 14, 2016

@bernd bernd added the ldap label Jul 14, 2016

@joschi joschi self-assigned this Jul 15, 2016

return new Dn(dn).getNormName();
} catch (LdapInvalidDnException e) {
LOG.warn("Invalid DN", e);
return dn;

This comment has been minimized.

@joschi

joschi Jul 15, 2016

Contributor

Shouldn't this be throwing an exception instead of returning the arguably invalid DN back to the caller?

This comment has been minimized.

@bernd

bernd Jul 15, 2016

Member

I didn't want to change the current behavior in case of an invalid DN. If we throw an exception here, we have to handle it somewhere or let it bubble up.

This comment has been minimized.

@joschi

joschi Jul 15, 2016

Contributor

@@ -35,6 +35,12 @@ objectClass: top
cn: Engineers
uniqueMember: cn=John Doe,ou=users,dc=example,dc=com

dn: cn=Whitespace Engineers,ou=groups,dc=example,dc=com

This comment has been minimized.

@joschi

joschi Jul 15, 2016

Contributor

Whitespace Engineers – Engineering something you can't see since 1942 (also non-breaking). 😆

* @param dn denormalized DN string
* @return normalized DN string
*/
private String normalizedDn(String dn) {

This comment has been minimized.

@joschi

joschi Jul 15, 2016

Contributor

The method could be annotated with @Nullable.

@joschi

This comment has been minimized.

Contributor

joschi commented Jul 15, 2016

LGTM. 👍

@joschi joschi merged commit 90a110b into master Jul 15, 2016

4 checks passed

ci-server-integration Jenkins build graylog2-server-integration-pr 1097 has succeeded
Details
ci-web-linter Jenkins build graylog-pr-linter-check 583 has succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@joschi joschi deleted the issue-1790 branch Jul 15, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment