Update presigner so host is set and content-hash is excluded from signed-headers #4560
Conversation
haydenbaker
force-pushed
the
haydenbaker/fix-presigner-headers
branch
from
29a6d11
to
471e42d
Compare
.../src/test/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4RequestSignerTest.java
Outdated
Show resolved
Hide resolved
...-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4RequestSigner.java
Show resolved
Hide resolved
...test/java/software/amazon/awssdk/http/auth/aws/internal/signer/DefaultRequestSignerTest.java
Outdated
Show resolved
Hide resolved
haydenbaker
force-pushed
the
haydenbaker/fix-presigner-headers
branch
from
471e42d
to
121a86f
Compare
haydenbaker
force-pushed
the
haydenbaker/fix-presigner-headers
branch
from
121a86f
to
5e18716
Compare
|
SonarCloud Quality Gate failed.
|
| addDateHeader(requestBuilder, formatDateTime(properties.getCredentialScope().getInstant())); | ||
|
|
||
| V4RequestSigningResult result = create(properties).sign(requestBuilder); | ||
| V4RequestSigningResult result = create(properties, getContentHash(requestBuilder)).sign(requestBuilder); |
There was a problem hiding this comment.
thinking aloud: should the contentHash be a constructor input like properties, or input to sign()?
There was a problem hiding this comment.
It's easier if it's a constructor param - sign's signature would change, which is a bigger change. If we decide that x-amz-content-sha256 shouldn't be added as a header unless required (s3), then we'll likely need to refactor it as an input to request-signer and payload-signer
| ((AwsSessionCredentialsIdentity) properties.getCredentials()).sessionToken()); | ||
| } | ||
| // We have to add the host-header here explicitly, since pre-signed request requires it in the signed-header param | ||
| addHostHeader(requestBuilder); |
There was a problem hiding this comment.
Does the v4a signer already do this for presigned case?
Does v4a signer always include x-amz-content-sha256?
There was a problem hiding this comment.
CRT would be the thing that manipulates headers - the signer simply delegates to it. However, the signer does configure CRT to always add the hash header, yes: signingConfig.setSignedBodyHeader(X_AMZ_CONTENT_SHA256)
...-aws/src/main/java/software/amazon/awssdk/http/auth/aws/internal/signer/V4RequestSigner.java
Show resolved
Hide resolved
haydenbaker
merged commit a00ee8a
into
feature/master/sra-identity-auth
Motivation and Context
Modifications
Testing
Screenshots (if appropriate)
Types of changes
Checklist
mvn installsucceedsscripts/new-changescript and following the instructions. Commit the new file created by the script in.changes/next-releasewith your changes.License