From 988f0b45139e38b071c22b8e0ddb2715f9d709fe Mon Sep 17 00:00:00 2001 From: Kieran Eglin Date: Mon, 30 Dec 2024 17:05:52 -0800 Subject: [PATCH] Docs, tests --- .../controllers/sources/source_html.ex | 2 +- lib/pinchflat_web/plugs.ex | 18 +- lib/pinchflat_web/router.ex | 1 + .../controllers/podcast_controller_test.exs | 21 ++- test/pinchflat_web/plugs_test.exs | 166 ++++++++++++++++++ test/pinchflat_web/routing_test.exs | 108 ------------ 6 files changed, 204 insertions(+), 112 deletions(-) create mode 100644 test/pinchflat_web/plugs_test.exs delete mode 100644 test/pinchflat_web/routing_test.exs diff --git a/lib/pinchflat_web/controllers/sources/source_html.ex b/lib/pinchflat_web/controllers/sources/source_html.ex index a552da2..1e113c4 100644 --- a/lib/pinchflat_web/controllers/sources/source_html.ex +++ b/lib/pinchflat_web/controllers/sources/source_html.ex @@ -40,7 +40,7 @@ defmodule PinchflatWeb.Sources.SourceHTML do end def rss_feed_url(conn, source) do - # TODO: finally address this. I shouldn't have to use concatination here + # NOTE: The reason for this concatenation is to avoid what appears to be a bug in Phoenix url(conn, ~p"/sources/#{source.uuid}/feed") <> ".xml" end diff --git a/lib/pinchflat_web/plugs.ex b/lib/pinchflat_web/plugs.ex index 67b5b71..f905d6f 100644 --- a/lib/pinchflat_web/plugs.ex +++ b/lib/pinchflat_web/plugs.ex @@ -1,8 +1,14 @@ defmodule PinchflatWeb.Plugs do - # TODO: doc and test + @moduledoc """ + Custom plugs for PinchflatWeb. + """ + use PinchflatWeb, :router alias Pinchflat.Settings + @doc """ + If the `expose_feed_endpoints` setting is true, this plug does nothing. Otherwise, it calls `basic_auth/2`. + """ def maybe_basic_auth(conn, opts) do if Application.get_env(:pinchflat, :expose_feed_endpoints) do conn @@ -11,6 +17,9 @@ defmodule PinchflatWeb.Plugs do end end + @doc """ + If the `basic_auth_username` and `basic_auth_password` settings are set, this plug calls `Plug.BasicAuth.basic_auth/3`. + """ def basic_auth(conn, _opts) do username = Application.get_env(:pinchflat, :basic_auth_username) password = Application.get_env(:pinchflat, :basic_auth_password) @@ -22,10 +31,17 @@ defmodule PinchflatWeb.Plugs do end end + @doc """ + Removes the `x-frame-options` header from the response to allow the page to be embedded in an iframe. + """ def allow_iframe_embed(conn, _opts) do delete_resp_header(conn, "x-frame-options") end + @doc """ + If the `route_token` query parameter matches the `route_token` setting, this plug does nothing. + Otherwise, it sends a 401 response. + """ def token_protected_route(%{query_params: %{"route_token" => route_token}} = conn, _opts) do if Settings.get!(:route_token) == route_token do conn diff --git a/lib/pinchflat_web/router.ex b/lib/pinchflat_web/router.ex index bdbb17e..cef9220 100644 --- a/lib/pinchflat_web/router.ex +++ b/lib/pinchflat_web/router.ex @@ -25,6 +25,7 @@ defmodule PinchflatWeb.Router do # has to match before /sources/:id get "/sources/opml", Podcasts.PodcastController, :opml_feed + get "/sources/:foo/opml", Podcasts.PodcastController, :opml_feed end # Routes in here _may not be_ protected by basic auth. This is necessary for diff --git a/test/pinchflat_web/controllers/podcast_controller_test.exs b/test/pinchflat_web/controllers/podcast_controller_test.exs index ada823d..91d31ed 100644 --- a/test/pinchflat_web/controllers/podcast_controller_test.exs +++ b/test/pinchflat_web/controllers/podcast_controller_test.exs @@ -4,17 +4,34 @@ defmodule PinchflatWeb.PodcastControllerTest do import Pinchflat.MediaFixtures import Pinchflat.SourcesFixtures + alias Pinchflat.Settings + describe "opml_feed" do test "renders the XML document", %{conn: conn} do source = source_fixture() + route_token = Settings.get!(:route_token) - conn = get(conn, ~p"/sources/opml" <> ".xml") + conn = get(conn, ~p"/sources/opml.xml?#{[route_token: route_token]}") assert conn.status == 200 assert {"content-type", "application/opml+xml; charset=utf-8"} in conn.resp_headers assert {"content-disposition", "inline"} in conn.resp_headers assert conn.resp_body =~ ~s"http://www.example.com/sources/#{source.uuid}/feed.xml" - assert conn.resp_body =~ "text=\"Cool and good internal name!\"" + assert conn.resp_body =~ "text=\"#{source.custom_name}\"" + end + + test "returns 401 if the route token is incorrect", %{conn: conn} do + conn = get(conn, ~p"/sources/opml.xml?route_token=incorrect") + + assert conn.status == 401 + assert conn.resp_body == "Unauthorized" + end + + test "returns 401 if the route token is missing", %{conn: conn} do + conn = get(conn, ~p"/sources/opml.xml") + + assert conn.status == 401 + assert conn.resp_body == "Unauthorized" end end diff --git a/test/pinchflat_web/plugs_test.exs b/test/pinchflat_web/plugs_test.exs new file mode 100644 index 0000000..9a02eba --- /dev/null +++ b/test/pinchflat_web/plugs_test.exs @@ -0,0 +1,166 @@ +defmodule PinchflatWeb.PlugsTest do + use PinchflatWeb.ConnCase + + alias PinchflatWeb.Plugs + alias Pinchflat.Settings + + describe "maybe_basic_auth/2" do + setup do + old_username = Application.get_env(:pinchflat, :basic_auth_username) + old_password = Application.get_env(:pinchflat, :basic_auth_password) + old_expose_feed_endpoints = Application.get_env(:pinchflat, :expose_feed_endpoints) + + on_exit(fn -> + Application.put_env(:pinchflat, :basic_auth_username, old_username) + Application.put_env(:pinchflat, :basic_auth_password, old_password) + Application.put_env(:pinchflat, :expose_feed_endpoints, old_expose_feed_endpoints) + end) + + :ok + end + + test "uses basic auth when expose_feed_endpoints is false" do + Application.put_env(:pinchflat, :basic_auth_username, "user") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + Application.put_env(:pinchflat, :expose_feed_endpoints, false) + + conn = Plugs.maybe_basic_auth(build_conn(), []) + + assert conn.status == 401 + assert {"www-authenticate", "Basic realm=\"Pinchflat\""} in conn.resp_headers + end + + test "supplying the correct username and password allows access" do + Application.put_env(:pinchflat, :basic_auth_username, "user") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + Application.put_env(:pinchflat, :expose_feed_endpoints, false) + + encoded_auth = Plug.BasicAuth.encode_basic_auth("user", "pass") + + conn = + build_conn() + |> put_req_header("authorization", encoded_auth) + |> Plugs.maybe_basic_auth([]) + + # nil here means the response is unset, but that's good. It just means we're moving to the next stage + assert conn.status == nil + end + + test "does not use basic auth when expose_feed_endpoints is true" do + Application.put_env(:pinchflat, :basic_auth_username, "user") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + Application.put_env(:pinchflat, :expose_feed_endpoints, true) + + conn = Plugs.maybe_basic_auth(build_conn(), []) + + assert conn.status == nil + end + + test "does not use basic auth when username/password aren't set" do + Application.put_env(:pinchflat, :basic_auth_username, nil) + Application.put_env(:pinchflat, :basic_auth_password, nil) + Application.put_env(:pinchflat, :expose_feed_endpoints, false) + + conn = Plugs.maybe_basic_auth(build_conn(), []) + + # nil here means the response is unset, but that's good. It just means we're moving to the next stage + assert conn.status == nil + end + end + + describe "basic_auth/2" do + setup do + old_username = Application.get_env(:pinchflat, :basic_auth_username) + old_password = Application.get_env(:pinchflat, :basic_auth_password) + + on_exit(fn -> + Application.put_env(:pinchflat, :basic_auth_username, old_username) + Application.put_env(:pinchflat, :basic_auth_password, old_password) + end) + + :ok + end + + test "uses basic auth when both username and password are set", %{conn: conn} do + Application.put_env(:pinchflat, :basic_auth_username, "user") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + + conn = Plugs.basic_auth(conn, []) + + assert conn.status == 401 + assert {"www-authenticate", "Basic realm=\"Pinchflat\""} in conn.resp_headers + end + + test "providing the username and password allows access", %{conn: conn} do + Application.put_env(:pinchflat, :basic_auth_username, "user") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + + conn = + conn + |> put_req_header("authorization", Plug.BasicAuth.encode_basic_auth("user", "pass")) + |> Plugs.basic_auth([]) + + # nil here means the response is unset, but that's good. It just means we're moving to the next stage + assert conn.status == nil + end + + test "does not use basic auth when either username or password is not set", %{conn: conn} do + Application.put_env(:pinchflat, :basic_auth_username, nil) + Application.put_env(:pinchflat, :basic_auth_password, "pass") + + conn = Plugs.basic_auth(conn, []) + + assert conn.status == nil + end + + test "treats empty strings as not being set when using basic auth", %{conn: conn} do + Application.put_env(:pinchflat, :basic_auth_username, "") + Application.put_env(:pinchflat, :basic_auth_password, "pass") + + conn = Plugs.basic_auth(conn, []) + + assert conn.status == nil + end + end + + describe "allow_iframe_embed/2" do + test "deletes the x-frame-options header", %{conn: conn} do + conn = put_resp_header(conn, "x-frame-options", "DENY") + assert ["DENY"] = get_resp_header(conn, "x-frame-options") + + conn = Plugs.allow_iframe_embed(conn, []) + + assert [] = get_resp_header(conn, "x-frame-options") + end + end + + describe "token_protected_route/2" do + test "allows access when the route token is correct", %{conn: conn} do + route_token = Settings.get!(:route_token) + conn = %{conn | query_params: %{"route_token" => route_token}} + + conn = Plugs.token_protected_route(conn, []) + + # nil here means the response is unset, but that's good. It just means we're moving to the next stage + assert conn.status == nil + end + + test "does not allow access when the route token is incorrect", %{conn: conn} do + conn = %{conn | query_params: %{"route_token" => "incorrect"}} + + conn = Plugs.token_protected_route(conn, []) + + assert conn.status == 401 + assert conn.resp_body == "Unauthorized" + end + + test "does not allow access when the route token is missing", %{conn: conn} do + conn = %{conn | query_params: %{}} + + conn = Plugs.token_protected_route(conn, []) + + assert conn.status == 401 + assert conn.resp_body == "Unauthorized" + end + end +end diff --git a/test/pinchflat_web/routing_test.exs b/test/pinchflat_web/routing_test.exs deleted file mode 100644 index cbe4dd8..0000000 --- a/test/pinchflat_web/routing_test.exs +++ /dev/null @@ -1,108 +0,0 @@ -defmodule PinchflatWeb.RoutingTest do - use PinchflatWeb.ConnCase - - import Pinchflat.SourcesFixtures - - describe "basic_auth plug" do - setup do - old_username = Application.get_env(:pinchflat, :basic_auth_username) - old_password = Application.get_env(:pinchflat, :basic_auth_password) - - on_exit(fn -> - Application.put_env(:pinchflat, :basic_auth_username, old_username) - Application.put_env(:pinchflat, :basic_auth_password, old_password) - end) - - :ok - end - - test "it uses basic auth when both username and password are set", %{conn: conn} do - Application.put_env(:pinchflat, :basic_auth_username, "user") - Application.put_env(:pinchflat, :basic_auth_password, "pass") - - conn = get(conn, "/") - - assert conn.status == 401 - assert {"www-authenticate", "Basic realm=\"Pinchflat\""} in conn.resp_headers - end - - test "providing the username and password allows access", %{conn: conn} do - Application.put_env(:pinchflat, :basic_auth_username, "user") - Application.put_env(:pinchflat, :basic_auth_password, "pass") - - conn = - conn - |> put_req_header("authorization", Plug.BasicAuth.encode_basic_auth("user", "pass")) - |> get("/") - - assert conn.status == 200 - end - - test "it does not use basic auth when either username or password is not set", %{conn: conn} do - Application.put_env(:pinchflat, :basic_auth_username, nil) - Application.put_env(:pinchflat, :basic_auth_password, "pass") - - conn = get(conn, "/") - - assert conn.status == 200 - end - - test "it treats empty strings as not being set when using basic auth", %{conn: conn} do - Application.put_env(:pinchflat, :basic_auth_username, "") - Application.put_env(:pinchflat, :basic_auth_password, "pass") - - conn = get(conn, "/") - - assert conn.status == 200 - end - end - - describe "maybe_basic_auth plug" do - setup do - old_username = Application.get_env(:pinchflat, :basic_auth_username) - old_password = Application.get_env(:pinchflat, :basic_auth_password) - old_expose_feed_endpoints = Application.get_env(:pinchflat, :expose_feed_endpoints) - - source = source_fixture() - - on_exit(fn -> - Application.put_env(:pinchflat, :basic_auth_username, old_username) - Application.put_env(:pinchflat, :basic_auth_password, old_password) - Application.put_env(:pinchflat, :expose_feed_endpoints, old_expose_feed_endpoints) - end) - - {:ok, source: source} - end - - test "uses basic auth when expose_feed_endpoints is false", %{source: source} do - Application.put_env(:pinchflat, :basic_auth_username, "user") - Application.put_env(:pinchflat, :basic_auth_password, "pass") - Application.put_env(:pinchflat, :expose_feed_endpoints, false) - - conn = get(build_conn(), "/sources/#{source.uuid}/feed") - - assert conn.status == 401 - assert {"www-authenticate", "Basic realm=\"Pinchflat\""} in conn.resp_headers - end - - test "does not use basic auth when expose_feed_endpoints is true", %{source: source} do - Application.put_env(:pinchflat, :basic_auth_username, "user") - Application.put_env(:pinchflat, :basic_auth_password, "pass") - Application.put_env(:pinchflat, :expose_feed_endpoints, true) - - conn = get(build_conn(), "/sources/#{source.uuid}/feed") - - assert conn.status == 200 - end - - test "does not use basic auth when username/password aren't set", %{source: source} do - Application.put_env(:pinchflat, :basic_auth_username, nil) - Application.put_env(:pinchflat, :basic_auth_password, nil) - Application.put_env(:pinchflat, :expose_feed_endpoints, false) - - conn = get(build_conn(), "/sources/#{source.uuid}/feed") - - assert conn.status == 200 - end - end -end