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".
This commit is contained in:
Tim Vernum 2018-12-28 15:26:46 +11:00 committed by GitHub
parent 51c18c0e76
commit e16fd4ebd6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 34 additions and 23 deletions

View File

@ -99,12 +99,12 @@ public class DefaultAuthenticationFailureHandler implements AuthenticationFailur
@Override @Override
public ElasticsearchSecurityException missingToken(RestRequest request, ThreadContext context) { 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 @Override
public ElasticsearchSecurityException missingToken(TransportMessage message, String action, ThreadContext context) { 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 @Override

View File

@ -13,7 +13,6 @@ import org.elasticsearch.rest.RestRequest;
import org.elasticsearch.rest.RestStatus; import org.elasticsearch.rest.RestStatus;
import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.core.XPackField; import org.elasticsearch.xpack.core.XPackField;
import org.mockito.Mockito;
import java.util.Arrays; import java.util.Arrays;
import java.util.Collections; 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.is;
import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.sameInstance; import static org.hamcrest.Matchers.sameInstance;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.when;
public class DefaultAuthenticationFailureHandlerTests extends ESTestCase { public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
@ -33,17 +34,17 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
final boolean testDefault = randomBoolean(); final boolean testDefault = randomBoolean();
final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""; final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\""; final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
final DefaultAuthenticationFailureHandler failuerHandler; final DefaultAuthenticationFailureHandler failureHandler;
if (testDefault) { if (testDefault) {
failuerHandler = new DefaultAuthenticationFailureHandler(Collections.emptyMap()); failureHandler = new DefaultAuthenticationFailureHandler(Collections.emptyMap());
} else { } else {
final Map<String, List<String>> failureResponeHeaders = new HashMap<>(); final Map<String, List<String>> failureResponseHeaders = new HashMap<>();
failureResponeHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme)); failureResponseHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme));
failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); failureHandler = new DefaultAuthenticationFailureHandler(failureResponseHeaders);
} }
assertThat(failuerHandler, is(notNullValue())); assertThat(failureHandler, is(notNullValue()));
final ElasticsearchSecurityException ese = 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, is(notNullValue()));
assertThat(ese.getMessage(), equalTo("action [someaction] requires authentication")); assertThat(ese.getMessage(), equalTo("action [someaction] requires authentication"));
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); 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() { public void testExceptionProcessingRequest() {
final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""; final String basicAuthScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\""; final String bearerAuthScheme = "Bearer realm=\"" + XPackField.SECURITY + "\"";
final String negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk"); final String negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk");
final Map<String, List<String>> failureResponeHeaders = new HashMap<>(); final Map<String, List<String>> failureResponseHeaders = new HashMap<>();
failureResponeHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme)); failureResponseHeaders.put("WWW-Authenticate", Arrays.asList(basicAuthScheme, bearerAuthScheme, negotiateAuthScheme));
final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); final DefaultAuthenticationFailureHandler failureHandler = new DefaultAuthenticationFailureHandler(failureResponseHeaders);
assertThat(failuerHandler, is(notNullValue())); assertThat(failureHandler, is(notNullValue()));
final boolean causeIsElasticsearchSecurityException = randomBoolean(); final boolean causeIsElasticsearchSecurityException = randomBoolean();
final boolean causeIsEseAndUnauthorized = causeIsElasticsearchSecurityException && randomBoolean(); final boolean causeIsEseAndUnauthorized = causeIsElasticsearchSecurityException && randomBoolean();
final ElasticsearchSecurityException eseCause = (causeIsEseAndUnauthorized) final ElasticsearchSecurityException eseCause = (causeIsEseAndUnauthorized)
@ -77,7 +88,7 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
if (causeIsElasticsearchSecurityException) { if (causeIsElasticsearchSecurityException) {
if (causeIsEseAndUnauthorized) { 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())); new ThreadContext(Settings.builder().build()));
assertThat(ese, is(notNullValue())); assertThat(ese, is(notNullValue()));
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
@ -93,11 +104,11 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
} }
assertThat(ese.getMessage(), equalTo("unauthorized")); assertThat(ese.getMessage(), equalTo("unauthorized"));
} else { } 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()))); new ThreadContext(Settings.builder().build())));
} }
} else { } 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())); new ThreadContext(Settings.builder().build()));
assertThat(ese, is(notNullValue())); assertThat(ese, is(notNullValue()));
assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue())); assertThat(ese.getHeader("WWW-Authenticate"), is(notNullValue()));
@ -117,7 +128,7 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
failureResponeHeaders.put("WWW-Authenticate", supportedSchemes); failureResponeHeaders.put("WWW-Authenticate", supportedSchemes);
final DefaultAuthenticationFailureHandler failuerHandler = new DefaultAuthenticationFailureHandler(failureResponeHeaders); 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())); new ThreadContext(Settings.builder().build()));
assertThat(ese, is(notNullValue())); assertThat(ese, is(notNullValue()));

View File

@ -233,7 +233,7 @@ public class AuthenticationServiceTests extends ESTestCase {
}); });
ElasticsearchSecurityException e = expectThrows(ElasticsearchSecurityException.class, () -> future.actionGet()); 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); verify(auditTrail).anonymousAccessDenied(reqId, "_action", message);
verifyNoMoreInteractions(auditTrail); verifyNoMoreInteractions(auditTrail);
} }

View File

@ -36,7 +36,7 @@
- do: - do:
headers: headers:
Authorization: Bearer boom Authorization: Bearer boom
catch: /missing authentication token/ catch: /missing authentication credentials/
search: search:
rest_total_hits_as_int: true rest_total_hits_as_int: true
index: token_index index: token_index

View File

@ -104,7 +104,7 @@ SETUP_OK
false 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 "!" # Disable bash history expansion because passwords can contain "!"
set +H set +H

View File

@ -362,7 +362,7 @@ DATA_SETTINGS
echo "$output" echo "$output"
false false
} }
echo "$output" | grep "missing authentication token" echo "$output" | grep "missing authentication credentials"
} }
@test "[$GROUP] test node to node communication" { @test "[$GROUP] test node to node communication" {

View File

@ -62,7 +62,7 @@ SETUP_AUTO
false 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 "!" # Disable bash history expansion because passwords can contain "!"
set +H set +H