Skip to content

Commit

Permalink
Fix Sign to not sign sigv2 cases (#2804)
Browse files Browse the repository at this point in the history
  • Loading branch information
mullermp committed Dec 8, 2022
1 parent 5de4f1b commit 88121bd
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.
2 changes: 2 additions & 0 deletions gems/aws-sdk-core/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
Unreleased Changes
------------------

* Issue - Fix Sign to not sign Sigv2 requests to S3.

3.168.3 (2022-12-02)
------------------

Expand Down
24 changes: 17 additions & 7 deletions gems/aws-sdk-core/lib/aws-sdk-core/plugins/sign.rb
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,25 @@ def self.signer_for(auth_scheme, config, region_override = nil)

class Handler < Seahorse::Client::Handler
def call(context)
signer = Sign.signer_for(
context[:auth_scheme],
context.config,
context[:sigv4_region]
)

signer.sign(context)
# Skip signing if using sigv2 signing from s3_signer in S3
unless v2_signing?(context.config)
signer = Sign.signer_for(
context[:auth_scheme],
context.config,
context[:sigv4_region]
)
signer.sign(context)
end
@handler.call(context)
end

private

def v2_signing?(config)
# 's3' is legacy signing, 'v4' is default
config.respond_to?(:signature_version) &&
config.signature_version == 's3'
end
end

# @api private
Expand Down
21 changes: 17 additions & 4 deletions gems/aws-sdk-core/spec/aws/plugins/sign_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@ module Plugins
}
end


let(:client) { TestClient.new(client_options) }

context 'sigv4' do
let(:auth_scheme) do
{
Expand Down Expand Up @@ -222,10 +222,23 @@ module Plugins
end

it 'raises an error when attempting to sign a request w/out a token' do
client = TestClient.new(client_options.merge(token_provider: nil) )
expect {
client = TestClient.new(client_options.merge(token_provider: nil))
expect do
client.operation
}.to raise_error(Errors::MissingBearerTokenError)
end.to raise_error(Errors::MissingBearerTokenError)
end
end

context 'sigv2' do
before do
allow(Struct).to receive(:responds_to?).with(:signature_version).and_return(true)
allow(Struct).to receive(:signature_version).and_return('s3')
end

it 'skips signing' do
resp = client.operation
req = resp.context.http_request
expect(req.headers['authorization']).to be_nil
end
end

Expand Down

0 comments on commit 88121bd

Please sign in to comment.