Skip to content

Commit d8f9e02

Browse files
authored
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.
1 parent f86bb25 commit d8f9e02

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityContext.java

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
*/
66
package org.elasticsearch.xpack.core.security;
77

8-
import org.apache.logging.log4j.Logger;
98
import org.apache.logging.log4j.LogManager;
9+
import org.apache.logging.log4j.Logger;
1010
import org.elasticsearch.Version;
1111
import org.elasticsearch.common.settings.Settings;
1212
import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -17,6 +17,7 @@
1717
import org.elasticsearch.xpack.core.security.user.User;
1818

1919
import java.io.IOException;
20+
import java.io.UncheckedIOException;
2021
import java.util.Collections;
2122
import java.util.Objects;
2223
import java.util.function.Consumer;
@@ -53,10 +54,8 @@ public Authentication getAuthentication() {
5354
try {
5455
return Authentication.readFromContext(threadContext);
5556
} catch (IOException e) {
56-
// TODO: this seems bogus, the only way to get an ioexception here is from a corrupt or tampered
57-
// auth header, which should be be audited?
5857
logger.error("failed to read authentication", e);
59-
return null;
58+
throw new UncheckedIOException(e);
6059
}
6160
}
6261

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityContextTests.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,18 @@
1515
import org.elasticsearch.xpack.core.security.authc.Authentication;
1616
import org.elasticsearch.xpack.core.security.authc.Authentication.AuthenticationType;
1717
import org.elasticsearch.xpack.core.security.authc.Authentication.RealmRef;
18+
import org.elasticsearch.xpack.core.security.authc.AuthenticationField;
1819
import org.elasticsearch.xpack.core.security.user.SystemUser;
1920
import org.elasticsearch.xpack.core.security.user.User;
2021
import org.junit.Before;
2122

23+
import java.io.EOFException;
2224
import java.io.IOException;
25+
import java.io.UncheckedIOException;
2326
import java.util.concurrent.atomic.AtomicReference;
2427

28+
import static org.hamcrest.Matchers.instanceOf;
29+
2530
public class SecurityContextTests extends ESTestCase {
2631

2732
private Settings settings;
@@ -51,6 +56,14 @@ public void testGetAuthenticationAndUser() throws IOException {
5156
assertEquals(user, securityContext.getUser());
5257
}
5358

59+
public void testGetAuthenticationDoesNotSwallowIOException() {
60+
threadContext.putHeader(AuthenticationField.AUTHENTICATION_KEY, ""); // an intentionally corrupt header
61+
final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext);
62+
final UncheckedIOException e = expectThrows(UncheckedIOException.class, securityContext::getAuthentication);
63+
assertNotNull(e.getCause());
64+
assertThat(e.getCause(), instanceOf(EOFException.class));
65+
}
66+
5467
public void testSetUser() {
5568
final User user = new User("test");
5669
assertNull(securityContext.getAuthentication());

0 commit comments

Comments
 (0)