Przeglądaj źródła

Remove Generic Typing From Authentication.Builder

It would be better to introduce parameter types for
principal and credentials into Authentication.Builder
at the same time as doing so for Authentication

Issue gh-17861
Josh Cummings 1 tydzień temu
rodzic
commit
dd50dc0c40
17 zmienionych plików z 47 dodań i 38 usunięć
  1. 1 1
      cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java
  2. 4 4
      cas/src/main/java/org/springframework/security/cas/authentication/CasServiceTicketAuthenticationToken.java
  3. 3 3
      core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java
  4. 1 1
      core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java
  5. 1 1
      core/src/main/java/org/springframework/security/authentication/TestingAuthenticationToken.java
  6. 1 1
      core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java
  7. 1 1
      core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthentication.java
  8. 4 4
      core/src/main/java/org/springframework/security/core/Authentication.java
  9. 9 1
      core/src/main/java/org/springframework/security/core/SimpleAuthentication.java
  10. 1 1
      core/src/test/java/org/springframework/security/authentication/AbstractAuthenticationBuilderTests.java
  11. 4 4
      oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationToken.java
  12. 1 1
      oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/AbstractOAuth2TokenAuthenticationToken.java
  13. 6 6
      saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2AssertionAuthentication.java
  14. 2 2
      saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Authentication.java
  15. 1 1
      saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/Saml2AssertionAuthenticationTests.java
  16. 1 1
      web/src/main/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationToken.java
  17. 6 5
      webauthn/src/main/java/org/springframework/security/web/webauthn/authentication/WebAuthnAuthentication.java

+ 1 - 1
cas/src/main/java/org/springframework/security/cas/authentication/CasAuthenticationToken.java

@@ -187,7 +187,7 @@ public class CasAuthenticationToken extends AbstractAuthenticationToken implemen
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Integer keyHash;
 

+ 4 - 4
cas/src/main/java/org/springframework/security/cas/authentication/CasServiceTicketAuthenticationToken.java

@@ -126,7 +126,7 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<String, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private String principal;
 
@@ -139,9 +139,9 @@ public class CasServiceTicketAuthenticationToken extends AbstractAuthenticationT
 		}
 
 		@Override
-		public B principal(@Nullable String principal) {
-			Assert.notNull(principal, "principal cannot be null");
-			this.principal = principal;
+		public B principal(@Nullable Object principal) {
+			Assert.isInstanceOf(String.class, principal, "principal must be of type String");
+			this.principal = (String) principal;
 			return (B) this;
 		}
 

+ 3 - 3
core/src/main/java/org/springframework/security/authentication/AbstractAuthenticationToken.java

@@ -69,7 +69,7 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 		this.authorities = Collections.unmodifiableList(new ArrayList<>(authorities));
 	}
 
