Browse Source

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.
Jason Tedor 6 years ago
parent
commit
d8f9e02604

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

@@ -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);
         }
     }
 

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

@@ -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());