From: remillet Date: Wed, 3 Jan 2018 07:02:21 +0000 (-0800) Subject: DRYD-221: Refactor persistence of AuthN/AuthZ entities to support better transaction... X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=e7586244f89ce570d66e3fab5f606b4698f36965;p=tmp%2Fjakarta-migration.git DRYD-221: Refactor persistence of AuthN/AuthZ entities to support better transaction support. --- diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/RoleResource.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/RoleResource.java index bde34a397..0ae23e998 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/RoleResource.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/RoleResource.java @@ -152,42 +152,43 @@ public class RoleResource extends SecurityResourceBase { @DELETE @Path("{csid}") - public Response deleteRole(@PathParam("csid") String csid, @Context UriInfo ui) { + public Response deleteRole(@PathParam("csid") String csid, @Context UriInfo ui) throws Exception { logger.debug("deleteRole with csid=" + csid); ensureCSID(csid, ServiceMessages.DELETE_FAILED + "deleteRole "); + ServiceContext ctx = createServiceContext((Role) null, Role.class); + TransactionContext transactionContext = ctx.openConnection(); // ensure we do all this in one transaction try { - ServiceContext ctx = createServiceContext((Role) null, Role.class); - ctx.openConnection(); // ensure we do all this in one transaction - try { - Role role = (Role)get(csid, Role.class); - // If marked as metadata immutable, do not delete - if (RoleClient.IMMUTABLE.equals(role.getMetadataProtection())) { - Response response = - Response.status(Response.Status.FORBIDDEN).entity("Role: "+csid+" is immutable.").type("text/plain").build(); - return response; - } - // - // delete all the permission/role relationships - // - PermissionRoleSubResource permRoleResource = - new PermissionRoleSubResource(PermissionRoleSubResource.ROLE_PERMROLE_SERVICE); - permRoleResource.deletePermissionRole(ctx, csid, SubjectType.PERMISSION); - // - //delete all the account/role relationships associate with this role - // - AccountRoleSubResource accountRoleResource = - new AccountRoleSubResource(AccountRoleSubResource.ROLE_ACCOUNTROLE_SERVICE); - accountRoleResource.deleteAccountRole(ctx, csid, SubjectType.ACCOUNT); - // - //finally, delete the role itself - // - ((JpaStorageClientImpl) getStorageClient(ctx)).deleteWhere(ctx, csid); - } finally { - ctx.closeConnection(); + transactionContext.beginTransaction(); + Role role = (Role)get(csid, Role.class); + // If marked as metadata immutable, do not delete + if (RoleClient.IMMUTABLE.equals(role.getMetadataProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Role: "+csid+" is immutable.").type("text/plain").build(); + return response; } - } catch (Exception e) { + // + // delete all the permission/role relationships + // + PermissionRoleSubResource permRoleResource = + new PermissionRoleSubResource(PermissionRoleSubResource.ROLE_PERMROLE_SERVICE); + permRoleResource.deletePermissionRole(ctx, csid, SubjectType.PERMISSION); + // + //delete all the account/role relationships associate with this role + // + AccountRoleSubResource accountRoleResource = + new AccountRoleSubResource(AccountRoleSubResource.ROLE_ACCOUNTROLE_SERVICE); + accountRoleResource.deleteAccountRole(ctx, csid, SubjectType.ACCOUNT); + // + //finally, delete the role itself + // + ((JpaStorageClientImpl) getStorageClient(ctx)).deleteWhere(ctx, csid); + transactionContext.commitTransaction(); + } catch(Exception e) { + transactionContext.markForRollback(); throw bigReThrow(e, ServiceMessages.DELETE_FAILED, csid); + } finally { + ctx.closeConnection(); } return Response.status(HttpResponseCodes.SC_OK).build(); diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java index 71c9ee302..58e7ac8de 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/AuthZ.java @@ -40,6 +40,7 @@ import org.springframework.security.core.Authentication; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.transaction.TransactionStatus; /** * AuthZ is the authorization service singleton used by the services runtime @@ -121,9 +122,18 @@ public class AuthZ { * @param principals * * @param grant true to grant false to deny */ - public void addPermissions(CSpaceResource res, String[] principals, boolean grant) throws PermissionException { - CSpaceAction action = res.getAction(); - addPermissions(res, action, principals, grant); + public void addPermissions(CSpaceResource[] resources, String[] principals, boolean grant) throws PermissionException { + TransactionStatus status = provider.beginTransaction("addPermssions"); + try { + for (CSpaceResource res : resources) { + CSpaceAction action = res.getAction(); + addPermissions(res, action, principals, grant); + } + provider.commitTransaction(status); + } catch (Throwable t) { + provider.rollbackTransaction(status); + throw t; + } } /** @@ -133,9 +143,9 @@ public class AuthZ { * @param principals * @param grant true to grant false to deny */ - public void addPermissions(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) + private void addPermissions(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) throws PermissionException { - provider.getPermissionManager().addPermissions(res, action, principals, grant); + provider.getPermissionManager().addPermissionsToRoles(res, action, principals, grant); provider.clearAclCache(); } @@ -146,10 +156,20 @@ public class AuthZ { * @param res * @param principals */ - public void deletePermissions(CSpaceResource res, String[] principals) + public void deletePermissionsFromRoles(CSpaceResource[] resources, String[] principals) throws PermissionNotFoundException, PermissionException { - CSpaceAction action = res.getAction(); - deletePermissions(res, action, principals); + + TransactionStatus status = provider.beginTransaction("deletePermssions"); + try { + for (CSpaceResource res : resources) { + CSpaceAction action = res.getAction(); + deletePermissionsFromRoles(res, action, principals); + } + provider.commitTransaction(status); + } catch (Throwable t) { + provider.rollbackTransaction(status); + throw t; + } } /** @@ -159,9 +179,9 @@ public class AuthZ { * @param action * @param principals */ - public void deletePermissions(CSpaceResource res, CSpaceAction action, String[] principals) + private void deletePermissionsFromRoles(CSpaceResource res, CSpaceAction action, String[] principals) throws PermissionNotFoundException, PermissionException { - provider.getPermissionManager().deletePermissions(res, action, principals); + provider.getPermissionManager().deletePermissionsFromRoles(res, action, principals); provider.clearAclCache(); } @@ -173,14 +193,23 @@ public class AuthZ { * @param res * @param principals */ - public void deletePermissions(CSpaceResource res) + public void deletePermissions(CSpaceResource[] resources) throws PermissionNotFoundException, PermissionException { - CSpaceAction action = res.getAction(); - if (action != null) { - deletePermissions(res, action); - } else { - provider.getPermissionManager().deletePermissions(res); - provider.clearAclCache(); + TransactionStatus status = provider.beginTransaction("deletePermssions"); + try { + for (CSpaceResource res : resources) { + CSpaceAction action = res.getAction(); + if (action != null) { + deletePermissions(res, action); + } else { + provider.getPermissionManager().deletePermissions(res); + provider.clearAclCache(); + } + } + provider.commitTransaction(status); + } catch (Throwable t) { + provider.rollbackTransaction(status); + throw t; } } @@ -191,7 +220,7 @@ public class AuthZ { * @param action * @param principals */ - public void deletePermissions(CSpaceResource res, CSpaceAction action) + private void deletePermissions(CSpaceResource res, CSpaceAction action) throws PermissionNotFoundException, PermissionException { provider.getPermissionManager().deletePermissions(res, action); provider.clearAclCache(); diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpaceAuthorizationProvider.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpaceAuthorizationProvider.java index 3fabc32d4..b806df84e 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpaceAuthorizationProvider.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpaceAuthorizationProvider.java @@ -28,6 +28,8 @@ package org.collectionspace.services.authorization.spi; +import org.springframework.transaction.TransactionStatus; + /** * CSpaceAuthorizationProvider acts as a main interface to access the provider * specific information @@ -44,4 +46,10 @@ public interface CSpaceAuthorizationProvider { public CSpacePermissionManager getPermissionManager(); public void clearAclCache(); + + public TransactionStatus beginTransaction(String name); + + public void rollbackTransaction(TransactionStatus status); + + public void commitTransaction(TransactionStatus status); } diff --git a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpacePermissionManager.java b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpacePermissionManager.java index 2b3a086e3..b8f0f30de 100644 --- a/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpacePermissionManager.java +++ b/services/authorization/service/src/main/java/org/collectionspace/services/authorization/spi/CSpacePermissionManager.java @@ -44,7 +44,7 @@ public interface CSpacePermissionManager { * @see CSpaceResource * @see CSpaceAction */ - public void addPermissions(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) + public void addPermissionsToRoles(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) throws PermissionException; /** @@ -57,7 +57,7 @@ public interface CSpacePermissionManager { * @see CSpaceResource * @see CSpaceAction */ - public void deletePermissions(CSpaceResource res, CSpaceAction action, String[] principals) + public void deletePermissionsFromRoles(CSpaceResource res, CSpaceAction action, String[] principals) throws PermissionNotFoundException, PermissionException; /** 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 b891f7a8d..5afd0f01a 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 @@ -182,7 +182,8 @@ public class SpringAuthorizationProvider implements CSpaceAuthorizationProvider /** * clear the ACL Cache associated with the provider */ - public void clearAclCache() { + @Override + public void clearAclCache() { if(providerAclCache != null) { providerAclCache.clearCache(); if (log.isDebugEnabled()) { @@ -193,7 +194,8 @@ public class SpringAuthorizationProvider implements CSpaceAuthorizationProvider } } - TransactionStatus beginTransaction(String name) { + @Override + public TransactionStatus beginTransaction(String name) { DefaultTransactionDefinition def = new DefaultTransactionDefinition(); // explicitly setting the transaction name is something that can only be done programmatically def.setName(name); @@ -201,11 +203,13 @@ public class SpringAuthorizationProvider implements CSpaceAuthorizationProvider return getTxManager().getTransaction(def); } - void rollbackTransaction(TransactionStatus status) { + @Override + public void rollbackTransaction(TransactionStatus status) { getTxManager().rollback(status); } - void commitTransaction(TransactionStatus status) { + @Override + public void commitTransaction(TransactionStatus status) { getTxManager().commit(status); } } 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 81324bc13..70e5f0b62 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 @@ -44,7 +44,6 @@ 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.TransactionStatus; /** * Manages permissions in Spring Security @@ -69,17 +68,16 @@ public class SpringPermissionManager implements CSpacePermissionManager { * @throws PermissionException */ @Override - public void addPermissions(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) + public void addPermissionsToRoles(CSpaceResource res, CSpaceAction action, String[] principals, boolean grant) throws PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.getObjectIdentity(res); Sid[] sids = SpringAuthorizationProvider.getSids(principals); Permission p = SpringAuthorizationProvider.getPermission(action); - TransactionStatus status = provider.beginTransaction("addPermssions"); //add permission for each sid for (Sid sid : sids) { try { - addPermission(oid, p, sid, grant); + addPermissionToRole(oid, p, sid, grant); if (log.isDebugEnabled()) { log.debug("addpermissions(res,action,prin[], grant), success for " + " res=" + res.toString() @@ -113,14 +111,12 @@ public class SpringPermissionManager implements CSpacePermissionManager { 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); } }//rof - provider.commitTransaction(status); if (log.isDebugEnabled()) { log.debug("addpermissions(res,action,prin[], grant), success for " + " res=" + res.toString() @@ -141,16 +137,15 @@ public class SpringPermissionManager implements CSpacePermissionManager { * @throws PermissionException */ @Override - public void deletePermissions(CSpaceResource res, CSpaceAction action, String[] principals) + public void deletePermissionsFromRoles(CSpaceResource res, CSpaceAction action, String[] principals) throws PermissionNotFoundException, PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.getObjectIdentity(res); Sid[] sids = SpringAuthorizationProvider.getSids(principals); Permission p = SpringAuthorizationProvider.getPermission(action); - TransactionStatus status = provider.beginTransaction("deletePermssions"); //delete permission for each sid for (Sid sid : sids) { try { - deletePermissions(oid, p, sid); + deletePermissionFromRole(oid, p, sid); if (log.isDebugEnabled()) { log.debug("deletedpermissions(res,action,prin[]), success for " + " res=" + res.toString() @@ -180,14 +175,12 @@ public class SpringPermissionManager implements CSpacePermissionManager { 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); } } - provider.commitTransaction(status); if (log.isDebugEnabled()) { log.debug("deletedpermissions(res,action,prin[]), success for " + " res=" + res.toString() @@ -210,12 +203,11 @@ public class SpringPermissionManager implements CSpacePermissionManager { @Override public void deletePermissions(CSpaceResource res, CSpaceAction action) throws PermissionNotFoundException, PermissionException { + ObjectIdentity oid = SpringAuthorizationProvider.getObjectIdentity(res); Permission p = SpringAuthorizationProvider.getPermission(action); - TransactionStatus status = provider.beginTransaction("deletePermssions"); try { - deletePermissions(oid, p, null); - provider.commitTransaction(status); + deletePermissionFromRole(oid, p, null); if (log.isDebugEnabled()) { log.debug("deletepermissions(res,action) success, " + " res=" + res.toString() @@ -224,7 +216,6 @@ public class SpringPermissionManager implements CSpacePermissionManager { + " perm=" + p.toString()); } } catch (AclDataAccessException aex) { - provider.rollbackTransaction(status); log.debug("deletepermissions(res,action) failed," + " oid=" + oid.toString() + " res=" + res.toString() @@ -233,7 +224,6 @@ public class SpringPermissionManager implements CSpacePermissionManager { + " 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() @@ -263,24 +253,20 @@ public class SpringPermissionManager implements CSpacePermissionManager { public void deletePermissions(CSpaceResource res) throws PermissionNotFoundException, PermissionException { ObjectIdentity oid = SpringAuthorizationProvider.getObjectIdentity(res); - TransactionStatus status = provider.beginTransaction("deletePermssion"); try { provider.getProviderAclService().deleteAcl(oid, true); - provider.commitTransaction(status); if (log.isDebugEnabled()) { log.debug("deletepermissions(res) success, " + " res=" + res.toString() + " oid=" + oid.toString()); } } 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() @@ -304,7 +290,7 @@ public class SpringPermissionManager implements CSpacePermissionManager { * @param grant * @throws PermissionException */ - private void addPermission(ObjectIdentity oid, Permission permission, + private void addPermissionToRole(ObjectIdentity oid, Permission permission, Sid sid, boolean grant) throws PermissionException { MutableAcl acl; @@ -323,7 +309,7 @@ public class SpringPermissionManager implements CSpacePermissionManager { // Need to see if there is already an entry, so we do not duplicate (e.g., // when we run our permission-roles init more than once. List aceEntries = acl.getEntries(); - if(aceListHasEntry(aceEntries, permission, sid, grant)) { + if (aceListHasEntry(aceEntries, permission, sid, grant)) { if (log.isDebugEnabled()) { log.debug("addPermission: Pre-existing acl for oid=" + oid.toString() + " perm=" + permission.toString() @@ -363,7 +349,7 @@ public class SpringPermissionManager implements CSpacePermissionManager { * @param sid */ //non-javadoc NOTE: if sid is null it would remove ACEs for all sid(s) - private void deletePermissions(ObjectIdentity oid, Permission permission, Sid sid) /** throws AclDataAccessException */ + private void deletePermissionFromRole(ObjectIdentity oid, Permission permission, Sid sid) /** throws AclDataAccessException */ { int i = 0; MutableAcl acl = getAcl(oid); diff --git a/services/common/src/main/java/org/collectionspace/services/authorization/PermissionResource.java b/services/common/src/main/java/org/collectionspace/services/authorization/PermissionResource.java index 4d8479597..f331ef5f2 100644 --- a/services/common/src/main/java/org/collectionspace/services/authorization/PermissionResource.java +++ b/services/common/src/main/java/org/collectionspace/services/authorization/PermissionResource.java @@ -95,6 +95,10 @@ public class PermissionResource extends SecurityResourceBase { } @POST + public Response createPermission(Permission input) { + return create(input); + } + public Response createPermission(JPATransactionContext jpaTransactionContext, Permission input) { return create(jpaTransactionContext, input); } @@ -139,32 +143,42 @@ public class PermissionResource extends SecurityResourceBase { @SuppressWarnings("unchecked") @DELETE @Path("{csid}") - public Response deletePermission(@PathParam("csid") String csid) throws Exception { + synchronized public Response deletePermission(@PathParam("csid") String csid) throws Exception { logger.debug("deletePermission with csid=" + csid); ensureCSID(csid, ServiceMessages.DELETE_FAILED + "permission "); ServiceContext ctx = createServiceContext((Permission) null, Permission.class); TransactionContext transactionContext = ctx.openConnection(); try { - //FIXME ideally the following two ops should be in the same tx CSPACE-658 - //delete all relationships for this permission + transactionContext.beginTransaction(); + // + // First, delete the relationships between the Permission resource and any Role resources. + // PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.PERMISSION_PERMROLE_SERVICE); subResource.deletePermissionRole(ctx, csid, SubjectType.ROLE); - //NOTE for delete permissions in the authz provider - //at the PermissionRoleSubResource/DocHandler level, there is no visibility - //if permission is deleted, so do it here, however, - //this is a very dangerous operation as it deletes the Spring ACL instead of ACE(s) - //the ACL might be needed for other ACEs roles... - AuthorizationDelegate.deletePermissions((JPATransactionContext)transactionContext, csid); - + // + // Next, delete the low-level (Spring Security) permissions. + // + // NOTE: For deletePermission() in the authz provider at the PermissionRoleSubResource/DocHandler level, there is no visibility + // if permission is deleted, so do it here. + // + // WARNING: This operation deletes the Spring ACL (not the ACEs). It's possible the ACL might be needed for other ACEs roles... + // + AuthorizationDelegate.deletePermissions((JPATransactionContext)transactionContext, csid); // Deletes the low-level (Spring Security) permissions + // + // Lastly, delete the Permission resource itself and commit the transaction + // getStorageClient(ctx).delete(ctx, csid); - return Response.status(HttpResponseCodes.SC_OK).build(); + transactionContext.commitTransaction(); } catch (Exception e) { + transactionContext.markForRollback(); throw bigReThrow(e, ServiceMessages.DELETE_FAILED, csid); } finally { ctx.closeConnection(); } + + return Response.status(HttpResponseCodes.SC_OK).build(); } @POST @@ -237,20 +251,23 @@ public class PermissionResource extends SecurityResourceBase { try { PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.PERMISSION_PERMROLE_SERVICE); - //delete all relationships for a permission + // + // Delete all role relationships for a permission + // subResource.deletePermissionRole((ServiceContext)null, permCsid, SubjectType.ROLE, input); return Response.status(HttpResponseCodes.SC_OK).build(); } catch (Exception e) { throw bigReThrow(e, ServiceMessages.DELETE_FAILED, permCsid); } } - + @DELETE @Path("{csid}/permroles") public Response deletePermissionRole( @PathParam("csid") String permCsid) { logger.debug("Delete all the role relationships of the permissions with permCsid=" + permCsid); - ensureCSID(permCsid, ServiceMessages.DELETE_FAILED + "permroles permission "); + ensureCSID(permCsid, ServiceMessages.DELETE_FAILED + "permroles permission "); + try { PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.PERMISSION_PERMROLE_SERVICE); diff --git a/services/common/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java b/services/common/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java index 4d02fd137..cc38e6a01 100644 --- a/services/common/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java +++ b/services/common/src/main/java/org/collectionspace/services/authorization/PermissionRoleSubResource.java @@ -34,6 +34,7 @@ import org.collectionspace.services.common.context.RemoteServiceContextFactory; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.context.ServiceContextFactory; import org.collectionspace.services.common.document.DocumentHandler; +import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.storage.StorageClient; import org.collectionspace.services.common.storage.jpa.JPATransactionContext; import org.collectionspace.services.common.storage.jpa.JpaRelationshipStorageClient; @@ -227,7 +228,7 @@ public class PermissionRoleSubResource DocumentHandler handler = createDocumentHandler(ctx); getStorageClient(ctx).get(ctx, csid, handler); result = (PermissionRole) ctx.getOutput(); - + return result; } @@ -246,8 +247,13 @@ public class PermissionRoleSubResource if (logger.isDebugEnabled()) { logger.debug("deletePermissionRole with csid=" + csid); } - PermissionRole permRole = this.getPermissionRole(parentCtx, csid, subject); - deletePermissionRole(parentCtx, csid, subject, permRole); + PermissionRole permRole = getPermissionRole(parentCtx, csid, subject); + if (permRole != null) { + deletePermissionRole(parentCtx, csid, subject, permRole); + } else { + String msg = String.format("The permission CSID=%s is missing or not related to any roles.", csid); + throw new DocumentNotFoundException(msg); + } } /** diff --git a/services/common/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java b/services/common/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java index 0adcb0186..ddc9c5584 100644 --- a/services/common/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java +++ b/services/common/src/main/java/org/collectionspace/services/authorization/storage/AuthorizationDelegate.java @@ -66,49 +66,45 @@ public class AuthorizationDelegate { * @throws Exception * @see PermissionRole */ - public static void addPermissions(ServiceContext ctx, PermissionRole pr) throws Exception { + public static void addRelationships(ServiceContext ctx, PermissionRole pr) throws Exception { JPATransactionContext jpaTransactionContext = (JPATransactionContext) ctx.getCurrentTransactionContext(); SubjectType subject = PermissionRoleUtil.getRelationSubject(ctx, pr); AuthZ authz = AuthZ.get(); if (subject.equals(SubjectType.ROLE)) { PermissionValue pv = pr.getPermission().get(0); - Permission p = getPermission(jpaTransactionContext, pv.getPermissionId()); - if (p == null) { + Permission permission = getPermission(jpaTransactionContext, pv.getPermissionId()); + if (permission == null) { String msg = "addPermissions: No permission found for id=" + pv.getPermissionId(); logger.error(msg); throw new DocumentNotFoundException(msg); } - CSpaceResource[] resources = getResources(p); + CSpaceResource[] resources = getResources(permission); String[] roles = getRoles(jpaTransactionContext, pr.getRole()); - for (CSpaceResource res : resources) { - boolean grant = p.getEffect().equals(EffectType.PERMIT) ? true : false; - authz.addPermissions(res, roles, grant); - } + boolean grant = permission.getEffect().equals(EffectType.PERMIT) ? true : false; + authz.addPermissions(resources, roles, grant); } else if (SubjectType.PERMISSION.equals(subject)) { RoleValue rv = pr.getRole().get(0); - Role r = getRole(jpaTransactionContext, rv.getRoleId()); - if (r == null) { + Role role = getRole(jpaTransactionContext, rv.getRoleId()); + if (role == null) { String msg = "addPermissions: No role found for id=" + rv.getRoleId(); logger.error(msg); throw new DocumentNotFoundException(msg); } - //using r not rv ensures we're getting the "ROLE" prefix/qualified name + // Using a Role and not RoleValue ensures we're getting the "ROLE" prefix/qualified name // This needs to use the qualified name, not the display name - String[] roles = {r.getRoleName()}; + String[] roles = {role.getRoleName()}; for (PermissionValue pv : pr.getPermission()) { Permission p = getPermission(jpaTransactionContext, pv.getPermissionId()); if (p == null) { - String msg = "addPermissions: No permission found for id=" + pv.getPermissionId(); + String msg = "addPermissions: No permission resource found for csid=" + pv.getPermissionId(); logger.error(msg); //TODO: would be nice contiue to still send 400 back continue; } CSpaceResource[] resources = getResources(p); - for (CSpaceResource res : resources) { - boolean grant = p.getEffect().equals(EffectType.PERMIT) ? true : false; - authz.addPermissions(res, roles, grant); - } + boolean grant = p.getEffect().equals(EffectType.PERMIT) ? true : false; + authz.addPermissions(resources, roles, grant); } } } @@ -119,7 +115,7 @@ public class AuthorizationDelegate { * @param pr permissionrole * @throws Exception */ - public static void deletePermissions(ServiceContext ctx, PermissionRole pr) + public static void deletePermissionsFromRoles(ServiceContext ctx, PermissionRole pr) throws Exception { JPATransactionContext jpaTransactionContext = (JPATransactionContext) ctx.getCurrentTransactionContext(); @@ -137,9 +133,7 @@ public class AuthorizationDelegate { } CSpaceResource[] resources = getResources(p); String[] roles = getRoles(jpaTransactionContext, pr.getRole()); - for (CSpaceResource res : resources) { - authz.deletePermissions(res, roles); - } + authz.deletePermissionsFromRoles(resources, roles); } } else if (SubjectType.PERMISSION.equals(subject)) { List roleValues = pr.getRole(); @@ -163,9 +157,7 @@ public class AuthorizationDelegate { continue; } CSpaceResource[] resources = getResources(p); - for (CSpaceResource res : resources) { - authz.deletePermissions(res, roles); - } + authz.deletePermissionsFromRoles(resources, roles); } } } @@ -187,18 +179,9 @@ public class AuthorizationDelegate { logger.error(msg); throw new DocumentNotFoundException(msg); } - CSpaceResource[] resources = getResources(p); - AuthZ authz = AuthZ.get(); - for (CSpaceResource res : resources) { - 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()); - } - } + CSpaceResource[] resources = getResources(p); + AuthZ.get().deletePermissions(resources); } /** diff --git a/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionJpaFilter.java b/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionJpaFilter.java index 374f9721a..815f0cdf3 100644 --- a/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionJpaFilter.java +++ b/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionJpaFilter.java @@ -154,4 +154,16 @@ public class PermissionJpaFilter extends JpaDocumentFilter { public List buildWhere(StringBuilder queryStrBldr) { return new ArrayList(); } + + /* + * The Permission resource would normally lazy-load the "action" field. Witht this clause, we force JPA to load the action field. + * + * (non-Javadoc) + * @see org.collectionspace.services.common.document.DocumentFilter#getJoinFetchClause() + */ + @Override + public String getJoinFetchClause() { + return "JOIN FETCH a.action i"; + } + } diff --git a/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java b/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java index 392671bb6..a3614ef1b 100644 --- a/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java +++ b/services/common/src/main/java/org/collectionspace/services/authorization/storage/PermissionRoleDocumentHandler.java @@ -149,7 +149,7 @@ public class PermissionRoleDocumentHandler @Override public void completeCreate(DocumentWrapper> wrapDoc) throws Exception { PermissionRole pr = getCommonPart(); - AuthorizationDelegate.addPermissions(getServiceContext(), pr); + AuthorizationDelegate.addRelationships(getServiceContext(), pr); } /* (non-Javadoc) @@ -201,7 +201,7 @@ public class PermissionRoleDocumentHandler @Override public void completeDelete(DocumentWrapper> wrapDoc) throws Exception { PermissionRole pr = getCommonPart(); - AuthorizationDelegate.deletePermissions(getServiceContext(), pr); + AuthorizationDelegate.deletePermissionsFromRoles(getServiceContext(), pr); } /* (non-Javadoc) diff --git a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java index 028085649..3ba8a36dc 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java +++ b/services/common/src/main/java/org/collectionspace/services/common/authorization_mgt/AuthorizationCommon.java @@ -25,6 +25,7 @@ import org.collectionspace.services.account.AccountListItem; import org.collectionspace.services.authentication.Token; import org.collectionspace.services.authorization.AuthZ; import org.collectionspace.services.authorization.CSpaceAction; +import org.collectionspace.services.authorization.CSpaceResource; import org.collectionspace.services.authorization.PermissionException; import org.collectionspace.services.authorization.PermissionRole; import org.collectionspace.services.authorization.PermissionRoleRel; @@ -227,23 +228,16 @@ public class AuthorizationCommon { for (RoleValue roleValue : permRole.getRole()) { principals.add(roleValue.getRoleName()); } + + boolean grant = perm.getEffect().equals(EffectType.PERMIT) ? true : false; List permActions = perm.getAction(); + ArrayList resources = new ArrayList(); for (PermissionAction permAction : permActions) { - try { - CSpaceAction action = URIResourceImpl.getAction(permAction.getName()); - URIResourceImpl uriRes = new URIResourceImpl(perm.getTenantId(), - perm.getResourceName(), action); - boolean grant = perm.getEffect().equals(EffectType.PERMIT) ? true : false; - AuthZ.get().addPermissions(uriRes, principals.toArray(new String[0]), grant);//CSPACE-4967 - } catch (PermissionException e) { - // - // Only throw the exception if it is *not* an already-exists exception - // - if (e.getCause() instanceof AlreadyExistsException == false) { - throw e; - } - } + CSpaceAction action = URIResourceImpl.getAction(permAction.getName()); + URIResourceImpl uriRes = new URIResourceImpl(perm.getTenantId(), perm.getResourceName(), action); + resources.add(uriRes); } + AuthZ.get().addPermissions(resources.toArray(new CSpaceResource[0]), principals.toArray(new String[0]), grant); // CSPACE-4967 } private static Connection getConnection(String databaseName) throws NamingException, SQLException { diff --git a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentFilter.java b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentFilter.java index cc885f10b..24e43f175 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentFilter.java +++ b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentFilter.java @@ -243,6 +243,10 @@ public abstract class DocumentFilter { return selectClause != null ? selectClause : DEFAULT_SELECT_CLAUSE; } + public String getJoinFetchClause() { + return null; + } + /** * Gets the where clause. * diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/TransactionContext.java b/services/common/src/main/java/org/collectionspace/services/common/storage/TransactionContext.java index 3a1ca5e7d..09f3516a0 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/TransactionContext.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/TransactionContext.java @@ -34,4 +34,6 @@ public abstract class TransactionContext { abstract public Query createQuery(String qlString); abstract public void remove(Object entity); + + abstract public Object merge(Object entity); } diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JPATransactionContext.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JPATransactionContext.java index 33045098d..5954593e9 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JPATransactionContext.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JPATransactionContext.java @@ -80,6 +80,11 @@ public class JPATransactionContext extends TransactionContext { em.persist(entity); } + @Override + public Object merge(Object entity) { + return em.merge(entity); + } + @Override public Object find(Class entityClass, Object primaryKey) { return em.find(entityClass, primaryKey); diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaDocumentFilter.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaDocumentFilter.java index 60a7f299c..9e469e65d 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaDocumentFilter.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaDocumentFilter.java @@ -76,9 +76,9 @@ public class JpaDocumentFilter extends DocumentFilter { protected String addTenant(boolean append, List paramList) { String whereClause = ""; if (!append) { - whereClause = " WHERE tenantId = :tenantId"; + whereClause = " WHERE a.tenantId = :tenantId"; } else { - whereClause = " AND tenantId = :tenantId"; + whereClause = " AND a.tenantId = :tenantId"; } paramList.add(new ParamBinding("tenantId", getTenantId())); return whereClause; diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java index 368b17963..49bc98036 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaRelationshipStorageClient.java @@ -39,7 +39,7 @@ import org.collectionspace.services.authorization.Role; import org.collectionspace.services.authorization.RoleValue; import org.collectionspace.services.authorization.AccountRoleRel; import org.collectionspace.services.authorization.PermissionRoleRel; - +import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.document.BadRequestException; import org.collectionspace.services.common.document.DocumentException; @@ -50,7 +50,7 @@ import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.document.DocumentWrapper; import org.collectionspace.services.common.document.DocumentWrapperImpl; import org.collectionspace.services.common.document.JaxbUtils; - +import org.hsqldb.lib.StringUtil; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -150,32 +150,30 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { public void get(ServiceContext ctx, String id, DocumentHandler handler) throws DocumentNotFoundException, DocumentException { - if (getObject(ctx, id) == null) { - String msg = "get: " - + "could not find the object with id=" + id; - logger.error(msg); - throw new DocumentNotFoundException(msg); - } - - String objectId = getObjectId(ctx); - if (logger.isDebugEnabled()) { - logger.debug("get: using objectId=" + objectId); - } - Class objectClass = getObjectClass(ctx); - if (logger.isDebugEnabled()) { - logger.debug("get: using object class=" + objectClass.getName()); - } - DocumentFilter docFilter = handler.getDocumentFilter(); - if (docFilter == null) { - docFilter = handler.createDocumentFilter(); - } - - JPATransactionContext jpaConnectionContext = (JPATransactionContext)ctx.openConnection(); + JPATransactionContext jpaConnectionContext = (JPATransactionContext)ctx.openConnection(); try { + if (getObject(ctx, id) == null) { + String msg = "get: " + "could not find the object with id=" + id; + logger.error(msg); + throw new DocumentNotFoundException(msg); + } + + String objectId = getObjectId(ctx); + Class objectClass = getObjectClass(ctx); + DocumentFilter docFilter = handler.getDocumentFilter(); + if (docFilter == null) { + docFilter = handler.createDocumentFilter(); + } + handler.prepare(Action.GET); StringBuilder queryStrBldr = new StringBuilder("SELECT a FROM "); queryStrBldr.append(getEntityName(ctx)); queryStrBldr.append(" a"); + + String joinFetch = docFilter.getJoinFetchClause(); + if (Tools.notBlank(joinFetch)) { + queryStrBldr.append(" " + joinFetch); + } queryStrBldr.append(" WHERE " + objectId + " = :objectId"); String where = docFilter.getWhereClause(); @@ -190,10 +188,10 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { Query q = jpaConnectionContext.createQuery(queryStr); q.setParameter("objectId", id); - List rl = new ArrayList(); + List relList = new ArrayList(); jpaConnectionContext.beginTransaction(); try { - rl = q.getResultList(); + relList = q.getResultList(); } catch (NoResultException nre) { String msg = "get(1): " + " could not find relationships for object class=" + objectClass.getName() + " id=" + id; @@ -201,20 +199,23 @@ public class JpaRelationshipStorageClient extends JpaStorageClientImpl { logger.debug(msg, nre); } } - if (rl.size() == 0) { + if (relList.size() == 0) { String msg = "get(2): " + " could not find relationships for object class=" + objectClass.getName() + " id=" + id; if (logger.isDebugEnabled()) { logger.debug(msg); } } - DocumentWrapper> wrapDoc = - new DocumentWrapperImpl>(rl); + DocumentWrapper> wrapDoc = new DocumentWrapperImpl>(relList); handler.handle(Action.GET, wrapDoc); handler.complete(Action.GET, wrapDoc); jpaConnectionContext.commitTransaction(); + } catch (DocumentNotFoundException nfe) { + jpaConnectionContext.markForRollback(); + throw nfe; } catch (DocumentException de) { jpaConnectionContext.markForRollback(); + throw de; } catch (Exception e) { jpaConnectionContext.markForRollback(); if (logger.isDebugEnabled()) { 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 88367d02b..613e7d017 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 @@ -38,6 +38,7 @@ import org.collectionspace.services.common.storage.StorageClient; import org.collectionspace.services.common.storage.TransactionContext; import org.collectionspace.services.common.vocabulary.RefNameServiceUtils.AuthorityItemSpecifier; import org.collectionspace.services.common.context.ServiceContextProperties; +import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.lifecycle.TransitionDef; @@ -168,7 +169,7 @@ public class JpaStorageClientImpl implements StorageClient { try { handler.prepare(Action.GET); Object o = null; - o = JpaStorageUtils.getEntity(jpaTransactionContext, getEntityName(ctx), id, ctx.getTenantId()); + o = JpaStorageUtils.getEntity(jpaTransactionContext, handler.getDocumentFilter(), getEntityName(ctx), id, ctx.getTenantId()); if (null == o) { String msg = "Could not find entity with id=" + id; throw new DocumentNotFoundException(msg); @@ -218,6 +219,11 @@ public class JpaStorageClientImpl implements StorageClient { queryStrBldr.append(getEntityName(ctx)); queryStrBldr.append(" a"); + String joinFetch = docFilter.getJoinFetchClause(); + if (Tools.notBlank(joinFetch)) { + queryStrBldr.append(" " + joinFetch); + } + List params = docFilter.buildWhereForSearch(queryStrBldr); String queryStr = queryStrBldr.toString(); //for debugging Query q = jpaConnectionContext.createQuery(queryStr); diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java index d3ff99c34..56f7f791b 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageUtils.java @@ -45,8 +45,10 @@ import org.collectionspace.services.authorization.CSpaceResource; import org.collectionspace.services.authorization.PermissionRoleRel; import org.collectionspace.services.authorization.PermissionValue; import org.collectionspace.services.authorization.URIResourceImpl; +import org.collectionspace.services.common.api.Tools; import org.collectionspace.services.common.authorization_mgt.AuthorizationRoleRel; import org.collectionspace.services.common.context.ServiceContext; +import org.collectionspace.services.common.document.DocumentFilter; import org.collectionspace.services.common.document.DocumentNotFoundException; import org.collectionspace.services.common.security.UnauthorizedException; import org.collectionspace.services.common.document.JaxbUtils; @@ -328,7 +330,19 @@ public class JpaStorageUtils { return result; } - public static Object getEnityByKey(JPATransactionContext jpaTransactionContext, String entityName, String key, String value, + public static Object getEnityByKey( + JPATransactionContext jpaTransactionContext, + String entityName, + String key, + String value, + String tenantId) throws TransactionException { + return getEnityByKey(jpaTransactionContext, (DocumentFilter)null, entityName, key, value, tenantId); + } + + public static Object getEnityByKey( + JPATransactionContext jpaTransactionContext, + DocumentFilter docFilter, + String entityName, String key, String value, String tenantId) throws TransactionException { Object result = null; @@ -337,9 +351,17 @@ public class JpaStorageUtils { StringBuilder queryStrBldr = new StringBuilder("SELECT a FROM "); queryStrBldr.append(entityName); queryStrBldr.append(" a"); - queryStrBldr.append(" WHERE " + key + " = :" + key); + + if (docFilter != null) { + String joinFetch = docFilter.getJoinFetchClause(); + if (Tools.notBlank(joinFetch)) { + queryStrBldr.append(" " + joinFetch); + } + } + + queryStrBldr.append(" WHERE a." + key + " = :" + key); if (useTenantId == true) { - queryStrBldr.append(" AND tenantId = :tenantId"); + queryStrBldr.append(" AND a.tenantId = :tenantId"); } String queryStr = queryStrBldr.toString(); //for debugging Query q = jpaTransactionContext.createQuery(queryStr); @@ -434,12 +456,13 @@ public class JpaStorageUtils { */ public static Object getEntity( JPATransactionContext jpaTransactionContext, + DocumentFilter docFilter, String entityName, String id, String tenantId) throws TransactionException { - return getEnityByKey(jpaTransactionContext, entityName, CSID_LABEL, id, tenantId); + return getEnityByKey(jpaTransactionContext, docFilter, entityName, CSID_LABEL, id, tenantId); } - + /** * getEntity * @param entityName fully qualified entity name