[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pki-devel] [PATCH] 0139 Merge duplicate authz plugin code into superclass



Acked by alee:
https://github.com/frasertweedale/pki/commit/2d6e917470fce977d2537eba0b9ef2ee17fd0a41

Pushed to master (bfcf597d569e24fe6ec60062e37908c62bcff76)

On Tue, Nov 29, 2016 at 07:04:26PM +1000, Fraser Tweedale wrote:
> The attached patch merges some duplicate authz manager code into the
> existing AAclAuthz superclass.
> 
> It simplifies things if we end up adding a new authz manager as part
> of external authentication / GSS-API support.  But it's a nice
> refactor to do anyway :)
> 
> Thanks,
> Fraser

> From afc5fc3da5f1ea61305fb237e002bbe8b3d26e8c Mon Sep 17 00:00:00 2001
> From: Fraser Tweedale <ftweedal redhat com>
> Date: Fri, 25 Nov 2016 14:29:40 +1000
> Subject: [PATCH 139/141] Merge duplicate authz plugin code into superclass
> 
> DirAclAuthz and BasicAclAuthz both extend AAclAuthz, but there is
> still a lot of duplicate code.  Push the duplicated bits up into the
> AAclAuthz.
> 
> Also remove abstract method flushResourceACLs() from AAclAuthz, and
> its implementation from BasicAclAuthz, because it is only
> implemented (meaningfully) by DirAclAuthz.
> 
> Part of: https://fedorahosted.org/pki/ticket/1359
> ---
>  .../com/netscape/cms/authorization/AAclAuthz.java  |  93 ++++++++++---
>  .../netscape/cms/authorization/BasicAclAuthz.java  | 144 +--------------------
>  .../netscape/cms/authorization/DirAclAuthz.java    | 105 +--------------
>  3 files changed, 78 insertions(+), 264 deletions(-)
> 
> diff --git a/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java b/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
> index b3e447cfca49951fe78f6b4896652921ffc43406..f95c98174a06dba9ebf3e43238e566be2e6b5594 100644
> --- a/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
> +++ b/base/server/cms/src/com/netscape/cms/authorization/AAclAuthz.java
> @@ -30,6 +30,9 @@ import com.netscape.certsrv.acls.IACL;
>  import com.netscape.certsrv.apps.CMS;
>  import com.netscape.certsrv.authentication.IAuthToken;
>  import com.netscape.certsrv.authorization.AuthzToken;
> +import com.netscape.certsrv.authorization.EAuthzAccessDenied;
> +import com.netscape.certsrv.authorization.EAuthzInternalError;
> +import com.netscape.certsrv.authorization.IAuthzManager;
>  import com.netscape.certsrv.base.EBaseException;
>  import com.netscape.certsrv.base.IConfigStore;
>  import com.netscape.certsrv.evaluators.IAccessEvaluator;
> @@ -61,7 +64,7 @@ import com.netscape.cmsutil.util.Utils;
>   * @version $Revision$, $Date$
>   * @see <A HREF="http://developer.netscape.com/library/documentation/enterprise/admnunix/aclfiles.htm";>ACL Files</A>
>   */
> -public abstract class AAclAuthz {
> +public abstract class AAclAuthz implements IAuthzManager {
>  
>      protected static final String PROP_CLASS = "class";
>      protected static final String PROP_IMPL = "impl";
> @@ -69,6 +72,12 @@ public abstract class AAclAuthz {
>  
>      protected static final String ACLS_ATTR = "aclResources";
>  
> +    /* name of this authorization manager instance */
> +    private String mName = null;
> +
> +    /* name of the authorization manager plugin */
> +    private String mImplName = null;
> +
>      private IConfigStore mConfig = null;
>  
>      private Hashtable<String, ACL> mACLs = new Hashtable<String, ACL>();
> @@ -93,14 +102,14 @@ public abstract class AAclAuthz {
>      /**
>       * Initializes
>       */
> -    protected void init(IConfigStore config)
> +    public void init(String name, String implName, IConfigStore config)
>              throws EBaseException {
> -
> +        mName = name;
> +        mImplName = implName;
> +        mConfig = config;
>          mLogger = CMS.getLogger();
>          CMS.debug("AAclAuthz: init begins");
>  
> -        mConfig = config;
> -
>          // load access evaluators specified in the config file
>          IConfigStore mainConfig = CMS.getConfigStore();
>          IConfigStore evalConfig = mainConfig.getSubStore(PROP_EVAL);
> @@ -144,6 +153,20 @@ public abstract class AAclAuthz {
>      }
>  
>      /**
> +     * gets the name of this authorization manager instance
> +     */
> +    public String getName() {
> +        return mName;
> +    }
> +
> +    /**
> +     * gets the plugin name of this authorization manager.
> +     */
> +    public String getImplName() {
> +        return mImplName;
> +    }
> +
> +    /**
>       * Parse ACL resource attributes, then update the ACLs memory store
>       * This is intended to be used if storing ACLs on ldap is not desired,
>       * and the caller is expected to call this method to add resource
> @@ -818,7 +841,7 @@ public abstract class AAclAuthz {
>          }
>      }
>  
> -    private void log(int level, String msg) {
> +    protected void log(int level, String msg) {
>          if (mLogger == null)
>              return;
>          mLogger.log(ILogger.EV_SYSTEM, null, ILogger.S_AUTHORIZATION,
> @@ -830,24 +853,58 @@ public abstract class AAclAuthz {
>       **********************************/
>  
>      /**
> -     * update acls. called after memory upate is done to flush to permanent
> -     * storage.
> -     * <p>
> -     */
> -    protected abstract void flushResourceACLs() throws EACLsException;
> -
> -    /**
> -     * an abstract class that enforces implementation of the
> -     * authorize() method that will authorize an operation on a
> -     * particular resource
> +     * check the authorization permission for the user associated with
> +     * authToken on operation
> +     *
> +     * Example:
> +     *
> +     * For example, if UsrGrpAdminServlet needs to authorize the
> +     * caller it would do be done in the following fashion:
> +     *
> +     * try {
> +     *     authzTok = mAuthz.authorize(
> +     *         "DirAclAuthz", authToken, RES_GROUP, "read");
> +     * } catch (EBaseException e) {
> +     *     log(ILogger.LL_FAILURE, "authorize call: " + e.toString());
> +     * }
>       *
>       * @param authToken the authToken associated with a user
>       * @param resource - the protected resource name
>       * @param operation - the protected resource operation name
> -     * @exception EBaseException If an internal error occurred.
> +     * @exception EAuthzAccessDenied If access was denied
> +     * @exception EAuthzInternalError If an internal error occurred.
>       * @return authzToken
>       */
> -    public abstract AuthzToken authorize(IAuthToken authToken, String resource, String operation) throws EBaseException;
> +    public AuthzToken authorize(IAuthToken authToken, String resource, String operation)
> +            throws EAuthzInternalError, EAuthzAccessDenied {
> +        try {
> +            checkPermission(authToken, resource, operation);
> +            // compose AuthzToken
> +            AuthzToken authzToken = new AuthzToken(this);
> +            authzToken.set(AuthzToken.TOKEN_AUTHZ_RESOURCE, resource);
> +            authzToken.set(AuthzToken.TOKEN_AUTHZ_OPERATION, operation);
> +            authzToken.set(AuthzToken.TOKEN_AUTHZ_STATUS, AuthzToken.AUTHZ_STATUS_SUCCESS);
> +            CMS.debug(mName + ": authorization passed");
> +            return authzToken;
> +        } catch (EACLsException e) {
> +            // audit here later
> +            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_AUTHORIZATION_FAILED"));
> +            String params[] = { resource, operation };
> +            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCESS_DENIED_2", params));
> +
> +            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_ERROR"));
> +        }
> +    }
> +
> +    public AuthzToken authorize(IAuthToken authToken, String expression)
> +            throws EAuthzAccessDenied {
> +        if (evaluateACLs(authToken, expression)) {
> +            return (new AuthzToken(this));
> +        } else {
> +            String params[] = { expression };
> +            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_AUTHZ_ACCESS_DENIED", params));
> +        }
> +    }
>  
>      public String getOrder() {
>          IConfigStore mainConfig = CMS.getConfigStore();
> diff --git a/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.java b/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.java
> index c883758b39ee018ab6aeb82bdfb5242bcc32c439..6b33c2041d0b41ac5db31c3ebf8a3ae1d33632b9 100644
> --- a/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.java
> +++ b/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.java
> @@ -18,12 +18,7 @@
>  package com.netscape.cms.authorization;
>  
>  // cert server imports.
> -import com.netscape.certsrv.acls.EACLsException;
>  import com.netscape.certsrv.apps.CMS;
> -import com.netscape.certsrv.authentication.IAuthToken;
> -import com.netscape.certsrv.authorization.AuthzToken;
> -import com.netscape.certsrv.authorization.EAuthzAccessDenied;
> -import com.netscape.certsrv.authorization.EAuthzInternalError;
>  import com.netscape.certsrv.authorization.IAuthzManager;
>  import com.netscape.certsrv.base.EBaseException;
>  import com.netscape.certsrv.base.IConfigStore;
> @@ -38,23 +33,6 @@ import com.netscape.certsrv.logging.ILogger;
>  public class BasicAclAuthz extends AAclAuthz
>          implements IAuthzManager, IExtendedPluginInfo {
>  
> -    // members
> -
> -    /* name of this authorization manager instance */
> -    private String mName = null;
> -
> -    /* name of the authorization manager plugin */
> -    private String mImplName = null;
> -
> -    /* configuration store */
> -    @SuppressWarnings("unused")
> -    private IConfigStore mConfig;
> -
> -    /* the system logger */
> -    private ILogger mLogger = null;
> -
> -    protected static final String PROP_BASEDN = "basedn";
> -
>      static {
>          mExtendedPluginInfo.add("nothing for now");
>      }
> @@ -80,135 +58,15 @@ public class BasicAclAuthz extends AAclAuthz
>       */
>      public void init(String name, String implName, IConfigStore config)
>              throws EBaseException {
> -        mName = name;
> -        mImplName = implName;
> -        mConfig = config;
> -        mLogger = CMS.getLogger();
> -
> -        super.init(config);
> +        super.init(name, implName, config);
>  
>          log(ILogger.LL_INFO, "initialization done");
>      }
>  
>      /**
> -     * gets the name of this authorization manager instance
> -     */
> -    public String getName() {
> -        return mName;
> -    }
> -
> -    /**
> -     * gets the plugin name of this authorization manager.
> -     */
> -    public String getImplName() {
> -        return mImplName;
> -    }
> -
> -    /**
> -     * check the authorization permission for the user associated with
> -     * authToken on operation
> -     * <p>
> -     * Example:
> -     * <p>
> -     * For example, if UsrGrpAdminServlet needs to authorize the caller it would do be done in the following fashion:
> -     *
> -     * <PRE>
> -     * try {
> -     *     authzTok = mAuthz.authorize(&quot;DirACLBasedAuthz&quot;, authToken, RES_GROUP, &quot;read&quot;);
> -     * } catch (EBaseException e) {
> -     *     log(ILogger.LL_FAILURE, &quot;authorize call: &quot; + e.toString());
> -     * }
> -     * </PRE>
> -     *
> -     * @param authToken the authToken associated with a user
> -     * @param resource - the protected resource name
> -     * @param operation - the protected resource operation name
> -     * @exception EAuthzInternalError if an internal error occurred.
> -     * @exception EAuthzAccessDenied if access denied
> -     * @return authzToken if success
> -     */
> -    public AuthzToken authorize(IAuthToken authToken, String resource, String operation)
> -            throws EAuthzInternalError, EAuthzAccessDenied {
> -        AuthzToken authzToken = new AuthzToken(this);
> -
> -        try {
> -            checkPermission(authToken, resource, operation);
> -
> -            CMS.debug("BasicAclAuthz: authorization passed");
> -
> -            // compose AuthzToken
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_RESOURCE, resource);
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_OPERATION, operation);
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_STATUS,
> -                    AuthzToken.AUTHZ_STATUS_SUCCESS);
> -        } catch (EACLsException e) {
> -            // audit here later
> -            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_AUTHORIZATION_FAILED"));
> -            String params[] = { resource, operation };
> -            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCESS_DENIED_2", params));
> -
> -            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_ERROR"));
> -        }
> -
> -        return authzToken;
> -    }
> -
> -    public AuthzToken authorize(IAuthToken authToken, String expression)
> -            throws EAuthzAccessDenied {
> -        if (evaluateACLs(authToken, expression)) {
> -            return (new AuthzToken(this));
> -        } else {
> -            String params[] = { expression };
> -            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_AUTHZ_ACCESS_DENIED", params));
> -        }
> -    }
> -
> -    /**
> -     * This currently does not flush to permanent storage
> -     *
> -     * @param id is the resource id
> -     * @param strACLs
> -     */
> -    public void updateACLs(String id, String rights, String strACLs,
> -            String desc) throws EACLsException {
> -        try {
> -            super.updateACLs(id, rights, strACLs, desc);
> -            //            flushResourceACLs();
> -        } catch (EACLsException ex) {
> -
> -            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_FLUSH_RESOURCES", ex.toString()));
> -
> -            throw new EACLsException(CMS.getUserMessage("CMS_ACL_UPDATE_FAIL"));
> -        }
> -    }
> -
> -    /**
> -     * updates resourceACLs to permanent storage.
> -     * currently not implemented for this authzMgr
> -     */
> -    protected void flushResourceACLs() throws EACLsException {
> -        log(ILogger.LL_FAILURE, "flushResourceACL() is not implemented");
> -        throw new EACLsException(CMS.getUserMessage("CMS_ACL_METHOD_NOT_IMPLEMENTED"));
> -    }
> -
> -    /**
>       * graceful shutdown
>       */
>      public void shutdown() {
>          log(ILogger.LL_INFO, "shutting down");
>      }
> -
> -    /**
> -     * Logs a message for this class in the system log file.
> -     *
> -     * @param level The log level.
> -     * @param msg The message to log.
> -     * @see com.netscape.certsrv.logging.ILogger
> -     */
> -    protected void log(int level, String msg) {
> -        if (mLogger == null)
> -            return;
> -        mLogger.log(ILogger.EV_SYSTEM, null, ILogger.S_AUTHORIZATION,
> -                level, msg);
> -    }
>  }
> diff --git a/base/server/cms/src/com/netscape/cms/authorization/DirAclAuthz.java b/base/server/cms/src/com/netscape/cms/authorization/DirAclAuthz.java
> index 4f14f4c4098c31bdad8b85260a1ea14b1c917f52..bcb81f3d0e390545fed2fbf530cf9b57e6bc48ea 100644
> --- a/base/server/cms/src/com/netscape/cms/authorization/DirAclAuthz.java
> +++ b/base/server/cms/src/com/netscape/cms/authorization/DirAclAuthz.java
> @@ -24,8 +24,6 @@ import com.netscape.certsrv.acls.EACLsException;
>  import com.netscape.certsrv.apps.CMS;
>  import com.netscape.certsrv.authentication.IAuthToken;
>  import com.netscape.certsrv.authorization.AuthzToken;
> -import com.netscape.certsrv.authorization.EAuthzAccessDenied;
> -import com.netscape.certsrv.authorization.EAuthzInternalError;
>  import com.netscape.certsrv.authorization.IAuthzManager;
>  import com.netscape.certsrv.base.EBaseException;
>  import com.netscape.certsrv.base.IConfigStore;
> @@ -54,18 +52,6 @@ public class DirAclAuthz extends AAclAuthz
>  
>      // members
>  
> -    /* name of this authentication manager instance */
> -    private String mName = null;
> -
> -    /* name of the authentication manager plugin */
> -    private String mImplName = null;
> -
> -    /* configuration store */
> -    private IConfigStore mConfig;
> -
> -    /* the system logger */
> -    private ILogger mLogger = null;
> -
>      protected static final String PROP_BASEDN = "basedn";
>  
>      private ILdapConnFactory mLdapConnFactory = null;
> @@ -118,15 +104,10 @@ public class DirAclAuthz extends AAclAuthz
>       */
>      public void init(String name, String implName, IConfigStore config)
>              throws EBaseException {
> -        mName = name;
> -        mImplName = implName;
> -        mConfig = config;
> -        mLogger = CMS.getLogger();
> -
> -        super.init(config);
> +        super.init(name, implName, config);
>  
>          // initialize LDAP connection factory
> -        IConfigStore ldapConfig = mConfig.getSubStore("ldap");
> +        IConfigStore ldapConfig = config.getSubStore("ldap");
>  
>          if (ldapConfig == null) {
>              log(ILogger.LL_MISCONF, "failed to get config ldap info");
> @@ -186,75 +167,6 @@ public class DirAclAuthz extends AAclAuthz
>      }
>  
>      /**
> -     * gets the name of this authorization manager instance
> -     */
> -    public String getName() {
> -        return mName;
> -    }
> -
> -    /**
> -     * gets the plugin name of this authorization manager.
> -     */
> -    public String getImplName() {
> -        return mImplName;
> -    }
> -
> -    /**
> -     * check the authorization permission for the user associated with
> -     * authToken on operation
> -     * <p>
> -     * Example:
> -     * <p>
> -     * For example, if UsrGrpAdminServlet needs to authorize the caller it would do be done in the following fashion:
> -     *
> -     * <PRE>
> -     * try {
> -     *     authzTok = mAuthz.authorize(&quot;DirAclAuthz&quot;, authToken, RES_GROUP, &quot;read&quot;);
> -     * } catch (EBaseException e) {
> -     *     log(ILogger.LL_FAILURE, &quot;authorize call: &quot; + e.toString());
> -     * }
> -     * </PRE>
> -     *
> -     * @param authToken the authToken associated with a user
> -     * @param resource - the protected resource name
> -     * @param operation - the protected resource operation name
> -     * @exception EBaseException If an internal error occurred.
> -     * @return authzToken
> -     */
> -    public AuthzToken authorize(IAuthToken authToken, String resource, String operation)
> -            throws EAuthzInternalError, EAuthzAccessDenied {
> -        AuthzToken authzToken = new AuthzToken(this);
> -
> -        try {
> -            checkPermission(authToken, resource, operation);
> -            // compose AuthzToken
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_RESOURCE, resource);
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_OPERATION, operation);
> -            authzToken.set(AuthzToken.TOKEN_AUTHZ_STATUS, AuthzToken.AUTHZ_STATUS_SUCCESS);
> -            CMS.debug("DirAclAuthz: authorization passed");
> -        } catch (EACLsException e) {
> -            // audit here later
> -            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_AUTHORIZATION_FAILED"));
> -            String params[] = { resource, operation };
> -            log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCESS_DENIED_2", params));
> -
> -            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_ERROR"));
> -        }
> -
> -        return authzToken;
> -    }
> -
> -    public AuthzToken authorize(IAuthToken authToken, String expression)
> -            throws EAuthzAccessDenied {
> -        if (evaluateACLs(authToken, expression)) {
> -            return (new AuthzToken(this));
> -        } else {
> -            String params[] = { expression };
> -            throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORIZATION_AUTHZ_ACCESS_DENIED", params));
> -        }
> -    }
> -
> -    /**
>       * update acls. when memory update is done, flush to ldap.
>       * <p>
>       * Currently, it is possible that when the memory is updated successfully, and the ldap isn't, the memory upates
> @@ -353,17 +265,4 @@ public class DirAclAuthz extends AAclAuthz
>          }
>      }
>  
> -    /**
> -     * Logs a message for this class in the system log file.
> -     *
> -     * @param level The log level.
> -     * @param msg The message to log.
> -     * @see com.netscape.certsrv.logging.ILogger
> -     */
> -    protected void log(int level, String msg) {
> -        if (mLogger == null)
> -            return;
> -        mLogger.log(ILogger.EV_SYSTEM, null, ILogger.S_AUTHORIZATION,
> -                level, msg);
> -    }
>  }
> -- 
> 2.7.4
> 

> _______________________________________________
> Pki-devel mailing list
> Pki-devel redhat com
> https://www.redhat.com/mailman/listinfo/pki-devel


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]