From 57aa27993fb3e078e5ebd77bbc9517489ea3182c Mon Sep 17 00:00:00 2001 From: Tyler Hall Date: Mon, 2 Dec 2024 16:15:58 -0500 Subject: [PATCH] test(hb_http_signature): implement tests and bug fixes #13 --- src/hb_http_signature.erl | 231 +++++++++++++++++++++++++++++++------- 1 file changed, 193 insertions(+), 38 deletions(-) diff --git a/src/hb_http_signature.erl b/src/hb_http_signature.erl index ca266438..95ef6d15 100644 --- a/src/hb_http_signature.erl +++ b/src/hb_http_signature.erl @@ -5,6 +5,8 @@ % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2.7-14 -define(EMPTY_QUERY_PARAMS, $?). +-include_lib("eunit/include/eunit.hrl"). + %%% %%% Ideal API %%% authority(ComponentIdentifiers, Params) -> Authority @@ -27,7 +29,7 @@ sign(Authority, Req, Res) -> SignatureComponentsLine = signature_components_line(ComponentIdentifiers, Req, Res), SignatureParamsLine = signature_params_line(ComponentIdentifiers, maps:get(sig_params, Authority)), SignatureBase = - <>, <<"\"@signature-params\": ">>, SignatureParamsLine/binary>>, + <>/binary, <<"\"@signature-params\": ">>/binary, SignatureParamsLine/binary>>, Name = random_an_binary(5), SignatureInput = SignatureParamsLine, % Create signature using SignatureBase and authority#key @@ -50,7 +52,7 @@ signature_components_line(ComponentIdentifiers, Req, Res) -> % TODO: handle errors? {ok, {I, V}} = identifier_to_component(Identifier, Req, Res), % https://datatracker.ietf.org/doc/html/rfc9421#section-2.5-7.2.1 - <>, V/binary, <<"\n">>>> + <>/binary, V/binary, <<"\n">>/binary>> end, <<>>, ComponentIdentifiers @@ -97,7 +99,7 @@ extract_field(Identifier, Req, Res) -> extract_field(Identifier, Req, Res, _Subject) -> % The Identifier may have params and so we need to parse it % See https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-6 - {IParsed, IParams} = sf_item(Identifier), + {item, {_Kind, IParsed}, IParams} = sf_item(Identifier), [IsStrictFormat, IsByteSequenceEncoded, DictKey] = [ find_sf_strict_format_param(IParams), find_sf_byte_sequence_param(IParams), @@ -140,13 +142,16 @@ extract_field(Identifier, Req, Res, _Subject) -> % The Field was found, but we still need to potentially parse it % (it could be a Structured Field) and potentially extract % subsequent values ie. specific dictionary key and its parameters, or further encode it - Extracted = extract_field_value( - [bin(Value) || {_Key, Value} <- FieldPairs], - [DictKey, IsStrictFormat, IsByteSequenceEncoded] - ), - {ok, {NormalizedItem, bin(Extracted)}} - end, - ok + case + extract_field_value( + [bin(Value) || {_Key, Value} <- FieldPairs], + [DictKey, IsStrictFormat, IsByteSequenceEncoded] + ) + of + {ok, Extracted} -> {ok, {NormalizedItem, bin(Extracted)}}; + E -> E + end + end end. extract_field_value(RawFields, [Key, IsStrictFormat, IsByteSequenceEncoded]) -> @@ -190,14 +195,13 @@ extract_dictionary_field_value(StructuredField = [Elem | _Rest], Key) -> false -> {sf_key_not_found_error, <<"Component Identifier references key not found in dictionary structured field">>}; {_, Value} -> - sf_serialize(Value) - end, - ok; + {ok, sf_serialize(Value)} + end; _ -> {sf_not_dictionary_error, <<"Component Identifier cannot reference key on a non-dictionary structured field">>} end. -derive_component(Identifier, Req, Res = #{}) -> +derive_component(Identifier, Req, Res) when map_size(Res) == 0 -> derive_component(Identifier, Req, Res, req); derive_component(Identifier, Req, Res) -> derive_component(Identifier, Req, Res, res). @@ -208,33 +212,33 @@ derive_component(Identifier, Req, Res, Subject) when is_atom(Identifier) -> derive_component(Identifier, Req, Res, Subject) when is_binary(Identifier) -> % The Identifier may have params and so we need to parse it % See https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-6 - {IParsed, IParams} = sf_item(Identifier), + {item, {_Kind, IParsed}, IParams} = sf_item(Identifier), case find_sf_request_param(IParams) andalso Subject =:= req of % https://datatracker.ietf.org/doc/html/rfc9421#section-2.5-7.2.2.5.2.3 true -> {req_identifier_error, - <<"A Component Identifier may not contain a req parameter if the target is a response message">>}; + <<"A Component Identifier may not contain a req parameter if the target is a request message">>}; _ -> Lowered = lower_bin(IParsed), - NormalizedItem = hb_http_structured_fields:item({item, {string, Lowered}, IParams}), - Derived = + NormalizedItem = bin(hb_http_structured_fields:item({item, {string, Lowered}, IParams})), + Result = case Lowered of % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.2.1 <<"@method">> -> - {ok, {NormalizedItem, upper_bin(maps:get(method, Req))}}; + {ok, upper_bin(maps:get(method, Req))}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.4.1 <<"@target-uri">> -> - {ok, {NormalizedItem, bin(maps:get(url, Req))}}; + {ok, bin(maps:get(url, Req))}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.6.1 <<"@authority">> -> URI = uri_string:parse(maps:get(url, Req)), Authority = maps:get(host, URI), - {ok, {NormalizedItem, lower_bin(Authority)}}; + {ok, lower_bin(Authority)}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.8.1 <<"@scheme">> -> URI = uri_string:parse(maps:get(url, Req)), - Scheme = maps:get(schema, URI), - {ok, {NormalizedItem, lower_bin(Scheme)}}; + Scheme = maps:get(scheme, URI), + {ok, lower_bin(Scheme)}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.10.1 <<"@request-target">> -> URI = uri_string:parse(maps:get(url, Req)), @@ -246,32 +250,36 @@ derive_component(Identifier, Req, Res, Subject) when is_binary(Identifier) -> % % See https://datatracker.ietf.org/doc/html/rfc9421#section-2.2.5-10 RequestTarget = - case maps:get(is_absolute_form, Req) of - true -> URI; + case maps:get(is_absolute_form, Req, false) of + true -> maps:get(url, Req); _ -> lists:join($?, [maps:get(path, URI), maps:get(query, URI, ?EMPTY_QUERY_PARAMS)]) end, - {ok, {NormalizedItem, bin(RequestTarget)}}; + {ok, bin(RequestTarget)}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.12.1 <<"@path">> -> URI = uri_string:parse(maps:get(url, Req)), Path = maps:get(path, URI), - {ok, {NormalizedItem, bin(Path)}}; + {ok, bin(Path)}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.14.1 <<"@query">> -> URI = uri_string:parse(maps:get(url, Req)), % No query params results in a "?" value % See https://datatracker.ietf.org/doc/html/rfc9421#section-2.2.7-14 - Query = maps:get(query, URI, ?EMPTY_QUERY_PARAMS), - {ok, {NormalizedItem, bin(Query)}}; + Query = + case maps:get(query, URI, <<>>) of + <<>> -> ?EMPTY_QUERY_PARAMS; + Q -> Q + end, + {ok, bin(Query)}; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.16.1 <<"@query-param">> -> - URI = uri_string:parse(maps:get(url, Req)), case find_sf_name_param(IParams) of % The name parameter MUST be provided when specifiying a @query-param % Derived Component. See https://datatracker.ietf.org/doc/html/rfc9421#section-2.2.8-1 false -> {req_identifier_error, <<"@query_param Derived Component Identifier must specify a name parameter">>}; Name -> + URI = uri_string:parse(maps:get(url, Req)), QueryParams = uri_string:dissect_query(maps:get(query, URI, "")), QueryParam = case lists:keyfind(Name, 1, QueryParams) of @@ -281,7 +289,7 @@ derive_component(Identifier, Req, Res, Subject) when is_binary(Identifier) -> % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2.8-4 _ -> "" end, - {ok, {NormalizedItem, bin(QueryParam)}} + {ok, bin(QueryParam)} end; % https://datatracker.ietf.org/doc/html/rfc9421#section-2.2-4.18.1 <<"@status">> -> @@ -291,10 +299,13 @@ derive_component(Identifier, Req, Res, Subject) when is_binary(Identifier) -> {res_identifier_error, <<"@status Derived Component must not be used if target is a request message">>}; _ -> Status = maps:get(status, Res, <<"200">>), - {ok, {NormalizedItem, Status}} + {ok, Status} end end, - Derived + case Result of + {ok, V} -> {ok, {bin(NormalizedItem), V}}; + E -> E + end end. %%% @@ -303,7 +314,11 @@ derive_component(Identifier, Req, Res, Subject) when is_binary(Identifier) -> sf_parse(Raw) when is_list(Raw) -> sf_parse(list_to_binary(Raw)); sf_parse(Raw) when is_binary(Raw) -> - Parsers = [], + Parsers = [ + fun hb_http_structured_fields:parse_list/1, + fun hb_http_structured_fields:parse_dictionary/1, + fun hb_http_structured_fields:parse_item/1 + ], sf_parse(Parsers, Raw). sf_parse([], _Raw) -> @@ -323,8 +338,8 @@ sf_serialize(StructuredField = [Elem | _Rest]) -> _ -> hb_http_structured_fields:list(StructuredField) end. -sf_item({item, {_Kind, Parsed}, Params}) -> - {Parsed, Params}; +sf_item(SfItem = {item, {_Kind, _Parsed}, _Params}) -> + SfItem; % TODO: should we check whether the string is already quoted? sf_item(ComponentIdentifier) when is_list(ComponentIdentifier) -> sf_item(<<$", (lower_bin(ComponentIdentifier))/binary, $">>); @@ -336,8 +351,11 @@ find_sf_param(Name, Params, Default) when is_list(Name) -> find_sf_param(Name, Params, Default) -> % [{<<"name">>,{string,<<"baz">>}}] case lists:keyfind(Name, 1, Params) of - {_, {_, Value}} -> Value; - _ -> Default + false -> Default; + {_, {string, Value}} -> Value; + {_, {token, Value}} -> Value; + {_, {binary, Value}} -> Value; + {_, Value} -> Value end. %%% @@ -381,3 +399,140 @@ random_an_binary(Length) -> RandomIndexes = [rand:uniform(ListLength) || _ <- lists:seq(1, Length)], RandomChars = [lists:nth(Index, Characters) || Index <- RandomIndexes], list_to_binary(RandomChars). + +%%% +%%% TESTS +%%% + +sign_test() -> + ok. + +signature_components_line_test() -> + ok. + +signature_params_line_test() -> + Params = #{created => 1733165109501, nonce => "foobar", keyid => "key1"}, + ContentIdentifiers = ["Content-Length", <<"\"@method\"">>, "@Path", "content-type;req", "example-dict;sf"], + Result = signature_params_line(ContentIdentifiers, Params), + ?assertEqual( + <<"(\"content-length\" \"@method\" \"@path\" \"content-type;req\" \"example-dict;sf\");created=1733165109501;nonce=\"foobar\";keyid=\"key1\"">>, + Result + ). + +extract_field_test() -> + ok. + +extract_field_error_conflicting_params_test() -> + ok. + +extract_field_error_field_not_found_test() -> + ok. + +extract_field_error_not_sf_test() -> + ok. + +extract_field_error_not_sf_dictionary_test() -> + ok. + +extract_field_error_sf_dictionary_key_not_found_test() -> + ok. + +derive_component_test() -> + Url = <<"https://foo.bar/id-123/Data?another=one&fizz=buzz">>, + Req = #{ + url => Url, + method => "get", + headers => #{} + }, + Res = #{ + status => 202 + }, + + % normalize method (uppercase) + method + ?assertEqual( + {ok, {<<"\"@method\"">>, <<"GET">>}}, + derive_component("\"@method\"", Req, Res) + ), + + ?assertEqual( + {ok, {<<"\"@target-uri\"">>, Url}}, + derive_component("\"@target-uri\"", Req, Res) + ), + + ?assertEqual( + {ok, {<<"\"@authority\"">>, <<"foo.bar">>}}, + derive_component("\"@authority\"", Req, Res) + ), + + ?assertEqual( + {ok, {<<"\"@scheme\"">>, <<"https">>}}, + derive_component("\"@scheme\"", Req, Res) + ), + + ?assertEqual( + {ok, {<<"\"@request-target\"">>, <<"/id-123/Data?another=one&fizz=buzz">>}}, + derive_component("\"@request-target\"", Req, Res) + ), + + % absolute form + ?assertEqual( + {ok, {<<"\"@request-target\"">>, Url}}, + derive_component("\"@request-target\"", maps:merge(Req, #{is_absolute_form => true}), Res) + ), + + ?assertEqual( + {ok, {<<"\"@path\"">>, <<"/id-123/Data">>}}, + derive_component("\"@path\"", Req, Res) + ), + + ?assertEqual( + {ok, {<<"\"@query\"">>, <<"another=one&fizz=buzz">>}}, + derive_component("\"@query\"", Req, Res) + ), + + % no query params + ?assertEqual( + {ok, {<<"\"@query\"">>, <<"?">>}}, + derive_component("\"@query\"", maps:merge(Req, #{url => <<"https://foo.bar/id-123/Data">>}), Res) + ), + + % empty query params + ?assertEqual( + {ok, {<<"\"@query\"">>, <<"?">>}}, + derive_component("\"@query\"", maps:merge(Req, #{url => <<"https://foo.bar/id-123/Data?">>}), Res) + ), + + ?assertEqual( + {ok, {<<"\"@query-param\";name=\"fizz\"">>, <<"buzz">>}}, + derive_component(<<"\"@query-param\";name=\"fizz\"">>, Req, Res) + ), + + % normalize identifier (lowercase) + @status + ?assertEqual( + {ok, {<<"\"@status\"">>, 202}}, + derive_component("\"@Status\"", Req, Res) + ), + + ok. + +derive_component_error_req_param_on_request_target_test() -> + Result = derive_component("\"@query-param\";req", #{}, #{}, req), + ?assertEqual( + {req_identifier_error, + <<"A Component Identifier may not contain a req parameter if the target is a request message">>}, + Result + ). + +derive_component_error_query_param_no_name_test() -> + Result = derive_component("\"@query-param\";noname=foo", #{}, #{}, req), + ?assertEqual( + {req_identifier_error, <<"@query_param Derived Component Identifier must specify a name parameter">>}, + Result + ). + +derive_component_error_status_req_target_test() -> + Result = derive_component("\"@status\"", #{}, #{}, req), + ?assertEqual( + {res_identifier_error, <<"@status Derived Component must not be used if target is a request message">>}, + Result + ).