Skip to content

Commit b11b3bd

Browse files
dagnirdagnir
committed
Validate signer client options based on authtype
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). This change only does the appropriate validations based on the authtypes present in the service model.
1 parent 46e49ba commit b11b3bd

File tree

27 files changed

+1110
-42
lines changed

27 files changed

+1110
-42
lines changed

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
{
2+
"type": "bugfix",
3+
"category": "AWS SDK for Java v2",
4+
"contributor": "",
5+
"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."
6+
}

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
import software.amazon.awssdk.codegen.poet.ClassSpec;
2828
import software.amazon.awssdk.codegen.poet.PoetUtils;
2929
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
30-
import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
30+
import software.amazon.awssdk.codegen.utils.AuthUtils;
31+
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
3132
import software.amazon.awssdk.core.client.config.SdkClientOption;
3233

3334
public class AsyncClientBuilderClass implements ClassSpec {
@@ -72,7 +73,7 @@ public TypeSpec poetSpec() {
7273
builder.addMethod(endpointProviderMethod());
7374
}
7475

75-
if (BearerAuthUtils.usesBearerAuth(model)) {
76+
if (AuthUtils.usesBearerAuth(model)) {
7677
builder.addMethod(bearerTokenProviderMethod());
7778
}
7879

@@ -120,7 +121,9 @@ private MethodSpec buildClientMethod() {
120121
.addAnnotation(Override.class)
121122
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
122123
.returns(clientInterfaceName)
123-
.addCode("return new $T(super.asyncClientConfiguration());", clientClassName)
124+
.addStatement("$T clientConfiguration = super.asyncClientConfiguration()", SdkClientConfiguration.class)
125+
.addStatement("this.validateClientOptions(clientConfiguration)")
126+
.addCode("return new $T(clientConfiguration);", clientClassName)
124127
.build();
125128
}
126129

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

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
import software.amazon.awssdk.codegen.poet.ClassSpec;
4949
import software.amazon.awssdk.codegen.poet.PoetUtils;
5050
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
51-
import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
51+
import software.amazon.awssdk.codegen.utils.AuthUtils;
5252
import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
5353
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
5454
import software.amazon.awssdk.core.client.config.SdkClientOption;
@@ -83,15 +83,15 @@ public BaseClientBuilderClass(IntermediateModel model) {
8383
@Override
8484
public TypeSpec poetSpec() {
8585
TypeSpec.Builder builder =
86-
PoetUtils.createClassBuilder(builderClassName)
87-
.addModifiers(Modifier.ABSTRACT)
88-
.addAnnotation(SdkInternalApi.class)
89-
.addTypeVariable(PoetUtils.createBoundedTypeVariableName("B", builderInterfaceName, "B", "C"))
90-
.addTypeVariable(TypeVariableName.get("C"))
91-
.superclass(PoetUtils.createParameterizedTypeName(AwsDefaultClientBuilder.class, "B", "C"))
92-
.addJavadoc("Internal base class for {@link $T} and {@link $T}.",
93-
ClassName.get(basePackage, model.getMetadata().getSyncBuilder()),
94-
ClassName.get(basePackage, model.getMetadata().getAsyncBuilder()));
86+
PoetUtils.createClassBuilder(builderClassName)
87+
.addModifiers(Modifier.ABSTRACT)
88+
.addAnnotation(SdkInternalApi.class)
89+
.addTypeVariable(PoetUtils.createBoundedTypeVariableName("B", builderInterfaceName, "B", "C"))
90+
.addTypeVariable(TypeVariableName.get("C"))
91+
.superclass(PoetUtils.createParameterizedTypeName(AwsDefaultClientBuilder.class, "B", "C"))
92+
.addJavadoc("Internal base class for {@link $T} and {@link $T}.",
93+
ClassName.get(basePackage, model.getMetadata().getSyncBuilder()),
94+
ClassName.get(basePackage, model.getMetadata().getAsyncBuilder()));
9595

9696
// Only services that require endpoint discovery for at least one of their operations get a default value of
9797
// 'true'
@@ -126,13 +126,15 @@ public TypeSpec poetSpec() {
126126
.addMethod(beanStyleSetServiceConfigurationMethod());
127127
}
128128

129-
if (BearerAuthUtils.usesBearerAuth(model)) {
129+
if (AuthUtils.usesBearerAuth(model)) {
130130
builder.addMethod(defaultBearerTokenProviderMethod());
131131
builder.addMethod(defaultTokenAuthSignerMethod());
132132
}
133133

134134
addServiceHttpConfigIfNeeded(builder, model);
135135

136+
builder.addMethod(validateClientOptionsMethod());
137+
136138

137139
return builder.build();
138140
}
@@ -205,7 +207,7 @@ private MethodSpec mergeServiceDefaultsMethod() {
205207
SdkClientOption.class, ClassName.bestGuess(clientConfigClassName));
206208
}
207209

208-
if (BearerAuthUtils.usesBearerAuth(model)) {
210+
if (AuthUtils.usesBearerAuth(model)) {
209211
builder.addCode(".option($T.TOKEN_PROVIDER, defaultTokenProvider())\n", AwsClientOption.class);
210212
builder.addCode(".option($T.TOKEN_SIGNER, defaultTokenSigner())", SdkAdvancedClientOption.class);
211213
}
@@ -217,7 +219,7 @@ private MethodSpec mergeServiceDefaultsMethod() {
217219
private Optional<MethodSpec> mergeInternalDefaultsMethod() {
218220
String userAgent = model.getCustomizationConfig().getUserAgent();
219221
RetryMode defaultRetryMode = model.getCustomizationConfig().getDefaultRetryMode();
220-
222+
221223
// If none of the options are customized, then we do not need to bother overriding the method
222224
if (userAgent == null && defaultRetryMode == null) {
223225
return Optional.empty();
@@ -246,10 +248,10 @@ private MethodSpec finalizeServiceConfigurationMethod() {
246248
String requestHandlerPath = String.format("%s/execution.interceptors", requestHandlerDirectory);
247249

248250
MethodSpec.Builder builder = MethodSpec.methodBuilder("finalizeServiceConfiguration")
249-
.addAnnotation(Override.class)
250-
.addModifiers(PROTECTED, FINAL)
251-
.returns(SdkClientConfiguration.class)
252-
.addParameter(SdkClientConfiguration.class, "config");
251+
.addAnnotation(Override.class)
252+
.addModifiers(PROTECTED, FINAL)
253+
.returns(SdkClientConfiguration.class)
254+
.addParameter(SdkClientConfiguration.class, "config");
253255

254256
// Initialize configuration values
255257

@@ -289,7 +291,7 @@ private MethodSpec finalizeServiceConfigurationMethod() {
289291
CollectionUtils.class);
290292

291293
builder.addCode("interceptors = $T.mergeLists(interceptors, config.option($T.EXECUTION_INTERCEPTORS));\n",
292-
CollectionUtils.class, SdkClientOption.class);
294+
CollectionUtils.class, SdkClientOption.class);
293295

294296
if (model.getMetadata().isQueryProtocol()) {
295297
TypeName listType = ParameterizedTypeName.get(List.class, ExecutionInterceptor.class);
@@ -488,7 +490,7 @@ private void mergeServiceConfiguration(MethodSpec.Builder builder, String client
488490

489491
private MethodSpec setServiceConfigurationMethod() {
490492
ClassName serviceConfiguration = ClassName.get(basePackage,
491-
model.getCustomizationConfig().getServiceConfig().getClassName());
493+
model.getCustomizationConfig().getServiceConfig().getClassName());
492494
return MethodSpec.methodBuilder("serviceConfiguration")
493495
.addModifiers(Modifier.PUBLIC)
494496
.returns(TypeVariableName.get("B"))
@@ -501,7 +503,7 @@ private MethodSpec setServiceConfigurationMethod() {
501503

502504
private MethodSpec beanStyleSetServiceConfigurationMethod() {
503505
ClassName serviceConfiguration = ClassName.get(basePackage,
504-
model.getCustomizationConfig().getServiceConfig().getClassName());
506+
model.getCustomizationConfig().getServiceConfig().getClassName());
505507
return MethodSpec.methodBuilder("setServiceConfiguration")
506508
.addModifiers(Modifier.PUBLIC)
507509
.addParameter(serviceConfiguration, "serviceConfiguration")
@@ -623,4 +625,33 @@ private boolean hasClientContextParams() {
623625
Map<String, ClientContextParam> clientContextParams = model.getClientContextParams();
624626
return clientContextParams != null && !clientContextParams.isEmpty();
625627
}
628+
629+
private MethodSpec validateClientOptionsMethod() {
630+
MethodSpec.Builder builder = MethodSpec.methodBuilder("validateClientOptions")
631+
.addModifiers(PROTECTED, Modifier.STATIC)
632+
.addParameter(SdkClientConfiguration.class, "c")
633+
.returns(void.class);
634+
635+
if (AuthUtils.usesAwsAuth(model)) {
636+
builder.addStatement("$T.notNull(c.option($T.SIGNER), $S)",
637+
Validate.class,
638+
SdkAdvancedClientOption.class,
639+
"The 'overrideConfiguration.advancedOption[SIGNER]' must be configured in the client builder.");
640+
}
641+
642+
if (AuthUtils.usesBearerAuth(model)) {
643+
builder.addStatement("$T.notNull(c.option($T.TOKEN_SIGNER), $S)",
644+
Validate.class,
645+
SdkAdvancedClientOption.class,
646+
"The 'overrideConfiguration.advancedOption[TOKEN_SIGNER]' "
647+
+ "must be configured in the client builder.");
648+
builder.addStatement("$T.notNull(c.option($T.TOKEN_PROVIDER), $S)",
649+
Validate.class,
650+
AwsClientOption.class,
651+
"The 'overrideConfiguration.advancedOption[TOKEN_PROVIDER]' "
652+
+ "must be configured in the client builder.");
653+
}
654+
655+
return builder.build();
656+
}
626657
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import software.amazon.awssdk.codegen.poet.ClassSpec;
3535
import software.amazon.awssdk.codegen.poet.PoetUtils;
3636
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
37-
import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
37+
import software.amazon.awssdk.codegen.utils.AuthUtils;
3838
import software.amazon.awssdk.core.client.config.SdkAdvancedClientOption;
3939
import software.amazon.awssdk.utils.internal.CodegenNamingUtils;
4040

@@ -164,7 +164,7 @@ private MethodSpec clientContextParamSetter(String name, ClientContextParam para
164164
}
165165

166166
private boolean generateTokenProviderMethod() {
167-
return BearerAuthUtils.usesBearerAuth(model);
167+
return AuthUtils.usesBearerAuth(model);
168168
}
169169

170170
private MethodSpec tokenProviderMethod() {

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
import software.amazon.awssdk.codegen.poet.ClassSpec;
2828
import software.amazon.awssdk.codegen.poet.PoetUtils;
2929
import software.amazon.awssdk.codegen.poet.rules.EndpointRulesSpecUtils;
30-
import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
30+
import software.amazon.awssdk.codegen.utils.AuthUtils;
31+
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
3132
import software.amazon.awssdk.core.client.config.SdkClientOption;
3233

3334
public class SyncClientBuilderClass implements ClassSpec {
@@ -72,7 +73,7 @@ public TypeSpec poetSpec() {
7273
builder.addMethod(endpointProviderMethod());
7374
}
7475

75-
if (BearerAuthUtils.usesBearerAuth(model)) {
76+
if (AuthUtils.usesBearerAuth(model)) {
7677
builder.addMethod(tokenProviderMethodImpl());
7778
}
7879

@@ -120,7 +121,10 @@ private MethodSpec buildClientMethod() {
120121
.addAnnotation(Override.class)
121122
.addModifiers(Modifier.PROTECTED, Modifier.FINAL)
122123
.returns(clientInterfaceName)
123-
.addCode("return new $T(super.syncClientConfiguration());", clientClassName)
124+
.addStatement("$T clientConfiguration = super.syncClientConfiguration()",
125+
SdkClientConfiguration.class)
126+
.addStatement("this.validateClientOptions(clientConfiguration)")
127+
.addCode("return new $T(clientConfiguration);", clientClassName)
124128
.build();
125129
}
126130

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import software.amazon.awssdk.codegen.model.intermediate.ShapeType;
3434
import software.amazon.awssdk.codegen.model.service.AuthType;
3535
import software.amazon.awssdk.codegen.poet.PoetExtension;
36-
import software.amazon.awssdk.codegen.utils.BearerAuthUtils;
36+
import software.amazon.awssdk.codegen.utils.AuthUtils;
3737
import software.amazon.awssdk.core.CredentialType;
3838
import software.amazon.awssdk.core.client.handler.SyncClientHandler;
3939
import software.amazon.awssdk.core.runtime.transform.AsyncStreamingRequestMarshaller;
@@ -114,7 +114,7 @@ default String discoveredEndpoint(OperationModel opModel) {
114114

115115
default CodeBlock credentialType(OperationModel opModel, IntermediateModel model) {
116116

117-
if (BearerAuthUtils.isOpBearerAuth(model, opModel)) {
117+
if (AuthUtils.isOpBearerAuth(model, opModel)) {
118118
return CodeBlock.of(".credentialType($T.TOKEN)\n", CredentialType.class);
119119
} else {
120120
return CodeBlock.of("");

‎codegen/src/main/java/software/amazon/awssdk/codegen/utils/BearerAuthUtils.java renamed to ‎codegen/src/main/java/software/amazon/awssdk/codegen/utils/AuthUtils.java

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
2020
import software.amazon.awssdk.codegen.model.service.AuthType;
2121

22-
public final class BearerAuthUtils {
23-
private BearerAuthUtils() {
22+
public final class AuthUtils {
23+
private AuthUtils() {
2424
}
2525

2626
/**
@@ -36,6 +36,16 @@ public static boolean usesBearerAuth(IntermediateModel model) {
3636
.anyMatch(authType -> authType == AuthType.BEARER);
3737
}
3838

39+
public static boolean usesAwsAuth(IntermediateModel model) {
40+
if (isServiceAwsAuthType(model)) {
41+
return true;
42+
}
43+
44+
return model.getOperations().values().stream()
45+
.map(OperationModel::getAuthType)
46+
.anyMatch(AuthUtils::isAuthTypeAws);
47+
}
48+
3949
/**
4050
* Returns {@code true} if the operation should use bearer auth.
4151
*/
@@ -50,6 +60,26 @@ private static boolean isServiceBearerAuth(IntermediateModel model) {
5060
return model.getMetadata().getAuthType() == AuthType.BEARER;
5161
}
5262

63+
private static boolean isServiceAwsAuthType(IntermediateModel model) {
64+
AuthType authType = model.getMetadata().getAuthType();
65+
return isAuthTypeAws(authType);
66+
}
67+
68+
private static boolean isAuthTypeAws(AuthType authType) {
69+
if (authType == null) {
70+
return false;
71+
}
72+
73+
switch (authType) {
74+
case V4:
75+
case S3:
76+
case S3V4:
77+
return true;
78+
default:
79+
return false;
80+
}
81+
}
82+
5383
private static boolean hasNoAuthType(OperationModel opModel) {
5484
return opModel.getAuthType() == null || opModel.getAuthType() == AuthType.NONE;
5585
}

‎codegen/src/test/java/software/amazon/awssdk/codegen/utils/BearerAuthUtilsTest.java renamed to ‎codegen/src/test/java/software/amazon/awssdk/codegen/utils/AuthUtilsTest.java

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@
3131
import software.amazon.awssdk.codegen.model.intermediate.OperationModel;
3232
import software.amazon.awssdk.codegen.model.service.AuthType;
3333

34-
public class BearerAuthUtilsTest {
34+
public class AuthUtilsTest {
3535

3636
@ParameterizedTest
3737
@MethodSource("serviceValues")
@@ -40,7 +40,7 @@ public void testIfServiceHasBearerAuth(AuthType serviceAuthType,
4040
Boolean expectedResult) {
4141
IntermediateModel model = modelWith(serviceAuthType);
4242
model.setOperations(createOperations(opAuthTypes));
43-
assertThat(BearerAuthUtils.usesBearerAuth(model)).isEqualTo(expectedResult);
43+
assertThat(AuthUtils.usesBearerAuth(model)).isEqualTo(expectedResult);
4444
}
4545

4646
private static Stream<Arguments> serviceValues() {
@@ -53,12 +53,32 @@ private static Stream<Arguments> serviceValues() {
5353
Arguments.of(AuthType.S3V4, oneBearerOp, true));
5454
}
5555

56+
@ParameterizedTest
57+
@MethodSource("awsAuthServiceValues")
58+
public void testIfServiceHasAwsAuthAuth(AuthType serviceAuthType,
59+
List<AuthType> opAuthTypes,
60+
Boolean expectedResult) {
61+
IntermediateModel model = modelWith(serviceAuthType);
62+
model.setOperations(createOperations(opAuthTypes));
63+
assertThat(AuthUtils.usesAwsAuth(model)).isEqualTo(expectedResult);
64+
}
65+
66+
private static Stream<Arguments> awsAuthServiceValues() {
67+
List<AuthType> oneAwsAuthOp = Arrays.asList(AuthType.V4, AuthType.BEARER, AuthType.NONE);
68+
List<AuthType> noAwsAuthOp = Arrays.asList(AuthType.BEARER, AuthType.NONE);
69+
70+
return Stream.of(Arguments.of(AuthType.BEARER, oneAwsAuthOp, true),
71+
Arguments.of(AuthType.BEARER, noAwsAuthOp, false),
72+
Arguments.of(AuthType.V4, oneAwsAuthOp, true),
73+
Arguments.of(AuthType.V4, noAwsAuthOp, true));
74+
}
75+
5676
@ParameterizedTest
5777
@MethodSource("opValues")
5878
public void testIfOperationIsBearerAuth(AuthType serviceAuthType, AuthType opAuthType, Boolean expectedResult) {
5979
IntermediateModel model = modelWith(serviceAuthType);
6080
OperationModel opModel = opModelWith(opAuthType);
61-
assertThat(BearerAuthUtils.isOpBearerAuth(model, opModel)).isEqualTo(expectedResult);
81+
assertThat(AuthUtils.isOpBearerAuth(model, opModel)).isEqualTo(expectedResult);
6282
}
6383

6484
private static Stream<Arguments> opValues() {

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import software.amazon.awssdk.annotations.SdkInternalApi;
55
import software.amazon.awssdk.auth.token.credentials.SdkTokenProvider;
66
import software.amazon.awssdk.awscore.client.config.AwsClientOption;
7+
import software.amazon.awssdk.core.client.config.SdkClientConfiguration;
78

89
/**
910
* Internal implementation of {@link JsonAsyncClientBuilder}.
@@ -20,6 +21,8 @@ public DefaultJsonAsyncClientBuilder tokenProvider(SdkTokenProvider tokenProvide
2021

2122
@Override
2223
protected final JsonAsyncClient buildClient() {
23-
return new DefaultJsonAsyncClient(super.asyncClientConfiguration());
24+
SdkClientConfiguration clientConfiguration = super.asyncClientConfiguration();
25+
this.validateClientOptions(clientConfiguration);
26+
return new DefaultJsonAsyncClient(clientConfiguration);
2427
}
2528
}

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

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import software.amazon.awssdk.core.interceptor.ExecutionInterceptor;
1717
import software.amazon.awssdk.core.signer.Signer;
1818
import software.amazon.awssdk.utils.CollectionUtils;
19+
import software.amazon.awssdk.utils.Validate;
1920

2021
/**
2122
* Internal base class for {@link DefaultJsonClientBuilder} and {@link DefaultJsonAsyncClientBuilder}.
@@ -65,4 +66,11 @@ private SdkTokenProvider defaultTokenProvider() {
6566
private Signer defaultTokenSigner() {
6667
return BearerTokenSigner.create();
6768
}
68-
}
69+
70+
protected static void validateClientOptions(SdkClientConfiguration c) {
71+
Validate.notNull(c.option(SdkAdvancedClientOption.TOKEN_SIGNER),
72+
"The 'overrideConfiguration.advancedOption[TOKEN_SIGNER]' must be configured in the client builder.");
73+
Validate.notNull(c.option(AwsClientOption.TOKEN_PROVIDER),
74+
"The 'overrideConfiguration.advancedOption[TOKEN_PROVIDER]' must be configured in the client builder.");
75+
}
76+
}

0 commit comments

Comments
 (0)