Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ All notable changes to this project will be documented in this file.

### Fixed

- To make internal stats API requests for password-protected shared links, shared link auth cookie must be set in the requests

## v3.1.0 - 2025-11-13

### Added
Expand Down
3 changes: 3 additions & 0 deletions lib/plausible/site/shared_link.ex
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,7 @@ defmodule Plausible.Site.SharedLink do
change(link, password_hash: hash)
end
end

def get_type(%__MODULE__{password_hash: hash}) when not is_nil(hash), do: :password_protected
def get_type(%__MODULE__{}), do: :unlisted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

naming: :unlisted doesn't feel descriptive to me, not sure if it's some kind of terminology I'm not familiar with? How about :public?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I agree that it's a bit mystical, but that name was in the codebase already. I'm wary of :public because it might be confused with the fully public dashboard.

I can go simple with :with_password, :without_password or just def password_protected?(link) true / false? If we go with the boolean, matching on it isn't as pretty in with ... <- {function name}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In plugins API, shared_link schema has a field :password_protected. I've decided to unify the checks with that in b8bb107

end
25 changes: 16 additions & 9 deletions lib/plausible_web/controllers/stats_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -291,14 +291,24 @@ defmodule PlausibleWeb.StatsController do
render_error(conn, 400)
end

defp render_password_protected_shared_link(conn, shared_link) do
with conn <- Plug.Conn.fetch_cookies(conn),
{:ok, token} <- Map.fetch(conn.req_cookies, shared_link_cookie_name(shared_link.slug)),
def validate_shared_link_password(conn, shared_link) do
with {:ok, token} <- Map.fetch(conn.req_cookies, shared_link_cookie_name(shared_link.slug)),
{:ok, %{slug: token_slug}} <- Plausible.Auth.Token.verify_shared_link(token),
true <- token_slug == shared_link.slug do
render_shared_link(conn, shared_link)
{:ok, shared_link}
else
_e ->
_e -> {:error, :unauthorized}
end
end

defp render_password_protected_shared_link(conn, shared_link) do
conn = Plug.Conn.fetch_cookies(conn)

case validate_shared_link_password(conn, shared_link) do
{:ok, shared_link} ->
render_shared_link(conn, shared_link)

_ ->
conn
|> render("shared_link_password.html",
link: shared_link,
Expand All @@ -320,11 +330,8 @@ defmodule PlausibleWeb.StatsController do
)

case Repo.one(link_query) do
%Plausible.Site.SharedLink{password_hash: hash} = link when not is_nil(hash) ->
{:password_protected, link}

%Plausible.Site.SharedLink{} = link ->
{:unlisted, link}
{Plausible.Site.SharedLink.get_type(link), link}

nil ->
:not_found
Expand Down
11 changes: 9 additions & 2 deletions lib/plausible_web/plugs/authorize_site_access.ex
Original file line number Diff line number Diff line change
Expand Up @@ -201,10 +201,17 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do
slug = conn.path_params["slug"] || conn.params["auth"]

if valid_path_fragment?(slug) do
if shared_link = Repo.get_by(Plausible.Site.SharedLink, slug: slug, site_id: site.id) do
with %Plausible.Site.SharedLink{} = shared_link <-
Repo.get_by(Plausible.Site.SharedLink, slug: slug, site_id: site.id),
{:password_protected, shared_link} <-
{Plausible.Site.SharedLink.get_type(shared_link), shared_link},
{:ok, shared_link} <-
PlausibleWeb.StatsController.validate_shared_link_password(conn, shared_link) do
{:ok, shared_link}
else
error_not_found(conn)
{:unlisted, shared_link} -> {:ok, shared_link}
{:error, :unauthorized} -> error_not_found(conn)
nil -> error_not_found(conn)
end
else
{:ok, nil}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,20 +1,24 @@
defmodule PlausibleWeb.Api.StatsController.AuthorizationTest do
use PlausibleWeb.ConnCase
use PlausibleWeb.ConnCase, async: true

describe "API authorization - as anonymous user" do
test "Sends 404 Not found for a site that doesn't exist", %{conn: conn} do
test "returns 404 for a site that doesn't exist", %{conn: conn} do
conn = init_session(conn)
conn = get(conn, "/api/stats/fake-site.com/main-graph")

assert conn.status == 404
assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "Sends 404 Not found for private site", %{conn: conn} do
test "returns 404 for private site", %{conn: conn} do
conn = init_session(conn)
site = insert(:site, public: false)
conn = get(conn, "/api/stats/#{site.domain}/main-graph")

assert conn.status == 404
assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "returns stats for public site", %{conn: conn} do
Expand All @@ -26,21 +30,102 @@ defmodule PlausibleWeb.Api.StatsController.AuthorizationTest do
end
end

describe "API authorization for shared links - as anonymous user" do
test "returns 404 for non-existent shared link", %{conn: conn} do
site = new_site()

conn = get(conn, "/api/stats/#{site.domain}/top-stats?auth=does-not-exist")

assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "returns 200 for unlisted shared link without cookie", %{conn: conn} do
site = new_site()
link = insert(:shared_link, site: site)

conn = get(conn, "/api/stats/#{site.domain}/top-stats?auth=#{link.slug}")

assert %{"top_stats" => _any} = json_response(conn, 200)
end

test "returns 200 for password-protected link with valid cookie", %{conn: conn} do
site = new_site()

link =
insert(:shared_link, site: site, password_hash: Plausible.Auth.Password.hash("password"))

token = Plausible.Auth.Token.sign_shared_link(link.slug)
cookie_name = "shared-link-" <> link.slug

conn =
conn
|> put_req_cookie(cookie_name, token)
|> get("/api/stats/#{site.domain}/top-stats?auth=#{link.slug}")

assert %{"top_stats" => _any} = json_response(conn, 200)
end

test "returns 404 for password-protected link with invalid cookie value", %{conn: conn} do
site = new_site()

link =
insert(:shared_link, site: site, password_hash: Plausible.Auth.Password.hash("password"))

other_link =
insert(:shared_link,
name: "other link",
site: site,
password_hash: Plausible.Auth.Password.hash("password")
)

other_link_token = Plausible.Auth.Token.sign_shared_link(other_link.slug)
cookie_name = "shared-link-" <> link.slug

conn =
conn
|> put_req_cookie(cookie_name, other_link_token)
|> get("/api/stats/#{site.domain}/top-stats?auth=#{link.slug}")

assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "returns 404 for password-protected link without cookie", %{conn: conn} do
site = new_site()

link =
insert(:shared_link, site: site, password_hash: Plausible.Auth.Password.hash("password"))

conn = get(conn, "/api/stats/#{site.domain}/top-stats?auth=#{link.slug}")

assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end
end

describe "API authorization - as logged in user" do
setup [:create_user, :log_in]

test "Sends 404 Not found for a site that doesn't exist", %{conn: conn} do
test "returns 404 for a site that doesn't exist", %{conn: conn} do
conn = init_session(conn)
conn = get(conn, "/api/stats/fake-site.com/main-graph/")

assert conn.status == 404
assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "Sends 404 Not found when user does not have access to site", %{conn: conn} do
test "returns 404 when user does not have access to site", %{conn: conn} do
site = new_site()
conn = get(conn, "/api/stats/#{site.domain}/main-graph")

assert conn.status == 404
assert json_response(conn, 404) == %{
"error" => "Site does not exist or user does not have sufficient access."
}
end

test "returns stats for public site", %{conn: conn} do
Expand Down
Loading