From e16fd4ebd6ca95ef7f1eafa02eb576cb295648ba Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Fri, 28 Dec 2018 15:26:46 +1100 Subject: [PATCH] Change missing authn message to not mention tokens (#36750) The phrase "missing authentication token" is historic and is based around the use of "AuthenticationToken" objects inside the Realm code. However, now that we have a TokenService and token API, this message would sometimes lead people in the wrong direction and they would try and generate a "token" for authentication purposes when they would typically just need a username:password Basic Auth header. This change replaces the word "token" with "credentials". --- .../DefaultAuthenticationFailureHandler.java | 4 +- ...aultAuthenticationFailureHandlerTests.java | 43 ++++++++++++------- .../authc/AuthenticationServiceTests.java | 2 +- .../test/upgraded_cluster/50_token_auth.yml | 2 +- .../packaging/tests/bootstrap_password.bash | 2 +- .../resources/packaging/tests/certgen.bash | 2 +- .../packaging/tests/setup_passwords.bash | 2 +- 7 files changed, 34 insertions(+), 23 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java index 4f548f80dd2..f27d95dc868 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java @@ -99,12 +99,12 @@ public class DefaultAuthenticationFailureHandler implements AuthenticationFailur @Override public ElasticsearchSecurityException missingToken(RestRequest request, ThreadContext context) { - return createAuthenticationError("missing authentication token for REST request [{}]", null, request.uri()); + return createAuthenticationError("missing authentication credentials for REST request [{}]", null, request.uri()); } @Override public ElasticsearchSecurityException missingToken(TransportMessage message, String action, ThreadContext context) { - return createAuthenticationError("missing authentication token for action [{}]", null, action); + return createAuthenticationError("missing authentication credentials for action [{}]", null, action); } @Override diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java index 07cdec4434a..ae779881573 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java @@ -13,7 +13,6 @@ import org.elasticsearch.rest.RestRequest; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.XPackField; -import org.mockito.Mockito; import java.util.Arrays; import java.util.Collections; @@ -26,6 +25,8 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.sameInstance; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { @@ -33,17 +34,17 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { final boolean testDefault = randomBoolean(); final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""; final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\""; - final DefaultAuthenticationFailureHandler failuerHandler; + final DefaultAuthenticationFailureHandler failureHandler; if (testDefault) { - failuerHandler = new DefaultAuthenticationFailureHandler(Collections.emptyMap()); + failureHandler = new DefaultAuthenticationFailureHandler(Collections.emptyMap()); } else { - final Map> failureResponeHeaders = new HashMap<>(); - failureResponeHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme)); - failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); + final Map> failureResponseHeaders = new HashMap<>(); + failureResponseHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme)); + failureHandler = new DefaultAuthenticationFailureHandler(failureResponseHeaders); } - assertThat(failuerHandler, is(notNullValue())); + assertThat(failureHandler, is(notNullValue())); final ElasticsearchSecurityException ese = - failuerHandler.authenticationRequired("someaction", new ThreadContext(Settings.builder().build())); + failureHandler.authenticationRequired("someaction", new ThreadContext(Settings.builder().build())); assertThat(ese, is(notNullValue())); assertThat(ese.getMessage(), equalTo("action [someaction] requires authentication")); assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); @@ -54,15 +55,25 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { } } + public void testMissingToken() { + final DefaultAuthenticationFailureHandler handler = new DefaultAuthenticationFailureHandler(Collections.emptyMap()); + final RestRequest request = mock(RestRequest.class); + when(request.uri()).thenReturn("https://secret.es.shield.gov/"); + final ThreadContext threadContext = new ThreadContext(Settings.EMPTY); + final ElasticsearchSecurityException ese = handler.missingToken(request, threadContext); + assertThat(ese, is(notNullValue())); + assertThat(ese.getMessage(), equalTo("missing authentication credentials for REST request [https://secret.es.shield.gov/]")); + } + public void testExceptionProcessingRequest() { final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""; final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\""; final String negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk"); - final Map> failureResponeHeaders = new HashMap<>(); - failureResponeHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme)); - final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); + final Map> failureResponseHeaders = new HashMap<>(); + failureResponseHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme)); + final DefaultAuthenticationFailureHandler failureHandler = new DefaultAuthenticationFailureHandler(failureResponseHeaders); - assertThat(failuerHandler, is(notNullValue())); + assertThat(failureHandler, is(notNullValue())); final boolean causeIsElasticsearchSecurityException = randomBoolean(); final boolean causeIsEseAndUnauthorized = causeIsElasticsearchSecurityException && randomBoolean(); final ElasticsearchSecurityException eseCause = (causeIsEseAndUnauthorized) @@ -77,7 +88,7 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { if (causeIsElasticsearchSecurityException) { if (causeIsEseAndUnauthorized) { - final ElasticsearchSecurityException ese = failuerHandler.exceptionProcessingRequest(Mockito.mock(RestRequest.class), cause, + final ElasticsearchSecurityException ese = failureHandler.exceptionProcessingRequest(mock(RestRequest.class), cause, new ThreadContext(Settings.builder().build())); assertThat(ese, is(notNullValue())); assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); @@ -93,11 +104,11 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { } assertThat(ese.getMessage(), equalTo("unauthorized")); } else { - expectThrows(AssertionError.class, () -> failuerHandler.exceptionProcessingRequest(Mockito.mock(RestRequest.class), cause, + expectThrows(AssertionError.class, () -> failureHandler.exceptionProcessingRequest(mock(RestRequest.class), cause, new ThreadContext(Settings.builder().build()))); } } else { - final ElasticsearchSecurityException ese = failuerHandler.exceptionProcessingRequest(Mockito.mock(RestRequest.class), cause, + final ElasticsearchSecurityException ese = failureHandler.exceptionProcessingRequest(mock(RestRequest.class), cause, new ThreadContext(Settings.builder().build())); assertThat(ese, is(notNullValue())); assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); @@ -117,7 +128,7 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { failureResponeHeaders.put("WWW-Authenticate", supportedSchemes); final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); - final ElasticsearchSecurityException ese = failuerHandler.exceptionProcessingRequest(Mockito.mock(RestRequest.class), null, + final ElasticsearchSecurityException ese = failuerHandler.exceptionProcessingRequest(mock(RestRequest.class), null, new ThreadContext(Settings.builder().build())); assertThat(ese, is(notNullValue())); diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java index 5e1fe282bdc..10740b33dc1 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java @@ -233,7 +233,7 @@ public class AuthenticationServiceTests extends ESTestCase { }); ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> future.actionGet()); - assertThat(e.getMessage(), containsString("missing authentication token")); + assertThat(e.getMessage(), containsString("missing authentication credentials")); verify(auditTrail).anonymousAccessDenied(reqId, "_action", message); verifyNoMoreInteractions(auditTrail); } diff --git a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml index fad78b2a085..a207c430860 100644 --- a/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml +++ b/x-pack/qa/rolling-upgrade/src/test/resources/rest-api-spec/test/upgraded_cluster/50_token_auth.yml @@ -36,7 +36,7 @@ - do: headers: Authorization: Bearer boom - catch: /missing authentication token/ + catch: /missing authentication credentials/ search: rest_total_hits_as_int: true index: token_index diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash index 587b782541d..2e62635b6ba 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/bootstrap_password.bash @@ -104,7 +104,7 @@ SETUP_OK false } - curl -s -XGET 'http://127.0.0.1:9200' | grep "missing authentication token for REST" + curl -s -XGET 'http://127.0.0.1:9200' | grep "missing authentication credentials for REST" # Disable bash history expansion because passwords can contain "!" set +H diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash index fbf0c9efcfc..1f81bfcbd49 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/certgen.bash @@ -362,7 +362,7 @@ DATA_SETTINGS echo "$output" false } - echo "$output" | grep "missing authentication token" + echo "$output" | grep "missing authentication credentials" } @test "[$GROUP] test node to node communication" { diff --git a/x-pack/qa/vagrant/src/test/resources/packaging/tests/setup_passwords.bash b/x-pack/qa/vagrant/src/test/resources/packaging/tests/setup_passwords.bash index c33947c1ac9..d892b379df4 100644 --- a/x-pack/qa/vagrant/src/test/resources/packaging/tests/setup_passwords.bash +++ b/x-pack/qa/vagrant/src/test/resources/packaging/tests/setup_passwords.bash @@ -62,7 +62,7 @@ SETUP_AUTO false } - curl -s -XGET localhost:9200 | grep "missing authentication token for REST" + curl -s -XGET localhost:9200 | grep "missing authentication credentials for REST" # Disable bash history expansion because passwords can contain "!" set +H