From c0fe2a3996932092d66ae49de817b535d5124079 Mon Sep 17 00:00:00 2001 From: Vini Brasil Date: Wed, 11 Oct 2023 17:24:16 -0300 Subject: [PATCH] Implement Stats API feature gate (#3411) * Include ApiKey functions in Auth context * Make feature notice work without %Site{} Previously the extra feature notice required a %Site{} in order to check the owner plan. However, not every feature is scoped by site, for example the Stats API. For features like this, a %User{} is required, and not a %Site{}. This commit replaces the `:site` param with `:billable_user`, which is common to both site and user-scoped features. * Add stats_api to the list of extra features * Limit API Key creation based on user plan --- lib/plausible/auth/api_key.ex | 14 +++--- lib/plausible/auth/auth.ex | 38 +++++++++++++++ lib/plausible/billing/feature.ex | 33 ++++++------- lib/plausible/billing/plans.ex | 2 +- lib/plausible/billing/quota.ex | 2 +- lib/plausible/goals.ex | 3 +- lib/plausible/props.ex | 3 +- lib/plausible_web/components/billing.ex | 12 ++--- lib/plausible_web/components/site/feature.ex | 2 +- .../controllers/auth_controller.ex | 32 +++++-------- .../controllers/site_controller.ex | 4 +- lib/plausible_web/live/goal_settings/form.ex | 4 +- .../plugs/authorize_stats_api.ex | 34 ++++++++----- .../templates/auth/user_settings.html.heex | 19 ++++++-- .../templates/site/settings_funnels.html.heex | 2 +- .../templates/site/settings_props.html.heex | 2 +- priv/plans_v1.json | 20 ++++---- priv/plans_v2.json | 20 ++++---- priv/plans_v3.json | 16 +++---- priv/plans_v4.json | 16 +++---- priv/sandbox_plans.json | 32 ++++++------- test/plausible/auth/auth_test.exs | 48 +++++++++++++++++++ test/plausible/billing/feature_test.exs | 47 ++++++++++++------ test/plausible/billing/quota_test.exs | 18 +++---- .../plausible_web/components/billing_test.exs | 21 ++------ .../external_stats_controller/auth_test.exs | 18 +++++++ .../controllers/auth_controller_test.exs | 6 +-- 27 files changed, 298 insertions(+), 170 deletions(-) diff --git a/lib/plausible/auth/api_key.ex b/lib/plausible/auth/api_key.ex index adb77ddf8..78fa5b3a9 100644 --- a/lib/plausible/auth/api_key.ex +++ b/lib/plausible/auth/api_key.ex @@ -2,6 +2,8 @@ defmodule Plausible.Auth.ApiKey do use Ecto.Schema import Ecto.Changeset + @type t() :: %__MODULE__{} + @required [:user_id, :name] @optional [:key, :scopes, :hourly_request_limit] schema "api_keys" do @@ -22,7 +24,7 @@ defmodule Plausible.Auth.ApiKey do schema |> cast(attrs, @required ++ @optional) |> validate_required(@required) - |> generate_key() + |> maybe_put_key() |> process_key() |> unique_constraint(:key_hash, error_key: :key) end @@ -50,12 +52,12 @@ defmodule Plausible.Auth.ApiKey do def process_key(changeset), do: changeset - defp generate_key(changeset) do - if !changeset.changes[:key] do - key = :crypto.strong_rand_bytes(64) |> Base.url_encode64() |> binary_part(0, 64) - change(changeset, key: key) - else + defp maybe_put_key(changeset) do + if get_change(changeset, :key) do changeset + else + key = :crypto.strong_rand_bytes(64) |> Base.url_encode64() |> binary_part(0, 64) + put_change(changeset, :key, key) end end diff --git a/lib/plausible/auth/auth.ex b/lib/plausible/auth/auth.ex index 126e27553..4f48e99f2 100644 --- a/lib/plausible/auth/auth.ex +++ b/lib/plausible/auth/auth.ex @@ -125,4 +125,42 @@ defmodule Plausible.Auth do |> Ecto.assoc(:enterprise_plan) |> Repo.exists?() end + + @spec create_api_key(Auth.User.t(), String.t(), String.t()) :: + {:ok, Auth.ApiKey.t()} | {:error, Ecto.Changeset.t()} + def create_api_key(user, name, key) do + params = %{name: name, user_id: user.id, key: key} + changeset = Auth.ApiKey.changeset(%Auth.ApiKey{}, params) + + with :ok <- Plausible.Billing.Feature.StatsAPI.check_availability(user), + do: Repo.insert(changeset) + end + + @spec delete_api_key(Auth.User.t(), integer()) :: :ok | {:error, :not_found} + def delete_api_key(user, id) do + query = from(api_key in Auth.ApiKey, where: api_key.id == ^id and api_key.user_id == ^user.id) + + case Repo.delete_all(query) do + {1, _} -> :ok + {0, _} -> {:error, :not_found} + end + end + + @spec find_api_key(String.t()) :: {:ok, Auth.ApiKey.t()} | {:error, :invalid_api_key} + def find_api_key(raw_key) do + hashed_key = Auth.ApiKey.do_hash(raw_key) + + query = + from(api_key in Auth.ApiKey, + join: user in assoc(api_key, :user), + where: api_key.key_hash == ^hashed_key, + preload: [user: user] + ) + + if found = Repo.one(query) do + {:ok, found} + else + {:error, :invalid_api_key} + end + end end diff --git a/lib/plausible/billing/feature.ex b/lib/plausible/billing/feature.ex index 37f9d8aa7..3fe5677ed 100644 --- a/lib/plausible/billing/feature.ex +++ b/lib/plausible/billing/feature.ex @@ -47,8 +47,8 @@ defmodule Plausible.Billing.Feature do @doc """ Checks whether the site owner or the user plan includes the given feature. """ - @callback check_availability(Plausible.Site.t() | Plausible.Auth.User.t()) :: - :ok | {:error, :upgrade_required} + @callback check_availability(Plausible.Auth.User.t()) :: + :ok | {:error, :upgrade_required} | {:error, :not_implemented} @features [ Plausible.Billing.Feature.Funnels, @@ -77,22 +77,17 @@ defmodule Plausible.Billing.Feature do def toggle_field, do: Keyword.get(unquote(opts), :toggle_field) @impl true - def enabled?(site) do + def enabled?(%Plausible.Site{} = site) do + site = Plausible.Repo.preload(site, :owner) + cond do - check_availability(site) !== :ok -> false + check_availability(site.owner) !== :ok -> false is_nil(toggle_field()) -> true - true -> Map.get(site, toggle_field()) + true -> Map.fetch!(site, toggle_field()) end end @impl true - def check_availability(site_or_user) - - def check_availability(%Plausible.Site{} = site) do - site = Plausible.Repo.preload(site, :owner) - check_availability(site.owner) - end - def check_availability(%Plausible.Auth.User{} = user) do extra_feature = Keyword.get(unquote(opts), :extra_feature) @@ -105,9 +100,10 @@ defmodule Plausible.Billing.Feature do end @impl true - def toggle(site, opts \\ []) do + def toggle(%Plausible.Site{} = site, opts \\ []) do with key when not is_nil(key) <- toggle_field(), - :ok <- check_availability(site) do + site <- Plausible.Repo.preload(site, :owner), + :ok <- check_availability(site.owner) do override = Keyword.get(opts, :override) toggle = if is_boolean(override), do: override, else: !Map.fetch!(site, toggle_field()) @@ -119,8 +115,6 @@ defmodule Plausible.Billing.Feature do {:error, :upgrade_required} -> {:error, :upgrade_required} end end - - defoverridable check_availability: 1, toggle: 1, toggle: 2 end end end @@ -154,3 +148,10 @@ defmodule Plausible.Billing.Feature.Props do toggle_field: :props_enabled, extra_feature: :props end + +defmodule Plausible.Billing.Feature.StatsAPI do + @moduledoc false + use Plausible.Billing.Feature, + display_name: "Stats API", + extra_feature: :stats_api +end diff --git a/lib/plausible/billing/plans.ex b/lib/plausible/billing/plans.ex index 7b4368be2..0b9cfe592 100644 --- a/lib/plausible/billing/plans.ex +++ b/lib/plausible/billing/plans.ex @@ -31,7 +31,7 @@ defmodule Plausible.Billing.Plans do alias Plausible.Billing.{Subscription, Plan, EnterprisePlan} alias Plausible.Auth.User - @available_features ["props", "revenue_goals", "funnels"] + @available_features ["props", "revenue_goals", "funnels", "stats_api"] for f <- [ :plans_v1, diff --git a/lib/plausible/billing/quota.ex b/lib/plausible/billing/quota.ex index 0879d9bcc..491d550bb 100644 --- a/lib/plausible/billing/quota.ex +++ b/lib/plausible/billing/quota.ex @@ -162,7 +162,7 @@ defmodule Plausible.Billing.Quota do end) end - @all_features [:props, :revenue_goals, :funnels] + @all_features [:props, :revenue_goals, :funnels, :stats_api] @doc """ Returns a list of extra features the user can use. Trial users have the ability to use all features during their trial. diff --git a/lib/plausible/goals.ex b/lib/plausible/goals.ex index 7d2f3f479..170ae0ee2 100644 --- a/lib/plausible/goals.ex +++ b/lib/plausible/goals.ex @@ -214,7 +214,8 @@ defmodule Plausible.Goals do defp maybe_check_feature_access(site, changeset) do if Ecto.Changeset.get_field(changeset, :currency) do - Plausible.Billing.Feature.RevenueGoals.check_availability(site) + site = Plausible.Repo.preload(site, :owner) + Plausible.Billing.Feature.RevenueGoals.check_availability(site.owner) else :ok end diff --git a/lib/plausible/props.ex b/lib/plausible/props.ex index 8a36a43b3..726da24fe 100644 --- a/lib/plausible/props.ex +++ b/lib/plausible/props.ex @@ -24,7 +24,8 @@ defmodule Plausible.Props do data to be dropped or lost. """ def allow(site, prop_or_props) do - with :ok <- Plausible.Billing.Feature.Props.check_availability(site) do + with site <- Plausible.Repo.preload(site, :owner), + :ok <- Plausible.Billing.Feature.Props.check_availability(site.owner) do site |> allow_changeset(prop_or_props) |> Plausible.Repo.update() diff --git a/lib/plausible_web/components/billing.ex b/lib/plausible_web/components/billing.ex index ee68b6b3b..9f039bb0b 100644 --- a/lib/plausible_web/components/billing.ex +++ b/lib/plausible_web/components/billing.ex @@ -7,7 +7,7 @@ defmodule PlausibleWeb.Components.Billing do alias PlausibleWeb.Router.Helpers, as: Routes alias Plausible.Billing.Subscription - attr(:site, Plausible.Site, required: true) + attr(:billable_user, Plausible.Auth.User, required: true) attr(:current_user, Plausible.Auth.User, required: true) attr(:feature_mod, :atom, required: true, values: Plausible.Billing.Feature.list()) attr(:grandfathered?, :boolean, default: false) @@ -17,8 +17,8 @@ defmodule PlausibleWeb.Components.Billing do # credo:disable-for-next-line Credo.Check.Refactor.CyclomaticComplexity def extra_feature_notice(assigns) do private_preview? = not FunWithFlags.enabled?(:business_tier, for: assigns.current_user) - owner? = assigns.current_user.id == assigns.site.owner.id - has_access? = assigns.feature_mod.check_availability(assigns.site.owner) == :ok + display_upgrade_link? = assigns.current_user.id == assigns.billable_user.id + has_access? = assigns.feature_mod.check_availability(assigns.billable_user) == :ok message = cond do @@ -28,10 +28,10 @@ defmodule PlausibleWeb.Components.Billing do private_preview? -> "#{assigns.feature_mod.display_name()} is an upcoming premium functionality that's free-to-use during the private preview. Pricing will be announced soon." - Plausible.Billing.on_trial?(assigns.site.owner) -> + Plausible.Billing.on_trial?(assigns.billable_user) -> "#{assigns.feature_mod.display_name()} is part of the Plausible Business plan. You can access it during your trial, but you'll need to subscribe to the Business plan to retain access after the trial ends." - not has_access? && owner? -> + not has_access? && display_upgrade_link? -> ~H""" <%= @feature_mod.display_name() %> is part of the Plausible Business plan. To get access to it, please <.link class="underline" href={Routes.billing_path(PlausibleWeb.Endpoint, :upgrade)}> @@ -39,7 +39,7 @@ defmodule PlausibleWeb.Components.Billing do to the Business plan. """ - not has_access? && not owner? -> + not has_access? && not display_upgrade_link? -> "#{assigns.feature_mod.display_name()} is part of the Plausible Business plan. To get access to it, please reach out to the site owner to upgrade your subscription to the Business plan." true -> diff --git a/lib/plausible_web/components/site/feature.ex b/lib/plausible_web/components/site/feature.ex index fbaa8bba9..07239108a 100644 --- a/lib/plausible_web/components/site/feature.ex +++ b/lib/plausible_web/components/site/feature.ex @@ -14,7 +14,7 @@ defmodule PlausibleWeb.Components.Site.Feature do assigns = assigns |> assign(:current_setting, assigns.feature_mod.enabled?(assigns.site)) - |> assign(:disabled?, assigns.feature_mod.check_availability(assigns.site) !== :ok) + |> assign(:disabled?, assigns.feature_mod.check_availability(assigns.site.owner) !== :ok) ~H"""
diff --git a/lib/plausible_web/controllers/auth_controller.ex b/lib/plausible_web/controllers/auth_controller.ex index 5b93e06cb..fbc0ff43b 100644 --- a/lib/plausible_web/controllers/auth_controller.ex +++ b/lib/plausible_web/controllers/auth_controller.ex @@ -414,8 +414,7 @@ defmodule PlausibleWeb.AuthController do end def new_api_key(conn, _params) do - key = :crypto.strong_rand_bytes(64) |> Base.url_encode64() |> binary_part(0, 64) - changeset = Auth.ApiKey.changeset(%Auth.ApiKey{}, %{key: key}) + changeset = Auth.ApiKey.changeset(%Auth.ApiKey{}) render(conn, "new_api_key.html", changeset: changeset, @@ -423,12 +422,8 @@ defmodule PlausibleWeb.AuthController do ) end - def create_api_key(conn, %{"api_key" => key_params}) do - api_key = %Auth.ApiKey{user_id: conn.assigns[:current_user].id} - key_params = Map.delete(key_params, "user_id") - changeset = Auth.ApiKey.changeset(api_key, key_params) - - case Repo.insert(changeset) do + def create_api_key(conn, %{"api_key" => %{"name" => name, "key" => key}}) do + case Auth.create_api_key(conn.assigns.current_user, name, key) do {:ok, _api_key} -> conn |> put_flash(:success, "API key created successfully") @@ -443,18 +438,17 @@ defmodule PlausibleWeb.AuthController do end def delete_api_key(conn, %{"id" => id}) do - query = - from(k in Auth.ApiKey, - where: k.id == ^id and k.user_id == ^conn.assigns[:current_user].id - ) + case Auth.delete_api_key(conn.assigns.current_user, id) do + :ok -> + conn + |> put_flash(:success, "API key revoked successfully") + |> redirect(to: "/settings#api-keys") - query - |> Repo.one!() - |> Repo.delete!() - - conn - |> put_flash(:success, "API key revoked successfully") - |> redirect(to: "/settings#api-keys") + {:error, :not_found} -> + conn + |> put_flash(:error, "Could not find API Key to delete") + |> redirect(to: "/settings#api-keys") + end end def delete_me(conn, params) do diff --git a/lib/plausible_web/controllers/site_controller.ex b/lib/plausible_web/controllers/site_controller.ex index 2e18ed74f..e69a596cd 100644 --- a/lib/plausible_web/controllers/site_controller.ex +++ b/lib/plausible_web/controllers/site_controller.ex @@ -210,7 +210,9 @@ defmodule PlausibleWeb.SiteController do end def settings_goals(conn, _params) do - site = conn.assigns[:site] |> Repo.preload(:custom_domain) + site = Repo.preload(conn.assigns[:site], [:custom_domain, :owner]) + owner = Plausible.Users.with_subscription(site.owner) + site = Map.put(site, :owner, owner) conn |> render("settings_goals.html", diff --git a/lib/plausible_web/live/goal_settings/form.ex b/lib/plausible_web/live/goal_settings/form.ex index 1fdad63c2..47abb2662 100644 --- a/lib/plausible_web/live/goal_settings/form.ex +++ b/lib/plausible_web/live/goal_settings/form.ex @@ -29,7 +29,7 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do site = Map.put(site, :owner, owner) has_access_to_revenue_goals? = - Plausible.Billing.Feature.RevenueGoals.check_availability(site) == :ok + Plausible.Billing.Feature.RevenueGoals.check_availability(owner) == :ok {:ok, assign(socket, @@ -193,7 +193,7 @@ defmodule PlausibleWeb.Live.GoalSettings.Form do + H.payment_required( + conn, + "#{Plausible.Billing.Feature.StatsAPI.display_name()} is part of the Plausible Business plan. To get access to this feature, please upgrade your account." + ) + {:error, :site_locked} -> H.payment_required( conn, @@ -61,10 +67,20 @@ defmodule PlausibleWeb.AuthorizeStatsApiPlug do is_super_admin? = Plausible.Auth.is_super_admin?(api_key.user_id) cond do - is_super_admin? -> {:ok, site} - Sites.locked?(site) -> {:error, :site_locked} - is_member? -> {:ok, site} - true -> {:error, :invalid_api_key} + is_super_admin? -> + {:ok, site} + + Sites.locked?(site) -> + {:error, :site_locked} + + Plausible.Billing.Feature.StatsAPI.check_availability(api_key.user) !== :ok -> + {:error, :upgrade_required} + + is_member? -> + {:ok, site} + + true -> + {:error, :invalid_api_key} end nil -> @@ -83,12 +99,6 @@ defmodule PlausibleWeb.AuthorizeStatsApiPlug do end end - defp find_api_key(token) do - hashed_key = ApiKey.do_hash(token) - found_key = Repo.get_by(ApiKey, key_hash: hashed_key) - if found_key, do: {:ok, found_key}, else: {:error, :invalid_api_key} - end - @one_hour 60 * 60 * 1000 defp check_api_key_rate_limit(api_key) do case Hammer.check_rate("api_request:#{api_key.id}", @one_hour, api_key.hourly_request_limit) do diff --git a/lib/plausible_web/templates/auth/user_settings.html.heex b/lib/plausible_web/templates/auth/user_settings.html.heex index d639950bb..80824a882 100644 --- a/lib/plausible_web/templates/auth/user_settings.html.heex +++ b/lib/plausible_web/templates/auth/user_settings.html.heex @@ -316,10 +316,15 @@ id="api-keys" class="max-w-2xl px-8 pt-6 pb-8 mx-auto mt-16 bg-white border-t-2 border-indigo-100 rounded rounded-t-none shadow-md dark:bg-gray-800 dark:border-indigo-500" > -

API keys

- +

API Keys

+ +
@@ -370,7 +375,13 @@
<% end %> - <%= link("+ New API key", to: "/settings/api-keys/new", class: "button mt-4") %> + <.link + :if={Plausible.Billing.Feature.StatsAPI.check_availability(@current_user) == :ok} + href={Routes.auth_path(@conn, :new_api_key)} + class="button mt-4" + > + + New API Key +
@@ -388,7 +399,7 @@

<%= if @subscription && @subscription.status == Plausible.Billing.Subscription.Status.active() do %> - + Delete my account

diff --git a/lib/plausible_web/templates/site/settings_funnels.html.heex b/lib/plausible_web/templates/site/settings_funnels.html.heex index 01981ae97..0ec25d855 100644 --- a/lib/plausible_web/templates/site/settings_funnels.html.heex +++ b/lib/plausible_web/templates/site/settings_funnels.html.heex @@ -1,6 +1,6 @@

diff --git a/lib/plausible_web/templates/site/settings_props.html.heex b/lib/plausible_web/templates/site/settings_props.html.heex index e43c26dbe..9e888b72f 100644 --- a/lib/plausible_web/templates/site/settings_props.html.heex +++ b/lib/plausible_web/templates/site/settings_props.html.heex @@ -1,6 +1,6 @@
insert(members: [me]) |> Plausible.Repo.preload(:owner) assert render_component(&Billing.extra_feature_notice/1, - site: site, + billable_user: me, current_user: me, feature_mod: Plausible.Billing.Feature.Props ) =~ @@ -20,11 +19,10 @@ defmodule PlausibleWeb.Components.BillingTest do test "extra_feature_notice/1 renders an upgrade link when user is the site owner and does not have access to the feature" do me = insert(:user, subscription: build(:subscription, paddle_plan_id: @v4_growth_plan_id)) - site = :site |> insert(members: [me]) |> Plausible.Repo.preload(:owner) rendered = render_component(&Billing.extra_feature_notice/1, - site: site, + billable_user: me, current_user: me, feature_mod: Plausible.Billing.Feature.Props ) @@ -38,19 +36,9 @@ defmodule PlausibleWeb.Components.BillingTest do me = insert(:user) owner = insert(:user, subscription: build(:subscription, paddle_plan_id: @v4_growth_plan_id)) - site = - :site - |> insert( - memberships: [ - build(:site_membership, user: owner, role: :owner), - build(:site_membership, user: me, role: :admin) - ] - ) - |> Plausible.Repo.preload(:owner) - rendered = render_component(&Billing.extra_feature_notice/1, - site: site, + billable_user: owner, current_user: me, feature_mod: Plausible.Billing.Feature.Funnels ) @@ -63,11 +51,10 @@ defmodule PlausibleWeb.Components.BillingTest do test "extra_feature_notice/1 does not render a notice when the user has access to the feature" do me = insert(:user, subscription: build(:subscription, paddle_plan_id: @v4_business_plan_id)) - site = :site |> insert(members: [me]) |> Plausible.Repo.preload(:owner) rendered = render_component(&Billing.extra_feature_notice/1, - site: site, + billable_user: me, current_user: me, feature_mod: Plausible.Billing.Feature.Funnels ) diff --git a/test/plausible_web/controllers/api/external_stats_controller/auth_test.exs b/test/plausible_web/controllers/api/external_stats_controller/auth_test.exs index 25016f6e0..9c44efe69 100644 --- a/test/plausible_web/controllers/api/external_stats_controller/auth_test.exs +++ b/test/plausible_web/controllers/api/external_stats_controller/auth_test.exs @@ -1,5 +1,6 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AuthTest do use PlausibleWeb.ConnCase + @v4_growth_plan_id "change-me-749342" setup [:create_user, :create_api_key] @@ -149,6 +150,23 @@ defmodule PlausibleWeb.Api.ExternalStatsController.AuthTest do }) end + test "returns HTTP 402 when user is on a growth plan", %{ + conn: conn, + user: user, + api_key: api_key + } do + insert(:subscription, user: user, paddle_plan_id: @v4_growth_plan_id) + site = insert(:site, members: [user]) + + conn + |> with_api_key(api_key) + |> get("/api/v1/stats/aggregate", %{"site_id" => site.domain, "metrics" => "pageviews"}) + |> assert_error( + 402, + "Stats API is part of the Plausible Business plan. To get access to this feature, please upgrade your account." + ) + end + defp with_api_key(conn, api_key) do Plug.Conn.put_req_header(conn, "authorization", "Bearer #{api_key}") end diff --git a/test/plausible_web/controllers/auth_controller_test.exs b/test/plausible_web/controllers/auth_controller_test.exs index 96cab5e65..10dcd2551 100644 --- a/test/plausible_web/controllers/auth_controller_test.exs +++ b/test/plausible_web/controllers/auth_controller_test.exs @@ -978,10 +978,8 @@ defmodule PlausibleWeb.AuthControllerTest do |> ApiKey.changeset(%{"name" => "other user's key"}) |> Repo.insert() - assert_raise Ecto.NoResultsError, fn -> - delete(conn, "/settings/api-keys/#{api_key.id}") - end - + conn = delete(conn, "/settings/api-keys/#{api_key.id}") + assert Phoenix.Flash.get(conn.assigns.flash, :error) == "Could not find API Key to delete" assert Repo.get(ApiKey, api_key.id) end end