Browse Source

AMBARI-9055. Pass LDAP URL and Principal container DN to Active Directory operations handler (rlevas)

Robert Levas 10 years ago
parent
commit
65393a52eb
13 changed files with 229 additions and 73 deletions
  1. 68 13
      ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java
  2. 38 11
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java
  3. 2 1
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java
  4. 7 2
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java
  5. 7 12
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java
  6. 26 3
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java
  7. 39 9
      ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java
  8. 12 3
      ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java
  9. 10 8
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java
  10. 4 0
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java
  11. 2 2
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java
  12. 6 4
      ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java
  13. 8 5
      ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java

+ 68 - 13
ambari-server/src/main/java/org/apache/ambari/server/controller/KerberosHelper.java

@@ -184,8 +184,26 @@ public class KerberosHelper {
         throw new AmbariException(message);
       }
 
+      Config configKerberosEnv = cluster.getDesiredConfigByType("kerberos-env");
+      if (configKerberosEnv == null) {
+        String message = "The 'kerberos-env' configuration is not available";
+        LOG.error(message);
+        throw new AmbariException(message);
+      }
+
+      Map<String, String> kerberosEnvProperties = configKerberosEnv.getProperties();
+      if (kerberosEnvProperties == null) {
+        String message = "The 'kerberos-env' configuration properties are not available";
+        LOG.error(message);
+        throw new AmbariException(message);
+      }
+
       KDCType kdcType = null;
