You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jetspeed-dev@portals.apache.org by at...@apache.org on 2009/02/04 23:43:23 UTC

svn commit: r740929 - in /portals/jetspeed-2/portal/trunk/components/jetspeed-security/src: main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java test/java/org/apache/jetspeed/security/TestPermissionManager.java

Author: ate
Date: Wed Feb  4 22:43:22 2009
New Revision: 740929

URL: http://svn.apache.org/viewvc?rev=740929&view=rev
Log:
Fixing bug in domainId criteria usage for revokePermission and revokeAllPermissions as discovered by Randy (thanks!).
After trying to fix this by correcting the relationship reference for domainId (using principal.domainId) however,
this unconvered a serious limitation of OJB in that it doesn't support (better said: completely ignores) relationship criteria arguments with deleteByQuery :(
To solve this I had to use a different implementation which first looks up the actual principal Id first (and therefore needs another SQL query).
Anyway, it now works and I adjusted the TestPermissionManager test-case to also check for this use-case. 

Modified:
    portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java
    portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/test/java/org/apache/jetspeed/security/TestPermissionManager.java

Modified: portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java
URL: http://svn.apache.org/viewvc/portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java?rev=740929&r1=740928&r2=740929&view=diff
==============================================================================
--- portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java (original)
+++ portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/main/java/org/apache/jetspeed/security/spi/impl/JetspeedSecurityPersistenceManager.java Wed Feb  4 22:43:22 2009
@@ -18,6 +18,7 @@
 
 import java.io.Serializable;
 import java.sql.SQLException;
+import java.sql.Types;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Iterator;
@@ -95,6 +96,24 @@
         super(repositoryPath);
     }
     
+    @SuppressWarnings("unchecked")
+    protected Long getPrincipalId(String name, String type, Long domainId) throws SecurityException
+    {
+        Criteria criteria = new Criteria();
+        criteria.addEqualTo("name", name);
+        criteria.addEqualTo("type", type);
+        criteria.addEqualTo("domainId", domainId);
+        ReportQueryByCriteria query = QueryFactory.newReportQuery(PersistentJetspeedPrincipal.class, criteria);
+        query.setAttributes(new String[]{"id"});
+        // need to force OJB to return a Long, otherwise it'll return a Integer causing a CCE
+        query.setJdbcTypes(new int[]{Types.BIGINT});
+        for (Iterator<Object[]> iter = getPersistenceBrokerTemplate().getReportQueryIteratorByQuery(query); iter.hasNext(); )
+        {
+            return (Long)iter.next()[0];
+        }
+        throw new SecurityException(SecurityException.PRINCIPAL_DOES_NOT_EXIST.createScoped(type, name));
+    }
+    
 	public boolean principalExists(JetspeedPrincipal principal)
     {
         if (principal.getId() == null)
@@ -906,17 +925,17 @@
 
     public void revokePermission(PersistentJetspeedPermission permission, JetspeedPrincipal principal) throws SecurityException
     {
+        Long principalId = null;
         Criteria criteria = new Criteria();
         if (principal.isTransient() || principal.getId() == null)
         {
-            criteria.addEqualTo("principal.type", principal.getType());
-            criteria.addEqualTo("principal.name", principal.getName());
-            criteria.addEqualTo("domainId", getDefaultSecurityDomainId());
+            principalId = getPrincipalId(principal.getName(), principal.getType().getName(), getDefaultSecurityDomainId());
         }
         else
         {
-            criteria.addEqualTo("principalId", principal.getId());
+            principalId = principal.getId();
         }
+        criteria.addEqualTo("principalId", principalId);
         if (permission.getId() == null)
         {
             criteria.addEqualTo("permission.type", permission.getType());
@@ -943,17 +962,17 @@
     
     public void revokeAllPermissions(JetspeedPrincipal principal) throws SecurityException
     {
+        Long principalId = null;
         Criteria criteria = new Criteria();
         if (principal.isTransient() || principal.getId() == null)
         {
-            criteria.addEqualTo("principal.type", principal.getType());
-            criteria.addEqualTo("principal.name", principal.getName());
-            criteria.addEqualTo("domainId", getDefaultSecurityDomainId());
+            principalId = getPrincipalId(principal.getName(), principal.getType().getName(), getDefaultSecurityDomainId());
         }
         else
         {
-            criteria.addEqualTo("principalId", principal.getId());
+            principalId = principal.getId();
         }
+        criteria.addEqualTo("principalId", principalId);
         Query query = QueryFactory.newQuery(JetspeedPrincipalPermission.class,criteria);
         try
         {

Modified: portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/test/java/org/apache/jetspeed/security/TestPermissionManager.java
URL: http://svn.apache.org/viewvc/portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/test/java/org/apache/jetspeed/security/TestPermissionManager.java?rev=740929&r1=740928&r2=740929&view=diff
==============================================================================
--- portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/test/java/org/apache/jetspeed/security/TestPermissionManager.java (original)
+++ portals/jetspeed-2/portal/trunk/components/jetspeed-security/src/test/java/org/apache/jetspeed/security/TestPermissionManager.java Wed Feb  4 22:43:22 2009
@@ -31,6 +31,8 @@
 
 import javax.security.auth.Subject;
 
+import org.apache.jetspeed.security.impl.TransientUser;
+
 import junit.framework.Test;
 import junit.framework.TestSuite;
 
@@ -161,7 +163,10 @@
         
         try
         {
-            pms.revokeAllPermissions(user);
+            // test revokeAllPrincipal a Transient User representation to ensure
+            // that use-case is covered too because it requires additional
+            // handling (lookup of the principal Id first)
+            pms.revokeAllPermissions(new TransientUser("test"));
             Permissions permissions = pms.getPermissions(user);
             assertEquals(
                 "permissions should be empty for user " + user.getName(),



---------------------------------------------------------------------
To unsubscribe, e-mail: jetspeed-dev-unsubscribe@portals.apache.org
For additional commands, e-mail: jetspeed-dev-help@portals.apache.org