-	protected AbstractAuthenticationToken(AbstractAuthenticationBuilder<?, ?, ?> builder) {
+	protected AbstractAuthenticationToken(AbstractAuthenticationBuilder<?> builder) {
 		this(builder.authorities);
 		this.authenticated = builder.authenticated;
 		this.details = builder.details;
@@ -197,8 +197,8 @@ public abstract class AbstractAuthenticationToken implements Authentication, Cre
 		return sb.toString();
 	}
 
-	protected abstract static class AbstractAuthenticationBuilder<P, C, B extends AbstractAuthenticationBuilder<P, C, B>>
-			implements Authentication.Builder<P, C, B> {
+	protected abstract static class AbstractAuthenticationBuilder<B extends AbstractAuthenticationBuilder<B>>
+			implements Authentication.Builder<B> {
 
 		protected boolean authenticated;
 

+ 1 - 1
core/src/main/java/org/springframework/security/authentication/RememberMeAuthenticationToken.java

@@ -126,7 +126,7 @@ public class RememberMeAuthenticationToken extends AbstractAuthenticationToken {
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Integer keyHash;
 

+ 1 - 1
core/src/main/java/org/springframework/security/authentication/TestingAuthenticationToken.java

@@ -91,7 +91,7 @@ public class TestingAuthenticationToken extends AbstractAuthenticationToken {
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Object principal;
 

+ 1 - 1
core/src/main/java/org/springframework/security/authentication/UsernamePasswordAuthenticationToken.java

@@ -141,7 +141,7 @@ public class UsernamePasswordAuthenticationToken extends AbstractAuthenticationT
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		protected @Nullable Object principal;
 

+ 1 - 1
core/src/main/java/org/springframework/security/authentication/ott/OneTimeTokenAuthentication.java

@@ -67,7 +67,7 @@ public class OneTimeTokenAuthentication extends AbstractAuthenticationToken {
 	/**
 	 * A builder for constructing a {@link OneTimeTokenAuthentication} instance
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Object principal;
 

+ 4 - 4
core/src/main/java/org/springframework/security/core/Authentication.java

@@ -143,7 +143,7 @@ public interface Authentication extends Principal, Serializable {
 	 * instance
 	 * @since 7.0
 	 */
-	default Builder<?, ?, ?> toBuilder() {
+	default Builder<?> toBuilder() {
 		return new SimpleAuthentication.Builder(this);
 	}
 
@@ -153,18 +153,18 @@ public interface Authentication extends Principal, Serializable {
 	 * @author Josh Cummings
 	 * @since 7.0
 	 */
-	interface Builder<P, C, B extends Builder<P, C, B>> {
+	interface Builder<B extends Builder<B>> {
 
 		B authorities(Consumer<Collection<GrantedAuthority>> authorities);
 
-		default B credentials(@Nullable C credentials) {
+		default B credentials(@Nullable Object credentials) {
 			throw new UnsupportedOperationException(
 					String.format("%s does not store credentials", this.getClass().getSimpleName()));
 		}
 
 		B details(@Nullable Object details);
 
-		B principal(@Nullable P principal);
+		B principal(@Nullable Object principal);
 
 		B authenticated(boolean authenticated);
 

+ 9 - 1
core/src/main/java/org/springframework/security/core/SimpleAuthentication.java

@@ -21,6 +21,8 @@ import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.function.Consumer;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.jspecify.annotations.Nullable;
 
 @Transient
@@ -83,7 +85,9 @@ final class SimpleAuthentication implements Authentication {
 		return (this.principal == null) ? "" : this.principal.toString();
 	}
 
-	static final class Builder implements Authentication.Builder<Object, Object, Builder> {
+	static final class Builder implements Authentication.Builder<Builder> {
+
+		private final Log logger = LogFactory.getLog(getClass());
 
 		private final Collection<GrantedAuthority> authorities = new LinkedHashSet<>();
 
@@ -96,11 +100,15 @@ final class SimpleAuthentication implements Authentication {
 		private boolean authenticated;
 
 		Builder(Authentication authentication) {
+			this.logger.debug("Creating a builder which will result in exchanging an authentication of type "
+					+ authentication.getClass() + " for " + SimpleAuthentication.class.getSimpleName() + ";"
+					+ " consider implementing " + authentication.getClass().getSimpleName() + "#toBuilder");
 			this.authorities.addAll(authentication.getAuthorities());
 			this.principal = authentication.getPrincipal();
 			this.credentials = authentication.getCredentials();
 			this.details = authentication.getDetails();
 			this.authenticated = authentication.isAuthenticated();
+
 		}
 
 		@Override

+ 1 - 1
core/src/test/java/org/springframework/security/authentication/AbstractAuthenticationBuilderTests.java

@@ -40,7 +40,7 @@ class AbstractAuthenticationBuilderTests {
 	}
 
 	private static final class TestAbstractAuthenticationBuilder
-			extends AbstractAuthenticationBuilder<Object, Object, TestAbstractAuthenticationBuilder> {
+			extends AbstractAuthenticationBuilder<TestAbstractAuthenticationBuilder> {
 
 		private TestAbstractAuthenticationBuilder(TestingAuthenticationToken token) {
 			super(token);

+ 4 - 4
oauth2/oauth2-client/src/main/java/org/springframework/security/oauth2/client/authentication/OAuth2AuthenticationToken.java

@@ -105,7 +105,7 @@ public class OAuth2AuthenticationToken extends AbstractAuthenticationToken {
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<OAuth2User, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private OAuth2User principal;
 
@@ -118,9 +118,9 @@ public class OAuth2AuthenticationToken extends AbstractAuthenticationToken {
 		}
 
 		@Override
-		public B principal(@Nullable OAuth2User principal) {
-			Assert.notNull(principal, "principal cannot be null");
-			this.principal = principal;
+		public B principal(@Nullable Object principal) {
+			Assert.isInstanceOf(OAuth2User.class, principal, "principal must be of type OAuth2User");
+			this.principal = (OAuth2User) principal;
 			return (B) this;
 		}
 

+ 1 - 1
oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/AbstractOAuth2TokenAuthenticationToken.java

@@ -123,7 +123,7 @@ public abstract class AbstractOAuth2TokenAuthenticationToken<T extends OAuth2Tok
 	 * @since 7.0
 	 */
 	public abstract static class AbstractOAuth2TokenAuthenticationBuilder<T extends OAuth2Token, B extends AbstractOAuth2TokenAuthenticationBuilder<T, B>>
-			extends AbstractAuthenticationBuilder<Object, Object, B> {
+			extends AbstractAuthenticationBuilder<B> {
 
 		private Object principal;
 

+ 6 - 6
saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2AssertionAuthentication.java

@@ -82,8 +82,7 @@ public class Saml2AssertionAuthentication extends Saml2Authentication {
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>>
-			extends Saml2Authentication.Builder<Saml2ResponseAssertionAccessor, B> {
+	public static class Builder<B extends Builder<B>> extends Saml2Authentication.Builder<B> {
 
 		private Saml2ResponseAssertionAccessor assertion;
 
@@ -96,10 +95,11 @@ public class Saml2AssertionAuthentication extends Saml2Authentication {
 		}
 
 		@Override
-		public B credentials(@Nullable Saml2ResponseAssertionAccessor credentials) {
-			saml2Response(credentials.getResponseValue());
-			Assert.notNull(credentials, "assertion cannot be null");
-			this.assertion = credentials;
+		public B credentials(@Nullable Object credentials) {
+			Assert.isInstanceOf(Saml2ResponseAssertionAccessor.class, credentials,
+					"credentials must be of type Saml2ResponseAssertionAccessor");
+			saml2Response(((Saml2ResponseAssertionAccessor) credentials).getResponseValue());
+			this.assertion = (Saml2ResponseAssertionAccessor) credentials;
 			return (B) this;
 		}
 

+ 2 - 2
saml2/saml2-service-provider/src/main/java/org/springframework/security/saml2/provider/service/authentication/Saml2Authentication.java

@@ -71,7 +71,7 @@ public class Saml2Authentication extends AbstractAuthenticationToken {
 		setAuthenticated(true);
 	}
 
-	Saml2Authentication(Builder<?, ?> builder) {
+	Saml2Authentication(Builder<?> builder) {
 		super(builder);
 		this.principal = builder.principal;
 		this.saml2Response = builder.saml2Response;
@@ -95,7 +95,7 @@ public class Saml2Authentication extends AbstractAuthenticationToken {
 		return getSaml2Response();
 	}
 
-	abstract static class Builder<C, B extends Builder<C, B>> extends AbstractAuthenticationBuilder<Object, C, B> {
+	abstract static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Object principal;
 

+ 1 - 1
saml2/saml2-service-provider/src/test/java/org/springframework/security/saml2/provider/service/authentication/Saml2AssertionAuthenticationTests.java

@@ -32,7 +32,7 @@ class Saml2AssertionAuthenticationTests {
 		Saml2AssertionAuthentication factorOne = new Saml2AssertionAuthentication("alice",
 				prototype.nameId("alice").build(), AuthorityUtils.createAuthorityList("FACTOR_ONE"), "alice");
 		Saml2AssertionAuthentication factorTwo = new Saml2AssertionAuthentication("bob",
-				prototype.nameId("alice").build(), AuthorityUtils.createAuthorityList("FACTOR_TWO"), "bob");
+				prototype.nameId("bob").build(), AuthorityUtils.createAuthorityList("FACTOR_TWO"), "bob");
 		Saml2AssertionAuthentication result = factorOne.toBuilder()
 			.authorities((a) -> a.addAll(factorTwo.getAuthorities()))
 			.principal(factorTwo.getPrincipal())

+ 1 - 1
web/src/main/java/org/springframework/security/web/authentication/preauth/PreAuthenticatedAuthenticationToken.java

@@ -100,7 +100,7 @@ public class PreAuthenticatedAuthenticationToken extends AbstractAuthenticationT
 	 *
 	 * @since 7.0
 	 */
-	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<Object, Object, B> {
+	public static class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private Object principal;
 

+ 6 - 5
webauthn/src/main/java/org/springframework/security/web/webauthn/authentication/WebAuthnAuthentication.java

@@ -85,19 +85,20 @@ public class WebAuthnAuthentication extends AbstractAuthenticationToken {
 	 *
 	 * @since 7.0
 	 */
-	public static final class Builder<B extends Builder<B>>
-			extends AbstractAuthenticationBuilder<PublicKeyCredentialUserEntity, Object, B> {
+	public static final class Builder<B extends Builder<B>> extends AbstractAuthenticationBuilder<B> {
 
 		private PublicKeyCredentialUserEntity principal;
 
 		private Builder(WebAuthnAuthentication token) {
 			super(token);
+			this.principal = token.principal;
 		}
 
 		@Override
-		public B principal(@Nullable PublicKeyCredentialUserEntity principal) {
-			Assert.notNull(principal, "principal cannot be null");
-			this.principal = principal;
+		public B principal(@Nullable Object principal) {
+			Assert.isInstanceOf(PublicKeyCredentialUserEntity.class, principal,
+					"principal must be of type PublicKeyCredentialUserEntity");
+			this.principal = (PublicKeyCredentialUserEntity) principal;
 			return (B) this;
 		}