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

Author: jzheaux

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

@@ -20,7 +20,7 @@
 import java.util.Collection;
 
 import org.apereo.cas.client.validation.Assertion;
-import org.jspecify.annotations.NonNull;
+import org.jspecify.annotations.Nullable;
 
 import org.springframework.security.authentication.AbstractAuthenticationToken;
 import org.springframework.security.core.Authentication;
@@ -106,6 +106,20 @@ private CasAuthenticationToken(final Integer keyHash, final Object principal, fi
 		setAuthenticated(true);
 	}
 
+	protected CasAuthenticationToken(Builder<?> builder) {
+		super(builder);
+		Assert.isTrue(builder.principal != null && !"".equals(builder.principal), "principal cannot be null or empty");
+		Assert.notNull(builder.credentials != null && !"".equals(builder.credentials),
+				"credentials cannot be null or empty");
+		Assert.notNull(builder.userDetails, "userDetails cannot be null");
+		Assert.notNull(builder.assertion, "assertion cannot be null");
+		this.keyHash = builder.keyHash;
+		this.principal = builder.principal;
+		this.credentials = builder.credentials;
+		this.userDetails = builder.userDetails;
+		this.assertion = builder.assertion;
+	}
+
 	private static Integer extractKeyHash(String key) {
 		Assert.hasLength(key, "key cannot be null or empty");
 		return key.hashCode();
@@ -156,8 +170,8 @@ public UserDetails getUserDetails() {
 	}
 
 	@Override
-	public Builder toBuilder() {
-		return new Builder().apply(this);
+	public Builder<?> toBuilder() {
+		return new Builder<>(this);
 	}
 
 	@Override
@@ -174,7 +188,7 @@ public String toString() {
 	 *
 	 * @since 7.0
 	 */
-	public static final class Builder extends AbstractAuthenticationBuilder<@NonNull CasAuthenticationToken, Builder> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
 
 		private Integer keyHash;
 
@@ -186,47 +200,47 @@ public static final class Builder extends AbstractAuthenticationBuilder<@NonNull
 
 		private Assertion assertion;
 
-		private Builder() {
-
+		protected Builder(CasAuthenticationToken token) {
+			super(token);
+			this.keyHash = token.keyHash;
+			this.principal = token.principal;
+			this.credentials = token.credentials;
+			this.userDetails = token.userDetails;
+			this.assertion = token.assertion;
 		}
 
-		public Builder apply(CasAuthenticationToken authentication) {
-			return super.apply(authentication).keyHash(authentication.keyHash)
-				.principal(authentication.principal)
-				.credentials(authentication.credentials)
-				.userDetails(authentication.userDetails)
-				.assertion(authentication.assertion);
-		}
-
-		public Builder keyHash(Integer keyHash) {
+		public B keyHash(Integer keyHash) {
 			this.keyHash = keyHash;
-			return this;
+			return (B) this;
 		}
 
-		public Builder principal(Object principal) {
+		@Override
+		public B principal(@Nullable Object principal) {
+			Assert.notNull(principal, "principal cannot be null");
 			this.principal = principal;
-			return this;
+			return (B) this;
 		}
 
-		public Builder credentials(Object credentials) {
+		@Override
+		public B credentials(@Nullable Object credentials) {
+			Assert.notNull(credentials, "credentials cannot be null");
 			this.credentials = credentials;
-			return this;
+			return (B) this;
 		}
 
-		public Builder userDetails(UserDetails userDetails) {
+		public B userDetails(UserDetails userDetails) {
 			this.userDetails = userDetails;
-			return this;
+			return (B) this;
 		}
 
-		public Builder assertion(Assertion assertion) {
+		public B assertion(Assertion assertion) {
 			this.assertion = assertion;
-			return this;
+			return (B) this;
 		}
 
 		@Override
-		protected @NonNull CasAuthenticationToken build(Collection<GrantedAuthority> authorities) {
-			return new CasAuthenticationToken(this.keyHash, this.principal, this.credentials, authorities,
-					this.userDetails, this.assertion);
+		public CasAuthenticationToken build() {
+			return new CasAuthenticationToken(this);
 		}
 
 	}

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

@@ -19,7 +19,10 @@
 import java.io.Serial;
 import java.util.Collection;
 
+import org.jspecify.annotations.Nullable;
+
 import org.springframework.security.authentication.AbstractAuthenticationToken;
+import org.springframework.security.core.Authentication;
 import org.springframework.security.core.GrantedAuthority;
 import org.springframework.util.Assert;
 
@@ -50,7 +53,7 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
 	 *
 	 */
 	public CasServiceTicketAuthenticationToken(String identifier, Object credentials) {
-		super(null);
+		super((Collection<? extends GrantedAuthority>) null);
 		this.identifier = identifier;
 		this.credentials = credentials;
 		setAuthenticated(false);
@@ -73,6 +76,12 @@ public CasServiceTicketAuthenticationToken(String identifier, Object credentials
 		super.setAuthenticated(true);
 	}
 
+	protected CasServiceTicketAuthenticationToken(Builder<?> builder) {
+		super(builder);
+		this.identifier = builder.principal;
+		this.credentials = builder.credentials;
+	}
+
 	public static CasServiceTicketAuthenticationToken stateful(Object credentials) {
 		return new CasServiceTicketAuthenticationToken(CAS_STATEFUL_IDENTIFIER, credentials);
 	}
@@ -108,4 +117,46 @@ public void eraseCredentials() {
 		this.credentials = null;
 	}
 
+	public Builder<?> toBuilder() {
+		return new Builder<>(this);
+	}
+
+	/**
+	 * A builder preserving the concrete {@link Authentication} type
+	 *
+	 * @since 7.0
+	 */
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<String, Object, B> {
+
+		private String principal;
+
+		private Object credentials;
+
+		protected Builder(CasServiceTicketAuthenticationToken token) {
+			super(token);
+			this.principal = token.identifier;
+			this.credentials = token.credentials;
+		}
+
+		@Override
+		public B principal(@Nullable String principal) {
+			Assert.notNull(principal, "principal cannot be null");
+			this.principal = principal;
+			return (B) this;
+		}
+
+		@Override
+		public B credentials(@Nullable Object credentials) {
+			Assert.notNull(credentials, "credentials cannot be null");
+			this.credentials = credentials;
+			return (B) this;
+		}
+
+		@Override
+		public CasServiceTicketAuthenticationToken build() {
+			return new CasServiceTicketAuthenticationToken(this);
+		}
+
+	}
+
 }

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

@@ -165,7 +165,14 @@ public void toBuilderWhenApplyThenCopies() {
 		Assertion assertionTwo = new AssertionImpl("test");
 		CasAuthenticationToken factorTwo = new CasAuthenticationToken("yek", "bob", "ssap",
 				AuthorityUtils.createAuthorityList("FACTOR_TWO"), PasswordEncodedUser.admin(), assertionTwo);
-		CasAuthenticationToken authentication = factorOne.toBuilder().apply(factorTwo).build();
+		CasAuthenticationToken authentication = factorOne.toBuilder()
+			.authorities((a) -> a.addAll(factorTwo.getAuthorities()))
+			.keyHash(factorTwo.getKeyHash())
+			.principal(factorTwo.getPrincipal())
+			.credentials(factorTwo.getCredentials())
+			.userDetails(factorTwo.getUserDetails())
+			.assertion(factorTwo.getAssertion())
+			.build();
 		Set<String> authorities = AuthorityUtils.authorityListToSet(authentication.getAuthorities());
 		assertThat(authentication.getKeyHash()).isEqualTo(factorTwo.getKeyHash());
 		assertThat(authentication.getPrincipal()).isEqualTo(factorTwo.getPrincipal());

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

@@ -16,6 +16,8 @@
 
 package org.springframework.security.config.annotation.web.configurers;
 
+import java.util.Collection;
+
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
@@ -31,6 +33,7 @@
 import org.springframework.security.config.test.SpringTestContextExtension;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.Transient;
 import org.springframework.security.web.SecurityFilterChain;
 import org.springframework.test.web.servlet.MockMvc;
@@ -113,7 +116,7 @@ public boolean supports(Class<?> authentication) {
 	static class SomeTransientAuthentication extends AbstractAuthenticationToken {
 
 		SomeTransientAuthentication() {
-			super(null);
+			super((Collection<? extends GrantedAuthority>) null);
 		}
 
 		@Override

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

@@ -16,6 +16,8 @@
 
 package org.springframework.security.config.http;
 
+import java.util.Collection;
+
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
@@ -26,6 +28,7 @@
 import org.springframework.security.config.test.SpringTestContextExtension;
 import org.springframework.security.core.Authentication;
 import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.GrantedAuthority;
 import org.springframework.security.core.Transient;
 import org.springframework.test.web.servlet.MockMvc;
 import org.springframework.test.web.servlet.MvcResult;
@@ -82,7 +85,7 @@ public boolean supports(Class<?> authentication) {
 	static class SomeTransientAuthentication extends AbstractAuthenticationToken {
 
 		SomeTransientAuthentication() {
-			super(null);
+			super((Collection<? extends GrantedAuthority>) null);
 		}
 
 		@Override

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

@@ -20,7 +20,7 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
-import java.util.HashSet;
+import java.util.LinkedHashSet;
 import java.util.function.Consumer;
 
 import org.jspecify.annotations.Nullable;
@@ -43,6 +43,8 @@
  */
 public abstract class AbstractAuthenticationToken implements Authentication, CredentialsContainer {
 
+	private static final long serialVersionUID = -3194696462184782834L;
+
 	private final Collection<GrantedAuthority> authorities;
 
 	private @Nullable Object details;
@@ -65,6 +67,12 @@ public AbstractAuthenticationToken(@Nullable Collection<? extends GrantedAuthori
 		this.authorities = Collections.unmodifiableList(new ArrayList<>(authorities));
 	}
 
+	protected AbstractAuthenticationToken(AbstractAuthenticationBuilder<?, ?, ?> builder) {
+		this(builder.authorities);
+		this.authenticated = builder.authenticated;
+		this.details = builder.details;
+	}
+
 	@Override
 	public Collection<GrantedAuthority> getAuthorities() {
 		return this.authorities;
@@ -187,36 +195,40 @@ public String toString() {
 		return sb.toString();
 	}
 
-	protected abstract static class AbstractAuthenticationBuilder<A extends Authentication, B extends AbstractAuthenticationBuilder<A, B>>
-			implements Builder<A, B> {
+	protected abstract static class AbstractAuthenticationBuilder<P, C, B extends AbstractAuthenticationBuilder<P, C, B>>
+			implements Authentication.Builder<P, C, B> {
 
-		private final Collection<GrantedAuthority> authorities = new HashSet<>();
+		protected boolean authenticated;
 
-		protected AbstractAuthenticationBuilder() {
+		protected @Nullable Object details;
 
+		protected final Collection<GrantedAuthority> authorities;
+
+		protected AbstractAuthenticationBuilder(AbstractAuthenticationToken token) {
+			this.authorities = new LinkedHashSet<>(token.getAuthorities());
+			this.authenticated = token.isAuthenticated();
+			this.details = token.getDetails();
 		}
 
 		@Override
-		public B authorities(Consumer<Collection<GrantedAuthority>> authorities) {
-			authorities.accept(this.authorities);
+		public B authenticated(boolean authenticated) {
+			this.authenticated = authenticated;
 			return (B) this;
 		}
 
 		@Override
-		public A build() {
-			return build(this.authorities);
+		public B details(@Nullable Object details) {
+			this.details = details;
+			return (B) this;
 		}
 
 		@Override
-		public B apply(Authentication token) {
-			Assert.isTrue(token.isAuthenticated(), "cannot mutate an unauthenticated token");
-			Assert.notNull(token.getPrincipal(), "principal cannot be null");
-			this.authorities.addAll(token.getAuthorities());
+		public B authorities(Consumer<Collection<GrantedAuthority>> authorities) {
+			authorities.accept(this.authorities);
+			this.authenticated = true;
 			return (B) this;
 		}
 
-		protected abstract A build(Collection<GrantedAuthority> authorities);
-
 	}
 
 }