From 59c7ce2ef163a3062c5a60ad5613a804c396c301 Mon Sep 17 00:00:00 2001
From: Artur Pata <artur.pata@gmail.com>
Date: Wed, 18 Sep 2024 18:46:05 +0300
Subject: [PATCH] Fix auth issue with POST /api/docs/query (#4593)

---
 .../plugs/authorize_site_access.ex            |  6 +-
 lib/plausible_web/router.ex                   |  2 -
 .../internal_controller/docs_query_test.exs   | 60 +++++++++++++++++++
 3 files changed, 65 insertions(+), 3 deletions(-)
 create mode 100644 test/plausible_web/controllers/api/internal_controller/docs_query_test.exs

diff --git a/lib/plausible_web/plugs/authorize_site_access.ex b/lib/plausible_web/plugs/authorize_site_access.ex
index 2f5f7da8d..ba0b47eb6 100644
--- a/lib/plausible_web/plugs/authorize_site_access.ex
+++ b/lib/plausible_web/plugs/authorize_site_access.ex
@@ -64,7 +64,11 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do
   end
 
   defp get_site_with_role(conn, current_user) do
-    domain = conn.path_params["domain"] || conn.path_params["website"]
+    # addition is flimsy, do we need an extra argument on plug init to control where we look for the domain?
+    domain =
+      conn.path_params["domain"] || conn.path_params["website"] ||
+        (conn.method == "POST" && conn.path_info == ["api", "docs", "query"] &&
+           conn.params["site_id"])
 
     site_query =
       from(
diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex
index 431245b80..5fa69f8fb 100644
--- a/lib/plausible_web/router.ex
+++ b/lib/plausible_web/router.ex
@@ -50,9 +50,7 @@ defmodule PlausibleWeb.Router do
     plug :accepts, ["json"]
     plug :fetch_session
     plug PlausibleWeb.AuthPlug
-
     plug PlausibleWeb.Plugs.AuthorizeSiteAccess, [:admin, :super_admin, :owner]
-
     plug PlausibleWeb.Plugs.NoRobots
   end
 
diff --git a/test/plausible_web/controllers/api/internal_controller/docs_query_test.exs b/test/plausible_web/controllers/api/internal_controller/docs_query_test.exs
new file mode 100644
index 000000000..610615497
--- /dev/null
+++ b/test/plausible_web/controllers/api/internal_controller/docs_query_test.exs
@@ -0,0 +1,60 @@
+defmodule PlausibleWeb.Api.InternalController.DocsQueryTest do
+  use PlausibleWeb.ConnCase, async: true
+  use Plausible.Repo
+  @user_id Enum.random(1000..9999)
+
+  describe "POST /api/docs/query not logged in" do
+    setup [:create_user, :create_new_site]
+
+    test "rejects request when not logged in", %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, timestamp: ~N[2021-01-01 00:00:00])
+      ])
+
+      conn =
+        post(conn, "/api/docs/query", %{
+          "site_id" => site.domain,
+          "metrics" => ["pageviews"],
+          "date_range" => "all"
+        })
+
+      assert json_response(conn, 404) == %{
+               "error" => "Site does not exist or user does not have sufficient access."
+             }
+    end
+  end
+
+  describe "POST /api/docs/query logged in" do
+    setup [:create_user, :create_new_site, :log_in]
+
+    test "rejects when accessing any other site", %{conn: conn} do
+      conn =
+        post(conn, "/api/docs/query", %{
+          "site_id" => "any.other.site",
+          "metrics" => ["pageviews"],
+          "date_range" => "all"
+        })
+
+      assert json_response(conn, 404) == %{
+               "error" => "Site does not exist or user does not have sufficient access."
+             }
+    end
+
+    test "returns aggregated metrics", %{conn: conn, site: site} do
+      populate_stats(site, [
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 00:00:00]),
+        build(:pageview, user_id: @user_id, timestamp: ~N[2021-01-01 00:25:00]),
+        build(:pageview, timestamp: ~N[2021-01-01 00:00:00])
+      ])
+
+      conn =
+        post(conn, "/api/docs/query", %{
+          "site_id" => site.domain,
+          "metrics" => ["pageviews"],
+          "date_range" => "all"
+        })
+
+      assert json_response(conn, 200)["results"] == [%{"metrics" => [3], "dimensions" => []}]
+    end
+  end
+end