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

Add validation for header parameters #160

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 80 additions & 13 deletions lib/phoenix_swagger/plug/validate_plug.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,27 +19,31 @@ defmodule PhoenixSwagger.Plug.Validate do
result =
with {:ok, path} <- find_matching_path(conn),
:ok <- validate_body_params(path, conn),
:ok <- validate_header_params(path, conn),
:ok <- validate_query_params(path, conn),
do: {:ok, conn}

case result do
{:ok, conn} ->
conn

{:error, :no_matching_path} ->
send_error_response(conn, 404, "API does not provide resource", conn.request_path)

{:error, message, path} ->
send_error_response(conn, validation_failed_status, message, path)
end
end

defp find_matching_path(conn) do
found = Enum.find(:ets.tab2list(@table), fn({path, base_path, _}) ->
base_path_segments = String.split(base_path || "", "/") |> tl
path_segments = String.split(path, "/") |> tl
path_info_without_base = remove_base_path(conn.path_info, base_path_segments)
req_path_segments = [String.downcase(conn.method) | path_info_without_base]
equal_paths?(path_segments, req_path_segments)
end)
found =
Enum.find(:ets.tab2list(@table), fn {path, base_path, _} ->
base_path_segments = String.split(base_path || "", "/") |> tl
path_segments = String.split(path, "/") |> tl
path_info_without_base = remove_base_path(conn.path_info, base_path_segments)
req_path_segments = [String.downcase(conn.method) | path_info_without_base]
equal_paths?(path_segments, req_path_segments)
end)

case found do
nil -> {:error, :no_matching_path}
Expand All @@ -64,46 +68,58 @@ defmodule PhoenixSwagger.Plug.Validate do
defp validate_boolean(_name, value, parameters) when value in ["true", "false"] do
validate_query_params(parameters)
end

defp validate_boolean(name, _value, _parameters) do
{:error, "Type mismatch. Expected Boolean but got String.", "#/#{name}"}
end

defp validate_integer(name, value, parameters) do
_ = String.to_integer(value)
validate_query_params(parameters)
rescue ArgumentError ->
rescue
ArgumentError ->
{:error, "Type mismatch. Expected Integer but got String.", "#/#{name}"}
end

defp validate_query_params([]), do: :ok

defp validate_query_params([{_type, _name, nil, false} | parameters]) do
validate_query_params(parameters)
end

defp validate_query_params([{_type, name, nil, true} | _]) do
{:error, "Required property #{name} was not present.", "#"}
end

defp validate_query_params([{"string", _name, _val, _} | parameters]) do
validate_query_params(parameters)
end

defp validate_query_params([{"integer", name, val, _} | parameters]) do
validate_integer(name, val, parameters)
end

defp validate_query_params([{"boolean", name, val, _} | parameters]) do
validate_boolean(name, val, parameters)
end

defp validate_query_params(path, conn) do
[{_path, _basePath, schema}] = :ets.lookup(@table, path)

parameters =
for parameter <- schema.schema["parameters"],
parameter["type"] != nil,
parameter["in"] in ["query", "path"] do
{parameter["type"], parameter["name"], get_param_value(conn.params, parameter["name"]), parameter["required"]}
{parameter["type"], parameter["name"], get_param_value(conn.params, parameter["name"]),
parameter["required"]}
end

validate_query_params(parameters)
end

defp get_in_nested(params = nil, _), do: params
defp get_in_nested(params, nil), do: params

defp get_in_nested(params, nested_map) when map_size(nested_map) == 1 do
[{key, child_nested_map}] = Map.to_list(nested_map)

Expand All @@ -119,19 +135,70 @@ defmodule PhoenixSwagger.Plug.Validate do
case Validator.validate(path, conn.body_params) do
:ok -> :ok
{:error, [{error, error_path} | _], _path} -> {:error, error, error_path}
{:error, error, error_path} -> {:error, error, error_path}
{:error, error, error_path} -> {:error, error, error_path}
end
end

defp equal_paths?([], []), do: true
defp equal_paths?([head | orig_path_rest], [head | req_path_rest]), do: equal_paths?(orig_path_rest, req_path_rest)
defp equal_paths?(["{" <> _ | orig_path_rest], [_ | req_path_rest]), do: equal_paths?(orig_path_rest, req_path_rest)

defp equal_paths?([head | orig_path_rest], [head | req_path_rest]),
do: equal_paths?(orig_path_rest, req_path_rest)

defp equal_paths?(["{" <> _ | orig_path_rest], [_ | req_path_rest]),
do: equal_paths?(orig_path_rest, req_path_rest)

defp equal_paths?(_, _), do: false

# It is pretty safe to strip request path by base path. They can't be
# non-equal. In this way, the router even will not execute this plug.
defp remove_base_path(path, []), do: path

defp remove_base_path([_path | rest], [_base_path | base_path_rest]) do
remove_base_path(rest, base_path_rest)
end
end

defp validate_header_params([]), do: :ok
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

