Options handling (#2)

* Adds NimbleOptions * !!! Adds options check to ExBankID.auth -- cert_file is now mandatory * !!! Adds options check to ExBankID.cancel -- cert_file is now mandatory * !!! Adds options check to ExBankID.collect -- cert_file is now mandatory * !!! Adds options check to ExBankID.sign -- cert_file is now mandatory * Removes hard coded values from ExBankID.HttpRequest * Adds supported options docs * Updates typspec for functions in ExBankID
anfly0 · Aug 10, 2020 · 7f56ff5 · 7f56ff5
1 parent d9c8729
commit 7f56ff5
Show file tree

Hide file tree

Showing 13 changed files with 135 additions and 39 deletions.
diff --git a/lib/ex_bank_id.ex b/lib/ex_bank_id.ex
@@ -4,27 +4,34 @@ defmodule ExBankID do
   """
 
   @spec auth(String.t(), url: String.t(), cert_file: String.t()) ::
-          {:error, %ExBankID.Error.Api{} | binary()} | {:ok, %ExBankID.Auth.Response{}}
+          {:error, %ExBankID.Error.Api{} | binary()}
+          | {:error, NimbleOptions.ValidationError.t()}
+          | {:ok, %ExBankID.Auth.Response{}}
   defdelegate auth(ip_address, opts \\ []), to: ExBankID.Auth
 
   @spec sign(String.t(), String.t(),
           url: String.t(),
           cert_file: String.t(),
           personal_number: String.t(),
           user_non_visible_data: String.t()
-        ) :: {:error, %ExBankID.Error.Api{} | binary()} | {:ok, %ExBankID.Sign.Response{}}
+        ) ::
+          {:error, %ExBankID.Error.Api{} | binary()}
+          | {:error, NimbleOptions.ValidationError.t()}
+          | {:ok, %ExBankID.Sign.Response{}}
   defdelegate sign(ip_address, user_visible_data, opts \\ []), to: ExBankID.Sign
 
   @spec collect(String.t() | %ExBankID.Sign.Response{} | %ExBankID.Auth.Response{},
           url: String.t(),
           cert_file: String.t()
         ) ::
-          {:error, %ExBankID.Error.Api{} | binary()} | {:ok, %ExBankID.Collect.Response{}}
+          {:error, %ExBankID.Error.Api{} | binary()}
+          | {:error, NimbleOptions.ValidationError.t()}
+          | {:ok, %ExBankID.Collect.Response{}}
   defdelegate collect(order_ref, opts \\ []), to: ExBankID.Collect
 
   @spec cancel(String.t() | %ExBankID.Sign.Response{} | %ExBankID.Auth.Response{},
           url: String.t(),
           cert_file: String.t()
-        ) :: {:error, %ExBankID.Error.Api{} | binary()} | {:ok, %{}}
+        ) :: {:error, %ExBankID.Error.Api{} | binary()} | {:error, NimbleOptions.ValidationError.t()} | {:ok, %{}}
   defdelegate cancel(order_ref, opts \\ []), to: ExBankID.Cancel
 end
diff --git a/lib/ex_bank_id/auth.ex b/lib/ex_bank_id/auth.ex
@@ -1,8 +1,23 @@
 defmodule ExBankID.Auth do
-  def auth(ip_address, opts)
-      when is_binary(ip_address) and is_list(opts) do
-    with payload = %ExBankID.Auth.Payload{} <-
-           ExBankID.Auth.Payload.new(ip_address, opts) do
+  @options [
+    url: [
+      type: :string,
+      default: "https://appapi2.test.bankid.com/rp/v5.1/"
+    ],
+    cert_file: [
+      type: :string,
+      required: true
+    ],
+    personal_number: [
+      type: :string
+      # TODO: Add validator
+    ]
+  ]
+  @doc "Supported options:\n#{NimbleOptions.docs(@options)}"
+
+  def auth(ip_address, opts) when is_binary(ip_address) and is_list(opts) do
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %ExBankID.Auth.Payload{} <- ExBankID.Auth.Payload.new(ip_address, opts) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end

diff --git a/lib/ex_bank_id/cancel.ex b/lib/ex_bank_id/cancel.ex
@@ -1,23 +1,39 @@
 defmodule ExBankID.Cancel do
   alias ExBankID.Cancel.Payload
+
+  @options [
+    url: [
+      type: :string,
+      default: "https://appapi2.test.bankid.com/rp/v5.1/"
+    ],
+    cert_file: [
+      type: :string,
+      required: true
+    ]
+  ]
+  @doc "Supported options:\n#{NimbleOptions.docs(@options)}"
+
   def cancel(token, opts \\ [])
 
   def cancel(token, opts) when is_binary(token) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end
 
   def cancel(token = %ExBankID.Auth.Response{}, opts) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end
 
   def cancel(token = %ExBankID.Sign.Response{}, opts) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end

diff --git a/lib/ex_bank_id/collect.ex b/lib/ex_bank_id/collect.ex
@@ -1,23 +1,39 @@
 defmodule ExBankID.Collect do
   alias ExBankID.Collect.Payload
+
+  @options [
+    url: [
+      type: :string,
+      default: "https://appapi2.test.bankid.com/rp/v5.1/"
+    ],
+    cert_file: [
+      type: :string,
+      required: true
+    ]
+  ]
+  @doc "Supported options:\n#{NimbleOptions.docs(@options)}"
+
   def collect(token, opts \\ [])
 
   def collect(token, opts) when is_binary(token) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end
 
   def collect(token = %ExBankID.Auth.Response{}, opts) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end
 
   def collect(token = %ExBankID.Sign.Response{}, opts) do
-    with payload = %Payload{} <-
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Payload{} <-
            Payload.new(token) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end

diff --git a/lib/ex_bank_id/http_request.ex b/lib/ex_bank_id/http_request.ex
@@ -1,6 +1,4 @@
 defmodule ExBankID.HttpRequest do
-  @base_url "https://appapi2.test.bankid.com/rp/v5.1/"
-  @cert_file __DIR__ <> "/../../assets/test.pem"
   @headers [{"Content-Type", "application/json"}]
 
   @type payload() ::
@@ -47,12 +45,12 @@ defmodule ExBankID.HttpRequest do
     |> handle_response(action)
   end
 
-  defp url(:auth, opts), do: Keyword.get(opts, :url, @base_url) <> "/auth"
-  defp url(:sign, opts), do: Keyword.get(opts, :url, @base_url) <> "/sign"
-  defp url(:collect, opts), do: Keyword.get(opts, :url, @base_url) <> "/collect"
-  defp url(:cancel, opts), do: Keyword.get(opts, :url, @base_url) <> "/cancel"
+  defp url(:auth, opts), do: Keyword.get(opts, :url) <> "/auth"
+  defp url(:sign, opts), do: Keyword.get(opts, :url) <> "/sign"
+  defp url(:collect, opts), do: Keyword.get(opts, :url) <> "/collect"
+  defp url(:cancel, opts), do: Keyword.get(opts, :url) <> "/cancel"
 
-  defp ssl_options(opts), do: [certfile: Keyword.get(opts, :cert_file, @cert_file)]
+  defp ssl_options(opts), do: [certfile: Keyword.get(opts, :cert_file)]
 
   defp handle_response({:ok, %HTTPoison.Response{status_code: 200, body: body}}, :collect) do
     Poison.decode(body, as: %ExBankID.Collect.Response{})

diff --git a/lib/ex_bank_id/sign.ex b/lib/ex_bank_id/sign.ex
@@ -1,9 +1,29 @@
 defmodule ExBankID.Sign do
   alias ExBankID.Sign
 
+  @options [
+    url: [
+      type: :string,
+      default: "https://appapi2.test.bankid.com/rp/v5.1/"
+    ],
+    cert_file: [
+      type: :string,
+      required: true
+    ],
+    personal_number: [
+      type: :string
+      # TODO: Add validator
+    ],
+    user_non_visible_data: [
+      type: :string
+    ]
+  ]
+  @doc "Supported options:\n#{NimbleOptions.docs(@options)}"
+
   def sign(ip_address, user_visible_data, opts \\ [])
       when is_binary(ip_address) and is_binary(user_visible_data) and is_list(opts) do
-    with payload = %Sign.Payload{} <- Sign.Payload.new(ip_address, user_visible_data, opts) do
+    with {:ok, opts} <- NimbleOptions.validate(opts, @options),
+         payload = %Sign.Payload{} <- Sign.Payload.new(ip_address, user_visible_data, opts) do
       ExBankID.HttpRequest.send_request(payload, opts)
     end
   end

diff --git a/mix.exs b/mix.exs
@@ -33,7 +33,8 @@ defmodule ExBankID.MixProject do
       {:dialyxir, "~> 1.0", only: [:dev], runtime: false},
       {:bypass, "~> 1.0", only: :test},
       {:excoveralls, "~> 0.10", only: :test},
-      {:ex_doc, ">= 0.0.0", only: :dev, runtime: false}
+      {:ex_doc, ">= 0.0.0", only: :dev, runtime: false},
+      {:nimble_options, "~> 0.3.0"}
     ]
   end
 

diff --git a/mix.lock b/mix.lock
@@ -20,6 +20,7 @@
   "metrics": {:hex, :metrics, "1.0.1", "25f094dea2cda98213cecc3aeff09e940299d950904393b2a29d191c346a8486", [:rebar3], [], "hexpm", "69b09adddc4f74a40716ae54d140f93beb0fb8978d8636eaded0c31b6f099f16"},
   "mime": {:hex, :mime, "1.3.1", "30ce04ab3175b6ad0bdce0035cba77bba68b813d523d1aac73d9781b4d193cf8", [:mix], [], "hexpm", "6cbe761d6a0ca5a31a0931bf4c63204bceb64538e664a8ecf784a9a6f3b875f1"},
   "mimerl": {:hex, :mimerl, "1.2.0", "67e2d3f571088d5cfd3e550c383094b47159f3eee8ffa08e64106cdf5e981be3", [:rebar3], [], "hexpm", "f278585650aa581986264638ebf698f8bb19df297f66ad91b18910dfc6e19323"},
+  "nimble_options": {:hex, :nimble_options, "0.3.0", "1872911bf50a048f04da26e02704e6aeafc362c2daa7636b6dbfda9492ccfcfa", [:mix], [], "hexpm", "180790a8644fea402452bc15bb54b9bf2c8e5c1fdeb6b39d8072e59c324edf7f"},
   "nimble_parsec": {:hex, :nimble_parsec, "0.6.0", "32111b3bf39137144abd7ba1cce0914533b2d16ef35e8abc5ec8be6122944263", [:mix], [], "hexpm", "27eac315a94909d4dc68bc07a4a83e06c8379237c5ea528a9acff4ca1c873c52"},
   "parse_trans": {:hex, :parse_trans, "3.3.0", "09765507a3c7590a784615cfd421d101aec25098d50b89d7aa1d66646bc571c1", [:rebar3], [], "hexpm", "17ef63abde837ad30680ea7f857dd9e7ced9476cdd7b0394432af4bfc241b960"},
   "plug": {:hex, :plug, "1.10.3", "c9cebe917637d8db0e759039cc106adca069874e1a9034fd6e3fdd427fd3c283", [:mix], [{:mime, "~> 1.0", [hex: :mime, repo: "hexpm", optional: false]}, {:plug_crypto, "~> 1.1.1 or ~> 1.2", [hex: :plug_crypto, repo: "hexpm", optional: false]}, {:telemetry, "~> 0.4", [hex: :telemetry, repo: "hexpm", optional: false]}], "hexpm", "01f9037a2a1de1d633b5a881101e6a444bcabb1d386ca1e00bb273a1f1d9d939"},

diff --git a/test/auth/client_test.exs b/test/auth/client_test.exs
@@ -32,7 +32,8 @@ defmodule Test.Auth.Client do
       Test.Helpers.endpoint_handler(200, response_payload, expected_request_payload)
     )
 
-    assert ^expected_response = ExBankID.auth("1.1.1.1", url: Test.Helpers.get_url(bypass.port()))
+    assert ^expected_response =
+             ExBankID.auth("1.1.1.1", url: Test.Helpers.get_url(bypass.port()), cert_file: Test.Helpers.cert_file())
   end
 
   test "client handles successful auth request with personal number", %{bypass: bypass} do
@@ -64,13 +65,13 @@ defmodule Test.Auth.Client do
     assert ^expected_response =
              ExBankID.auth("1.1.1.1",
                url: Test.Helpers.get_url(bypass.port()),
-               personal_number: "190000000000"
+               personal_number: "190000000000",
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
   test "client handles unsuccessful auth request", %{bypass: bypass} do
-    expected_response =
-      {:error, %ExBankID.Error.Api{errorCode: "invalidParameters", details: "No such order"}}
+    expected_response = {:error, %ExBankID.Error.Api{errorCode: "invalidParameters", details: "No such order"}}
 
     expected_request_payload = %{"endUserIp" => "1.1.1.1"}
 
@@ -86,6 +87,7 @@ defmodule Test.Auth.Client do
       Test.Helpers.endpoint_handler(400, response_payload, expected_request_payload)
     )
 
-    assert ^expected_response = ExBankID.auth("1.1.1.1", url: Test.Helpers.get_url(bypass.port()))
+    assert ^expected_response =
+             ExBankID.auth("1.1.1.1", url: Test.Helpers.get_url(bypass.port()), cert_file: Test.Helpers.cert_file())
   end
 end
diff --git a/test/cancel/client_test.exs b/test/cancel/client_test.exs
@@ -19,7 +19,10 @@ defmodule Test.Cancel.Client do
     )
 
     assert ^expected_response =
-             ExBankID.cancel("131daac9-16c6-4618-beb0-365768f37288", url: Test.Helpers.get_url(bypass.port()))
+             ExBankID.cancel("131daac9-16c6-4618-beb0-365768f37288",
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
+             )
   end
 
   test "Client handles successful cancel request give a auth response struct", %{bypass: bypass} do
@@ -36,7 +39,8 @@ defmodule Test.Cancel.Client do
 
     assert ^expected_response =
              ExBankID.cancel(%ExBankID.Auth.Response{orderRef: "131daac9-16c6-4618-beb0-365768f37288"},
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -54,7 +58,8 @@ defmodule Test.Cancel.Client do
 
     assert ^expected_response =
              ExBankID.cancel(%ExBankID.Sign.Response{orderRef: "131daac9-16c6-4618-beb0-365768f37288"},
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -74,6 +79,9 @@ defmodule Test.Cancel.Client do
     )
 
     assert ^expected_response =
-             ExBankID.cancel("131daac9-16c6-4618-beb0-365768f37288", url: Test.Helpers.get_url(bypass.port()))
+             ExBankID.cancel("131daac9-16c6-4618-beb0-365768f37288",
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
+             )
   end
 end
diff --git a/test/collect/client_test.exs b/test/collect/client_test.exs
@@ -32,7 +32,8 @@ defmodule Test.Auth.Collect do
 
     assert ^expected_response =
              ExBankID.collect("131daac9-16c6-4618-beb0-365768f37288",
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -64,7 +65,8 @@ defmodule Test.Auth.Collect do
                  qrStartToken: "67df3917-fa0d-44e5-b327-edcc928297f8",
                  qrStartSecret: "d28db9a7-4cde-429e-a983-359be676944c"
                },
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -91,7 +93,8 @@ defmodule Test.Auth.Collect do
                  qrStartToken: "67df3917-fa0d-44e5-b327-edcc928297f8",
                  qrStartSecret: "d28db9a7-4cde-429e-a983-359be676944c"
                },
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -133,7 +136,8 @@ defmodule Test.Auth.Collect do
                  qrStartToken: "67df3917-fa0d-44e5-b327-edcc928297f8",
                  qrStartSecret: "d28db9a7-4cde-429e-a983-359be676944c"
                },
-               url: Test.Helpers.get_url(bypass.port())
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
              )
   end
 end
diff --git a/test/helpers/helpers_test.exs b/test/helpers/helpers_test.exs
@@ -13,4 +13,6 @@ defmodule Test.Helpers do
       )
     end
   end
+
+  def cert_file(), do: __DIR__ <> "../../assets/test.pem"
 end
diff --git a/test/sign/client_test.exs b/test/sign/client_test.exs
@@ -25,7 +25,11 @@ defmodule Test.Auth.Sign do
               autoStartToken: "7c40b5c9-fa74-49cf-b98c-bfe651f9a7c6",
               qrStartToken: "67df3917-fa0d-44e5-b327-edcc928297f8",
               qrStartSecret: "d28db9a7-4cde-429e-a983-359be676944c"
-            }} = ExBankID.sign("1.1.1.1", "Visible data", url: Test.Helpers.get_url(bypass.port()))
+            }} =
+             ExBankID.sign("1.1.1.1", "Visible data",
+               url: Test.Helpers.get_url(bypass.port()),
+               cert_file: Test.Helpers.cert_file()
+             )
   end
 
   test "client handles successful sign request with personal number", %{bypass: bypass} do
@@ -46,7 +50,8 @@ defmodule Test.Auth.Sign do
             }} =
              ExBankID.sign("1.1.1.1", "Visible data",
                url: Test.Helpers.get_url(bypass.port()),
-               personal_number: "190000000000"
+               personal_number: "190000000000",
+               cert_file: Test.Helpers.cert_file()
              )
   end
 
@@ -71,7 +76,8 @@ defmodule Test.Auth.Sign do
              ExBankID.sign("1.1.1.1", "Visible data",
                url: Test.Helpers.get_url(bypass.port()),
                personal_number: "190000000000",
-               user_non_visible_data: "Not visible data"
+               user_non_visible_data: "Not visible data",
+               cert_file: Test.Helpers.cert_file()
              )
   end
 end