diff --git a/server/src/main/java/org/elasticsearch/ElasticsearchException.java b/server/src/main/java/org/elasticsearch/ElasticsearchException.java index 8dbc4cf5ee7..0abc9f4848c 100644 --- a/server/src/main/java/org/elasticsearch/ElasticsearchException.java +++ b/server/src/main/java/org/elasticsearch/ElasticsearchException.java @@ -88,7 +88,7 @@ public class ElasticsearchException extends RuntimeException implements ToXConte private static final String REASON = "reason"; private static final String CAUSED_BY = "caused_by"; private static final ParseField SUPPRESSED = new ParseField("suppressed"); - private static final String STACK_TRACE = "stack_trace"; + public static final String STACK_TRACE = "stack_trace"; private static final String HEADER = "header"; private static final String ERROR = "error"; private static final String ROOT_CAUSE = "root_cause"; diff --git a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java index d6ed68bcafa..3babe434742 100644 --- a/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java +++ b/server/src/main/java/org/elasticsearch/rest/BytesRestResponse.java @@ -19,8 +19,8 @@ package org.elasticsearch.rest; -import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; import org.elasticsearch.ElasticsearchException; @@ -46,6 +46,8 @@ public class BytesRestResponse extends RestResponse { private static final String STATUS = "status"; + private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed"); + private final RestStatus status; private final BytesReference content; private final String contentType; @@ -92,8 +94,20 @@ public class BytesRestResponse extends RestResponse { } public BytesRestResponse(RestChannel channel, RestStatus status, Exception e) throws IOException { + ToXContent.Params params = paramsFromRequest(channel.request()); + if (params.paramAsBoolean(REST_EXCEPTION_SKIP_STACK_TRACE, REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) && e != null) { + // log exception only if it is not returned in the response + Supplier messageSupplier = () -> new ParameterizedMessage("path: {}, params: {}", + channel.request().rawPath(), channel.request().params()); + if (status.getStatus() < 500) { + SUPPRESSED_ERROR_LOGGER.debug(messageSupplier, e); + } else { + SUPPRESSED_ERROR_LOGGER.warn(messageSupplier, e); + } + } this.status = status; - try (XContentBuilder builder = build(channel, status, e)) { + try (XContentBuilder builder = channel.newErrorBuilder()) { + build(builder, params, status, channel.detailedErrorsEnabled(), e); this.content = BytesReference.bytes(builder); this.contentType = builder.contentType().mediaType(); } @@ -117,28 +131,24 @@ public class BytesRestResponse extends RestResponse { return this.status; } - private static final Logger SUPPRESSED_ERROR_LOGGER = LogManager.getLogger("rest.suppressed"); - - private static XContentBuilder build(RestChannel channel, RestStatus status, Exception e) throws IOException { - ToXContent.Params params = channel.request(); - if (params.paramAsBoolean("error_trace", !REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT)) { + private ToXContent.Params paramsFromRequest(RestRequest restRequest) { + ToXContent.Params params = restRequest; + if (params.paramAsBoolean("error_trace", !REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT) && false == skipStackTrace()) { params = new ToXContent.DelegatingMapParams(singletonMap(REST_EXCEPTION_SKIP_STACK_TRACE, "false"), params); - } else if (e != null) { - Supplier messageSupplier = () -> new ParameterizedMessage("path: {}, params: {}", - channel.request().rawPath(), channel.request().params()); - - if (status.getStatus() < 500) { - SUPPRESSED_ERROR_LOGGER.debug(messageSupplier, e); - } else { - SUPPRESSED_ERROR_LOGGER.warn(messageSupplier, e); - } } + return params; + } - XContentBuilder builder = channel.newErrorBuilder().startObject(); - ElasticsearchException.generateFailureXContent(builder, params, e, channel.detailedErrorsEnabled()); + protected boolean skipStackTrace() { + return false; + } + + private void build(XContentBuilder builder, ToXContent.Params params, RestStatus status, + boolean detailedErrorsEnabled, Exception e) throws IOException { + builder.startObject(); + ElasticsearchException.generateFailureXContent(builder, params, e, detailedErrorsEnabled); builder.field(STATUS, status.getStatus()); builder.endObject(); - return builder; } static BytesRestResponse createSimpleErrorResponse(RestChannel channel, RestStatus status, String errorMessage) throws IOException { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java index 40373d38193..d2327dcb3e8 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/rest/SecurityRestFilter.java @@ -9,6 +9,7 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.logging.log4j.util.Supplier; +import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -19,6 +20,7 @@ import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestRequest.Method; +import org.elasticsearch.rest.RestStatus; import org.elasticsearch.xpack.core.security.rest.RestRequestFilter; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authc.support.SecondaryAuthenticator; @@ -82,8 +84,14 @@ public class SecurityRestFilter implements RestHandler { private void handleException(String actionType, RestRequest request, RestChannel channel, Exception e) { logger.debug(new ParameterizedMessage("{} failed for REST request [{}]", actionType, request.uri()), e); + final RestStatus restStatus = ExceptionsHelper.status(e); try { - channel.sendResponse(new BytesRestResponse(channel, e)); + channel.sendResponse(new BytesRestResponse(channel, restStatus, e) { + + @Override + protected boolean skipStackTrace() { return restStatus == RestStatus.UNAUTHORIZED; } + + }); } catch (Exception inner) { inner.addSuppressed(e); logger.error((Supplier) () -> diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index 465a855112a..140d3e991e1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -164,7 +164,6 @@ public class SecurityTests extends ESTestCase { assertEquals("Realm type [" + FileRealmSettings.TYPE + "] is already registered", e.getMessage()); } - public void testAuditEnabled() throws Exception { Settings settings = Settings.builder().put(XPackSettings.AUDIT_ENABLED.getKey(), true).build(); Collection components = createComponents(settings); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java index 09f7a6b16d9..d4fc7e68415 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/rest/SecurityRestFilterTests.java @@ -7,6 +7,7 @@ package org.elasticsearch.xpack.security.rest; import com.nimbusds.jose.util.StandardCharset; import org.apache.lucene.util.SetOnce; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.ActionListener; import org.elasticsearch.client.node.NodeClient; import org.elasticsearch.common.bytes.BytesArray; @@ -22,6 +23,7 @@ import org.elasticsearch.rest.BytesRestResponse; import org.elasticsearch.rest.RestChannel; import org.elasticsearch.rest.RestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestResponse; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.SecuritySettingsSourceField; @@ -39,11 +41,15 @@ import org.mockito.ArgumentCaptor; import java.util.Base64; import java.util.Collections; +import java.util.HashMap; import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static org.elasticsearch.xpack.core.security.support.Exceptions.authenticationError; +import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; import static org.mockito.Matchers.any; @@ -141,21 +147,54 @@ public class SecurityRestFilterTests extends ESTestCase { verifyZeroInteractions(channel, authcService); } - public void testProcessAuthenticationError() throws Exception { - RestRequest request = mock(RestRequest.class); - Exception exception = authenticationError("failed authc"); + public void testProcessAuthenticationFailedNoTrace() throws Exception { + filter = new SecurityRestFilter(licenseState, threadContext, authcService, secondaryAuthenticator, restHandler, false); + testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " + + "cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, true, true, false); + testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " + + "cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, true, false, false); + testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " + + "cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, false, true, false); + testProcessAuthenticationFailed(randomBoolean() ? authenticationError("failed authn") : authenticationError("failed authn with " + + "cause", new ElasticsearchException("cause")), RestStatus.UNAUTHORIZED, false, false, false); + testProcessAuthenticationFailed(new ElasticsearchException("dummy"), RestStatus.INTERNAL_SERVER_ERROR, false, false, false); + testProcessAuthenticationFailed(new IllegalArgumentException("dummy"), RestStatus.BAD_REQUEST, true, false, false); + testProcessAuthenticationFailed(new ElasticsearchException("dummy"), RestStatus.INTERNAL_SERVER_ERROR, false, true, false); + testProcessAuthenticationFailed(new IllegalArgumentException("dummy"), RestStatus.BAD_REQUEST, true, true, true); + } + + private void testProcessAuthenticationFailed(Exception authnException, RestStatus expectedRestStatus, boolean errorTrace, + boolean detailedErrorsEnabled, boolean traceExists) throws Exception { + RestRequest request; + if (errorTrace != !ElasticsearchException.REST_EXCEPTION_SKIP_STACK_TRACE_DEFAULT || randomBoolean()) { + request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY) + .withParams(Collections.unmodifiableMap(new HashMap() {{ + put("error_trace", Boolean.toString(errorTrace)); + }})).build(); + } else { + // sometimes do not fill in the default value + request = new FakeRestRequest.Builder(NamedXContentRegistry.EMPTY).build(); + } doAnswer((i) -> { ActionListener callback = (ActionListener) i.getArguments()[1]; - callback.onFailure(exception); + callback.onFailure(authnException); return Void.TYPE; }).when(authcService).authenticate(eq(request), any(ActionListener.class)); + RestChannel channel = mock(RestChannel.class); + when(channel.detailedErrorsEnabled()).thenReturn(detailedErrorsEnabled); when(channel.request()).thenReturn(request); when(channel.newErrorBuilder()).thenReturn(JsonXContent.contentBuilder()); filter.handleRequest(request, channel, null); ArgumentCaptor response = ArgumentCaptor.forClass(BytesRestResponse.class); verify(channel).sendResponse(response.capture()); - assertEquals(RestStatus.UNAUTHORIZED, response.getValue().status()); + RestResponse restResponse = response.getValue(); + assertThat(restResponse.status(), is(expectedRestStatus)); + if (traceExists) { + assertThat(restResponse.content().utf8ToString(), containsString(ElasticsearchException.STACK_TRACE)); + } else { + assertThat(restResponse.content().utf8ToString(), not(containsString(ElasticsearchException.STACK_TRACE))); + } verifyZeroInteractions(restHandler); }