-      String kdcTypeProperty = krb5ConfProperties.get("kdc_type");
+      String kdcTypeProperty = kerberosEnvProperties.get("kdc_type");
+      if (kdcTypeProperty == null) {
+        // TODO: (rlevas) Only pull from kerberos-env, this is only for transitional purposes (AMBARI 9121)
+        kdcTypeProperty = krb5ConfProperties.get("kdc_type");
+      }
       if (kdcTypeProperty != null) {
         try {
           kdcType = KDCType.translate(kdcTypeProperty);
@@ -201,12 +219,18 @@ public class KerberosHelper {
         kdcType = KDCType.MIT_KDC;
       }
 
+      KDCDetails kdcDetails = new KDCDetails(
+          kdcType,
+          (kerberosEnvProperties == null) ? null : kerberosEnvProperties.get("ldap_url"),
+          (kerberosEnvProperties == null) ? null : kerberosEnvProperties.get("container_dn")
+      );
+
       if ("true".equalsIgnoreCase(securityEnabled)) {
         LOG.info("Configuring Kerberos for realm {} on cluster, {}", defaultRealm, cluster.getClusterName());
-        requestStageContainer = handle(cluster, kerberosDescriptor, defaultRealm, kdcType, requestStageContainer, enableKerberosHandler);
+        requestStageContainer = handle(cluster, kerberosDescriptor, defaultRealm, kdcDetails, requestStageContainer, enableKerberosHandler);
       } else if ("false".equalsIgnoreCase(securityEnabled)) {
         LOG.info("Disabling Kerberos from cluster, {}", cluster.getClusterName());
-        requestStageContainer = handle(cluster, kerberosDescriptor, defaultRealm, kdcType, requestStageContainer, disableKerberosHandler);
+        requestStageContainer = handle(cluster, kerberosDescriptor, defaultRealm, kdcDetails, requestStageContainer, disableKerberosHandler);
       } else {
         String message = String.format("Invalid value for `security_enabled` property of cluster-env: %s", securityEnabled);
         LOG.error(message);
@@ -229,7 +253,7 @@ public class KerberosHelper {
    * @param cluster               the relevant Cluster
    * @param kerberosDescriptor    the (derived) KerberosDescriptor
    * @param realm                 the default Kerberos realm for the Cluster
-   * @param kdcType               the relevant KDC type (MIT KDC or Active Directory)
+   * @param kdcDetails            a KDCDetails containing information about relevant KDC
    * @param requestStageContainer a RequestStageContainer to place generated stages, if needed -
    *                              if null a new RequestStageContainer will be created.
    * @return the updated or a new RequestStageContainer containing the stages that need to be
@@ -239,7 +263,7 @@ public class KerberosHelper {
   @Transactional
   private RequestStageContainer handle(Cluster cluster,
                                        KerberosDescriptor kerberosDescriptor,
-                                       String realm, KDCType kdcType,
+                                       String realm, KDCDetails kdcDetails,
                                        RequestStageContainer requestStageContainer,
                                        Handler handler) throws AmbariException {
 
@@ -401,7 +425,7 @@ public class KerberosHelper {
                       "}"
               );
             } else {
-              KerberosOperationHandler operationHandler = kerberosOperationHandlerFactory.getKerberosOperationHandler(kdcType);
+              KerberosOperationHandler operationHandler = kerberosOperationHandlerFactory.getKerberosOperationHandler(kdcDetails.getKdcType());
 
               if (operationHandler == null) {
                 throw new AmbariException("Failed to get an appropriate Kerberos operation handler.");
@@ -410,7 +434,7 @@ public class KerberosHelper {
                 KerberosCredential kerberosCredentials = KerberosCredential.decrypt(credentials, key);
 
                 try {
-                  operationHandler.open(kerberosCredentials, realm);
+                  operationHandler.open(kerberosCredentials, realm, kdcDetails.getLdapUrl(), kdcDetails.getPrincipalContainerDn());
                   if (!operationHandler.testAdministratorCredentials()) {
                     throw new IllegalArgumentException(
                         "Invalid KDC administrator credentials.\n" +
@@ -502,7 +526,7 @@ public class KerberosHelper {
 
         // Use the handler implementation to setup the relevant stages.
         int lastStageId = handler.createStages(cluster, hosts, kerberosConfigurations,
-            clusterHostInfoJson, hostParamsJson, event, roleCommandOrder, realm, kdcType.toString(),
+            clusterHostInfoJson, hostParamsJson, event, roleCommandOrder, realm, kdcDetails,
             dataDirectory, requestStageContainer, serviceComponentHostsToProcess);
 
         // Add the cleanup stage...
@@ -993,7 +1017,7 @@ public class KerberosHelper {
      * @param event                  a ServiceComponentHostServerActionEvent to pass to any created tasks
      * @param roleCommandOrder       the RoleCommandOrder to use to generate the RoleGraph for any newly created Stages
      * @param realm                  a String declaring the cluster's Kerberos realm
-     * @param kdcType                a relevant KDCType
+     * @param kdcDetails             a KDCDetails containing the information about the relevant KDC
      * @param dataDirectory          a File pointing to the (temporary) data directory
      * @param requestStageContainer  a RequestStageContainer to store the new stages in, if null a
      *                               new RequestStageContainer will be created
@@ -1006,7 +1030,7 @@ public class KerberosHelper {
                      String clusterHostInfo, String hostParams,
                      ServiceComponentHostServerActionEvent event,
                      RoleCommandOrder roleCommandOrder,
-                     String realm, String kdcType, File dataDirectory,
+                     String realm, KDCDetails kdcDetails, File dataDirectory,
                      RequestStageContainer requestStageContainer,
                      List<ServiceComponentHost> serviceComponentHosts)
         throws AmbariException;
@@ -1059,7 +1083,7 @@ public class KerberosHelper {
                             Map<String, Map<String, String>> kerberosConfigurations,
                             String clusterHostInfoJson, String hostParamsJson,
                             ServiceComponentHostServerActionEvent event,
-                            RoleCommandOrder roleCommandOrder, String realm, String kdcType,
+                            RoleCommandOrder roleCommandOrder, String realm, KDCDetails kdcDetails,
                             File dataDirectory, RequestStageContainer requestStageContainer,
                             List<ServiceComponentHost> serviceComponentHosts)
         throws AmbariException {
@@ -1121,7 +1145,9 @@ public class KerberosHelper {
       Map<String, String> commandParameters = new HashMap<String, String>();
       commandParameters.put(KerberosServerAction.DATA_DIRECTORY, dataDirectory.getAbsolutePath());
       commandParameters.put(KerberosServerAction.DEFAULT_REALM, realm);
-      commandParameters.put(KerberosServerAction.KDC_TYPE, kdcType);
+      commandParameters.put(KerberosServerAction.KDC_TYPE, kdcDetails.getKdcType().name());
+      commandParameters.put(KerberosServerAction.KDC_LDAP_URL, kdcDetails.getLdapUrl());
+      commandParameters.put(KerberosServerAction.KDC_PRINCIPAL_CONTAINER_DN, kdcDetails.getPrincipalContainerDn());
       commandParameters.put(KerberosServerAction.ADMINISTRATOR_CREDENTIAL, getEncryptedAdministratorCredentials(cluster));
 
       // *****************************************************************
@@ -1258,7 +1284,7 @@ public class KerberosHelper {
                             Map<String, Map<String, String>> kerberosConfigurations,
                             String clusterHostInfoJson, String hostParamsJson,
                             ServiceComponentHostServerActionEvent event,
-                            RoleCommandOrder roleCommandOrder, String realm, String kdcType,
+                            RoleCommandOrder roleCommandOrder, String realm, KDCDetails kdcDetails,
                             File dataDirectory, RequestStageContainer requestStageContainer,
                             List<ServiceComponentHost> serviceComponentHosts) {
       // TODO (rlevas): If there are principals, keytabs, and configurations to process, setup the following sages:
@@ -1269,4 +1295,33 @@ public class KerberosHelper {
       return -1;
     }
   }
+
+
+  /**
+   * KDCDetails is a helper class to hold the details of the relevant KDC so they may be passed
+   * around more easily.
+   */
+  private static class KDCDetails {
+    private final KDCType kdcType;
+    private final String ldapUrl;
+    private final String principalContainerDn;
+
+    public KDCDetails(KDCType kdcType, String ldapUrl, String principalContainerDn) {
+      this.kdcType = kdcType;
+      this.ldapUrl = ldapUrl;
+      this.principalContainerDn = principalContainerDn;
+    }
+
+    public KDCType getKdcType() {
+      return kdcType;
+    }
+
+    public String getLdapUrl() {
+      return ldapUrl;
+    }
+
+    public String getPrincipalContainerDn() {
+      return principalContainerDn;
+    }
+  }
 }

+ 38 - 11
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandler.java

@@ -145,7 +145,7 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
   @Override
   public boolean principalExists(String principal) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -154,7 +154,7 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
     try {
       searchResultEnum = ldapContext.search(
           principalContainerDn,
-          "(cn=" + principal + ")",
+          "(userPrincipalName=" + principal + ")",
           searchControls);
       if (searchResultEnum.hasMore()) {
         return true;
@@ -180,21 +180,36 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
    *
    * @param principal a String containing the principal to add
    * @param password  a String containing the password to use when creating the principal
+   * @param service   a boolean value indicating whether the principal is to be created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosOperationException
    */
   @Override
-  public Integer createServicePrincipal(String principal, String password)
+  public Integer createPrincipal(String principal, String password, boolean service)
       throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
+
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
     }
     if (password == null) {
       throw new KerberosOperationException("principal password is null");
     }
+
+    // TODO: (rlevas) pass components and realm in separately (AMBARI-9122)
+    String realm = getDefaultRealm();
+    int atIndex = principal.indexOf("@");
+    if (atIndex >= 0) {
+      realm = principal.substring(atIndex + 1);
+      principal = principal.substring(0, atIndex);
+    }
+
+    if (realm == null) {
+      realm = "";
+    }
+
     Attributes attributes = new BasicAttributes();
 
     Attribute objectClass = new BasicAttribute("objectClass");
@@ -206,12 +221,14 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
     attributes.put(cn);
 
     Attribute upn = new BasicAttribute("userPrincipalName");
-    upn.add(String.format("%s@%s", principal, getDefaultRealm().toLowerCase()));
+    upn.add(String.format("%s@%s", principal, realm.toLowerCase()));
     attributes.put(upn);
 
-    Attribute spn = new BasicAttribute("servicePrincipalName");
-    spn.add(principal);
-    attributes.put(spn);
+    if (service) {
+      Attribute spn = new BasicAttribute("servicePrincipalName");
+      spn.add(principal);
+      attributes.put(spn);
+    }
 
     Attribute uac = new BasicAttribute("userAccountControl");  // userAccountControl
     uac.add("512");
@@ -248,7 +265,7 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
   @Override
   public Integer setPrincipalPassword(String principal, String password) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -289,9 +306,9 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
    * @throws KerberosOperationException
    */
   @Override
-  public boolean removeServicePrincipal(String principal) throws KerberosOperationException {
+  public boolean removePrincipal(String principal) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
     if (principal == null) {
       throw new KerberosOperationException("principal is null");
@@ -313,6 +330,16 @@ public class ADKerberosOperationHandler extends KerberosOperationHandler {
     return true;
   }
 
+  @Override
+  public boolean testAdministratorCredentials() throws KerberosOperationException {
+    if (!isOpen()) {
+      throw new KerberosOperationException("This operation handler has not been opened");
+    }
+    // If this KerberosOperationHandler was successfully opened, successful authentication has
+    // already occurred.
+    return true;
+  }
+
   /**
    * Helper method to create the LDAP context needed to interact with the Active Directory.
    *

+ 2 - 1
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/CreatePrincipalsServerAction.java

@@ -114,7 +114,8 @@ public class CreatePrincipalsServerAction extends KerberosServerAction {
           }
         } else {
           LOG.debug("Creating new principal - {}", evaluatedPrincipal);
-          Integer keyNumber = operationHandler.createServicePrincipal(evaluatedPrincipal, password);
+          boolean servicePrincipal = "service".equalsIgnoreCase(identityRecord.get(KerberosActionDataFile.PRINCIPAL_TYPE));
+          Integer keyNumber = operationHandler.createPrincipal(evaluatedPrincipal, password, servicePrincipal);
 
           if (keyNumber != null) {
             principalPasswordMap.put(evaluatedPrincipal, password);

+ 7 - 2
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandler.java

@@ -178,10 +178,11 @@ public abstract class KerberosOperationHandler {
    *
    * @param principal a String containing the principal to add
    * @param password  a String containing the password to use when creating the principal
+   * @param service a boolean value indicating whether the principal is to be created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosOperationException
    */
-  public abstract Integer createServicePrincipal(String principal, String password)
+  public abstract Integer createPrincipal(String principal, String password, boolean service)
       throws KerberosOperationException;
 
   /**
@@ -206,7 +207,7 @@ public abstract class KerberosOperationHandler {
    * @return true if the principal was successfully removed; otherwise false
    * @throws KerberosOperationException
    */
-  public abstract boolean removeServicePrincipal(String principal)
+  public abstract boolean removePrincipal(String principal)
       throws KerberosOperationException;
 
   /**
@@ -218,6 +219,10 @@ public abstract class KerberosOperationHandler {
    *                                    administrator credentials
    */
   public boolean testAdministratorCredentials() throws KerberosOperationException {
+    if (!isOpen()) {
+      throw new KerberosOperationException("This operation handler has not been opened");
+    }
+
     KerberosCredential credentials = getAdministratorCredentials();
     if (credentials == null) {
       throw new KerberosOperationException("Missing KDC administrator credentials");

+ 7 - 12
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactory.java

@@ -27,32 +27,27 @@ import com.google.inject.Singleton;
 public class KerberosOperationHandlerFactory {
 
   /**
-   * Gets a relevant KerberosOperationHandler give some KDCType.
+   * Gets the relevant KerberosOperationHandler for some KDCType.
    * <p/>
    * If no KDCType is specified, {@link org.apache.ambari.server.serveraction.kerberos.KDCType#MIT_KDC}
    * will be assumed.
    *
    * @param kdcType the relevant KDCType
    * @return a KerberosOperationHandler
+   * @throws java.lang.IllegalArgumentException if kdcType is null or the KDCType is an unexpected value
    */
   public KerberosOperationHandler getKerberosOperationHandler(KDCType kdcType) {
-    KerberosOperationHandler handler = null;
-
-    // If not specified, use KDCType.MIT_KDC as a default
     if (kdcType == null) {
-      kdcType = KDCType.MIT_KDC;
+      throw new IllegalArgumentException("kdcType may not be null");
     }
 
     switch (kdcType) {
-
       case MIT_KDC:
-        handler = new MITKerberosOperationHandler();
-        break;
+        return new MITKerberosOperationHandler();
       case ACTIVE_DIRECTORY:
-        handler = new ADKerberosOperationHandler();
-        break;
+        return new ADKerberosOperationHandler();
+      default:
+        throw new IllegalArgumentException(String.format("Unexpected kdcType value: %s", kdcType.name()));
     }
-
-    return handler;
   }
 }

+ 26 - 3
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/KerberosServerAction.java

@@ -69,6 +69,18 @@ public abstract class KerberosServerAction extends AbstractServerAction {
    */
   public static final String KDC_TYPE = "kdc_type";
 
+  /**
+   * A (command parameter) property name used to hold the URL for the LDAP interface to the KDC.
+   * This value may be null.
+   */
+  public static final String KDC_LDAP_URL = "kdc_ldap_url";
+
+  /**
+   * A (command parameter) property name used to hold the distinguished name (DN) of the container
+   * in which to store principals within the KDC.  This value may be null.
+   */
+  public static final String KDC_PRINCIPAL_CONTAINER_DN = "kdc_principal_container_dn";
+
   /**
    * The prefix to use for the data directory name.
    */
@@ -360,11 +372,20 @@ public abstract class KerberosServerAction extends AbstractServerAction {
               throw new AmbariException(message);
             }
 
+            String ldapUrl = getCommandParameterValue(KDC_LDAP_URL);
+            String principalContainerDn = getCommandParameterValue(KDC_PRINCIPAL_CONTAINER_DN);
+            try {
+              handler.open(administratorCredential, defaultRealm, ldapUrl, principalContainerDn);
+            } catch (KerberosOperationException e) {
+              String message = String.format("Failed to process the identities, could not properly open the KDC operation handler: %s",
+                  e.getMessage());
+              LOG.error(message);
+              throw new AmbariException(message, e);
+            }
+
             // Create the data file reader to parse and iterate through the records
             KerberosActionDataFileReader reader = null;
             try {
-              handler.open(administratorCredential, defaultRealm);
-
               reader = new KerberosActionDataFileReader(indexFile);
               for (Map<String, String> record : reader) {
                 // Process the current record
@@ -376,7 +397,9 @@ public abstract class KerberosServerAction extends AbstractServerAction {
                   break;
                 }
               }
-            } catch (KerberosOperationException e) {
+            } catch (AmbariException e) {
+              // Catch this separately from IOException since the reason it was thrown was not the same
+              // Note: AmbariException is an IOException, so there may be some confusion
               throw new AmbariException(e.getMessage(), e);
             } catch (IOException e) {
               String message = String.format("Failed to process the identities, cannot read the index file: %s",

+ 39 - 9
ambari-server/src/main/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandler.java

@@ -26,6 +26,7 @@ import java.io.File;
 import java.text.NumberFormat;
 import java.text.ParseException;
 import java.util.ArrayList;
+import java.util.Iterator;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
@@ -79,7 +80,7 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
       throws KerberosOperationException {
 
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
 
     if (principal == null) {
@@ -112,6 +113,7 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
    *
    * @param principal a String containing the principal add
    * @param password  a String containing the password to use when creating the principal
+   * @param service a boolean value indicating whether the principal is to be created as a service principal or not
    * @return an Integer declaring the generated key number
    * @throws KerberosKDCConnectionException       if a connection to the KDC cannot be made
    * @throws KerberosAdminAuthenticationException if the administrator credentials fail to authenticate
@@ -119,11 +121,11 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
    * @throws KerberosOperationException           if an unexpected error occurred
    */
   @Override
-  public Integer createServicePrincipal(String principal, String password)
+  public Integer createPrincipal(String principal, String password, boolean service)
       throws KerberosOperationException {
 
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -168,7 +170,7 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
   @Override
   public Integer setPrincipalPassword(String principal, String password) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -201,9 +203,9 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
    * @throws KerberosOperationException           if an unexpected error occurred
    */
   @Override
-  public boolean removeServicePrincipal(String principal) throws KerberosOperationException {
+  public boolean removePrincipal(String principal) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -237,7 +239,7 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
    */
   private Integer getKeyNumber(String principal) throws KerberosOperationException {
     if (!isOpen()) {
-      throw new KerberosOperationException("This operation handler has not be opened");
+      throw new KerberosOperationException("This operation handler has not been opened");
     }
 
     if ((principal == null) || principal.isEmpty()) {
@@ -357,8 +359,36 @@ public class MITKerberosOperationHandler extends KerberosOperationHandler {
       result = executeCommand(command.toArray(new String[command.size()]));
 
       if (!result.isSuccessful()) {
-        String message = String.format("Failed to execute kadmin:\n\tExitCode: %s\n\tSTDOUT: %s\n\tSTDERR: %s",
-            result.getExitCode(), result.getStdout(), result.getStderr());
+        // Build command string, replacing administrator password with "********"
+        StringBuilder cleanCommand = new StringBuilder();
+        Iterator<String> iterator = command.iterator();
+
+        if(iterator.hasNext())
+          cleanCommand.append(iterator.next());
+
+        while(iterator.hasNext()){
+          String part = iterator.next();
+
+          cleanCommand.append(' ');
+
+          if(part.contains(" ")) {
+            cleanCommand.append('"');
+            cleanCommand.append(part);
+            cleanCommand.append('"');
+          }
+          else {
+            cleanCommand.append(part);
+          }
+
+          if("-w".equals(part)) {
+            // Skip the password and use "********" instead
+            if(iterator.hasNext())
+              iterator.next();
+            cleanCommand.append(" ********");
+          }
+        }
+        String message = String.format("Failed to execute kadmin:\n\tCommand: %s\n\tExitCode: %s\n\tSTDOUT: %s\n\tSTDERR: %s",
+            cleanCommand.toString(), result.getExitCode(), result.getStdout(), result.getStderr());
         LOG.warn(message);
 
         // Test STDERR to see of any "expected" error conditions were encountered...

+ 12 - 3
ambari-server/src/test/java/org/apache/ambari/server/controller/KerberosHelperTest.java

@@ -37,7 +37,6 @@ import org.apache.ambari.server.serveraction.kerberos.KDCType;
 import org.apache.ambari.server.serveraction.kerberos.KerberosCredential;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationException;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandler;
-import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerTest;
 import org.apache.ambari.server.serveraction.kerberos.KerberosOperationHandlerFactory;
 import org.apache.ambari.server.state.Cluster;
 import org.apache.ambari.server.state.Clusters;
@@ -92,6 +91,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           public void open(KerberosCredential administratorCredentials, String defaultRealm) throws KerberosOperationException {
             setAdministratorCredentials(administratorCredentials);
             setDefaultRealm(defaultRealm);
+            setOpen(true);
           }
 
           @Override
@@ -105,7 +105,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           }
 
           @Override
-          public Integer createServicePrincipal(String principal, String password) throws KerberosOperationException {
+          public Integer createPrincipal(String principal, String password, boolean service) throws KerberosOperationException {
             return null;
           }
 
@@ -115,7 +115,7 @@ public class KerberosHelperTest extends EasyMockSupport {
           }
 
           @Override
-          public boolean removeServicePrincipal(String principal) throws KerberosOperationException {
+          public boolean removePrincipal(String principal) throws KerberosOperationException {
             return false;
           }
         })
@@ -251,10 +251,18 @@ public class KerberosHelperTest extends EasyMockSupport {
     final Config clusterEnvConfig = createNiceMock(Config.class);
     expect(clusterEnvConfig.getProperties()).andReturn(clusterEnvProperties).once();
 
+    final Map<String, String> kerberosEnvProperties = createNiceMock(Map.class);
+    // TODO: (rlevas) Add when AMBARI 9121 is complete
+    // expect(kerberosEnvProperties.get("kdc_type")).andReturn("mit-kdc").once();
+
+    final Config kerberosEnvConfig = createNiceMock(Config.class);
+    expect(kerberosEnvConfig.getProperties()).andReturn(kerberosEnvProperties).once();
+
     final Map<String, String> krb5ConfProperties = createNiceMock(Map.class);
     expect(krb5ConfProperties.get("kdc_type")).andReturn("mit-kdc").once();
 
     final Config krb5ConfConfig = createNiceMock(Config.class);
+    // TODO: (rlevas) Remove when AMBARI 9121 is complete
     expect(krb5ConfConfig.getProperties()).andReturn(krb5ConfProperties).once();
 
     final MaintenanceStateHelper maintenanceStateHelper = injector.getInstance(MaintenanceStateHelper.class);
@@ -264,6 +272,7 @@ public class KerberosHelperTest extends EasyMockSupport {
     final Cluster cluster = createNiceMock(Cluster.class);
     expect(cluster.getDesiredConfigByType("cluster-env")).andReturn(clusterEnvConfig).once();
     expect(cluster.getDesiredConfigByType("krb5-conf")).andReturn(krb5ConfConfig).once();
+    expect(cluster.getDesiredConfigByType("kerberos-env")).andReturn(kerberosEnvConfig).once();
     expect(cluster.getClusterName()).andReturn("c1").anyTimes();
     expect(cluster.getServices())
         .andReturn(new HashMap<String, Service>() {

+ 10 - 8
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/ADKerberosOperationHandlerTest.java

@@ -38,11 +38,11 @@ import static org.easymock.EasyMock.expect;
 import static org.easymock.EasyMock.replay;
 
 public class ADKerberosOperationHandlerTest extends EasyMockSupport {
-  private static final String DEFAULT_ADMIN_PRINCIPAL = "admin@example.com";
-  private static final String DEFAULT_ADMIN_PASSWORD = "hadoop";
-  private static final String DEFAULT_LDAP_URL = "ldaps://ad.example.com";
-  private static final String DEFAULT_PRINCIPAL_CONTAINER_DN = "ou=cluster,dc=example,dc=com";
-  private static final String DEFAULT_REALM = "EXAMPLE.COM";
+  private static final String DEFAULT_ADMIN_PRINCIPAL = "cluser_admin@HDP01.LOCAL";
+  private static final String DEFAULT_ADMIN_PASSWORD = "Hadoop12345";
+  private static final String DEFAULT_LDAP_URL = "ldaps://10.0.100.4";
+  private static final String DEFAULT_PRINCIPAL_CONTAINER_DN = "ou=HDP,DC=HDP01,DC=LOCAL";
+  private static final String DEFAULT_REALM = "HDP01.LOCAL";
 
   @Test(expected = KerberosKDCConnectionException.class)
   public void testOpenExceptionLdapUrlNotProvided() throws Exception {
@@ -220,21 +220,23 @@ public class ADKerberosOperationHandlerTest extends EasyMockSupport {
     }
 
     if (containerDN == null) {
-      containerDN = "";
+      containerDN = DEFAULT_PRINCIPAL_CONTAINER_DN;
     }
 
     KerberosCredential credentials = new KerberosCredential(principal, password, null);
 
     handler.open(credentials, realm, ldapUrl, containerDN);
 
+    System.out.println("Test Admin Credentials: " + handler.testAdministratorCredentials());
     // does the principal already exist?
     System.out.println("Principal exists: " + handler.principalExists("nn/c1508.ambari.apache.org"));
 
     //create principal
-    handler.createServicePrincipal("nn/c1508.ambari.apache.org", "welcome");
+    handler.createPrincipal("nn/c1508.ambari.apache.org@" + DEFAULT_REALM.toLowerCase(), handler.createSecurePassword(), true);
+    handler.createPrincipal("nn/c1508.ambari.apache.org", handler.createSecurePassword(), true);
 
     //update the password
-    handler.setPrincipalPassword("nn/c1508.ambari.apache.org", "welcome10");
+    handler.setPrincipalPassword("nn/c1508.ambari.apache.org", handler.createSecurePassword());
 
     // remove the principal
     // handler.removeServicePrincipal("nn/c1508.ambari.apache.org");

+ 4 - 0
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerFactoryTest.java

@@ -36,4 +36,8 @@ public class KerberosOperationHandlerFactoryTest {
       new KerberosOperationHandlerFactory().getKerberosOperationHandler(KDCType.ACTIVE_DIRECTORY).getClass());
   }
 
+  @Test(expected = IllegalArgumentException.class)
+  public void testForNull() {
+    Assert.assertNull(new KerberosOperationHandlerFactory().getKerberosOperationHandler(null));
+  }
 }

+ 2 - 2
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/KerberosOperationHandlerTest.java

@@ -220,7 +220,7 @@ public abstract class KerberosOperationHandlerTest {
       }
 
       @Override
-      public Integer createServicePrincipal(String principal, String password) throws KerberosOperationException {
+      public Integer createPrincipal(String principal, String password, boolean serivce) throws KerberosOperationException{
         return 0;
       }
 
@@ -230,7 +230,7 @@ public abstract class KerberosOperationHandlerTest {
       }
 
       @Override
-      public boolean removeServicePrincipal(String principal) throws KerberosOperationException {
+      public boolean removePrincipal(String principal) throws KerberosOperationException {
         return false;
       }
     };

+ 6 - 4
ambari-server/src/test/java/org/apache/ambari/server/serveraction/kerberos/MITKerberosOperationHandlerTest.java

@@ -51,6 +51,8 @@ public class MITKerberosOperationHandlerTest extends EasyMockSupport {
     try {
       handler.setPrincipalPassword(DEFAULT_ADMIN_PRINCIPAL, "");
       Assert.fail("KerberosOperationException not thrown for empty password");
+      handler.createPrincipal("", "1234", false);
+      Assert.fail("AmbariException not thrown for empty principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
@@ -76,28 +78,28 @@ public class MITKerberosOperationHandlerTest extends EasyMockSupport {
     handler.open(new KerberosCredential(DEFAULT_ADMIN_PRINCIPAL, DEFAULT_ADMIN_PASSWORD, null), DEFAULT_REALM);
 
     try {
-      handler.createServicePrincipal(DEFAULT_ADMIN_PRINCIPAL, null);
+      handler.createPrincipal(DEFAULT_ADMIN_PRINCIPAL, null, false);
       Assert.fail("KerberosOperationException not thrown for null password");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal(DEFAULT_ADMIN_PRINCIPAL, "");
+      handler.createPrincipal(DEFAULT_ADMIN_PRINCIPAL, "", false);
       Assert.fail("KerberosOperationException not thrown for empty password");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal(null, DEFAULT_ADMIN_PASSWORD);
+      handler.createPrincipal(null, DEFAULT_ADMIN_PASSWORD, false);
       Assert.fail("KerberosOperationException not thrown for null principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());
     }
 
     try {
-      handler.createServicePrincipal("", DEFAULT_ADMIN_PASSWORD);
+      handler.createPrincipal("", DEFAULT_ADMIN_PASSWORD, false);
       Assert.fail("KerberosOperationException not thrown for empty principal");
     } catch (Throwable t) {
       Assert.assertEquals(KerberosOperationException.class, t.getClass());

+ 8 - 5
ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/KerberosPrincipalDescriptorTest.java

@@ -29,17 +29,17 @@ public class KerberosPrincipalDescriptorTest {
   public static final String JSON_VALUE =
       "{" +
           "\"value\": \"service/_HOST@_REALM\"," +
-          "\"type\": \"service\"," +
           "\"configuration\": \"service-site/service.component.kerberos.principal\"," +
+          "\"type\": \"service\"," +
           "\"local_username\": \"localUser\"" +
           "}";
 
   public static final Map<String, Object> MAP_VALUE =
       new HashMap<String, Object>() {
         {
-          put("value", "HTTP/_HOST@_REALM");
-          put("type", "service");
+          put("value", "user@_REALM");
           put("configuration", "service-site/service.component.kerberos.https.principal");
+          put("type", "user");
           put("local_username", null);
         }
       };
@@ -49,21 +49,24 @@ public class KerberosPrincipalDescriptorTest {
     Assert.assertFalse(principalDescriptor.isContainer());
     Assert.assertEquals("service/_HOST@_REALM", principalDescriptor.getValue());
     Assert.assertEquals("service-site/service.component.kerberos.principal", principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.SERVICE, principalDescriptor.getType());
     Assert.assertEquals("localUser", principalDescriptor.getLocalUsername());
   }
 
   public static void validateFromMap(KerberosPrincipalDescriptor principalDescriptor) {
     Assert.assertNotNull(principalDescriptor);
     Assert.assertFalse(principalDescriptor.isContainer());
-    Assert.assertEquals("HTTP/_HOST@_REALM", principalDescriptor.getValue());
+    Assert.assertEquals("user@_REALM", principalDescriptor.getValue());
     Assert.assertEquals("service-site/service.component.kerberos.https.principal", principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.USER, principalDescriptor.getType());
     Assert.assertNull(principalDescriptor.getLocalUsername());
   }
 
   public static void validateUpdatedData(KerberosPrincipalDescriptor principalDescriptor) {
     Assert.assertNotNull(principalDescriptor);
-    Assert.assertEquals("HTTP/_HOST@_REALM", principalDescriptor.getValue());
+    Assert.assertEquals("user@_REALM", principalDescriptor.getValue());
     Assert.assertEquals("service-site/service.component.kerberos.https.principal", principalDescriptor.getConfiguration());
+    Assert.assertEquals(KerberosPrincipalType.USER, principalDescriptor.getType());
     Assert.assertEquals("localUser", principalDescriptor.getLocalUsername());
   }