Commit b6d8810a authored by Brian Major's avatar Brian Major
Browse files

t72306 - Fixed ac-admin help, subject use, null pointer avoidance

parent dc007bdf
Loading
Loading
Loading
Loading
+7 −11
Original line number Diff line number Diff line
@@ -91,15 +91,13 @@
  <property name="log4j" value="${ext.lib}/log4j.jar"/>
  <property name="commons-logging" value="${ext.lib}/commons-logging.jar"/>
  <property name="unboundid" value="${ext.lib}/unboundid-ldapsdk-se.jar"/>
  <property name="servlet-api" value="${ext.lib}/servlet-api.jar"/>

  <property name="jars"
            value="${cadcAC}:${cadcAC-Server}:${cadcUtil}:${log4j}"/>
  <property name="cadc" value="${cadcAC}:${cadcAC-Server}:${cadcUtil}"/>
  <property name="client.cadc.jars" value="${cadcAC}:${cadcAC-Server}:${cadcLog}:${cadcUtil}"/>
  <property name="client.external.jars" value="${unboundid}:${log4j}:${servlet-api}"/>

  <property name="client.cadc.jars"
            value="${cadcAC}:${cadcAC-Server}:${cadcLog}:${cadcUtil}"/>
  <property name="client.external.jars" value="${unboundid}:${log4j}"/>

  <property name="jars" value="${cadc}:${external}"/>
  <property name="jars" value="${cadc}:${client.cadc.jars}:${client.external.jars}"/>

  <target name="build" depends="compile,manifest">
    <jar jarfile="${build}/lib/${project}.jar"
@@ -136,8 +134,7 @@

    <manifest file="${build}/tmp/${project}.mf" mode="replace">
      <attribute name="Main-Class" value="ca.nrc.cadc.ac.admin.Main"/>
      <attribute name="Class-Path"
                 value="${client.flat.manifest} ${client.non-flat.manifest}"/>
      <attribute name="Class-Path" value="${client.flat.manifest} ${client.non-flat.manifest}"/>
    </manifest>
  </target>

@@ -149,8 +146,7 @@
  <property name="junit"      value="${ext.dev}/junit.jar" />
  <property name="objenesis"  value="${ext.dev}/objenesis.jar" />

  <property name="testingJars"
            value="${junit}:${asm}:${cglib}:${easymock}:${objenesis}:{unboundid}:${cadcLog}"/>
  <property name="testingJars" value="${junit}:${asm}:${cglib}:${easymock}:${objenesis}:{unboundid}:${cadcLog}"/>

  <target name="int-test" depends="build,compile-test,setup-test">
    <echo message="Running test suite..."/>
+43 −74
Original line number Diff line number Diff line
@@ -72,13 +72,9 @@
import java.io.PrintStream;
import java.security.cert.CertificateException;

import javax.security.auth.Subject;

import org.apache.log4j.Level;
import org.apache.log4j.Logger;

import ca.nrc.cadc.auth.CertCmdArgUtil;
import ca.nrc.cadc.auth.SSLUtil;
import ca.nrc.cadc.util.ArgumentMap;
import ca.nrc.cadc.util.Log4jInit;
import ca.nrc.cadc.util.StringUtil;
@@ -97,9 +93,8 @@ public class CmdLineParser

    // no need to proceed further if false
    private Level logLevel = Level.OFF;
    private boolean proceed = true;
    private AbstractCommand command;
    private Subject subject;
    private boolean isHelpCommand = false;

    /**
     * Constructor.
@@ -115,16 +110,6 @@ public class CmdLineParser
    	this.parse(am, outStream, errStream);
    }

    /**
     * Return proceed status.
     * @return true  program should proceed with further processing
     *         false program should not proceed further
     */
    public boolean proceed()
    {
        return this.proceed;
    }

    /**
     * Get the user admin command to be performed.
     * @return user admin command
@@ -134,14 +119,6 @@ public class CmdLineParser
    	return this.command;
    }

    /**
     * Get the subject representing the user executing this user admin tool.
     */
    public Subject getSubject()
    {
    	return this.subject;
    }
    
    /**
     * Get the logging level.
     */
