Skip to content

Conversation

@millems
Copy link
Contributor

@millems millems commented Sep 28, 2023

This is currently redundant, but when useSraAuth is made true, it will ensure that payload signing remains disabled.

@millems millems requested a review from a team as a code owner September 28, 2023 18:05
Copy link
Contributor

@gosar gosar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre SRA does customer have a way to enable payload signing? Thought being, that if there's a way, maybe have a test that confirms that still works.

With SRA, I think customer can enable payload signing like this -

(from branch used for surface area review), which I think should still work with this due to putAttributeIfAbsent.

@millems
Copy link
Contributor Author

millems commented Sep 28, 2023

Pre SRA does customer have a way to enable payload signing? Thought being, that if there's a way, maybe have a test that confirms that still works.

Yes, via interceptors. That's tested in the backwards-compatibility changes I pushed yesterday, but it's not actually tested with the latest changes but useSraAuth = false. Let me add a test for that...

This is currently redundant, but when useSraAuth is made true, it will ensure that payload signing remains disabled.
@millems millems force-pushed the millem/disable-s3-signing branch from 6985723 to 2fa682d Compare September 28, 2023 19:35
@gosar
Copy link
Contributor

gosar commented Sep 28, 2023

Yes, via interceptors.

yeah, I was wondering if there's another supported way (sounds like no), since the S3SignerExecutionAttribute.ENABLE_PAYLOAD_SIGNING is SdkProtectedApi. But yeah, we still want to make sure it continues to work, so the test is good.

With SRA there will be a publicly supported way via SignerProperty! We should add a test for that (non-blocking right now).

My ship it stands anyways!

@zoewangg zoewangg merged commit 9e3d3e3 into feature/master/sra-identity-auth Sep 28, 2023
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug C 4 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 2 Security Hotspots
Code Smell A 473 Code Smells

84.2% 84.2% Coverage
4.0% 4.0% Duplication

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

@gosar gosar mentioned this pull request Dec 29, 2023
12 tasks
@millems millems deleted the millem/disable-s3-signing branch February 5, 2024 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants