Add comma before charset parameter in WWW-Authenticate response hea…

…der (elastic#110906) Add comma before charset parameter in WWW-Authenticate response header, according to RFC 7617.
albertzaharovits · Jul 22, 2024 · 1a95da5 · 1a95da5
1 parent 4a705e4
commit 1a95da5
Show file tree

Hide file tree

Showing 10 changed files with 20 additions and 15 deletions.
diff --git a/docs/changelog/110906.yaml b/docs/changelog/110906.yaml
@@ -0,0 +1,5 @@
+pr: 110906
+summary: "Add comma before charset parameter in WWW-Authenticate response header"
+area: Authentication
+type: bug
+issues: []
diff --git a/...java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java b/...java/org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandler.java
@@ -43,7 +43,7 @@ public DefaultAuthenticationFailureHandler(final Map<String, List<String>> failu
         if (failureResponseHeaders == null || failureResponseHeaders.isEmpty()) {
             this.defaultFailureResponseHeaders = Collections.singletonMap(
                 "WWW-Authenticate",
-                Collections.singletonList("Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"")
+                Collections.singletonList("Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"")
             );
         } else {
             this.defaultFailureResponseHeaders = Collections.unmodifiableMap(

diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Realm.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/Realm.java
@@ -69,7 +69,7 @@ public int order() {
     public Map<String, List<String>> getAuthenticationFailureHeaders() {
         return Collections.singletonMap(
             "WWW-Authenticate",
-            Collections.singletonList("Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"")
+            Collections.singletonList("Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"")
         );
     }
 

diff --git a/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Exceptions.java b/...k/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/support/Exceptions.java
@@ -18,13 +18,13 @@ private Exceptions() {}
 
     public static ElasticsearchSecurityException authenticationError(String msg, Throwable cause, Object... args) {
         ElasticsearchSecurityException e = new ElasticsearchSecurityException(msg, RestStatus.UNAUTHORIZED, cause, args);
-        e.addHeader("WWW-Authenticate", "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"");
+        e.addHeader("WWW-Authenticate", "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"");
         return e;
     }
 
     public static ElasticsearchSecurityException authenticationError(String msg, Object... args) {
         ElasticsearchSecurityException e = new ElasticsearchSecurityException(msg, RestStatus.UNAUTHORIZED, args);
-        e.addHeader("WWW-Authenticate", "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"");
+        e.addHeader("WWW-Authenticate", "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"");
         return e;
     }
 

diff --git a/...org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java b/...org/elasticsearch/xpack/core/security/authc/DefaultAuthenticationFailureHandlerTests.java
@@ -33,7 +33,7 @@ public class DefaultAuthenticationFailureHandlerTests extends ESTestCase {
 
     public void testAuthenticationRequired() {
         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 DefaultAuthenticationFailureHandler failureHandler;
         if (testDefault) {
@@ -69,7 +69,7 @@ public void testMissingToken() {
     }
 
     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 negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk");
         final Map<String, List<String>> failureResponseHeaders = new HashMap<>();
@@ -134,7 +134,7 @@ public void testExceptionProcessingRequest() {
     }
 
     public void testSortsWWWAuthenticateHeaderValues() {
-        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 negotiateAuthScheme = randomFrom("Negotiate", "Negotiate Ijoijksdk");
         final String apiKeyAuthScheme = "ApiKey";

diff --git a/...gin/core/src/test/java/org/elasticsearch/xpack/core/security/test/SecurityAssertions.java b/...gin/core/src/test/java/org/elasticsearch/xpack/core/security/test/SecurityAssertions.java
@@ -22,6 +22,6 @@ public static void assertContainsWWWAuthenticateHeader(ElasticsearchSecurityExce
         assertThat(e.status(), is(RestStatus.UNAUTHORIZED));
         assertThat(e.getHeaderKeys(), hasSize(1));
         assertThat(e.getHeader("WWW-Authenticate"), notNullValue());
-        assertThat(e.getHeader("WWW-Authenticate"), contains("Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\""));
+        assertThat(e.getHeader("WWW-Authenticate"), contains("Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\""));
     }
 }
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
@@ -748,7 +748,7 @@ public void testLicenseUpdateFailureHandlerUpdate() throws Exception {
                 // On trial license, kerberos is allowed and the WWW-Authenticate response header should reflect that
                 verifyHasAuthenticationHeaderValue(
                     e,
-                    "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"",
+                    "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"",
                     "Negotiate",
                     "ApiKey"
                 );
@@ -760,7 +760,7 @@ public void testLicenseUpdateFailureHandlerUpdate() throws Exception {
             request.getHttpRequest(),
             ActionListener.wrap(result -> { assertTrue(completed.compareAndSet(false, true)); }, e -> {
                 // On basic or gold license, kerberos is not allowed and the WWW-Authenticate response header should also reflect that
-                verifyHasAuthenticationHeaderValue(e, "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"", "ApiKey");
+                verifyHasAuthenticationHeaderValue(e, "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"", "ApiKey");
             })
         );
         if (completed.get()) {

diff --git a/...rity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java b/...rity/src/test/java/org/elasticsearch/xpack/security/authc/AuthenticationServiceTests.java
@@ -1500,7 +1500,7 @@ public void testRealmAuthenticateTerminateAuthenticationProcessWithException() {
         final boolean throwElasticsearchSecurityException = randomBoolean();
         final boolean withAuthenticateHeader = throwElasticsearchSecurityException && randomBoolean();
         Exception throwE = new Exception("general authentication error");
-        final String basicScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
+        final String basicScheme = "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"";
         String selectedScheme = randomFrom(basicScheme, "Negotiate IOJoj");
         if (throwElasticsearchSecurityException) {
             throwE = new ElasticsearchSecurityException("authentication error", RestStatus.UNAUTHORIZED);
@@ -1547,7 +1547,7 @@ public void testRealmAuthenticateGracefulTerminateAuthenticationProcess() {
         when(token.principal()).thenReturn(principal);
         when(firstRealm.token(threadContext)).thenReturn(token);
         when(firstRealm.supports(token)).thenReturn(true);
-        final String basicScheme = "Basic realm=\"" + XPackField.SECURITY + "\" charset=\"UTF-8\"";
+        final String basicScheme = "Basic realm=\"" + XPackField.SECURITY + "\", charset=\"UTF-8\"";
         mockAuthenticate(firstRealm, token, null, true);
 
         ElasticsearchSecurityException e = expectThrows(

diff --git a/...n/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java b/...n/sql/sql-client/src/test/java/org/elasticsearch/xpack/sql/client/RemoteFailureTests.java
@@ -61,7 +61,7 @@ public void testParseMissingAuth() throws IOException {
         assertEquals("missing authentication token for REST request [/?pretty&error_trace]", failure.reason());
         assertThat(failure.remoteTrace(), containsString("DefaultAuthenticationFailureHandler.missingToken"));
         assertNull(failure.cause());
-        assertEquals(singletonMap("WWW-Authenticate", "Basic realm=\"security\" charset=\"UTF-8\""), failure.headers());
+        assertEquals(singletonMap("WWW-Authenticate", "Basic realm=\"security\", charset=\"UTF-8\""), failure.headers());
     }
 
     public void testNoError() {