Author: julien(a)jboss.com Date: 2007-06-26 09:25:00 -0400 (Tue, 26 Jun 2007) New Revision: 7548 Modified: trunk/core/build.xml trunk/core/src/main/org/jboss/portal/core/impl/model/instance/InstanceContainerImpl.java trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/persistent-jboss-beans.xml trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/transient-jboss-beans.xml trunk/security/src/main/org/jboss/portal/security/impl/jacc/JACCPortalAuthorizationManager.java trunk/security/src/main/org/jboss/portal/security/spi/auth/PortalAuthorizationManager.java Log: - improve implementation of security checks in instance container - no jacc policy context id was leading to NPE wrapped in PortalSecurityException, now we do an explicit check about the existence of the policycontextid and throw an exception ahead in the call stack - clarified contract of checkPermission(Permission p, ...) to add an IAE if no permission is provided Modified: trunk/core/build.xml =================================================================== --- trunk/core/build.xml 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/core/build.xml 2007-06-26 13:25:00 UTC (rev 7548) @@ -561,21 +561,25 @@ <parameter name="CacheNaturalId" value="true"/> <parameter name="Config" value="persistent-jboss-beans.xml"/> </zest> +<!-- <zest todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalNodeTestCase" outfile="TEST-PortalObjectContainerTestCase"> <parameter name="CacheNaturalId" value="true"/> <parameter name="Config" value="transient-jboss-beans.xml"/> </zest> +--> <zest todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalObjectContainerTestCase" outfile="TEST-PortalObjectContainerTestCase"> <parameter name="CacheNaturalId" value="true"/> <parameter name="Config" value="persistent-jboss-beans.xml"/> </zest> +<!-- <zest todir="${test.reports}" name="org.jboss.portal.test.core.model.portal.PortalObjectContainerTestCase" outfile="TEST-PortalObjectContainerTestCase"> <parameter name="CacheNaturalId" value="true"/> <parameter name="Config" value="transient-jboss-beans.xml"/> </zest> +--> <zest todir="${test.reports}" name="org.jboss.portal.test.core.model.instance.InstanceContainerTestCase" outfile="TEST-PersistedLocally-ClonedOnCreate-InstanceContainerTestCase"> <parameter name="PersistLocally" value="true"/> Modified: trunk/core/src/main/org/jboss/portal/core/impl/model/instance/InstanceContainerImpl.java =================================================================== --- trunk/core/src/main/org/jboss/portal/core/impl/model/instance/InstanceContainerImpl.java 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/core/src/main/org/jboss/portal/core/impl/model/instance/InstanceContainerImpl.java 2007-06-26 13:25:00 UTC (rev 7548) @@ -45,6 +45,7 @@ import org.jboss.portal.security.PortalPermissionCollection; import org.jboss.portal.security.impl.JBossAuthorizationDomainRegistry; import org.jboss.portal.security.spi.auth.PortalAuthorizationManagerFactory; +import org.jboss.portal.security.spi.auth.PortalAuthorizationManager; import org.jboss.portal.security.spi.provider.AuthorizationDomain; import org.jboss.portal.security.spi.provider.DomainConfigurator; import org.jboss.portal.security.spi.provider.PermissionFactory; @@ -103,12 +104,21 @@ } }; + /** Used to bypass security checks for testing. */ + protected boolean performSecurityChecks; + /** If true clone the portlet on an instance creation. */ protected boolean cloneOnCreate; /** The container context. */ protected JBossInstanceContainerContext containerContext; + public InstanceContainerImpl() + { + performSecurityChecks = true; + cloneOnCreate = false; + } + public InterceptorStackFactory getStackFactory() { return stackFactory; @@ -203,6 +213,16 @@ this.cloneOnCreate = cloneOnCreate; } + public boolean getPerformSecurityChecks() + { + return performSecurityChecks; + } + + public void setPerformSecurityChecks(boolean performSecurityChecks) + { + this.performSecurityChecks = performSecurityChecks; + } + public JBossInstanceContainerContext getContainerContext() { return containerContext; @@ -361,21 +381,23 @@ public Collection getDefinitions() { - // -// PortalAuthorizationManager mgr = portalAuthorizationManagerFactory.getManager(); - - // Collection list = containerContext.getInstanceDefinitions(); // Filter the list - for (Iterator i = list.iterator();i.hasNext();) + if (performSecurityChecks) { - Instance instance = (Instance)i.next(); - InstancePermission perm = new InstancePermission(instance.getId(), InstancePermission.VIEW_ACTION); -// if (mgr.checkPermission(perm) == false) -// { -// i.remove(); -// } + PortalAuthorizationManager mgr = portalAuthorizationManagerFactory.getManager(); + + // + for (Iterator i = list.iterator();i.hasNext();) + { + Instance instance = (Instance)i.next(); + InstancePermission perm = new InstancePermission(instance.getId(), InstancePermission.VIEW_ACTION); + if (!mgr.checkPermission(perm)) + { + i.remove(); + } + } } // Modified: trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java =================================================================== --- trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/core/src/main/org/jboss/portal/test/core/model/instance/InstanceContainerTestCase.java 2007-06-26 13:25:00 UTC (rev 7548) @@ -298,6 +298,16 @@ return "org/jboss/portal/test/core/model/instance/"; } + protected void setUp() throws Exception + { + super.setUp(); + } + + protected void tearDown() throws Exception + { + super.tearDown(); + } + public void testConfigureInstance() throws Exception { PortletInvokerSupport.InternalPortlet internalPortlet = portletContainer.addInternalPortlet("MyPortlet", new PortletSupport()); Modified: trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/persistent-jboss-beans.xml =================================================================== --- trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/persistent-jboss-beans.xml 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/persistent-jboss-beans.xml 2007-06-26 13:25:00 UTC (rev 7548) @@ -111,6 +111,7 @@ <property name="portletInvoker"><inject bean="Producer"/></property> <property name="containerContext"><inject bean="ContainerContext"/></property> <property name="stackFactory"><inject bean="StackFactory"/></property> + <property name="performSecurityChecks">false</property> </bean> <bean name="TestBean" class="org.jboss.portal.test.core.model.instance.InstanceContainerTestCase"> Modified: trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/transient-jboss-beans.xml =================================================================== --- trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/transient-jboss-beans.xml 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/core/src/resources/portal-core-test-jar/org/jboss/portal/test/core/model/instance/transient-jboss-beans.xml 2007-06-26 13:25:00 UTC (rev 7548) @@ -109,6 +109,7 @@ <property name="portletInvoker"><inject bean="Producer"/></property> <property name="containerContext"><inject bean="ContainerContext"/></property> <property name="stackFactory"><inject bean="StackFactory"/></property> + <property name="performSecurityChecks">false</property> </bean> <bean name="TestBean" class="org.jboss.portal.test.core.model.instance.InstanceContainerTestCase"> Modified: trunk/security/src/main/org/jboss/portal/security/impl/jacc/JACCPortalAuthorizationManager.java =================================================================== --- trunk/security/src/main/org/jboss/portal/security/impl/jacc/JACCPortalAuthorizationManager.java 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/security/src/main/org/jboss/portal/security/impl/jacc/JACCPortalAuthorizationManager.java 2007-06-26 13:25:00 UTC (rev 7548) @@ -82,7 +82,7 @@ } } - public void checkRoleConfig(String contextID, String roleName) throws Exception + public void checkRoleConfig(String contextID, String roleName) throws PortalSecurityException { Map configuredRoles = factory.configuredRoles; synchronized (configuredRoles) @@ -91,57 +91,83 @@ PolicyConfiguration pc = null; try { - if (!configuredRoles.containsKey(roleName)) + try { - // Iterate over all domains to add their container - Collection domains = factory.getAuthorizationDomainRegistry().getDomains(); - for (Iterator j = domains.iterator(); j.hasNext();) + if (!configuredRoles.containsKey(roleName)) { - AuthorizationDomain domain = (AuthorizationDomain)j.next(); - JACCPortalPermissionCollection collection = new JACCPortalPermissionCollection(roleName, domain); - PermissionFactory permissionFactory = domain.getPermissionFactory(); - PortalPermission container = permissionFactory.createPermissionContainer(collection); + // Iterate over all domains to add their container + Collection domains = factory.getAuthorizationDomainRegistry().getDomains(); + for (Iterator j = domains.iterator(); j.hasNext();) + { + AuthorizationDomain domain = (AuthorizationDomain)j.next(); + JACCPortalPermissionCollection collection = new JACCPortalPermissionCollection(roleName, domain); + PermissionFactory permissionFactory = domain.getPermissionFactory(); + PortalPermission container = permissionFactory.createPermissionContainer(collection); - if (pc == null) - { - pc = pcf.getPolicyConfiguration(contextID, false); + if (pc == null) + { + pc = pcf.getPolicyConfiguration(contextID, false); + } + + if (SecurityConstants.UNCHECKED_ROLE_NAME.equals(roleName)) + { + pc.addToUncheckedPolicy(container); + } + else + { + pc.addToRole(roleName, container); + } } - if (SecurityConstants.UNCHECKED_ROLE_NAME.equals(roleName)) - { - pc.addToUncheckedPolicy(container); - } - else - { - pc.addToRole(roleName, container); - } + // + configuredRoles.put(roleName, roleName); } - - // - configuredRoles.put(roleName, roleName); } + catch (PolicyContextException e) + { + throw new PortalSecurityException(e); + } } finally { // JBossSX implies check does not happen until the PC has committed // and the policy has been refreshed - if (pc != null && !pc.inService()) + try { - pc.commit(); - Policy.getPolicy().refresh(); + if (pc != null && !pc.inService()) + { + pc.commit(); + Policy.getPolicy().refresh(); + } } + catch (PolicyContextException e) + { + log.error("Error when commiting policy config", e); + } } } } /** - * + * @throws IllegalArgumentException if the permission is null + * @throws PortalSecurityException */ - private boolean internalCheckPermission(PortalPermission permission) throws Exception + private boolean internalCheckPermission(PortalPermission permission) throws IllegalArgumentException, PortalSecurityException { + if (permission == null) + { + throw new IllegalArgumentException("No null permission can be checked"); + } + // Get the current context id. String contextID = PolicyContext.getContextID(); + // + if (contextID == null) + { + throw new PortalSecurityException("No policy context id"); + } + // Get the current authenticated subject through the JACC contract Subject currentSubject = (Subject)checkedSubjectLocal.get(); @@ -190,13 +216,14 @@ } - public boolean checkPermission(Subject checkedSubject, PortalPermission permission) throws PortalSecurityException + public boolean checkPermission(Subject checkedSubject, PortalPermission permission) throws IllegalArgumentException, PortalSecurityException { try { - // Set the subject for later use in that layer checkedSubjectLocal.set(checkedSubject); + + // if (trace) { log.trace("hasPermission:uri=" + permission.getURI() + "::action=" + permission.getType() + "::type=" + permission.getType()); @@ -204,6 +231,8 @@ // boolean result = internalCheckPermission(permission); + + // if (trace) { log.trace("hasPermission:result=" + result); @@ -212,20 +241,13 @@ // return result; } - catch (Exception e) - { - log.trace("hasPermission:error", e); - - // - throw new PortalSecurityException(e); - } finally { checkedSubjectLocal.set(null); } } - public boolean checkPermission(PortalPermission permission) throws PortalSecurityException + public boolean checkPermission(PortalPermission permission) throws IllegalArgumentException, PortalSecurityException { try { Modified: trunk/security/src/main/org/jboss/portal/security/spi/auth/PortalAuthorizationManager.java =================================================================== --- trunk/security/src/main/org/jboss/portal/security/spi/auth/PortalAuthorizationManager.java 2007-06-26 09:57:20 UTC (rev 7547) +++ trunk/security/src/main/org/jboss/portal/security/spi/auth/PortalAuthorizationManager.java 2007-06-26 13:25:00 UTC (rev 7548) @@ -39,14 +39,16 @@ * @param permission * @return * @throws PortalSecurityException + * @throws IllegalArgumentException if the permission is null */ - public boolean checkPermission(PortalPermission permission) throws PortalSecurityException; + public boolean checkPermission(PortalPermission permission) throws IllegalArgumentException, PortalSecurityException; /** * @param checkedSubject * @param permission * @return * @throws PortalSecurityException + * @throws IllegalArgumentException if the permission is null */ - public boolean checkPermission(Subject checkedSubject, PortalPermission permission) throws PortalSecurityException; + public boolean checkPermission(Subject checkedSubject, PortalPermission permission) throws IllegalArgumentException, PortalSecurityException; }