gianpaolo-pestilli opened a new pull request, #1606:
URL: https://github.com/apache/syncope/pull/1606
## Problem
`DefaultPasswordGenerator.merge(List<DefaultPasswordRuleConf>)` always
returns
a configuration with `usernameAllowed == true`, even when every input
configuration has `usernameAllowed == false`.
## How to reproduce
The test added in this PR fails on current `master` without the fix:
```java
@Test
void allForbid() {
List<DefaultPasswordRuleConf> rules = new ArrayList();
rules.add(configRule(false));
rules.add(configRule(false));
assertFalse(passwordGenerator.merge(rules).isUsernameAllowed());
}
```
## Cause
In `merge`, `result` is initialized with `result.setUsernameAllowed(true)`
and is never set to `false` before the following check
https://github.com/apache/syncope/blob/193c94e019034071f13de4875d006377a3c5dac0/core/spring/src/main/java/org/apache/syncope/core/spring/security/DefaultPasswordGenerator.java#L153-L155
```java
if (!result.isUsernameAllowed()) {
result.setUsernameAllowed(ruleConf.isUsernameAllowed());
}
```
The condition can therefore never be true, and the value of
`usernameAllowed` in the input configurations is never read.
## Proposed fix
Remove the negation, so that `usernameAllowed` stays `true` only if all the
configurations allow it (consistently with the other fields, where the most
restrictive value wins):
```java
if (result.isUsernameAllowed()) {
result.setUsernameAllowed(ruleConf.isUsernameAllowed());
}
```
## Tests
Added `PasswordGeneratorTest.allForbid()`: with
two configurations both having `usernameAllowed == false`, `merge()` must
return `usernameAllowed == false`. It fails without the fix and passes with
it.
## Note
If the current behavior is intended, please let me know and I will adjust
or close this PR.
cc @gulyx
--
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]
