#54 Replace SessionIdGenerator with RandomStringUtils.randomGraph(15)

This is only used for ticket keys and random passwords, so while Commons ID impl attempted to avoid clashes by using some time elements, we have no concern here unless someone finds an issue as these features are also unused by ECCO and part of bits that could be removed (e.g. authentication).
eccosolutions · Dec 28, 2023 · 03acd89 · 03acd89
1 parent cc32b57
commit 03acd89
Show file tree

Hide file tree

Showing 6 changed files with 50 additions and 98 deletions.
diff --git a/core/src/main/java/org/osaf/cosmo/dao/hibernate/ItemDaoImpl.java b/core/src/main/java/org/osaf/cosmo/dao/hibernate/ItemDaoImpl.java
@@ -21,7 +21,6 @@
 import java.util.UUID;
 import javax.validation.ConstraintViolation;
 import javax.validation.ConstraintViolationException;
-import org.apache.commons.id.IdentifierGenerator;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.hibernate.FlushMode;
@@ -63,7 +62,6 @@ public abstract class ItemDaoImpl extends HibernateSessionSupport implements Ite
 
     private static final Log log = LogFactory.getLog(ItemDaoImpl.class);
 
-    private IdentifierGenerator ticketKeyGenerator = null;
     private ItemPathTranslator itemPathTranslator = null;
     private ItemFilterProcessor itemFilterProcessor = null;
 
@@ -424,19 +422,6 @@ public String generateUid() {
         return UUID.randomUUID().toString();
     }
 
-    /**
-     * Set the unique key generator for new tickets
-     *
-     * @param ticketKeyGenerator
-     */
-    public void setTicketKeyGenerator(IdentifierGenerator ticketKeyGenerator) {
-        this.ticketKeyGenerator = ticketKeyGenerator;
-    }
-
-    public IdentifierGenerator getTicketKeyGenerator() {
-        return ticketKeyGenerator;
-    }
-
     public ItemPathTranslator getItemPathTranslator() {
         return itemPathTranslator;
     }
