From e6ebd5a51eeca62774c149a5940cf5adbc47930a Mon Sep 17 00:00:00 2001 From: Mitch Miller Date: Wed, 6 Mar 2024 22:19:28 -0500 Subject: [PATCH 1/6] part-way implementation of revised salting and hashing of passwords --- .../UserProfileAuthenticationResult.java | 28 +++++++++ .../main/java/ix/core/models/UserProfile.java | 34 ++++++++--- .../src/test/java/gsrs/LegacySalterTests.java | 33 ++++++++++ .../java/gsrs/util/GsrsPasswordHasher.java | 60 +++++++++++++++++++ gsrs-core/src/main/java/gsrs/util/Hasher.java | 8 +++ .../main/java/gsrs/util/LegacyTypeSalter.java | 33 ++++++++++ gsrs-core/src/main/java/gsrs/util/Salter.java | 10 ++++ .../java/gsrs/controller/LoginController.java | 5 +- .../security/LegacyAuthenticationFilter.java | 8 ++- .../LegacyGsrsAuthenticationProvider.java | 12 ++-- .../service/UserProfileServiceTest.java | 31 +++++----- 11 files changed, 227 insertions(+), 35 deletions(-) create mode 100644 gsrs-core-entities/src/main/java/gsrs/model/UserProfileAuthenticationResult.java create mode 100644 gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java create mode 100644 gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java create mode 100644 gsrs-core/src/main/java/gsrs/util/Hasher.java create mode 100644 gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java create mode 100644 gsrs-core/src/main/java/gsrs/util/Salter.java diff --git a/gsrs-core-entities/src/main/java/gsrs/model/UserProfileAuthenticationResult.java b/gsrs-core-entities/src/main/java/gsrs/model/UserProfileAuthenticationResult.java new file mode 100644 index 00000000..3169edbb --- /dev/null +++ b/gsrs-core-entities/src/main/java/gsrs/model/UserProfileAuthenticationResult.java @@ -0,0 +1,28 @@ +package gsrs.model; + +import lombok.AllArgsConstructor; +import lombok.NoArgsConstructor; + +@NoArgsConstructor +@AllArgsConstructor +public class UserProfileAuthenticationResult { + private boolean matchesRepository; + + public boolean matchesRepository() { + return matchesRepository; + } + + public void setMatchesRepository(boolean matchesRepository) { + this.matchesRepository = matchesRepository; + } + + public boolean needsSave() { + return needsSave; + } + + public void setNeedsSave(boolean needsSave) { + this.needsSave = needsSave; + } + + private boolean needsSave; +} diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index dd744772..cf419e91 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -1,17 +1,20 @@ package ix.core.models; -import ch.qos.logback.core.rolling.helper.TokenConverter; import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.databind.ObjectMapper; import gov.nih.ncats.common.util.CachedSupplier; import gov.nih.ncats.common.util.TimeUtil; +import gsrs.model.UserProfileAuthenticationResult; import gsrs.security.TokenConfiguration; import gsrs.springUtils.StaticContextAccessor; +import gsrs.util.GsrsPasswordHasher; +import gsrs.util.Hasher; +import gsrs.util.LegacyTypeSalter; +import gsrs.util.Salter; import ix.utils.Util; import lombok.extern.slf4j.Slf4j; -import org.springframework.beans.factory.annotation.Autowired; import javax.persistence.*; import java.util.*; @@ -24,6 +27,10 @@ public class UserProfile extends IxModel{ private static ObjectMapper om = new ObjectMapper(); + private static Salter salter = new LegacyTypeSalter(new GsrsPasswordHasher()); + + private static Hasher hasher = new GsrsPasswordHasher(); + private static CachedSupplier GUEST_PROF= CachedSupplier.of(()->{ UserProfile up = new UserProfile(new Principal("GUEST")); up.addRole(Role.Query); @@ -182,21 +189,32 @@ public boolean acceptToken(String token) { return false; } - public boolean acceptPassword(String password) { - if (this.hashp == null || this.salt == null) - return false; - return this.hashp.equals(Util.encrypt(password, this.salt)); + public UserProfileAuthenticationResult acceptPassword(String password) { + UserProfileAuthenticationResult result = new UserProfileAuthenticationResult(false, false); + result.setNeedsSave(false); + if (this.hashp == null || this.salt == null) { + return result; + } + boolean pwOk = this.hashp.equals(Util.encrypt(password, this.salt)); + result.setMatchesRepository(true); + if( pwOk && !salter.mayBeOneOfMine(this.salt)){ + log.trace("going to redo password"); + setPassword(password); + result.setNeedsSave(true); + } + return result; } public void setPassword(String password) { if (password == null || password.length() <= 0) { password = UUID.randomUUID().toString(); } - this.salt = Util.generateSalt(); - this.hashp = Util.encrypt(password, this.salt); + this.salt = salter.generateSalt(); + this.hashp = hasher.hash(password, salt); //Util.encrypt(password, this.salt); setIsDirty("salt"); setIsDirty("hashp"); } + @Indexable(indexed = false) @JsonIgnore public String getEncodePassword(){ diff --git a/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java new file mode 100644 index 00000000..0c1b810f --- /dev/null +++ b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java @@ -0,0 +1,33 @@ +package gsrs; + +import gsrs.util.GsrsPasswordHasher; +import gsrs.util.Hasher; +import gsrs.util.LegacyTypeSalter; +import gsrs.util.Salter; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class LegacySalterTests { + + private Hasher hasher = new GsrsPasswordHasher(); + private Salter salter = new LegacyTypeSalter(hasher); + @Test + void testGenerateSalt() { + String salt1 = salter.generateSalt(); + Assertions.assertNotNull(salt1); + } + + @Test + void testGenerateOwnSalt() { + String salt1 = salter.generateSalt(); + Assertions.assertTrue(salter.mayBeOneOfMine(salt1)); + } + + @Test + void testGenerateNotOwnSalt() { + String salt1 = salter.generateSalt(); + String changedSalt = salt1.replace('G', 'N'); + Assertions.assertFalse(salter.mayBeOneOfMine(changedSalt)); + } + +} diff --git a/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java new file mode 100644 index 00000000..5fe4c8ec --- /dev/null +++ b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java @@ -0,0 +1,60 @@ +package gsrs.util; + +import lombok.extern.slf4j.Slf4j; + +import javax.crypto.SecretKeyFactory; +import javax.crypto.spec.PBEKeySpec; +import java.io.UnsupportedEncodingException; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.spec.InvalidKeySpecException; + +@Slf4j +public class GsrsPasswordHasher implements Hasher { + + String preferredHashAlgorithm = "PBKDF2"; + static int iterations = 1000; + static String characterSet ="utf8"; + + @Override + public String getHashType() { + return this.preferredHashAlgorithm; + } + + @Override + public String hash(String... values) { + if (values == null) { + return null; + } + try { + if(preferredHashAlgorithm.equals("PBKDF2")) { + return hash(values[0], values.length > 1 ? values[1] : null, iterations); + } + MessageDigest md = MessageDigest.getInstance(preferredHashAlgorithm); + for (String v : values) { + md.update(v.getBytes(characterSet)); + } + return toHex(md.digest()); + } catch (Exception ex) { + log.error("Can't generate hash!", ex); + throw new RuntimeException(ex); + } + } + + public static String toHex(byte[] d) { + StringBuilder sb = new StringBuilder(); + for (byte b : d) { + sb.append(String.format("%1$02x", b & 0xff)); + } + return sb.toString(); + } + + public static String hash(String input, String salt, int iterations) throws NoSuchAlgorithmException, InvalidKeySpecException, UnsupportedEncodingException { + PBEKeySpec spec = new PBEKeySpec(input.toCharArray(), salt != null ? salt.getBytes(characterSet) : + input.getBytes(characterSet), iterations, 64 * 8); + SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + + byte[] hash = skf.generateSecret(spec).getEncoded(); + return new String(hash); + } +} diff --git a/gsrs-core/src/main/java/gsrs/util/Hasher.java b/gsrs-core/src/main/java/gsrs/util/Hasher.java new file mode 100644 index 00000000..1b7370ca --- /dev/null +++ b/gsrs-core/src/main/java/gsrs/util/Hasher.java @@ -0,0 +1,8 @@ +package gsrs.util; + +public interface Hasher { + + String getHashType(); + + String hash(String... values); +} diff --git a/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java new file mode 100644 index 00000000..1fa864fc --- /dev/null +++ b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java @@ -0,0 +1,33 @@ +package gsrs.util; + +import gov.nih.ncats.common.util.TimeUtil; +import lombok.Data; + +import static java.lang.String.valueOf; + +@Data +public class LegacyTypeSalter implements Salter { + + Hasher hasher; + + String prefix = "G"; + + public LegacyTypeSalter(Hasher newHasher) { + hasher= newHasher; + } + @Override + public void setHasher(Hasher hasher) { + this.hasher = hasher; + } + + @Override + public String generateSalt() { + String text = "---" + TimeUtil.getCurrentDate().toString() + "---" + String.valueOf(Math.random()) + "---"; + return prefix + hasher.hash(text); + } + + @Override + public boolean mayBeOneOfMine(String testHash) { + return testHash != null && testHash.startsWith(prefix); + } +} diff --git a/gsrs-core/src/main/java/gsrs/util/Salter.java b/gsrs-core/src/main/java/gsrs/util/Salter.java new file mode 100644 index 00000000..eb43a72e --- /dev/null +++ b/gsrs-core/src/main/java/gsrs/util/Salter.java @@ -0,0 +1,10 @@ +package gsrs.util; + +public interface Salter { + + void setHasher(Hasher hasher); + + String generateSalt(); + + boolean mayBeOneOfMine(String testHash); +} diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/LoginController.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/LoginController.java index aad73967..95f0be1f 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/LoginController.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/controller/LoginController.java @@ -2,6 +2,7 @@ import gsrs.cache.GsrsCache; import gsrs.controller.hateoas.GsrsUnwrappedEntityModel; +import gsrs.model.UserProfileAuthenticationResult; import gsrs.repository.GroupRepository; import gsrs.repository.SessionRepository; import gsrs.repository.UserProfileRepository; @@ -10,7 +11,6 @@ import ix.core.models.Session; import ix.core.models.UserProfile; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.beans.factory.annotation.Value; import org.springframework.http.HttpStatus; import org.springframework.http.ResponseEntity; import org.springframework.security.access.prepost.PreAuthorize; @@ -135,7 +135,8 @@ public ResponseEntity changePassword(Principal principal, return gsrsControllerConfiguration.handleBadRequest(400,"password can not be blank or all whitespace", queryParameters); } //check old password - if(!up.acceptPassword(passwordChangeRequest.getOldPassword())){ + UserProfileAuthenticationResult authenticationResult =up.acceptPassword(passwordChangeRequest.getOldPassword()); + if(!authenticationResult.matchesRepository()){ return gsrsControllerConfiguration.unauthorized("incorrect password", queryParameters); } up.setPassword(passwordChangeRequest.getNewPassword()); diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java index 989b725a..c59240c7 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java @@ -13,6 +13,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import gsrs.model.UserProfileAuthenticationResult; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; @@ -172,10 +173,13 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse } if(up!=null && up.active){ - if(up.acceptPassword(pass)){ + UserProfileAuthenticationResult authenticationResult =up.acceptPassword(pass); + if(authenticationResult.matchesRepository()){ + if(authenticationResult.needsSave()){ + repository.saveAndFlush(up); + } //valid password! auth = new UserProfilePasswordAuthentication(up); - }else{ throw new BadCredentialsException("invalid credentials for username: " + username); } diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java index ff06eaf5..10d5342d 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java @@ -1,6 +1,6 @@ package gsrs.security; -import gsrs.repository.PrincipalRepository; +import gsrs.model.UserProfileAuthenticationResult; import gsrs.repository.SessionRepository; import gsrs.repository.UserProfileRepository; import ix.core.models.Principal; @@ -11,12 +11,10 @@ import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; -import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.transaction.annotation.Transactional; import java.util.Optional; import java.util.UUID; -import java.util.stream.Collectors; //@Component public class LegacyGsrsAuthenticationProvider implements AuthenticationProvider { @@ -83,11 +81,13 @@ public Authentication authenticate(Authentication authentication) throws Authent } if(up!=null){ String rawPassword = (String) auth.getCredentials(); - if(up.acceptPassword(rawPassword)){ + UserProfileAuthenticationResult authenticationResult =up.acceptPassword(rawPassword); + if(authenticationResult.matchesRepository() ){ //valid password! - + if( authenticationResult.needsSave()) { + repository.saveAndFlush(up); + } return new UserProfilePasswordAuthentication(up); - }else{ throw new BadCredentialsException("invalid credentials for username" + auth.getUsername()); } diff --git a/gsrs-spring-starter-tests/src/test/java/gsrs/startertests/service/UserProfileServiceTest.java b/gsrs-spring-starter-tests/src/test/java/gsrs/startertests/service/UserProfileServiceTest.java index 833232e2..dd2caff4 100644 --- a/gsrs-spring-starter-tests/src/test/java/gsrs/startertests/service/UserProfileServiceTest.java +++ b/gsrs-spring-starter-tests/src/test/java/gsrs/startertests/service/UserProfileServiceTest.java @@ -10,13 +10,10 @@ import ix.core.models.Role; import ix.core.models.UserProfile; import org.junit.jupiter.api.Assertions; -import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest; import org.springframework.boot.test.autoconfigure.orm.jpa.TestEntityManager; import org.springframework.boot.test.context.TestConfiguration; -import org.springframework.context.annotation.Bean; import org.springframework.security.access.AccessDeniedException; import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity; import org.springframework.security.core.AuthenticationException; @@ -157,7 +154,7 @@ public void createUserWithRoles(){ assertEquals(1, userProfileRepository.count()); assertEquals(1, principalRepository.count()); - assertFalse(up.acceptPassword("fakepass")); + assertFalse(up.acceptPassword("fakepass").matchesRepository()); } @Test @@ -199,7 +196,7 @@ public void createUserSetPassword(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); } @@ -215,8 +212,8 @@ public void modifyUserChangePassword(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); - assertFalse(up.acceptPassword("newPass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); + assertFalse(up.acceptPassword("newPass").matchesRepository()); UserProfile up2 = userProfileService.updateUserProfile(UserProfileService.NewUserRequest.builder() .username("myUser") @@ -226,8 +223,8 @@ public void modifyUserChangePassword(){ assertEquals(up2.id,up.id); assertEquals(up2.user.id, up.user.id); - assertFalse(up2.acceptPassword("mypass")); - assertTrue(up2.acceptPassword("newPass")); + assertFalse(up2.acceptPassword("mypass").matchesRepository()); + assertTrue(up2.acceptPassword("newPass").matchesRepository()); } @@ -243,8 +240,8 @@ public void modifyUserAddGroupFromNone(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); - assertFalse(up.acceptPassword("newPass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); + assertFalse(up.acceptPassword("newPass").matchesRepository()); UserProfile up2 = userProfileService.updateUserProfile(UserProfileService.NewUserRequest.builder() .username("myUser") @@ -275,8 +272,8 @@ public void modifyUserAddGroupFromAlreadyExistingList(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); - assertFalse(up.acceptPassword("newPass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); + assertFalse(up.acceptPassword("newPass").matchesRepository()); UserProfile up2 = userProfileService.updateUserProfile(UserProfileService.NewUserRequest.builder() .username("myUser") @@ -313,8 +310,8 @@ public void modifyUserRemoveGroupFromAlreadyExistingList(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); - assertFalse(up.acceptPassword("newPass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); + assertFalse(up.acceptPassword("newPass").matchesRepository()); UserProfile up2 = userProfileService.updateUserProfile(UserProfileService.NewUserRequest.builder() .username("myUser") @@ -351,8 +348,8 @@ public void modifyUserRemoveAndAddGroupsFromAlreadyExistingList(){ assertNotNull(up.id); assertNotNull(up.user.id); - assertTrue(up.acceptPassword("mypass")); - assertFalse(up.acceptPassword("newPass")); + assertTrue(up.acceptPassword("mypass").matchesRepository()); + assertFalse(up.acceptPassword("newPass").matchesRepository()); UserProfile up2 = userProfileService.updateUserProfile(UserProfileService.NewUserRequest.builder() .username("myUser") From 213831da40e3f9876db3fa83859632f1793b3ec7 Mon Sep 17 00:00:00 2001 From: Mitch Miller Date: Thu, 7 Mar 2024 10:27:33 -0500 Subject: [PATCH 2/6] made new hasher convert values to hex before returning --- .../src/main/java/ix/core/models/UserProfile.java | 2 +- .../src/test/java/gsrs/LegacySalterTests.java | 1 + .../src/main/java/gsrs/util/GsrsPasswordHasher.java | 2 +- .../gsrs/security/LegacyAuthenticationFilter.java | 7 ++++++- .../security/LegacyGsrsAuthenticationProvider.java | 13 ++++++++++++- 5 files changed, 21 insertions(+), 4 deletions(-) diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index cf419e91..21f30d21 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -198,7 +198,7 @@ public UserProfileAuthenticationResult acceptPassword(String password) { boolean pwOk = this.hashp.equals(Util.encrypt(password, this.salt)); result.setMatchesRepository(true); if( pwOk && !salter.mayBeOneOfMine(this.salt)){ - log.trace("going to redo password"); + log.trace("going to rehash password"); setPassword(password); result.setNeedsSave(true); } diff --git a/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java index 0c1b810f..e22b6c40 100644 --- a/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java +++ b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java @@ -15,6 +15,7 @@ public class LegacySalterTests { void testGenerateSalt() { String salt1 = salter.generateSalt(); Assertions.assertNotNull(salt1); + System.out.printf("salt: %s\n", salt1); } @Test diff --git a/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java index 5fe4c8ec..ec08d3e1 100644 --- a/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java +++ b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java @@ -55,6 +55,6 @@ public static String hash(String input, String salt, int iterations) throws NoSu SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); byte[] hash = skf.generateSecret(spec).getEncoded(); - return new String(hash); + return toHex(hash); } } diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java index c59240c7..2f0dd591 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java @@ -176,7 +176,12 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse UserProfileAuthenticationResult authenticationResult =up.acceptPassword(pass); if(authenticationResult.matchesRepository()){ if(authenticationResult.needsSave()){ - repository.saveAndFlush(up); + UserProfile finalUp = up; + TransactionTemplate transactionTemplate = new TransactionTemplate(platformTransactionManager); + transactionTemplate.executeWithoutResult(status ->{ + log.trace("saving up within transaction"); + repository.saveAndFlush(finalUp); + }); } //valid password! auth = new UserProfilePasswordAuthentication(up); diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java index 10d5342d..dd374e48 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyGsrsAuthenticationProvider.java @@ -6,17 +6,21 @@ import ix.core.models.Principal; import ix.core.models.Session; import ix.core.models.UserProfile; +import lombok.extern.slf4j.Slf4j; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.security.authentication.AuthenticationProvider; import org.springframework.security.authentication.BadCredentialsException; import org.springframework.security.core.Authentication; import org.springframework.security.core.AuthenticationException; +import org.springframework.transaction.PlatformTransactionManager; import org.springframework.transaction.annotation.Transactional; +import org.springframework.transaction.support.TransactionTemplate; import java.util.Optional; import java.util.UUID; //@Component +@Slf4j public class LegacyGsrsAuthenticationProvider implements AuthenticationProvider { @Autowired @@ -30,6 +34,10 @@ public class LegacyGsrsAuthenticationProvider implements AuthenticationProvider @Autowired(required = false) private UserTokenCache userTokenCache; + + @Autowired + private PlatformTransactionManager platformTransactionManager; + @Override @Transactional public Authentication authenticate(Authentication authentication) throws AuthenticationException { @@ -84,8 +92,11 @@ public Authentication authenticate(Authentication authentication) throws Authent UserProfileAuthenticationResult authenticationResult =up.acceptPassword(rawPassword); if(authenticationResult.matchesRepository() ){ //valid password! + UserProfile finalUp = up; if( authenticationResult.needsSave()) { - repository.saveAndFlush(up); + log.trace("going to save up within transaction"); + TransactionTemplate transactionTemplate = new TransactionTemplate(platformTransactionManager); + transactionTemplate.executeWithoutResult(u->repository.saveAndFlush(finalUp)); } return new UserProfilePasswordAuthentication(up); }else{ From 360f18f1859564681b1ee7143d1de7d4c239f5de Mon Sep 17 00:00:00 2001 From: Mitch Miller Date: Thu, 7 Mar 2024 15:12:22 -0500 Subject: [PATCH 3/6] added TODO --- .../src/main/java/ix/core/models/UserProfile.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index 21f30d21..14262dda 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -15,6 +15,7 @@ import gsrs.util.Salter; import ix.utils.Util; import lombok.extern.slf4j.Slf4j; +import org.springframework.beans.factory.annotation.Autowired; import javax.persistence.*; import java.util.*; @@ -27,6 +28,7 @@ public class UserProfile extends IxModel{ private static ObjectMapper om = new ObjectMapper(); + //todo: look into autowiring the salter and hasher private static Salter salter = new LegacyTypeSalter(new GsrsPasswordHasher()); private static Hasher hasher = new GsrsPasswordHasher(); From be241ad10fde5e73413fc51259822ec99dba3246 Mon Sep 17 00:00:00 2001 From: Mitch Miller Date: Tue, 12 Mar 2024 18:44:35 -0400 Subject: [PATCH 4/6] Got password management changes partially done. We now use PBKDF2 for password hashing. Additional clean-ups and testing required! --- .../main/java/ix/core/models/UserProfile.java | 4 ++-- .../main/java/gsrs/util/LegacyTypeSalter.java | 1 + .../security/LegacyAuthenticationFilter.java | 22 ++++++++++++++----- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index 14262dda..4217b1b8 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -200,8 +200,8 @@ public UserProfileAuthenticationResult acceptPassword(String password) { boolean pwOk = this.hashp.equals(Util.encrypt(password, this.salt)); result.setMatchesRepository(true); if( pwOk && !salter.mayBeOneOfMine(this.salt)){ - log.trace("going to rehash password"); - setPassword(password); + log.trace("going to request rehash of password"); + //setPassword(password); result.setNeedsSave(true); } return result; diff --git a/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java index 1fa864fc..67e60ef0 100644 --- a/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java +++ b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java @@ -12,6 +12,7 @@ public class LegacyTypeSalter implements Salter { String prefix = "G"; + //todo: default prefix to be hullable; add a prefix to constructor public LegacyTypeSalter(Hasher newHasher) { hasher= newHasher; } diff --git a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java index 2f0dd591..a9ae172b 100644 --- a/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java +++ b/gsrs-spring-boot-autoconfigure/src/main/java/gsrs/security/LegacyAuthenticationFilter.java @@ -12,6 +12,7 @@ import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import javax.transaction.Transactional; import gsrs.model.UserProfileAuthenticationResult; import org.springframework.beans.factory.annotation.Autowired; @@ -175,16 +176,21 @@ protected void doFilterInternal(HttpServletRequest request, HttpServletResponse if(up!=null && up.active){ UserProfileAuthenticationResult authenticationResult =up.acceptPassword(pass); if(authenticationResult.matchesRepository()){ + //valid password! if(authenticationResult.needsSave()){ UserProfile finalUp = up; TransactionTemplate transactionTemplate = new TransactionTemplate(platformTransactionManager); - transactionTemplate.executeWithoutResult(status ->{ - log.trace("saving up within transaction"); - repository.saveAndFlush(finalUp); + UserProfile savedUp=transactionTemplate.execute(status -> { + Optional opt = Optional.ofNullable(repository.findByUser_UsernameIgnoreCase(finalUp.user.username)); + opt.get().setPassword(pass); + saveUserProfile(opt.get()); + return opt.get(); }); + auth = new UserProfilePasswordAuthentication(savedUp); + } else { + auth = new UserProfilePasswordAuthentication(up); } - //valid password! - auth = new UserProfilePasswordAuthentication(up); + }else{ throw new BadCredentialsException("invalid credentials for username: " + username); } @@ -272,4 +278,10 @@ private UserProfile autoregisterNewUser(String username, String email, List Date: Mon, 18 Mar 2024 14:27:37 -0400 Subject: [PATCH 5/6] got updated user profiles to save --- .../src/main/java/ix/core/models/UserProfile.java | 7 ++++--- .../src/test/java/gsrs/LegacySalterTests.java | 8 ++++++-- gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java | 8 ++++---- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index 4217b1b8..913dbd44 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -15,7 +15,6 @@ import gsrs.util.Salter; import ix.utils.Util; import lombok.extern.slf4j.Slf4j; -import org.springframework.beans.factory.annotation.Autowired; import javax.persistence.*; import java.util.*; @@ -26,10 +25,12 @@ @SequenceGenerator(name = "LONG_SEQ_ID", sequenceName = "ix_core_userprof_seq", allocationSize = 1) @EntityListeners(UserProfileEntityProcessor.class) public class UserProfile extends IxModel{ - private static ObjectMapper om = new ObjectMapper(); + private final static String SALT_PREFIX = "G"; + + private static ObjectMapper om = new ObjectMapper(); //todo: look into autowiring the salter and hasher - private static Salter salter = new LegacyTypeSalter(new GsrsPasswordHasher()); + private static Salter salter = new LegacyTypeSalter(new GsrsPasswordHasher(), SALT_PREFIX); private static Hasher hasher = new GsrsPasswordHasher(); diff --git a/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java index e22b6c40..7c608969 100644 --- a/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java +++ b/gsrs-core-entities/src/test/java/gsrs/LegacySalterTests.java @@ -9,8 +9,12 @@ public class LegacySalterTests { - private Hasher hasher = new GsrsPasswordHasher(); - private Salter salter = new LegacyTypeSalter(hasher); + private final String SALT_PREFIX = "G"; + + private final Hasher hasher = new GsrsPasswordHasher(); + + private final Salter salter = new LegacyTypeSalter(hasher, SALT_PREFIX); + @Test void testGenerateSalt() { String salt1 = salter.generateSalt(); diff --git a/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java index 67e60ef0..89de3fa0 100644 --- a/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java +++ b/gsrs-core/src/main/java/gsrs/util/LegacyTypeSalter.java @@ -10,11 +10,11 @@ public class LegacyTypeSalter implements Salter { Hasher hasher; - String prefix = "G"; + String prefix = ""; - //todo: default prefix to be hullable; add a prefix to constructor - public LegacyTypeSalter(Hasher newHasher) { - hasher= newHasher; + public LegacyTypeSalter(Hasher newHasher, String newPrefix) { + hasher = newHasher; + prefix = newPrefix; } @Override public void setHasher(Hasher hasher) { From bdffc49f94c1fdc50f61db0bc9925148e9332bb4 Mon Sep 17 00:00:00 2001 From: Mitch Miller Date: Mon, 25 Mar 2024 19:46:48 -0400 Subject: [PATCH 6/6] Fixed 2 issues within UserProfile: 1) reject logins when password does not match 2) try out legacy hashing when current hashing fails --- .../main/java/ix/core/models/UserProfile.java | 16 +++++++++++----- .../main/java/gsrs/util/GsrsPasswordHasher.java | 4 +++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java index 913dbd44..8a8f1237 100644 --- a/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java +++ b/gsrs-core-entities/src/main/java/ix/core/models/UserProfile.java @@ -194,17 +194,23 @@ public boolean acceptToken(String token) { public UserProfileAuthenticationResult acceptPassword(String password) { UserProfileAuthenticationResult result = new UserProfileAuthenticationResult(false, false); - result.setNeedsSave(false); if (this.hashp == null || this.salt == null) { return result; } - boolean pwOk = this.hashp.equals(Util.encrypt(password, this.salt)); - result.setMatchesRepository(true); - if( pwOk && !salter.mayBeOneOfMine(this.salt)){ + boolean pwOk = this.hashp.equals(hasher.hash(password, this.salt)); + log.trace("pwOk: {}", pwOk); + boolean legacyPwOk = false; + //when authentication using latest algorithms fails, see if the password works with the legacy methods + if( !pwOk) { + legacyPwOk= this.hashp.equals(Util.encrypt(password, this.salt)); + } + if( legacyPwOk && !salter.mayBeOneOfMine(this.salt)) { + //we have a pw assigned using the older algorithm so we need to resalt and rehash log.trace("going to request rehash of password"); - //setPassword(password); + setPassword(password); result.setNeedsSave(true); } + result.setMatchesRepository(pwOk || legacyPwOk); return result; } diff --git a/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java index ec08d3e1..3955e2d1 100644 --- a/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java +++ b/gsrs-core/src/main/java/gsrs/util/GsrsPasswordHasher.java @@ -16,6 +16,8 @@ public class GsrsPasswordHasher implements Hasher { static int iterations = 1000; static String characterSet ="utf8"; + private final static String HASHING_ALGORITHM = "PBKDF2WithHmacSHA512"; + @Override public String getHashType() { return this.preferredHashAlgorithm; @@ -52,7 +54,7 @@ public static String toHex(byte[] d) { public static String hash(String input, String salt, int iterations) throws NoSuchAlgorithmException, InvalidKeySpecException, UnsupportedEncodingException { PBEKeySpec spec = new PBEKeySpec(input.toCharArray(), salt != null ? salt.getBytes(characterSet) : input.getBytes(characterSet), iterations, 64 * 8); - SecretKeyFactory skf = SecretKeyFactory.getInstance("PBKDF2WithHmacSHA1"); + SecretKeyFactory skf = SecretKeyFactory.getInstance(HASHING_ALGORITHM); byte[] hash = skf.generateSecret(spec).getEncoded(); return toHex(hash);