Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix(aws-connector): Limit signing to signed headers from original request #1432

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

doodlesbykumbi
Copy link
Contributor

What does this PR do?

Prior to this change the AWS connector was signing requests using all the headers present on the original request. This was resulting signature mismatches and failed auth, particularly visible when creating a new s3 bucket. With this change the aws connector will sign only the headers on the original request, it achieves this by temporarily hiding the rest of the headers before signing, then them after signing.

What ticket does this PR close?

Resolves #1430

Checklists

Change log

  • The CHANGELOG has been updated, or
  • This PR does not include user-facing changes and doesn't require a CHANGELOG update

Test coverage

  • This PR includes new unit and integration tests to go with the code changes, or
  • The changes in this PR do not require tests

Documentation

  • This PR does not require updating any documentation, or
  • Docs (e.g. READMEs) were updated in this PR, and/or there is a follow-on issue to update docs

(For releases only) Manual tests

@doodlesbykumbi doodlesbykumbi requested a review from a team as a code owner October 19, 2021 21:04
internal/plugin/connectors/http/aws/connector.go Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
Copy link
Contributor

@diverdane diverdane left a comment

Choose a reason for hiding this comment

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

Looks good, but I do have one question (to help me understand the change), and one very minor editorial comment.

internal/plugin/connectors/http/aws/aws.go Show resolved Hide resolved
internal/plugin/connectors/http/aws/connector.go Outdated Show resolved Hide resolved
@doodlesbykumbi doodlesbykumbi force-pushed the fix/aws-connector branch 3 times, most recently from caf895f to 8d0bda2 Compare October 20, 2021 21:55
diverdane
diverdane previously approved these changes Oct 20, 2021
Copy link
Contributor

@diverdane diverdane left a comment

Choose a reason for hiding this comment

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

LGTM!!!

…uest

Prior to this change the AWS connector was signing requests using all the headers present on the original request. This was resulting signature mismatches and failed auth, particularly visible when creating a new s3 bucket. With this change the aws connector will sign only the headers on the original request, it achieves this by temporarily hiding the rest of the headers before signing, then them after signing.
@codeclimate
Copy link

codeclimate bot commented Oct 22, 2021

Code Climate has analyzed commit 0bd3002 and detected 1 issue on this pull request.

Here's the issue category breakdown:

Category Count
Style 1

The test coverage on the diff in this pull request is 93.7% (50% is the threshold).

This pull request will bring the total coverage in the repository to 54.8% (1.1% change).

View more on Code Climate.

Copy link
Contributor

@diverdane diverdane left a comment

Choose a reason for hiding this comment

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

LGTM!!!
Thanks for adding UT!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Failed to create new s3 bucket with secretless broker for aws
2 participants