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

Change logging in normalizeDn() to debug to avoid noisy warnings #2599

Merged
merged 3 commits into from Aug 3, 2016

Conversation

@bernd
Copy link
Member

@bernd bernd commented Aug 3, 2016

Group entries might be invalid DNs, which is okay. This avoids noisy WARN logging when handling groups.

Example:

2016-08-03 12:24:34,114 WARN : org.graylog2.security.ldap.LdapConnector - Invalid DN
org.apache.directory.api.ldap.model.exception.LdapInvalidDnException: ERR_04202 A value is missing on some RDN
    at org.apache.directory.api.ldap.model.name.Dn.<init>(Dn.java:283) ~[api-all-1.0.0-RC1.jar:1.0.0-RC1]
    at org.apache.directory.api.ldap.model.name.Dn.<init>(Dn.java:215) ~[api-all-1.0.0-RC1.jar:1.0.0-RC1]
    at org.graylog2.security.ldap.LdapConnector.normalizedDn(LdapConnector.java:321) [classes/:?]
    at org.graylog2.security.ldap.LdapConnector.findGroups(LdapConnector.java:276) [classes/:?]
    at org.graylog2.security.ldap.LdapConnector.search(LdapConnector.java:200) [classes/:?]
    at org.graylog2.security.realm.LdapUserAuthenticator.doGetAuthenticationInfo(LdapUserAuthenticator.java:119) [classes/:?]
    at org.apache.shiro.realm.AuthenticatingRealm.getAuthenticationInfo(AuthenticatingRealm.java:568) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.authc.pam.ModularRealmAuthenticator.doMultiRealmAuthentication(ModularRealmAuthenticator.java:219) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.authc.pam.ModularRealmAuthenticator.doAuthenticate(ModularRealmAuthenticator.java:269) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.authc.AbstractAuthenticator.authenticate(AbstractAuthenticator.java:198) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.mgt.AuthenticatingSecurityManager.authenticate(AuthenticatingSecurityManager.java:106) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.mgt.DefaultSecurityManager.login(DefaultSecurityManager.java:270) [shiro-core-1.3.0.jar:1.3.0]
    at org.apache.shiro.subject.support.DelegatingSubject.login(DelegatingSubject.java:256) [shiro-core-1.3.0.jar:1.3.0]
    at org.graylog2.rest.resources.system.SessionsResource.newSession(SessionsResource.java:128) [classes/:?]
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[?:1.8.0_101]
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[?:1.8.0_101]
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[?:1.8.0_101]
    at java.lang.reflect.Method.invoke(Method.java:498) ~[?:1.8.0_101]
    at org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory$1.invoke(ResourceMethodInvocationHandlerFactory.java:81) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:144) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:161) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$TypeOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:205) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:99) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:389) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:347) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:102) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.ServerRuntime$2.run(ServerRuntime.java:326) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.internal.Errors$1.call(Errors.java:271) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.internal.Errors$1.call(Errors.java:267) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.internal.Errors.process(Errors.java:315) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.internal.Errors.process(Errors.java:297) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.internal.Errors.process(Errors.java:267) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:317) [jersey-common-2.22.1.jar:?]
    at org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:305) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:1154) [jersey-server-2.22.1.jar:?]
    at org.glassfish.jersey.grizzly2.httpserver.GrizzlyHttpContainer.service(GrizzlyHttpContainer.java:384) [jersey-container-grizzly2-http-2.22.1.jar:?]
    at org.glassfish.grizzly.http.server.HttpHandler$1.run(HttpHandler.java:224) [grizzly-http-server-2.3.23.jar:2.3.23]
    at com.codahale.metrics.InstrumentedExecutorService$InstrumentedRunnable.run(InstrumentedExecutorService.java:176) [metrics-core-3.1.2.jar:3.1.2]
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142) [?:1.8.0_101]
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617) [?:1.8.0_101]
    at java.lang.Thread.run(Thread.java:745) [?:1.8.0_101]
Group entries might be invalid DNs, which is okay. This avoids noisy
WARN logging when handling groups.
@bernd bernd added this to the 2.1.0 milestone Aug 3, 2016
@@ -326,7 +326,7 @@ private String normalizedDn(String dn) {
try {
return new Dn(dn).getNormName();
} catch (LdapInvalidDnException e) {
LOG.warn("Invalid DN", e);
LOG.debug("Invalid DN", e);

This comment has been minimized.

@joschi

joschi Aug 3, 2016
Contributor

I'm totally for this change but I fear that the handling inside the catch block will leave people clueless why stuff didn't work.

The caller of this method has basically no idea whether he really got back a normalized DN or an invalid DN. 😞

This comment has been minimized.

@bernd

bernd Aug 3, 2016
Author Member

Any idea how to solve this?

This comment has been minimized.

@bernd

bernd Aug 3, 2016
Author Member

Maybe the method is not named correctly. We want to use this to:

  1. normalize DN strings if the passed value is a DN
  2. if it's not a DN we do not have to normalize it and just return the original

We need this because in the following case the member attribute might be a DN string or a regular string like just the username.

LOG.trace("DN {} == {} member?", dn, member.getString());
if (dn.equalsIgnoreCase(normalizedDn(member.getString()))) {
    groups.add(groupId);
} else {

We do not really care if it's a DN or not. The important part is that if the value is a DN it should be normalized.

This comment has been minimized.

@joschi

joschi Aug 3, 2016
Contributor

Fair enough. Could you please add a short Javadoc comment to the normalizeDn() method which says exactly this?

This way future me at least won't scratch his head about it. 😉

This comment has been minimized.

@bernd

bernd Aug 3, 2016
Author Member

Aye, done! 😄

@joschi joschi self-assigned this Aug 3, 2016
@joschi
Copy link
Contributor

@joschi joschi commented Aug 3, 2016

LGTM. 👍

@joschi joschi merged commit fecb562 into master Aug 3, 2016
4 checks passed
4 checks passed
@garybot2
ci-server-integration Jenkins build graylog2-server-integration-pr 1213 has succeeded
Details
@garybot2
ci-web-linter Jenkins build graylog-pr-linter-check 696 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 fix-verbose-ldap-logging branch Aug 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants