Refactor Paddle webhook (#4938)

* Refactor Paddle webhook handler

* Make Paddle webhook work with both old and new format and add EE prefix

* Don't send EE enabled prefix from CE

lib/mix/tasks/create_free_subscription.ex

@@ -17,7 +17,8 @@ defmodule Mix.Tasks.CreateFreeSubscription do
     user = Repo.get(Plausible.Auth.User, user_id)
     {:ok, team} = Plausible.Teams.get_or_create(user)
 
-    Subscription.free(%{team_id: team.id})
+    team
+    |> Subscription.free()
     |> Repo.insert!()
 
     IO.puts("Created a free subscription for user: #{user.name}")

lib/plausible/billing/billing.ex

@@ -2,8 +2,8 @@ defmodule Plausible.Billing do
   use Plausible
   use Plausible.Repo
   require Plausible.Billing.Subscription.Status
+  alias Plausible.Auth
   alias Plausible.Billing.Subscription
-  alias Plausible.Auth.User
   alias Plausible.Teams
 
   def subscription_created(params) do
@@ -44,25 +44,15 @@ defmodule Plausible.Billing do
   end
 
   defp handle_subscription_created(params) do
-    params =
+    team = get_team!(params)
-      if present?(params["passthrough"]) do
-        format_params(params)
-      else
-        user = Repo.get_by!(User, email: params["email"])
-        {:ok, team} = Plausible.Teams.get_or_create(user)
-
-        params
-        |> Map.put("passthrough", user.id)
-        |> Map.put("team_id", team.id)
-      end
 
     subscription_params =
       params
       |> format_subscription()
       |> add_last_bill_date(params)
 
-    %Subscription{}
+    team
-    |> Subscription.changeset(subscription_params)
+    |> Subscription.create_changeset(subscription_params)
     |> Repo.insert!()
     |> after_subscription_update()
   end
@@ -86,10 +76,7 @@ defmodule Plausible.Billing do
     irrelevant? = params["old_status"] == "paused" && params["status"] == "past_due"
 
     if subscription && not irrelevant? do
-      params =
+      params = format_subscription(params)
-        params
-        |> format_params()
-        |> format_subscription()
 
       subscription
       |> Subscription.changeset(params)
@@ -145,42 +132,66 @@ defmodule Plausible.Billing do
     end
   end
 
-  defp format_params(%{"passthrough" => passthrough} = params) do
+  defp get_team!(%{"passthrough" => passthrough}) do
-    case String.split(to_string(passthrough), ";") do
+    case parse_passthrough!(passthrough) do
-      [user_id] ->
+      {:team_id, team_id} ->
-        user = Repo.get!(User, user_id)
+        Teams.get!(team_id)
-        {:ok, team} = Plausible.Teams.get_or_create(user)
-        Map.put(params, "team_id", team.id)
 
-      ["user:" <> user_id, "team:" <> team_id] ->
+      {:user_id, user_id} ->
-        params
+        user = Repo.get!(Auth.User, user_id)
-        |> Map.put("passthrough", user_id)
+        {:ok, team} = Teams.get_or_create(user)
-        |> Map.put("team_id", team_id)
+        team
     end
   end
 
-  defp format_params(params) do
+  defp get_team!(_params) do
-    params
+    raise "Missing passthrough"
+  end
+
+  defp parse_passthrough!(passthrough) do
+    {user_id, team_id} =
+      case String.split(to_string(passthrough), ";") do
+        ["ee:true", "user:" <> user_id, "team:" <> team_id] ->
+          {user_id, team_id}
+
+        ["ee:true", "user:" <> user_id] ->
+          {user_id, "0"}
+
+        # NOTE: legacy pattern, to be removed in a follow-up
+        ["user:" <> user_id, "team:" <> team_id] ->
+          {user_id, team_id}
+
+        # NOTE: legacy pattern, to be removed in a follow-up
+        [user_id] ->
+          {user_id, "0"}
+
+        _ ->
+          raise "Invalid passthrough sent via Paddle: #{inspect(passthrough)}"
+      end
+
+    case {Integer.parse(user_id), Integer.parse(team_id)} do
+      {{user_id, ""}, {0, ""}} when user_id > 0 ->
+        {:user_id, user_id}
+
+      {{_user_id, ""}, {team_id, ""}} when team_id > 0 ->
+        {:team_id, team_id}
+
+      _ ->
+        raise "Invalid passthrough sent via Paddle: #{inspect(passthrough)}"
+    end
   end
 
   defp format_subscription(params) do
-    subscription_params = %{
+    %{
       paddle_subscription_id: params["subscription_id"],
       paddle_plan_id: params["subscription_plan_id"],
       cancel_url: params["cancel_url"],
       update_url: params["update_url"],
-      user_id: params["passthrough"],
       status: params["status"],
       next_bill_date: params["next_bill_date"],
       next_bill_amount: params["unit_price"] || params["new_unit_price"],
       currency_code: params["currency"]
     }
-
-    if team_id = params["team_id"] do
-      Map.put(subscription_params, :team_id, team_id)
-    else
-      subscription_params
-    end
   end
 
   defp add_last_bill_date(subscription_params, paddle_params) do
@@ -193,10 +204,6 @@ defmodule Plausible.Billing do
     end
   end
 
-  defp present?(""), do: false
-  defp present?(nil), do: false
-  defp present?(_), do: true
-
   @spec format_price(Money.t()) :: String.t()
   def format_price(money) do
     Money.to_string!(money, fractional_digits: 2, no_fraction_if_integer: true)

lib/plausible/billing/subscription.ex

@@ -19,7 +19,7 @@ defmodule Plausible.Billing.Subscription do
     :currency_code
   ]
 
-  @optional_fields [:last_bill_date, :team_id]
+  @optional_fields [:last_bill_date]
 
   schema "subscriptions" do
     field :paddle_subscription_id, :string
@@ -37,14 +37,20 @@ defmodule Plausible.Billing.Subscription do
     timestamps()
   end
 
-  def changeset(model, attrs \\ %{}) do
+  def create_changeset(team, attrs \\ %{}) do
-    model
+    %__MODULE__{}
+    |> changeset(attrs)
+    |> put_assoc(:team, team)
+  end
+
+  def changeset(subscription, attrs \\ %{}) do
+    subscription
     |> cast(attrs, @required_fields ++ @optional_fields)
     |> validate_required(@required_fields)
     |> unique_constraint(:paddle_subscription_id)
   end
 
-  def free(attrs \\ %{}) do
+  def free(team, attrs \\ %{}) do
     %__MODULE__{
       paddle_plan_id: "free_10k",
       status: Subscription.Status.active(),
@@ -52,7 +58,7 @@ defmodule Plausible.Billing.Subscription do
       currency_code: "EUR"
     }
     |> cast(attrs, @required_fields ++ @optional_fields)
-    |> validate_required([:team_id])
+    |> put_assoc(:team, team)
     |> unique_constraint(:paddle_subscription_id)
   end
 end

lib/plausible/teams.ex

@@ -12,6 +12,11 @@ defmodule Plausible.Teams do
 
   @accept_traffic_until_free ~D[2135-01-01]
 
+  @spec get!(pos_integer()) :: Teams.Team.t()
+  def get!(team_id) do
+    Repo.get!(Teams.Team, team_id)
+  end
+
   @spec get_owner(Teams.Team.t()) ::
           {:ok, Plausible.Auth.User.t()} | {:error, :no_owner | :multiple_owners}
   def get_owner(team) do

lib/plausible_web/components/billing/billing.ex

@@ -2,6 +2,7 @@ defmodule PlausibleWeb.Components.Billing do
   @moduledoc false
 
   use PlausibleWeb, :component
+  use Plausible
 
   require Plausible.Billing.Subscription.Status
   alias Plausible.Billing.{Subscription, Subscriptions}
@@ -237,9 +238,9 @@ defmodule PlausibleWeb.Components.Billing do
 
     passthrough =
       if assigns.team do
-        "user:#{assigns.user.id};team:#{assigns.team.id}"
+        "ee:#{ee?()};user:#{assigns.user.id};team:#{assigns.team.id}"
       else
-        assigns.user.id
+        "ee:#{ee?()};user:#{assigns.user.id}"
       end
 
     assigns =

test/plausible/billing/billing_test.exs

@@ -131,10 +131,72 @@ defmodule Plausible.BillingTest do
   }
 
   describe "subscription_created" do
-    test "creates a subscription" do
+    test "fails on callback without passthrough" do
-      user = new_user()
+      _user = new_user()
 
-      %{@subscription_created_params | "passthrough" => user.id}
+      assert_raise RuntimeError, ~r/Missing passthrough/, fn ->
+        @subscription_created_params
+        |> Map.delete("passthrough")
+        |> Billing.subscription_created()
+      end
+    end
+
+    test "fails on callback without valid passthrough" do
+      _user = new_user()
+
+      assert_raise RuntimeError, ~r/Invalid passthrough sent via Paddle/, fn ->
+        %{@subscription_created_params | "passthrough" => "invalid"}
+        |> Billing.subscription_created()
+      end
+
+      assert_raise RuntimeError, ~r/Invalid passthrough sent via Paddle/, fn ->
+        %{@subscription_created_params | "passthrough" => "ee:true;user:invalid"}
+        |> Billing.subscription_created()
+      end
+
+      assert_raise RuntimeError, ~r/Invalid passthrough sent via Paddle/, fn ->
+        %{@subscription_created_params | "passthrough" => "ee:true;user:123;team:invalid"}
+        |> Billing.subscription_created()
+      end
+
+      assert_raise RuntimeError, ~r/Invalid passthrough sent via Paddle/, fn ->
+        %{
+          @subscription_created_params
+          | "passthrough" => "ee:true;user:123;team:456;some:invalid"
+        }
+        |> Billing.subscription_created()
+      end
+    end
+
+    test "fails on callback with non-existent user" do
+      user = new_user()
+      Repo.delete!(user)
+
+      assert_raise Ecto.NoResultsError, fn ->
+        %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id}"}
+        |> Billing.subscription_created()
+      end
+    end
+
+    test "fails on callback with non-existent team" do
+      user = new_user()
+      {:ok, team} = Plausible.Teams.get_or_create(user)
+      Repo.delete!(team)
+
+      assert_raise Ecto.NoResultsError, fn ->
+        %{
+          @subscription_created_params
+          | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
+        }
+        |> Billing.subscription_created()
+      end
+    end
+
+    test "creates a subscription with teams passthrough" do
+      user = new_user()
+      {:ok, team} = Plausible.Teams.get_or_create(user)
+
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"}
       |> Billing.subscription_created()
 
       subscription =
@@ -147,8 +209,23 @@ defmodule Plausible.BillingTest do
       assert subscription.currency_code == "EUR"
     end
 
-    @tag :teams
+    test "supports user without a team case" do
-    test "creates a subscription with teams passthrough" do
+      user = new_user()
+
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id}"}
+      |> Billing.subscription_created()
+
+      subscription =
+        user |> team_of() |> Plausible.Teams.with_subscription() |> Map.fetch!(:subscription)
+
+      assert subscription.paddle_subscription_id == @subscription_id
+      assert subscription.next_bill_date == ~D[2019-06-01]
+      assert subscription.last_bill_date == ~D[2019-05-01]
+      assert subscription.next_bill_amount == "6.00"
+      assert subscription.currency_code == "EUR"
+    end
+
+    test "supports old format without prefix" do
       user = new_user()
       {:ok, team} = Plausible.Teams.get_or_create(user)
 
@@ -158,25 +235,21 @@ defmodule Plausible.BillingTest do
       assert user |> team_of() |> Plausible.Teams.with_subscription() |> Map.fetch!(:subscription)
     end
 
-    test "create with email address" do
+    test "supports old format without prefix for user without a team" do
       user = new_user()
 
-      %{@subscription_created_params | "email" => user.email}
+      %{@subscription_created_params | "passthrough" => user.id}
       |> Billing.subscription_created()
 
-      subscription = subscription_of(user)
+      assert user |> team_of() |> Plausible.Teams.with_subscription() |> Map.fetch!(:subscription)
-
-      assert subscription.paddle_subscription_id == @subscription_id
-      assert subscription.next_bill_date == ~D[2019-06-01]
-      assert subscription.last_bill_date == ~D[2019-05-01]
-      assert subscription.next_bill_amount == "6.00"
     end
 
     test "unlocks sites if user has any locked sites" do
       user = new_user()
       site = new_site(owner: user, locked: true)
+      team = team_of(user)
 
-      %{@subscription_created_params | "passthrough" => user.id}
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"}
       |> Billing.subscription_created()
 
       refute Repo.reload!(site).locked
@@ -185,8 +258,10 @@ defmodule Plausible.BillingTest do
     @tag :ee_only
     test "updates accept_traffic_until" do
       user = new_user()
+      new_site(owner: user)
+      team = team_of(user)
 
