diff --git a/modules/eeuser/src/main/java/org/jpos/ee/User.java b/modules/eeuser/src/main/java/org/jpos/ee/User.java index df34269b17..91f8b15f37 100644 --- a/modules/eeuser/src/main/java/org/jpos/ee/User.java +++ b/modules/eeuser/src/main/java/org/jpos/ee/User.java @@ -28,7 +28,7 @@ public class User extends Cloneable implements Serializable, SoftDelete { private Long id; private String nick; - private String password; + private String passwordHash; private String name; private String email; private Set roles; @@ -73,11 +73,11 @@ public Long getId() { public void setId (Long id) { this.id = id; } - public void setPassword (String password) { - this.password = password; + public void setPasswordHash(String passwordHash) { + this.passwordHash = passwordHash; } - public String getPassword () { - return password; + public String getPasswordHash() { + return passwordHash; } public void setDeleted (boolean deleted) { this.deleted = deleted; diff --git a/modules/eeuser/src/main/java/org/jpos/ee/UserManager.java b/modules/eeuser/src/main/java/org/jpos/ee/UserManager.java index 08f9337ad1..24bf9328e1 100644 --- a/modules/eeuser/src/main/java/org/jpos/ee/UserManager.java +++ b/modules/eeuser/src/main/java/org/jpos/ee/UserManager.java @@ -18,7 +18,6 @@ package org.jpos.ee; -import java.nio.charset.StandardCharsets; import java.security.MessageDigest; import java.security.NoSuchAlgorithmException; import java.security.SecureRandom; @@ -28,7 +27,6 @@ import org.bouncycastle.util.encoders.Base64; import org.hibernate.Criteria; import org.hibernate.HibernateException; -import org.hibernate.ObjectNotFoundException; import org.hibernate.criterion.Restrictions; import org.jpos.iso.ISOUtil; import org.jpos.security.SystemSeed; @@ -42,10 +40,9 @@ public class UserManager { DB db; VERSION version; + public UserManager (DB db) { - super (); - this.db = db; - version = VERSION.ZERO; + this (db, VERSION.ONE); } public UserManager (DB db, VERSION version) { @@ -54,6 +51,32 @@ public UserManager (DB db, VERSION version) { this.version = version; } + public void setPassword (User u, String clearpass) throws BLException { + setPassword(u, clearpass, null, version); + } + + public void setPassword (User u, String clearpass, User author) throws BLException { + setPassword(u, clearpass, author, version); + } + + public void setPassword (User u, String clearpass, User author, VERSION v) throws BLException { + if (u.getPasswordHash() != null) + u.addPasswordHistoryValue(u.getPasswordHash()); + switch (v) { + case ZERO: + setV0Password (u, clearpass); + break; + case ONE: + setV1Password (u, clearpass); + break; + } + RevisionManager revmgr = new RevisionManager(db); + if (author == null) + author = u; + revmgr.createRevision(author, "user." + u.getId(), "Password changed"); + db.session().saveOrUpdate(u); + } + /** * @return all users * @throws HibernateException on low level hibernate related exception @@ -71,14 +94,11 @@ public User getUserByNick (String nick) public User getUserByNick (String nick, boolean includeDeleted) throws HibernateException { - try { - Criteria crit = db.session().createCriteria (User.class) - .add (Restrictions.eq ("nick", nick)); - if (!includeDeleted) - crit = crit.add (Restrictions.eq ("deleted", Boolean.FALSE)); - return (User) crit.uniqueResult(); - } catch (ObjectNotFoundException ignored) { } - return null; + Criteria crit = db.session().createCriteria(User.class) + .add(Restrictions.eq("nick", nick)); + if (!includeDeleted) + crit = crit.add (Restrictions.eq ("deleted", Boolean.FALSE)); + return (User) crit.uniqueResult(); } /** @@ -101,22 +121,15 @@ public boolean checkPassword (User u, String clearpass) throws HibernateException, BLException { assertNotNull(clearpass, "Invalid pass"); - String passwordHash = u.getPassword(); + String passwordHash = u.getPasswordHash(); assertNotNull(passwordHash, "Password is null"); VERSION v = VERSION.getVersion(passwordHash); assertTrue(v != VERSION.UNKNOWN, "Unknown password"); - switch (v.getVersion()) { - case (byte) 0: - return (passwordHash.equals(getV0Hash(u.getId(), clearpass))); - case (byte) 1: - byte[] b = Base64.decode(passwordHash); - byte[] salt = new byte[VERSION.ONE.getSalt().length]; - byte[] clientSalt = new byte[VERSION.ONE.getSalt().length]; - System.arraycopy (b, 1, salt, 0, salt.length); - System.arraycopy(b, 1 + salt.length, clientSalt, 0, clientSalt.length); - clearpass = clearpass.startsWith("v1:") ? clearpass.substring(3) : upgradeClearPassword(clearpass, clientSalt); - String computedPasswordHash = genV1Hash(clearpass, VERSION.ONE.getSalt(salt), clientSalt); - return computedPasswordHash.equals(u.getPassword()); + switch (v) { + case ZERO: + return checkV0Password(passwordHash, u.getId(), clearpass); + case ONE: + return checkV1Password(passwordHash, clearpass); } return false; } @@ -126,138 +139,78 @@ public boolean checkPassword (User u, String clearpass) * @param clearpass new password in clear * @return true if password is in PasswordHistory */ - public boolean checkNewPassword (User u, String clearpass) { - // TODO - we need to reuse client hash in order to check duplicates - String newHash = getV0Hash(u.getId(), clearpass); - if (newHash.equals(u.getPassword())) - return false; + public boolean checkNewPassword (User u, String clearpass) throws BLException { + if (checkPassword (u, clearpass)) { + return false; // same password not allowed + } for (PasswordHistory p : u.getPasswordhistory()) { - if (p.getValue().equals(newHash)) - return false; + VERSION v = VERSION.getVersion(p.getValue()); + switch (v) { + case ZERO: + if (checkV0Password(p.getValue(), u.getId(), clearpass)) + return false; + case ONE: + if (checkV1Password (p.getValue(), clearpass)) + return false; + } } return true; } - public void setPassword(User u, String clearpass) throws BLException { - setPassword(u, clearpass, null); - } - - public void setPassword (User u, String clearpass, User author) throws BLException { - if (u.getPassword() != null) - u.addPasswordHistoryValue(u.getPassword()); - switch (version) { - case ZERO: - setV0Password(u, clearpass); - break; - case ONE: - setV1Password (u, clearpass); - break; - } - RevisionManager revmgr = new RevisionManager(db); - if (author == null) - author = u; - revmgr.createRevision (author, "user." + u.getId(), "Password changed"); - db.session().saveOrUpdate(u); - } - - // VERSION ZERO IMPLEMENTATION - private static String getV0Hash (long id, String pass) { - String hash = null; - try { - MessageDigest md = MessageDigest.getInstance ("SHA"); - md.update (Long.toString(id,16).getBytes()); - hash = ISOUtil.hexString ( - md.digest (pass.getBytes()) - ).toLowerCase(); - } catch (NoSuchAlgorithmException e) { - // should never happen - } - return hash; - } - private void setV0Password (User u, String clearpass) { - u.setPassword(getV0Hash(u.getId(), clearpass)); - } - - // VERSION ONE IMPLEMENTATION - private String upgradeClearPassword (String clearpass, byte[] salt) { - return ISOUtil.hexString(genV1ClientHash(clearpass, salt)); - } - - private byte[] genSalt(int l) { - SecureRandom sr; - try { - sr = SecureRandom.getInstance("SHA1PRNG"); - byte[] salt = new byte[l]; - sr.nextBytes(salt); - return salt; - } catch (NoSuchAlgorithmException ignored) { - // Should never happen, SHA1PRNG is a supported algorithm - } - return null; - } - public boolean upgradePassword (User u, String clearpass) throws HibernateException, BLException { assertNotNull(clearpass, "Invalid pass"); - String passwordHash = u.getPassword(); + String passwordHash = u.getPasswordHash(); assertNotNull(passwordHash, "Password is null"); VERSION v = VERSION.getVersion(passwordHash); - if (v == VERSION.ZERO && passwordHash.equals(getV0Hash(u.getId(), clearpass))) { - setV1Password(u, clearpass); + if (v == VERSION.ZERO && checkV0Password(passwordHash, u.getId(), clearpass)) { + setPassword(u, clearpass, null, VERSION.ONE); return true; } return false; } - private void setV1Password (User u, String pass) throws BLException { - assertNotNull(pass, "Invalid password"); - byte[] clientSalt; - byte[] serverSalt = genSalt(VERSION.ONE.getSalt().length); - if (pass.startsWith("v1:")) { - byte[] b = Base64.decode(u.getPassword()); - clientSalt = new byte[VERSION.ONE.getSalt().length]; - System.arraycopy (b, 1+clientSalt.length, clientSalt, 0, clientSalt.length); - pass = pass.substring(3); - } else { - clientSalt = genSalt(VERSION.ONE.getSalt().length); - pass = upgradeClearPassword(pass, clientSalt); - } - u.setPassword(genV1Hash(pass, serverSalt, clientSalt)); + public VERSION getVersion() { + return version; } - private String genV1Hash(String password, byte[] salt, byte[] clientSalt) throws BLException { + public void setVersion(VERSION version) { + this.version = version; + } + + private void setV1Password (User u, String clearpass) throws BLException { + assertNotNull(clearpass, "Invalid password"); + byte[] salt = genSalt(VERSION.ONE.getSalt().length); + u.setPasswordHash(genV1Hash(clearpass, salt)); + } + + private String genV1Hash(String password, byte[] salt) throws BLException { try { SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA256"); int iterations = VERSION.ONE.getIterations(); - PBEKeySpec spec = new PBEKeySpec(password.toCharArray(), salt, iterations, 2048); + PBEKeySpec spec = new PBEKeySpec(password.toCharArray(), salt, iterations, VERSION.ONE.getKeylength()); return org.bouncycastle.util.encoders.Base64.toBase64String( - Arrays.concatenate( - new byte[]{VERSION.ONE.getVersion()}, - VERSION.ONE.getSalt(salt), - clientSalt, - skf.generateSecret(spec).getEncoded()) + Arrays.concatenate( + new byte[]{VERSION.ONE.getVersion()}, + VERSION.ONE.getSalt(salt), + skf.generateSecret(spec).getEncoded()) ); } catch (Exception e) { throw new BLException (e.getLocalizedMessage()); } } - private byte[] genV1ClientHash (String clearpass, byte[] seed) { - try { - MessageDigest md = MessageDigest.getInstance("SHA-256"); - int iterations = VERSION.ONE.getIterations(); - byte[] passAsBytes = clearpass.getBytes(StandardCharsets.UTF_8); - for (int i=0; i - + @@ -43,7 +43,7 @@ - + diff --git a/modules/eeuser/src/test/java/org/jpos/ee/EEUserTest.java b/modules/eeuser/src/test/java/org/jpos/ee/EEUserTest.java index a372d94c6a..a950db78e6 100644 --- a/modules/eeuser/src/test/java/org/jpos/ee/EEUserTest.java +++ b/modules/eeuser/src/test/java/org/jpos/ee/EEUserTest.java @@ -1,12 +1,10 @@ package org.jpos.ee; -import org.bouncycastle.util.encoders.Base64; import org.junit.After; import org.junit.Before; import org.junit.Test; import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.util.Set; import static org.junit.Assert.*; @@ -33,8 +31,8 @@ private void createUser() throws BLException { user.setName("User Administrator"); user.setActive(true); db.session().save(user); - UserManager mgr = new UserManager(db); - mgr.setPassword(user, "test"); + UserManager mgr = new UserManager(db, UserManager.VERSION.ZERO); + mgr.setPassword(user, "test", null); Role role = new Role("admin"); Set perms = role.getPermissions(); perms.add(Permission.valueOf("login")); @@ -56,19 +54,23 @@ public void checkUser() throws BLException, NoSuchMethodException, InvocationTar assertTrue("User has any permissions", u.hasAnyPermission(new String[]{"nologin", "admin", "role.admin"})); assertFalse("User don't have 'superuser' permission", u.hasPermission("superuser")); assertTrue("User password is 'test'", mgr.checkPassword(u, "test")); - assertEquals("User hash is correct", "ee89026a6c5603c51b4504d218ac60f6874b7750", u.getPassword()); - assertTrue("Upgrade password", mgr.upgradePassword(u, "test")); - assertNotEquals("User hash has changed", "ee89026a6c5603c51b4504d218ac60f6874b7750", u.getPassword()); - assertTrue("User upgraded password is still 'test'", mgr.checkPassword(u, "test")); - - byte[] b = Base64.decode(u.getPassword()); - byte[] clientSalt = new byte[UserManager.VERSION.ONE.getSalt().length]; - System.arraycopy(b, 1 + clientSalt.length, clientSalt, 0, clientSalt.length); - - Method method = UserManager.class.getDeclaredMethod("upgradeClearPassword", String.class, byte[].class); - method.setAccessible(true); - String upgradedClearPass = "v1:" + method.invoke(mgr, "test", clientSalt); - assertTrue("User upgraded password is still 'hash(test)'", mgr.checkPassword(u, upgradedClearPass)); + assertEquals("User hash is correct", "ee89026a6c5603c51b4504d218ac60f6874b7750", u.getPasswordHash()); + assertFalse("Password has to be in history", mgr.checkNewPassword(u, "test")); + mgr.upgradePassword(u, "test"); + assertNotEquals("User hash has changed", "ee89026a6c5603c51b4504d218ac60f6874b7750", u.getPasswordHash()); + assertTrue("User password is still 'test'", mgr.checkPassword(u, "test")); + assertNotEquals("User hash has changed", "ee89026a6c5603c51b4504d218ac60f6874b7750", u.getPasswordHash()); + assertFalse("Password has to be in history", mgr.checkNewPassword(u, "test")); + mgr.setPassword(u, "test1"); + mgr.setPassword(u, "test2"); + mgr.setPassword(u, "test3"); + assertFalse("Password 1 has to be in history", mgr.checkNewPassword(u, "test1")); + assertFalse("Password 2 has to be in history", mgr.checkNewPassword(u, "test2")); + assertFalse("Password 3 has to be in history", mgr.checkNewPassword(u, "test3")); + assertTrue("User password is now 'test3'", mgr.checkPassword(u, "test3")); + mgr.setPassword(u, "test"); + assertTrue("User password is back to 'test'", mgr.checkPassword(u, "test")); + assertEquals ("History size is ", 5, u.getPasswordhistory().size()); db.commit(); }