@@ -281,36 +258,22 @@ public class CmdLineParser
    protected void parse(final ArgumentMap am, final PrintStream out,
        final PrintStream err) throws UsageException, CertificateException
    {
        this.proceed = false;

        if (!am.isSet("h") && !am.isSet("help") && isValid(am))
        {
            Subject subject = CertCmdArgUtil.initSubject(am, true);
            
            try 
            {
                SSLUtil.validateSubject(subject, null);
                log.debug("subject: " + subject);
                this.subject = subject;
                this.proceed = true;
            } 
            catch (CertificateException e) 
            {
            	if (am.isSet("list"))
            	{
                    // we can use anonymous subject
                     this.proceed = true;
            // the following statements are executed only when proceed is true
            this.command.setSystemOut(out);
            this.command.setSystemErr(err);
        }
        else
        {
                    throw e;
            isHelpCommand = true;
        }
    }

            // the following statements are executed only when proceed is true
            this.command.setSystemOut(out);
            this.command.setSystemErr(err);            
        }
    public boolean isHelpCommand()
    {
        return isHelpCommand;
    }

    /**
@@ -320,7 +283,7 @@ public class CmdLineParser
    {
    	StringBuilder sb = new StringBuilder();
    	sb.append("\n");
    	sb.append("Usage: " + APP_NAME + " [--cert=<path to pem file>] <command> [-v|--verbose|-d|--debug] [-h|--help]\n");
    	sb.append("Usage: " + APP_NAME + " <command> [-v|--verbose|-d|--debug] [-h|--help]\n");
    	sb.append("Where command is\n");
    	sb.append("--list               :list users in the Users tree\n");
    	sb.append("                     :can be executed as an anonymous user\n");
@@ -334,6 +297,12 @@ public class CmdLineParser
    	sb.append("-v|--verbose         : Verbose mode print progress and error messages\n");
    	sb.append("-d|--debug           : Debug mode print all the logging messages\n");
    	sb.append("-h|--help            : Print this message and exit\n");
    	sb.append("\n");
        sb.append("Authentication and authorization:\n");
        sb.append("  - An LdapConfig.properties file must exist in directory ~/config/\n");
        sb.append("  - The corresponding host entry (devLdap or prodLdap) must exist\n");
        sb.append("    in your ~/.dbrc file.");

    	return sb.toString();
    }
}
+47 −33
Original line number Diff line number Diff line
@@ -68,14 +68,22 @@

package ca.nrc.cadc.ac.admin;

import ca.nrc.cadc.ac.User;
import java.security.Principal;
import java.util.HashSet;
import java.util.Set;

import javax.security.auth.Subject;

import org.apache.log4j.Logger;

import ca.nrc.cadc.ac.UserNotFoundException;
import ca.nrc.cadc.ac.server.UserPersistence;
import ca.nrc.cadc.auth.AuthenticationUtil;
import ca.nrc.cadc.auth.DelegationToken;
import ca.nrc.cadc.auth.PrincipalExtractor;
import ca.nrc.cadc.auth.SSOCookieCredential;
import ca.nrc.cadc.auth.X509CertificateChain;
import ca.nrc.cadc.net.TransientException;
import org.apache.log4j.Logger;

import javax.security.auth.Subject;
import java.security.Principal;


public class CommandRunner
@@ -98,13 +106,17 @@ public class CommandRunner
     *
     */
    public void run() throws UserNotFoundException, TransientException
    {
        if (commandLineParser.proceed())
    {
        AbstractCommand command = commandLineParser.getCommand();
        command.setUserPersistence(userPersistence);

            if (commandLineParser.getSubject() == null)
        Principal userIDPrincipal = null;
        if (command instanceof AbstractUserCommand)
        {
            userIDPrincipal = ((AbstractUserCommand) command).getPrincipal();
        }

        if (userIDPrincipal == null)
        {
            // no credential, but command works with an anonymous user
            LOGGER.debug("running as anon user");
@@ -112,28 +124,30 @@ public class CommandRunner
        }
        else
        {
                Subject subject = commandLineParser.getSubject();
                LOGGER.debug("running as " + subject);

                // augment the subject
                if (subject.getPrincipals().isEmpty())
            LOGGER.debug("running as " + userIDPrincipal.getName());
            final Set<Principal> userPrincipals = new HashSet<Principal>(1);
            userPrincipals.add(userIDPrincipal);
            PrincipalExtractor principalExtractor = new PrincipalExtractor()
            {
                    throw new RuntimeException("BUG: subject with no principals");
                }
                Principal userID = subject.getPrincipals().iterator().next();
                User<Principal> subjectUser =
                        userPersistence.getAugmentedUser(userID);
                for (Principal identity : subjectUser.getIdentities())
                public Set<Principal> getPrincipals()
                {
                    subject.getPrincipals().add(identity);
                    return userPrincipals;
                }
                LOGGER.debug("augmented subject: " + subject);
                Subject.doAs(subject, command);
                public X509CertificateChain getCertificateChain()
                {
                    return null;
                }
                public DelegationToken getDelegationToken()
                {
                    return null;
                }
        else
                public SSOCookieCredential getSSOCookieCredential()
                {
            throw new IllegalStateException("Not ready to proceed.");
                    return null;
                }
            };
            Subject subject = AuthenticationUtil.getSubject(principalExtractor);
            Subject.doAs(subject, command);
        }
    }
}
+21 −15
Original line number Diff line number Diff line
@@ -70,14 +70,11 @@
package ca.nrc.cadc.ac.admin;

