fix(jaas): fix jaas login (#12848)

datahub-frontend/app/security/AuthenticationManager.java

@@ -3,11 +3,19 @@ package security;
 import com.google.common.base.Preconditions;
 import javax.annotation.Nonnull;
 import javax.naming.AuthenticationException;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.PasswordCallback;
+import javax.security.auth.login.LoginContext;
+import javax.security.auth.login.LoginException;
 import org.apache.commons.lang3.StringUtils;
-import org.eclipse.jetty.security.UserPrincipal;
-import org.eclipse.jetty.util.security.Credential;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class AuthenticationManager {
+  private static final Logger log = LoggerFactory.getLogger(AuthenticationManager.class);
+
   private AuthenticationManager() {} // Prevent instantiation
 
   public static void authenticateJaasUser(@Nonnull String userName, @Nonnull String password)
@@ -15,19 +23,45 @@ public class AuthenticationManager {
     Preconditions.checkArgument(!StringUtils.isAnyEmpty(userName), "Username cannot be empty");
 
     try {
-      // Create and configure credentials for authentication
-      UserPrincipal userPrincipal = new UserPrincipal(userName, Credential.getCredential(password));
+      // Create a login context with our custom callback handler
+      LoginContext loginContext =
+          new LoginContext("WHZ-Authentication", new WHZCallbackHandler(userName, password));
 
-      // Verify credentials
-      if (!userPrincipal.authenticate(password)) {
-        throw new AuthenticationException("Invalid credentials for user: " + userName);
-      }
+      // Attempt login
+      loginContext.login();
 
-    } catch (Exception e) {
+      // If we get here, authentication succeeded
+      log.debug("Authentication succeeded for user: {}", userName);
+
+    } catch (LoginException le) {
+      log.info("Authentication failed for user {}: {}", userName, le.getMessage());
       AuthenticationException authenticationException =
-          new AuthenticationException("Authentication failed");
-      authenticationException.setRootCause(e);
+          new AuthenticationException(le.getMessage());
+      authenticationException.setRootCause(le);
       throw authenticationException;
     }
   }
+
+  private static class WHZCallbackHandler implements CallbackHandler {
+    private final String password;
+    private final String username;
+
+    private WHZCallbackHandler(@Nonnull String username, @Nonnull String password) {
+      this.username = username;
+      this.password = password;
+    }
+
+    @Override
+    public void handle(@Nonnull Callback[] callbacks) {
+      for (Callback callback : callbacks) {
+        if (callback instanceof NameCallback) {
+          NameCallback nc = (NameCallback) callback;
+          nc.setName(username);
+        } else if (callback instanceof PasswordCallback) {
+          PasswordCallback pc = (PasswordCallback) callback;
+          pc.setPassword(password.toCharArray());
+        }
+      }
+    }
+  }
 }

datahub-frontend/app/security/DataHubUserPrincipal.java

@@ -0,0 +1,35 @@
+package security;
+
+import java.security.Principal;
+
+public class DataHubUserPrincipal implements Principal {
+  private final String name;
+
+  public DataHubUserPrincipal(String name) {
+    this.name = name;
+  }
+
+  @Override
+  public String getName() {
+    return name;
+  }
+
+  @Override
+  public boolean equals(Object o) {
+    if (this == o) return true;
+    if (o == null || getClass() != o.getClass()) return false;
+
+    DataHubUserPrincipal that = (DataHubUserPrincipal) o;
+    return name.equals(that.name);
+  }
+
+  @Override
+  public int hashCode() {
+    return name.hashCode();
+  }
+
+  @Override
+  public String toString() {
+    return "DataHubUserPrincipal[" + name + "]";
+  }
+}

datahub-frontend/app/security/PropertyFileLoginModule.java

@@ -0,0 +1,139 @@
+package security;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.util.Map;
+import java.util.Properties;
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.PasswordCallback;
+import javax.security.auth.callback.UnsupportedCallbackException;
+import javax.security.auth.login.LoginException;
+import javax.security.auth.spi.LoginModule;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class PropertyFileLoginModule implements LoginModule {
+  private static final Logger log = LoggerFactory.getLogger(PropertyFileLoginModule.class);
+
+  private Subject subject;
+  private CallbackHandler callbackHandler;
+  private boolean debug = false;
+  private String file;
+  private boolean succeeded = false;
+  private String username;
+
+  @Override
+  public void initialize(
+      Subject subject,
+      CallbackHandler callbackHandler,
+      Map<String, ?> sharedState,
+      Map<String, ?> options) {
+    this.subject = subject;
+    this.callbackHandler = callbackHandler;
+
+    // Get configuration options
+    this.debug = "true".equalsIgnoreCase((String) options.get("debug"));
+    this.file = (String) options.get("file");
+
+    if (debug) {
+      log.debug("PropertyFileLoginModule initialized with file: {}", file);
+    }
+  }
+
+  @Override
+  public boolean login() throws LoginException {
+    // If no file specified, this module can't authenticate
+    if (file == null) {
+      if (debug) log.debug("No property file specified");
+      return false;
+    }
+
+    // Get username and password from callbacks
+    NameCallback nameCallback = new NameCallback("Username: ");
+    PasswordCallback passwordCallback = new PasswordCallback("Password: ", false);
+
+    try {
+      callbackHandler.handle(new Callback[] {nameCallback, passwordCallback});
+    } catch (IOException | UnsupportedCallbackException e) {
+      if (debug) log.debug("Error getting callbacks", e);
+      throw new LoginException("Error during callback handling: " + e.getMessage());
+    }
+
+    this.username = nameCallback.getName();
+    char[] password = passwordCallback.getPassword();
+    passwordCallback.clearPassword();
+
+    if (username == null || username.isEmpty() || password == null) {
+      if (debug) log.debug("Username or password is empty");
+      return false;
+    }
+
+    // Load properties file
+    Properties props = new Properties();
+    File propsFile = new File(file);
+
+    if (!propsFile.exists() || !propsFile.isFile() || !propsFile.canRead()) {
+      if (debug) log.debug("Cannot read property file: {}", file);
+      return false;
+    }
+
+    try (FileInputStream fis = new FileInputStream(propsFile)) {
+      props.load(fis);
+    } catch (IOException e) {
+      if (debug) log.debug("Failed to load property file", e);
+      throw new LoginException("Error loading property file: " + e.getMessage());
+    }
+
+    // Check if username exists and password matches
+    String storedPassword = props.getProperty(username);
+    if (storedPassword == null) {
+      if (debug) log.debug("User not found: {}", username);
+      return false;
+    }
+
+    // Compare passwords
+    succeeded = storedPassword.equals(new String(password));
+
+    if (debug) {
+      if (succeeded) {
+        log.debug("Authentication succeeded for user: {}", username);
+      } else {
+        log.debug("Authentication failed for user: {}", username);
+      }
+    }
+
+    return succeeded;
+  }
+
+  @Override
+  public boolean commit() throws LoginException {
+    if (!succeeded) {
+      return false;
+    }
+
+    // Add principal to the subject if authentication succeeded
+    subject.getPrincipals().add(new DataHubUserPrincipal(username));
+
+    return true;
+  }
+
+  @Override
+  public boolean abort() throws LoginException {
+    succeeded = false;
+    username = null;
+    return true;
+  }
+
+  @Override
+  public boolean logout() throws LoginException {
+    // Remove principals that were added by this module
+    subject.getPrincipals().removeIf(p -> p instanceof DataHubUserPrincipal);
+    succeeded = false;
+    username = null;
+    return true;
+  }
+}

datahub-frontend/conf/jaas.conf

@@ -1,7 +1,11 @@
 // This is a sample JAAS config that uses the following login module
-// org.eclipse.jetty.jaas.spi.PropertyFileLoginModule -- this module can work with a username and any password defined in the `../conf/user.props` file
+// security.PropertyFileLoginModule -- this module can work with a username and any password defined in the `../conf/user.props` file
 
 WHZ-Authentication {
-  org.eclipse.jetty.jaas.spi.PropertyFileLoginModule sufficient debug="true" file="/etc/datahub/plugins/frontend/auth/user.props";
-  org.eclipse.jetty.jaas.spi.PropertyFileLoginModule sufficient debug="true" file="/datahub-frontend/conf/user.props";
-};
+  security.PropertyFileLoginModule sufficient
+    debug="true"
+    file="/etc/datahub/plugins/frontend/auth/user.props";
+  security.PropertyFileLoginModule sufficient
+    debug="true"
+    file="/datahub-frontend/conf/user.props";
+};

