Do not echo back username on auth failure (#10097)

* Do not echo back username on auth failure

* use bad username

* Remove username from exception messages

* fix tests

* fix the tests

* hopefully this time

* this time the tests work

* fixed this time

* fix

* upgrade to Jetty 9.4.30

* Unknown users echo back Unauthorized

* fix

extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/BasicHTTPAuthenticator.java

@@ -48,7 +48,6 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 import java.io.IOException;
 import java.util.EnumSet;
-import java.util.Locale;
 import java.util.Map;
 
 @JsonTypeName("basic")
@@ -218,9 +217,7 @@ public class BasicHTTPAuthenticator implements Authenticator
       }
       catch (BasicSecurityAuthenticationException ex) {
         LOG.info("Exception authenticating user %s - %s", user, ex.getMessage());
-        httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED,
+        httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
-                           String.format(Locale.getDefault(),
-                                         "User authentication failed username[%s].", user));
       }
     }
 

extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/LDAPCredentialsValidator.java

@@ -165,7 +165,7 @@ public class LDAPCredentialsValidator implements CredentialsValidator
 
       if (!validatePassword(this.ldapConfig, userDn, password)) {
         LOG.debug("Password incorrect for LDAP user %s", username);
-        throw new BasicSecurityAuthenticationException("User LDAP authentication failed username[%s].", userDn.toString());
+        throw new BasicSecurityAuthenticationException("User LDAP authentication failed.");
       }
 
       byte[] salt = BasicAuthUtils.generateSalt();

extensions-core/druid-basic-security/src/main/java/org/apache/druid/security/basic/authentication/validator/MetadataStoreCredentialsValidator.java

@@ -83,7 +83,7 @@ public class MetadataStoreCredentialsValidator implements CredentialsValidator
       return new AuthenticationResult(username, authorizerName, authenticatorName, null);
     } else {
       LOG.debug("Password incorrect for metadata store user %s", username);
-      throw new BasicSecurityAuthenticationException("User metadata store authentication failed username[%s].", username);
+      throw new BasicSecurityAuthenticationException("User metadata store authentication failed.");
     }
   }
 }

extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/BasicHTTPAuthenticatorTest.java

@@ -174,7 +174,7 @@ public class BasicHTTPAuthenticatorTest
     EasyMock.replay(req);
 
     HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
-    resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed username[userA].");
+    resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
     EasyMock.expectLastCall().times(1);
     EasyMock.replay(resp);
 
@@ -210,7 +210,7 @@ public class BasicHTTPAuthenticatorTest
             validator.validateCredentials(EasyMock.eq("basic"), EasyMock.eq("basic"), EasyMock.eq("userA"), EasyMock.aryEq("badpassword".toCharArray()))
         )
         .andThrow(
-            new BasicSecurityAuthenticationException("User authentication failed username[%s].", "userA")
+            new BasicSecurityAuthenticationException("User authentication failed.")
         )
         .times(1);
     EasyMock.replay(validator);
@@ -220,7 +220,7 @@ public class BasicHTTPAuthenticatorTest
     EasyMock.replay(req);
 
     HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
-    resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed username[userA].");
+    resp.sendError(HttpServletResponse.SC_UNAUTHORIZED, "User authentication failed.");
     EasyMock.expectLastCall().times(1);
     EasyMock.replay(resp);
 

extensions-core/druid-basic-security/src/test/java/org/apache/druid/security/authentication/validator/DBCredentialsValidatorTest.java

@@ -143,7 +143,7 @@ public class DBCredentialsValidatorTest
     String password = "badpassword";
 
     expectedException.expect(BasicSecurityAuthenticationException.class);
-    expectedException.expectMessage("User metadata store authentication failed username[userA].");
+    expectedException.expectMessage("User metadata store authentication failed.");
     validator.validateCredentials(authenticatorName, authorizerName, username, password.toCharArray());
   }
 }

integration-tests/src/test/java/org/apache/druid/tests/security/ITBasicAuthConfigurationTest.java

@@ -291,8 +291,8 @@ public class ITBasicAuthConfigurationTest
         datasourceOnlyUserClient,
         SYS_SCHEMA_TASKS_QUERY,
         adminTasks.stream()
-                     .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
+                  .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
-                     .collect(Collectors.toList())
+                  .collect(Collectors.toList())
     );
 
     // as user that can read auth_test and STATE
@@ -317,7 +317,8 @@ public class ITBasicAuthConfigurationTest
         datasourceWithStateUserClient,
         SYS_SCHEMA_SERVER_SEGMENTS_QUERY,
         adminServerSegments.stream()
-                           .filter((serverSegmentEntry) -> ((String) serverSegmentEntry.get("segment_id")).contains("auth_test"))
+                           .filter((serverSegmentEntry) -> ((String) serverSegmentEntry.get("segment_id")).contains(
+                               "auth_test"))
                            .collect(Collectors.toList())
     );
 
@@ -326,8 +327,8 @@ public class ITBasicAuthConfigurationTest
         datasourceWithStateUserClient,
         SYS_SCHEMA_TASKS_QUERY,
         adminTasks.stream()
-                     .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
+                  .filter((taskEntry) -> "auth_test".equals(taskEntry.get("datasource")))
-                     .collect(Collectors.toList())
+                  .collect(Collectors.toList())
     );
 
     // as user that can only read STATE
@@ -471,6 +472,26 @@ public class ITBasicAuthConfigurationTest
     testOptionsRequests(adminClient);
   }
 
+  @Test
+  public void testMaliciousUser()
+  {
+    String maliciousUsername = "<script>alert('hello')</script>";
+    HttpClient maliciousClient = new CredentialedHttpClient(
+        new BasicCredentials(maliciousUsername, "noPass"),
+        httpClient
+    );
+    StatusResponseHolder responseHolder = HttpUtil.makeRequestWithExpectedStatus(
+        maliciousClient,
+        HttpMethod.GET,
+        config.getBrokerUrl() + "/status",
+        null,
+        HttpResponseStatus.UNAUTHORIZED
+    );
+    String responseContent = responseHolder.getContent();
+    Assert.assertTrue(responseContent.contains("<tr><th>MESSAGE:</th><td>Unauthorized</td></tr>"));
+    Assert.assertFalse(responseContent.contains(maliciousUsername));
+  }
+
   private void testOptionsRequests(HttpClient httpClient)
   {
     HttpUtil.makeRequest(httpClient, HttpMethod.OPTIONS, config.getCoordinatorUrl() + "/status", null);
@@ -522,7 +543,7 @@ public class ITBasicAuthConfigurationTest
     catch (AvaticaSqlException ase) {
       Assert.assertEquals(
           ase.getErrorMessage(),
-          "Error while executing SQL \"SELECT * FROM INFORMATION_SCHEMA.COLUMNS\": Remote driver error: BasicSecurityAuthenticationException: User metadata store authentication failed username[admin]."
+          "Error while executing SQL \"SELECT * FROM INFORMATION_SCHEMA.COLUMNS\": Remote driver error: BasicSecurityAuthenticationException: User metadata store authentication failed."
       );
       return;
     }