Do not swallow I/O exception getting authentication (#44398)
When getting authentication info from the thread context, it might be that we encounter an I/O exception. Today we swallow this exception and return a null authentication info to the caller. Yet, this could be hiding bugs or errors. This commits adjusts this behavior so that we no longer swallow the exception.
This commit is contained in:
parent
4b50de2e2e
commit
be98a12cd0
|
@ -5,8 +5,8 @@
|
|||
*/
|
||||
package org.elasticsearch.xpack.core.security;
|
||||
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.apache.logging.log4j.LogManager;
|
||||
import org.apache.logging.log4j.Logger;
|
||||
import org.elasticsearch.Version;
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
import org.elasticsearch.common.util.concurrent.ThreadContext;
|
||||
|
@ -17,6 +17,7 @@ import org.elasticsearch.xpack.core.security.authc.Authentication.Authentication
|
|||
import org.elasticsearch.xpack.core.security.user.User;
|
||||
|
||||
import java.io.IOException;
|
||||
import java.io.UncheckedIOException;
|
||||
import java.util.Collections;
|
||||
import java.util.Objects;
|
||||
import java.util.function.Consumer;
|
||||
|
@ -53,10 +54,8 @@ public class SecurityContext {
|
|||
try {
|
||||
return Authentication.readFromContext(threadContext);
|
||||
} catch (IOException e) {
|
||||
// TODO: this seems bogus, the only way to get an ioexception here is from a corrupt or tampered
|
||||
// auth header, which should be be audited?
|
||||
logger.error("failed to read authentication", e);
|
||||
return null;
|
||||
throw new UncheckedIOException(e);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
@ -15,13 +15,18 @@ import org.elasticsearch.xpack.core.security.SecurityContext;
|
|||
import org.elasticsearch.xpack.core.security.authc.Authentication;
|
||||
import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
|
||||
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
|
||||
import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
|
||||
import org.elasticsearch.xpack.core.security.user.SystemUser;
|
||||
import org.elasticsearch.xpack.core.security.user.User;
|
||||
import org.junit.Before;
|
||||
|
||||
import java.io.EOFException;
|
||||
import java.io.IOException;
|
||||
import java.io.UncheckedIOException;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import static org.hamcrest.Matchers.instanceOf;
|
||||
|
||||
public class SecurityContextTests extends ESTestCase {
|
||||
|
||||
private Settings settings;
|
||||
|
@ -51,6 +56,14 @@ public class SecurityContextTests extends ESTestCase {
|
|||
assertEquals(user, securityContext.getUser());
|
||||
}
|
||||
|
||||
public void testGetAuthenticationDoesNotSwallowIOException() {
|
||||
threadContext.putHeader(AuthenticationField.AUTHENTICATION_KEY, ""); // an intentionally corrupt header
|
||||
final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext);
|
||||
final UncheckedIOException e = expectThrows(UncheckedIOException.class, securityContext::getAuthentication);
|
||||
assertNotNull(e.getCause());
|
||||
assertThat(e.getCause(), instanceOf(EOFException.class));
|
||||
}
|
||||
|
||||
public void testSetUser() {
|
||||
final User user = new User("test");
|
||||
assertNull(securityContext.getAuthentication());
|
||||
|
|
Loading…
Reference in New Issue