From: Sanjay Dalal Date: Fri, 27 Nov 2009 00:26:52 +0000 (+0000) Subject: CSPACE-307 CSPACE-579 CSPACE-581 X-Git-Url: https://git.aero2k.de/?a=commitdiff_plain;h=0227e57c662a0c46baaeda15e41360d2a2133349;p=tmp%2Fjakarta-migration.git CSPACE-307 CSPACE-579 CSPACE-581 integrated account management with default id provider management. create account would create a user in the default id provider and delete would delete if present. if password is provided on account create, a user would be created in the default id provider. added tenant id and userid in account. consolidated persistence.xml (jaxrsprovider/src/main/resources) so that DML could be performed atomically on all cs entities involved under the same transaction. added gen_ddl task to account(client) test: account service tests and all service tests --- diff --git a/services/account/service/src/main/resources/META-INF/persistence.xml b/services/JaxRsServiceProvider/src/main/resources/META-INF/persistence.xml similarity index 80% rename from services/account/service/src/main/resources/META-INF/persistence.xml rename to services/JaxRsServiceProvider/src/main/resources/META-INF/persistence.xml index 9abf6226f..c8b051378 100644 --- a/services/account/service/src/main/resources/META-INF/persistence.xml +++ b/services/JaxRsServiceProvider/src/main/resources/META-INF/persistence.xml @@ -1,12 +1,15 @@ - + org.hibernate.ejb.HibernatePersistence CspaceDS org.collectionspace.services.account.AccountsCommon org.collectionspace.services.account.AccountsCommonList org.collectionspace.services.account.AccountsCommonList$AccountListItem + org.collectionspace.services.authentication.User + org.collectionspace.services.authentication.Role + org.collectionspace.services.authentication.UserRole diff --git a/services/account/client/build.xml b/services/account/client/build.xml index 1a316e46e..380c36fa9 100644 --- a/services/account/client/build.xml +++ b/services/account/client/build.xml @@ -107,6 +107,31 @@ + + + + + + + + + + + + + + + + + + + + + + + + res = client.create(account); int statusCode = res.getStatus(); @@ -100,18 +100,13 @@ public class AccountServiceTest extends AbstractServiceTest { } } - /* (non-Javadoc) - * @see org.collectionspace.services.client.test.ServiceTest#createList() - */ + //to not cause uniqueness violation for account, createList is removed @Override @Test(dataProvider = "testName", dataProviderClass = AbstractServiceTest.class, dependsOnMethods = {"create"}) public void createList(String testName) throws Exception { - for (int i = 0; i < 3; i++) { - create(testName); - } } - + // Failure outcomes // Placeholders until the three tests below can be uncommented. // See Issue CSPACE-401. @@ -413,7 +408,7 @@ public class AccountServiceTest extends AbstractServiceTest { account.setFirstName(firstName); account.setLastName(lastName); account.setScreenName(screenName); - account.setUserName(screenName); + account.setUserId(screenName); byte[] b64passwd = Base64.encodeBase64(passwd.getBytes()); account.setPassword(b64passwd); account.setEmail(email); diff --git a/services/account/client/src/test/java/org/collectionspace/services/client/test/AccountTest.java b/services/account/client/src/test/java/org/collectionspace/services/client/test/AccountTest.java index 2e3b06cc5..2a4f04e53 100644 --- a/services/account/client/src/test/java/org/collectionspace/services/client/test/AccountTest.java +++ b/services/account/client/src/test/java/org/collectionspace/services/client/test/AccountTest.java @@ -60,8 +60,10 @@ public class AccountTest { account.setFirstName("John"); account.setLastName("Doe"); account.setEmail("john.doe@berkeley.edu"); + account.setUserId("johndoe"); id = UUID.randomUUID().toString(); account.setCsid(id); + account.setTenantid("123"); //set by service runtime em.getTransaction().begin(); em.persist(account); // Commit the transaction diff --git a/services/account/client/src/test/resources/log4j.properties b/services/account/client/src/test/resources/log4j.properties index 18c510350..fa374e924 100644 --- a/services/account/client/src/test/resources/log4j.properties +++ b/services/account/client/src/test/resources/log4j.properties @@ -21,3 +21,4 @@ log4j.logger.org.collectionspace=DEBUG log4j.logger.org.apache=INFO log4j.logger.httpclient=INFO log4j.logger.org.jboss.resteasy=INFO +log4j.logger.org.hibernate=INFO diff --git a/services/account/jaxb/src/main/resources/accounts_common.xsd b/services/account/jaxb/src/main/resources/accounts_common.xsd index 4c8914f7c..9384ea9fa 100644 --- a/services/account/jaxb/src/main/resources/accounts_common.xsd +++ b/services/account/jaxb/src/main/resources/accounts_common.xsd @@ -103,15 +103,17 @@ - - + + - + + + - + @@ -119,6 +121,19 @@ + + + + + + + + + + + + + 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 cfbe19371..8131f10ff 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 @@ -23,14 +23,185 @@ */ package org.collectionspace.services.account.storage; +import javax.persistence.EntityManager; +import javax.persistence.EntityManagerFactory; +import javax.persistence.Query; +import org.apache.commons.codec.binary.Base64; +import org.collectionspace.services.account.AccountsCommon; +import org.collectionspace.services.account.AccountsCommon; +import org.collectionspace.services.authentication.User; +import org.collectionspace.services.common.context.ServiceContext; +import org.collectionspace.services.common.document.BadRequestException; +import org.collectionspace.services.common.document.DocumentException; +import org.collectionspace.services.common.document.DocumentHandler; +import org.collectionspace.services.common.document.DocumentHandler.Action; +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.security.SecurityUtils; import org.collectionspace.services.common.storage.jpa.JpaStorageClient; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * AccountStorageClient deals with both Account and Default Identity Provider's * state in persistent storage * @author */ -public class AccountStorageClient extends JpaStorageClient -{ +public class AccountStorageClient extends JpaStorageClient { + private final Logger logger = LoggerFactory.getLogger(AccountStorageClient.class); + + public AccountStorageClient() { + } + + @Override + public String create(ServiceContext ctx, + DocumentHandler handler) throws BadRequestException, + DocumentException { + + String docType = ctx.getDocumentType(); + if (docType == null) { + throw new DocumentNotFoundException( + "Unable to find DocumentType for service " + ctx.getServiceName()); + } + if (handler == null) { + throw new IllegalArgumentException( + "AccountStorageClient.create: handler is missing"); + } + EntityManagerFactory emf = null; + EntityManager em = null; + try { + handler.prepare(Action.CREATE); + AccountsCommon account = (AccountsCommon) handler.getCommonPart(); + DocumentWrapper wrapDoc = + new DocumentWrapperImpl(account); + handler.handle(Action.CREATE, wrapDoc); + emf = getEntityManagerFactory(); + em = emf.createEntityManager(); + em.getTransaction().begin(); + //if userid and password are given, add to default id provider + if (account.getUserId() != null && account.getPassword() != null) { + User user = createUser(account); + em.persist(user); + } + account.setTenantid(ctx.getTenantId()); + em.persist(account); + em.getTransaction().commit(); + handler.complete(Action.CREATE, wrapDoc); + return (String) getValue(account, "getCsid"); + } catch (Exception e) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + if (logger.isDebugEnabled()) { + logger.debug("Caught exception ", e); + } + throw new DocumentException(e); + } finally { + if (em != null) { + releaseEntityManagerFactory(emf); + } + } + } + + @Override + public void delete(ServiceContext ctx, String id) + throws DocumentNotFoundException, + DocumentException { + + if (logger.isDebugEnabled()) { + logger.debug("deleting entity with id=" + id); + } + String docType = ctx.getDocumentType(); + if (docType == null) { + throw new DocumentNotFoundException( + "Unable to find DocumentType for service " + ctx.getServiceName()); + } + EntityManagerFactory emf = null; + EntityManager em = null; + try { + emf = getEntityManagerFactory(); + em = emf.createEntityManager(); + //TODO investigate if deep delete is possible + //query an delete is inefficient + AccountsCommon accountFound = em.find(AccountsCommon.class, id); + if (accountFound == null) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + String msg = "could not find entity with id=" + id; + logger.error(msg); + throw new DocumentNotFoundException(msg); + } + + StringBuilder accDelStr = new StringBuilder("DELETE FROM "); + accDelStr.append(getEntityName(ctx)); + accDelStr.append(" WHERE csid = :csid"); + //TODO: add tenant id + Query accDel = em.createQuery(accDelStr.toString()); + accDel.setParameter("csid", id); + //TODO: add tenant id + + //if userid gives any indication about the id provider, it should + //be used to avoid the following approach + User userLocal = em.find(User.class, accountFound.getUserId()); + Query usrDel = null; + if (userLocal != null) { + StringBuilder usrDelStr = new StringBuilder("DELETE FROM "); + usrDelStr.append(User.class.getCanonicalName()); + usrDelStr.append(" WHERE username = :username"); + //TODO: add tenant id + usrDel = em.createQuery(usrDelStr.toString()); + usrDel.setParameter("username", accountFound.getUserId()); + } + em.getTransaction().begin(); + int accDelCount = accDel.executeUpdate(); + if (accDelCount != 1) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + } + if (userLocal != null) { + int usrDelCount = usrDel.executeUpdate(); + if (usrDelCount != 1) { + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + } + if (usrDelCount != 1) { + String msg = "could not find user with username=" + accountFound.getUserId(); + logger.error(msg); + throw new DocumentNotFoundException(msg); + } + } + em.getTransaction().commit(); + + } catch (DocumentException de) { + throw de; + } catch (Exception e) { + if (logger.isDebugEnabled()) { + logger.debug("Caught exception ", e); + } + if (em != null && em.getTransaction().isActive()) { + em.getTransaction().rollback(); + } + throw new DocumentException(e); + } finally { + if (emf != null) { + releaseEntityManagerFactory(emf); + } + } + } + + private User createUser(AccountsCommon account) { + User user = new User(); + user.setUsername(account.getUserId()); + byte[] bpass = Base64.decodeBase64(account.getPassword()); + SecurityUtils.validatePassword(new String(bpass)); + String secEncPasswd = SecurityUtils.createPasswordHash( + account.getUserId(), new String(bpass)); + user.setPasswd(secEncPasswd); + return user; + } } diff --git a/services/authentication/jaxb/src/main/resources/META-INF/persistence.xml b/services/authentication/jaxb/src/main/resources/META-INF/persistence.xml deleted file mode 100644 index 6a124e8db..000000000 --- a/services/authentication/jaxb/src/main/resources/META-INF/persistence.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - java:comp/env/jdbc/CspaceDS - org.collectionspace.services.authentication.User - org.collectionspace.services.authentication.Role - org.collectionspace.services.authentication.UserRole - - - - - - - diff --git a/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd b/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd index 899c0675d..a49a1e755 100644 --- a/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd +++ b/services/authentication/jaxb/src/main/resources/authentication_identity_provider.xsd @@ -1,9 +1,9 @@ + + jboss + jbosssx + 4.2.3.GA + provided + jboss jboss-remoting diff --git a/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java new file mode 100644 index 000000000..c7e8d46db --- /dev/null +++ b/services/common/src/main/java/org/collectionspace/services/common/security/SecurityUtils.java @@ -0,0 +1,69 @@ +/** + * This document is a part of the source code and related artifacts + * for CollectionSpace, an open source collections management system + * for museums and related institutions: + * + * http://www.collectionspace.org + * http://wiki.collectionspace.org + * + * Copyright © 2009 Regents of the University of California + * + * Licensed under the Educational Community License (ECL), Version 2.0. + * You may not use this file except in compliance with this License. + * + * You may obtain a copy of the ECL 2.0 License at + * https://source.collectionspace.org/collection-space/LICENSE.txt + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.collectionspace.services.common.security; + +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +/** + * + * @author + */ +public class SecurityUtils { + + private static final Logger logger = LoggerFactory.getLogger(SecurityUtils.class); + + /** + * createPasswordHash creates password has using configured digest algorithm + * and encoding + * @param user + * @param password in cleartext + * @return hashed password + */ + public static String createPasswordHash(String username, String password) { + //TODO: externalize digest algo and encoding + return org.jboss.security.Util.createPasswordHash("SHA-256", //digest algo + "base64", //encoding + null, //charset + username, + password); + } + + /** + * validatePassword validates password per configured password policy + * @param password + */ + public static void validatePassword(String password) { + //TODO: externalize password length + if (password == null) { + String msg = "Password missing "; + logger.error(msg); + throw new IllegalArgumentException(msg); + } + if (password.length() < 8 || password.length() > 24) { + String msg = "Password length should be >8 and <24"; + logger.error(msg); + throw new IllegalArgumentException(msg); + } + } +} diff --git a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClient.java b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClient.java index 4a8ef9f51..3bf24e5f4 100644 --- a/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClient.java +++ b/services/common/src/main/java/org/collectionspace/services/common/storage/jpa/JpaStorageClient.java @@ -17,6 +17,7 @@ */ package org.collectionspace.services.common.storage.jpa; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.List; import javax.persistence.EntityManager; @@ -48,7 +49,7 @@ import org.slf4j.LoggerFactory; public class JpaStorageClient implements StorageClient { private final Logger logger = LoggerFactory.getLogger(JpaStorageClient.class); - + protected final static String CS_PERSISTENCE_UNIT = "org.collectionspace.services"; public JpaStorageClient() { } @@ -73,13 +74,15 @@ public class JpaStorageClient implements StorageClient { Object entity = handler.getCommonPart(); DocumentWrapper wrapDoc = new DocumentWrapperImpl(entity); handler.handle(Action.CREATE, wrapDoc); - emf = getEntityManagerFactory(docType); + emf = getEntityManagerFactory(); em = emf.createEntityManager(); em.getTransaction().begin(); em.persist(entity); em.getTransaction().commit(); handler.complete(Action.CREATE, wrapDoc); - return getCsid(entity); + return (String) getValue(entity, "getCsid"); + } catch (DocumentException de) { + throw de; } catch (Exception e) { if (em != null && em.getTransaction().isActive()) { em.getTransaction().rollback(); @@ -126,7 +129,7 @@ public class JpaStorageClient implements StorageClient { if ((null != where) && (where.length() > 0)) { queryStr.append(" AND " + where); } - emf = getEntityManagerFactory(docType); + emf = getEntityManagerFactory(); em = emf.createEntityManager(); Query q = em.createQuery(queryStr.toString()); q.setParameter("csid", id); @@ -154,8 +157,6 @@ public class JpaStorageClient implements StorageClient { DocumentWrapper wrapDoc = new DocumentWrapperImpl(o); handler.handle(Action.GET, wrapDoc); handler.complete(Action.GET, wrapDoc); - } catch (IllegalArgumentException iae) { - throw iae; } catch (DocumentException de) { throw de; } catch (Exception e) { @@ -176,14 +177,7 @@ public class JpaStorageClient implements StorageClient { throw new UnsupportedOperationException("use getFiltered instead"); } - /** - * getFiltered get all documents for an entity service from the Document repository, - * given filter parameters specified by the handler. - * @param ctx service context under which this method is invoked - * @param handler should be used by the caller to provide and transform the document - * @throws DocumentNotFoundException if workspace not found - * @throws DocumentException - */ + @Override public void getFiltered(ServiceContext ctx, DocumentHandler handler) throws DocumentNotFoundException, DocumentException { if (handler == null) { @@ -214,7 +208,7 @@ public class JpaStorageClient implements StorageClient { if ((null != where) && (where.length() > 0)) { queryStr.append(" AND " + where); } - emf = getEntityManagerFactory(docType); + emf = getEntityManagerFactory(); em = emf.createEntityManager(); Query q = em.createQuery(queryStr.toString()); //TODO: add tenant id @@ -264,11 +258,11 @@ public class JpaStorageClient implements StorageClient { setCsid(entity, id); DocumentWrapper wrapDoc = new DocumentWrapperImpl(entity); handler.handle(Action.UPDATE, wrapDoc); - emf = getEntityManagerFactory(docType); + emf = getEntityManagerFactory(); em = emf.createEntityManager(); em.getTransaction().begin(); Object entityFound = em.find(entity.getClass(), id); - if(entityFound == null) { + if (entityFound == null) { if (em != null && em.getTransaction().isActive()) { em.getTransaction().rollback(); } @@ -293,13 +287,6 @@ public class JpaStorageClient implements StorageClient { } } - /** - * delete a document from the Nuxeo repository - * @param ctx service context under which this method is invoked - * @param id - * of the document - * @throws DocumentException - */ @Override public void delete(ServiceContext ctx, String id) throws DocumentNotFoundException, @@ -321,7 +308,7 @@ public class JpaStorageClient implements StorageClient { deleteStr.append(" WHERE csid = :csid"); //TODO: add tenant id - emf = getEntityManagerFactory(docType); + emf = getEntityManagerFactory(); em = emf.createEntityManager(); Query q = em.createQuery(deleteStr.toString()); q.setParameter("csid", id); @@ -356,40 +343,87 @@ public class JpaStorageClient implements StorageClient { } } - private EntityManagerFactory getEntityManagerFactory( + protected EntityManagerFactory getEntityManagerFactory() { + return getEntityManagerFactory(CS_PERSISTENCE_UNIT); + } + protected EntityManagerFactory getEntityManagerFactory( String persistenceUnit) { return Persistence.createEntityManagerFactory(persistenceUnit); } - private void releaseEntityManagerFactory(EntityManagerFactory emf) { + protected void releaseEntityManagerFactory(EntityManagerFactory emf) { if (emf != null) { emf.close(); } } - private String getCsid(Object o) throws Exception { - Class c = o.getClass(); - Method m = c.getMethod("getCsid"); - if (m == null) { - String msg = "Could not find csid in entity of class=" + o.getClass().getCanonicalName(); + /** + * getValue gets invokes specified accessor method on given object. Assumption + * is that this is used for JavaBean pattern getXXX methods only. + * @param o object to return value from + * @param methodName of method to invoke + * @return value returned of invocation + * @throws NoSuchMethodException + * @throws IllegalAccessException + * @throws InvocationTargetException + */ + protected Object getValue(Object o, String methodName) + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { + if (methodName == null) { + String msg = methodName + " cannot be null"; logger.error(msg); throw new IllegalArgumentException(msg); } + Class c = o.getClass(); + Method m = c.getMethod(methodName); Object r = m.invoke(o); if (logger.isDebugEnabled()) { - logger.debug("getCsid returned csid=" + r + + logger.debug("getValue returned value=" + r + " for " + c.getName()); } + return r; + } - return (String) r; + /** + * setValue mutates the given object by invoking specified method. Assumption + * is that this is used for JavaBean pattern setXXX methods only. + * @param o object to mutate + * @param methodName indicates method to invoke + * @param argType type of the only argument (assumed) to method + * @param argValue value of the only argument (assumed) to method + * @return + * @throws NoSuchMethodException + * @throws IllegalAccessException + * @throws InvocationTargetException + */ + protected Object setValue(Object o, String methodName, Class argType, Object argValue) + throws NoSuchMethodException, IllegalAccessException, InvocationTargetException { + if (methodName == null) { + String msg = methodName + " cannot be null"; + logger.error(msg); + throw new IllegalArgumentException(msg); + } + if (argType == null) { + String msg = "argType cannot be null"; + logger.error(msg); + throw new IllegalArgumentException(msg); + } + Class c = o.getClass(); + Method m = c.getMethod(methodName, argType); + Object r = m.invoke(o, argValue); + if (logger.isDebugEnabled()) { + logger.debug("completed invocation of " + methodName + + " for " + c.getName()); + } + return r; } - private void setCsid(Object o, String csid) throws Exception { - //verify csid - String id = getCsid(o); + protected void setCsid(Object o, String csid) throws Exception { + //verify csid + String id = (String) getValue(o, "getCsid"); if (id != null) { if (!id.equals(csid)) { String msg = "Csids do not match!"; @@ -402,23 +436,10 @@ public class JpaStorageClient implements StorageClient { } //set csid - Class c = o.getClass(); - Method m = c.getMethod("setCsid", java.lang.String.class); - if (m == null) { - String msg = "Could not find csid in entity of class=" + o.getClass().getCanonicalName(); - logger.error(msg); - throw new IllegalArgumentException(msg); - } - - Object r = m.invoke(o, csid); - if (logger.isDebugEnabled()) { - logger.debug("completed setCsid " + - " for " + c.getName()); - } - + setValue(o, "setCsid", java.lang.String.class, csid); } - private String getEntityName(ServiceContext ctx) { + protected String getEntityName(ServiceContext ctx) { Object o = ctx.getProperty("entity-name"); if (o == null) { throw new IllegalArgumentException("property entity-name missing in context " +