Allow configuring AuthorizeSiteAccess plug site param (#4597)

* Stop typescript command clearing previous server start output in dev

* Allow auth site access plug to specify that domain is found in request body at some key

* Fix init order

* Make domain retrieval part of the role pipeline

* Add tests

* Refactor how is_binary is applied in get_domain/2

* Make plug tests rely on dedicated test routes for more stability

* Consistently treat empty `allowed_roles` list as permitting all roles

* Fix async test param, add extra case for init

* Make `DocsQueryTest` async again

* Improve a bit and document plug configuration

* Make docs more legible when viewed from source directly

---------

Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
This commit is contained in:
Artur Pata 2024-09-26 12:52:45 +03:00 committed by GitHub
parent 2d7943aec8
commit cac4ad20c9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
6 changed files with 253 additions and 43 deletions

View File

@ -8,7 +8,7 @@ config :plausible, PlausibleWeb.Endpoint,
watchers: [
esbuild: {Esbuild, :install_and_run, [:default, ~w(--sourcemap=inline --watch)]},
tailwind: {Tailwind, :install_and_run, [:default, ~w(--watch)]},
npm: ["--prefix", "assets", "run", "typecheck", "--", "--watch"],
npm: ["--prefix", "assets", "run", "typecheck", "--", "--watch", "--preserveWatchOutput"],
npm: [
"run",
"deploy",

View File

@ -1,6 +1,36 @@
defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do
@moduledoc """
Plug restricting access to site and shared link, when present.
In order to permit access to site regardless of role:
```elixir
plug AuthorizeSiteAccess
```
or
```elixir
plug AuthorizeSiteAccess, :all_roles
```
Permit access for a subset of roles only:
```elixir
plug AuthorizeSiteAccess, [:admin, :owner, :super_admin]
```
Permit access using a custom site param:
```elixir
plug AuthorizeSiteAccess, {[:admin, :owner, :super_admin], "site_id"}
```
or in case where any role is allowed:
```elixir
plug AuthorizeSiteAccess, {:all_roles, "site_id"}
```
"""
use Plausible.Repo
@ -10,22 +40,45 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do
@all_roles [:public, :viewer, :admin, :super_admin, :owner]
def init([]), do: @all_roles
def init([]), do: {@all_roles, nil}
def init(:all_roles), do: {@all_roles, nil}
def init(allowed_roles) when is_list(allowed_roles) do
init({allowed_roles, nil})
end
def init({:all_roles, site_param}) do
init({@all_roles, site_param})
end
def init({allowed_roles, site_param}) when is_list(allowed_roles) do
allowed_roles =
if allowed_roles == [] do
@all_roles
else
allowed_roles
end
def init(allowed_roles) do
unknown_roles = allowed_roles -- @all_roles
if unknown_roles != [] do
raise ArgumentError, "Unknown allowed roles configured: #{inspect(unknown_roles)}"
end
allowed_roles
if !is_binary(site_param) && !is_nil(site_param) do
raise ArgumentError, "Invalid site param configured: #{inspect(site_param)}"
end
{allowed_roles, site_param}
end
def call(conn, allowed_roles) do
def call(conn, {allowed_roles, site_param}) do
current_user = conn.assigns[:current_user]
with {:ok, %{site: site, role: membership_role}} <- get_site_with_role(conn, current_user),
with {:ok, domain} <- get_domain(conn, site_param),
{:ok, %{site: site, role: membership_role}} <-
get_site_with_role(conn, current_user, domain),
{:ok, shared_link} <- maybe_get_shared_link(conn, site) do
role =
cond do
@ -63,13 +116,25 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do
end
end
defp get_site_with_role(conn, current_user) do
# 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.method == "POST" && conn.path_info == ["api", "docs", "query"] &&
conn.params["site_id"])
defp get_domain(conn, nil) do
if domain = conn.path_params["domain"] do
{:ok, domain}
else
error_not_found(conn)
end
end
defp get_domain(conn, site_param) do
domain = conn.params[site_param]
if is_binary(domain) do
{:ok, domain}
else
error_not_found(conn)
end
end
defp get_site_with_role(conn, current_user, domain) do
site_query =
from(
s in Plausible.Site,

View File

@ -50,7 +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.AuthorizeSiteAccess, {[:admin, :super_admin, :owner], "site_id"}
plug PlausibleWeb.Plugs.NoRobots
end
@ -99,6 +99,26 @@ defmodule PlausibleWeb.Router do
end
end
# Routes for plug integration testing
if Mix.env() in [:test, :ce_test] do
scope "/plug-tests", PlausibleWeb do
scope [] do
pipe_through :browser
get("/basic", TestController, :browser)
get("/:domain/shared-link/:slug", TestController, :browser)
get("/:domain/with-domain", TestController, :browser)
end
scope [] do
pipe_through :api
get("/api-basic", TestController, :api)
get("/:domain/api-with-domain", TestController, :api)
end
end
end
scope path: "/api/plugins", as: :plugins_api do
pipeline :plugins_api_auth do
plug(PlausibleWeb.Plugs.AuthorizePluginsAPI)

View File

@ -40,7 +40,20 @@ defmodule PlausibleWeb.Api.InternalController.DocsQueryTest do
}
end
test "returns aggregated metrics", %{conn: conn, site: site} do
test "rejects when using invalid param name", %{conn: conn, site: site} do
conn =
post(conn, "/api/docs/query", %{
"domain" => 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
test "returns aggregated metrics when site id given", %{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]),
@ -48,7 +61,7 @@ defmodule PlausibleWeb.Api.InternalController.DocsQueryTest do
])
conn =
post(conn, "/api/docs/query", %{
post(conn, "/api/docs/query?site_id=ignored_when_site_id_present_in_body", %{
"site_id" => site.domain,
"metrics" => ["pageviews"],
"date_range" => "all"
@ -56,5 +69,22 @@ defmodule PlausibleWeb.Api.InternalController.DocsQueryTest do
assert json_response(conn, 200)["results"] == [%{"metrics" => [3], "dimensions" => []}]
end
test "succeeds when site_id missing in body, but present in query params", %{
conn: conn,
site: site
} do
populate_stats(site, [
build(:pageview, user_id: @user_id, 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" => [1], "dimensions" => []}]
end
end
end

View File

@ -10,43 +10,105 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
end
end
for init_argument <- [
[],
:all_roles,
{:all_roles, nil},
{[:public, :viewer, :admin, :super_admin, :owner], nil}
] do
test "init resolves to expected options with argument #{inspect(init_argument)}" do
assert {[:public, :viewer, :admin, :super_admin, :owner], nil} ==
AuthorizeSiteAccess.init(unquote(init_argument))
end
end
for invalid_site_param <- [[], 1, :invalid] do
test "init rejects invalid site_param #{inspect(invalid_site_param)}" do
assert_raise ArgumentError, fn ->
AuthorizeSiteAccess.init({[:super_admin], unquote(invalid_site_param)})
end
end
end
test "returns 404 on non-existent site", %{conn: conn} do
opts = AuthorizeSiteAccess.init(:all_roles)
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/invalid.domain")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [])
|> get("/plug-tests/invalid.domain/with-domain")
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert html_response(conn, 404)
end
test "rejects user completely unrelated to the site", %{conn: conn} do
opts = AuthorizeSiteAccess.init(:all_roles)
site = insert(:site, members: [build(:user)])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert html_response(conn, 404)
end
test "can be configured to expect site domain at conn.params['some_key'], fails when this is not met",
%{
conn: conn,
site: site
} do
opts =
AuthorizeSiteAccess.init({:all_roles, "some_key"})
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/plug-tests/basic", %{"wrong_key" => site.domain})
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert conn.status == 404
end
test "can be configured to expect site domain at conn.params['some_key'], succeeds when it is met",
%{
conn: conn,
site: site
} do
opts =
AuthorizeSiteAccess.init({:all_roles, "some_key"})
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/plug-tests/basic", %{"some_key" => site.domain})
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
end
test "doesn't allow bypassing :domain in path with :domain in query param", %{
conn: conn,
site: site
} do
other_site = insert(:site, members: [build(:user)])
opts = AuthorizeSiteAccess.init([:admin, :owner])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/sites/#{other_site.domain}/change-domain", %{
|> get("/plug-tests/#{other_site.domain}/with-domain", %{
"domain" => site.domain
})
|> AuthorizeSiteAccess.call(_allowed_roles = [:admin, :owner])
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert conn.status == 404
@ -56,11 +118,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
test "returns 404 with custom error message for failed API routes", %{conn: conn, user: user} do
site = insert(:site, memberships: [build(:site_membership, user: user, role: :viewer)])
opts = AuthorizeSiteAccess.init([:admin, :owner, :super_admin])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/api/stats/#{site.domain}/main-graph")
|> AuthorizeSiteAccess.call([:admin, :owner, :super_admin])
|> get("/plug-tests/#{site.domain}/api-with-domain")
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
@ -77,11 +141,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
params = %{"shared_link" => %{"name" => "some name"}}
opts = AuthorizeSiteAccess.init([:super_admin, :admin, :owner])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> put("/sites/#{site.domain}/shared-links/#{shared_link_other_site.slug}", params)
|> AuthorizeSiteAccess.call(_allowed_roles = [:super_admin, :admin, :owner])
|> get("/plug-tests/#{site.domain}/shared-link/#{shared_link_other_site.slug}", params)
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert conn.status == 404
@ -96,11 +162,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
shared_link = insert(:shared_link, site: site)
other_site = insert(:site, members: [user])
opts = AuthorizeSiteAccess.init([:super_admin, :admin, :owner])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{other_site.domain}", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(_all_allowed_roles = [])
|> get("/plug-tests/#{other_site.domain}/with-domain", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert conn.status == 404
@ -119,11 +187,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
"auth" => shared_link.slug
}
opts = AuthorizeSiteAccess.init([:super_admin, :admin, :owner])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> put("/sites/#{site.domain}/shared-links/#{shared_link_other_site.slug}", params)
|> AuthorizeSiteAccess.call(_allowed_roles = [:super_admin, :admin, :owner])
|> get("/plug-tests/#{site.domain}/shared-link/#{shared_link_other_site.slug}", params)
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert conn.status == 404
@ -133,11 +203,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
site =
insert(:site, memberships: [build(:site_membership, user: user, role: :admin)])
opts = AuthorizeSiteAccess.init([:owner])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:owner])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
assert conn.halted
assert html_response(conn, 404)
@ -148,11 +220,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
site =
insert(:site, memberships: [build(:site_membership, user: user, role: unquote(role))])
opts = AuthorizeSiteAccess.init([unquote(role)])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [unquote(role)])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
@ -166,11 +240,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
patch_env(:super_admin_user_ids, [user.id])
opts = AuthorizeSiteAccess.init([:super_admin])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:super_admin])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
@ -180,11 +256,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
test "allows user based on website visibility (authenticated user)", %{conn: conn} do
site = insert(:site, members: [build(:user)], public: true)
opts = AuthorizeSiteAccess.init([:public])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:public])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
@ -193,11 +271,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
test "allows user based on website visibility (anonymous request)" do
site = insert(:site, members: [build(:user)], public: true)
opts = AuthorizeSiteAccess.init([:public])
conn =
build_conn()
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}")
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:public])
|> get("/plug-tests/#{site.domain}/with-domain")
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
@ -207,11 +287,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
site = insert(:site, members: [build(:user)])
shared_link = insert(:shared_link, site: site)
opts = AuthorizeSiteAccess.init([:public])
conn =
conn
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:public])
|> get("/plug-tests/#{site.domain}/with-domain", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id
@ -221,11 +303,13 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do
site = insert(:site, members: [build(:user)])
shared_link = insert(:shared_link, site: site)
opts = AuthorizeSiteAccess.init([:public])
conn =
build_conn()
|> bypass_through(PlausibleWeb.Router)
|> get("/#{site.domain}", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(_all_allowed_roles = [:public])
|> get("/plug-tests/#{site.domain}/with-domain", %{"auth" => shared_link.slug})
|> AuthorizeSiteAccess.call(opts)
refute conn.halted
assert conn.assigns.site.id == site.id

View File

@ -0,0 +1,11 @@
defmodule PlausibleWeb.TestController do
use PlausibleWeb, :controller
def browser_basic(conn, _params) do
send_resp(conn, 200, "ok")
end
def api_basic(conn, _params) do
send_resp(conn, 200, Jason.encode!(%{"ok" => true}))
end
end