Merge 9894646 into 1e5121e

lib/pigeon/adm/config.ex

@@ -55,4 +55,25 @@ defmodule Pigeon.ADM.Config do
   end
 
   defp valid_item?(item), do: is_binary(item) and String.length(item) > 0
+
+  @spec validate!(any) :: :ok
+  def validate!(config) do
+    if valid?(config) do
+      :ok
+    else
+      raise Pigeon.ConfigError,
+        reason: "attempted to start without valid client id and secret",
+        config: redact(config)
+    end
+  end
+
+  defp redact(config) do
+    [:client_id, :client_secret]
+    |> Enum.reduce(config, fn k, acc ->
+      case Map.get(acc, k) do
+        nil -> acc
+        _ -> Map.put(acc, k, "[FILTERED]")
+      end
+    end)
+  end
 end

lib/pigeon/adm/worker.ex

@@ -25,6 +25,7 @@ defmodule Pigeon.ADM.Worker do
   def init({:ok, config}), do: initialize_worker(config)
 
   def initialize_worker(config) do
+    Pigeon.ADM.Config.validate!(config)
     {:ok,
      %{
        config: config,

lib/pigeon/apns/config.ex

@@ -116,17 +116,37 @@ defmodule Pigeon.APNS.Config do
     %__MODULE__{
       name: opts[:name],
       reconnect: Keyword.get(opts, :reconnect, false),
-      cert: ConfigParser.cert(opts[:cert]),
+      cert: cert(opts[:cert]),
       certfile: ConfigParser.file_path(opts[:cert]),
-      key: ConfigParser.key(opts[:key]),
+      key: key(opts[:key]),
       keyfile: ConfigParser.file_path(opts[:key]),
       uri: Keyword.get(opts, :uri, ConfigParser.uri_for_mode(opts[:mode])),
       port: Keyword.get(opts, :port, 443),
       ping_period: Keyword.get(opts, :ping_period, 600_000)
     }
+    |> ConfigParser.strip_errors(:cert, :certfile)
+    |> ConfigParser.strip_errors(:key, :keyfile)
   end
 
   def new(name) when is_atom(name), do: ConfigParser.parse(name)
+
+  defp cert(bin) when is_binary(bin) do
+    case :public_key.pem_decode(bin) do
+      [{:Certificate, cert, _}] -> cert
+      _ -> {:error, {:invalid, bin}}
+    end
+  end
+
+  defp cert(other), do: {:error, {:invalid, other}}
+
+  defp key(bin) when is_binary(bin) do
+    case :public_key.pem_decode(bin) do
+      [{:RSAPrivateKey, key, _}] -> {:RSAPrivateKey, key}
+      _ -> {:error, {:invalid, bin}}
+    end
+  end
+
+  defp key(other), do: {:error, {:invalid, other}}
 end
 
 defimpl Pigeon.Configurable, for: Pigeon.APNS.Config do
@@ -166,12 +186,32 @@ defimpl Pigeon.Configurable, for: Pigeon.APNS.Config do
 
   defdelegate close(config), to: Shared
 
-  def connect_socket_options(%{cert: nil, certfile: nil}) do
-    {:error, :invalid_config}
+  def validate!(config) do
+    case config do
+      %{cert: {:error, _}, certfile: {:error, _}} ->
+        raise Pigeon.ConfigError,
+          reason: "attempted to start without valid certificate",
+          config: redact(config)
+
+      %{key: {:error, _}, keyfile: {:error, _}} ->
+        raise Pigeon.ConfigError,
+          reason: "attempted to start without valid key",
+          config: redact(config)
+
+      _ ->
+        :ok
+    end
   end
 
-  def connect_socket_options(%{key: nil, keyfile: nil}) do
-    {:error, :invalid_config}
+  defp redact(config) do
+    [:cert, :key]
+    |> Enum.reduce(config, fn key, acc ->
+      case Map.get(acc, key) do
+        bin when is_binary(bin) -> Map.put(acc, key, "[FILTERED]")
+        {:RSAPrivateKey, _bin} -> Map.put(acc, key, "[FILTERED]")
+        _ -> acc
+      end
+    end)
   end
 
   def connect_socket_options(config) do

lib/pigeon/apns/config_parser.ex

@@ -24,7 +24,7 @@ defmodule Pigeon.APNS.ConfigParser do
   @spec parse(atom | config_opts) :: config | {:error, :invalid_config}
   def parse(opts) when is_list(opts) do
     case config_type(Enum.into(opts, %{})) do
-      :error -> raise "invalid apns configuration #{inspect(opts)}"
+      :error -> raise Pigeon.ConfigError, reason: "configuration is invalid", config: opts
       type -> type.new(opts)
     end
   end
@@ -42,47 +42,32 @@ defmodule Pigeon.APNS.ConfigParser do
   defp config_type(_else), do: :error
 
   @doc false
-  def file_path(nil), do: nil
-
   def file_path(path) when is_binary(path) do
-    path
-    |> Path.expand()
-    |> validate_file_path!()
+    if :filelib.is_file(path) do
+      Path.expand(path)
+    else
+      {:error, {:nofile, path}}
+    end
   end
 
   def file_path({app_name, path}) when is_atom(app_name) do
     path
     |> Path.expand(:code.priv_dir(app_name))
-    |> validate_file_path!()
+    |> file_path()
   end
 
-  @doc false
-  def cert({_app_name, _path}), do: nil
-  def cert(nil), do: nil
-
-  def cert(bin) do
-    case :public_key.pem_decode(bin) do
-      [{:Certificate, cert, _}] -> cert
-      _ -> nil
-    end
-  end
+  def file_path(other), do: {:error, {:nofile, other}}
 
   @doc false
-  def key({_app_name, _path}), do: nil
-  def key(nil), do: nil
-
-  def key(bin) do
-    case :public_key.pem_decode(bin) do
-      [{:RSAPrivateKey, key, _}] -> {:RSAPrivateKey, key}
-      _ -> nil
+  def strip_errors(config, key1, key2) do
+    case {Map.get(config, key1), Map.get(config, key2)} do
+      {{:error, _}, {:error, _}} -> config
+      {{:error, _}, _} -> Map.put(config, key1, nil)
+      {_, {:error, _}} -> Map.put(config, key2, nil)
     end
   end
 
   def uri_for_mode(:dev), do: @apns_development_api_uri
   def uri_for_mode(:prod), do: @apns_production_api_uri
   def uri_for_mode(_else), do: nil
-
-  defp validate_file_path!(path) do
-    if :filelib.is_file(path), do: path, else: raise("file not found: #{path}")
-  end
 end

lib/pigeon/apns/jwt_config.ex

@@ -101,7 +101,6 @@ defmodule Pigeon.APNS.JWTConfig do
       ...> )
       %Pigeon.APNS.JWTConfig{uri: "api.push.apple.com", name: :test,
       team_id: "DEF1234567", key_identifier: "ABC1234567", 
-      key: "test/support/AuthKey.p8-mock",
       keyfile: Path.expand("test/support/AuthKey.p8-mock"),
       ping_period: 300000, port: 2197, reconnect: false}
 
@@ -118,14 +117,24 @@ defmodule Pigeon.APNS.JWTConfig do
       uri: Keyword.get(opts, :uri, ConfigParser.uri_for_mode(opts[:mode])),
       port: Keyword.get(opts, :port, 443),
       ping_period: Keyword.get(opts, :ping_period, 600_000),
-      key: opts[:key],
+      key: key(opts[:key]),
       keyfile: ConfigParser.file_path(opts[:key]),
       key_identifier: Keyword.get(opts, :key_identifier),
       team_id: Keyword.get(opts, :team_id)
     }