-      %{@subscription_created_params | "passthrough" => user.id}
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"}
       |> Billing.subscription_created()
 
       next_bill = Date.from_iso8601!(@subscription_created_params["next_bill_date"])
@@ -197,8 +272,9 @@ defmodule Plausible.BillingTest do
 
     test "sets user.allow_next_upgrade_override field to false" do
       user = new_user(team: [allow_next_upgrade_override: true])
+      team = team_of(user)
 
-      %{@subscription_created_params | "passthrough" => user.id}
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"}
       |> Billing.subscription_created()
 
       refute Repo.reload!(team_of(user)).allow_next_upgrade_override
@@ -212,9 +288,11 @@ defmodule Plausible.BillingTest do
         paddle_plan_id: @plan_id_10k
       )
 
+      team = team_of(user)
+
       api_key = insert(:api_key, user: user, hourly_request_limit: 1)
 
-      %{@subscription_created_params | "passthrough" => user.id}
+      %{@subscription_created_params | "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"}
       |> Billing.subscription_created()
 
       assert Repo.reload!(api_key).hourly_request_limit == 10_000
@@ -225,11 +303,12 @@ defmodule Plausible.BillingTest do
     test "updates an existing subscription" do
       user = new_user()
       subscribe_to_growth_plan(user)
+      team = team_of(user)
 
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_of(user).paddle_subscription_id,
-        "passthrough" => user.id
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
       })
       |> Billing.subscription_updated()
 
@@ -250,7 +329,7 @@ defmodule Plausible.BillingTest do
       |> Map.merge(%{
         "next_bill_date" => "2021-01-01",
         "subscription_id" => subscription_id,
-        "passthrough" => "user:#{user.id};team:#{team.id}"
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
       })
       |> Billing.subscription_updated()
 
@@ -263,11 +342,12 @@ defmodule Plausible.BillingTest do
     test "status update from 'paused' to 'past_due' is ignored" do
       user = new_user()
       subscribe_to_growth_plan(user, status: Subscription.Status.paused())
+      team = team_of(user)
 
       %{@subscription_updated_params | "old_status" => "paused", "status" => "past_due"}
       |> Map.merge(%{
         "subscription_id" => subscription_of(user).paddle_subscription_id,
-        "passthrough" => user.id
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
       })
       |> Billing.subscription_updated()
 
@@ -278,11 +358,12 @@ defmodule Plausible.BillingTest do
       user = new_user()
       subscribe_to_growth_plan(user, status: Subscription.Status.past_due())
       site = new_site(locked: true, owner: user)
+      team = team_of(user)
 
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_of(user).paddle_subscription_id,
-        "passthrough" => user.id,
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}",
         "old_status" => "past_due"
       })
       |> Billing.subscription_updated()
@@ -293,11 +374,12 @@ defmodule Plausible.BillingTest do
     @tag :ee_only
     test "updates accept_traffic_until" do
       user = new_user() |> subscribe_to_growth_plan()
+      team = team_of(user)
 
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_of(user).paddle_subscription_id,
-        "passthrough" => user.id
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
       })
       |> Billing.subscription_updated()
 
@@ -310,6 +392,7 @@ defmodule Plausible.BillingTest do
     test "sets user.allow_next_upgrade_override field to false" do
       user = new_user(team: [allow_next_upgrade_override: true])
       subscribe_to_growth_plan(user)
+      team = team_of(user)
 
       assert Repo.reload!(team_of(user)).allow_next_upgrade_override
 
@@ -318,7 +401,7 @@ defmodule Plausible.BillingTest do
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_id,
-        "passthrough" => user.id
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
       })
       |> Billing.subscription_updated()
 
@@ -333,12 +416,14 @@ defmodule Plausible.BillingTest do
           hourly_api_request_limit: 10_000
         )
 
+      team = team_of(user)
+
       api_key = insert(:api_key, user: user, hourly_request_limit: 1)
 
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_of(user).paddle_subscription_id,
-        "passthrough" => user.id,
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}",
         "subscription_plan_id" => "new-plan-id"
       })
       |> Billing.subscription_updated()
@@ -352,6 +437,8 @@ defmodule Plausible.BillingTest do
 
       subscribe_to_growth_plan(user)
 
+      team = team_of(user)
+
       site = new_site(locked: true, owner: user)
 
       subscription_id = subscription_of(user).paddle_subscription_id
