diff --git a/lib/plausible_web/plugs/authorize_public_api.ex b/lib/plausible_web/plugs/authorize_public_api.ex index ead3fc6fd72c..6b54d3aeb2c6 100644 --- a/lib/plausible_web/plugs/authorize_public_api.ex +++ b/lib/plausible_web/plugs/authorize_public_api.ex @@ -122,7 +122,13 @@ defmodule PlausibleWeb.Plugs.AuthorizePublicAPI do defp verify_by_scope(conn, api_key, "stats:read:" <> _ = scope) do with :ok <- check_scope(api_key, scope), {:ok, site} <- find_site(conn.params["site_id"]), - :ok <- verify_site_access(api_key, site, Plausible.Billing.Feature.StatsAPI) do + :ok <- + verify_site_access( + site: site, + api_key: api_key, + feature: Plausible.Billing.Feature.StatsAPI, + allow_consolidated_views: conn.private[:allow_consolidated_views] + ) do Plausible.OpenTelemetry.add_site_attributes(site) site = Plausible.Repo.preload(site, :completed_imports) {:ok, assign(conn, :site, site)} @@ -193,7 +199,11 @@ defmodule PlausibleWeb.Plugs.AuthorizePublicAPI do defp maybe_verify_site_access(conn, api_key, feature) do case find_site(conn.params["site_id"]) do {:ok, site} -> - verify_site_access(api_key, site, feature) + verify_site_access( + site: site, + api_key: api_key, + feature: feature + ) _ -> :ok @@ -226,13 +236,21 @@ defmodule PlausibleWeb.Plugs.AuthorizePublicAPI do end end - defp verify_site_access(api_key, site, feature) do + defp verify_site_access(opts) do + site = Keyword.fetch!(opts, :site) + api_key = Keyword.fetch!(opts, :api_key) + feature = Keyword.fetch!(opts, :feature) + allow_consolidated_views = Keyword.fetch!(opts, :allow_consolidated_views) + team = Repo.preload(site, :team).team is_member? = Plausible.Teams.Memberships.site_member?(site, api_key.user) is_super_admin? = Auth.is_super_admin?(api_key.user_id) cond do + Plausible.Sites.consolidated?(site) && !allow_consolidated_views -> + {:error, :unavailable_for_consolidated_view} + is_super_admin? -> :ok @@ -278,6 +296,13 @@ defmodule PlausibleWeb.Plugs.AuthorizePublicAPI do end end + defp send_error(conn, _, {:error, :unavailable_for_consolidated_view}) do + H.bad_request( + conn, + "This operation is unavailable for a consolidated view" + ) + end + defp send_error(conn, _, {:error, :missing_api_key}) do H.unauthorized( conn, diff --git a/lib/plausible_web/plugs/authorize_site_access.ex b/lib/plausible_web/plugs/authorize_site_access.ex index 589cac5ff7d9..fa22353baa85 100644 --- a/lib/plausible_web/plugs/authorize_site_access.ex +++ b/lib/plausible_web/plugs/authorize_site_access.ex @@ -81,6 +81,7 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do with {:ok, domain} <- get_domain(conn, site_param), {:ok, %{site: site, role: membership_role, member_type: member_type}} <- get_site_with_role(conn, current_user, domain), + :ok <- ensure_consolidated_view_access(conn, site), {:ok, shared_link} <- maybe_get_shared_link(conn, site) do role = cond do @@ -188,6 +189,14 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccess do end end + defp ensure_consolidated_view_access(conn, site) do + if Plausible.Sites.consolidated?(site) && !conn.private[:allow_consolidated_views] do + error_not_found(conn) + else + :ok + end + end + defp maybe_get_shared_link(conn, site) do slug = conn.path_params["slug"] || conn.params["auth"] diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index dadd328576f6..03c869400e1a 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -268,38 +268,41 @@ defmodule PlausibleWeb.Router do get "/:domain/funnels/:id", StatsController, :funnel end - get "/:domain/current-visitors", StatsController, :current_visitors - get "/:domain/main-graph", StatsController, :main_graph - get "/:domain/top-stats", StatsController, :top_stats - get "/:domain/sources", StatsController, :sources - get "/:domain/channels", StatsController, :channels - get "/:domain/utm_mediums", StatsController, :utm_mediums - get "/:domain/utm_sources", StatsController, :utm_sources - get "/:domain/utm_campaigns", StatsController, :utm_campaigns - get "/:domain/utm_contents", StatsController, :utm_contents - get "/:domain/utm_terms", StatsController, :utm_terms - get "/:domain/referrers/:referrer", StatsController, :referrer_drilldown - get "/:domain/pages", StatsController, :pages - get "/:domain/entry-pages", StatsController, :entry_pages - get "/:domain/exit-pages", StatsController, :exit_pages - get "/:domain/countries", StatsController, :countries - get "/:domain/regions", StatsController, :regions - get "/:domain/cities", StatsController, :cities - get "/:domain/browsers", StatsController, :browsers - get "/:domain/browser-versions", StatsController, :browser_versions - get "/:domain/operating-systems", StatsController, :operating_systems - get "/:domain/operating-system-versions", StatsController, :operating_system_versions - get "/:domain/screen-sizes", StatsController, :screen_sizes - get "/:domain/conversions", StatsController, :conversions - get "/:domain/custom-prop-values/:prop_key", StatsController, :custom_prop_values - get "/:domain/suggestions/:filter_name", StatsController, :filter_suggestions - - get "/:domain/suggestions/custom-prop-values/:prop_key", - StatsController, - :custom_prop_value_filter_suggestions - end - - scope "/:domain/segments", PlausibleWeb.Api.Internal do + scope private: %{allow_consolidated_views: true} do + get "/:domain/current-visitors", StatsController, :current_visitors + get "/:domain/main-graph", StatsController, :main_graph + get "/:domain/top-stats", StatsController, :top_stats + get "/:domain/sources", StatsController, :sources + get "/:domain/channels", StatsController, :channels + get "/:domain/utm_mediums", StatsController, :utm_mediums + get "/:domain/utm_sources", StatsController, :utm_sources + get "/:domain/utm_campaigns", StatsController, :utm_campaigns + get "/:domain/utm_contents", StatsController, :utm_contents + get "/:domain/utm_terms", StatsController, :utm_terms + get "/:domain/referrers/:referrer", StatsController, :referrer_drilldown + get "/:domain/pages", StatsController, :pages + get "/:domain/entry-pages", StatsController, :entry_pages + get "/:domain/exit-pages", StatsController, :exit_pages + get "/:domain/countries", StatsController, :countries + get "/:domain/regions", StatsController, :regions + get "/:domain/cities", StatsController, :cities + get "/:domain/browsers", StatsController, :browsers + get "/:domain/browser-versions", StatsController, :browser_versions + get "/:domain/operating-systems", StatsController, :operating_systems + get "/:domain/operating-system-versions", StatsController, :operating_system_versions + get "/:domain/screen-sizes", StatsController, :screen_sizes + get "/:domain/conversions", StatsController, :conversions + get "/:domain/custom-prop-values/:prop_key", StatsController, :custom_prop_values + get "/:domain/suggestions/:filter_name", StatsController, :filter_suggestions + + get "/:domain/suggestions/custom-prop-values/:prop_key", + StatsController, + :custom_prop_value_filter_suggestions + end + end + + scope "/:domain/segments", PlausibleWeb.Api.Internal, + private: %{allow_consolidated_views: true} do post "/", SegmentsController, :create patch "/:segment_id", SegmentsController, :update delete "/:segment_id", SegmentsController, :delete @@ -317,7 +320,14 @@ defmodule PlausibleWeb.Router do end scope "/api/v2", PlausibleWeb.Api, - assigns: %{api_scope: "stats:read:*", api_context: :site, schema_type: :public} do + private: %{ + allow_consolidated_views: true + }, + assigns: %{ + api_scope: "stats:read:*", + api_context: :site, + schema_type: :public + } do pipe_through [:public_api, PlausibleWeb.Plugs.AuthorizePublicAPI] post "/query", ExternalQueryApiController, :query @@ -559,44 +569,6 @@ defmodule PlausibleWeb.Router do post "/sites", SiteController, :create_site post "/sites/:domain/make-public", SiteController, :make_public post "/sites/:domain/make-private", SiteController, :make_private - post "/sites/:domain/weekly-report/enable", SiteController, :enable_weekly_report - post "/sites/:domain/weekly-report/disable", SiteController, :disable_weekly_report - post "/sites/:domain/weekly-report/recipients", SiteController, :add_weekly_report_recipient - - delete "/sites/:domain/weekly-report/recipients/:recipient", - SiteController, - :remove_weekly_report_recipient - - post "/sites/:domain/monthly-report/enable", SiteController, :enable_monthly_report - post "/sites/:domain/monthly-report/disable", SiteController, :disable_monthly_report - - post "/sites/:domain/monthly-report/recipients", - SiteController, - :add_monthly_report_recipient - - delete "/sites/:domain/monthly-report/recipients/:recipient", - SiteController, - :remove_monthly_report_recipient - - post "/sites/:domain/traffic-change-notification/:type/enable", - SiteController, - :enable_traffic_change_notification - - post "/sites/:domain/traffic-change-notification/:type/disable", - SiteController, - :disable_traffic_change_notification - - put "/sites/:domain/traffic-change-notification/:type", - SiteController, - :update_traffic_change_notification - - post "/sites/:domain/traffic-change-notification/:type/recipients", - SiteController, - :add_traffic_change_notification_recipient - - delete "/sites/:domain/traffic-change-notification/:type/recipients/:recipient", - SiteController, - :remove_traffic_change_notification_recipient get "/sites/:domain/memberships/invite", Site.MembershipController, :invite_member_form post "/sites/:domain/memberships/invite", Site.MembershipController, :invite_member @@ -619,9 +591,6 @@ defmodule PlausibleWeb.Router do delete "/sites/:domain/memberships/u/:id", Site.MembershipController, :remove_member_by_user - get "/sites/:domain/weekly-report/unsubscribe", UnsubscribeController, :weekly_report - get "/sites/:domain/monthly-report/unsubscribe", UnsubscribeController, :monthly_report - scope alias: Live, assigns: %{connect_live_socket: true} do pipe_through [:app_layout, PlausibleWeb.RequireAccountPlug] @@ -648,28 +617,18 @@ defmodule PlausibleWeb.Router do end end - get "/:domain/settings", SiteController, :settings - get "/:domain/settings/general", SiteController, :settings_general get "/:domain/settings/people", SiteController, :settings_people get "/:domain/settings/visibility", SiteController, :settings_visibility - get "/:domain/settings/goals", SiteController, :settings_goals - get "/:domain/settings/properties", SiteController, :settings_props on_ee do get "/:domain/settings/funnels", SiteController, :settings_funnels end - get "/:domain/settings/email-reports", SiteController, :settings_email_reports get "/:domain/settings/danger-zone", SiteController, :settings_danger_zone get "/:domain/settings/integrations", SiteController, :settings_integrations get "/:domain/settings/shields/:shield", SiteController, :settings_shields get "/:domain/settings/imports-exports", SiteController, :settings_imports_exports - put "/:domain/settings/features/visibility/:setting", - SiteController, - :update_feature_visibility - - put "/:domain/settings", SiteController, :update_settings put "/:domain/settings/google", SiteController, :update_google_auth delete "/:domain/settings/google-search", SiteController, :delete_google_auth delete "/:domain/settings/google-import", SiteController, :delete_google_auth @@ -695,7 +654,63 @@ defmodule PlausibleWeb.Router do get "/debug/clickhouse", DebugController, :clickhouse - get "/:domain/export", StatsController, :csv_export - get "/:domain/*path", StatsController, :stats + scope private: %{allow_consolidated_views: true} do + post "/sites/:domain/weekly-report/enable", SiteController, :enable_weekly_report + post "/sites/:domain/weekly-report/disable", SiteController, :disable_weekly_report + post "/sites/:domain/weekly-report/recipients", SiteController, :add_weekly_report_recipient + + delete "/sites/:domain/weekly-report/recipients/:recipient", + SiteController, + :remove_weekly_report_recipient + + post "/sites/:domain/monthly-report/enable", SiteController, :enable_monthly_report + post "/sites/:domain/monthly-report/disable", SiteController, :disable_monthly_report + + post "/sites/:domain/monthly-report/recipients", + SiteController, + :add_monthly_report_recipient + + delete "/sites/:domain/monthly-report/recipients/:recipient", + SiteController, + :remove_monthly_report_recipient + + post "/sites/:domain/traffic-change-notification/:type/enable", + SiteController, + :enable_traffic_change_notification + + post "/sites/:domain/traffic-change-notification/:type/disable", + SiteController, + :disable_traffic_change_notification + + put "/sites/:domain/traffic-change-notification/:type", + SiteController, + :update_traffic_change_notification + + post "/sites/:domain/traffic-change-notification/:type/recipients", + SiteController, + :add_traffic_change_notification_recipient + + delete "/sites/:domain/traffic-change-notification/:type/recipients/:recipient", + SiteController, + :remove_traffic_change_notification_recipient + + get "/sites/:domain/weekly-report/unsubscribe", UnsubscribeController, :weekly_report + get "/sites/:domain/monthly-report/unsubscribe", UnsubscribeController, :monthly_report + + get "/:domain/settings", SiteController, :settings + get "/:domain/settings/general", SiteController, :settings_general + get "/:domain/settings/goals", SiteController, :settings_goals + get "/:domain/settings/properties", SiteController, :settings_props + get "/:domain/settings/email-reports", SiteController, :settings_email_reports + + put "/:domain/settings/features/visibility/:setting", + SiteController, + :update_feature_visibility + + put "/:domain/settings", SiteController, :update_settings + + get "/:domain/export", StatsController, :csv_export + get "/:domain/*path", StatsController, :stats + end end end diff --git a/test/plausible_web/controllers/api/external_sites_controller_sites_crud_api_test.exs b/test/plausible_web/controllers/api/external_sites_controller_sites_crud_api_test.exs index 04ad9708211a..785ddfccf254 100644 --- a/test/plausible_web/controllers/api/external_sites_controller_sites_crud_api_test.exs +++ b/test/plausible_web/controllers/api/external_sites_controller_sites_crud_api_test.exs @@ -895,6 +895,24 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerSitesCrudApiTest do assert site.domain_changed_from == nil end + test "cannot update consolidated view", %{conn: conn, user: user} do + new_site(owner: user) + {:ok, team} = Plausible.Teams.get_or_create(user) + consolidated_view = new_consolidated_view(team) + + old_domain = consolidated_view.domain + + conn = + put(conn, "/api/v1/sites/#{consolidated_view.domain}", %{ + "domain" => "updated.domain.com" + }) + + assert json_response(conn, 400)["error"] =~ + "This operation is unavailable for a consolidated view" + + assert Repo.reload!(consolidated_view).domain == old_domain + end + test "fails when team does not match team-scoped key", %{conn: conn, user: user, site: site} do another_team = new_user() |> subscribe_to_business_plan() |> team_of() add_member(another_team, user: user, role: :admin) diff --git a/test/plausible_web/controllers/site_controller_test.exs b/test/plausible_web/controllers/site_controller_test.exs index b57d5db186c1..5555caee4568 100644 --- a/test/plausible_web/controllers/site_controller_test.exs +++ b/test/plausible_web/controllers/site_controller_test.exs @@ -615,6 +615,18 @@ defmodule PlausibleWeb.SiteControllerTest do describe "GET /:domain/settings/people" do setup [:create_user, :log_in, :create_site] + on_ee do + test "returns 404 for consolidated view", %{conn: conn, user: user} do + {:ok, team} = Plausible.Teams.get_or_create(user) + new_site(team: team) + new_site(team: team) + consolidated_view = new_consolidated_view(team) + + conn = get(conn, "/#{consolidated_view.domain}/settings/people") + assert html_response(conn, 404) + end + end + test "lists current members", %{conn: conn, user: user} do site = new_site(owner: user) editor = add_guest(site, role: :editor) diff --git a/test/plausible_web/plugs/authorize_public_api_test.exs b/test/plausible_web/plugs/authorize_public_api_test.exs index 4847668e1ae3..c078b8cd944d 100644 --- a/test/plausible_web/plugs/authorize_public_api_test.exs +++ b/test/plausible_web/plugs/authorize_public_api_test.exs @@ -89,6 +89,31 @@ defmodule PlausibleWeb.Plugs.AuthorizePublicAPITest do "The account that owns this API key does not have access" end + on_ee do + test "rejects access to a site that is a consolidated view (unless instructed otherwise)", %{ + conn: conn + } do + user = new_user() + api_key = insert(:api_key, user: user, scopes: ["sites:provision:*"]) + {:ok, team} = new_user() |> Plausible.Teams.get_or_create() + new_site(team: team) + new_site(team: team) + consolidated_view = new_consolidated_view(team) + + conn = + conn + |> put_req_header("authorization", "Bearer #{api_key.key}") + |> get("/", %{"site_id" => consolidated_view.domain}) + |> assign(:api_scope, "sites:provision:*") + |> AuthorizePublicAPI.call(nil) + + assert conn.halted + + assert json_response(conn, 400)["error"] =~ + "This operation is unavailable for a consolidated view" + end + end + @tag :ee_only test "halts with error when upgrade is required", %{conn: conn} do user = new_user() |> subscribe_to_enterprise_plan(paddle_plan_id: "123321", features: []) diff --git a/test/plausible_web/plugs/authorize_site_access_test.exs b/test/plausible_web/plugs/authorize_site_access_test.exs index 3440325ba1b9..47519121f375 100644 --- a/test/plausible_web/plugs/authorize_site_access_test.exs +++ b/test/plausible_web/plugs/authorize_site_access_test.exs @@ -148,6 +148,25 @@ defmodule PlausibleWeb.Plugs.AuthorizeSiteAccessTest do } end + on_ee do + test "returns 404 for consolidated views by default", %{conn: conn, user: user} do + {:ok, team} = Plausible.Teams.get_or_create(user) + new_site(team: team) + consolidated_view = new_consolidated_view(team) + + opts = AuthorizeSiteAccess.init([:super_admin, :admin, :owner]) + + conn = + conn + |> bypass_through(PlausibleWeb.Router) + |> get("/plug-tests/#{consolidated_view.domain}/with-domain") + |> AuthorizeSiteAccess.call(opts) + + assert conn.halted + assert conn.status == 404 + end + end + test "rejects unrelated shared link slug even if user is permitted for site", %{ conn: conn, site: site