From ftweedal at redhat.com Sun Dec 11 20:58:08 2016 Content-Type: multipart/mixed; boundary="===============2185785071882326739==" MIME-Version: 1.0 From: Fraser Tweedale To: devel at lists.dogtagpki.org Subject: Re: [Pki-devel] [PATCH] 0139 Merge duplicate authz plugin code into superclass Date: Mon, 12 Dec 2016 11:58:04 +1000 Message-ID: <20161212015804.GF4232@dhcp-40-8.bne.redhat.com> In-Reply-To: 20161129090426.GD28337@dhcp-40-8.bne.redhat.com --===============2185785071882326739== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Acked by alee: https://github.com/frasertweedale/pki/commit/2d6e917470fce977d2537eba0b9ef2= ee17fd0a41 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 > 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..f95c98174a06dba9ebf3e4323= 8e566be2e6b5594 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 ACL Files > */ > -public abstract class AAclAuthz { > +public abstract class AAclAuthz implements IAuthzManager { > = > protected static final String PROP_CLASS =3D "class"; > protected static final String PROP_IMPL =3D "impl"; > @@ -69,6 +72,12 @@ public abstract class AAclAuthz { > = > protected static final String ACLS_ATTR =3D "aclResources"; > = > + /* name of this authorization manager instance */ > + private String mName =3D null; > + > + /* name of the authorization manager plugin */ > + private String mImplName =3D null; > + > private IConfigStore mConfig =3D null; > = > private Hashtable mACLs =3D new Hashtable(= ); > @@ -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 =3D name; > + mImplName =3D implName; > + mConfig =3D config; > mLogger =3D CMS.getLogger(); > CMS.debug("AAclAuthz: init begins"); > = > - mConfig =3D config; > - > // load access evaluators specified in the config file > IConfigStore mainConfig =3D CMS.getConfigStore(); > IConfigStore evalConfig =3D 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 desire= d, > * 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 =3D=3D 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 perman= ent > - * storage. > - *

> - */ > - 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 =3D 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 re= source, String operation) throws EBaseException; > + public AuthzToken authorize(IAuthToken authToken, String resource, S= tring operation) > + throws EAuthzInternalError, EAuthzAccessDenied { > + try { > + checkPermission(authToken, resource, operation); > + // compose AuthzToken > + AuthzToken authzToken =3D new AuthzToken(this); > + authzToken.set(AuthzToken.TOKEN_AUTHZ_RESOURCE, resource); > + authzToken.set(AuthzToken.TOKEN_AUTHZ_OPERATION, operation); > + authzToken.set(AuthzToken.TOKEN_AUTHZ_STATUS, AuthzToken.AUT= HZ_STATUS_SUCCESS); > + CMS.debug(mName + ": authorization passed"); > + return authzToken; > + } catch (EACLsException e) { > + // audit here later > + log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_A= UTHORIZATION_FAILED")); > + String params[] =3D { resource, operation }; > + log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCES= S_DENIED_2", params)); > + > + throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_ERROR")); > + } > + } > + > + public AuthzToken authorize(IAuthToken authToken, String expression) > + throws EAuthzAccessDenied { > + if (evaluateACLs(authToken, expression)) { > + return (new AuthzToken(this)); > + } else { > + String params[] =3D { expression }; > + throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_AUTHZ_ACCESS_DENIED", params)); > + } > + } > = > public String getOrder() { > IConfigStore mainConfig =3D CMS.getConfigStore(); > diff --git a/base/server/cms/src/com/netscape/cms/authorization/BasicAclA= uthz.java b/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuth= z.java > index c883758b39ee018ab6aeb82bdfb5242bcc32c439..6b33c2041d0b41ac5db31c3eb= f8a3ae1d33632b9 100644 > --- a/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.ja= va > +++ b/base/server/cms/src/com/netscape/cms/authorization/BasicAclAuthz.ja= va > @@ -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 =3D null; > - > - /* name of the authorization manager plugin */ > - private String mImplName =3D null; > - > - /* configuration store */ > - @SuppressWarnings("unused") > - private IConfigStore mConfig; > - > - /* the system logger */ > - private ILogger mLogger =3D null; > - > - protected static final String PROP_BASEDN =3D "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 =3D name; > - mImplName =3D implName; > - mConfig =3D config; > - mLogger =3D 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 > - *

> - * Example: > - *

> - * For example, if UsrGrpAdminServlet needs to authorize the caller = it would do be done in the following fashion: > - * > - * 

> -     * try {
> -     *     authzTok =3D mAuthz.authorize("DirACLBasedAuthz", a=
uthToken, RES_GROUP, "read");
> -     * } catch (EBaseException e) {
> -     *     log(ILogger.LL_FAILURE, "authorize call: " + e.toSt=
ring());
> -     * }
> -     *
> - * > - * @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, S= tring operation) > - throws EAuthzInternalError, EAuthzAccessDenied { > - AuthzToken authzToken =3D 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_A= UTHORIZATION_FAILED")); > - String params[] =3D { resource, operation }; > - log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCES= S_DENIED_2", params)); > - > - throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_ERROR")); > - } > - > - return authzToken; > - } > - > - public AuthzToken authorize(IAuthToken authToken, String expression) > - throws EAuthzAccessDenied { > - if (evaluateACLs(authToken, expression)) { > - return (new AuthzToken(this)); > - } else { > - String params[] =3D { expression }; > - throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_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_F= LUSH_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 =3D=3D null) > - return; > - mLogger.log(ILogger.EV_SYSTEM, null, ILogger.S_AUTHORIZATION, > - level, msg); > - } > } > diff --git a/base/server/cms/src/com/netscape/cms/authorization/DirAclAut= hz.java b/base/server/cms/src/com/netscape/cms/authorization/DirAclAuthz.ja= va > index 4f14f4c4098c31bdad8b85260a1ea14b1c917f52..bcb81f3d0e390545fed2fbf53= 0cf9b57e6bc48ea 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 =3D null; > - > - /* name of the authentication manager plugin */ > - private String mImplName =3D null; > - > - /* configuration store */ > - private IConfigStore mConfig; > - > - /* the system logger */ > - private ILogger mLogger =3D null; > - > protected static final String PROP_BASEDN =3D "basedn"; > = > private ILdapConnFactory mLdapConnFactory =3D null; > @@ -118,15 +104,10 @@ public class DirAclAuthz extends AAclAuthz > */ > public void init(String name, String implName, IConfigStore config) > throws EBaseException { > - mName =3D name; > - mImplName =3D implName; > - mConfig =3D config; > - mLogger =3D CMS.getLogger(); > - > - super.init(config); > + super.init(name, implName, config); > = > // initialize LDAP connection factory > - IConfigStore ldapConfig =3D mConfig.getSubStore("ldap"); > + IConfigStore ldapConfig =3D config.getSubStore("ldap"); > = > if (ldapConfig =3D=3D 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 > - *

