From 6f3d6014367ba292c3ddc610c1eaed23e98f1f7d Mon Sep 17 00:00:00 2001 From: Matthias Schoeneich Date: Thu, 10 Aug 2023 19:01:02 +0000 Subject: [PATCH 1/2] add tests for region "local" exceptional cases --- src/aws_s3_presigned_url.erl | 49 ++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) diff --git a/src/aws_s3_presigned_url.erl b/src/aws_s3_presigned_url.erl index 39fff041..89738a54 100644 --- a/src/aws_s3_presigned_url.erl +++ b/src/aws_s3_presigned_url.erl @@ -96,4 +96,53 @@ presigned_url_test() -> ?assertEqual(<<"Token">>, proplists:get_value(<<"X-Amz-Security-Token">>, ParsedQs)), ?assertEqual(<<"host">>, proplists:get_value(<<"X-Amz-SignedHeaders">>, ParsedQs)). +presigned_url_local_with_endpoint_test() -> + Client = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, + <<"Token">>, <<"local">>), + {ok, Url} = aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, <<"bucket">>, <<"key">>), + HackneyUrl = hackney_url:parse_url(Url), + ParsedQs = hackney_url:parse_qs(HackneyUrl#hackney_url.qs), + Credential = proplists:get_value(<<"X-Amz-Credential">>, ParsedQs), + [AccessKeyId, _ShortDate, Region, Service, Request] = binary:split(Credential, <<"/">>, [global]), + ?assertEqual(https, HackneyUrl#hackney_url.scheme), + ?assertEqual(443, HackneyUrl#hackney_url.port), + ?assertEqual("amazonaws.com", HackneyUrl#hackney_url.host), + ?assertEqual(<<"/bucket/key">>, HackneyUrl#hackney_url.path), + ?assertEqual(7, length(ParsedQs)), + ?assertEqual(<<"AccessKeyID">>, AccessKeyId), + ?assertEqual(<<"local">>, Region), + ?assertEqual(<<"s3">>, Service), + ?assertEqual(<<"aws4_request">>, Request), + ?assertEqual(<<"AWS4-HMAC-SHA256">>, proplists:get_value(<<"X-Amz-Algorithm">>, ParsedQs)), + ?assertEqual(<<"3600">>, proplists:get_value(<<"X-Amz-Expires">>, ParsedQs)), + ?assertEqual(<<"Token">>, proplists:get_value(<<"X-Amz-Security-Token">>, ParsedQs)), + ?assertEqual(<<"host">>, proplists:get_value(<<"X-Amz-SignedHeaders">>, ParsedQs)). + +presigned_url_local_without_endpoint_test() -> + Client0 = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, + <<"Token">>, <<"local">>), + Client = maps:without([endpoint],Client0), + {ok, Url} = aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, <<"bucket">>, <<"key">>), + HackneyUrl = hackney_url:parse_url(Url), + ParsedQs = hackney_url:parse_qs(HackneyUrl#hackney_url.qs), + Credential = proplists:get_value(<<"X-Amz-Credential">>, ParsedQs), + [AccessKeyId, _ShortDate, Region, Service, Request] = binary:split(Credential, <<"/">>, [global]), + ?assertEqual(https, HackneyUrl#hackney_url.scheme), + ?assertEqual(443, HackneyUrl#hackney_url.port), + ?assertEqual("localhost", HackneyUrl#hackney_url.host), + ?assertEqual(<<"/bucket/key">>, HackneyUrl#hackney_url.path), + ?assertEqual(7, length(ParsedQs)), + ?assertEqual(<<"AccessKeyID">>, AccessKeyId), + ?assertEqual(<<"local">>, Region), + ?assertEqual(<<"s3">>, Service), + ?assertEqual(<<"aws4_request">>, Request), + ?assertEqual(<<"AWS4-HMAC-SHA256">>, proplists:get_value(<<"X-Amz-Algorithm">>, ParsedQs)), + ?assertEqual(<<"3600">>, proplists:get_value(<<"X-Amz-Expires">>, ParsedQs)), + ?assertEqual(<<"Token">>, proplists:get_value(<<"X-Amz-Security-Token">>, ParsedQs)), + ?assertEqual(<<"host">>, proplists:get_value(<<"X-Amz-SignedHeaders">>, ParsedQs)). + +presigned_url_local_without_without_bucket_does_not_work_test() -> + Client = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, + <<"Token">>, <<"local">>), + ?assertException (error,function_clause,aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, undefined, <<"key">>)). -endif. From da23fb5ba217b89007d739c7c0eaafb8884911b0 Mon Sep 17 00:00:00 2001 From: Matthias Schoeneich Date: Thu, 10 Aug 2023 19:44:51 +0000 Subject: [PATCH 2/2] Allow user to choose between S3 path-style and virtual_host style addressing --- src/aws_s3_presigned_url.erl | 87 ++++++++++++++++++++++++++++-------- 1 file changed, 69 insertions(+), 18 deletions(-) diff --git a/src/aws_s3_presigned_url.erl b/src/aws_s3_presigned_url.erl index 89738a54..eb99f3d3 100644 --- a/src/aws_s3_presigned_url.erl +++ b/src/aws_s3_presigned_url.erl @@ -8,7 +8,8 @@ %%% - https://docs.aws.amazon.com/AmazonS3/latest/userguide/ShareObjectPreSignedURL.html -module(aws_s3_presigned_url). --export([ make_presigned_v4_url/5 +-export([ make_presigned_v4_url/5, + make_presigned_v4_url/6 ]). -include_lib("hackney/include/hackney_lib.hrl"). @@ -18,16 +19,20 @@ %%==================================================================== -spec make_presigned_v4_url(map(), get | put, integer(), binary(), binary()) -> {ok, binary()}. make_presigned_v4_url(Client0, Method, ExpireSeconds, Bucket, Key) -> + make_presigned_v4_url(Client0, Method, ExpireSeconds, Bucket, Key, path). + +-spec make_presigned_v4_url(map(), get | put, integer(), binary(), binary(),path|virtual_host) -> {ok, binary()}. +make_presigned_v4_url(Client0, Method, ExpireSeconds, Bucket, Key, Style) -> MethodBin = aws_request:method_to_binary(Method), - Path = ["/", aws_util:encode_uri(Bucket), "/", aws_util:encode_multi_segment_uri(Key), ""], + Path = build_path(Client0,Bucket,Key,Style), Client = Client0#{service => <<"s3">>}, SecurityToken = aws_client:token(Client), AccessKeyID = aws_client:access_key_id(Client), SecretAccessKey = aws_client:secret_access_key(Client), Region = aws_client:region(Client), Service = aws_client:service(Client), - Host = build_host(<<"s3">>, Client, Bucket), - URL = build_url(Host, Path, Client, Bucket), + Host = build_host(<<"s3">>, Client, Bucket,Style), + URL = build_url(Host, Path, Client), Now = calendar:universal_time(), Options0 = [ {ttl, ExpireSeconds} , {body_digest, <<"UNSIGNED-PAYLOAD">>} @@ -43,26 +48,28 @@ make_presigned_v4_url(Client0, Method, ExpireSeconds, Bucket, Key) -> %%==================================================================== %% Internal functions %%==================================================================== -build_host(_EndpointPrefix, #{region := <<"local">>, endpoint := Endpoint}, _Bucket) -> +%% Mocks are notoriously bad with host-style requests, just skip it and use path-style for anything local +%% At some points once the mocks catch up, we should remove this ugly hacks... +build_host(_EndpointPrefix, #{region := <<"local">>, endpoint := Endpoint}, _Bucket, _Style) -> <>; -build_host(_EndpointPrefix, #{region := <<"local">>}, _Bucket) -> +build_host(_EndpointPrefix, #{region := <<"local">>}, _Bucket, _Style) -> <<"localhost">>; -build_host(EndpointPrefix, #{region := Region, endpoint := Endpoint}, Bucket) -> +build_host(EndpointPrefix, #{region := Region, endpoint := Endpoint}, _Bucket, path = _Style) -> + aws_util:binary_join([EndpointPrefix, Region, Endpoint], <<".">>); +build_host(EndpointPrefix, #{region := Region, endpoint := Endpoint}, Bucket, virtual_host = _Style) -> aws_util:binary_join([Bucket, EndpointPrefix, Region, Endpoint], <<".">>). -build_url(Host0, Path0, Client, Bucket) -> +build_path(#{region := <<"local">>} = _Client,Bucket,Key, path = _Style) -> + ["/", aws_util:encode_uri(Bucket), "/", aws_util:encode_multi_segment_uri(Key), ""]; +build_path(_Client,Bucket,Key, path = _Style) -> + ["/", aws_util:encode_uri(Bucket), "/", aws_util:encode_multi_segment_uri(Key), ""]; +build_path(_Client,_Bucket,Key,virtual_host = _Style) -> + ["/", aws_util:encode_multi_segment_uri(Key), ""]. + +build_url(Host0, Path0, Client) -> Proto = aws_client:proto(Client), - %% Mocks are notoriously bad with host-style requests, just skip it and use path-style for anything local - %% At some points once the mocks catch up, we should remove this ugly hack... - Host1 = erlang:iolist_to_binary(Host0), - IsLocalHost = aws_client:region(Client) =:= <<"local">>, Path = erlang:iolist_to_binary(Path0), - Host = case Bucket of - _ when not IsLocalHost andalso Bucket =/= undefined -> - erlang:iolist_to_binary(string:replace(Host1, <>, <<"">>, all)); - _ -> - Host1 - end, + Host = erlang:iolist_to_binary(Host0), Port = aws_client:port(Client), aws_util:binary_join([Proto, <<"://">>, Host, <<":">>, Port, Path], <<"">>). @@ -145,4 +152,48 @@ presigned_url_local_without_without_bucket_does_not_work_test() -> Client = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, <<"Token">>, <<"local">>), ?assertException (error,function_clause,aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, undefined, <<"key">>)). + +presigned_url_path_style_test() -> + Client = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, + <<"Token">>, <<"eu-west-1">>), + {ok, Url} = aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, <<"bucket">>, <<"key">>,path), + HackneyUrl = hackney_url:parse_url(Url), + ParsedQs = hackney_url:parse_qs(HackneyUrl#hackney_url.qs), + Credential = proplists:get_value(<<"X-Amz-Credential">>, ParsedQs), + [AccessKeyId, _ShortDate, Region, Service, Request] = binary:split(Credential, <<"/">>, [global]), + ?assertEqual(https, HackneyUrl#hackney_url.scheme), + ?assertEqual(443, HackneyUrl#hackney_url.port), + ?assertEqual("s3.eu-west-1.amazonaws.com", HackneyUrl#hackney_url.host), + ?assertEqual(<<"/bucket/key">>, HackneyUrl#hackney_url.path), + ?assertEqual(7, length(ParsedQs)), + ?assertEqual(<<"AccessKeyID">>, AccessKeyId), + ?assertEqual(<<"eu-west-1">>, Region), + ?assertEqual(<<"s3">>, Service), + ?assertEqual(<<"aws4_request">>, Request), + ?assertEqual(<<"AWS4-HMAC-SHA256">>, proplists:get_value(<<"X-Amz-Algorithm">>, ParsedQs)), + ?assertEqual(<<"3600">>, proplists:get_value(<<"X-Amz-Expires">>, ParsedQs)), + ?assertEqual(<<"Token">>, proplists:get_value(<<"X-Amz-Security-Token">>, ParsedQs)), + ?assertEqual(<<"host">>, proplists:get_value(<<"X-Amz-SignedHeaders">>, ParsedQs)). + +presigned_url_virtual_host_style_test() -> + Client = aws_client:make_temporary_client(<<"AccessKeyID">>, <<"SecretAccessKey">>, + <<"Token">>, <<"eu-west-1">>), + {ok, Url} = aws_s3_presigned_url:make_presigned_v4_url(Client, put, 3600, <<"bucket">>, <<"key">>,virtual_host), + HackneyUrl = hackney_url:parse_url(Url), + ParsedQs = hackney_url:parse_qs(HackneyUrl#hackney_url.qs), + Credential = proplists:get_value(<<"X-Amz-Credential">>, ParsedQs), + [AccessKeyId, _ShortDate, Region, Service, Request] = binary:split(Credential, <<"/">>, [global]), + ?assertEqual(https, HackneyUrl#hackney_url.scheme), + ?assertEqual(443, HackneyUrl#hackney_url.port), + ?assertEqual("bucket.s3.eu-west-1.amazonaws.com", HackneyUrl#hackney_url.host), + ?assertEqual(<<"/key">>, HackneyUrl#hackney_url.path), + ?assertEqual(7, length(ParsedQs)), + ?assertEqual(<<"AccessKeyID">>, AccessKeyId), + ?assertEqual(<<"eu-west-1">>, Region), + ?assertEqual(<<"s3">>, Service), + ?assertEqual(<<"aws4_request">>, Request), + ?assertEqual(<<"AWS4-HMAC-SHA256">>, proplists:get_value(<<"X-Amz-Algorithm">>, ParsedQs)), + ?assertEqual(<<"3600">>, proplists:get_value(<<"X-Amz-Expires">>, ParsedQs)), + ?assertEqual(<<"Token">>, proplists:get_value(<<"X-Amz-Security-Token">>, ParsedQs)), + ?assertEqual(<<"host">>, proplists:get_value(<<"X-Amz-SignedHeaders">>, ParsedQs)). -endif.