added tests for Web.MediaProxy

2 files changed, 27 insertions, 22 deletions

diff --git a/lib/pleroma/web/media_proxy/media_proxy.ex b/lib/pleroma/web/media_proxy/media_proxy.ex
index dd8888a02..a661e9bb7 100644
--- a/lib/pleroma/web/media_proxy/media_proxy.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy.ex
@@ -3,68 +3,71 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.MediaProxy do
-  @base64_opts [padding: false]
-
-  def url(nil), do: nil
+  alias Pleroma.Config
+  alias Pleroma.Web
 
-  def url(""), do: nil
+  @base64_opts [padding: false]
 
+  def url(url) when is_nil(url) or url == "", do: nil
   def url("/" <> _ = url), do: url
 
   def url(url) do
-    if !enabled?() or local?(url) or whitelisted?(url) do
+    if disabled?() or local?(url) or whitelisted?(url) do
       url
     else
       encode_url(url)
     end
   end
 
-  defp enabled?, do: Pleroma.Config.get([:media_proxy, :enabled], false)
+  defp disabled?, do: !Config.get([:media_proxy, :enabled], false)
 
   defp local?(url), do: String.starts_with?(url, Pleroma.Web.base_url())
 
   defp whitelisted?(url) do
     %{host: domain} = URI.parse(url)
 
-    Enum.any?(Pleroma.Config.get([:media_proxy, :whitelist]), fn pattern ->
+    Enum.any?(Config.get([:media_proxy, :whitelist]), fn pattern ->
       String.equivalent?(domain, pattern)
     end)
   end
 
   def encode_url(url) do
-    secret = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base])
     base64 = Base.url_encode64(url, @base64_opts)
-    sig = :crypto.hmac(:sha, secret, base64)
-    sig64 = sig |> Base.url_encode64(@base64_opts)
+
+    sig64 =
+      base64
+      |> signed_url
+      |> Base.url_encode64(@base64_opts)
 
     build_url(sig64, base64, filename(url))
   end
 
   def decode_url(sig, url) do
-    secret = Pleroma.Config.get([Pleroma.Web.Endpoint, :secret_key_base])
-    sig = Base.url_decode64!(sig, @base64_opts)
-    local_sig = :crypto.hmac(:sha, secret, url)
-
-    if local_sig == sig do
+    with {:ok, sig} <- Base.url_decode64(sig, @base64_opts),
+         signature when signature == sig <- signed_url(url) do
       {:ok, Base.url_decode64!(url, @base64_opts)}
     else
-      {:error, :invalid_signature}
+      _ -> {:error, :invalid_signature}
     end
   end
 
+  defp signed_url(url) do
+    :crypto.hmac(:sha, Config.get([Web.Endpoint, :secret_key_base]), url)
+  end
+
   def filename(url_or_path) do
     if path = URI.parse(url_or_path).path, do: Path.basename(path)
   end
 
   def build_url(sig_base64, url_base64, filename \\ nil) do
     [
-      Pleroma.Config.get([:media_proxy, :base_url], Pleroma.Web.base_url()),
+      Pleroma.Config.get([:media_proxy, :base_url], Web.base_url()),
       "proxy",
       sig_base64,
       url_base64,
       filename
     ]
-    |> Enum.filter(fn value -> value end)
+    |> Enum.filter(& &1)
     |> Path.join()
   end
 end
diff --git a/lib/pleroma/web/media_proxy/controller.ex b/lib/pleroma/web/media_proxy/media_proxy_controller.ex
index ea33d7685..1e9520d46 100644
--- a/lib/pleroma/web/media_proxy/controller.ex
+++ b/lib/pleroma/web/media_proxy/media_proxy_controller.ex
@@ -13,7 +13,7 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
     with config <- Pleroma.Config.get([:media_proxy], []),
          true <- Keyword.get(config, :enabled, false),
          {:ok, url} <- MediaProxy.decode_url(sig64, url64),
-         :ok <- filename_matches(Map.has_key?(params, "filename"), conn.request_path, url) do
+         :ok <- filename_matches(params, conn.request_path, url) do
       ReverseProxy.call(conn, url, Keyword.get(config, :proxy_opts, @default_proxy_opts))
     else
       false ->
@@ -27,13 +27,15 @@ defmodule Pleroma.Web.MediaProxy.MediaProxyController do
     end
   end
 
-  def filename_matches(has_filename, path, url) do
-    filename = url |> MediaProxy.filename()
+  def filename_matches(%{"filename" => _} = _, path, url) do
+    filename = MediaProxy.filename(url)
 
-    if has_filename && filename && Path.basename(path) != filename do
+    if filename && Path.basename(path) != filename do
       {:wrong_filename, filename}
     else
       :ok
     end
   end
+
+  def filename_matches(_, _, _), do: :ok
 end