More resilient parsing of JWKs (#28)

* More resilient parsing of JWKs

JWEs are becoming more popular and there are a few JWKs with keys
meant for encryption. We must ignore keys with:

- use == enc
- alg in JWE algs

* Update test/default_strategy_template_test.exs

Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>

* Fix review comments

* Fix comment place

* Use config for capturing all logs in tests

* More fixes from review

Co-authored-by: Paulo Valente <16843419+polvalente@users.noreply.github.com>

config/config.exs

@@ -1,3 +1,7 @@
 use Mix.Config
 
 config :tesla, JokenJwks.HttpFetcher, adapter: Tesla.Adapter.Hackney
+
+if Mix.env() == :test do
+  config :ex_unit, capture_log: true
+end

lib/joken_jwks/default_strategy_template.ex

@@ -178,12 +178,15 @@ defmodule JokenJwks.DefaultStrategyTemplate do
 
         telemetry_prefix = Keyword.get(opts, :telemetry_prefix, __MODULE__)
 
+        [_, _, {:jws, {:alg, algs}}] = JOSE.JWA.supports()
+
         opts =
           opts
           |> Keyword.put(:time_interval, time_interval)
           |> Keyword.put(:log_level, log_level)
           |> Keyword.put(:jwks_url, url)
           |> Keyword.put(:telemetry_prefix, telemetry_prefix)
+          |> Keyword.put(:jws_supported_algs, algs)
 
         do_init(start?, first_fetch_sync, opts)
       end
@@ -258,6 +261,11 @@ defmodule JokenJwks.DefaultStrategyTemplate do
         with {:ok, keys} <- HttpFetcher.fetch_signers(url, opts),
              {:ok, signers} <- validate_and_parse_keys(keys, opts) do
           JokenJwks.log(:debug, log_level, "Fetched signers. #{inspect(signers)}")
+
+          if signers == %{} do
+            JokenJwks.log(:warn, log_level, "NO VALID SIGNERS FOUND!")
+          end
+
           EtsCache.put_signers(signers)
           EtsCache.set_status(:ok)
         else
@@ -280,18 +288,26 @@ defmodule JokenJwks.DefaultStrategyTemplate do
         Enum.reduce_while(keys, {:ok, %{}}, fn key, {:ok, acc} ->
           case parse_signer(key, opts) do
             {:ok, signer} -> {:cont, {:ok, Map.put(acc, key["kid"], signer)}}
+            # We don't support "enc" keys but should not break otherwise
+            {:error, :not_signing_key} -> {:cont, {:ok, acc}}
+            # We skip unknown JWS algorithms or JWEs
+            {:error, :not_signing_alg} -> {:cont, {:ok, acc}}
             e -> {:halt, e}
           end
         end)
       end
 
       defp parse_signer(key, opts) do
-        with {:kid, kid} when is_binary(kid) <- {:kid, key["kid"]},
+        with {:use, true} <- {:use, key["use"] != "enc"},
+             {:kid, kid} when is_binary(kid) <- {:kid, key["kid"]},
              {:ok, alg} <- get_algorithm(key["alg"], opts[:explicit_alg]),
+             {:jws_alg?, true} <- {:jws_alg?, alg in opts[:jws_supported_algs]},
              {:ok, _signer} = res <- {:ok, Signer.create(alg, key)} do
           res
         else
+          {:use, false} -> {:error, :not_signing_key}
           {:kid, _} -> {:error, :kid_not_binary}
+          {:jws_alg?, false} -> {:error, :not_signing_alg}
           err -> err
         end
       rescue

test/default_strategy_template_test.exs

@@ -1,44 +1,30 @@
+:ok = Application.ensure_started(:telemetry)
+
 defmodule JokenJwks.DefaultStrategyTest do
   use ExUnit.Case, async: false
 
   import ExUnit.CaptureLog
   import Mox
   import Tesla.Mock, only: [json: 1, json: 2]
-  alias Joken.Signer
   alias JokenJwks.TestUtils
 
   setup :set_mox_global
   setup :verify_on_exit!
 
-  defmodule TestToken do
-    use Joken.Config
-
-    defmodule Strategy do
-      use JokenJwks.DefaultStrategyTemplate
-    end
-
-    add_hook(JokenJwks, strategy: Strategy)
-
-    def token_config, do: %{}
-  end
-
-  @tag :capture_log
   test "can fetch keys" do
     setup_jwks()
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id2"))
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "fails if kid does not match" do
     setup_jwks()
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id3"))
     assert {:error, :kid_does_not_match} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "fails if it can't fetch" do
     parent = self()
     ref = make_ref()
@@ -48,22 +34,20 @@ defmodule JokenJwks.DefaultStrategyTest do
       {:ok, %Tesla.Env{status: 500}}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks/500")
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks/500"})
 
     assert_receive {^ref, :continue}
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id1"))
     assert {:error, :no_signers_fetched} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "fails if no option was provided" do
     assert_raise(RuntimeError, ~r/No url set for fetching JWKS!/, fn ->
-      TestToken.Strategy.start_link([])
+      start_supervised!({TestToken.Strategy, []})
     end)
   end
 
-  @tag :capture_log
   test "can configure window of time for searching for new signers" do
     setup_jwks(500)
 
@@ -85,7 +69,6 @@ defmodule JokenJwks.DefaultStrategyTest do
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "fetches only one per window of time invariably" do
     setup_jwks(2_000)
 
@@ -110,13 +93,12 @@ defmodule JokenJwks.DefaultStrategyTest do
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "fails if no signers are fetched" do
     expect_call(fn %{url: "http://jwks"} ->
       {:ok, json(%{"keys" => [TestUtils.build_key("id1")]}, status: 500)}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks")
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks"})
     :timer.sleep(100)
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id3"))
@@ -126,7 +108,7 @@ defmodule JokenJwks.DefaultStrategyTest do
   test "can skip start polling and fetching" do
     # expect 0 invocations
     expect_call(0, fn _, _opts -> :ok end)
-    TestToken.Strategy.start_link(jwks_url: "http://jwks", should_start: false)
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", should_start: false})
     assert TestToken.Strategy.EtsCache.check_state() == 0
   end
 
@@ -137,7 +119,7 @@ defmodule JokenJwks.DefaultStrategyTest do
 
     log =
       capture_log(fn ->
-        TestToken.Strategy.start_link(jwks_url: "http://jwks", log_level: :none)
+        start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", log_level: :none})
         :timer.sleep(100)
       end)
 
@@ -151,7 +133,7 @@ defmodule JokenJwks.DefaultStrategyTest do
 
     log =
       capture_log(fn ->
-        TestToken.Strategy.start_link(jwks_url: "http://jwks", log_level: :error)
+        start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", log_level: :error})
         :timer.sleep(100)
       end)
 
@@ -164,20 +146,21 @@ defmodule JokenJwks.DefaultStrategyTest do
 
     log =
       capture_log(fn ->
-        TestToken.Strategy.start_link(jwks_url: "http://jwks/500", log_level: :error)
+        start_supervised!({TestToken.Strategy, jwks_url: "http://jwks/500", log_level: :error})
         :timer.sleep(100)
       end)
 
     assert log =~ "Failed to fetch signers."
   end
 
-  @tag :capture_log
   test "set telemetry_prefix to default prefix" do
     self = self()
 
+    on_exit(fn -> :telemetry.detach("telemetry-test-default") end)
+
     :telemetry.attach(
-      "telemetry_test_default",
-      [JokenJwks.DefaultStrategyTest.TestToken.Strategy, :joken_jwks, :request],
+      "telemetry-test-default",
+      [TestToken.Strategy, :joken_jwks, :request],
       fn name, measurements, metadata, _ ->
         send(self, {:telemetry_event, name, measurements, metadata})
       end,
@@ -186,14 +169,12 @@ defmodule JokenJwks.DefaultStrategyTest do
 
     expect_call(fn %{url: "http://jwks/500"} -> {:ok, json(%{}, status: 500)} end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks/500")
+    start_supervised!({TestToken.Strategy, [jwks_url: "http://jwks/500"]})
 
-    assert_receive {:telemetry_event,
-                    [JokenJwks.DefaultStrategyTest.TestToken.Strategy, :joken_jwks, :request],
+    assert_receive {:telemetry_event, [TestToken.Strategy, :joken_jwks, :request],
                     %{request_time: _}, %{result: {:ok, %Tesla.Env{}}}}
   end
 
-  @tag :capture_log
   test "can set telemetry_prefix to a custom prefix" do
     self = self()
 
@@ -208,100 +189,75 @@ defmodule JokenJwks.DefaultStrategyTest do
 
     expect_call(fn %{url: "http://jwks/500"} -> {:ok, json(%{}, status: 500)} end)
 
-    TestToken.Strategy.start_link(
-      jwks_url: "http://jwks/500",
-      telemetry_prefix: :my_custom_prefix
+    start_supervised!(
+      {TestToken.Strategy, jwks_url: "http://jwks/500", telemetry_prefix: :my_custom_prefix}
     )
 
     assert_receive {:telemetry_event, [:my_custom_prefix, :joken_jwks, :request],
                     %{request_time: _}, %{result: {:ok, %Tesla.Env{}}}}
   end
 
-  @tag :capture_log
   test "can set options on callback init_opts/1" do
-    defmodule InitOptsToken do
-      use Joken.Config
-
-      defmodule Strategy do
-        use JokenJwks.DefaultStrategyTemplate
-
-        @doc false
-        def init_opts(other_opts) do
-          assert other_opts == [log_level: :none]
-
-          # override and add option
-          [log_level: :debug, jwks_url: "http://jwks"]
-        end
-      end
-
-      add_hook(JokenJwks, strategy: Strategy)
-
-      def token_config, do: %{}
-    end
-
     expect_call(fn %{url: "http://jwks"} ->
       {:ok, json(%{"keys" => [TestUtils.build_key("id1"), TestUtils.build_key("id2")]})}
     end)
 
-    InitOptsToken.Strategy.start_link(log_level: :none)
+    start_supervised!({InitOptsToken.Strategy, log_level: :none})
     :timer.sleep(100)
 
     assert InitOptsToken.Strategy.EtsCache.get_signers()[:signers] |> Map.keys() == ["id1", "id2"]
   end
 
-  @tag :capture_log
   test "can override alg" do
     expect_call(fn %{url: "http://jwks"} ->
       assert key = "id1" |> TestUtils.build_key() |> Map.put("alg", "RS256")
       assert key["alg"] == "RS256"
       {:ok, json(%{"keys" => [key]})}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks", explicit_alg: "RS384")
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", explicit_alg: "RS384"})
     :timer.sleep(100)
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id1", "RS384"))
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "can parse key without alg with option explicit_alg" do
     expect_call(fn %{url: "http://jwks"} ->
       assert key = "id1" |> TestUtils.build_key() |> Map.delete("alg")
       refute key["alg"]
       {:ok, json(%{"keys" => [key]})}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks", explicit_alg: "RS384")
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", explicit_alg: "RS384"})
     :timer.sleep(100)
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id1", "RS384"))
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "can start first fetch synchronously" do
     expect_call(fn %{url: "http://jwks"} ->
       {:ok, json(%{"keys" => [TestUtils.build_key("id1")]})}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks", first_fetch_sync: true)
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", first_fetch_sync: true})
 
     # no sleep here
 
     token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id1"))
     assert {:ok, %{}} == TestToken.verify_and_validate(token)
   end
 
-  @tag :capture_log
   test "even if first fetch sync fails will try to poll" do
     expect_call(2, fn %{url: "http://jwks"} -> {:error, :internal_error} end)
 
-    TestToken.Strategy.start_link(
-      jwks_url: "http://jwks",
-      first_fetch_sync: true,
-      time_interval: 100,
-      http_max_retries_per_fetch: 1
+    start_supervised!(
+      {TestToken.Strategy,
+       jwks_url: "http://jwks",
+       first_fetch_sync: true,
+       time_interval: 100,
+       http_max_retries_per_fetch: 1}
     )
 
     # We expect 3 calls in the timespan of 150 milliseconds:
@@ -310,12 +266,52 @@ defmodule JokenJwks.DefaultStrategyTest do
     :timer.sleep(120)
   end
 
+  test "ignores keys with `use` as `enc`" do
+    expect_call(fn %{url: "http://jwks"} ->
+      {:ok,
+       json(%{
+         "keys" => [
+           TestUtils.build_key("id1"),
+           Map.merge(TestUtils.build_key("id2"), %{"use" => "enc", "alg" => "RSA-OAEP-256"})
+         ]
+       })}
+    end)
+
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", first_fetch_sync: true})
+
+    # use is ignored
+    assert length(TestToken.Strategy.EtsCache.get_signers()) == 1
+
+    token = TestToken.generate_and_sign!(%{}, TestUtils.create_signer_with_kid("id1"))
+    assert {:ok, %{}} == TestToken.verify_and_validate(token)
+  end
+
+  test "ignores keys algorithms that are not JWS" do
+    expect_call(fn %{url: "http://jwks"} ->
+      {:ok,
+       json(%{
+         "keys" => [
+           Map.merge(TestUtils.build_key("id1"), %{"use" => "sig", "alg" => "RSA-OAEP-256"})
+         ]
+       })}
+    end)
+
+    assert capture_log(fn ->
+             start_supervised!(
+               {TestToken.Strategy, jwks_url: "http://jwks", first_fetch_sync: true}
+             )
+           end) =~ "NO VALID SIGNERS FOUND!"
+
+    # use is ignored
+    assert Enum.count(TestToken.Strategy.EtsCache.get_signers()[:signers]) == 0
+  end
+
   def setup_jwks(time_interval \\ 1_000) do
     expect_call(fn %{url: "http://jwks"} ->
       {:ok, json(%{"keys" => [TestUtils.build_key("id1"), TestUtils.build_key("id2")]})}
     end)
 
-    TestToken.Strategy.start_link(jwks_url: "http://jwks", time_interval: time_interval)
+    start_supervised!({TestToken.Strategy, jwks_url: "http://jwks", time_interval: time_interval})
     :timer.sleep(100)
   end