Skip to content

Commit a0fe6a5

Browse files
jzheauxjzheaux
committed
Polish Builders
- Added remaining properties - Removed apply method since Spring Security isn't using it right now - Made builders extensible since the authentications are extensible Issue spring-projectsgh-17861
1 parent 44fef78 commit a0fe6a5

File tree

37 files changed

+609
-326
lines changed

37 files changed

+609
-326
lines changed

‎cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java‎

Lines changed: 40 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.util.Collection;
2121

2222
import org.apereo.cas.client.validation.Assertion;
23-
import org.jspecify.annotations.NonNull;
23+
import org.jspecify.annotations.Nullable;
2424

2525
import org.springframework.security.authentication.AbstractAuthenticationToken;
2626
import org.springframework.security.core.Authentication;
@@ -106,6 +106,19 @@ private CasAuthenticationToken(final Integer keyHash, final Object principal, fi
106106
setAuthenticated(true);
107107
}
108108

109+
protected CasAuthenticationToken(Builder<?> builder) {
110+
super(builder);
111+
Assert.isTrue(!"".equals(builder.principal), "principal cannot be null or empty");
112+
Assert.notNull(!"".equals(builder.credentials), "credentials cannot be null or empty");
113+
Assert.notNull(builder.userDetails, "userDetails cannot be null");
114+
Assert.notNull(builder.assertion, "assertion cannot be null");
115+
this.keyHash = builder.keyHash;
116+
this.principal = builder.principal;
117+
this.credentials = builder.credentials;
118+
this.userDetails = builder.userDetails;
119+
this.assertion = builder.assertion;
120+
}
121+
109122
private static Integer extractKeyHash(String key) {
110123
Assert.hasLength(key, "key cannot be null or empty");
111124
return key.hashCode();
@@ -156,8 +169,8 @@ public UserDetails getUserDetails() {
156169
}
157170

158171
@Override
159-
public Builder toBuilder() {
160-
return new Builder().apply(this);
172+
public Builder<?> toBuilder() {
173+
return new Builder<>(this);
161174
}
162175

163176
@Override
@@ -174,7 +187,7 @@ public String toString() {
174187
*
175188
* @since 7.0
176189
*/
177-
public static final class Builder extends AbstractAuthenticationBuilder<@NonNull CasAuthenticationToken, Builder> {
190+
public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
178191

179192
private Integer keyHash;
180193

@@ -186,47 +199,47 @@ public static final class Builder extends AbstractAuthenticationBuilder<@NonNull
186199

187200
private Assertion assertion;
188201

189-
private Builder() {
190-
202+
protected Builder(CasAuthenticationToken token) {
203+
super(token);
204+
this.keyHash = token.keyHash;
205+
this.principal = token.principal;
206+
this.credentials = token.credentials;
207+
this.userDetails = token.userDetails;
208+
this.assertion = token.assertion;
191209
}
192210

193-
public Builder apply(CasAuthenticationToken authentication) {
194-
return super.apply(authentication).keyHash(authentication.keyHash)
195-
.principal(authentication.principal)
196-
.credentials(authentication.credentials)
197-
.userDetails(authentication.userDetails)
198-
.assertion(authentication.assertion);
199-
}
200-
201-
public Builder keyHash(Integer keyHash) {
211+
public B keyHash(Integer keyHash) {
202212
this.keyHash = keyHash;
203-
return this;
213+
return (B) this;
204214
}
205215

206-
public Builder principal(Object principal) {
216+
@Override
217+
public B principal(@Nullable Object principal) {
218+
Assert.notNull(principal, "principal cannot be null");
207219
this.principal = principal;
208-
return this;
220+
return (B) this;
209221
}
210222

211-
public Builder credentials(Object credentials) {
223+
@Override
224+
public B credentials(@Nullable Object credentials) {
225+
Assert.notNull(credentials, "credentials cannot be null");
212226
this.credentials = credentials;
213-
return this;
227+
return (B) this;
214228
}
215229

216-
public Builder userDetails(UserDetails userDetails) {
230+
public B userDetails(UserDetails userDetails) {
217231
this.userDetails = userDetails;
218-
return this;
232+
return (B) this;
219233
}
220234

221-
public Builder assertion(Assertion assertion) {
235+
public B assertion(Assertion assertion) {
222236
this.assertion = assertion;
223-
return this;
237+
return (B) this;
224238
}
225239

226240
@Override
227-
protected @NonNull CasAuthenticationToken build(Collection<GrantedAuthority> authorities) {
228-
return new CasAuthenticationToken(this.keyHash, this.principal, this.credentials, authorities,
229-
this.userDetails, this.assertion);
241+
public CasAuthenticationToken build() {
242+
return new CasAuthenticationToken(this);
230243
}
231244

232245
}

‎cas/src/main/java/org/springframework/security/cas/authentication/CasServiceTicketAuthenticationToken.java‎

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.jspecify.annotations.Nullable;
2323

2424
import org.springframework.security.authentication.AbstractAuthenticationToken;
25+
import org.springframework.security.core.Authentication;
2526
import org.springframework.security.core.GrantedAuthority;
2627
import org.springframework.util.Assert;
2728

@@ -52,7 +53,7 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
5253
*
5354
*/
5455
public CasServiceTicketAuthenticationToken(String identifier, Object credentials) {
55-
super(null);
56+
super((Collection<? extends GrantedAuthority>) null);
5657
this.identifier = identifier;
5758
this.credentials = credentials;
5859
setAuthenticated(false);
@@ -75,6 +76,12 @@ public CasServiceTicketAuthenticationToken(String identifier, Object credentials
7576
super.setAuthenticated(true);
7677
}
7778

79+
protected CasServiceTicketAuthenticationToken(Builder<?> builder) {
80+
super(builder);
81+
this.identifier = builder.principal;
82+
this.credentials = builder.credentials;
83+
}
84+
7885
public static CasServiceTicketAuthenticationToken stateful(Object credentials) {
7986
return new CasServiceTicketAuthenticationToken(CAS_STATEFUL_IDENTIFIER, credentials);
8087
}
@@ -110,4 +117,46 @@ public void eraseCredentials() {
110117
this.credentials = null;
111118
}
112119

120+
public Builder<?> toBuilder() {
121+
return new Builder<>(this);
122+
}
123+
124+
/**
125+
* A builder preserving the concrete {@link Authentication} type
126+
*
127+
* @since 7.0
128+
*/
129+
public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<String, Object, B> {
130+
131+
private String principal;
132+
133+
private @Nullable Object credentials;
134+
135+
protected Builder(CasServiceTicketAuthenticationToken token) {
136+
super(token);
137+
this.principal = token.identifier;
138+
this.credentials = token.credentials;
139+
}
140+
141+
@Override
142+
public B principal(@Nullable String principal) {
143+
Assert.notNull(principal, "principal cannot be null");
144+
this.principal = principal;
145+
return (B) this;
146+
}
147+
148+
@Override
149+
public B credentials(@Nullable Object credentials) {
150+
Assert.notNull(credentials, "credentials cannot be null");
151+
this.credentials = credentials;
152+
return (B) this;
153+
}
154+
155+
@Override
156+
public CasServiceTicketAuthenticationToken build() {
157+
return new CasServiceTicketAuthenticationToken(this);
158+
}
159+
160+
}
161+
113162
}

‎cas/src/test/java/org/springframework/security/cas/authentication/CasAuthenticationTokenTests.java‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,14 @@ public void toBuilderWhenApplyThenCopies() {
165165
Assertion assertionTwo = new AssertionImpl("test");
166166
CasAuthenticationToken factorTwo = new CasAuthenticationToken("yek", "bob", "ssap",
167167
AuthorityUtils.createAuthorityList("FACTOR_TWO"), PasswordEncodedUser.admin(), assertionTwo);
168-
CasAuthenticationToken authentication = factorOne.toBuilder().apply(factorTwo).build();
168+
CasAuthenticationToken authentication = factorOne.toBuilder()
169+
.authorities((a) -> a.addAll(factorTwo.getAuthorities()))
170+
.keyHash(factorTwo.getKeyHash())
171+
.principal(factorTwo.getPrincipal())
172+
.credentials(factorTwo.getCredentials())
173+
.userDetails(factorTwo.getUserDetails())
174+
.assertion(factorTwo.getAssertion())
175+
.build();
169176
Set<String> authorities = AuthorityUtils.authorityListToSet(authentication.getAuthorities());
170177
assertThat(authentication.getKeyHash()).isEqualTo(factorTwo.getKeyHash());
171178
assertThat(authentication.getPrincipal()).isEqualTo(factorTwo.getPrincipal());

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.security.config.annotation.web.configurers;
1818

19+
import java.util.Collection;
20+
1921
import org.junit.jupiter.api.Test;
2022
import org.junit.jupiter.api.extension.ExtendWith;
2123

@@ -31,6 +33,7 @@
3133
import org.springframework.security.config.test.SpringTestContextExtension;
3234
import org.springframework.security.core.Authentication;
3335
import org.springframework.security.core.AuthenticationException;
36+
import org.springframework.security.core.GrantedAuthority;
3437
import org.springframework.security.core.Transient;
3538
import org.springframework.security.web.SecurityFilterChain;
3639
import org.springframework.test.web.servlet.MockMvc;
@@ -113,7 +116,7 @@ public boolean supports(Class<?> authentication) {
113116
static class SomeTransientAuthentication extends AbstractAuthenticationToken {
114117

115118
SomeTransientAuthentication() {
116-
super(null);
119+
super((Collection<? extends GrantedAuthority>) null);
117120
}
118121

119122
@Override

‎config/src/test/java/org/springframework/security/config/http/SessionManagementConfigTransientAuthenticationTests.java‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616

1717
package org.springframework.security.config.http;
1818

19+
import java.util.Collection;
20+
1921
import org.junit.jupiter.api.Test;
2022
import org.junit.jupiter.api.extension.ExtendWith;
2123

@@ -26,6 +28,7 @@
2628
import org.springframework.security.config.test.SpringTestContextExtension;
2729
import org.springframework.security.core.Authentication;
2830
import org.springframework.security.core.AuthenticationException;
31+
import org.springframework.security.core.GrantedAuthority;
2932
import org.springframework.security.core.Transient;
3033
import org.springframework.test.web.servlet.MockMvc;
3134
import org.springframework.test.web.servlet.MvcResult;
@@ -82,7 +85,7 @@ public boolean supports(Class<?> authentication) {
8285
static class SomeTransientAuthentication extends AbstractAuthenticationToken {
8386

8487
SomeTransientAuthentication() {
85-
super(null);
88+
super((Collection<? extends GrantedAuthority>) null);
8689
}
8790

8891
@Override

‎core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java‎

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@
2020
import java.util.ArrayList;
2121
import java.util.Collection;
2222
import java.util.Collections;
23-
import java.util.HashSet;
23+
import java.util.LinkedHashSet;
2424
import java.util.function.Consumer;
2525

2626
import org.jspecify.annotations.Nullable;
@@ -43,6 +43,8 @@
4343
*/
4444
public abstract class AbstractAuthenticationToken implements Authentication, CredentialsContainer {
4545

46+
private static final long serialVersionUID = -3194696462184782834L;
47+
4648
private final Collection<GrantedAuthority> authorities;
4749

4850
private @Nullable Object details;
@@ -65,6 +67,12 @@ public AbstractAuthenticationToken(@Nullable Collection<? extends GrantedAuthori
6567
this.authorities = Collections.unmodifiableList(new ArrayList<>(authorities));
6668
}
6769

70+
protected AbstractAuthenticationToken(AbstractAuthenticationBuilder<?, ?, ?> builder) {
71+
this(builder.authorities);
72+
this.authenticated = builder.authenticated;
73+
this.details = builder.details;
74+
}
75+
6876
@Override
6977
public Collection<GrantedAuthority> getAuthorities() {
7078
return this.authorities;
@@ -187,36 +195,40 @@ public String toString() {
187195
return sb.toString();
188196
}
189197

190-
protected abstract static class AbstractAuthenticationBuilder<A extends Authentication, B extends AbstractAuthenticationBuilder<A, B>>
191-
implements Builder<A, B> {
198+
protected abstract static class AbstractAuthenticationBuilder<P, C, B extends AbstractAuthenticationBuilder<P, C, B>>
199+
implements Authentication.Builder<P, C, B> {
192200

193-
private final Collection<GrantedAuthority> authorities = new HashSet<>();
201+
protected boolean authenticated;
194202

195-
protected AbstractAuthenticationBuilder() {
203+
protected @Nullable Object details;
196204

205+
protected final Collection<GrantedAuthority> authorities;
206+
207+
protected AbstractAuthenticationBuilder(AbstractAuthenticationToken token) {
208+
this.authorities = new LinkedHashSet<>(token.getAuthorities());
209+
this.authenticated = token.isAuthenticated();
210+
this.details = token.getDetails();
197211
}
198212

199213
@Override
200-
public B authorities(Consumer<Collection<GrantedAuthority>> authorities) {
201-
authorities.accept(this.authorities);
214+
public B authenticated(boolean authenticated) {
215+
this.authenticated = authenticated;
202216
return (B) this;
203217
}
204218

205219
@Override
206-
public A build() {
207-
return build(this.authorities);
220+
public B details(@Nullable Object details) {
221+
this.details = details;
222+
return (B) this;
208223
}
209224

210225
@Override
211-
public B apply(Authentication token) {
212-
Assert.isTrue(token.isAuthenticated(), "cannot mutate an unauthenticated token");
213-
Assert.notNull(token.getPrincipal(), "principal cannot be null");
214-
this.authorities.addAll(token.getAuthorities());
226+
public B authorities(Consumer<Collection<GrantedAuthority>> authorities) {
227+
authorities.accept(this.authorities);
228+
this.authenticated = true;
215229
return (B) this;
216230
}
217231

218-
protected abstract A build(Collection<GrantedAuthority> authorities);
219-
220232
}
221233

222234
}

0 commit comments

Comments
 (0)