validate_header_params/1 looks very similar to validate_query_params/1.
I think they can be combined into something like validate_parameter_list.

validate_header_params/2 is fine, since it needs to pull the params from a different key in the Conn anyway.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll try to fix this as soon as I can


defp validate_header_params([{_type, _name, nil, false} | parameters]) do
validate_header_params(parameters)
end

defp validate_header_params([{_type, name, nil, true} | _]) do
{:error, "Required header #{name} was not present.", "#"}
end

defp validate_header_params([{"string", _name, _val, _} | parameters]) do
validate_query_params(parameters)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a copy & paste bug here :)

end

defp validate_header_params([{"integer", name, val, _} | parameters]) do
validate_integer(name, val, parameters)
end

defp validate_header_params([{"boolean", name, val, _} | parameters]) do
validate_boolean(name, val, parameters)
end

defp validate_header_params(path, conn) do
[{_path, _basePath, schema}] = :ets.lookup(@table, path)

parameters =
for parameter <- schema.schema["parameters"],
parameter["type"] != nil,
parameter["in"] in ["header"] do
{parameter["type"], parameter["name"],
get_header_value(conn.req_headers, parameter["name"]), parameter["required"]}
end

validate_header_params(parameters)
end

defp get_header_value(headers, header_name) do
header_name_down_case = String.downcase(header_name)

headers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a handy function in the erlang std lib to search a list of tuples:

:lists.keyfind(header_name_down_case, 1, headers) || nil

|> Enum.find(fn {k, _v} ->
header_name_down_case == k
end)
end
end
16 changes: 16 additions & 0 deletions test/plug/validate_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ defmodule PhoenixSwagger.Plug.ValidateTest do
test "required param returns error when not present" do
conn = :get
|> conn("/shapes?filter[route]=Red")
|> put_req_header("request-id", "d92578b3-d281-48a8-9e91-32b276fe6458")
|> parse()
assert %Conn{halted: true, resp_body: body, status: 400} = Validate.call(conn, @opts)
assert Poison.decode!(body) == %{
Expand All @@ -35,6 +36,7 @@ defmodule PhoenixSwagger.Plug.ValidateTest do
test "required nested param returns error when not present" do
conn = :get
|> conn("/shapes?api_key=SECRET")
|> put_req_header("request-id", "d92578b3-d281-48a8-9e91-32b276fe6458")
|> parse()
assert %Conn{halted: true, resp_body: body, status: 400} = Validate.call(conn, @opts)
assert Poison.decode!(body) == %{
Expand All @@ -45,9 +47,23 @@ defmodule PhoenixSwagger.Plug.ValidateTest do
}
end

test "required header returns error when not present" do
conn = :get
|> conn("/shapes?filter[route]=Red")
|> parse()
assert %Conn{halted: true, resp_body: body, status: 400} = Validate.call(conn, @opts)
assert Poison.decode!(body) == %{
"error" => %{
"message" => "Required header request-id was not present.",
"path" => "#"
}
}
end

test "does not halt when required params present" do
conn = :get
|> conn("/shapes?api_key=SECRET&filter[route]=Red")
|> put_req_header("request-id", "d92578b3-d281-48a8-9e91-32b276fe6458")
|> parse()
assert %Conn{halted: false} = Validate.call(conn, @opts)
end
Expand Down
12 changes: 12 additions & 0 deletions test/test_spec/swagger_jsonapi_test_spec.json
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,12 @@
"in": "query",
"description": "Filter by `/data/{index}/relationships/route/data/id`. Multiple `/data/{index}/relationships/route/data/id` **MUST** be a comma-separated (U+002C COMMA, \",\") list."
},
{
"type": "string",
"required": false,
"name": "optional-header",
"in": "header"
},
{
"type": "string",
"required": false,
Expand All @@ -80,6 +86,12 @@
"1"
],
"description": "Filter by direction of travel along the route.\n\nThe meaning of `direction_id` varies based on the route. You can programmatically get the direction names from `/routes` `/data/{index}/attributes/direction_names` or `/routes/{id}` `/data/attriutes/direction_names`. The general pattern is as follows:\n\n| Route ID Pattern | `direction_id` | Direction Name |\n|------------------|----------------|----------------|\n| `Red` | `0` | `\"Southbound\"` |\n| `Red` | `1` | `\"Northbound\"` |\n| `Orange` | `0` | `\"Southbound\"` |\n| `Orange` | `1` | `\"Northbound\"` |\n| `Blue` | `0` | `\"Westbound\"` |\n| `Blue` | `1` | `\"Eastbound\"` |\n| `Green-*` | `0` | `\"Westbound\"` |\n| `Green-*` | `1` | `\"Eastbound\"` |\n| `*` | `0` | `\"Outbound\"` |\n| `*` | `1` | `\"Inbound\"` |\n\n\n\n\n"
},
{
"type": "string",
"required": true,
"name": "request-id",
"in": "header"
}
],
"operationId": "Api.ShapeController.index",
Expand Down