Fix isClientSignerOverridden to check actual value

[gosar]

Motivation and Context

isSignerOverridden is used in AwsExecutionContextBuilder.

The SIGNER_OVERRIDDEN ExecutionAttribute is set here

aws-sdk-java-v2/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java

Line 110 in 32390a2

.putAttribute(SdkExecutionAttribute.SIGNER_OVERRIDDEN, clientConfig.option(SdkClientOption.SIGNER_OVERRIDDEN))

And isSignerOverridden is called here

aws-sdk-java-v2/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java

Line 156 in 32390a2

private static boolean loadOldSigner(ExecutionAttributes attributes, SdkRequest request) {

which is called here

aws-sdk-java-v2/core/aws-core/src/main/java/software/amazon/awssdk/awscore/internal/AwsExecutionContextBuilder.java

Lines 129 to 136 in 32390a2

	if (loadOldSigner(executionAttributes, originalRequest)) {
	AuthorizationStrategyFactory authorizationStrategyFactory =
	new AuthorizationStrategyFactory(interceptorContext.request(), metricCollector, clientConfig);
	AuthorizationStrategy authorizationStrategy =
	authorizationStrategyFactory.strategyFor(executionParams.credentialType());
	authorizationStrategy.addCredentialsToExecutionAttributes(executionAttributes);
	signer = authorizationStrategy.resolveSigner();
	}

So when the client is configured if SIGNER_OVERRIDDEN is non-null, loadSigner would have been true. In master branch, SIGNER_OVERRIDDEN was always null or true. In the SRA branch now, it can be null or true or false. See comment in.

In SRA branch now, Signer is always null, so even if loadSigner is called, the ExecutionContext.signer here will be null, so the SRA HttpSigner gets used, which is all fine. However, the call to addCredentialsToExecutionAttributes means identity resolution happens there a second time (1st time happened in AuthSchemeInterceptor, which happens before this point). More than the issue of happening twice (which adds a small risk of that call failing), because of the derived attribute logic from #4396, addCredentialsToExecutionAttributes will result in the identity to be updated in SelectedAuthScheme, which would break identity property based auth resolution for an upcoming service.

Modifications

Check that the value of SIGNER_OVERRIDDEN is present instead of just isPresent.

Testing

I realized this issue while stepping through the debugger for some other testing I was doing and noticed going into loadSigner branch unexpectedly. Verified after this change, it doesn't go into that branch.

Screenshots (if appropriate)

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)

Checklist

• I have read the CONTRIBUTING document
• Local run of mvn install succeeds
• My code follows the code style of this project
• My change requires a change to the Javadoc documentation
• I have updated the Javadoc documentation accordingly
• I have added tests to cover my changes
• All new and existing tests passed
• I have added a changelog entry. Adding a new entry must be accomplished by running the scripts/new-change script and following the instructions. Commit the new file created by the script in .changes/next-release with your changes.
• My change is to implement 1.11 parity feature and I have updated LaunchChangelog

License

• I confirm that this pull request can be released under the Apache 2 license

[millems]

Yes, if we're still using it, let's not deprecate it.

[sonarqubecloud]

SonarCloud Quality Gate failed.   

4 Bugs
0 Vulnerabilities
2 Security Hotspots
443 Code Smells

84.8% Coverage
3.7% Duplication

Catch issues before they fail your Quality Gate with our IDE extension SonarLint