Validate signer client options based on authtype

.changes/next-release/bugfix-AWSSDKforJavav2-7cb7fcf.json

@@ -0,0 +1,6 @@
+{
+    "type": "bugfix",
+    "category": "AWS SDK for Java v2",
+    "contributor": "",
+    "description": "This fixes an issue with the client option validation where the client builder is incorrectly validating that a value for the SIGNER option is present, even when it's not required (because the service does not use AWS auth).\n \n This change only does the appropriate validations based on the authtypes present in the service model."
+}

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/AsyncClientBuilderClass.java

@@ -27,7 +27,8 @@
 import software.amazon.awssdk.codegen.poet.ClassSpec;
 import software.amazon.awssdk.codegen.poet.PoetUtils;
 import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
-import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
+import software.amazon.awssdk.codegen.utils.AuthUtils;
+import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 import software.amazon.awssdk.core.client.config.SdkClientOption;
 
 public class AsyncClientBuilderClass implements ClassSpec {
@@ -72,7 +73,7 @@ public TypeSpec poetSpec() {
             builder.addMethod(endpointProviderMethod());
         }
 
-        if (BearerAuthUtils.usesBearerAuth(model)) {
+        if (AuthUtils.usesBearerAuth(model)) {
             builder.addMethod(bearerTokenProviderMethod());
         }
 
@@ -120,7 +121,9 @@ private MethodSpec buildClientMethod() {
                          .addAnnotation(Override.class)
                          .addModifiers(Modifier.PROTECTED, Modifier.FINAL)
                          .returns(clientInterfaceName)
-                         .addCode("return new $T(super.asyncClientConfiguration());", clientClassName)
+                         .addStatement("$T clientConfiguration = super.asyncClientConfiguration()", SdkClientConfiguration.class)
+                         .addStatement("this.validateClientOptions(clientConfiguration)")
+                         .addCode("return new $T(clientConfiguration);", clientClassName)
                          .build();
     }
 

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderClass.java

@@ -48,7 +48,7 @@
 import software.amazon.awssdk.codegen.poet.ClassSpec;
 import software.amazon.awssdk.codegen.poet.PoetUtils;
 import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
-import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
+import software.amazon.awssdk.codegen.utils.AuthUtils;
 import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
 import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 import software.amazon.awssdk.core.client.config.SdkClientOption;
@@ -83,15 +83,15 @@ public BaseClientBuilderClass(IntermediateModel model) {
     @Override
     public TypeSpec poetSpec() {
         TypeSpec.Builder builder =
-                PoetUtils.createClassBuilder(builderClassName)
-                         .addModifiers(Modifier.ABSTRACT)
-                         .addAnnotation(SdkInternalApi.class)
-                         .addTypeVariable(PoetUtils.createBoundedTypeVariableName("B", builderInterfaceName, "B", "C"))
-                         .addTypeVariable(TypeVariableName.get("C"))
-                         .superclass(PoetUtils.createParameterizedTypeName(AwsDefaultClientBuilder.class, "B", "C"))
-                         .addJavadoc("Internal base class for {@link $T} and {@link $T}.",
-                                     ClassName.get(basePackage, model.getMetadata().getSyncBuilder()),
-                                     ClassName.get(basePackage, model.getMetadata().getAsyncBuilder()));
+            PoetUtils.createClassBuilder(builderClassName)
+                     .addModifiers(Modifier.ABSTRACT)
+                     .addAnnotation(SdkInternalApi.class)
+                     .addTypeVariable(PoetUtils.createBoundedTypeVariableName("B", builderInterfaceName, "B", "C"))
+                     .addTypeVariable(TypeVariableName.get("C"))
+                     .superclass(PoetUtils.createParameterizedTypeName(AwsDefaultClientBuilder.class, "B", "C"))
+                     .addJavadoc("Internal base class for {@link $T} and {@link $T}.",
+                                 ClassName.get(basePackage, model.getMetadata().getSyncBuilder()),
+                                 ClassName.get(basePackage, model.getMetadata().getAsyncBuilder()));
 
         // Only services that require endpoint discovery for at least one of their operations get a default value of
         // 'true'
@@ -126,13 +126,15 @@ public TypeSpec poetSpec() {
                    .addMethod(beanStyleSetServiceConfigurationMethod());
         }
 
-        if (BearerAuthUtils.usesBearerAuth(model)) {
+        if (AuthUtils.usesBearerAuth(model)) {
             builder.addMethod(defaultBearerTokenProviderMethod());
             builder.addMethod(defaultTokenAuthSignerMethod());
         }
 
         addServiceHttpConfigIfNeeded(builder, model);
 
+        builder.addMethod(validateClientOptionsMethod());
+
 
         return builder.build();
     }
@@ -205,7 +207,7 @@ private MethodSpec mergeServiceDefaultsMethod() {
                             SdkClientOption.class, ClassName.bestGuess(clientConfigClassName));
         }
 
-        if (BearerAuthUtils.usesBearerAuth(model)) {
+        if (AuthUtils.usesBearerAuth(model)) {
             builder.addCode(".option($T.TOKEN_PROVIDER, defaultTokenProvider())\n", AwsClientOption.class);
             builder.addCode(".option($T.TOKEN_SIGNER, defaultTokenSigner())", SdkAdvancedClientOption.class);
         }
@@ -217,7 +219,7 @@ private MethodSpec mergeServiceDefaultsMethod() {
     private Optional<MethodSpec> mergeInternalDefaultsMethod() {
         String userAgent = model.getCustomizationConfig().getUserAgent();
         RetryMode defaultRetryMode = model.getCustomizationConfig().getDefaultRetryMode();
-        
+
         // If none of the options are customized, then we do not need to bother overriding the method
         if (userAgent == null && defaultRetryMode == null) {
             return Optional.empty();
@@ -246,10 +248,10 @@ private MethodSpec finalizeServiceConfigurationMethod() {
         String requestHandlerPath = String.format("%s/execution.interceptors", requestHandlerDirectory);
 
         MethodSpec.Builder builder = MethodSpec.methodBuilder("finalizeServiceConfiguration")
-                         .addAnnotation(Override.class)
-                         .addModifiers(PROTECTED, FINAL)
-                         .returns(SdkClientConfiguration.class)
-                         .addParameter(SdkClientConfiguration.class, "config");
+                                               .addAnnotation(Override.class)
+                                               .addModifiers(PROTECTED, FINAL)
+                                               .returns(SdkClientConfiguration.class)
+                                               .addParameter(SdkClientConfiguration.class, "config");
 
         // Initialize configuration values
 
@@ -289,7 +291,7 @@ private MethodSpec finalizeServiceConfigurationMethod() {
                              CollectionUtils.class);
 
         builder.addCode("interceptors = $T.mergeLists(interceptors, config.option($T.EXECUTION_INTERCEPTORS));\n",
-                                CollectionUtils.class, SdkClientOption.class);
+                        CollectionUtils.class, SdkClientOption.class);
 
         if (model.getMetadata().isQueryProtocol()) {
             TypeName listType = ParameterizedTypeName.get(List.class, ExecutionInterceptor.class);
@@ -488,7 +490,7 @@ private void mergeServiceConfiguration(MethodSpec.Builder builder, String client
 
     private MethodSpec setServiceConfigurationMethod() {
         ClassName serviceConfiguration = ClassName.get(basePackage,
-                                                        model.getCustomizationConfig().getServiceConfig().getClassName());
+                                                       model.getCustomizationConfig().getServiceConfig().getClassName());
         return MethodSpec.methodBuilder("serviceConfiguration")
                          .addModifiers(Modifier.PUBLIC)
                          .returns(TypeVariableName.get("B"))
@@ -501,7 +503,7 @@ private MethodSpec setServiceConfigurationMethod() {
 
     private MethodSpec beanStyleSetServiceConfigurationMethod() {
         ClassName serviceConfiguration = ClassName.get(basePackage,
-                                                        model.getCustomizationConfig().getServiceConfig().getClassName());
+                                                       model.getCustomizationConfig().getServiceConfig().getClassName());
         return MethodSpec.methodBuilder("setServiceConfiguration")
                          .addModifiers(Modifier.PUBLIC)
                          .addParameter(serviceConfiguration, "serviceConfiguration")
@@ -623,4 +625,33 @@ private boolean hasClientContextParams() {
         Map<String, ClientContextParam> clientContextParams = model.getClientContextParams();
         return clientContextParams != null && !clientContextParams.isEmpty();
     }
+
+    private MethodSpec validateClientOptionsMethod() {
+        MethodSpec.Builder builder = MethodSpec.methodBuilder("validateClientOptions")
+                                               .addModifiers(PROTECTED, Modifier.STATIC)
+                                               .addParameter(SdkClientConfiguration.class, "c")
+                                               .returns(void.class);
+
+        if (AuthUtils.usesAwsAuth(model)) {
+            builder.addStatement("$T.notNull(c.option($T.SIGNER), $S)",
+                                 Validate.class,
+                                 SdkAdvancedClientOption.class,
+                                 "The 'overrideConfiguration.advancedOption[SIGNER]' must be configured in the client builder.");
+        }
+
+        if (AuthUtils.usesBearerAuth(model)) {
+            builder.addStatement("$T.notNull(c.option($T.TOKEN_SIGNER), $S)",
+                                 Validate.class,
+                                 SdkAdvancedClientOption.class,
+                                 "The 'overrideConfiguration.advancedOption[TOKEN_SIGNER]' "
+                                 + "must be configured in the client builder.");
+            builder.addStatement("$T.notNull(c.option($T.TOKEN_PROVIDER), $S)",
+                                 Validate.class,
+                                 AwsClientOption.class,
+                                 "The 'overrideConfiguration.advancedOption[TOKEN_PROVIDER]' "
+                                 + "must be configured in the client builder.");
+        }
+
+        return builder.build();
+    }
 }

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/BaseClientBuilderInterface.java

@@ -34,7 +34,7 @@
 import software.amazon.awssdk.codegen.poet.ClassSpec;
 import software.amazon.awssdk.codegen.poet.PoetUtils;
 import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
-import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
+import software.amazon.awssdk.codegen.utils.AuthUtils;
 import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
 import software.amazon.awssdk.utils.internal.CodegenNamingUtils;
 
@@ -164,7 +164,7 @@ private MethodSpec clientContextParamSetter(String name, ClientContextParam para
     }
 
     private boolean generateTokenProviderMethod() {
-        return BearerAuthUtils.usesBearerAuth(model);
+        return AuthUtils.usesBearerAuth(model);
     }
 
     private MethodSpec tokenProviderMethod() {

codegen/src/main/java/software/amazon/awssdk/codegen/poet/builder/SyncClientBuilderClass.java

@@ -27,7 +27,8 @@
 import software.amazon.awssdk.codegen.poet.ClassSpec;
 import software.amazon.awssdk.codegen.poet.PoetUtils;
 import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
-import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
+import software.amazon.awssdk.codegen.utils.AuthUtils;
+import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 import software.amazon.awssdk.core.client.config.SdkClientOption;
 
 public class SyncClientBuilderClass implements ClassSpec {
@@ -72,7 +73,7 @@ public TypeSpec poetSpec() {
             builder.addMethod(endpointProviderMethod());
         }
 
-        if (BearerAuthUtils.usesBearerAuth(model)) {
+        if (AuthUtils.usesBearerAuth(model)) {
             builder.addMethod(tokenProviderMethodImpl());
         }
 
@@ -120,7 +121,10 @@ private MethodSpec buildClientMethod() {
                              .addAnnotation(Override.class)
                              .addModifiers(Modifier.PROTECTED, Modifier.FINAL)
                              .returns(clientInterfaceName)
-                             .addCode("return new $T(super.syncClientConfiguration());", clientClassName)
+                             .addStatement("$T clientConfiguration = super.syncClientConfiguration()",
+                                           SdkClientConfiguration.class)
+                             .addStatement("this.validateClientOptions(clientConfiguration)")
+                             .addCode("return new $T(clientConfiguration);", clientClassName)
                              .build();
     }
 

codegen/src/main/java/software/amazon/awssdk/codegen/poet/client/specs/ProtocolSpec.java

@@ -33,7 +33,7 @@
 import software.amazon.awssdk.codegen.model.intermediate.ShapeType;
 import software.amazon.awssdk.codegen.model.service.AuthType;
 import software.amazon.awssdk.codegen.poet.PoetExtension;
-import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
+import software.amazon.awssdk.codegen.utils.AuthUtils;
 import software.amazon.awssdk.core.CredentialType;
 import software.amazon.awssdk.core.client.handler.SyncClientHandler;
 import software.amazon.awssdk.core.runtime.transform.AsyncStreamingRequestMarshaller;
@@ -114,7 +114,7 @@ default String discoveredEndpoint(OperationModel opModel) {
 
     default CodeBlock credentialType(OperationModel opModel, IntermediateModel model) {
 
-        if (BearerAuthUtils.isOpBearerAuth(model, opModel)) {
+        if (AuthUtils.isOpBearerAuth(model, opModel)) {
             return CodeBlock.of(".credentialType($T.TOKEN)\n", CredentialType.class);
         } else {
             return CodeBlock.of("");

codegen/src/main/java/software/amazon/awssdk/codegen/utils/BearerAuthUtils.java → codegen/src/main/java/software/amazon/awssdk/codegen/utils/AuthUtils.java

@@ -19,8 +19,8 @@
 import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
 import software.amazon.awssdk.codegen.model.service.AuthType;
 
-public final class BearerAuthUtils {
-    private BearerAuthUtils() {
+public final class AuthUtils {
+    private AuthUtils() {
     }
 
     /**
@@ -36,6 +36,16 @@ public static boolean usesBearerAuth(IntermediateModel model) {
                     .anyMatch(authType -> authType == AuthType.BEARER);
     }
 
+    public static boolean usesAwsAuth(IntermediateModel model) {
+        if (isServiceAwsAuthType(model)) {
+            return true;
+        }
+
+        return model.getOperations().values().stream()
+                    .map(OperationModel::getAuthType)
+                    .anyMatch(AuthUtils::isAuthTypeAws);
+    }
+
     /**
      * Returns {@code true} if the operation should use bearer auth.
      */
@@ -50,6 +60,26 @@ private static boolean isServiceBearerAuth(IntermediateModel model) {
         return model.getMetadata().getAuthType() == AuthType.BEARER;
     }
 
+    private static boolean isServiceAwsAuthType(IntermediateModel model) {
+        AuthType authType = model.getMetadata().getAuthType();
+        return isAuthTypeAws(authType);
+    }
+
+    private static boolean isAuthTypeAws(AuthType authType) {
+        if (authType == null) {
+            return false;
+        }
+
+        switch (authType) {
+            case V4:
+            case S3:
+            case S3V4:
+                return true;
+            default:
+                return false;
+        }
+    }
+
     private static boolean hasNoAuthType(OperationModel opModel) {
         return opModel.getAuthType() == null || opModel.getAuthType() == AuthType.NONE;
     }

codegen/src/test/java/software/amazon/awssdk/codegen/utils/BearerAuthUtilsTest.java → codegen/src/test/java/software/amazon/awssdk/codegen/utils/AuthUtilsTest.java

@@ -31,7 +31,7 @@
 import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
 import software.amazon.awssdk.codegen.model.service.AuthType;
 
-public class BearerAuthUtilsTest {
+public class AuthUtilsTest {
 
     @ParameterizedTest
     @MethodSource("serviceValues")
@@ -40,7 +40,7 @@ public void testIfServiceHasBearerAuth(AuthType serviceAuthType,
                                            Boolean expectedResult) {
         IntermediateModel model = modelWith(serviceAuthType);
         model.setOperations(createOperations(opAuthTypes));
-        assertThat(BearerAuthUtils.usesBearerAuth(model)).isEqualTo(expectedResult);
+        assertThat(AuthUtils.usesBearerAuth(model)).isEqualTo(expectedResult);
     }
 
     private static Stream<Arguments> serviceValues() {
@@ -53,12 +53,32 @@ private static Stream<Arguments> serviceValues() {
                          Arguments.of(AuthType.S3V4, oneBearerOp, true));
     }
 
+    @ParameterizedTest
+    @MethodSource("awsAuthServiceValues")
+    public void testIfServiceHasAwsAuthAuth(AuthType serviceAuthType,
+                                           List<AuthType> opAuthTypes,
+                                           Boolean expectedResult) {
+        IntermediateModel model = modelWith(serviceAuthType);
+        model.setOperations(createOperations(opAuthTypes));
+        assertThat(AuthUtils.usesAwsAuth(model)).isEqualTo(expectedResult);
+    }
+
+    private static Stream<Arguments> awsAuthServiceValues() {
+        List<AuthType> oneAwsAuthOp = Arrays.asList(AuthType.V4, AuthType.BEARER, AuthType.NONE);
+        List<AuthType> noAwsAuthOp = Arrays.asList(AuthType.BEARER, AuthType.NONE);
+
+        return Stream.of(Arguments.of(AuthType.BEARER, oneAwsAuthOp, true),
+                         Arguments.of(AuthType.BEARER, noAwsAuthOp, false),
+                         Arguments.of(AuthType.V4, oneAwsAuthOp, true),
+                         Arguments.of(AuthType.V4, noAwsAuthOp, true));
+    }
+
     @ParameterizedTest
     @MethodSource("opValues")
     public void testIfOperationIsBearerAuth(AuthType serviceAuthType, AuthType opAuthType, Boolean expectedResult) {
         IntermediateModel model = modelWith(serviceAuthType);
         OperationModel opModel = opModelWith(opAuthType);
-        assertThat(BearerAuthUtils.isOpBearerAuth(model, opModel)).isEqualTo(expectedResult);
+        assertThat(AuthUtils.isOpBearerAuth(model, opModel)).isEqualTo(expectedResult);
     }
 
     private static Stream<Arguments> opValues() {

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/test-async-client-builder-class.java

@@ -4,6 +4,7 @@
 import software.amazon.awssdk.annotations.SdkInternalApi;
 import software.amazon.awssdk.auth.token.credentials.SdkTokenProvider;
 import software.amazon.awssdk.awscore.client.config.AwsClientOption;
+import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
 
 /**
  * Internal implementation of {@link JsonAsyncClientBuilder}.
@@ -20,6 +21,8 @@ public DefaultJsonAsyncClientBuilder tokenProvider(SdkTokenProvider tokenProvide
 
     @Override
     protected final JsonAsyncClient buildClient() {
-        return new DefaultJsonAsyncClient(super.asyncClientConfiguration());
+        SdkClientConfiguration clientConfiguration = super.asyncClientConfiguration();
+        this.validateClientOptions(clientConfiguration);
+        return new DefaultJsonAsyncClient(clientConfiguration);
     }
 }

codegen/src/test/resources/software/amazon/awssdk/codegen/poet/builder/test-bearer-auth-client-builder-class.java

@@ -16,6 +16,7 @@
 import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
 import software.amazon.awssdk.core.signer.Signer;
 import software.amazon.awssdk.utils.CollectionUtils;
+import software.amazon.awssdk.utils.Validate;
 
 /**
  * Internal base class for {@link DefaultJsonClientBuilder} and {@link DefaultJsonAsyncClientBuilder}.
@@ -65,4 +66,11 @@ private SdkTokenProvider defaultTokenProvider() {
     private Signer defaultTokenSigner() {
         return BearerTokenSigner.create();
     }
-}
+
+    protected static void validateClientOptions(SdkClientConfiguration c) {
+        Validate.notNull(c.option(SdkAdvancedClientOption.TOKEN_SIGNER),
+                         "The 'overrideConfiguration.advancedOption[TOKEN_SIGNER]' must be configured in the client builder.");
+        Validate.notNull(c.option(AwsClientOption.TOKEN_PROVIDER),
+                         "The 'overrideConfiguration.advancedOption[TOKEN_PROVIDER]' must be configured in the client builder.");
+    }
+}