datahub-frontend/run/jaas.conf

@@ -1,9 +1,8 @@
 // This is a sample JAAS config that uses the following login module
-// This is a sample JAAS config that uses the following login module
-// org.eclipse.jetty.jaas.spi.PropertyFileLoginModule -- this module can work with a username and any password defined in the `../conf/user.props` file
+// security.PropertyFileLoginModule -- this module can work with a username and any password defined in the `../conf/user.props` file
 
 WHZ-Authentication {
-  org.eclipse.jetty.jaas.spi.PropertyFileLoginModule sufficient
+  security.PropertyFileLoginModule sufficient
   debug="true"
   file="../conf/user.props";
 };

datahub-frontend/test/security/AuthenticationManagerTest.java

@@ -0,0 +1,163 @@
+package security;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.util.HashMap;
+import java.util.Map;
+import javax.naming.AuthenticationException;
+import javax.security.auth.login.AppConfigurationEntry;
+import javax.security.auth.login.Configuration;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class AuthenticationManagerTest {
+
+  private File tempPropsFile;
+  private TestJaasConfiguration jaasConfig;
+  private static Configuration originalConfig;
+
+  @BeforeAll
+  public static void setUpClass() {
+    // Save the original JAAS configuration
+    originalConfig = Configuration.getConfiguration();
+  }
+
+  @BeforeEach
+  public void setUp() throws IOException {
+    // Create a temporary properties file for testing
+    tempPropsFile = Files.createTempFile("test-users", ".props").toFile();
+
+    // Write test users to the file
+    try (FileWriter writer = new FileWriter(tempPropsFile)) {
+      writer.write("testuser:testpassword\n");
+      writer.write("datahub:datahub\n");
+      writer.write("admin:admin123\n");
+    }
+
+    // Set up a test JAAS configuration - use the fully qualified name of our custom login module
+    jaasConfig = new TestJaasConfiguration();
+    // We need to use the actual class that's available in the test classpath
+    jaasConfig.setLoginModuleClass(PropertyFileLoginModule.class.getName());
+    jaasConfig.setOption("file", tempPropsFile.getAbsolutePath());
+    jaasConfig.setOption("debug", "true");
+
+    // Install the test configuration
+    Configuration.setConfiguration(jaasConfig);
+
+    // Verify our configuration was properly applied
+    AppConfigurationEntry[] entries =
+        Configuration.getConfiguration().getAppConfigurationEntry("WHZ-Authentication");
+    assertNotNull(entries, "JAAS configuration should be applied");
+    assertEquals(1, entries.length, "Should have one login module configured");
+    assertEquals(
+        PropertyFileLoginModule.class.getName(),
+        entries[0].getLoginModuleName(),
+        "Login module class should match PropertyFileLoginModule");
+  }
+
+  @AfterEach
+  public void tearDown() {
+    // Restore the original JAAS configuration
+    Configuration.setConfiguration(originalConfig);
+
+    // Clean up the temporary file
+    if (tempPropsFile != null && tempPropsFile.exists()) {
+      tempPropsFile.delete();
+    }
+  }
+
+  // Since we can't easily mock static methods without mockito-inline,
+  // we'll test the validation logic directly and try a real integration test
+
+  @Test
+  public void testEmptyUsername() {
+    // Test with empty username
+    IllegalArgumentException exception =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> AuthenticationManager.authenticateJaasUser("", "password"),
+            "Should throw IllegalArgumentException for empty username");
+
+    assertEquals("Username cannot be empty", exception.getMessage());
+  }
+
+  @Test
+  public void testNullUsername() {
+    // Test with null username
+    IllegalArgumentException exception =
+        assertThrows(
+            IllegalArgumentException.class,
+            () -> AuthenticationManager.authenticateJaasUser(null, "password"),
+            "Should throw IllegalArgumentException for null username");
+
+    assertEquals("Username cannot be empty", exception.getMessage());
+  }
+
+  /**
+   * Integration test that actually performs authentication against the custom login module. This
+   * test will be skipped if the PropertyFileLoginModule class is not available.
+   */
+  @Test
+  public void testRealAuthentication() {
+    // Test successful authentication
+    try {
+      AuthenticationManager.authenticateJaasUser("datahub", "datahub");
+      // If we get here, authentication was successful
+    } catch (Exception e) {
+      fail("Authentication should succeed with valid credentials: " + e.getMessage());
+    }
+
+    // Test failed authentication
+    Exception exception =
+        assertThrows(
+            AuthenticationException.class,
+            () -> AuthenticationManager.authenticateJaasUser("datahub", "wrongpassword"),
+            "Should throw AuthenticationException for invalid credentials");
+
+    // Make sure we get a login failure message
+    assertTrue(
+        exception.getMessage() != null && !exception.getMessage().isEmpty(),
+        "Exception message should not be empty");
+  }
+
+  /** Method used by the @EnabledIf annotation to conditionally enable the integration test. */
+  boolean isPropertyFileLoginModuleAvailable() {
+    try {
+      Class.forName(PropertyFileLoginModule.class.getName());
+      return true;
+    } catch (ClassNotFoundException e) {
+      return false;
+    }
+  }
+
+  /** Simple test JAAS configuration that can be programmatically configured. */
+  private static class TestJaasConfiguration extends Configuration {
+    private String loginModuleClass;
+    private final Map<String, String> options = new HashMap<>();
+
+    public void setLoginModuleClass(String loginModuleClass) {
+      this.loginModuleClass = loginModuleClass;
+    }
+
+    public void setOption(String key, String value) {
+      options.put(key, value);
+    }
+
+    @Override
+    public AppConfigurationEntry[] getAppConfigurationEntry(String name) {
+      if ("WHZ-Authentication".equals(name) && loginModuleClass != null) {
+        return new AppConfigurationEntry[] {
+          new AppConfigurationEntry(
+              loginModuleClass, AppConfigurationEntry.LoginModuleControlFlag.SUFFICIENT, options)
+        };
+      }
+      return null;
+    }
+  }
+}