@@ -466,10 +451,6 @@ public void setItemFilterProcessor(ItemFilterProcessor itemFilterProcessor) {
      * @see org.osaf.cosmo.dao.Dao#init()
      */
     public void init() {
-        if (ticketKeyGenerator == null) {
-            throw new IllegalStateException("ticketKeyGenerator is required");
-        }
-
         if (itemPathTranslator == null) {
             throw new IllegalStateException("itemPathTranslator is required");
         }

diff --git a/core/src/main/java/org/osaf/cosmo/service/impl/StandardUserService.java b/core/src/main/java/org/osaf/cosmo/service/impl/StandardUserService.java
@@ -17,9 +17,8 @@
 
 import java.security.MessageDigest;
 import java.util.Set;
-
 import org.apache.commons.codec.binary.Hex;
-import org.apache.commons.id.StringIdentifierGenerator;
+import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.osaf.cosmo.dao.ContentDao;
@@ -52,7 +51,6 @@ public class StandardUserService extends BaseService implements UserService {
 
     private MessageDigest digest;
     private String digestAlgorithm;
-    private StringIdentifierGenerator passwordGenerator;
     private ContentDao contentDao;
     private UserDao userDao;
 
@@ -302,7 +300,7 @@ public void removeUsersByName(Set<String> usernames) throws OverlordDeletionExce
      * presentation as an authentication credential.
      */
     public String generatePassword() {
-        String password = passwordGenerator.nextStringIdentifier();
+        String password = RandomStringUtils.randomGraph(15);
         return password.length() <= User.PASSWORD_LEN_MAX ?
             password :
             password.substring(0, User.PASSWORD_LEN_MAX - 1);
@@ -321,9 +319,6 @@ public void init() {
         if (userDao == null) {
             throw new IllegalStateException("userDao is required");
         }
-        if (passwordGenerator == null) {
-            throw new IllegalStateException("passwordGenerator is required");
-        }
         if (digestAlgorithm == null) {
             digestAlgorithm = DEFAULT_DIGEST_ALGORITHM;
         }
@@ -375,18 +370,6 @@ public void setDigestAlgorithm(String digestAlgorithm) {
         this.digestAlgorithm = digestAlgorithm;
     }
 
-    /**
-     */
-    public StringIdentifierGenerator getPasswordGenerator() {
-        return this.passwordGenerator;
-    }
-
-    /**
-     */
-    public void setPasswordGenerator(StringIdentifierGenerator generator) {
-        this.passwordGenerator = generator;
-    }
-
     /**
      */
     public ContentDao getContentDao() {

diff --git a/core/src/main/resources/applicationContext.xml b/core/src/main/resources/applicationContext.xml
@@ -208,9 +208,6 @@
         class="org.osaf.cosmo.dao.hibernate.DefaultItemPathTranslator">
   </bean>
 
-  <bean id="ticketKeyGenerator"
-        class="org.apache.commons.id.random.SessionIdGenerator" />
-
   <bean id="serverPropertyDao"
         class="org.osaf.cosmo.dao.hibernate.ServerPropertyDaoImpl"
         init-method="init"
@@ -231,7 +228,6 @@
         init-method="init"
         destroy-method="destroy">
     <property name="itemPathTranslator" ref="itemPathTranslator"/>
-    <property name="ticketKeyGenerator" ref="ticketKeyGenerator"/>
     <property name="itemFilterProcessor" ref="standardItemFilterProcessor"/>
   </bean>
 
@@ -248,9 +244,6 @@
   </bean>
 
   <!-- services -->
-  <bean id="passwordGenerator"
-        class="org.apache.commons.id.random.SessionIdGenerator"/>
-
   <bean id="ootbHelper"
         class="org.osaf.cosmo.service.account.OutOfTheBoxHelper">
     <property name="contentDao" ref="contentDao"/>
@@ -265,7 +258,6 @@
         destroy-method="destroy">
     <property name="contentDao" ref="contentDao"/>
     <property name="userDao" ref="userDao"/>
-    <property name="passwordGenerator" ref="passwordGenerator"/>
   </bean>
 
   <bean id="contentLockManager"

diff --git a/core/src/test/java/org/osaf/cosmo/MockHelper.java b/core/src/test/java/org/osaf/cosmo/MockHelper.java
@@ -15,7 +15,6 @@
  */
 package org.osaf.cosmo;
 
-import org.apache.commons.id.random.SessionIdGenerator;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.osaf.cosmo.calendar.query.CalendarQueryProcessor;
@@ -25,7 +24,13 @@
 import org.osaf.cosmo.dao.mock.MockDaoStorage;
 import org.osaf.cosmo.dao.mock.MockUserDao;
 import org.osaf.cosmo.icalendar.ICalendarClientFilterManager;
-import org.osaf.cosmo.model.*;
+import org.osaf.cosmo.model.CollectionItem;
+import org.osaf.cosmo.model.CollectionSubscription;
+import org.osaf.cosmo.model.ContentItem;
+import org.osaf.cosmo.model.EntityFactory;
+import org.osaf.cosmo.model.HomeCollectionItem;
+import org.osaf.cosmo.model.NoteItem;
+import org.osaf.cosmo.model.User;
 import org.osaf.cosmo.model.mock.MockEntityFactory;
 import org.osaf.cosmo.security.CosmoSecurityManager;
 import org.osaf.cosmo.security.mock.MockSecurityManager;
@@ -90,7 +95,6 @@ public MockHelper() {
         userService = new StandardUserService();
         userService.setContentDao(contentDao);
         userService.setUserDao(userDao);
-        userService.setPasswordGenerator(new SessionIdGenerator());
         userService.init();
 
         user = userService.getUser("test");

diff --git a/core/src/test/java/org/osaf/cosmo/dao/mock/MockDaoStorage.java b/core/src/test/java/org/osaf/cosmo/dao/mock/MockDaoStorage.java
@@ -15,16 +15,26 @@
  */
 package org.osaf.cosmo.dao.mock;
 
-import org.apache.commons.id.random.SessionIdGenerator;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.LinkedList;
+import java.util.Set;
+import org.apache.commons.lang3.RandomStringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
-import org.osaf.cosmo.model.*;
+import org.osaf.cosmo.model.CollectionItem;
+import org.osaf.cosmo.model.DuplicateItemNameException;
+import org.osaf.cosmo.model.HomeCollectionItem;
+import org.osaf.cosmo.model.Item;
+import org.osaf.cosmo.model.NoteItem;
+import org.osaf.cosmo.model.User;
 import org.osaf.cosmo.model.mock.MockCollectionItem;
 import org.osaf.cosmo.model.mock.MockHomeCollectionItem;
 import org.osaf.cosmo.model.mock.MockItem;
 
-import java.util.*;
-
 /**
  * Simple in-memory storage system for mock data access objects.
  */
@@ -36,15 +46,13 @@ public class MockDaoStorage {
     private HashMap<String, Item> itemsByUid;
     private HashMap<String, String> rootUidsByUsername;
     private HashMap<String, Set> tickets;
-    private SessionIdGenerator idGenerator;
 
     /** */
     public MockDaoStorage() {
         itemsByPath = new HashMap<>();
         itemsByUid = new HashMap<>();
         rootUidsByUsername = new HashMap<>();
         tickets = new HashMap<>();
-        idGenerator = new SessionIdGenerator();
     }
 
     /** */
@@ -227,11 +235,11 @@ public String getItemPath(String uid) {
     }
 
     public String calculateUid() {
-        return idGenerator.nextStringIdentifier();
+        return RandomStringUtils.randomGraph(15);
     }
 
     private String calculateTicketKey() {
-        return idGenerator.nextStringIdentifier();
+        return RandomStringUtils.randomGraph(15);
     }
 
     private MockItem getMockItem(Item item) {

diff --git a/core/src/test/java/org/osaf/cosmo/service/impl/StandardUserServiceTest.java b/core/src/test/java/org/osaf/cosmo/service/impl/StandardUserServiceTest.java
@@ -16,10 +16,7 @@
 package org.osaf.cosmo.service.impl;
 
 import java.util.Set;
-
 import junit.framework.TestCase;
-
-import org.apache.commons.id.random.SessionIdGenerator;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.osaf.cosmo.TestHelper;
@@ -54,7 +51,6 @@ protected void setUp() throws Exception {
         service = new StandardUserService();
         service.setContentDao(contentDao);
         service.setUserDao(userDao);
-        service.setPasswordGenerator(new SessionIdGenerator());
         service.init();
     }
 
@@ -117,7 +113,7 @@ public void testUpdateUser() throws Exception {
         User u1 = testHelper.makeDummyUser();
         u1.setPassword(service.digestPassword(u1.getPassword()));
         String digestedPassword = u1.getPassword();
-        
+
         userDao.createUser(u1);
 
         // change password
@@ -192,18 +188,6 @@ public void testNullUserDao() throws Exception {
         }
     }
 
-    /**
-     */
-    public void testNullPasswordGenerator() throws Exception {
-        service.setPasswordGenerator(null);
-        try {
-            service.init();
-            fail("Should not be able to initialize service without passwordGenerator");
-        } catch (IllegalStateException e) {
-            // expected
-        }
-    }
-
     /**
      */
     public void testDefaultDigestAlgorithm() throws Exception {
@@ -223,60 +207,60 @@ public void testDigestPassword() throws Exception {
         // tests hex
         assertTrue("Digest not hex encoded", digested.matches("^[0-9a-f]+$"));
     }
-    
+
     public void testCreatePasswordRecovery(){
         User user = testHelper.makeDummyUser();
         user = userDao.createUser(user);
-        
-        PasswordRecovery passwordRecovery = 
+
+        PasswordRecovery passwordRecovery =
             new HibPasswordRecovery(user, "pwrecovery1");
-        
+
         passwordRecovery = service.createPasswordRecovery(passwordRecovery);
 
-        PasswordRecovery storedPasswordRecovery = 
+        PasswordRecovery storedPasswordRecovery =
             service.getPasswordRecovery(passwordRecovery.getKey());
 
         assertEquals(passwordRecovery, storedPasswordRecovery);
-        
+
         service.deletePasswordRecovery(storedPasswordRecovery);
-        
-        storedPasswordRecovery = 
+
+        storedPasswordRecovery =
             service.getPasswordRecovery(storedPasswordRecovery.getKey());
-        
+
         assertNull(storedPasswordRecovery);
     }
-    
+
     public void testRecoverPassword(){
         User user = testHelper.makeDummyUser();
-        
+
         userDao.createUser(user);
 
         PasswordRecovery passwordRecovery = new HibPasswordRecovery(user, "pwrecovery2");
-        
+
         passwordRecovery = service.createPasswordRecovery(passwordRecovery);
-        
+
         assertEquals(user, passwordRecovery.getUser());
-        
+
         // Recover password
-        
-        PasswordRecovery storedPasswordRecovery = 
+
+        PasswordRecovery storedPasswordRecovery =
             service.getPasswordRecovery(passwordRecovery.getKey());
-        
+
         User changingUser = storedPasswordRecovery.getUser();
-        
+
         String newPassword = service.generatePassword();
 
         changingUser.setPassword(newPassword);
-        
+
         changingUser = service.updateUser(changingUser);
-        
+
         String changedPassword = changingUser.getPassword();
-        
+
         User changedUser = service.getUser(changingUser.getUsername());
-        
+
         assertEquals(changedUser, changingUser);
-        
+
         assertEquals(changedPassword, changedUser.getPassword());
-       
+
     }
 }