Skip to content

Commit f7a03b8

Browse files
jzheauxjzheaux
committed
Improvements
- Improved class names - Introduced defaults to guide usage
1 parent f9fc34a commit f7a03b8

File tree

15 files changed

+144
-131
lines changed

15 files changed

+144
-131
lines changed

‎config/src/test/java/org/springframework/security/config/annotation/web/configurers/PasswordManagementConfigurerTests.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -288,10 +288,15 @@ PasswordAdvice index(PasswordAdvice advice) {
288288
ResponseEntity<?> changePassword(@AuthenticationPrincipal UserDetails user,
289289
@RequestParam("password") String password, HttpServletRequest request, HttpServletResponse response) {
290290
PasswordAdvice advice = this.passwordAdvisor.advise(user, null, password);
291-
if (advice.getAction() != PasswordAction.ABSTAIN) {
291+
if (advice.getAction() != PasswordAction.NONE) {
292292
return ResponseEntity.badRequest().body(advice);
293293
}
294-
this.passwords.changePassword(null, this.encoder.encode(password));
294+
UserDetails updated = User.withUserDetails(user)
295+
.passwordEncoder(this.encoder::encode)
296+
.password(password)
297+
.passwordAction(PasswordAction.NONE)
298+
.build();
299+
this.passwords.updateUser(updated);
295300
this.passwordAdviceRepository.removePasswordAdvice(request, response);
296301
return ResponseEntity.ok().build();
297302
}

‎core/src/main/java/org/springframework/security/authentication/password/CompositePasswordAdvisor.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,45 +18,55 @@
1818

1919
import java.util.Collection;
2020
import java.util.Collections;
21+
import java.util.HashMap;
2122
import java.util.List;
23+
import java.util.Map;
2224

2325
import org.jspecify.annotations.Nullable;
2426

2527
import org.springframework.security.core.userdetails.UserDetails;
2628

2729
public final class CompositePasswordAdvisor implements PasswordAdvisor {
2830

29-
private final List<PasswordAdvisor> advisors;
31+
private final Collection<PasswordAdvisor> advisors;
3032

31-
private CompositePasswordAdvisor(List<PasswordAdvisor> advisors) {
32-
this.advisors = Collections.unmodifiableList(advisors);
33+
private CompositePasswordAdvisor(Collection<PasswordAdvisor> advisors) {
34+
this.advisors = Collections.unmodifiableCollection(advisors);
3335
}
3436

3537
public static PasswordAdvisor of(PasswordAdvisor... advisors) {
3638
return new CompositePasswordAdvisor(List.of(advisors));
3739
}
3840

41+
public static PasswordAdvisor withDefaults(PasswordAdvisor... advisors) {
42+
Map<Class<? extends PasswordAdvisor>, PasswordAdvisor> defaults = new HashMap<>();
43+
defaults.put(UserDetailsPasswordAdvisor.class, new UserDetailsPasswordAdvisor());
44+
defaults.put(PasswordLengthAdvisor.class, new PasswordLengthAdvisor());
45+
for (PasswordAdvisor advisor : advisors) {
46+
defaults.put(advisor.getClass(), advisor);
47+
}
48+
return new CompositePasswordAdvisor(defaults.values());
49+
}
50+
3951
@Override
4052
public PasswordAdvice advise(UserDetails user, @Nullable String password) {
4153
Collection<PasswordAdvice> advice = this.advisors.stream()
4254
.map((advisor) -> advisor.advise(user, password))
4355
.toList();
44-
return new Advice(advice);
56+
return new CompositePasswordAdvice(advice);
4557
}
4658

47-
public static final class Advice implements PasswordAdvice {
48-
49-
private final PasswordAction action;
59+
public static final class CompositePasswordAdvice extends SimplePasswordAdvice {
5060

5161
private final Collection<PasswordAdvice> advice;
5262

53-
private Advice(Collection<PasswordAdvice> advice) {
54-
this.action = findMostUrgentAction(advice);
63+
private CompositePasswordAdvice(Collection<PasswordAdvice> advice) {
64+
super(findMostUrgentAction(advice));
5565
this.advice = advice;
5666
}
5767

58-
private PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
59-
PasswordAction mostUrgentAction = PasswordAction.ABSTAIN;
68+
private static PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
69+
PasswordAction mostUrgentAction = PasswordAction.NONE;
6070
for (PasswordAdvice a : advice) {
6171
if (mostUrgentAction.ordinal() < a.getAction().ordinal()) {
6272
mostUrgentAction = a.getAction();
@@ -65,18 +75,13 @@ private PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
6575
return mostUrgentAction;
6676
}
6777

68-
@Override
69-
public PasswordAction getAction() {
70-
return this.action;
71-
}
72-
7378
public Collection<PasswordAdvice> getAdvice() {
7479
return this.advice;
7580
}
7681

7782
@Override
7883
public String toString() {
79-
return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]";
84+
return "Composite [" + "action=" + super.toString() + ", advice=" + this.advice + "]";
8085
}
8186

8287
}

‎core/src/main/java/org/springframework/security/authentication/password/CompositeUpdatePasswordAdvisor.java

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,45 +18,54 @@
1818

1919
import java.util.Collection;
2020
import java.util.Collections;
21+
import java.util.HashMap;
2122
import java.util.List;
23+
import java.util.Map;
2224

2325
import org.jspecify.annotations.Nullable;
2426

2527
import org.springframework.security.core.userdetails.UserDetails;
2628

2729
public final class CompositeUpdatePasswordAdvisor implements UpdatePasswordAdvisor {
2830

29-
private final List<UpdatePasswordAdvisor> advisors;
31+
private final Collection<UpdatePasswordAdvisor> advisors;
3032

31-
private CompositeUpdatePasswordAdvisor(List<UpdatePasswordAdvisor> advisors) {
32-
this.advisors = Collections.unmodifiableList(advisors);
33+
private CompositeUpdatePasswordAdvisor(Collection<UpdatePasswordAdvisor> advisors) {
34+
this.advisors = Collections.unmodifiableCollection(advisors);
3335
}
3436

3537
public static UpdatePasswordAdvisor of(UpdatePasswordAdvisor... advisors) {
3638
return new CompositeUpdatePasswordAdvisor(List.of(advisors));
3739
}
3840

41+
public static UpdatePasswordAdvisor withDefaults(UpdatePasswordAdvisor... advisors) {
42+
Map<Class<? extends UpdatePasswordAdvisor>, UpdatePasswordAdvisor> defaults = new HashMap<>();
43+
defaults.put(RepeatedPasswordAdvisor.class, new RepeatedPasswordAdvisor());
44+
defaults.put(PasswordLengthAdvisor.class, new PasswordLengthAdvisor());
45+
for (UpdatePasswordAdvisor advisor : advisors) {
46+
defaults.put(advisor.getClass(), advisor);
47+
}
48+
return new CompositeUpdatePasswordAdvisor(defaults.values());
49+
}
3950
@Override
4051
public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) {
4152
Collection<PasswordAdvice> advice = this.advisors.stream()
4253
.map((advisor) -> advisor.advise(user, oldPassword, newPassword))
4354
.toList();
44-
return new CompositeUpdatePasswordAdvisor.Advice(advice);
55+
return new CompositePasswordAdvice(advice);
4556
}
4657

47-
public static final class Advice implements PasswordAdvice {
48-
49-
private final PasswordAction action;
58+
public static final class CompositePasswordAdvice extends SimplePasswordAdvice {
5059

5160
private final Collection<PasswordAdvice> advice;
5261

53-
private Advice(Collection<PasswordAdvice> advice) {
54-
this.action = findMostUrgentAction(advice);
62+
private CompositePasswordAdvice(Collection<PasswordAdvice> advice) {
63+
super(findMostUrgentAction(advice));
5564
this.advice = advice;
5665
}
5766

58-
private PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
59-
PasswordAction mostUrgentAction = PasswordAction.ABSTAIN;
67+
private static PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
68+
PasswordAction mostUrgentAction = PasswordAction.NONE;
6069
for (PasswordAdvice a : advice) {
6170
if (mostUrgentAction.ordinal() < a.getAction().ordinal()) {
6271
mostUrgentAction = a.getAction();
@@ -65,18 +74,13 @@ private PasswordAction findMostUrgentAction(Collection<PasswordAdvice> advice) {
6574
return mostUrgentAction;
6675
}
6776

68-
@Override
69-
public PasswordAction getAction() {
70-
return this.action;
71-
}
72-
7377
public Collection<PasswordAdvice> getAdvice() {
7478
return this.advice;
7579
}
7680

7781
@Override
7882
public String toString() {
79-
return "Composite [" + "action=" + this.action + ", advice=" + this.advice + "]";
83+
return "Composite [" + "action=" + super.toString() + ", advice=" + this.advice + "]";
8084
}
8185

8286
}

‎core/src/main/java/org/springframework/security/authentication/password/PasswordAction.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,9 @@
1818

1919
public enum PasswordAction {
2020

21-
ABSTAIN, SHOULD_CHANGE, MUST_CHANGE;
21+
NONE, SHOULD_CHANGE, MUST_CHANGE;
2222

23-
boolean advisedBy(PasswordAdvice advice) {
23+
public boolean advisedBy(PasswordAdvice advice) {
2424
return advice.getAction().equals(this);
2525
}
2626

‎core/src/main/java/org/springframework/security/authentication/password/PasswordAdvice.java

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@
2121
@NullMarked
2222
public interface PasswordAdvice {
2323

24-
PasswordAdvice ABSTAIN = () -> PasswordAction.ABSTAIN;
25-
2624
PasswordAction getAction();
2725

2826
}

‎core/src/main/java/org/springframework/security/authentication/password/LengthPasswordAdvisor.java renamed to ‎core/src/main/java/org/springframework/security/authentication/password/PasswordLengthAdvisor.java

Lines changed: 28 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -21,25 +21,31 @@
2121
import org.springframework.security.core.userdetails.UserDetails;
2222
import org.springframework.util.Assert;
2323

24-
public final class LengthPasswordAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor {
24+
public final class PasswordLengthAdvisor implements PasswordAdvisor, UpdatePasswordAdvisor {
25+
26+
/**
27+
* The ASVS v5.0 minimum password length
28+
* @see <a href=
29+
* "https://github.com/OWASP/ASVS/blob/v5.0.0/5.0/docs_en/OWASP_Application_Security_Verification_Standard_5.0.0_en.csv#L108">ASVS
30+
* 5.0 Password Security Standard</a>
31+
*/
32+
private static final int ASVS_V5_MINIMUM_PASSWORD_LENGTH = 8;
2533

2634
private final int minLength;
2735

2836
private final int maxLength;
2937

30-
private PasswordAction tooShortAction = PasswordAction.MUST_CHANGE;
31-
32-
private PasswordAction tooLongAction = PasswordAction.SHOULD_CHANGE;
38+
private PasswordAction passwordAction = PasswordAction.SHOULD_CHANGE;
3339

34-
public LengthPasswordAdvisor() {
35-
this(12, 64);
40+
public PasswordLengthAdvisor() {
41+
this(ASVS_V5_MINIMUM_PASSWORD_LENGTH);
3642
}
3743

38-
public LengthPasswordAdvisor(int minLength) {
44+
public PasswordLengthAdvisor(int minLength) {
3945
this(minLength, Integer.MAX_VALUE);
4046
}
4147

42-
public LengthPasswordAdvisor(int minLength, int maxLength) {
48+
public PasswordLengthAdvisor(int minLength, int maxLength) {
4349
Assert.isTrue(minLength > 0, "minLength must be greater than 0");
4450
this.minLength = minLength;
4551
this.maxLength = maxLength;
@@ -48,82 +54,43 @@ public LengthPasswordAdvisor(int minLength, int maxLength) {
4854
@Override
4955
public PasswordAdvice advise(UserDetails user, @Nullable String password) {
5056
if (password == null) {
51-
return new TooShortAdvice(this.tooShortAction, this.minLength, 0);
57+
return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, 0);
5258
}
5359
if (password.length() < this.minLength) {
54-
return new TooShortAdvice(this.tooShortAction, this.minLength, password.length());
60+
return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, password.length());
5561
}
5662
if (password.length() > this.maxLength) {
57-
return new TooLongAdvice(this.tooLongAction, this.maxLength, password.length());
63+
return new PasswordLengthAdvice(this.passwordAction, this.minLength, this.maxLength, password.length());
5864
}
59-
return PasswordAdvice.ABSTAIN;
65+
return SimplePasswordAdvice.NONE;
6066
}
6167

6268
@Override
6369
public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) {
6470
return advise(user, newPassword);
6571
}
6672

67-
public void setTooShortAction(PasswordAction tooShortAction) {
68-
this.tooShortAction = tooShortAction;
73+
public void setPasswordAction(PasswordAction passwordAction) {
74+
this.passwordAction = passwordAction;
6975
}
7076

71-
public void setTooLongAction(PasswordAction tooLongAction) {
72-
this.tooLongAction = tooLongAction;
73-
}
74-
75-
public static final class TooShortAdvice implements PasswordAdvice {
76-
77-
private final PasswordAction action;
77+
public static final class PasswordLengthAdvice extends SimplePasswordAdvice {
7878

7979
private final int minLength;
8080

81-
private final int actualLength;
82-
83-
private TooShortAdvice(PasswordAction action, int minLength, int actualLength) {
84-
this.action = action;
85-
this.minLength = minLength;
86-
this.actualLength = actualLength;
87-
}
88-
89-
@Override
90-
public PasswordAction getAction() {
91-
return this.action;
92-
}
93-
94-
public int getMinLength() {
95-
return this.minLength;
96-
}
97-
98-
public int getActualLength() {
99-
return this.actualLength;
100-
}
101-
102-
@Override
103-
public String toString() {
104-
return "TooShort [" + "action=" + this.action + ", minLength=" + this.minLength + ", actualLength="
105-
+ this.actualLength + "]";
106-
}
107-
108-
}
109-
110-
public static final class TooLongAdvice implements PasswordAdvice {
111-
112-
private final PasswordAction action;
113-
11481
private final int maxLength;
11582

11683
private final int actualLength;
11784

118-
private TooLongAdvice(PasswordAction action, int maxLength, int actualLength) {
119-
this.action = action;
85+
private PasswordLengthAdvice(PasswordAction action, int minLength, int maxLength, int actualLength) {
86+
super(action);
87+
this.minLength = minLength;
12088
this.maxLength = maxLength;
12189
this.actualLength = actualLength;
12290
}
12391

124-
@Override
125-
public PasswordAction getAction() {
126-
return this.action;
92+
public int getMinLength() {
93+
return this.minLength;
12794
}
12895

12996
public int getMaxLength() {
@@ -136,8 +103,8 @@ public int getActualLength() {
136103

137104
@Override
138105
public String toString() {
139-
return "TooLong [" + "action=" + this.action + ", maxLength=" + this.maxLength + ", actualLength="
140-
+ this.actualLength + "]";
106+
return "Length [action=" + super.toString() + ", minLength=" + this.minLength + ", maxLength="
107+
+ this.maxLength + ", actualLength=" + this.actualLength + "]";
141108
}
142109

143110
}

‎core/src/main/java/org/springframework/security/authentication/password/RepeatedPasswordAdvisor.java

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,17 +26,14 @@ public final class RepeatedPasswordAdvisor implements UpdatePasswordAdvisor {
2626

2727
@Override
2828
public PasswordAdvice advise(UserDetails user, @Nullable String oldPassword, @Nullable String newPassword) {
29-
if (Objects.equals(oldPassword, newPassword)) {
30-
return new Advice();
31-
}
32-
return PasswordAdvice.ABSTAIN;
29+
boolean repeated = Objects.equals(oldPassword, newPassword);
30+
return new RepeatedPasswordAdvice(repeated ? PasswordAction.MUST_CHANGE : PasswordAction.NONE);
3331
}
3432

35-
public static final class Advice implements PasswordAdvice {
33+
public static final class RepeatedPasswordAdvice extends SimplePasswordAdvice {
3634

37-
@Override
38-
public PasswordAction getAction() {
39-
return PasswordAction.MUST_CHANGE;
35+
RepeatedPasswordAdvice(PasswordAction action) {
36+
super(action);
4037
}
4138

4239
}

0 commit comments

Comments
 (0)