From: Patrick Schmitz Date: Tue, 23 Aug 2011 00:23:50 +0000 (+0000) Subject: CSPACE-2844 - Added flags and logic to preclude changes to the prebuilt Accounts... X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=11d46a32ee6c1fcc38ca2cd64a0a3be70f178abc;p=tmp%2Fjakarta-migration.git CSPACE-2844 - Added flags and logic to preclude changes to the prebuilt Accounts and Roles. Added tests for same. Fixed some subtle bugs in the semi-related document handlers for Permission, Role, and Account. --- diff --git a/services/account/client/src/main/java/org/collectionspace/services/client/AccountClient.java b/services/account/client/src/main/java/org/collectionspace/services/client/AccountClient.java index 6fff4a5e7..52f67e58f 100644 --- a/services/account/client/src/main/java/org/collectionspace/services/client/AccountClient.java +++ b/services/account/client/src/main/java/org/collectionspace/services/client/AccountClient.java @@ -40,6 +40,7 @@ public class AccountClient extends AbstractServiceClientImpl allResourceIdsCreated = new ArrayList(); /** The acc values. */ @@ -486,6 +489,33 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { } + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"read"}) + public void deleteLockedAccount(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + testBanner(testName, CLASS_NAME); + } + + findPrebuiltAdminAccount(); + + // Perform setup. + EXPECTED_STATUS_CODE = Response.Status.FORBIDDEN.getStatusCode(); + REQUEST_TYPE = ServiceRequestType.DELETE; + testSetup(EXPECTED_STATUS_CODE, REQUEST_TYPE); + + AccountRoleClient client = new AccountRoleClient(); + ClientResponse res = client.delete(prebuiltAdminCSID); + try { + int statusCode = res.getStatus(); + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + } finally { + res.releaseConnection(); + } + } + // Failure outcomes /* (non-Javadoc) * @see org.collectionspace.services.client.test.AbstractServiceTestImpl#deleteNonExistent(java.lang.String) @@ -625,6 +655,27 @@ public class AccountRoleServiceTest extends AbstractServiceTestImpl { return extractId(res); } + private void findPrebuiltAdminAccount() { + // Search for the prebuilt admin user and then hold its CSID + if(prebuiltAdminCSID == null) { + setupReadList(); + AccountClient client = new AccountClient(); + ClientResponse res = + client.readSearchList(null, this.prebuiltAdminUserId, null); + AccountsCommonList list = res.getEntity(); + int statusCode = res.getStatus(); + + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + List items = list.getAccountListItem(); + Assert.assertEquals(1, items.size(), "Found more than one Admin account!"); + AccountsCommonList.AccountListItem item = items.get(0); + prebuiltAdminCSID = item.getCsid(); + if (logger.isDebugEnabled()) { + logger.debug("Found Admin Account with ID: " + prebuiltAdminCSID); + } + } + } + /** * Delete account. * diff --git a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountServiceTest.java b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountServiceTest.java index dac8a3586..ba080fa92 100644 --- a/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountServiceTest.java +++ b/services/account/client/src/test/java/org/collectionspace/services/account/client/test/AccountServiceTest.java @@ -57,6 +57,8 @@ public class AccountServiceTest extends AbstractServiceTestImpl { // Instance variables specific to this test. /** The known resource id. */ private String knownResourceId = null; + private String prebuiltAdminCSID = null; + private String prebuiltAdminUserId = "admin@core.collectionspace.org"; private String knownUserId = "barney"; private String knownUserPassword = "hithere08"; /** The add tenant. */ @@ -931,6 +933,173 @@ public class AccountServiceTest extends AbstractServiceTestImpl { Assert.assertEquals(statusCode, Response.Status.BAD_REQUEST.getStatusCode()); } + + private void findPrebuiltAdminAccount() { + // Search for the prebuilt admin user and then hold its CSID + if(prebuiltAdminCSID == null) { + setupReadList(); + AccountClient client = new AccountClient(); + ClientResponse res = + client.readSearchList(null, this.prebuiltAdminUserId, null); + AccountsCommonList list = res.getEntity(); + int statusCode = res.getStatus(); + + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + List items = list.getAccountListItem(); + Assert.assertEquals(1, items.size(), "Found more than one Admin account!"); + AccountsCommonList.AccountListItem item = items.get(0); + prebuiltAdminCSID = item.getCsid(); + if (logger.isDebugEnabled()) { + logger.debug("Found Admin Account with ID: " + prebuiltAdminCSID); + } + } + } + + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"read", "readList", "readNonExistent"}) + public void verifyMetadataProtection(String testName) throws Exception { + findPrebuiltAdminAccount(); + // Try to update the metadata - it should just get ignored + if (logger.isDebugEnabled()) { + logger.debug(testBanner(testName, CLASS_NAME)); + } + // Perform setup. + setupUpdate(); + + AccountClient client = new AccountClient(); + ClientResponse res = client.read(prebuiltAdminCSID); + if (logger.isDebugEnabled()) { + logger.debug(testName + ": read status = " + res.getStatus()); + } + Assert.assertEquals(res.getStatus(), EXPECTED_STATUS_CODE); + + if (logger.isDebugEnabled()) { + logger.debug("Did get on Admin Account to update with ID: " + prebuiltAdminCSID); + } + AccountsCommon accountFound = (AccountsCommon) res.getEntity(); + Assert.assertNotNull(accountFound); + + //create a new account object to test partial updates + AccountsCommon accountToUpdate = new AccountsCommon(); + accountToUpdate.setCsid(prebuiltAdminCSID); + accountToUpdate.setUserId(accountFound.getUserId()); + // Update the content of this resource. + accountToUpdate.setEmail("updated-" + accountFound.getEmail()); + if (logger.isDebugEnabled()) { + logger.debug("updated object"); + logger.debug(objectAsXmlString(accountFound, + AccountsCommon.class)); + } + + // Submit the request to the service and store the response. + res = client.update(prebuiltAdminCSID, accountToUpdate); + int statusCode = res.getStatus(); + // Check the status code of the response: does it match the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + // Note that the error is not returned, it is just ignored + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + AccountsCommon accountUpdated = (AccountsCommon) res.getEntity(); + Assert.assertNotNull(accountUpdated); + + Assert.assertFalse(accountUpdated.getEmail().equals(accountToUpdate.getEmail()), + "Admin Account (with metadata lock) allowed update to change the email!"); + } + + + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"read", "readList", "readNonExistent"}) + public void verifyProtectionReadOnly(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + logger.debug(testBanner(testName, CLASS_NAME)); + } + + setupCreate(); + + // Submit the request to the service and store the response. + AccountClient client = new AccountClient(); + AccountsCommon account = + createAccountInstance("mdTest", "mdTest", "mdTestPW", "md@test.com", + client.getTenantId(), true, false, true, true); + account.setMetadataProtection(AccountClient.IMMUTABLE); + account.setRolesProtection(AccountClient.IMMUTABLE); + ClientResponse res = client.create(account); + int statusCode = res.getStatus(); + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + // Store the ID returned from this create operation + // for additional tests below. + String testResourceId = extractId(res); + allResourceIdsCreated.add(testResourceId); + if (logger.isDebugEnabled()) { + logger.debug(testName + ": testResourceId=" + testResourceId); + } + setupRead(); + + // Submit the request to the service and store the response. + ClientResponse accountRes = client.read(testResourceId); + statusCode = accountRes.getStatus(); + + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + AccountsCommon accountRead = (AccountsCommon) accountRes.getEntity(); + Assert.assertNotNull(accountRead); + String mdProtection = accountRead.getMetadataProtection(); + String rolesProtection = accountRead.getRolesProtection(); + if (logger.isTraceEnabled()) { + logger.trace(testName + ": metadataProtection=" + mdProtection); + logger.trace(testName + ": rolesProtection=" + rolesProtection); + } + Assert.assertFalse(account.getMetadataProtection().equals(mdProtection), + "Account allowed create to set the metadata protection flag."); + Assert.assertFalse(account.getRolesProtection().equals(rolesProtection), + "Account allowed create to set the perms protection flag."); + + setupUpdate(); + + AccountsCommon accountToUpdate = + createAccountInstance("mdTest", "mdTest", "mdTestPW", "md@test.com", + client.getTenantId(), true, false, true, true); + accountToUpdate.setMetadataProtection(AccountClient.IMMUTABLE); + accountToUpdate.setRolesProtection(AccountClient.IMMUTABLE); + + // Submit the request to the service and store the response. + accountRes = client.update(testResourceId, accountToUpdate); + statusCode = accountRes.getStatus(); + // Check the status code of the response: does it match the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + + AccountsCommon accountUpdated = (AccountsCommon) accountRes.getEntity(); + Assert.assertNotNull(accountUpdated); + if (logger.isDebugEnabled()) { + logger.debug(testName + "Updated account: "); + logger.debug(objectAsXmlString(accountUpdated,AccountsCommon.class)); + } + + Assert.assertFalse( + AccountClient.IMMUTABLE.equalsIgnoreCase(accountUpdated.getMetadataProtection()), + "Account allowed update of the metadata protection flag."); + Assert.assertFalse( + AccountClient.IMMUTABLE.equalsIgnoreCase(accountUpdated.getRolesProtection()), + "Account allowed update of the roles protection flag."); + } + + /** * Deactivate. diff --git a/services/account/jaxb/src/main/resources/accounts_common.xsd b/services/account/jaxb/src/main/resources/accounts_common.xsd index 27ed6de85..80454216f 100644 --- a/services/account/jaxb/src/main/resources/accounts_common.xsd +++ b/services/account/jaxb/src/main/resources/accounts_common.xsd @@ -139,7 +139,25 @@ - + + + + + + + + + + + + + + + + + + + diff --git a/services/account/pstore/src/main/resources/db/mysql/account.sql b/services/account/pstore/src/main/resources/db/mysql/account.sql index c92067828..7e71097b3 100644 --- a/services/account/pstore/src/main/resources/db/mysql/account.sql +++ b/services/account/pstore/src/main/resources/db/mysql/account.sql @@ -2,7 +2,7 @@ alter table accounts_tenants drop foreign key FKFDA649B05A9CEEB5; drop table if exists accounts_common; drop table if exists accounts_tenants; drop table if exists tenants; -create table accounts_common (csid varchar(128) not null, created_at datetime not null, email varchar(255) not null, mobile varchar(255), person_ref_name varchar(255), phone varchar(255), screen_name varchar(128) not null, status varchar(15) not null, updated_at datetime, userid varchar(128) not null, primary key (csid)); +create table accounts_common (csid varchar(128) not null, created_at datetime not null, email varchar(255) not null, mobile varchar(255), person_ref_name varchar(255), phone varchar(255), screen_name varchar(128) not null, status varchar(15) not null, updated_at datetime, userid varchar(128) not null, metadata_protection varchar(255), roles_protection varchar(255), primary key (csid)); create table accounts_tenants (HJID bigint not null auto_increment, tenant_id varchar(128) not null, TENANTS_ACCOUNTSCOMMON_CSID varchar(128), primary key (HJID)); create table tenants (id varchar(128) not null, created_at datetime not null, name varchar(255) not null, updated_at datetime, primary key (id)); alter table accounts_tenants add index FKFDA649B05A9CEEB5 (TENANTS_ACCOUNTSCOMMON_CSID), add constraint FKFDA649B05A9CEEB5 foreign key (TENANTS_ACCOUNTSCOMMON_CSID) references accounts_common (csid); diff --git a/services/account/pstore/src/main/resources/db/postgresql/account.sql b/services/account/pstore/src/main/resources/db/postgresql/account.sql index 2a7fa297c..49f81aede 100644 --- a/services/account/pstore/src/main/resources/db/postgresql/account.sql +++ b/services/account/pstore/src/main/resources/db/postgresql/account.sql @@ -3,7 +3,7 @@ DROP TABLE IF EXISTS accounts_common CASCADE; DROP TABLE IF EXISTS accounts_tenants CASCADE; DROP TABLE IF EXISTS tenants CASCADE; DROP SEQUENCE IF EXISTS hibernate_sequence; -create table accounts_common (csid varchar(128) not null, created_at timestamp not null, email varchar(255) not null, mobile varchar(255), person_ref_name varchar(255), phone varchar(255), screen_name varchar(128) not null, status varchar(15) not null, updated_at timestamp, userid varchar(128) not null, primary key (csid)); +create table accounts_common (csid varchar(128) not null, created_at timestamp not null, email varchar(255) not null, mobile varchar(255), person_ref_name varchar(255), phone varchar(255), screen_name varchar(128) not null, status varchar(15) not null, updated_at timestamp, userid varchar(128) not null, metadata_protection varchar(255), roles_protection varchar(255), primary key (csid)); create table accounts_tenants (HJID int8 not null, tenant_id varchar(128) not null, TENANTS_ACCOUNTSCOMMON_CSID varchar(128), primary key (HJID)); create table tenants (id varchar(128) not null, created_at timestamp not null, name varchar(255) not null, updated_at timestamp, primary key (id)); alter table accounts_tenants add constraint FKFDA649B05A9CEEB5 foreign key (TENANTS_ACCOUNTSCOMMON_CSID) references accounts_common; diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/AccountResource.java b/services/account/service/src/main/java/org/collectionspace/services/account/AccountResource.java index 61da822b9..4be106a4d 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/AccountResource.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/AccountResource.java @@ -49,6 +49,7 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; @@ -120,7 +121,14 @@ public class AccountResource extends SecurityResourceBase { logger.debug("deleteAccount with csid=" + csid); ensureCSID(csid, ServiceMessages.DELETE_FAILED); try { - //FIXME ideally the following two ops shoudl be in the same tx CSPACE-658 + AccountsCommon account = (AccountsCommon)get(csid, AccountsCommon.class); + // If marked as metadata immutable, do not delete + if(AccountClient.IMMUTABLE.equals(account.getMetadataProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Account: "+csid+" is immutable.").type("text/plain").build(); + return response; + } + //FIXME ideally the following two ops should be in the same tx CSPACE-658 //delete all relationships AccountRoleSubResource subResource = new AccountRoleSubResource("accounts/accountroles"); subResource.deleteAccountRole(csid, SubjectType.ROLE); @@ -147,6 +155,13 @@ public class AccountResource extends SecurityResourceBase { logger.debug("createAccountRole with accCsid=" + accCsid); ensureCSID(accCsid, ServiceMessages.POST_FAILED+ "accountroles account "); try { + AccountsCommon account = (AccountsCommon)get(accCsid, AccountsCommon.class); + // If marked as roles immutable, do not create + if(AccountClient.IMMUTABLE.equals(account.getRolesProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Roles for Account: "+accCsid+" are immutable.").type("text/plain").build(); + return response; + } AccountRoleSubResource subResource = new AccountRoleSubResource(AccountRoleSubResource.ACCOUNT_ACCOUNTROLE_SERVICE); String accrolecsid = subResource.createAccountRole(input, SubjectType.ROLE); @@ -218,6 +233,13 @@ public class AccountResource extends SecurityResourceBase { logger.debug("deleteAccountRole with accCsid=" + accCsid); ensureCSID(accCsid, ServiceMessages.DELETE_FAILED+ "accountroles account "); try { + AccountsCommon account = (AccountsCommon)get(accCsid, AccountsCommon.class); + // If marked as roles immutable, do not delete + if(AccountClient.IMMUTABLE.equals(account.getRolesProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Roles for Account: "+accCsid+" are immutable.").type("text/plain").build(); + return response; + } AccountRoleSubResource subResource = new AccountRoleSubResource(AccountRoleSubResource.ACCOUNT_ACCOUNTROLE_SERVICE); //delete all relationships for an account @@ -234,6 +256,13 @@ public class AccountResource extends SecurityResourceBase { logger.debug("deleteAccountRole: All roles related to account with accCsid=" + accCsid); ensureCSID(accCsid, ServiceMessages.DELETE_FAILED+ "accountroles account "); try { + // If marked as roles immutable, do not delete + AccountsCommon account = (AccountsCommon)get(accCsid, AccountsCommon.class); + if(AccountClient.IMMUTABLE.equals(account.getRolesProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Roles for Account: "+accCsid+" are immutable.").type("text/plain").build(); + return response; + } AccountRoleSubResource subResource = new AccountRoleSubResource(AccountRoleSubResource.ACCOUNT_ACCOUNTROLE_SERVICE); //delete all relationships for an account diff --git a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountDocumentHandler.java b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountDocumentHandler.java index 03c192217..5ccb9f0f5 100644 --- a/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountDocumentHandler.java +++ b/services/account/service/src/main/java/org/collectionspace/services/account/storage/AccountDocumentHandler.java @@ -34,6 +34,7 @@ import org.collectionspace.services.account.AccountsCommonList; import org.collectionspace.services.account.AccountsCommonList.AccountListItem; import org.collectionspace.services.account.Status; +import org.collectionspace.services.client.AccountClient; import org.collectionspace.services.common.storage.jpa.JpaDocumentHandler; import org.collectionspace.services.common.context.ServiceContext; import org.collectionspace.services.common.document.DocumentFilter; @@ -62,13 +63,19 @@ public class AccountDocumentHandler account.setCsid(id); setTenant(account); account.setStatus(Status.ACTIVE); + // We do not allow creation of locked accounts through the services. + account.setMetadataProtection(null); + account.setRolesProtection(null); } @Override public void handleUpdate(DocumentWrapper wrapDoc) throws Exception { AccountsCommon accountFound = wrapDoc.getWrappedObject(); AccountsCommon accountReceived = getCommonPart(); - merge(accountReceived, accountFound); + // If marked as metadata immutable, do not do update + if(!AccountClient.IMMUTABLE.equals(accountFound.getMetadataProtection())) { + merge(accountReceived, accountFound); + } } /** @@ -99,6 +106,7 @@ public class AccountDocumentHandler if (from.getPersonRefName() != null) { to.setPersonRefName(from.getPersonRefName()); } + // Note that we do not allow update of locks //fixme update for tenant association if (logger.isDebugEnabled()) { @@ -111,7 +119,7 @@ public class AccountDocumentHandler @Override public void completeUpdate(DocumentWrapper wrapDoc) throws Exception { AccountsCommon upAcc = wrapDoc.getWrappedObject(); - getServiceContext().setOutput(account); + getServiceContext().setOutput(upAcc); sanitize(upAcc); } 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 6252ec518..2bf5b401e 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 @@ -205,6 +205,8 @@ public class AccountStorageClient extends JpaStorageClientImpl { AccountsCommon accountFound = getAccount(em, id); checkAllowedUpdates(accountReceived, accountFound); //if userid and password are given, add to default id provider + // Note that this ignores the immutable flag, as we allow + // password changes. if (accountReceived.getUserId() != null && isForCSIdP(accountReceived.getPassword())) { userStorageClient.update(em, diff --git a/services/authorization-mgt/client/src/main/java/org/collectionspace/services/client/RoleClient.java b/services/authorization-mgt/client/src/main/java/org/collectionspace/services/client/RoleClient.java index 544d173a3..1d888e918 100644 --- a/services/authorization-mgt/client/src/main/java/org/collectionspace/services/client/RoleClient.java +++ b/services/authorization-mgt/client/src/main/java/org/collectionspace/services/client/RoleClient.java @@ -43,6 +43,7 @@ public class RoleClient extends AbstractServiceClientImpl public static final String SERVICE_PATH_COMPONENT = SERVICE_NAME; public static final String SERVICE_PATH = "/" + SERVICE_PATH_COMPONENT; public static final String SERVICE_PATH_PROXY = SERVICE_PATH + "/"; + public final static String IMMUTABLE = "immutable"; @Override public String getServiceName() { diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionServiceTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionServiceTest.java index 5965c49cc..aa45d93aa 100644 --- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionServiceTest.java +++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/PermissionServiceTest.java @@ -421,10 +421,11 @@ public class PermissionServiceTest extends AbstractServiceTestImpl { } // Optionally output additional data about list members for debugging. boolean iterateThroughList = true; - if (iterateThroughList && logger.isDebugEnabled()) { + if ((iterateThroughList || (EXPECTED_ITEMS != list.getPermissions().size())) + && logger.isDebugEnabled()) { printList(testName, list); } - Assert.assertEquals(EXPECTED_ITEMS, list.getPermissions().size()); + Assert.assertEquals(list.getPermissions().size(), EXPECTED_ITEMS); } diff --git a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RoleServiceTest.java b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RoleServiceTest.java index 29c30f900..fe636436c 100644 --- a/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RoleServiceTest.java +++ b/services/authorization-mgt/client/src/test/java/org/collectionspace/services/authorization/client/test/RoleServiceTest.java @@ -610,6 +610,92 @@ public class RoleServiceTest extends AbstractServiceTestImpl { roleToUpdate.getDescription(), "Data in updated object did not match submitted data."); } + + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, + dependsOnMethods = {"read", "readList", "readNonExistent"}) + public void verifyProtectionReadOnly(String testName) throws Exception { + + if (logger.isDebugEnabled()) { + logger.debug(testBanner(testName, CLASS_NAME)); + } + + setupCreate(); + + // Submit the request to the service and store the response. + RoleClient client = new RoleClient(); + Role role = createRoleInstance(knownRoleName+"_PT", "Just a temp", true); + role.setMetadataProtection(RoleClient.IMMUTABLE); + role.setPermsProtection(RoleClient.IMMUTABLE); + ClientResponse res = client.create(role); + int statusCode = res.getStatus(); + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + // Store the ID returned from this create operation + // for additional tests below. + String testResourceId = extractId(res); + allResourceIdsCreated.add(testResourceId); + if (logger.isDebugEnabled()) { + logger.debug(testName + ": testResourceId=" + testResourceId); + } + setupRead(); + + // Submit the request to the service and store the response. + ClientResponse roleRes = client.read(testResourceId); + statusCode = roleRes.getStatus(); + + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + Role roleRead = (Role) roleRes.getEntity(); + Assert.assertNotNull(roleRead); + String mdProtection = roleRead.getMetadataProtection(); + String permsProtection = roleRead.getPermsProtection(); + if (logger.isTraceEnabled()) { + logger.trace(testName + ": metadataProtection=" + mdProtection); + logger.trace(testName + ": permsProtection=" + permsProtection); + } + Assert.assertFalse(role.getMetadataProtection().equals(mdProtection), + "Role allowed create to set the metadata protection flag."); + Assert.assertFalse(role.getPermsProtection().equals(permsProtection), + "Role allowed create to set the perms protection flag."); + + setupUpdate(); + + Role roleToUpdate = createRoleInstance(knownRoleName+"_PT", "Just a temp", true); + roleToUpdate.setMetadataProtection(RoleClient.IMMUTABLE); + roleToUpdate.setPermsProtection(RoleClient.IMMUTABLE); + + // Submit the request to the service and store the response. + roleRes = client.update(testResourceId, roleToUpdate); + statusCode = roleRes.getStatus(); + // Check the status code of the response: does it match the expected response(s)? + if (logger.isDebugEnabled()) { + logger.debug(testName + ": status = " + statusCode); + } + Assert.assertTrue(REQUEST_TYPE.isValidStatusCode(statusCode), + invalidStatusCodeMessage(REQUEST_TYPE, statusCode)); + Assert.assertEquals(statusCode, EXPECTED_STATUS_CODE); + + + Role roleUpdated = (Role) roleRes.getEntity(); + Assert.assertNotNull(roleUpdated); + if (logger.isDebugEnabled()) { + logger.debug(testName + "Updated role: "); + logger.debug(objectAsXmlString(roleUpdated,Role.class)); + } + + Assert.assertFalse( + RoleClient.IMMUTABLE.equalsIgnoreCase(roleUpdated.getMetadataProtection()), + "Role allowed update of the metadata protection flag."); + Assert.assertFalse( + RoleClient.IMMUTABLE.equalsIgnoreCase(roleUpdated.getPermsProtection()), + "Role allowed update of the perms protection flag."); + } + + @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, dependsOnMethods = {"read", "readList", "readNonExistent"}) @@ -714,7 +800,7 @@ public class RoleServiceTest extends AbstractServiceTestImpl { */ @Override @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTestImpl.class, - dependsOnMethods = {"updateNotAllowed", "testSubmitRequest"}) + dependsOnMethods = {"updateNotAllowed", "testSubmitRequest", "verifyProtectionReadOnly"}) public void delete(String testName) throws Exception { if (logger.isDebugEnabled()) { diff --git a/services/authorization-mgt/import/pom.xml b/services/authorization-mgt/import/pom.xml index 5fe8e4035..a141df8ba 100644 --- a/services/authorization-mgt/import/pom.xml +++ b/services/authorization-mgt/import/pom.xml @@ -50,6 +50,11 @@ ${project.version} provided + + org.collectionspace.services + org.collectionspace.services.authorization-mgt.client + ${project.version} + org.testng testng diff --git a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java index aa974c52c..24802713b 100644 --- a/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java +++ b/services/authorization-mgt/import/src/main/java/org/collectionspace/services/authorization/importer/AuthorizationGen.java @@ -42,6 +42,7 @@ import org.collectionspace.services.authorization.PermissionRole; import org.collectionspace.services.authorization.PermissionValue; import org.collectionspace.services.authorization.PermissionsList; import org.collectionspace.services.authorization.PermissionsRolesList; +import org.collectionspace.services.client.RoleClient; import org.collectionspace.services.authorization.Role; import org.collectionspace.services.authorization.RoleValue; import org.collectionspace.services.authorization.RolesList; @@ -266,31 +267,26 @@ public class AuthorizationGen { } private Role buildTenantAdminRole(String tenantId) { - Role role = new Role(); - role.setCreatedAtItem(new Date()); - role.setDisplayName(ROLE_TENANT_ADMINISTRATOR); - role.setRoleName(ROLE_PREFIX + - tenantId + "_" + - role.getDisplayName()); - - String id = UUID.randomUUID().toString(); - role.setCsid(id); - role.setDescription("generated tenant admin role"); - role.setTenantId(tenantId); - return role; + return buildTenantRole(tenantId, ROLE_TENANT_ADMINISTRATOR, "admin"); } private Role buildTenantReaderRole(String tenantId) { + return buildTenantRole(tenantId, ROLE_TENANT_READER, "read only"); + } + + private Role buildTenantRole(String tenantId, String name, String type) { Role role = new Role(); role.setCreatedAtItem(new Date()); - role.setDisplayName(ROLE_TENANT_READER); + role.setDisplayName(name); role.setRoleName(ROLE_PREFIX + tenantId + "_" + role.getDisplayName()); String id = UUID.randomUUID().toString(); role.setCsid(id); - role.setDescription("generated tenant read only role"); + role.setDescription("generated tenant "+type+" role"); role.setTenantId(tenantId); + role.setMetadataProtection(RoleClient.IMMUTABLE); + role.setPermsProtection(RoleClient.IMMUTABLE); return role; } 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 fbfaac52a..7a8d04bc9 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 @@ -45,6 +45,7 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.QueryParam; +import javax.ws.rs.WebApplicationException; import javax.ws.rs.core.Context; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; @@ -104,7 +105,18 @@ public class RoleResource extends SecurityResourceBase { @PUT @Path("{csid}") public Role updateRole(@PathParam("csid") String csid, Role theUpdate) { - return (Role)update(csid, theUpdate, Role.class); + try { + Role role = (Role)get(csid, Role.class); + // If marked as metadata immutable, do not update + if(RoleClient.IMMUTABLE.equals(role.getMetadataProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Role: "+csid+" is immutable.").type("text/plain").build(); + throw new WebApplicationException(response); + } + return (Role)update(csid, theUpdate, Role.class); + } catch (Exception e) { + throw bigReThrow(e, ServiceMessages.UPDATE_FAILED, csid); + } } @DELETE @@ -114,6 +126,13 @@ public class RoleResource extends SecurityResourceBase { ensureCSID(csid, ServiceMessages.DELETE_FAILED + "deleteRole "); 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; + } //FIXME ideally the following three operations should be in the same tx CSPACE-658 //delete all relationships for this permission PermissionRoleSubResource permRoleResource = @@ -144,6 +163,13 @@ public class RoleResource extends SecurityResourceBase { logger.debug("createRolePermission with roleCsid=" + roleCsid); ensureCSID(roleCsid, ServiceMessages.PUT_FAILED + "permroles role "); try { + Role role = (Role)get(roleCsid, Role.class); + // If marked as metadata immutable, do not delete + if(RoleClient.IMMUTABLE.equals(role.getPermsProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Role: "+roleCsid+" is immutable.").type("text/plain").build(); + return response; + } PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.ROLE_PERMROLE_SERVICE); String permrolecsid = subResource.createPermissionRole(input, SubjectType.PERMISSION); @@ -199,6 +225,13 @@ public class RoleResource extends SecurityResourceBase { logger.debug("deleteRolePermission with roleCsid=" + roleCsid); ensureCSID(roleCsid, ServiceMessages.DELETE_FAILED + "permroles role "); try { + Role role = (Role)get(roleCsid, Role.class); + // If marked as metadata immutable, do not delete + if(RoleClient.IMMUTABLE.equals(role.getPermsProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Role: "+roleCsid+" is immutable.").type("text/plain").build(); + return response; + } PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.ROLE_PERMROLE_SERVICE); //delete all relationships for a permission @@ -216,6 +249,13 @@ public class RoleResource extends SecurityResourceBase { logger.debug("deleteRolePermission with roleCsid=" + roleCsid); ensureCSID(roleCsid, ServiceMessages.DELETE_FAILED + "permroles role "); try { + Role role = (Role)get(roleCsid, Role.class); + // If marked as metadata immutable, do not delete + if(RoleClient.IMMUTABLE.equals(role.getPermsProtection())) { + Response response = + Response.status(Response.Status.FORBIDDEN).entity("Role: "+roleCsid+" is immutable.").type("text/plain").build(); + return response; + } PermissionRoleSubResource subResource = new PermissionRoleSubResource(PermissionRoleSubResource.ROLE_PERMROLE_SERVICE); //delete all relationships for a permission diff --git a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionDocumentHandler.java b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionDocumentHandler.java index bf6977c14..8fda75587 100644 --- a/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionDocumentHandler.java +++ b/services/authorization-mgt/service/src/main/java/org/collectionspace/services/authorization/storage/PermissionDocumentHandler.java @@ -164,7 +164,7 @@ public class PermissionDocumentHandler @Override public void completeUpdate(DocumentWrapper wrapDoc) throws Exception { Permission upAcc = wrapDoc.getWrappedObject(); - getServiceContext().setOutput(permission); + getServiceContext().setOutput(upAcc); sanitize(upAcc); //FIXME update lower-layer authorization (acls) //will require deleting old permissions for this resource and adding 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 344eadb16..ccec2ad69 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 @@ -30,6 +30,7 @@ import java.util.UUID; import org.collectionspace.services.authorization.Role; import org.collectionspace.services.authorization.RolesList; +import org.collectionspace.services.client.RoleClient; import org.collectionspace.services.common.document.BadRequestException; import org.collectionspace.services.common.document.DocumentFilter; import org.collectionspace.services.common.document.DocumentWrapper; @@ -69,15 +70,21 @@ public class RoleDocumentHandler role.setRoleName(fixRoleName(role.getRoleName(), role.getTenantId())); role.setCsid(id); + // We do not allow creation of locked roles through the services. + role.setMetadataProtection(null); + role.setPermsProtection(null); } @Override public void handleUpdate(DocumentWrapper wrapDoc) throws Exception { Role roleFound = wrapDoc.getWrappedObject(); Role roleReceived = getCommonPart(); - roleReceived.setRoleName(fixRoleName(roleReceived.getRoleName(), - roleFound.getTenantId())); - merge(roleReceived, roleFound); + // If marked as metadata immutable, do not do update + if(!RoleClient.IMMUTABLE.equals(roleFound.getMetadataProtection())) { + roleReceived.setRoleName(fixRoleName(roleReceived.getRoleName(), + roleFound.getTenantId())); + merge(roleReceived, roleFound); + } } /** @@ -103,6 +110,7 @@ public class RoleDocumentHandler if (from.getDescription() != null) { to.setDescription(from.getDescription()); } + // Note that we do not allow update of locks if (logger.isDebugEnabled()) { logger.debug("merged role=" + JaxbUtils.toString(to, Role.class)); } @@ -112,7 +120,7 @@ public class RoleDocumentHandler @Override public void completeUpdate(DocumentWrapper wrapDoc) throws Exception { Role upAcc = wrapDoc.getWrappedObject(); - getServiceContext().setOutput(role); + getServiceContext().setOutput(upAcc); sanitize(upAcc); } diff --git a/services/authorization/jaxb/src/main/resources/roles.xsd b/services/authorization/jaxb/src/main/resources/roles.xsd index a1eab6d5c..bc5d34982 100644 --- a/services/authorization/jaxb/src/main/resources/roles.xsd +++ b/services/authorization/jaxb/src/main/resources/roles.xsd @@ -109,6 +109,24 @@ + + + + + + + + + + + + + + + + + + diff --git a/services/authorization/pstore/src/main/resources/db/mysql/authorization.sql b/services/authorization/pstore/src/main/resources/db/mysql/authorization.sql index e63e1b37b..a8d688f75 100644 --- a/services/authorization/pstore/src/main/resources/db/mysql/authorization.sql +++ b/services/authorization/pstore/src/main/resources/db/mysql/authorization.sql @@ -8,5 +8,5 @@ create table accounts_roles (HJID bigint not null auto_increment, account_id var create table permissions (csid varchar(128) not null, action_group varchar(128), attribute_name varchar(128), created_at datetime not null, description varchar(255), effect varchar(32) not null, resource_name varchar(128) not null, tenant_id varchar(128) not null, updated_at datetime, primary key (csid)); create table permissions_actions (HJID bigint not null auto_increment, name varchar(128) not null, objectIdentity varchar(128) not null, objectIdentityResource varchar(128) not null, ACTIONS_PERMISSION_CSID varchar(128), primary key (HJID)); create table permissions_roles (HJID bigint not null auto_increment, actionGroup varchar(255), created_at datetime not null, permission_id varchar(128) not null, permission_resource varchar(255), role_id varchar(128) not null, role_name varchar(255), primary key (HJID), unique (permission_id, role_id)); -create table roles (csid varchar(128) not null, created_at datetime not null, description varchar(255), displayname varchar(200) not null, rolegroup varchar(255), rolename varchar(200) not null, tenant_id varchar(128) not null, updated_at datetime, primary key (csid), unique (rolename, tenant_id), unique (displayname, tenant_id)); +create table roles (csid varchar(128) not null, created_at datetime not null, description varchar(255), displayname varchar(200) not null, rolegroup varchar(255), rolename varchar(200) not null, tenant_id varchar(128) not null, metadata_protection varchar(255), perms_protection varchar(255), updated_at datetime, primary key (csid), unique (rolename, tenant_id), unique (displayname, tenant_id)); alter table permissions_actions add index FK85F82042E2DC84FD (ACTIONS_PERMISSION_CSID), add constraint FK85F82042E2DC84FD foreign key (ACTIONS_PERMISSION_CSID) references permissions (csid); diff --git a/services/authorization/pstore/src/main/resources/db/postgresql/authorization.sql b/services/authorization/pstore/src/main/resources/db/postgresql/authorization.sql index cdde3c99c..ee386d3d3 100644 --- a/services/authorization/pstore/src/main/resources/db/postgresql/authorization.sql +++ b/services/authorization/pstore/src/main/resources/db/postgresql/authorization.sql @@ -9,6 +9,6 @@ create table accounts_roles (HJID int8 not null, account_id varchar(128) not nul create table permissions (csid varchar(128) not null, action_group varchar(128), attribute_name varchar(128), created_at timestamp not null, description varchar(255), effect varchar(32) not null, resource_name varchar(128) not null, tenant_id varchar(128) not null, updated_at timestamp, primary key (csid)); create table permissions_actions (HJID int8 not null, name varchar(128) not null, objectIdentity varchar(128) not null, objectIdentityResource varchar(128) not null, ACTIONS_PERMISSION_CSID varchar(128), primary key (HJID)); create table permissions_roles (HJID int8 not null, actionGroup varchar(255), created_at timestamp not null, permission_id varchar(128) not null, permission_resource varchar(255), role_id varchar(128) not null, role_name varchar(255), primary key (HJID), unique (permission_id, role_id)); -create table roles (csid varchar(128) not null, created_at timestamp not null, description varchar(255), displayname varchar(200) not null, rolegroup varchar(255), rolename varchar(200) not null, tenant_id varchar(128) not null, updated_at timestamp, primary key (csid), unique (rolename, tenant_id), unique (displayname, tenant_id)); +create table roles (csid varchar(128) not null, created_at timestamp not null, description varchar(255), displayname varchar(200) not null, rolegroup varchar(255), rolename varchar(200) not null, tenant_id varchar(128) not null, metadata_protection varchar(255), perms_protection varchar(255), updated_at timestamp, primary key (csid), unique (rolename, tenant_id), unique (displayname, tenant_id)); alter table permissions_actions add constraint FK85F82042E2DC84FD foreign key (ACTIONS_PERMISSION_CSID) references permissions; create sequence hibernate_sequence; diff --git a/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java b/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java index a4a5902c8..064bb8180 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java +++ b/services/common/src/main/java/org/collectionspace/services/common/ServiceMain.java @@ -265,8 +265,8 @@ public class ServiceMain { // then the accounts were as well String insertAccountSQL = "INSERT INTO accounts_common " - + "(csid, email, userid, status, screen_name, created_at) " - + "VALUES (?,?,?,'ACTIVE',?, now())"; + + "(csid, email, userid, status, screen_name, metadata_protection, roles_protection, created_at) " + + "VALUES (?,?,?,'ACTIVE',?, 'immutable', 'immutable', now())"; Hashtable tenantAdminAcctCSIDs = new Hashtable(); Hashtable tenantReaderAcctCSIDs = new Hashtable(); pstmt = conn.prepareStatement(insertAccountSQL); // create a statement