> - * Example: > - *

> - * For example, if UsrGrpAdminServlet needs to authorize the caller = it would do be done in the following fashion: > - * > - * 

> -     * try {
> -     *     authzTok =3D mAuthz.authorize("DirAclAuthz", authTo=
ken, RES_GROUP, "read");
> -     * } catch (EBaseException e) {
> -     *     log(ILogger.LL_FAILURE, "authorize call: " + e.toSt=
ring());
> -     * }
> -     *
> - * > - * @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, S= tring operation) > - throws EAuthzInternalError, EAuthzAccessDenied { > - AuthzToken authzToken =3D 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.AUT= HZ_STATUS_SUCCESS); > - CMS.debug("DirAclAuthz: authorization passed"); > - } catch (EACLsException e) { > - // audit here later > - log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_EVALUATOR_A= UTHORIZATION_FAILED")); > - String params[] =3D { resource, operation }; > - log(ILogger.LL_FAILURE, CMS.getLogMessage("AUTHZ_AUTHZ_ACCES= S_DENIED_2", params)); > - > - throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_ERROR")); > - } > - > - return authzToken; > - } > - > - public AuthzToken authorize(IAuthToken authToken, String expression) > - throws EAuthzAccessDenied { > - if (evaluateACLs(authToken, expression)) { > - return (new AuthzToken(this)); > - } else { > - String params[] =3D { expression }; > - throw new EAuthzAccessDenied(CMS.getUserMessage("CMS_AUTHORI= ZATION_AUTHZ_ACCESS_DENIED", params)); > - } > - } > - > - /** > * update acls. when memory update is done, flush to ldap. > *

> * Currently, it is possible that when the memory is updated success= fully, 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 =3D=3D null) > - return; > - mLogger.log(ILogger.EV_SYSTEM, null, ILogger.S_AUTHORIZATION, > - level, msg); > - } > } > -- = > 2.7.4 > = > _______________________________________________ > Pki-devel mailing list > Pki-devel(a)redhat.com > https://www.redhat.com/mailman/listinfo/pki-devel --===============2185785071882326739==--