datahub-frontend/test/security/DataHubUserPrincipalTest.java

@@ -0,0 +1,63 @@
+package security;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import org.junit.jupiter.api.Test;
+
+public class DataHubUserPrincipalTest {
+
+  @Test
+  public void testGetName() {
+    DataHubUserPrincipal principal = new DataHubUserPrincipal("testuser");
+    assertEquals("testuser", principal.getName(), "Principal name should match constructor value");
+  }
+
+  @Test
+  public void testEquals() {
+    DataHubUserPrincipal principal1 = new DataHubUserPrincipal("testuser");
+    DataHubUserPrincipal principal2 = new DataHubUserPrincipal("testuser");
+    DataHubUserPrincipal principal3 = new DataHubUserPrincipal("otheruser");
+
+    // Test equality with same name
+    assertTrue(principal1.equals(principal2), "Principals with same name should be equal");
+    assertTrue(principal2.equals(principal1), "Equals should be symmetric");
+
+    // Test inequality with different name
+    assertFalse(
+        principal1.equals(principal3), "Principals with different names should not be equal");
+
+    // Test with null and different object type
+    assertFalse(principal1.equals(null), "Principal should not equal null");
+    assertFalse(principal1.equals("testuser"), "Principal should not equal string with same name");
+
+    // Test reflexivity
+    assertTrue(principal1.equals(principal1), "Principal should equal itself");
+  }
+
+  @Test
+  public void testHashCode() {
+    DataHubUserPrincipal principal1 = new DataHubUserPrincipal("testuser");
+    DataHubUserPrincipal principal2 = new DataHubUserPrincipal("testuser");
+
+    // Test hash code consistency
+    assertEquals(
+        principal1.hashCode(),
+        principal2.hashCode(),
+        "Equal principals should have same hash code");
+
+    // Test hash code is based on name
+    assertEquals(
+        "testuser".hashCode(),
+        principal1.hashCode(),
+        "Principal hash code should be based on name");
+  }
+
+  @Test
+  public void testToString() {
+    DataHubUserPrincipal principal = new DataHubUserPrincipal("testuser");
+    String expectedString = "DataHubUserPrincipal[testuser]";
+
+    assertEquals(
+        expectedString, principal.toString(), "toString should return formatted principal name");
+  }
+}

