plausible/analytics
1
1
Fork 0
mirror of https://github.com/plausible/analytics.git synced 2024-09-11 18:07:33 +03:00
Code Issues Packages Projects Releases Wiki Activity

Improve ownership transfers (#3326)

Browse Source 
* Move inline functions to module

* Extend invite/4 for ownership transfers

* Verify inviter has sufficient permissions

* Ensure ownership transfers don't count as team member

This commit changes the team member usage query to exclude ownership
transfer invitations. Previously, when an ownership transfer was
pending, the team member usage was incremented.

* Draw attention to payment notice when transferring ownership

* Remove duplicate mail sending from membership_controller

---------

Co-authored-by: Uku Taht <uku.taht@gmail.com>
This commit is contained in:
Vini Brasil 2023-09-12 04:06:24 -03:00 committed by GitHub
parent 45ed56b4d5
commit 9b029c1558
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 167 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
lib/plausible/billing/quota.ex
View File

@ -121,6 +121,7 @@ defmodule Plausible.Billing.Quota do
from i in Plausible.Auth.Invitation,
inner_join: os in subquery(owned_sites_query),
on: i.site_id == os.site_id,
where: i.role != :owner,
select: %{email: i.email},
union: ^team_members_query

106
lib/plausible/sites.ex
View File

@ -1,7 +1,6 @@
defmodule Plausible.Sites do
use Plausible.Repo
alias Plausible.Site
alias Plausible.Site.SharedLink
alias Plausible.{Repo, Site, Site.SharedLink, Auth.User, Billing.Quota}
alias PlausibleWeb.Email
import Ecto.Query
def get_by_domain(domain) do
@ -17,12 +16,10 @@ defmodule Plausible.Sites do
Ecto.Multi.new()
|> Ecto.Multi.run(:limit, fn _, _ ->
limit = Plausible.Billing.Quota.site_limit(user)
usage = Plausible.Billing.Quota.site_usage(user)
limit = Quota.site_limit(user)
usage = Quota.site_usage(user)
if Plausible.Billing.Quota.within_limit?(usage, limit),
do: {:ok, usage},
else: {:error, limit}
if Quota.within_limit?(usage, limit), do: {:ok, usage}, else: {:error, limit}
end)
|> Ecto.Multi.insert(:site, site_changeset)
|> Ecto.Multi.run(:site_membership, fn repo, %{site: site} ->
@ -54,6 +51,18 @@ defmodule Plausible.Sites do
| {:error, Ecto.Changeset.t()}
| {:error, :already_a_member}
| {:error, {:over_limit, non_neg_integer()}}
| {:error, :forbidden}
@doc """
Invites a new team member to the given site. Returns a
%Plausible.Auth.Invitation{} struct and sends the invitee an email to accept
this invitation.
The inviter must have enough permissions to invite the new team member,
otherwise this function returns `{:error, :forbidden}`.
If the new team member role is `:owner`, this function handles the invitation
as an ownership transfer and requires the inviter to be the owner of the site.
"""
def invite(site, inviter, invitee_email, role) do
Repo.transaction(fn ->
do_invite(site, inviter, invitee_email, role)
@ -61,45 +70,70 @@ defmodule Plausible.Sites do
end
defp do_invite(site, inviter, invitee_email, role) do
send_invitation_email = fn invitation, invitee ->
invitation = Repo.preload(invitation, [:site, :inviter])
email =
if invitee,
do: PlausibleWeb.Email.existing_user_invitation(invitation),
else: PlausibleWeb.Email.new_user_invitation(invitation)
Plausible.Mailer.send(email)
end
ensure_new_membership = fn site, invitee ->
if invitee && is_member?(invitee.id, site), do: {:error, :already_a_member}, else: :ok
end
check_limit = fn site ->
owner = owner_for(site)
usage = Plausible.Billing.Quota.team_member_usage(owner)
limit = Plausible.Billing.Quota.team_member_limit(owner)
if Plausible.Billing.Quota.within_limit?(usage, limit),
do: :ok,
else: {:error, {:over_limit, limit}}
end
attrs = %{email: invitee_email, role: role, site_id: site.id, inviter_id: inviter.id}
with :ok <- check_limit.(site),
with :ok <- check_invitation_permissions(site, inviter, role),
:ok <- check_team_member_limit(site, role),
invitee <- Plausible.Auth.find_user_by(email: invitee_email),
:ok <- ensure_new_membership.(site, invitee),
:ok <- ensure_new_membership(site, invitee, role),
%Ecto.Changeset{} = changeset <- Plausible.Auth.Invitation.new(attrs),
{:ok, invitation} <- Repo.insert(changeset) do
send_invitation_email.(invitation, invitee)
send_invitation_email(invitation, invitee)
invitation
else
{:error, cause} -> Repo.rollback(cause)
end
end
defp check_invitation_permissions(site, inviter, requested_role) do
required_roles = if requested_role == :owner, do: [:owner], else: [:admin, :owner]
membership_query =
from m in Plausible.Site.Membership,
where: m.user_id == ^inviter.id and m.site_id == ^site.id and m.role in ^required_roles
if Repo.exists?(membership_query), do: :ok, else: {:error, :forbidden}
end
defp send_invitation_email(invitation, invitee) do
invitation = Repo.preload(invitation, [:site, :inviter])
email =
case {invitee, invitation.role} do
{invitee, :owner} -> Email.ownership_transfer_request(invitation, invitee)
{nil, _role} -> Email.new_user_invitation(invitation)
{%User{}, _role} -> Email.existing_user_invitation(invitation)
end
Plausible.Mailer.send(email)
end
defp ensure_new_membership(_site, _invitee, :owner) do
:ok
end
defp ensure_new_membership(site, invitee, _role) do
if invitee && is_member?(invitee.id, site) do
{:error, :already_a_member}
else
:ok
end
end
defp check_team_member_limit(_site, :owner) do
:ok
end
defp check_team_member_limit(site, _role) do
site_owner = owner_for(site)
limit = Quota.team_member_limit(site_owner)
usage = Quota.team_member_usage(site_owner)
if Quota.within_limit?(usage, limit),
do: :ok,
else: {:error, {:over_limit, limit}}
end
@spec stats_start_date(Plausible.Site.t()) :: Date.t() | nil
@doc """
Returns the date of the first event of the given site, or `nil` if the site

52
lib/plausible_web/controllers/site/membership_controller.ex
View File

@ -14,7 +14,6 @@ defmodule PlausibleWeb.Site.MembershipController do
use Plausible.Repo
alias Plausible.Sites
alias Plausible.Site.Membership
alias Plausible.Auth.Invitation
@only_owner_is_allowed_to [:transfer_ownership_form, :transfer_ownership]
@ -98,43 +97,28 @@ defmodule PlausibleWeb.Site.MembershipController do
def transfer_ownership(conn, %{"email" => email}) do
site_domain = conn.assigns[:site].domain
site = Sites.get_for_user!(conn.assigns[:current_user].id, site_domain)
user = Plausible.Auth.find_user_by(email: email)
invite_result =
Invitation.new(%{
email: email,
role: :owner,
site_id: site.id,
inviter_id: conn.assigns[:current_user].id
})
|> Repo.insert()
case Sites.invite(site, conn.assigns.current_user, email, :owner) do
{:ok, _invitation} ->
conn
|> put_flash(:success, "Site transfer request has been sent to #{email}")
|> redirect(to: Routes.site_path(conn, :settings_people, site.domain))
conn =
case invite_result do
{:ok, invitation} ->
invitation
|> Repo.preload([:site, :inviter])
|> PlausibleWeb.Email.ownership_transfer_request(user)
|> Plausible.Mailer.send()
{:error, changeset} ->
errors = Plausible.ChangesetHelpers.traverse_errors(changeset)
put_flash(conn, :success, "Site transfer request has been sent to #{email}")
message =
case errors do
%{invitation: ["already sent" | _]} -> "Invitation has already been sent"
_other -> "Site transfer request to #{email} has failed"
end
{:error, changeset} ->
errors = Plausible.ChangesetHelpers.traverse_errors(changeset)
message =
case errors do
%{invitation: ["already sent" | _]} -> "Invitation has already been sent"
_other -> "Site transfer request to #{email} has failed"
end
conn
|> put_flash(:ttl, :timer.seconds(5))
|> put_flash(:error_title, "Transfer error")
|> put_flash(:error, message)
end
redirect(conn, to: Routes.site_path(conn, :settings_people, site.domain))
conn
|> put_flash(:ttl, :timer.seconds(5))
|> put_flash(:error_title, "Transfer error")
|> put_flash(:error, message)
|> redirect(to: Routes.site_path(conn, :settings_people, site.domain))
end
end
@doc """

2
lib/plausible_web/templates/site/index.html.eex
View File

@ -166,7 +166,7 @@
<% end %>
<%= if Plausible.Billing.on_trial?(@current_user) do %>
<br/><br />
Your 30-day free trial will end immediately and you will have to enter your card details to keep using Plausible.
NB: Your 30-day free trial will end immediately and <strong>you will have to enter your card details</strong> to keep using Plausible.
<% end %>
</p>
</div>

9
test/plausible/billing/quota_test.exs
View File

@ -282,6 +282,15 @@ defmodule Plausible.Billing.QuotaTest do
assert Quota.team_member_usage(me) == 3
end
test "does not count ownership transfer as a team member" do
me = insert(:user)
site_i_own = insert(:site, memberships: [build(:site_membership, user: me, role: :owner)])
insert(:invitation, site: site_i_own, inviter: me, role: :owner)
assert Quota.team_member_usage(me) == 0
end
test "returns zero when user does not have any site" do
me = insert(:user)
assert Quota.team_member_usage(me) == 0

71
test/plausible/site/sites_test.exs
View File

@ -81,11 +81,10 @@ defmodule Plausible.SitesTest do
test "returns validation errors" do
inviter = insert(:user)
invitee = insert(:user)
site = insert(:site, memberships: [build(:site_membership, user: inviter, role: :owner)])
assert {:error, changeset} = Sites.invite(site, inviter, invitee.email, :invalid_role)
assert {"is invalid", _} = changeset.errors[:role]
assert {:error, changeset} = Sites.invite(site, inviter, "", :viewer)
assert {"can't be blank", _} = changeset.errors[:email]
end
test "returns error when user is already a member" do
@ -142,6 +141,72 @@ defmodule Plausible.SitesTest do
site = insert(:site, memberships: memberships)
assert {:error, {:over_limit, 5}} = Sites.invite(site, inviter, invitee.email, :viewer)
end
test "sends ownership transfer email when invitee role is owner" do
inviter = insert(:user)
site = insert(:site, memberships: [build(:site_membership, user: inviter, role: :owner)])
assert {:ok, %Plausible.Auth.Invitation{}} =
Sites.invite(site, inviter, "vini@plausible.test", :owner)
assert_email_delivered_with(
to: [nil: "vini@plausible.test"],
subject: "[Plausible Analytics] Request to transfer ownership of #{site.domain}"
)
end
test "only allows owners to transfer ownership" do
inviter = insert(:user)
site =
insert(:site,
memberships: [
build(:site_membership, user: build(:user), role: :owner),
build(:site_membership, user: inviter, role: :admin)
]
)
assert {:error, :forbidden} = Sites.invite(site, inviter, "vini@plausible.test", :owner)
end
test "does not check for limits when transferring ownership" do
inviter = insert(:user)
memberships =
[build(:site_membership, user: inviter, role: :owner)] ++ build_list(5, :site_membership)
site = insert(:site, memberships: memberships)
assert {:ok, _invitation} = Sites.invite(site, inviter, "newowner@plausible.test", :owner)
end
test "does not allow viewers to invite users" do
inviter = insert(:user)
site =
insert(:site,
memberships: [
build(:site_membership, user: build(:user), role: :owner),
build(:site_membership, user: inviter, role: :viewer)
]
)
assert {:error, :forbidden} = Sites.invite(site, inviter, "vini@plausible.test", :viewer)
end
test "allows admins to invite other admins" do
inviter = insert(:user)
site =
insert(:site,
memberships: [
build(:site_membership, user: build(:user), role: :owner),
build(:site_membership, user: inviter, role: :admin)
]
)
assert {:ok, %Plausible.Auth.Invitation{}} =
Sites.invite(site, inviter, "vini@plausible.test", :admin)
end
end
describe "get_for_user/2" do