import java.io.PrintStream;
import java.security.Principal;
import java.security.cert.CertificateException;

import javax.security.auth.Subject;
import org.apache.log4j.Logger;

import ca.nrc.cadc.ac.User;
import ca.nrc.cadc.ac.server.PluginFactory;
import org.apache.log4j.Logger;

/**
 * A command line admin tool for LDAP users.
@@ -117,14 +114,15 @@ public class Main
        try
        {
            main.execute(args);
            System.exit(0);
        }
        catch(UsageException | CertificateException e)
        {
            System.exit(0);
            System.exit(-1);
        }
        catch(Exception t)
        {
            System.exit(-1);
            System.exit(-2);
        }
    }

@@ -138,9 +136,16 @@ public class Main
    {
        try
        {

            final CmdLineParser parser = new CmdLineParser(args, systemOut,
                                                           systemErr);

            if (parser.isHelpCommand())
            {
                systemOut.println(CmdLineParser.getUsage());
            }
            else
            {
                // Set the necessary JNDI system property for lookups.
                System.setProperty("java.naming.factory.initial",
                                   ContextFactoryImpl.class.getName());
@@ -151,6 +156,7 @@ public class Main

                runner.run();
            }
        }
        catch(UsageException e)
        {
            systemErr.println("ERROR: " + e.getMessage());
+11 −13
Original line number Diff line number Diff line
@@ -68,13 +68,11 @@
 */
package ca.nrc.cadc.ac.server.ldap;

import ca.nrc.cadc.ac.Group;
import java.security.AccessControlException;
import java.security.Principal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Random;
@@ -82,11 +80,9 @@ import java.util.Set;

import javax.security.auth.x500.X500Principal;

import ca.nrc.cadc.auth.DNPrincipal;
import ca.nrc.cadc.util.StringUtil;
import com.unboundid.ldap.sdk.ModifyDNRequest;
import org.apache.log4j.Logger;

import ca.nrc.cadc.ac.Group;
import ca.nrc.cadc.ac.PersonalDetails;
import ca.nrc.cadc.ac.PosixDetails;
import ca.nrc.cadc.ac.Role;
@@ -97,26 +93,27 @@ import ca.nrc.cadc.ac.UserNotFoundException;
import ca.nrc.cadc.ac.UserRequest;
import ca.nrc.cadc.ac.client.GroupMemberships;
import ca.nrc.cadc.auth.AuthenticationUtil;
import ca.nrc.cadc.auth.DNPrincipal;
import ca.nrc.cadc.auth.HttpPrincipal;
import ca.nrc.cadc.auth.NumericPrincipal;
import ca.nrc.cadc.net.TransientException;
import ca.nrc.cadc.profiler.Profiler;
import ca.nrc.cadc.util.StringUtil;

import com.unboundid.ldap.sdk.AddRequest;
import com.unboundid.ldap.sdk.Attribute;
import com.unboundid.ldap.sdk.BindRequest;
import com.unboundid.ldap.sdk.BindResult;
import com.unboundid.ldap.sdk.Control;
import com.unboundid.ldap.sdk.DN;
import com.unboundid.ldap.sdk.DeleteRequest;
import com.unboundid.ldap.sdk.Filter;
import com.unboundid.ldap.sdk.LDAPConnection;
import com.unboundid.ldap.sdk.LDAPConnectionPool;
import com.unboundid.ldap.sdk.LDAPException;
import com.unboundid.ldap.sdk.LDAPResult;
import com.unboundid.ldap.sdk.LDAPSearchException;
import com.unboundid.ldap.sdk.Modification;
import com.unboundid.ldap.sdk.ModificationType;
import com.unboundid.ldap.sdk.ModifyDNRequest;
import com.unboundid.ldap.sdk.ModifyRequest;
import com.unboundid.ldap.sdk.ResultCode;
import com.unboundid.ldap.sdk.SearchRequest;
@@ -124,7 +121,6 @@ import com.unboundid.ldap.sdk.SearchResult;
import com.unboundid.ldap.sdk.SearchResultEntry;
import com.unboundid.ldap.sdk.SearchScope;
import com.unboundid.ldap.sdk.SimpleBindRequest;
import com.unboundid.ldap.sdk.controls.ProxiedAuthorizationV2RequestControl;
import com.unboundid.ldap.sdk.extensions.PasswordModifyExtendedRequest;
import com.unboundid.ldap.sdk.extensions.PasswordModifyExtendedResult;

@@ -505,6 +501,8 @@ public class LdapUserDAO<T extends Principal> extends LdapDAO

        String x500str = searchResult.getAttributeValue(userLdapAttrib.get(X500Principal.class));
        logger.debug("x500principal: " + x500str);

        if (x500str != null)
            user.getIdentities().add(new X500Principal(x500str));

        String fname = searchResult.getAttributeValue(LDAP_FIRST_NAME);