@@ -359,7 +446,7 @@ defmodule Plausible.BillingTest do
       @subscription_updated_params
       |> Map.merge(%{
         "subscription_id" => subscription_id,
-        "passthrough" => user.id,
+        "passthrough" => "ee:true;user:#{user.id};team:#{team.id}",
         "subscription_plan_id" => @plan_id_100k
       })
       |> Billing.subscription_updated()
@@ -370,12 +457,14 @@ defmodule Plausible.BillingTest do
 
     test "ignores if subscription cannot be found" do
       user = insert(:user)
+      _site = new_site(owner: user)
+      team = team_of(user)
 
       res =
         @subscription_updated_params
         |> Map.merge(%{
           "subscription_id" => "666",
-          "passthrough" => user.id
+          "passthrough" => "ee:true;user:#{user.id};team:#{team.id}"
         })
         |> Billing.subscription_updated()
 

test/plausible_web/controllers/api/paddle_controller_test.exs

@@ -32,8 +32,8 @@ defmodule PlausibleWeb.Api.PaddleControllerTest do
   describe "webhook verification" do
     test "is verified when signature is correct", %{conn: conn} do
       insert(:user, id: 235)
-      conn = post(conn, Routes.paddle_path(conn, :webhook), @webhook_body)
 
+      conn = post(conn, Routes.paddle_path(conn, :webhook), @webhook_body)
       assert conn.status == 200
     end
 

test/plausible_web/controllers/billing_controller_test.exs

@@ -162,7 +162,7 @@ defmodule PlausibleWeb.BillingControllerTest do
       assert %{
                "disableLogout" => true,
                "email" => user.email,
-               "passthrough" => "user:#{user.id};team:#{team.id}",
+               "passthrough" => "ee:#{Plausible.ee?()};user:#{user.id};team:#{team.id}",
                "product" => @configured_enterprise_plan_paddle_plan_id,
                "success" => Routes.billing_path(PlausibleWeb.Endpoint, :upgrade_success),
                "theme" => "none"
@@ -337,7 +337,7 @@ defmodule PlausibleWeb.BillingControllerTest do
       assert %{
                "disableLogout" => true,
                "email" => user.email,
-               "passthrough" => "user:#{user.id};team:#{team.id}",
+               "passthrough" => "ee:#{Plausible.ee?()};user:#{user.id};team:#{team.id}",
                "product" => @configured_enterprise_plan_paddle_plan_id,
                "success" => Routes.billing_path(PlausibleWeb.Endpoint, :upgrade_success),
                "theme" => "none"

test/plausible_web/controllers/settings_controller_test.exs

@@ -477,7 +477,9 @@ defmodule PlausibleWeb.SettingsControllerTest do
       # for a free_10k subscription (without a `last_bill_date`)
       Repo.delete!(subscription)
 
-      Plausible.Billing.Subscription.free(%{team_id: team_of(user).id})
+      user
+      |> team_of()
+      |> Plausible.Billing.Subscription.free()
       |> Repo.insert!()
 
       conn
@@ -533,7 +535,9 @@ defmodule PlausibleWeb.SettingsControllerTest do
     test "does not show invoice section for a free subscription", %{conn: conn, user: user} do
       new_site(owner: user)
 
-      Plausible.Billing.Subscription.free(%{team_id: team_of(user).id, currency_code: "EUR"})
+      user
+      |> team_of()
+      |> Plausible.Billing.Subscription.free(%{currency_code: "EUR"})
       |> Repo.insert!()
 
       html =

test/plausible_web/live/choose_plan_test.exs

@@ -202,7 +202,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do
       assert %{
                "disableLogout" => true,
                "email" => user.email,
-               "passthrough" => "user:#{user.id};team:#{team.id}",
+               "passthrough" => "ee:true;user:#{user.id};team:#{team.id}",
                "product" => @v4_growth_200k_yearly_plan_id,
                "success" => Routes.billing_path(PlausibleWeb.Endpoint, :upgrade_success),
                "theme" => "none"
@@ -1034,7 +1034,7 @@ defmodule PlausibleWeb.Live.ChoosePlanTest do
       assert %{
                "disableLogout" => true,
                "email" => user.email,
-               "passthrough" => "user:#{user.id};team:#{team.id}",
+               "passthrough" => "ee:true;user:#{user.id};team:#{team.id}",
                "product" => @v4_growth_200k_yearly_plan_id,
                "success" => Routes.billing_path(PlausibleWeb.Endpoint, :upgrade_success),
                "theme" => "none"