From 830a9a81d1d6db436cf810b471b03fb607dd4a85 Mon Sep 17 00:00:00 2001 From: remillet Date: Sun, 14 Jan 2018 17:51:50 -0800 Subject: [PATCH] DRYD-221: Fixed bug in account update code. Added new XMLReplay tests for accounts and roles. --- .../test-data/xmlreplay/security.xml | 57 +++++++++++++++---- .../xmlreplay/security/6-account-elmo.xml | 28 ++++----- .../BasicRoles/createSimpleAccount-1.xml | 13 +++++ .../BasicRoles/createSimpleAccount-2.xml | 20 +++++++ .../BasicRoles/createSimpleRole-1.xml | 5 ++ .../BasicRoles/createSimpleRole-2.xml | 11 ++++ .../slipOutAccount-1.xml} | 6 +- .../{ => SlipOut}/slipOutPerm-update.xml | 0 .../security/{ => SlipOut}/slipOutPerm.xml | 0 .../security/{ => SlipOut}/slipOutRole.xml | 0 .../security/{ => SlipOut}/slipOutVocab-1.xml | 0 .../security/{ => SlipOut}/slipOutVocab-2.xml | 0 .../test-data/xmlreplay/xml-replay-master.xml | 10 +++- .../client/test/AccountRoleServiceTest.java | 10 ++-- .../services/account/AccountResource.java | 9 ++- .../storage/AccountDocumentHandler.java | 17 +++--- .../account/storage/AccountStorageClient.java | 7 ++- .../test/PermissionRoleServiceTest.java | 51 ++--------------- .../client/test/RoleServiceTest.java | 8 ++- .../client/test/AbstractServiceTestImpl.java | 7 ++- .../services/common/SecurityResourceBase.java | 12 +++- .../document/AbstractDocumentHandlerImpl.java | 8 +++ .../common/document/DocumentHandler.java | 3 + .../common/storage/jpa/JpaStorageUtils.java | 3 +- 24 files changed, 184 insertions(+), 101 deletions(-) create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-1.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-2.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-1.xml create mode 100644 services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-2.xml rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{create-account-grover.xml => SlipOut/slipOutAccount-1.xml} (82%) rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{ => SlipOut}/slipOutPerm-update.xml (100%) rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{ => SlipOut}/slipOutPerm.xml (100%) rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{ => SlipOut}/slipOutRole.xml (100%) rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{ => SlipOut}/slipOutVocab-1.xml (100%) rename services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/{ => SlipOut}/slipOutVocab-2.xml (100%) diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security.xml index 5074eeaa4..66014a9dd 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security.xml @@ -2,45 +2,78 @@ + YWRtaW5AY29yZS5jb2xsZWN0aW9uc3BhY2Uub3JnOkFkbWluaXN0cmF0b3I= dXNlcjFAbXVzZXVtMS5vcmc6dXNlcjFAbXVzZXVtMS5vcmc= YmlnYmlyZDIwMTA6YmlnYmlyZDIwMTA= ZWxtbzIwMTA6ZWxtbzIwMTA= - Z3JvdmVyMjAxODpncm92ZXIyMDE4 + Z3JvdmVyOmdyb3ZlcjIwMTg= - + + + POST + /cspace-services/accounts + security/BasicRoles/createSimpleAccount-1.xml + + + POST + /cspace-services/authorization/roles + security/BasicRoles/createSimpleRole-1.xml + + + + POST + /cspace-services/authorization/roles + security/BasicRoles/createSimpleRole-2.xml + + + POST + /cspace-services/accounts + security/BasicRoles/createSimpleAccount-2.xml + + + + POST /cspace-services/authorization/permissions - security/slipOutPerm.xml + security/SlipOut/slipOutPerm.xml POST /cspace-services/authorization/roles - security/slipOutRole.xml + security/SlipOut/slipOutRole.xml - + POST /cspace-services/accounts - security/create-account-grover.xml + security/SlipOut/slipOutAccount-1.xml - + POST /cspace-services/vocabularies - security/slipOutVocab-1.xml + security/SlipOut/slipOutVocab-1.xml PUT /cspace-services/authorization/permissions/${slipOutPerm.CSID} - security/slipOutPerm-update.xml + security/SlipOut/slipOutPerm-update.xml - + 403 POST /cspace-services/vocabularies - security/slipOutVocab-2.xml + security/SlipOut/slipOutVocab-2.xml @@ -60,7 +93,7 @@ - + POST diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/6-account-elmo.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/6-account-elmo.xml index af1447aee..80180083a 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/6-account-elmo.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/6-account-elmo.xml @@ -1,16 +1,16 @@ - - elmo2010 - elmo2010 - elmo@cspace.org - 1234567890 - elmo2010 - - ZWxtbzIwMTA= - - 1 - + + elmo2018 + elmo2018 + elmo2018@DeleteBug.accountelemo.org + 1234567890 + elmo2018 + + Z3JvdmVyMjAxOA== + + 1 + + + ${permElmo.CSID} + - diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-1.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-1.xml new file mode 100644 index 000000000..3612f7c5b --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-1.xml @@ -0,0 +1,13 @@ + + + simpleAccount-1 + simpleAccount-1 + simpleAccount-1@security.simpleroles.org + 1234567890 + simpleAccount-1 + + ZWxtbzIwMTA= + + 1 + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-2.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-2.xml new file mode 100644 index 000000000..657682ef9 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleAccount-2.xml @@ -0,0 +1,20 @@ + + + simpleAccount-2 + simpleAccount-2 + simpleAccount-2@security.simpleroles.org + 1234567890 + simpleAccount-2 + + ZWxtbzIwMTA= + + 1 + + + 9a1fed44-25b0-48f9-8356-d16ac7555cae + ROLE_1_TENANT_ADMINISTRATOR + + + ${simpleRole_2.CSID} + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-1.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-1.xml new file mode 100644 index 000000000..3bf4d1861 --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-1.xml @@ -0,0 +1,5 @@ + + + BasicRoles-SimpleRole-1 + Role for BasicRoles-simpleRole-1 create test. + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-2.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-2.xml new file mode 100644 index 000000000..41640460c --- /dev/null +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/BasicRoles/createSimpleRole-2.xml @@ -0,0 +1,11 @@ + + + BasicRoles-SimpleRole-2 + Role for BasicRoles-simpleRole-2 create test. + + 1-vocabularies-RL + + + 1-groups-RL + + diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/create-account-grover.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutAccount-1.xml similarity index 82% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/create-account-grover.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutAccount-1.xml index 6ca2535e9..6eac120fa 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/create-account-grover.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutAccount-1.xml @@ -1,10 +1,10 @@ - grover2018 - grover2018 + grover + grover grover@cspace.org 1234567890 - grover2018 + grover Z3JvdmVyMjAxOA== diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutPerm-update.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutPerm-update.xml similarity index 100% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutPerm-update.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutPerm-update.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutPerm.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutPerm.xml similarity index 100% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutPerm.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutPerm.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutRole.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutRole.xml similarity index 100% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutRole.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutRole.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutVocab-1.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutVocab-1.xml similarity index 100% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutVocab-1.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutVocab-1.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutVocab-2.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutVocab-2.xml similarity index 100% rename from services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/slipOutVocab-2.xml rename to services/IntegrationTests/src/test/resources/test-data/xmlreplay/security/SlipOut/slipOutVocab-2.xml diff --git a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml index 796a932c7..03449900d 100644 --- a/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml +++ b/services/IntegrationTests/src/test/resources/test-data/xmlreplay/xml-replay-master.xml @@ -5,13 +5,17 @@ + YWRtaW5AY29yZS5jb2xsZWN0aW9uc3BhY2Uub3JnOkFkbWluaXN0cmF0b3I= - - - diff --git a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java index a6b7b8a2f..967e2d9d0 100644 --- a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java +++ b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountRoleServiceTest.java @@ -114,14 +114,14 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl wrapDoc) throws Exception { AccountsCommon upAcc = wrapDoc.getWrappedObject(); - getServiceContext().setOutput(upAcc); - sanitize(upAcc); + getServiceContext().setOutput(upAcc); } @Override @@ -277,17 +274,23 @@ public class AccountDocumentHandler * sanitize removes data not needed to be sent to the consumer * @param account */ - private void sanitize(AccountsCommon account) { + @Override + public void sanitize(DocumentWrapper wrapDoc) { + AccountsCommon account = wrapDoc.getWrappedObject(); + sanitize(account); + } + + private void sanitize(AccountsCommon account) { account.setPassword(null); if (!SecurityUtils.isCSpaceAdmin()) { account.setTenants(new ArrayList(0)); } - } + } /* (non-Javadoc) * @see org.collectionspace.services.common.document.DocumentHandler#initializeDocumentFilter(org.collectionspace.services.common.context.ServiceContext) */ - public void initializeDocumentFilter(ServiceContext ctx) { + public void initializeDocumentFilter(ServiceContext ctx) { // set a default document filter in this method } } 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 0977ce61c..1a0df03bf 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 @@ -186,9 +186,12 @@ public class AccountStorageClient extends JpaStorageClientImpl { DocumentWrapper wrapDoc = new DocumentWrapperImpl(accountFound); handler.handle(Action.UPDATE, wrapDoc); - handler.complete(Action.UPDATE, wrapDoc); - + handler.complete(Action.UPDATE, wrapDoc); jpaConnectionContext.commitTransaction(); + // + // Don't sanitize until we've committed changes to the DB + // + handler.sanitize(wrapDoc); } catch (BadRequestException bre) { jpaConnectionContext.markForRollback(); throw bre; 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 c444233ed..45aed137d 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 @@ -290,59 +290,19 @@ public class PermissionRoleServiceTest extends AbstractServiceTestImpl extends AbstractCollectionSpa logger.debug("get with csid=" + csid); ensureCSID(csid, ServiceMessages.GET_FAILED + "csid"); Object result = null; + try { ServiceContext ctx = createServiceContext((IT) null, objectClass, ui); DocumentHandler handler = createDocumentHandler(ctx); getStorageClient(ctx).get(ctx, csid, handler); result = ctx.getOutput(); - } catch (Exception e) { + } catch (DocumentException e) { + Exception cause = (Exception) e.getCause(); + if (cause instanceof NoResultException) { + Response response = Response.status(Response.Status.NOT_FOUND).entity(result).type("text/plain").build(); + throw new CSWebApplicationException(response); + } + } catch (Exception e) { throw bigReThrow(e, ServiceMessages.GET_FAILED, csid); } + checkResult(result, csid, ServiceMessages.GET_FAILED); return result; } diff --git a/services/common/src/main/java/org/collectionspace/services/common/document/AbstractDocumentHandlerImpl.java b/services/common/src/main/java/org/collectionspace/services/common/document/AbstractDocumentHandlerImpl.java index f6af6159b..8ffa1d259 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/document/AbstractDocumentHandlerImpl.java +++ b/services/common/src/main/java/org/collectionspace/services/common/document/AbstractDocumentHandlerImpl.java @@ -266,6 +266,14 @@ public abstract class AbstractDocumentHandlerImpl return result; } + + @Override + public void sanitize(DocumentWrapper wrapDoc) { + // + // By default, do nothing. Sub-classes can override if they want to. + // + } + /* (non-Javadoc) * @see org.collectionspace.services.common.document.DocumentHandler#handleCreate(org.collectionspace.services.common.document.DocumentWrapper) diff --git a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentHandler.java b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentHandler.java index 5e1db5046..668357c64 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/document/DocumentHandler.java +++ b/services/common/src/main/java/org/collectionspace/services/common/document/DocumentHandler.java @@ -19,6 +19,7 @@ package org.collectionspace.services.common.document; import java.util.Map; +import org.collectionspace.services.account.AccountsCommon; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.query.QueryContext; import org.collectionspace.services.common.vocabulary.RefNameServiceUtils.Specifier; @@ -384,4 +385,6 @@ public interface DocumentHandler { */ void completeSync(DocumentWrapper wrapDoc) throws Exception; + public void sanitize(DocumentWrapper wrapDoc); + } 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 705cd756e..7fc0aff2f 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 @@ -32,6 +32,7 @@ import java.util.Map; import javax.persistence.PersistenceException; import javax.persistence.EntityManager; import javax.persistence.EntityManagerFactory; +import javax.persistence.EntityNotFoundException; import javax.persistence.NoResultException; import javax.persistence.Persistence; import javax.persistence.Query; @@ -544,7 +545,7 @@ public class JpaStorageUtils { q.setParameter(paramName, paramBindings.get(paramName)); } - result = q.getSingleResult(); + result = q.getSingleResult(); if (result == null) { logger.debug("Call to getEntity() returned empty set."); -- 2.47.3