From: Sanjay Dalal Date: Fri, 7 May 2010 23:39:59 +0000 (+0000) Subject: CSPACE-1482 wired permission delete operation with the spring acl service. still... X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=b13c7855cb9eeefcb1f00cbd1e15143d2eccf411;p=tmp%2Fjakarta-migration.git CSPACE-1482 wired permission delete operation with the spring acl service. still to wire role removal with acl update test: all service tests M services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionResource.java M services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java M services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java M services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/RoleDocumentHandler.java M services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java M services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java M services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java M services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java M services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java M services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java M services/account/service/src/main/java/org/collectionspace/services/account/AccountRoleSubResource.java M services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountStorageClient.java --- diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/AccountRoleSubResource.java b/services/account/service/src/main/java/org/collectionspace/services/account/AccountRoleSubResource.java index bc628b958..6793b8457 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/AccountRoleSubResource.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/AccountRoleSubResource.java @@ -176,6 +176,7 @@ public class AccountRoleSubResource logger.debug("deleteAccountRole with csid=" + csid); } ServiceContext ctx = createServiceContext((AccountRole) null, subject); - getStorageClient(ctx).delete(ctx, csid); + DocumentHandler handler = createDocumentHandler(ctx); + getStorageClient(ctx).delete(ctx, csid, handler); } } diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountStorageClient.java b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountStorageClient.java index 55f2bad8a..ea7e3d834 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountStorageClient.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountStorageClient.java @@ -45,7 +45,7 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * AccountStorageClient deals with both Account and CSIP's + * AccountStorageClient deals with both Account and CSIdP's * state in persistent storage. The rationale behind creating this class is that * this class manages pesistence for both account and CSIP's user. Transactions * are used where possible to permorme the persistence operations atomically. diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java index d916765f8..24318d2ec 100644 --- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java +++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionRoleServiceTest.java @@ -88,19 +88,19 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { pva.setPermissionId(accPermId); permValues.put(pva.getResourceName(), pva); - String rc = "collectionobjects"; - String coPermId = createPermission(rc, EffectType.DENY); - PermissionValue pvc = new PermissionValue(); - pvc.setResourceName(rc); - pvc.setPermissionId(coPermId); - permValues.put(pvc.getResourceName(), pvc); - - String ri = "intakes"; - String iPermId = createPermission(ri, EffectType.DENY); - PermissionValue pvi = new PermissionValue(); - pvi.setResourceName(ri); - pvi.setPermissionId(iPermId); - permValues.put(pvi.getResourceName(), pvi); +// String rc = "collectionobjects"; +// String coPermId = createPermission(rc, EffectType.DENY); +// PermissionValue pvc = new PermissionValue(); +// pvc.setResourceName(rc); +// pvc.setPermissionId(coPermId); +// permValues.put(pvc.getResourceName(), pvc); +// +// String ri = "intakes"; +// String iPermId = createPermission(ri, EffectType.DENY); +// PermissionValue pvi = new PermissionValue(); +// pvi.setResourceName(ri); +// pvi.setPermissionId(iPermId); +// permValues.put(pvi.getResourceName(), pvi); String rn1 = "ROLE_CO1"; String r1RoleId = createRole(rn1); @@ -122,28 +122,29 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { */ @Override protected CollectionSpaceClient getClientInstance() { - return new PermissionRoleClient(); + return new PermissionRoleClient(); } - + /* (non-Javadoc) * @see org.collectionspace.services.client.test.BaseServiceTest#getAbstractCommonList(org.jboss.resteasy.client.ClientResponse) */ @Override - protected AbstractCommonList getAbstractCommonList( - ClientResponse response) { - //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 - throw new UnsupportedOperationException(); + protected AbstractCommonList getAbstractCommonList( + ClientResponse response) { + //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 + throw new UnsupportedOperationException(); } - - @Test(dataProvider = "testName") - @Override + + @Test(dataProvider = "testName") + @Override public void readPaginatedList(String testName) throws Exception { - //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 - } + //FIXME: http://issues.collectionspace.org/browse/CSPACE-1697 + } // --------------------------------------------------------------- // CRUD tests : CREATE tests // --------------------------------------------------------------- // Success outcomes + @Override @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class) public void create(String testName) throws Exception { @@ -403,35 +404,20 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl { @AfterClass(alwaysRun = true) public void cleanUp() { - setupDelete("delete"); + setupDelete("cleanup"); String noTest = System.getProperty("noTestCleanup"); - if(Boolean.TRUE.toString().equalsIgnoreCase(noTest)) { + if (Boolean.TRUE.toString().equalsIgnoreCase(noTest)) { if (logger.isDebugEnabled()) { logger.debug("Skipping Cleanup phase ..."); } return; - } + } if (logger.isDebugEnabled()) { logger.debug("Cleaning up temporary resources created for testing ..."); } - PermissionRoleClient client = new PermissionRoleClient(); - for (String resourceId : allResourceIdsCreated) { - - ClientResponse res = client.delete(resourceId, "123"); - int statusCode = res.getStatus(); - if (logger.isDebugEnabled()) { - logger.debug("cleanup: delete relationships for permission id=" - + resourceId + " status=" + statusCode); - } - Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), - invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); - Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); - } - for (PermissionValue pv : permValues.values()) { deletePermission(pv.getPermissionId()); } - for (RoleValue rv : roleValues.values()) { deleteRole(rv.getRoleId()); } diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionResource.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionResource.java index 85dd2be86..d2530a0ce 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionResource.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionResource.java @@ -37,6 +37,7 @@ import javax.ws.rs.core.MultivaluedMap; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; +import org.collectionspace.services.authorization.storage.AuthorizationDelegate; import org.collectionspace.services.common.AbstractCollectionSpaceResourceImpl; //import org.collectionspace.services.common.context.RemoteServiceContextImpl; @@ -51,7 +52,6 @@ import org.collectionspace.services.common.document.DocumentHandler; import org.collectionspace.services.common.security.UnauthorizedException; import org.collectionspace.services.common.storage.StorageClient; import org.collectionspace.services.common.storage.jpa.JpaStorageClientImpl; -import org.collectionspace.services.common.context.ServiceContextProperties; import org.jboss.resteasy.util.HttpResponseCodes; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -351,6 +351,11 @@ public class PermissionResource //delete all relationships for this permission PermissionRoleSubResource subResource = new PermissionRoleSubResource(); subResource.deletePermissionRole(csid, SubjectType.ROLE); + //delete permissions at the provider too + //at the PermissionRoleSubResource/DocHandler levels, there is no visibility + //if permission is deleted + AuthorizationDelegate.deletePermissions(csid); + ServiceContext ctx = createServiceContext((Permission)null, Permission.class); getStorageClient(ctx).delete(ctx, csid); return Response.status(HttpResponseCodes.SC_OK).build(); diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java index 386b85557..931849cdb 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java @@ -177,6 +177,7 @@ public class PermissionRoleSubResource logger.debug("deletePermissionRole with csid=" + csid); } ServiceContext ctx = createServiceContext((PermissionRole) null, subject); - getStorageClient(ctx).delete(ctx, csid); + DocumentHandler handler = createDocumentHandler(ctx); + getStorageClient(ctx).delete(ctx, csid, handler); } } diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java index 47aa54670..79a22b7be 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java @@ -32,6 +32,7 @@ import org.collectionspace.services.authorization.CSpaceResource; import org.collectionspace.services.authorization.Permission; import org.collectionspace.services.authorization.PermissionAction; import org.collectionspace.services.authorization.PermissionException; +import org.collectionspace.services.authorization.PermissionNotFoundException; import org.collectionspace.services.authorization.PermissionRole; import org.collectionspace.services.authorization.PermissionValue; import org.collectionspace.services.authorization.RoleValue; @@ -43,20 +44,28 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * AuthorizationDelegate delegates permissions management to the authorization - * service from the RESTful service + * AuthorizationDelegate delegates permissions management to the underlying authorization + * service from the RESTful service layer. The authorization service for example + * might manage permissions with the help of a provider (e.g. Spring Security ACL) * @author */ public class AuthorizationDelegate { - private final Logger logger = LoggerFactory.getLogger(AuthorizationDelegate.class); + private static final Logger logger = LoggerFactory.getLogger(AuthorizationDelegate.class); + /** + * addPermissions add permissions represented given PermissionRole + * @param ctx + * @param pr permission role + * @throws Exception + * @see PermissionRole + */ static void addPermissions(ServiceContext ctx, PermissionRole pr) throws Exception { SubjectType subject = PermissionRoleUtil.getRelationSubject(ctx, pr); AuthZ authz = AuthZ.get(); if (subject.equals(SubjectType.ROLE)) { PermissionValue pv = pr.getPermissions().get(0); - CSpaceResource[] resources = getResources(pv); + CSpaceResource[] resources = getResources(pv.getPermissionId()); String[] roles = getRoles(pr.getRoles()); for (CSpaceResource res : resources) { authz.addPermissions(res, roles); @@ -65,7 +74,7 @@ public class AuthorizationDelegate { RoleValue rv = pr.getRoles().get(0); String[] roles = {rv.getRoleName()}; for (PermissionValue pv : pr.getPermissions()) { - CSpaceResource[] resources = getResources(pv); + CSpaceResource[] resources = getResources(pv.getPermissionId()); for (CSpaceResource res : resources) { authz.addPermissions(res, roles); } @@ -73,22 +82,49 @@ public class AuthorizationDelegate { } } + /** + * deletePermissions delete all permissions associated with given permission role + * @param ctx + * @param pr permissionrole + * @throws Exception + */ static void deletePermissions(ServiceContext ctx, PermissionRole pr) throws Exception { PermissionValue pv = pr.getPermissions().get(0); deletePermissions(pv); } + /** + * deletePermissions delete permissions associated with given PermissionValue + * @param pv permission value + * @throws Exception + * @see PermissionValue + */ static void deletePermissions(PermissionValue pv) throws Exception { - CSpaceResource[] resources = getResources(pv); + deletePermissions(pv.getPermissionId()); + } + + /** + * deletePermissions delete permissions associated with given permission id + * @param permCsid + * @throws Exception + */ + static public void deletePermissions(String permCsid) throws Exception { + CSpaceResource[] resources = getResources(permCsid); AuthZ authz = AuthZ.get(); + for (CSpaceResource res : resources) { - authz.deletePermissions(res); + try { + authz.deletePermissions(res); + } catch (PermissionException pe) { + //perms are created downthere only if roles are related to the permissions + logger.info("no permissions found in authz service provider for " + + "permCsid=" + permCsid + " res=" + res.getId()); + } } } - /** * addPermissionsForUri add permissions from given permission configuration * with assumption that resource is of type URI @@ -135,19 +171,19 @@ public class AuthorizationDelegate { /** * getResources from given PermissionValue - * @param pv permission value + * @param permisison csid * @return array of CSpaceResource * @see PermissionValue * @see CSpaceResource */ - private static CSpaceResource[] getResources(PermissionValue pv) { + private static CSpaceResource[] getResources(String permCsid) { List rl = new ArrayList(); - Permission p = (Permission) JpaStorageUtils.getEntity(pv.getPermissionId(), + Permission p = (Permission) JpaStorageUtils.getEntity(permCsid, Permission.class); if (p != null) { for (PermissionAction pa : p.getActions()) { - CSpaceResource res = new URIResourceImpl(pv.getResourceName(), + CSpaceResource res = new URIResourceImpl(p.getResourceName(), getAction(pa.getName())); rl.add(res); } @@ -155,7 +191,6 @@ public class AuthorizationDelegate { return rl.toArray(new CSpaceResource[0]); } - /** * getAction is a convenience method to get corresponding action for * given ActionType @@ -182,5 +217,4 @@ public class AuthorizationDelegate { } throw new IllegalArgumentException("action = " + action.toString()); } - } diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java index 25ebb3b80..87640f908 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java @@ -85,8 +85,7 @@ public class PermissionRoleDocumentHandler @Override public void completeDelete(DocumentWrapper> wrapDoc) throws Exception { - PermissionRole pr = getCommonPart(); - AuthorizationDelegate.deletePermissions(getServiceContext(), pr); +// } @Override diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/RoleDocumentHandler.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/RoleDocumentHandler.java index c55b5c4e6..5bc0aef98 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/RoleDocumentHandler.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/RoleDocumentHandler.java @@ -74,8 +74,6 @@ public class RoleDocumentHandler * @return merged role */ private Role merge(Role from, Role to) { - Date now = new Date(); - to.setUpdatedAtItem(now); if (from.getRoleName() != null) { to.setRoleName(from.getRoleName()); } diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java index 8c53fd96c..3bf90ebb3 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringAuthorizationProvider.java @@ -122,7 +122,7 @@ public class SpringAuthorizationProvider implements CSpaceAuthorizationProvider return sids.toArray(new Sid[0]); } - static Permission mapPermssion(CSpaceAction perm) { + static Permission mapPermission(CSpaceAction perm) { switch (perm) { case ADMIN: return BasePermission.ADMINISTRATION; diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java index 9b75ef956..18b6f40d8 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionEvaluator.java @@ -50,7 +50,7 @@ public class SpringPermissionEvaluator implements CSpacePermissionEvaluator { @Override public boolean hasPermission(CSpaceResource res, CSpaceAction perm) { PermissionEvaluator eval = provider.getProviderPermissionEvaluator(); - Permission p = SpringAuthorizationProvider.mapPermssion(perm); + Permission p = SpringAuthorizationProvider.mapPermission(perm); Authentication authToken = SecurityContextHolder.getContext().getAuthentication(); return eval.hasPermission(authToken, Long.valueOf(res.getId().hashCode()), diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java index 62e6c1b34..8b942352b 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spring/SpringPermissionManager.java @@ -23,6 +23,8 @@ */ package org.collectionspace.services.authorization.spring; +import java.util.ArrayList; +import java.util.Iterator; import java.util.List; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -31,16 +33,15 @@ import org.collectionspace.services.authorization.spi.CSpacePermissionManager; import org.collectionspace.services.authorization.CSpaceResource; import org.collectionspace.services.authorization.PermissionException; import org.collectionspace.services.authorization.PermissionNotFoundException; -import org.springframework.jdbc.datasource.DataSourceTransactionManager; import org.springframework.security.acls.model.AccessControlEntry; +import org.springframework.security.acls.model.AclDataAccessException; +import org.springframework.security.acls.model.AlreadyExistsException; import org.springframework.security.acls.model.MutableAcl; import org.springframework.security.acls.model.NotFoundException; import org.springframework.security.acls.model.ObjectIdentity; import org.springframework.security.acls.model.Permission; import org.springframework.security.acls.model.Sid; -import org.springframework.transaction.TransactionDefinition; import org.springframework.transaction.TransactionStatus; -import org.springframework.transaction.support.DefaultTransactionDefinition; /** * Manages permissions in Spring Security @@ -60,29 +61,59 @@ public class SpringPermissionManager implements CSpacePermissionManager { throws PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res); Sid[] sids = SpringAuthorizationProvider.mapPrincipal(principals); - Permission p = SpringAuthorizationProvider.mapPermssion(action); + Permission p = SpringAuthorizationProvider.mapPermission(action); TransactionStatus status = provider.beginTransaction("addPermssions"); - try { - for (Sid sid : sids) { + + //add permission for each sid + for (Sid sid : sids) { + try { addPermission(oid, p, sid); if (log.isDebugEnabled()) { - log.debug("added permission " + log.debug("addpermissions(res,action,prin[]), success for " + " res=" + res.toString() + " action=" + action.toString() + " oid=" + oid.toString() + " perm=" + p.toString() - + " sid=" + sids.toString()); + + " sid=" + sid.toString()); } + + } catch (AlreadyExistsException aex) { + if (log.isWarnEnabled()) { + log.warn("addpermissions(res,action,prin[]) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(), aex); + } + //keep going + } catch (Exception ex) { + String msg = "addpermissions(res,action,prin[]) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(); + if (log.isDebugEnabled()) { + log.debug(msg, ex); + } + //don't know what might be wrong...stop + provider.rollbackTransaction(status); + if (ex instanceof PermissionException) { + throw (PermissionException) ex; + } + throw new PermissionException(msg, ex); } - } catch (Exception ex) { - provider.rollbackTransaction(status); - if (log.isDebugEnabled()) { - ex.printStackTrace(); - } - throw new PermissionException(ex); - } + }//rof provider.commitTransaction(status); - + if (log.isDebugEnabled()) { + log.debug("addpermissions(res,action,prin[]), success for " + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString() + + " sids=" + sids.toString()); + } } @Override @@ -90,55 +121,100 @@ public class SpringPermissionManager implements CSpacePermissionManager { throws PermissionNotFoundException, PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res); Sid[] sids = SpringAuthorizationProvider.mapPrincipal(principals); - Permission p = SpringAuthorizationProvider.mapPermssion(action); + Permission p = SpringAuthorizationProvider.mapPermission(action); TransactionStatus status = provider.beginTransaction("deletePermssions"); - try { - for (Sid sid : sids) { + //delete permission for each sid + for (Sid sid : sids) { + try { deletePermissions(oid, p, sid); if (log.isDebugEnabled()) { - log.debug("deleted permission " + log.debug("deletedpermissions(res,action,prin[]), success for " + " res=" + res.toString() + " action=" + action.toString() + " oid=" + oid.toString() + " perm=" + p.toString() - + " sid=" + sids.toString()); + + " sid=" + sid.toString()); } + } catch (AclDataAccessException aex) { + if (log.isWarnEnabled()) { + log.debug("deletepermissions(res,action,prin[]) failed, " + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(), aex); + } + //keep going + } catch (Exception ex) { + String msg = "deletepermissions(res,action,prin[]) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(); + if (log.isDebugEnabled()) { + log.debug(msg, ex); + } + //don't know what might be wrong...stop + provider.rollbackTransaction(status); + if (ex instanceof PermissionException) { + throw (PermissionException) ex; + } + throw new PermissionException(msg, ex); } - } catch (Exception ex) { - provider.rollbackTransaction(status); - if (log.isDebugEnabled()) { - ex.printStackTrace(); - } - throw new PermissionException(ex); } provider.commitTransaction(status); - + if (log.isDebugEnabled()) { + log.debug("deletedpermissions(res,action,prin[]), success for " + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString() + + " sids=" + sids.toString()); + } } @Override public void deletePermissions(CSpaceResource res, CSpaceAction action) throws PermissionNotFoundException, PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.mapResource(res); - Permission p = SpringAuthorizationProvider.mapPermssion(action); + Permission p = SpringAuthorizationProvider.mapPermission(action); TransactionStatus status = provider.beginTransaction("deletePermssions"); try { deletePermissions(oid, p, null); - if (log.isDebugEnabled()) { - log.debug("deleted permissions " - + " res=" + res.toString() - + " action=" + action.toString() - + " oid=" + oid.toString() - + " perm=" + p.toString()); - } + } catch (AclDataAccessException aex) { + provider.rollbackTransaction(status); + log.debug("deletepermissions(res,action) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(), aex); + throw new PermissionException(aex); } catch (Exception ex) { provider.rollbackTransaction(status); + String msg = "deletepermissions(res,action,prin[]) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString(); if (log.isDebugEnabled()) { - ex.printStackTrace(); + log.debug(msg, ex); + } + if (ex instanceof PermissionException) { + throw (PermissionException) ex; } - throw new PermissionException(ex); + throw new PermissionException(msg, ex); } provider.commitTransaction(status); - + if (log.isDebugEnabled()) { + log.debug("deletepermissions(res,action) success, " + + " res=" + res.toString() + + " action=" + action.toString() + + " oid=" + oid.toString() + + " perm=" + p.toString()); + } } @@ -149,86 +225,106 @@ public class SpringPermissionManager implements CSpacePermissionManager { TransactionStatus status = provider.beginTransaction("addPermssion"); try { provider.getProviderAclService().deleteAcl(oid, true); + } catch (AclDataAccessException aex) { + provider.rollbackTransaction(status); + log.debug("deletepermissions(res) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " oid=" + oid.toString(), aex); + throw new PermissionException(aex); } catch (Exception ex) { provider.rollbackTransaction(status); + String msg = "deletepermissions(res) failed," + + " oid=" + oid.toString() + + " res=" + res.toString() + + " oid=" + oid.toString(); if (log.isDebugEnabled()) { - ex.printStackTrace(); + log.debug(msg, ex); } - throw new PermissionException(ex); + if (ex instanceof PermissionException) { + throw (PermissionException) ex; + } + throw new PermissionException(msg, ex); } provider.commitTransaction(status); if (log.isDebugEnabled()) { - log.debug("deleted permissions " + log.debug("deletepermissions(res) success, " + " res=" + res.toString() + " oid=" + oid.toString()); } } private void addPermission(ObjectIdentity oid, Permission permission, - Sid recipient) throws PermissionException { + Sid sid) throws PermissionException { MutableAcl acl; try { acl = getAcl(oid); - } catch (PermissionException pnfe) { + } catch (NotFoundException nfe) { + if (log.isDebugEnabled()) { + log.debug("addPermission: acl not found for oid=" + oid.toString() + + " perm=" + permission.toString() + + " sid=" + sid.toString() + + " adding..."); + } acl = provider.getProviderAclService().createAcl(oid); } - acl.insertAce(acl.getEntries().size(), permission, recipient, true); + acl.insertAce(acl.getEntries().size(), permission, sid, true); provider.getProviderAclService().updateAcl(acl); if (log.isDebugEnabled()) { log.debug("addPermission: added acl for oid=" + oid.toString() + " perm=" + permission.toString() - + " sid=" + recipient.toString()); + + " sid=" + sid.toString()); } } - private void deletePermissions(ObjectIdentity oid, Permission permission, Sid recipient) - throws PermissionException { - - int j = 0; + private void deletePermissions(ObjectIdentity oid, Permission permission, Sid sid) /** throws AclDataAccessException */ + { + int i = 0; MutableAcl acl = getAcl(oid); - List entries = acl.getEntries(); + List acel = acl.getEntries(); + int aces = acel.size(); if (log.isDebugEnabled()) { log.debug("deletePermissions: for acl oid=" + oid.toString() - + " found " + entries.size() + " aces"); + + " found " + aces + " aces"); } - - for (int i = 0; i < entries.size(); i++) { - AccessControlEntry ace = entries.get(i); - if (recipient != null) { - if (ace.getSid().equals(recipient) + ArrayList foundAces = new ArrayList(); + Iterator iter = acel.listIterator(); + //not possible to delete while iterating + while(iter.hasNext()) { + AccessControlEntry ace = (AccessControlEntry)iter.next(); + if (sid != null) { + if (ace.getSid().equals(sid) && ace.getPermission().equals(permission)) { - acl.deleteAce(i); - j++; + foundAces.add(i); + i++; } } else { if (ace.getPermission().equals(permission)) { - acl.deleteAce(i); - j++; + foundAces.add(i); + i++; } } } + for (int j = foundAces.size() - 1; j >= 0; j--) { + //the following operation does not work while iterating in the while loop + acl.deleteAce(foundAces.get(j)); //autobox + } provider.getProviderAclService().updateAcl(acl); if (log.isDebugEnabled()) { log.debug("deletePermissions: for acl oid=" + oid.toString() - + " deleted " + j + " aces"); + + " deleted " + i + " aces"); } } - private MutableAcl getAcl(ObjectIdentity oid) throws PermissionNotFoundException { + private MutableAcl getAcl(ObjectIdentity oid) throws NotFoundException { MutableAcl acl = null; - try { - acl = (MutableAcl) provider.getProviderAclService().readAclById(oid); - if (log.isDebugEnabled()) { - log.debug("found acl for oid=" + oid.toString()); - } - } catch (NotFoundException nfe) { - String msg = "Cound not find acl for oid=" + oid.toString(); - log.error(msg); - throw new PermissionNotFoundException(msg); + acl = (MutableAcl) provider.getProviderAclService().readAclById(oid); + if (log.isDebugEnabled()) { + log.debug("found acl for oid=" + oid.toString()); } return acl; } diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java index e4adc202a..3955d11f1 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClientImpl.java @@ -63,11 +63,11 @@ import org.slf4j.LoggerFactory; - + - + @@ -123,6 +123,9 @@ public class JpaStorageClientImpl implements StorageClient { } throw bre; } catch (DocumentException de) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } throw de; } catch (Exception e) { if (em != null && em.getTransaction().isActive()) { @@ -297,6 +300,7 @@ public class JpaStorageClientImpl implements StorageClient { Object entityFound = getEntity(em, id, entityReceived.getClass()); DocumentWrapper wrapDoc = new DocumentWrapperImpl(entityFound); handler.handle(Action.UPDATE, wrapDoc); + JaxbUtils.setValue(entityFound, "setUpdatedAtItem", Date.class, new Date()); em.getTransaction().commit(); handler.complete(Action.UPDATE, wrapDoc); } catch (BadRequestException bre) { @@ -305,6 +309,9 @@ public class JpaStorageClientImpl implements StorageClient { } throw bre; } catch (DocumentException de) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } throw de; } catch (Exception e) { if (logger.isDebugEnabled()) { @@ -357,6 +364,9 @@ public class JpaStorageClientImpl implements StorageClient { em.getTransaction().commit(); } catch (DocumentException de) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } throw de; } catch (Exception e) { if (logger.isDebugEnabled()) { @@ -420,6 +430,9 @@ public class JpaStorageClientImpl implements StorageClient { em.getTransaction().commit(); } catch (DocumentException de) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } throw de; } catch (Exception e) { if (logger.isDebugEnabled()) { @@ -457,6 +470,9 @@ public class JpaStorageClientImpl implements StorageClient { handler.handle(Action.DELETE, wrapDoc); handler.complete(Action.DELETE, wrapDoc); } catch (DocumentException de) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } throw de; } catch (Exception e) { if (logger.isDebugEnabled()) { @@ -494,6 +510,7 @@ public class JpaStorageClientImpl implements StorageClient { /** * getEntity returns persistent entity for given id. it assumes that * service context has property ServiceContextProperties.ENTITY_CLASS set + * rolls back the transaction if not found * @param ctx service context * @param em entity manager * @param csid received @@ -514,6 +531,7 @@ public class JpaStorageClientImpl implements StorageClient { /** * getEntity retrieves the persistent entity of given class for given id + * rolls back the transaction if not found * @param em * @param id entity id * @param entityClazz