You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@syncope.apache.org by an...@apache.org on 2022/03/03 16:35:04 UTC
[syncope] branch 2_1_X updated: [SYNCOPE-1666] added security answer hashing (#319)
This is an automated email from the ASF dual-hosted git repository.
andreapatricelli pushed a commit to branch 2_1_X
in repository https://gitbox.apache.org/repos/asf/syncope.git
The following commit(s) were added to refs/heads/2_1_X by this push:
new 0084592 [SYNCOPE-1666] added security answer hashing (#319)
0084592 is described below
commit 008459240094e7d021553b484d47aa1569626250
Author: Andrea Patricelli <an...@apache.org>
AuthorDate: Thu Mar 3 17:34:55 2022 +0100
[SYNCOPE-1666] added security answer hashing (#319)
[SYNCOPE-1666] added security answer hashing
---
.../org/apache/syncope/core/logic/UserLogic.java | 31 +++---
.../core/persistence/api/entity/user/Account.java | 6 +-
.../core/persistence/api/entity/user/User.java | 4 +
.../jpa/entity/user/JPALinkedAccount.java | 21 +++-
.../core/persistence/jpa/entity/user/JPAUser.java | 49 +++++++--
.../persistence/jpa/inner/MultitenancyTest.java | 3 +-
.../core/persistence/jpa/inner/UserTest.java | 61 +++++++++--
.../core/persistence/jpa/outer/UserTest.java | 3 +-
.../core/provisioning/api/data/UserDataBinder.java | 2 -
.../core/provisioning/java/MappingManagerImpl.java | 2 +-
.../provisioning/java/data/UserDataBinderImpl.java | 119 +++++++++++++--------
.../java/propagation/DeletingLinkedAccount.java | 9 +-
.../provisioning/java/MappingManagerImplTest.java | 7 +-
.../core/spring/policy/DefaultPasswordRule.java | 2 +-
.../spring/policy/HaveIBeenPwnedPasswordRule.java | 2 +-
.../syncope/core/logic/UserRequestLogic.java | 2 +-
.../fit/core/reference/TestPasswordRule.java | 2 +-
.../configurationparameters.adoc | 3 +-
18 files changed, 236 insertions(+), 92 deletions(-)
diff --git a/core/logic/src/main/java/org/apache/syncope/core/logic/UserLogic.java b/core/logic/src/main/java/org/apache/syncope/core/logic/UserLogic.java
index ad59396..dd5ec19 100644
--- a/core/logic/src/main/java/org/apache/syncope/core/logic/UserLogic.java
+++ b/core/logic/src/main/java/org/apache/syncope/core/logic/UserLogic.java
@@ -60,6 +60,7 @@ import org.apache.syncope.core.provisioning.api.data.UserDataBinder;
import org.apache.syncope.core.provisioning.api.serialization.POJOHelper;
import org.apache.syncope.core.provisioning.api.utils.RealmUtils;
import org.apache.syncope.core.spring.security.AuthContextUtils;
+import org.apache.syncope.core.spring.security.Encryptor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.security.access.prepost.PreAuthorize;
import org.springframework.stereotype.Component;
@@ -107,14 +108,14 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
return Triple.of(
POJOHelper.serialize(AuthContextUtils.getAuthorizations()),
POJOHelper.serialize(delegationDAO.findValidDelegating(authenticatedUser.getKey())),
- binder.returnUserTO(authenticatedUser));
+ authenticatedUser);
}
@PreAuthorize("hasRole('" + StandardEntitlement.USER_READ + "')")
@Transactional(readOnly = true)
@Override
public UserTO read(final String key) {
- return binder.returnUserTO(binder.getUserTO(key));
+ return binder.getUserTO(key);
}
@PreAuthorize("hasRole('" + StandardEntitlement.USER_SEARCH + "')")
@@ -137,7 +138,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
List<User> matching = searchDAO.search(authRealms, effectiveCond, page, size, orderBy, AnyTypeKind.USER);
List<UserTO> result = matching.stream().
- map(user -> binder.returnUserTO(binder.getUserTO(user, details))).
+ map(user -> binder.getUserTO(user, details)).
collect(Collectors.toList());
return Pair.of(count, result);
@@ -189,8 +190,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
Pair<String, List<PropagationStatus>> created =
provisioningManager.create(before.getLeft(), storePassword, nullPriorityAsync);
- return afterCreate(
- binder.returnUserTO(binder.getUserTO(created.getKey())), created.getRight(), before.getRight());
+ return afterCreate(binder.getUserTO(created.getKey()), created.getRight(), before.getRight());
}
@PreAuthorize("isAuthenticated() "
@@ -246,7 +246,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
provisioningManager.update(before.getLeft(), nullPriorityAsync);
ProvisioningResult<UserTO> result = afterUpdate(
- binder.returnUserTO(binder.getUserTO(after.getLeft().getKey())),
+ binder.getUserTO(after.getLeft().getKey()),
after.getRight(),
before.getRight());
@@ -297,7 +297,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
Pair<String, List<PropagationStatus>> updated = setStatusOnWfAdapter(statusPatch, nullPriorityAsync);
return afterUpdate(
- binder.returnUserTO(binder.getUserTO(updated.getKey())),
+ binder.getUserTO(updated.getKey()),
updated.getRight(),
Collections.emptyList());
}
@@ -308,7 +308,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
Pair<String, List<PropagationStatus>> updated = setStatusOnWfAdapter(statusPatch, nullPriorityAsync);
return afterUpdate(
- binder.returnUserTO(binder.getUserTO(updated.getKey())),
+ binder.getUserTO(updated.getKey()),
updated.getRight(),
Collections.emptyList());
}
@@ -334,8 +334,9 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
}
if (syncopeLogic.isPwdResetRequiringSecurityQuestions()
- && (securityAnswer == null || !securityAnswer.equals(user.getSecurityAnswer()))) {
-
+ && (securityAnswer == null
+ || !Encryptor.getInstance().verify(securityAnswer, user.getCipherAlgorithm(),
+ user.getSecurityAnswer()))) {
throw SyncopeClientException.build(ClientExceptionType.InvalidSecurityAnswer);
}
@@ -399,7 +400,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
deletedTO = binder.getUserTO(before.getLeft().getKey());
}
- return afterDelete(binder.returnUserTO(deletedTO), statuses, before.getRight());
+ return afterDelete(deletedTO, statuses, before.getRight());
}
protected void updateChecks(final String key) {
@@ -428,7 +429,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
map(r -> new StringPatchItem.Builder().operation(PatchOperation.DELETE).value(r).build()).
collect(Collectors.toList()));
- return binder.returnUserTO(binder.getUserTO(provisioningManager.unlink(patch)));
+ return binder.getUserTO(provisioningManager.unlink(patch));
}
@PreAuthorize("hasRole('" + StandardEntitlement.USER_UPDATE + "')")
@@ -442,7 +443,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
map(r -> new StringPatchItem.Builder().operation(PatchOperation.ADD_REPLACE).value(r).build()).
collect(Collectors.toList()));
- return binder.returnUserTO(binder.getUserTO(provisioningManager.link(patch)));
+ return binder.getUserTO(provisioningManager.link(patch));
}
@PreAuthorize("hasRole('" + StandardEntitlement.USER_UPDATE + "')")
@@ -496,7 +497,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
List<PropagationStatus> statuses = provisioningManager.deprovision(key, resources, nullPriorityAsync);
ProvisioningResult<UserTO> result = new ProvisioningResult<>();
- result.setEntity(binder.returnUserTO(binder.getUserTO(key)));
+ result.setEntity(binder.getUserTO(key));
result.getPropagationStatuses().addAll(statuses);
return result;
}
@@ -516,7 +517,7 @@ public class UserLogic extends AbstractAnyLogic<UserTO, UserPatch> {
nullPriorityAsync);
ProvisioningResult<UserTO> result = new ProvisioningResult<>();
- result.setEntity(binder.returnUserTO(binder.getUserTO(key)));
+ result.setEntity(binder.getUserTO(key));
result.getPropagationStatuses().addAll(statuses);
return result;
}
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/Account.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/Account.java
index bbfe93e..dc34148 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/Account.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/Account.java
@@ -28,14 +28,16 @@ public interface Account {
CipherAlgorithm getCipherAlgorithm();
- boolean canDecodePassword();
+ boolean canDecodeSecrets();
String getPassword();
void setEncodedPassword(String password, CipherAlgorithm cipherAlgoritm);
- void setPassword(String password, CipherAlgorithm cipherAlgoritm);
+ void setPassword(String password);
+ void setCipherAlgorithm(CipherAlgorithm cipherAlgorithm);
+
Boolean isSuspended();
void setSuspended(Boolean suspended);
diff --git a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/User.java b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/User.java
index 458d415..8406759 100644
--- a/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/User.java
+++ b/core/persistence-api/src/main/java/org/apache/syncope/core/persistence/api/entity/user/User.java
@@ -54,7 +54,11 @@ public interface User extends Account, GroupableRelatable<User, UMembership, UPl
void setSecurityQuestion(SecurityQuestion securityQuestion);
String getSecurityAnswer();
+
+ String getClearSecurityAnswer();
+ void setEncodedSecurityAnswer(String securityAnswer);
+
void setSecurityAnswer(String securityAnswer);
Integer getFailedLogins();
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java
index dbd47e8..ef48ffa 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPALinkedAccount.java
@@ -39,6 +39,7 @@ import javax.persistence.UniqueConstraint;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import org.apache.syncope.common.lib.types.CipherAlgorithm;
+import org.apache.syncope.core.persistence.api.dao.ConfDAO;
import org.apache.syncope.core.persistence.api.entity.Privilege;
import org.apache.syncope.core.persistence.api.entity.resource.ExternalResource;
import org.apache.syncope.core.persistence.api.entity.user.LAPlainAttr;
@@ -47,6 +48,7 @@ import org.apache.syncope.core.persistence.api.entity.user.User;
import org.apache.syncope.core.persistence.jpa.entity.AbstractGeneratedKeyEntity;
import org.apache.syncope.core.persistence.jpa.entity.JPAPrivilege;
import org.apache.syncope.core.persistence.jpa.entity.resource.JPAExternalResource;
+import org.apache.syncope.core.spring.ApplicationContextProvider;
import org.apache.syncope.core.spring.security.Encryptor;
@Entity
@@ -141,7 +143,16 @@ public class JPALinkedAccount extends AbstractGeneratedKeyEntity implements Link
}
@Override
- public boolean canDecodePassword() {
+ public void setCipherAlgorithm(final CipherAlgorithm cipherAlgorithm) {
+ if (this.cipherAlgorithm == null || cipherAlgorithm == null) {
+ this.cipherAlgorithm = cipherAlgorithm;
+ } else {
+ throw new IllegalArgumentException("Cannot override existing cipher algorithm");
+ }
+ }
+
+ @Override
+ public boolean canDecodeSecrets() {
return this.cipherAlgorithm != null && this.cipherAlgorithm.isInvertible();
}
@@ -157,10 +168,12 @@ public class JPALinkedAccount extends AbstractGeneratedKeyEntity implements Link
}
@Override
- public void setPassword(final String password, final CipherAlgorithm cipherAlgoritm) {
+ public void setPassword(final String password) {
try {
- this.password = ENCRYPTOR.encode(password, cipherAlgoritm);
- this.cipherAlgorithm = cipherAlgoritm;
+ this.password = ENCRYPTOR.encode(password, cipherAlgorithm == null
+ ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfDAO.class).
+ find("password.cipher.algorithm", CipherAlgorithm.AES.name()))
+ : cipherAlgorithm);
} catch (Exception e) {
LOG.error("Could not encode password", e);
this.password = null;
diff --git a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java
index 3c93202..1e59265 100644
--- a/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java
+++ b/core/persistence-jpa/src/main/java/org/apache/syncope/core/persistence/jpa/entity/user/JPAUser.java
@@ -47,6 +47,7 @@ import javax.persistence.UniqueConstraint;
import javax.validation.Valid;
import javax.validation.constraints.NotNull;
import org.apache.syncope.common.lib.types.CipherAlgorithm;
+import org.apache.syncope.core.persistence.api.dao.ConfDAO;
import org.apache.syncope.core.persistence.api.entity.user.SecurityQuestion;
import org.apache.syncope.core.persistence.api.entity.user.UPlainAttr;
import org.apache.syncope.core.persistence.api.entity.user.User;
@@ -185,6 +186,9 @@ public class JPAUser
@Column(nullable = true)
private String securityAnswer;
+ @Transient
+ private String clearSecurityAnswer;
+
@OneToMany(cascade = CascadeType.ALL, orphanRemoval = true, mappedBy = "owner")
@Valid
private List<JPALinkedAccount> linkedAccounts = new ArrayList<>();
@@ -241,21 +245,23 @@ public class JPAUser
}
@Override
- public void setEncodedPassword(final String password, final CipherAlgorithm cipherAlgoritm) {
+ public void setEncodedPassword(final String password, final CipherAlgorithm cipherAlgorithm) {
this.clearPassword = null;
this.password = password;
- this.cipherAlgorithm = cipherAlgoritm;
+ this.cipherAlgorithm = cipherAlgorithm;
setMustChangePassword(false);
}
@Override
- public void setPassword(final String password, final CipherAlgorithm cipherAlgoritm) {
+ public void setPassword(final String password) {
this.clearPassword = password;
try {
- this.password = ENCRYPTOR.encode(password, cipherAlgoritm);
- this.cipherAlgorithm = cipherAlgoritm;
+ this.password = ENCRYPTOR.encode(password, cipherAlgorithm == null
+ ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfDAO.class).
+ find("password.cipher.algorithm", CipherAlgorithm.AES.name()))
+ : cipherAlgorithm);
setMustChangePassword(false);
} catch (Exception e) {
LOG.error("Could not encode password", e);
@@ -269,7 +275,16 @@ public class JPAUser
}
@Override
- public boolean canDecodePassword() {
+ public void setCipherAlgorithm(final CipherAlgorithm cipherAlgorithm) {
+ if (this.cipherAlgorithm == null || cipherAlgorithm == null) {
+ this.cipherAlgorithm = cipherAlgorithm;
+ } else {
+ throw new IllegalArgumentException("Cannot override existing cipher algorithm");
+ }
+ }
+
+ @Override
+ public boolean canDecodeSecrets() {
return this.cipherAlgorithm != null && this.cipherAlgorithm.isInvertible();
}
@@ -425,8 +440,30 @@ public class JPAUser
}
@Override
+ public String getClearSecurityAnswer() {
+ return clearSecurityAnswer;
+ }
+
+ @Override
+ public void setEncodedSecurityAnswer(final String securityAnswer) {
+ this.clearSecurityAnswer = null;
+
+ this.securityAnswer = securityAnswer;
+ }
+
+ @Override
public void setSecurityAnswer(final String securityAnswer) {
this.securityAnswer = securityAnswer;
+
+ try {
+ this.securityAnswer = ENCRYPTOR.encode(securityAnswer, cipherAlgorithm == null
+ ? CipherAlgorithm.valueOf(ApplicationContextProvider.getBeanFactory().getBean(ConfDAO.class).
+ find("password.cipher.algorithm", CipherAlgorithm.AES.name()))
+ : cipherAlgorithm);
+ } catch (Exception e) {
+ LOG.error("Could not encode security answer", e);
+ this.securityAnswer = null;
+ }
}
@Override
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/MultitenancyTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/MultitenancyTest.java
index 565b3b8..4f06a73 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/MultitenancyTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/MultitenancyTest.java
@@ -94,7 +94,8 @@ public class MultitenancyTest extends AbstractTest {
User user = entityFactory.newEntity(User.class);
user.setRealm(realmDAO.getRoot());
- user.setPassword("password", CipherAlgorithm.SHA256);
+ user.setPassword("password");
+ user.setCipherAlgorithm(CipherAlgorithm.SHA256);
user.setUsername("username");
User actual = userDAO.save(user);
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java
index a7272da..2b7e6e1 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/inner/UserTest.java
@@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
@@ -33,11 +34,13 @@ import org.apache.syncope.core.persistence.api.attrvalue.validation.InvalidEntit
import org.apache.syncope.core.persistence.api.dao.DerSchemaDAO;
import org.apache.syncope.core.persistence.api.dao.ExternalResourceDAO;
import org.apache.syncope.core.persistence.api.dao.PlainSchemaDAO;
+import org.apache.syncope.core.persistence.api.dao.SecurityQuestionDAO;
import org.apache.syncope.core.persistence.api.dao.UserDAO;
import org.apache.syncope.core.persistence.api.entity.user.UPlainAttrValue;
import org.apache.syncope.core.persistence.api.entity.user.User;
import org.apache.syncope.core.persistence.jpa.AbstractTest;
import org.apache.syncope.core.spring.policy.InvalidPasswordRuleConf;
+import org.apache.syncope.core.spring.security.Encryptor;
import org.apache.syncope.core.spring.security.PasswordGenerator;
import org.apache.syncope.core.persistence.api.dao.RealmDAO;
import org.apache.syncope.core.persistence.api.entity.PlainSchema;
@@ -68,6 +71,9 @@ public class UserTest extends AbstractTest {
@Autowired
private DerSchemaDAO derSchemaDAO;
+ @Autowired
+ private SecurityQuestionDAO securityQuestionDAO;
+
@Test
public void find() {
User user = userDAO.find("823074dc-d280-436d-a7dd-07399fae48ec");
@@ -202,7 +208,8 @@ public class UserTest extends AbstractTest {
user.setRealm(realmDAO.findByFullPath("/even/two"));
user.setCreator("admin");
user.setCreationDate(new Date());
- user.setPassword("pass", CipherAlgorithm.SHA256);
+ user.setCipherAlgorithm(CipherAlgorithm.SHA256);
+ user.setPassword("pass");
try {
userDAO.save(user);
@@ -219,7 +226,8 @@ public class UserTest extends AbstractTest {
user.setRealm(realmDAO.findByFullPath("/even/two"));
user.setCreator("admin");
user.setCreationDate(new Date());
- user.setPassword("password123", CipherAlgorithm.SHA256);
+ user.setCipherAlgorithm(CipherAlgorithm.SHA256);
+ user.setPassword("password123");
try {
userDAO.save(user);
@@ -236,7 +244,8 @@ public class UserTest extends AbstractTest {
user.setRealm(realmDAO.findByFullPath("/even/two"));
user.setCreator("admin");
user.setCreationDate(new Date());
- user.setPassword("password123", CipherAlgorithm.SHA256);
+ user.setCipherAlgorithm(CipherAlgorithm.SHA256);
+ user.setPassword("password123");
User actual = userDAO.save(user);
assertNotNull(actual);
@@ -263,7 +272,8 @@ public class UserTest extends AbstractTest {
user.setCreator("admin");
user.setCreationDate(new Date());
- user.setPassword("password123", CipherAlgorithm.AES);
+ user.setCipherAlgorithm(CipherAlgorithm.AES);
+ user.setPassword("password123");
User actual = userDAO.save(user);
assertNotNull(actual);
@@ -273,7 +283,8 @@ public class UserTest extends AbstractTest {
public void issueSYNCOPE391() {
User user = entityFactory.newEntity(User.class);
user.setUsername("username");
- user.setPassword(null, CipherAlgorithm.AES);
+ user.setCipherAlgorithm(CipherAlgorithm.AES);
+ user.setPassword(null);
user.setRealm(realmDAO.findByFullPath("/even/two"));
User actual = userDAO.save(user);
@@ -292,7 +303,45 @@ public class UserTest extends AbstractTest {
assertNotNull(password);
User user = userDAO.find("c9b2dec2-00a7-4855-97c0-d854842b4b24");
- user.setPassword(password, CipherAlgorithm.SHA);
+ user.setPassword(password);
userDAO.save(user);
}
+
+ @Test
+ public void passwordGeneratorFailing() {
+ assertThrows(IllegalArgumentException.class, () -> {
+ String password = "";
+ try {
+ password = passwordGenerator.generate(resourceDAO.find("ws-target-resource-nopropagation"));
+ } catch (InvalidPasswordRuleConf e) {
+ fail(e.getMessage());
+ }
+ assertNotNull(password);
+
+ User user = userDAO.find("c9b2dec2-00a7-4855-97c0-d854842b4b24");
+ // SYNCOPE-1666 fail because cipherAlgorithm is already set
+ user.setCipherAlgorithm(CipherAlgorithm.SHA);
+ user.setPassword(password);
+ userDAO.save(user);
+ });
+ }
+
+ @Test
+ public void issueSYNCOPE1666() {
+ User user = entityFactory.newEntity(User.class);
+ user.setUsername("username");
+ user.setRealm(realmDAO.findByFullPath("/even/two"));
+ user.setCreator("admin");
+ user.setCreationDate(new Date());
+ user.setCipherAlgorithm(CipherAlgorithm.SSHA256);
+ user.setPassword("password123");
+ user.setSecurityQuestion(securityQuestionDAO.find("887028ea-66fc-41e7-b397-620d7ea6dfbb"));
+ String securityAnswer = "my complex answer to @ $complex question è ? £12345";
+ user.setSecurityAnswer(securityAnswer);
+
+ User actual = userDAO.save(user);
+ assertNotNull(actual);
+ assertNotNull(actual.getSecurityAnswer());
+ assertTrue(Encryptor.getInstance().verify(securityAnswer, CipherAlgorithm.SSHA256, actual.getSecurityAnswer()));
+ }
}
diff --git a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/UserTest.java b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/UserTest.java
index e3093ac..03e498d 100644
--- a/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/UserTest.java
+++ b/core/persistence-jpa/src/test/java/org/apache/syncope/core/persistence/jpa/outer/UserTest.java
@@ -256,7 +256,8 @@ public class UserTest extends AbstractTest {
account.add(applicationDAO.findPrivilege("getMighty"));
account.setUsername(UUID.randomUUID().toString());
- account.setPassword("Password123", CipherAlgorithm.AES);
+ account.setCipherAlgorithm(CipherAlgorithm.AES);
+ account.setPassword("Password123");
AnyUtils anyUtils = anyUtilsFactory.getLinkedAccountInstance();
LAPlainAttr attr = anyUtils.newPlainAttr();
diff --git a/core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/data/UserDataBinder.java b/core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/data/UserDataBinder.java
index 61bf2ff..6877eca 100644
--- a/core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/data/UserDataBinder.java
+++ b/core/provisioning-api/src/main/java/org/apache/syncope/core/provisioning/api/data/UserDataBinder.java
@@ -28,8 +28,6 @@ import org.apache.syncope.core.persistence.api.entity.user.User;
public interface UserDataBinder {
- UserTO returnUserTO(UserTO userTO);
-
UserTO getAuthenticatedUserTO();
UserTO getUserTO(String key);
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/MappingManagerImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/MappingManagerImpl.java
index 79b5d59..afaa464 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/MappingManagerImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/MappingManagerImpl.java
@@ -497,7 +497,7 @@ public class MappingManagerImpl implements MappingManager {
} else {
if (StringUtils.isNotBlank(defaultValue)) {
passwordAttrValue = defaultValue;
- } else if (account.canDecodePassword()) {
+ } else if (account.canDecodeSecrets()) {
passwordAttrValue = decodePassword(account);
} else {
passwordAttrValue = null;
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
index e116afd..058557d 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/data/UserDataBinderImpl.java
@@ -50,6 +50,7 @@ import org.apache.syncope.core.persistence.api.dao.AccessTokenDAO;
import org.apache.syncope.core.persistence.api.dao.ConfDAO;
import org.apache.syncope.core.persistence.api.dao.SecurityQuestionDAO;
import org.apache.syncope.core.persistence.api.entity.group.Group;
+import org.apache.syncope.core.persistence.api.entity.user.Account;
import org.apache.syncope.core.persistence.api.entity.user.SecurityQuestion;
import org.apache.syncope.core.persistence.api.entity.user.User;
import org.apache.syncope.core.provisioning.api.PropagationByResource;
@@ -112,16 +113,6 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
@Transactional(readOnly = true)
@Override
- public UserTO returnUserTO(final UserTO userTO) {
- if (!confDAO.find("return.password.value", false)) {
- userTO.setPassword(null);
- userTO.getLinkedAccounts().forEach(account -> account.setPassword(null));
- }
- return userTO;
- }
-
- @Transactional(readOnly = true)
- @Override
public UserTO getAuthenticatedUserTO() {
UserTO authUserTO;
@@ -144,17 +135,42 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
private void setPassword(final User user, final String password, final SyncopeClientCompositeException scce) {
try {
- String algorithm = confDAO.find("password.cipher.algorithm", CipherAlgorithm.AES.name());
- user.setPassword(password, CipherAlgorithm.valueOf(algorithm));
+ setCipherAlgorithm(user);
+ user.setPassword(password);
} catch (IllegalArgumentException e) {
- SyncopeClientException invalidCiperAlgorithm = SyncopeClientException.build(ClientExceptionType.NotFound);
- invalidCiperAlgorithm.getElements().add(e.getMessage());
- scce.addException(invalidCiperAlgorithm);
+ throw aggregateException(scce, e, ClientExceptionType.NotFound);
+ }
+ }
- throw scce;
+ private void setSecurityAnswer(
+ final User user,
+ final String securityAnswer,
+ final SyncopeClientCompositeException scce) {
+ try {
+ setCipherAlgorithm(user);
+ user.setSecurityAnswer(securityAnswer);
+ } catch (IllegalArgumentException e) {
+ throw aggregateException(scce, e, ClientExceptionType.NotFound);
}
}
+ private void setCipherAlgorithm(final Account account) {
+ if (account.getCipherAlgorithm() == null) {
+ account.setCipherAlgorithm(
+ CipherAlgorithm.valueOf(confDAO.find("password.cipher.algorithm", CipherAlgorithm.AES.name())));
+ }
+ }
+
+ private RuntimeException aggregateException(
+ final SyncopeClientCompositeException scce,
+ final RuntimeException e,
+ final ClientExceptionType clientExceptionType) {
+ SyncopeClientException sce = SyncopeClientException.build(clientExceptionType);
+ sce.getElements().add(e.getMessage());
+ scce.addException(sce);
+ return scce;
+ }
+
private void linkedAccount(
final User user,
final LinkedAccountTO accountTO,
@@ -188,7 +204,8 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
if (StringUtils.isBlank(accountTO.getPassword())) {
account.setEncodedPassword(null, null);
} else if (!accountTO.getPassword().equals(account.getPassword())) {
- account.setPassword(accountTO.getPassword(), CipherAlgorithm.AES);
+ setCipherAlgorithm(account);
+ account.setPassword(accountTO.getPassword());
}
account.setSuspended(accountTO.isSuspended());
@@ -225,6 +242,28 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
}
}
+ private LinkedAccountTO getLinkedAccountTO(final LinkedAccount account, final boolean returnPasswordValue) {
+ LinkedAccountTO accountTO = new LinkedAccountTO.Builder(
+ account.getKey(), account.getResource().getKey(), account.getConnObjectKeyValue()).
+ username(account.getUsername()).
+ suspended(BooleanUtils.isTrue(account.isSuspended())).
+ build();
+ if (returnPasswordValue) {
+ accountTO.setPassword(account.getPassword());
+ }
+
+ account.getPlainAttrs().forEach(plainAttr -> {
+ accountTO.getPlainAttrs().add(new AttrTO.Builder().
+ schema(plainAttr.getSchema().getKey()).
+ values(plainAttr.getValuesAsStrings()).build());
+ });
+
+ accountTO.getPrivileges().addAll(account.getPrivileges().stream().
+ map(Entity::getKey).collect(Collectors.toList()));
+
+ return accountTO;
+ }
+
@Override
public void create(final User user, final UserTO userTO, final boolean storePassword) {
SyncopeClientCompositeException scce = SyncopeClientException.buildComposite();
@@ -249,7 +288,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
user.setSecurityQuestion(securityQuestion);
}
}
- user.setSecurityAnswer(userTO.getSecurityAnswer());
+ setSecurityAnswer(user, userTO.getSecurityAnswer(), scce);
// roles
userTO.getRoles().forEach(roleKey -> {
@@ -440,7 +479,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
securityQuestionDAO.find(userPatch.getSecurityQuestion().getValue());
if (securityQuestion != null) {
user.setSecurityQuestion(securityQuestion);
- user.setSecurityAnswer(userPatch.getSecurityAnswer().getValue());
+ setSecurityAnswer(user, userPatch.getSecurityAnswer().getValue(), scce);
}
}
}
@@ -594,7 +633,7 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
// SYNCOPE-686: if password is invertible and we are adding resources with password mapping,
// ensure that they are counted for password propagation
- if (toBeUpdated.canDecodePassword()) {
+ if (toBeUpdated.canDecodeSecrets()) {
if (userPatch.getPassword() == null) {
userPatch.setPassword(new PasswordPatch());
}
@@ -681,32 +720,21 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
@Transactional(readOnly = true)
@Override
public LinkedAccountTO getLinkedAccountTO(final LinkedAccount account) {
- LinkedAccountTO accountTO = new LinkedAccountTO.Builder(
- account.getKey(), account.getResource().getKey(), account.getConnObjectKeyValue()).
- username(account.getUsername()).
- password(account.getPassword()).
- suspended(BooleanUtils.isTrue(account.isSuspended())).
- build();
-
- account.getPlainAttrs().forEach(plainAttr -> {
- accountTO.getPlainAttrs().add(new AttrTO.Builder().
- schema(plainAttr.getSchema().getKey()).
- values(plainAttr.getValuesAsStrings()).build());
- });
-
- accountTO.getPrivileges().addAll(account.getPrivileges().stream().
- map(Entity::getKey).collect(Collectors.toList()));
-
- return accountTO;
+ return getLinkedAccountTO(account, true);
}
@Transactional(readOnly = true)
@Override
public UserTO getUserTO(final User user, final boolean details) {
+ boolean returnPasswordValue = confDAO.find("return.password.value", false);
+
UserTO userTO = new UserTO();
userTO.setKey(user.getKey());
userTO.setUsername(user.getUsername());
- userTO.setPassword(user.getPassword());
+ if (returnPasswordValue) {
+ userTO.setPassword(user.getPassword());
+ userTO.setSecurityAnswer(user.getSecurityAnswer());
+ }
userTO.setType(user.getType().getKey());
userTO.setCreationDate(user.getCreationDate());
userTO.setCreator(user.getCreator());
@@ -756,20 +784,21 @@ public class UserDataBinderImpl extends AbstractAnyDataBinder implements UserDat
// memberships
userTO.getMemberships().addAll(
user.getMemberships().stream().map(membership -> getMembershipTO(
- user.getPlainAttrs(membership),
- derAttrHandler.getValues(user, membership),
- virAttrHandler.getValues(user, membership),
- membership)).collect(Collectors.toList()));
+ user.getPlainAttrs(membership),
+ derAttrHandler.getValues(user, membership),
+ virAttrHandler.getValues(user, membership),
+ membership)).collect(Collectors.toList()));
// dynamic memberships
userTO.getDynMemberships().addAll(
userDAO.findDynGroups(user.getKey()).stream().map(group -> new MembershipTO.Builder().
- group(group.getKey(), group.getName()).
- build()).collect(Collectors.toList()));
+ group(group.getKey(), group.getName()).
+ build()).collect(Collectors.toList()));
// linked accounts
userTO.getLinkedAccounts().addAll(
- user.getLinkedAccounts().stream().map(this::getLinkedAccountTO).collect(Collectors.toList()));
+ user.getLinkedAccounts().stream().map(a -> getLinkedAccountTO(a, returnPasswordValue))
+ .collect(Collectors.toList()));
// delegations
userTO.getDelegatingDelegations().addAll(
diff --git a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/DeletingLinkedAccount.java b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/DeletingLinkedAccount.java
index c96eebc..c456da7 100644
--- a/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/DeletingLinkedAccount.java
+++ b/core/provisioning-java/src/main/java/org/apache/syncope/core/provisioning/java/propagation/DeletingLinkedAccount.java
@@ -108,7 +108,12 @@ public class DeletingLinkedAccount implements LinkedAccount {
}
@Override
- public boolean canDecodePassword() {
+ public void setCipherAlgorithm(final CipherAlgorithm cipherAlgorithm) {
+ // unsupported
+ }
+
+ @Override
+ public boolean canDecodeSecrets() {
return false;
}
@@ -123,7 +128,7 @@ public class DeletingLinkedAccount implements LinkedAccount {
}
@Override
- public void setPassword(final String password, final CipherAlgorithm cipherAlgoritm) {
+ public void setPassword(final String password) {
// unsupported
}
diff --git a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/MappingManagerImplTest.java b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/MappingManagerImplTest.java
index f18a116..527ace7 100644
--- a/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/MappingManagerImplTest.java
+++ b/core/provisioning-java/src/test/java/org/apache/syncope/core/provisioning/java/MappingManagerImplTest.java
@@ -41,6 +41,7 @@ import org.identityconnectors.framework.common.objects.AttributeUtil;
import org.identityconnectors.framework.common.objects.OperationalAttributes;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.test.util.ReflectionTestUtils;
import org.springframework.transaction.annotation.Transactional;
@Transactional("Master")
@@ -116,7 +117,8 @@ public class MappingManagerImplTest extends AbstractTest {
assertNull(AttributeUtil.getPasswordValue(attrs.getRight()));
// 5. with no clear-text password, random password generation disabled but AES
- bellini.setPassword("newPassword123", CipherAlgorithm.AES);
+ ReflectionTestUtils.setField(bellini, "cipherAlgorithm", CipherAlgorithm.AES);
+ bellini.setPassword("newPassword123");
userDAO.save(bellini);
entityManager().flush();
@@ -142,7 +144,8 @@ public class MappingManagerImplTest extends AbstractTest {
account.setResource(ldap);
account.setOwner(vivaldi);
account.setSuspended(false);
- account.setPassword("Password321", CipherAlgorithm.AES);
+ account.setCipherAlgorithm(CipherAlgorithm.AES);
+ account.setPassword("Password321");
vivaldi.add(account);
vivaldi = userDAO.save(vivaldi);
diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java
index 4326075..cd8d60f 100644
--- a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java
+++ b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java
@@ -207,7 +207,7 @@ public class DefaultPasswordRule implements PasswordRule {
if (account.getPassword() != null) {
String clear = null;
- if (account.canDecodePassword()) {
+ if (account.canDecodeSecrets()) {
try {
clear = ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm());
} catch (Exception e) {
diff --git a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java
index 4a2bd31..a843b0c 100644
--- a/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java
+++ b/core/spring/src/main/java/org/apache/syncope/core/spring/policy/HaveIBeenPwnedPasswordRule.java
@@ -109,7 +109,7 @@ public class HaveIBeenPwnedPasswordRule implements PasswordRule {
public void enforce(final LinkedAccount account) {
if (account.getPassword() != null) {
String clear = null;
- if (account.canDecodePassword()) {
+ if (account.canDecodeSecrets()) {
try {
clear = ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm());
} catch (Exception e) {
diff --git a/ext/flowable/logic/src/main/java/org/apache/syncope/core/logic/UserRequestLogic.java b/ext/flowable/logic/src/main/java/org/apache/syncope/core/logic/UserRequestLogic.java
index 507b5d9..16a75be 100644
--- a/ext/flowable/logic/src/main/java/org/apache/syncope/core/logic/UserRequestLogic.java
+++ b/ext/flowable/logic/src/main/java/org/apache/syncope/core/logic/UserRequestLogic.java
@@ -237,7 +237,7 @@ public class UserRequestLogic extends AbstractTransactionalLogic<EntityTO> {
} else {
userTO = binder.getUserTO(wfResult.getResult().getKey());
}
- result.setEntity(binder.returnUserTO(userTO));
+ result.setEntity(userTO);
return result;
}
diff --git a/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestPasswordRule.java b/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestPasswordRule.java
index df61508..2a3eb71 100644
--- a/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestPasswordRule.java
+++ b/fit/core-reference/src/main/java/org/apache/syncope/fit/core/reference/TestPasswordRule.java
@@ -66,7 +66,7 @@ public class TestPasswordRule implements PasswordRule {
public void enforce(final LinkedAccount account) {
if (account.getPassword() != null) {
String clear = null;
- if (account.canDecodePassword()) {
+ if (account.canDecodeSecrets()) {
try {
clear = ENCRYPTOR.decode(account.getPassword(), account.getCipherAlgorithm());
} catch (Exception e) {
diff --git a/src/main/asciidoc/reference-guide/workingwithapachesyncope/systemadministration/configurationparameters.adoc b/src/main/asciidoc/reference-guide/workingwithapachesyncope/systemadministration/configurationparameters.adoc
index 5dedb84..d621d04 100644
--- a/src/main/asciidoc/reference-guide/workingwithapachesyncope/systemadministration/configurationparameters.adoc
+++ b/src/main/asciidoc/reference-guide/workingwithapachesyncope/systemadministration/configurationparameters.adoc
@@ -56,7 +56,8 @@ mechanism to work properly;
[WARNING]
Suspended Users are anyway not allowed to authenticate.
* `log.lastlogindate` - whether the system updates the `lastLoginDate` field of users upon authentication;
-* `return.password.value` - whether the hashed password value shall be returned when reading users;
+* `return.password.value` - whether the hashed password value and the hashed security answer (if any) value shall be
+returned when reading users;
* `connector.test.timeout` - timeout (in seconds) to check connector connection in <<Admin Console>>;
`0` to skip any check;