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

Can't create S3 presigned URLs #2874

Closed
trusanen-fluidit opened this issue Jun 29, 2023 · 9 comments · Fixed by #2876
Closed

Can't create S3 presigned URLs #2874

trusanen-fluidit opened this issue Jun 29, 2023 · 9 comments · Fixed by #2876
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.

Comments

@trusanen-fluidit
Copy link

Describe the bug

Trying to create a presigned URL with object.presigned_url(:get) results in a TypeError.

Expected Behavior

The method call should return a valid presigned URL for S3

Current Behavior

A TypeError is raised:

/Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sigv4-1.6.0/lib/aws-sigv4/signer.rb:729:in `-': Time can't be coerced into Integer (TypeError)
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sigv4-1.6.0/lib/aws-sigv4/signer.rb:729:in `presigned_url_expiration'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sigv4-1.6.0/lib/aws-sigv4/signer.rb:426:in `presign_url'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/presigner.rb:243:in `block in sign_but_dont_send'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/plugins/request_callback.rb:87:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/streaming_retry.rb:71:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/s3_signer.rb:73:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/s3_host_id.rb:17:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/xml/error_handler.rb:10:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/transfer_encoding.rb:26:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/helpful_socket_errors.rb:12:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/s3_signer.rb:48:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/redirects.rb:20:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/retry_errors.rb:360:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/user_agent.rb:37:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/http_checksum.rb:19:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/endpoint_pattern.rb:30:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/checksum_algorithm.rb:136:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/expect_100_continue.rb:23:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/bucket_name_restrictions.rb:21:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/rest/handler.rb:10:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/recursion_detection.rb:18:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/endpoints.rb:41:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/endpoint_discovery.rb:84:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/plugins/endpoint.rb:47:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/param_validator.rb:26:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/plugins/raise_response_errors.rb:16:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/skip_whole_multipart_get_checksums.rb:18:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/sse_cpk.rb:24:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/dualstack.rb:21:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/plugins/accelerate.rb:43:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/checksum_algorithm.rb:111:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/jsonvalue_converter.rb:16:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/idempotency_token.rb:19:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/param_converter.rb:26:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/plugins/request_callback.rb:71:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/aws-sdk-core/plugins/response_paging.rb:12:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/plugins/response_target.rb:24:in `call'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/presigner.rb:187:in `block in handle_presigned_url_context'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-core-3.176.0/lib/seahorse/client/request.rb:72:in `send_request'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/presigner.rb:148:in `_presigned_request'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/presigner.rb:79:in `presigned_url'
	from /Users/trusanen/ruby-test/vendor/bundle/ruby/2.6.0/gems/aws-sdk-s3-1.127.0/lib/aws-sdk-s3/customizations/object.rb:219:in `presigned_url'
	from init.rb:7:in `<main>'

Reproduction Steps

With the following Gemfile and script, the TypeError is raised with bundle install --path vendor/bundle && bundle exec ruby init.rb

Gemfile

source 'https://rubygems.org'
gem 'aws-sdk-core', '3.176.0'
gem 'aws-sdk-s3', '1.127.0'
gem 'aws-sdk-kms', '~> 1'
gem 'aws-sigv4', '1.6'

Script

require 'aws-sdk-s3'

resource = Aws::S3::Resource.new
object = resource.bucket('my-bucket').object('my-object')
puts object.presigned_url(:get)

Possible Solution

There seems to be a bug in the recently added presigned_url_expiration method.

Additional Information/Context

Tested with aws-sdk-core (3.174.0), aws-sdk-s3 (1.126.0) and aws-sigv4 (1.5.0) and with these versions the method call works as expected and a valid presigned URL is printed to the console.

Gem name ('aws-sdk', 'aws-sdk-resources' or service gems like 'aws-sdk-s3') and its version

aws-sdk-core (3.176.0), aws-sdk-s3 (1.127.0), aws-sigv4 (1.6.0)

Environment details (Version of Ruby, OS environment)

ruby 2.6.10p210 (2022-04-12 revision 67958) [universal.x86_64-darwin22]

@trusanen-fluidit trusanen-fluidit added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jun 29, 2023
@mullermp
Copy link
Contributor

I'm sorry this happened. The change looks at credentials expiration and compares it to expires_in. What type of credentials are you using? Are you able to create a presigned url using Presigner class directly instead of using a resource object?

@mullermp
Copy link
Contributor

I'm not able to reproduce this. Using either AssumeRoleCredentials or Static Credentials (access key, etc), I can generate a presigned URL using either of these:

Aws::S3::Resource.new(client: Aws::S3::Client.new).bucket('mamuller-us-west-2').object('test').presigned_url(:get)

Aws::S3::Resource.new(client: Aws::S3::Client.new(credentials: Aws::AssumeRoleCredentials.new(role_session_name: 'test', role_arn: role_arn))).bucket('mamuller-us-west-2').object('test').presigned_url(:get)

@trusanen-fluidit
Copy link
Author

Hi @mullermp and thanks for the fast response.

Currently I have AWS_PROFILE environment variable set to a profile with an active AWS SSO configuration. For example aws s3 presign s3://my-bucket/my-object creates a working URL that I can use in my browser. I'm assuming the Ruby script utilises the same credentials through the default credential provider chain.

The same TypeError gets raised if I try to use the Presigner to create the URL

require 'aws-sdk-s3'

signer = Aws::S3::Presigner.new
puts signer.presigned_url(:get_object, bucket: "my-bucket", key: "my-object")

@mullermp
Copy link
Contributor

I can take a look at that. Can you confirm through client.config.credentials that it is indeed an instance of SSO credentials?

@trusanen-fluidit
Copy link
Author

Thanks. The following script will print #<Aws::SSOCredentials:0x00007f9407bbc9c8>

require 'aws-sdk-s3'

client = Aws::S3::Client.new
puts client.config.credentials

@mullermp
Copy link
Contributor

I think I see the issue. The credentials expiration is a "long" instead of a "timestamp" for SSO's RoleCredentials API. This (integer) expiration is set directly in SSOCredentials, instead of a Time. i.e.

[6] pry(Aws)> (12312345 - Time.now).to_i
TypeError: Time can't be coerced into Integer

I think that we simply need to wrap this with Time.at. I can try to get a fix out today for this. I personally have never used the SSO flow - it would be helpful if you can confirm the output of Time.at(Aws::SSOCredentials.new.expiration) would produce a valid Time that expires in the short future as expected.

@trusanen-fluidit
Copy link
Author

Thanks for looking into the issue.

I took your suggestion and monkey patched https://github.com/aws/aws-sdk-ruby/blob/version-3/gems/aws-sigv4/lib/aws-sigv4/signer.rb#L729 into

expiration_seconds = (Time.at(expiration) - Time.now).to_i

With this change the initial script is working as expected.

@github-actions
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@danielmklein
Copy link

Forgive me for the ping, but we encountered an issue that rhymed with this one, and @mullermp I just want to say thank you for fixing this so quickly, cleanly, and completely. 👏🏻 👏🏻

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. needs-triage This issue or PR still needs to be triaged.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants