ilgrosso commented on code in PR #1432:
URL: https://github.com/apache/syncope/pull/1432#discussion_r3453388312
##########
core/spring/src/main/java/org/apache/syncope/core/spring/policy/DefaultPasswordRule.java:
##########
@@ -89,8 +89,8 @@ public void setConf(final PasswordRuleConf conf) {
protected void enforce(final String username, final String clearPassword,
final Collection<String> notPermitted) {
List<Rule> rules = PasswordGenerator.conf2Rules(conf);
if (!notPermitted.isEmpty()) {
- rules.add(new DictionaryRule(new WordListDictionary(new
ArrayWordList(
-
notPermitted.stream().distinct().sorted(Comparator.naturalOrder()).toArray(String[]::new),
true)),
+ rules.add(new DictionarySubstringRule(new WordListDictionary(new
ArrayWordList(
+
notPermitted.stream().distinct().sorted(Comparator.naturalOrder()).toArray(String[]::new),
false)),
Review Comment:
Why `false` by default here?
Since you are introducing case insensitive checks, I would expect that the
policy rule has a flag to configure this, and also that such new flag is
applied everywhere it makes sense.
##########
core/spring/src/main/java/org/apache/syncope/core/spring/security/PasswordGenerator.java:
##########
@@ -96,8 +96,8 @@ public String getCharacters() {
if (!conf.getWordsNotPermitted().isEmpty()) {
conf.getWordsNotPermitted().sort(Comparator.naturalOrder());
- rules.add(new DictionaryRule(new WordListDictionary(
- new
ArrayWordList(conf.getWordsNotPermitted().toArray(String[]::new), true)),
true));
+ rules.add(new DictionarySubstringRule((new WordListDictionary(
+ new
ArrayWordList(conf.getWordsNotPermitted().toArray(String[]::new), false))),
true));
Review Comment:
See above.
##########
fit/core-reference/src/test/java/org/apache/syncope/fit/core/PolicyITCase.java:
##########
@@ -482,4 +484,40 @@ public void issueSYNCOPE682() {
RESOURCE_SERVICE.update(ldap);
}
}
+
+ @Test
+ public void issueSYNCOPE1979() {
+ // 1. Set a new password policy with not permitted schemas and not
permitted words
+ ImplementationTO originalRule =
+
IMPLEMENTATION_SERVICE.read(IdRepoImplementationType.PASSWORD_RULE,
"DefaultPasswordRuleConf2");
+ DefaultPasswordRuleConf defaultPasswordRuleConf =
+ POJOHelper.deserialize(originalRule.getBody(),
DefaultPasswordRuleConf.class);
+ defaultPasswordRuleConf.getSchemasNotPermitted().add("firstname");
+ originalRule.setBody(POJOHelper.serialize(defaultPasswordRuleConf));
+ IMPLEMENTATION_SERVICE.update(originalRule);
+ try {
+ UserCR userCR =
UserITCase.getUniqueSample("[email protected]");
+ // 1. set password with not permitted word inside
+ userCR.setPassword("Notpermitted12345!");
+ try {
+ createUser(userCR);
+ fail("This should not happen");
+ } catch (SyncopeClientException e) {
+ assertEquals(ClientExceptionType.InvalidUser, e.getType());
+
assertTrue(e.getElements().iterator().next().startsWith("InvalidPassword"));
+ }
+ // 2. set password with not permitted schema inside
+
userCR.setPassword(userCR.getPlainAttr("firstname").get().getValues().getFirst()
+ "12345!");
+ try {
Review Comment:
same here
##########
fit/core-reference/src/test/java/org/apache/syncope/fit/core/PolicyITCase.java:
##########
@@ -482,4 +484,40 @@ public void issueSYNCOPE682() {
RESOURCE_SERVICE.update(ldap);
}
}
+
+ @Test
+ public void issueSYNCOPE1979() {
+ // 1. Set a new password policy with not permitted schemas and not
permitted words
+ ImplementationTO originalRule =
+
IMPLEMENTATION_SERVICE.read(IdRepoImplementationType.PASSWORD_RULE,
"DefaultPasswordRuleConf2");
+ DefaultPasswordRuleConf defaultPasswordRuleConf =
+ POJOHelper.deserialize(originalRule.getBody(),
DefaultPasswordRuleConf.class);
+ defaultPasswordRuleConf.getSchemasNotPermitted().add("firstname");
+ originalRule.setBody(POJOHelper.serialize(defaultPasswordRuleConf));
+ IMPLEMENTATION_SERVICE.update(originalRule);
+ try {
+ UserCR userCR =
UserITCase.getUniqueSample("[email protected]");
+ // 1. set password with not permitted word inside
+ userCR.setPassword("Notpermitted12345!");
+ try {
Review Comment:
Use `assertThrows()`, not the `try / catch` block
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