+    |> ConfigParser.strip_errors(:key, :keyfile)
   end
 
   def new(name) when is_atom(name), do: ConfigParser.parse(name)
+
+  defp key(bin) when is_binary(bin) do
+    case :public_key.pem_decode(bin) do
+      [] -> {:error, {:invalid, bin}}
+      [{_, _, _}] -> bin
+    end
+  end
+
+  defp key(other), do: {:error, {:invalid, other}}
 end
 
 defimpl Pigeon.Configurable, for: Pigeon.APNS.JWTConfig do
@@ -176,8 +185,37 @@ defimpl Pigeon.Configurable, for: Pigeon.APNS.JWTConfig do
 
   defdelegate close(config), to: Shared
 
-  def connect_socket_options(%{key: nil}) do
-    {:error, :invalid_config}
+  def validate!(config) do
+    case config do
+      %{team_id: nil} ->
+        raise Pigeon.ConfigError,
+          reason: "attempted to start without valid team_id",
+          config: redact(config)
+
+      %{key_identifier: nil} ->
+        raise Pigeon.ConfigError,
+          reason: "attempted to start without valid key_identifier",
+          config: redact(config)
+
+      %{key: {:error, _}, keyfile: {:error, _}} ->
+        raise Pigeon.ConfigError,
+          reason: "attempted to start without valid key",
+          config: redact(config)
+
+      _ ->
+        :ok
+    end
+  end
+
+  defp redact(config) do
+    [:key, :keyfile]
+    |> Enum.reduce(config, fn key, acc ->
+      case Map.get(acc, key) do
+        bin when is_binary(bin) -> Map.put(acc, key, "[FILTERED]")
+        {:RSAPrivateKey, _bin} -> Map.put(acc, key, "[FILTERED]")
+        _ -> acc
+      end
+    end)
   end
 
   def connect_socket_options(%{key: _jwt_key} = config) do
