feat(electric): Try decoding the JWT key using base64 only when the c…

…onfig option is set (#1168)

This reverts [the
change](0deba4d#diff-20b7a1cb738afa77ff1e885426e33a8c92faa3cc84aa9f62fd2f67b217e6f020R102-R113)
introduced in the 0.9.3 release where the configured signing key would
get automatically decoded if it looked like a valid base64-encoded
string. That change has caused unintended problems to arise for users of
Supabase Auth since its secret keys are base64-encoded but are meant to
be used as is.

This PR introduces a separate configuration option
`AUTH_JWT_KEY_IS_BASE64_ENCODED`. Corresponding doc update -
#1169.

components/electric/config/runtime.exs

@@ -106,6 +106,8 @@ auth_mode = env!("AUTH_MODE", :string, default_auth_mode)
 auth_opts = [
   alg: {"AUTH_JWT_ALG", env!("AUTH_JWT_ALG", :string, nil)},
   key: {"AUTH_JWT_KEY", env!("AUTH_JWT_KEY", :string, nil)},
+  key_is_base64_encoded:
+    {"AUTH_JWT_KEY_IS_BASE64_ENCODED", env!("AUTH_JWT_KEY_IS_BASE64_ENCODED", :boolean, nil)},
   namespace: {"AUTH_JWT_NAMESPACE", env!("AUTH_JWT_NAMESPACE", :string, nil)},
   iss: {"AUTH_JWT_ISS", env!("AUTH_JWT_ISS", :string, nil)},
   aud: {"AUTH_JWT_AUD", env!("AUTH_JWT_AUD", :string, nil)}

components/electric/lib/electric/config.ex

@@ -52,10 +52,13 @@ defmodule Electric.Config do
     end
   end
 
+  # Remove unset options and those set to an empty string.
   defp filter_auth_opts(auth_opts) do
-    for {key, {_, val}} <- auth_opts, is_binary(val) and String.trim(val) != "" do
-      {key, val}
-    end
+    auth_opts
+    |> Enum.map(fn {key, {_, val}} -> {key, val} end)
+    |> Enum.reject(fn {_key, val} ->
+      is_nil(val) or (is_binary(val) and String.trim(val) == "")
+    end)
   end
 
   @spec parse_database_url(maybe_string, :dev | :test | :prod) ::

components/electric/lib/electric/satellite/auth/secure.ex

@@ -41,6 +41,8 @@ defmodule Electric.Satellite.Auth.Secure do
 
          "-----BEGIN PUBLIC KEY-----\nMFkwEwYHKoZIzj0CAQY..."
 
+    * `key_is_base64_encoded: <boolean>` - optional flag that indicates whether the key should be base64-decoded.
+
     * `namespace: <string>` - optional namespace under which the "sub" or "user_id" claim will be looked up. If omitted,
       "sub" or "user_id" must be a top-level claim.
 
@@ -93,26 +95,19 @@ defmodule Electric.Satellite.Auth.Secure do
 
   defp validate_key(alg, opts) when is_binary(alg) and is_list(opts) do
     if key = opts[:key] do
-      validate_key(key, alg)
+      validate_key(key, alg, opts)
     else
       {:error, :key, "not set"}
     end
   end
 
-  defp validate_key(key, "HS" <> _ = alg) do
-    # Try decoding the user-provided key as base64. If that fails, assume the key is a raw
-    # binary and use it verbatim.
-    # The `padding: false` is required to accept keys that are base64-encoded without padding.
-    raw_key =
-      case Base.decode64(key, padding: false) do
-        {:ok, raw_key} -> raw_key
-        :error -> key
-      end
-
-    validate_key_length(raw_key, alg)
+  defp validate_key(maybe_encoded_key, "HS" <> _ = alg, opts) do
+    with {:ok, key} <- decode_key(maybe_encoded_key, opts) do
+      validate_key_length(key, alg)
+    end
   end
 
-  defp validate_key(key, pk_alg) do
+  defp validate_key(key, pk_alg, _opts) do
     algorithms_for_key =
       key
       |> JOSE.JWK.from_pem()
@@ -147,6 +142,18 @@ defmodule Electric.Satellite.Auth.Secure do
 
   defp validate_key_length(raw_key, "HS" <> _), do: {:ok, raw_key}
 
+  defp decode_key(maybe_encoded_key, opts) do
+    if opts[:key_is_base64_encoded] do
+      # The `padding: false` is required to accept keys that are base64-encoded without padding.
+      case Base.decode64(maybe_encoded_key, padding: false) do
+        {:ok, key} -> {:ok, key}
+        :error -> {:error, :key, "has invalid base64 encoding"}
+      end
+    else
+      {:ok, maybe_encoded_key}
+    end
+  end
+
   defp add_exp_claim(token_config, in: seconds) do
     Joken.Config.add_claim(
       token_config,

components/electric/test/electric/config_test.exs

@@ -53,6 +53,13 @@ defmodule Electric.ConfigTest do
 
       assert {{Electric.Satellite.Auth.Insecure, %{joken_config: %{}, namespace: "ns"}}, []} =
                validate_auth_config("insecure", namespace: {"AUTH_JWT_NAMESPACE", "ns"})
+
+      # Settings without values are ignored
+      assert {{Electric.Satellite.Auth.Insecure, %{joken_config: %{}, namespace: "ns"}}, []} =
+               validate_auth_config("insecure",
+                 namespace: {"AUTH_JWT_NAMESPACE", "ns"},
+                 key: {"AUTH_JWT_KEY", nil}
+               )
     end
 
     test "validates secure mode" do
@@ -62,6 +69,13 @@ defmodule Electric.ConfigTest do
                  key: {"AUTH_JWT_KEY", String.duplicate(".", 32)}
                )
 
+      assert {{Electric.Satellite.Auth.Secure, %{joken_config: %{}, namespace: nil}}, []} =
+               validate_auth_config("secure",
+                 alg: {"AUTH_JWT_ALG", "HS256"},
+                 key: {"AUTH_JWT_KEY", :crypto.strong_rand_bytes(32) |> Base.encode64()},
+                 key_is_base64_encoded: {"AUTH_JWT_KEY_IS_BASE64_ENCODED", true}
+               )
+
       assert {{Electric.Satellite.Auth.Secure,
                %{
                  joken_config: %{},
@@ -72,12 +86,22 @@ defmodule Electric.ConfigTest do
                validate_auth_config("secure",
                  alg: {"AUTH_JWT_ALG", "HS256"},
                  key: {"AUTH_JWT_KEY", String.duplicate(".", 32)},
+                 key_is_base64_encoded: {"AUTH_JWT_KEY_IS_BASE64_ENCODED", false},
                  namespace: {"AUTH_JWT_NAMESPACE", "ns"},
                  iss: {"AUTH_JWT_ISS", "foo"},
                  aud: {"AUTH_JWT_AUD", "bar"}
                )
     end
 
+    test "complains about invalid key encoding" do
+      assert {nil, [{"AUTH_JWT_KEY", {:error, "has invalid base64 encoding"}}]} ==
+               validate_auth_config("secure",
+                 alg: {"AUTH_JWT_ALG", "HS256"},
+                 key: {"AUTH_JWT_KEY", String.duplicate(".", 32)},
+                 key_is_base64_encoded: {"AUTH_JWT_KEY_IS_BASE64_ENCODED", true}
+               )
+    end
+
     test "complains about invalid auth mode" do
       assert {nil,
               [

components/electric/test/electric/satellite/auth/secure_test.exs

@@ -43,13 +43,37 @@ defmodule Electric.Satellite.Auth.SecureTest do
 
       Enum.each(options, fn opts ->
         raw_key = String.duplicate(<<1, 2, 3, 4, 5, 6, 7, 8>>, 4)
-        opts = [alg: "HS256", key: Base.encode64(raw_key, opts)]
+        opts = [alg: "HS256", key: Base.encode64(raw_key, opts), key_is_base64_encoded: true]
 
         assert {:ok, config} = build_config(opts)
-        assert is_map(config)
+        assert {:jose_jwk_kty_oct, raw_key} == config.joken_signer.jwk.kty
       end)
     end
 
+    test "does not try to automatically decode a base64-encoded symmetric key" do
+      options = [
+        [],
+        [padding: true],
+        [padding: false]
+      ]
+
+      Enum.each(options, fn opts ->
+        raw_key = String.duplicate(<<1, 2, 3, 4, 5, 6, 7, 8>>, 4)
+        encoded_key = Base.encode64(raw_key, opts)
+        opts = [alg: "HS256", key: encoded_key]
+
+        assert {:ok, config} = build_config(opts)
+        assert {:jose_jwk_kty_oct, encoded_key} == config.joken_signer.jwk.kty
+      end)
+    end
+
+    test "validates proper base64 encoding for a symmetric key" do
+      key = String.duplicate(<<1, 2, 3, 4, 5, 6, 7, 8>>, 4)
+
+      assert {:error, :key, "has invalid base64 encoding"} ==
+               build_config(alg: "HS256", key: key, key_is_base64_encoded: true)
+    end
+
     test "checks for missing 'alg'" do
       assert {:error, :alg, "not set"} == build_config([])
     end
@@ -79,7 +103,7 @@ defmodule Electric.Satellite.Auth.SecureTest do
       assert byte_size(key) >= 32
 
       assert {:error, :key, "has to be at least 32 bytes long for HS256"} ==
-               build_config(alg: "HS256", key: key)
+               build_config(alg: "HS256", key: key, key_is_base64_encoded: true)
 
       ###
 
@@ -88,7 +112,7 @@ defmodule Electric.Satellite.Auth.SecureTest do
       assert byte_size(key) >= 48
 
       assert {:error, :key, "has to be at least 48 bytes long for HS384"} ==
-               build_config(alg: "HS384", key: key)
+               build_config(alg: "HS384", key: key, key_is_base64_encoded: true)
 
       ###
 
@@ -97,7 +121,7 @@ defmodule Electric.Satellite.Auth.SecureTest do
       assert byte_size(key) >= 64
 
       assert {:error, :key, "has to be at least 64 bytes long for HS512"} ==
-               build_config(alg: "HS512", key: key)
+               build_config(alg: "HS512", key: key, key_is_base64_encoded: true)
     end
 
     test "validates the public key format" do