datahub-frontend/test/security/PropertyFileLoginModuleTest.java

@@ -0,0 +1,180 @@
+package security;
+
+import static org.junit.jupiter.api.Assertions.*;
+
+import java.io.File;
+import java.io.FileWriter;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.util.HashMap;
+import java.util.Map;
+import javax.security.auth.Subject;
+import javax.security.auth.callback.Callback;
+import javax.security.auth.callback.CallbackHandler;
+import javax.security.auth.callback.NameCallback;
+import javax.security.auth.callback.PasswordCallback;
+import javax.security.auth.login.LoginException;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class PropertyFileLoginModuleTest {
+
+  private PropertyFileLoginModule loginModule;
+  private Path tempFilePath;
+  private File tempPropsFile;
+
+  @BeforeEach
+  public void setUp() throws IOException {
+    loginModule = new PropertyFileLoginModule();
+
+    // Create a temporary properties file for testing
+    tempFilePath = Files.createTempFile("test-users", ".props");
+    tempPropsFile = tempFilePath.toFile();
+
+    // Write test users to the file
+    try (FileWriter writer = new FileWriter(tempPropsFile)) {
+      writer.write("testuser:testpassword\n");
+      writer.write("datahub:datahub\n");
+      writer.write("admin:admin123\n");
+    }
+  }
+
+  @AfterEach
+  public void tearDown() {
+    // Clean up the temporary file
+    if (tempPropsFile != null && tempPropsFile.exists()) {
+      tempPropsFile.delete();
+    }
+  }
+
+  @Test
+  public void testSuccessfulAuthentication() throws LoginException {
+    // Set up the login module with necessary options
+    Subject subject = new Subject();
+    TestCallbackHandler callbackHandler = new TestCallbackHandler("datahub", "datahub");
+    Map<String, Object> options = new HashMap<>();
+    options.put("debug", "true");
+    options.put("file", tempPropsFile.getAbsolutePath());
+
+    loginModule.initialize(subject, callbackHandler, null, options);
+
+    // Perform login
+    boolean loginResult = loginModule.login();
+    assertTrue(loginResult, "Login should succeed with correct credentials");
+
+    // Commit the authentication
+    boolean commitResult = loginModule.commit();
+    assertTrue(commitResult, "Commit should succeed after successful login");
+
+    // Verify principal was added to the subject
+    assertEquals(1, subject.getPrincipals().size(), "Subject should have one principal added");
+    assertTrue(
+        subject.getPrincipals().stream().anyMatch(p -> p.getName().equals("datahub")),
+        "Subject should have the correct principal");
+  }
+
+  @Test
+  public void testFailedAuthentication() throws LoginException {
+    // Set up the login module with necessary options
+    Subject subject = new Subject();
+    TestCallbackHandler callbackHandler = new TestCallbackHandler("datahub", "wrongpassword");
+    Map<String, Object> options = new HashMap<>();
+    options.put("debug", "true");
+    options.put("file", tempPropsFile.getAbsolutePath());
+
+    loginModule.initialize(subject, callbackHandler, null, options);
+
+    // Perform login
+    boolean loginResult = loginModule.login();
+    assertFalse(loginResult, "Login should fail with incorrect credentials");
+
+    // Commit should return false after failed login
+    boolean commitResult = loginModule.commit();
+    assertFalse(commitResult, "Commit should fail after unsuccessful login");
+
+    // Verify no principals were added
+    assertEquals(
+        0, subject.getPrincipals().size(), "Subject should have no principals after failed login");
+  }
+
+  @Test
+  public void testNonexistentUser() throws LoginException {
+    // Set up the login module with necessary options
+    Subject subject = new Subject();
+    TestCallbackHandler callbackHandler = new TestCallbackHandler("nonexistentuser", "password");
+    Map<String, Object> options = new HashMap<>();
+    options.put("debug", "true");
+    options.put("file", tempPropsFile.getAbsolutePath());
+
+    loginModule.initialize(subject, callbackHandler, null, options);
+
+    // Perform login
+    boolean loginResult = loginModule.login();
+    assertFalse(loginResult, "Login should fail with nonexistent user");
+  }
+
+  @Test
+  public void testNonexistentFile() throws LoginException {
+    // Set up the login module with a nonexistent file
+    Subject subject = new Subject();
+    TestCallbackHandler callbackHandler = new TestCallbackHandler("datahub", "datahub");
+    Map<String, Object> options = new HashMap<>();
+    options.put("debug", "true");
+    options.put("file", "/nonexistent/file.props");
+
+    loginModule.initialize(subject, callbackHandler, null, options);
+
+    // Perform login
+    boolean loginResult = loginModule.login();
+    assertFalse(loginResult, "Login should fail with nonexistent file");
+  }
+
+  @Test
+  public void testLogoutClearsCredentials() throws LoginException {
+    // Set up and perform successful login
+    Subject subject = new Subject();
+    TestCallbackHandler callbackHandler = new TestCallbackHandler("admin", "admin123");
+    Map<String, Object> options = new HashMap<>();
+    options.put("debug", "true");
+    options.put("file", tempPropsFile.getAbsolutePath());
+
+    loginModule.initialize(subject, callbackHandler, null, options);
+    loginModule.login();
+    loginModule.commit();
+
+    // Verify principal was added
+    assertEquals(1, subject.getPrincipals().size(), "Subject should have one principal added");
+
+    // Perform logout
+    boolean logoutResult = loginModule.logout();
+    assertTrue(logoutResult, "Logout should succeed");
+
+    // Verify principal was removed
+    assertEquals(
+        0, subject.getPrincipals().size(), "Subject should have no principals after logout");
+  }
+
+  /** Simple test callback handler that provides fixed username and password. */
+  private static class TestCallbackHandler implements CallbackHandler {
+    private final String username;
+    private final String password;
+
+    public TestCallbackHandler(String username, String password) {
+      this.username = username;
+      this.password = password;
+    }
+
+    @Override
+    public void handle(Callback[] callbacks) {
+      for (Callback callback : callbacks) {
+        if (callback instanceof NameCallback) {
+          ((NameCallback) callback).setName(username);
+        } else if (callback instanceof PasswordCallback) {
+          ((PasswordCallback) callback).setPassword(password.toCharArray());
+        }
+      }
+    }
+  }
+}