@@ -194,8 +232,6 @@ defimpl Pigeon.Configurable, for: Pigeon.APNS.JWTConfig do
   end
 
   @spec put_bearer_token(Config.headers(), JWTConfig.t()) :: Config.headers()
-  defp put_bearer_token(headers, %{key: nil}), do: headers
-
   defp put_bearer_token(headers, config) do
     token_storage_key = config.key_identifier <> ":" <> config.team_id
     {timestamp, saved_token} = Pigeon.APNS.Token.get(token_storage_key)

lib/pigeon/config_error.ex

@@ -0,0 +1,14 @@
+defmodule Pigeon.ConfigError do
+  defexception reason: nil, config: nil
+
+  @impl true
+  def message(%{config: config, reason: reason}) do
+    """
+    #{reason}
+
+    The following configuration was given: 
+
+    #{inspect(config, pretty: true)}
+    """
+  end
+end

lib/pigeon/configurable.ex

@@ -52,4 +52,7 @@ defprotocol Pigeon.Configurable do
   def schedule_ping(config)
 
   def close(config)
+
+  @spec validate!(any) :: :ok
+  def validate!(config)
 end

lib/pigeon/fcm/config.ex

@@ -150,6 +150,22 @@ defimpl Pigeon.Configurable, for: Pigeon.FCM.Config do
   def close(_config) do
   end
 
+  def validate!(%{key: key}) when is_binary(key) do
+    :ok
+  end
+
+  def validate!(config) do
+    raise Pigeon.ConfigError,
+      reason: "attempted to start without valid key",
+      config: redact(config)
+  end
+
+  defp redact(%{key: key} = config) when is_binary(key) do
+    Map.put(config, :key, "[FILTERED]")
+  end
+
+  defp redact(config), do: config
+
   # no on_response callback, ignore
   def parse_result(_, _, nil, _notif), do: :ok
 

lib/pigeon/worker.ex

@@ -41,6 +41,7 @@ defmodule Pigeon.Worker do
   end
 
   def init({:ok, config}) do
+    Configurable.validate!(config)
     state = %Worker{config: config}
     {:producer, state}
   end

test/adm/config_test.exs

@@ -1,4 +1,25 @@
 defmodule Pigeon.ADM.ConfigTest do
   use ExUnit.Case
   doctest Pigeon.ADM.Config, import: true
+
+  @invalid_key_msg ~r/^attempted to start without valid client id and secret/
+
+  test "success if configured with client id and secret" do
+      config = Pigeon.ADM.Config.new(name: :test, client_id: "amzn1.iba-client.abc123", client_secret: "abc123")
+      {:ok, _} = Pigeon.ADM.Worker.init({:ok, config})
+  end
+
+  test "raises if configured with invalid client id" do
+    assert_raise(Pigeon.ConfigError, @invalid_key_msg, fn ->
+      config = Pigeon.ADM.Config.new(name: :test, client_id: nil, client_secret: "abc123")
+      Pigeon.ADM.Worker.init({:ok, config})
+    end)
+  end
+
+  test "raises if configured with invalid client secret" do
+    assert_raise(Pigeon.ConfigError, @invalid_key_msg, fn ->
+      config = Pigeon.ADM.Config.new(name: :test, client_id: "amzn1.iba-client.abc123", client_secret: nil)
+      Pigeon.ADM.Worker.init({:ok, config})
+    end)
+  end
 end