-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Add enhanced presign request feature #1477
Changes from 12 commits
508b77e
6d72846
b59469c
85bebc9
fd5d7e9
50cf8bb
e642334
a7151fc
baf05e6
80393c2
464defa
cc457c1
494fe06
c207f92
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,51 @@ | ||
# language: en | ||
@s3 @presigned_request | ||
Feature: Enhanced presigned request interface | ||
|
||
Background: | ||
Given I create a bucket | ||
|
||
Scenario: Presigning a put object request | ||
When I create a presigned request for "put_object" with: | ||
| key | test | | ||
| acl | public-read | | ||
And I send an HTTP put request with uri headers and body "hello" | ||
And I make an unauthenticated HTTPS GET request for key "test" | ||
Then the response should be "hello" | ||
|
||
Scenario: Presigning a HTTPS get object request | ||
When I put "signed" to the key "retrieve_me" | ||
And I create a presigned request for "get_object" with: | ||
| key | retrieve_me | | ||
And I send an HTTP "get" request with uri and headers | ||
Then the response should be "signed" | ||
|
||
Scenario: Presigning a HTTP get object request | ||
When I put "signed" to the key "retrieve_me" | ||
And I create a non-secure presigned request for "get_object" with: | ||
| key | retrieve_me | | ||
And I send an HTTP "get" request with uri and headers | ||
Then the response should be "signed" | ||
|
||
@headers | ||
Scenario: Presigning a put object request with custom headers | ||
Given I have a custom header hash to sign: | ||
| foo | bar | | ||
When I create a presigned request for "put_object" with the hash and: | ||
| key | test | | ||
| acl | public-read | | ||
And I send an HTTP "put" request with uri and headers | ||
Then Signed headers should be: | ||
| x-amz-acl | public-read | | ||
| x-amz-foo | bar | | ||
Then the response should have a 200 status code | ||
|
||
@headers | ||
Scenario: Presigning a put object request with whitelist headers | ||
When I create a presigned request for "put_object" with: | ||
| key | test | | ||
| cache-control | max-age=20000 | | ||
And I send an HTTP "put" request with uri and headers | ||
Then Signed headers should be: | ||
| x-amz-cache-control | max-age=20000 | | ||
Then the response should have a 200 status code |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,181 @@ | ||
require 'aws-sigv4' | ||
|
||
module Aws | ||
module S3 | ||
|
||
# Utility class for creating a presigned request with presigned URL and signed | ||
# headers hash. | ||
# | ||
# Example Use: | ||
# | ||
# presigned_request = Aws::S3::PresignedRequest.new( | ||
# :put_object, | ||
# bucket: "bucket", | ||
# key: "key", | ||
# acl: "private", | ||
# ... | ||
# headers: { | ||
# foo: "bar" | ||
# } | ||
# ) | ||
# | ||
# presigned_request.uri | ||
# # => #<URI::HTTPS https://...&X-Amz-SignedHeaders=host%3Bx-amz-acl%3Bx-amz-foo... | ||
# | ||
# presigned_request.headers | ||
# # => {"x-amz-acl"=>"private", "x-amz-foo"=>'bar', ...} | ||
# | ||
class PresignedRequest | ||
|
||
# @api private | ||
ONE_WEEK = 60 * 60 * 24 * 7 | ||
|
||
# @param [Symbol] method Symbolized method name of the operation you | ||
# want to presign. | ||
# | ||
# @option params [Client] :client Optionally provide an existing | ||
# S3 client | ||
# | ||
# @option params [Integer<Seconds>] :expires_in (900) | ||
# How long the presigned URL should be valid for. Defaults | ||
# to 15 minutes (900 seconds). | ||
# | ||
# @option params [Boolean] :secure (true) When `false`, a HTTP URL | ||
# is returned instead of the default HTTPS URL. | ||
# | ||
# @option params [Boolean] :virtual_host (false) When | ||
# `true`, the bucket name will be used as the hostname. | ||
# This will cause the returned URL to be 'http' and not | ||
# 'https'. | ||
# | ||
# @option options [Hash] :headers ({}) Customized headers to | ||
# be signed and sent along with the request. | ||
# | ||
# @option options [Time] :time (Time.now) Time of the signature. | ||
# You should only set this value for testing. | ||
def initialize(method, params = {}) | ||
@bucket, @key = bucket_and_key(params) | ||
@client = params.delete(:client) || Aws::S3::Client.new | ||
@http_method = method[/[^_]+/].upcase | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We can determine this correctly for every API operation by simply using the API object. Aws::S3::Client.api.operation(method).http_method This has the benefit of it will work for more than object operations. |
||
@time = params.delete(:time) || Time.now | ||
@expires = expires_in(params) | ||
@virtual_host = !!params.delete(:virtual_host) | ||
@secure = params.delete(:secure) | ||
@headers = build_headers(params) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This implementation of It would be ideal to preserve the same interface we use when making client calls. |
||
end | ||
|
||
# Returns the presigned URL for the S3 operation | ||
# | ||
# @return [HTTPS::URI, HTTP::URI] | ||
def uri | ||
v4_signer.presign_url( | ||
http_method: @http_method, | ||
url: build_url(@client.config.endpoint), | ||
headers: @headers, | ||
body_digest: 'UNSIGNED-PAYLOAD', | ||
expires_in: @expires, | ||
time: @time | ||
) | ||
end | ||
|
||
# Returns signed headers | ||
# | ||
# @return [Hash] headers | ||
attr_reader :headers | ||
|
||
private | ||
|
||
def v4_signer | ||
Aws::Sigv4::Signer.new( | ||
service: 's3', | ||
region: @client.config.region, | ||
credentials_provider: @client.config.credentials, | ||
apply_checksum_header: @client.config.compute_checksums, | ||
uri_escape_path: false, | ||
unsigned_headers: Aws::Signers::V4::BLACKLIST_HEADERS - @headers.keys | ||
) | ||
end | ||
|
||
def bucket_and_key(params) | ||
if params[:bucket].nil? or params[:bucket] == '' | ||
raise ArgumentError, ":bucket must not be blank" | ||
end | ||
if params[:key].nil? or params[:key] == '' | ||
raise ArgumentError, ":key must not be blank" | ||
end | ||
[ params.delete(:bucket), params.delete(:key) ] | ||
end | ||
|
||
def expires_in(params) | ||
if expires = params.delete(:expires_in) | ||
if expires > ONE_WEEK | ||
msg = "expires_in value of #{expires} exceeds one-week maximum" | ||
raise ArgumentError, msg | ||
end | ||
expires | ||
end | ||
end | ||
|
||
def build_headers(params) | ||
h = params.delete(:headers) || {} | ||
headers = h.inject({}) {|h, (k, v)| h["x-amz-#{k.to_s.gsub(/_/, '-')}"] = v; h} | ||
params.inject(headers) {|h, (k, v)| h["x-amz-#{k.to_s.gsub(/_/, '-')}"] = v; h} | ||
end | ||
|
||
# Build url from endpoint | ||
def build_url(uri) | ||
uri = scheme(uri) unless @virtual_host | ||
if @virtual_host | ||
virtual_host(uri) | ||
elsif @client.config.force_path_style | ||
uri.path = "/#{@bucket}/#{@key}" | ||
uri | ||
else | ||
# bucket DNS | ||
dns(uri) | ||
end | ||
end | ||
|
||
def virtual_host(uri) | ||
uri.host = @bucket | ||
uri.path = "/#{@key}" | ||
http_scheme(uri) | ||
end | ||
|
||
def dns(uri) | ||
if Aws::Plugins::S3BucketDns.dns_compatible?(@bucket, @secure) | ||
uri.host = "#{@bucket}.#{uri.host}" | ||
uri.path = "/#{@key}" | ||
else | ||
uri.path = "/#{@bucket}/#{@key}" | ||
end | ||
uri | ||
end | ||
|
||
def scheme(uri) | ||
return uri if @secure.nil? | ||
if @secure | ||
https_scheme(uri) | ||
else | ||
http_scheme(uri) | ||
end | ||
end | ||
|
||
def http_scheme(uri) | ||
return uri if uri.scheme == 'http' | ||
uri.scheme = 'http' | ||
uri.port = 80 | ||
uri | ||
end | ||
|
||
def https_scheme(uri) | ||
return uri if uri.scheme == 'https' | ||
uri.scheme = 'https' | ||
uri.port = 443 | ||
uri | ||
end | ||
|
||
end | ||
|
||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do this in multiple places, so this pattern is not new (inject or construct a client), but we do not actually require a client. We actually need a region, and a set of credentials. The client object in inconsequential. It seems like a better solution would be to have a public interface for getting the default region, and a public interface for getting default credentials.
The client configuration can be updated to use these interfaces, so the behavior remains consistent, and then the code can be reduced down to injecting an optional region and or credentials.