mirror of
https://github.com/plausible/analytics.git
synced 2024-12-23 01:22:15 +03:00
Allow admins to initiate ownership transfer from the CRM (#3340)
* Allow admins to initiate ownership transfer from the CRM * Add stronger assertion for bulk invite action * Fix compile warning * Move bulk transfer logic to Sites module * Replaces unused variables with _ * Add typespec for `bulk_transfer_ownership` * Extract from keywordlist options instead of matching * Fix and extend bulk transfer tests --------- Co-authored-by: Adrian Gruntkowski <adrian.gruntkowski@gmail.com>
This commit is contained in:
parent
27a11fc5b7
commit
7cb2c6bfa3
@ -112,8 +112,9 @@ defmodule Plausible.Auth do
|
|||||||
end
|
end
|
||||||
|
|
||||||
def is_super_admin?(nil), do: false
|
def is_super_admin?(nil), do: false
|
||||||
|
def is_super_admin?(%Plausible.Auth.User{id: id}), do: is_super_admin?(id)
|
||||||
|
|
||||||
def is_super_admin?(user_id) do
|
def is_super_admin?(user_id) when is_integer(user_id) do
|
||||||
user_id in Application.get_env(:plausible, :super_admin_user_ids)
|
user_id in Application.get_env(:plausible, :super_admin_user_ids)
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@ -53,6 +53,38 @@ defmodule Plausible.SiteAdmin do
|
|||||||
]
|
]
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def list_actions(_conn) do
|
||||||
|
[
|
||||||
|
transfer_ownership: %{
|
||||||
|
name: "Transfer ownership",
|
||||||
|
inputs: [
|
||||||
|
%{name: "email", title: "New Owner Email", default: nil}
|
||||||
|
],
|
||||||
|
action: fn conn, sites, params -> transfer_ownership(conn, sites, params) end
|
||||||
|
}
|
||||||
|
]
|
||||||
|
end
|
||||||
|
|
||||||
|
defp transfer_ownership(_conn, [], _params) do
|
||||||
|
{:error, "Please select at least one site from the list"}
|
||||||
|
end
|
||||||
|
|
||||||
|
defp transfer_ownership(conn, sites, %{"email" => email}) do
|
||||||
|
new_owner = Plausible.Auth.find_user_by(email: email)
|
||||||
|
inviter = conn.assigns[:current_user]
|
||||||
|
|
||||||
|
if new_owner do
|
||||||
|
{:ok, _} =
|
||||||
|
Plausible.Sites.bulk_transfer_ownership(sites, inviter, new_owner.email,
|
||||||
|
check_permissions: false
|
||||||
|
)
|
||||||
|
|
||||||
|
:ok
|
||||||
|
else
|
||||||
|
{:error, "User could not be found"}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
defp format_date(date) do
|
defp format_date(date) do
|
||||||
Timex.format!(date, "{Mshort} {D}, {YYYY}")
|
Timex.format!(date, "{Mshort} {D}, {YYYY}")
|
||||||
end
|
end
|
||||||
|
@ -3,6 +3,12 @@ defmodule Plausible.Sites do
|
|||||||
alias PlausibleWeb.Email
|
alias PlausibleWeb.Email
|
||||||
import Ecto.Query
|
import Ecto.Query
|
||||||
|
|
||||||
|
@type invite_error() ::
|
||||||
|
Ecto.Changeset.t()
|
||||||
|
| :already_a_member
|
||||||
|
| {:over_limit, non_neg_integer()}
|
||||||
|
| :forbidden
|
||||||
|
|
||||||
def get_by_domain(domain) do
|
def get_by_domain(domain) do
|
||||||
Repo.get_by(Site, domain: domain)
|
Repo.get_by(Site, domain: domain)
|
||||||
end
|
end
|
||||||
@ -46,12 +52,22 @@ defmodule Plausible.Sites do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
@spec bulk_transfer_ownership(
|
||||||
|
[Site.t()],
|
||||||
|
Plausible.Auth.User.t(),
|
||||||
|
String.t(),
|
||||||
|
Keyword.t()
|
||||||
|
) :: {:ok, [Plausible.Auth.Invitation.t()]} | {:error, invite_error()}
|
||||||
|
def bulk_transfer_ownership(sites, inviter, invitee_email, opts \\ []) do
|
||||||
|
Repo.transaction(fn ->
|
||||||
|
for site <- sites do
|
||||||
|
do_invite(site, inviter, invitee_email, :owner, opts)
|
||||||
|
end
|
||||||
|
end)
|
||||||
|
end
|
||||||
|
|
||||||
@spec invite(Site.t(), Plausible.Auth.User.t(), String.t(), atom()) ::
|
@spec invite(Site.t(), Plausible.Auth.User.t(), String.t(), atom()) ::
|
||||||
{:ok, Plausible.Auth.Invitation.t()}
|
{:ok, Plausible.Auth.Invitation.t()} | {:error, invite_error()}
|
||||||
| {:error, Ecto.Changeset.t()}
|
|
||||||
| {:error, :already_a_member}
|
|
||||||
| {:error, {:over_limit, non_neg_integer()}}
|
|
||||||
| {:error, :forbidden}
|
|
||||||
@doc """
|
@doc """
|
||||||
Invites a new team member to the given site. Returns a
|
Invites a new team member to the given site. Returns a
|
||||||
%Plausible.Auth.Invitation{} struct and sends the invitee an email to accept
|
%Plausible.Auth.Invitation{} struct and sends the invitee an email to accept
|
||||||
@ -69,10 +85,10 @@ defmodule Plausible.Sites do
|
|||||||
end)
|
end)
|
||||||
end
|
end
|
||||||
|
|
||||||
defp do_invite(site, inviter, invitee_email, role) do
|
defp do_invite(site, inviter, invitee_email, role, opts \\ []) do
|
||||||
attrs = %{email: invitee_email, role: role, site_id: site.id, inviter_id: inviter.id}
|
attrs = %{email: invitee_email, role: role, site_id: site.id, inviter_id: inviter.id}
|
||||||
|
|
||||||
with :ok <- check_invitation_permissions(site, inviter, role),
|
with :ok <- check_invitation_permissions(site, inviter, role, opts),
|
||||||
:ok <- check_team_member_limit(site, role),
|
:ok <- check_team_member_limit(site, role),
|
||||||
invitee <- Plausible.Auth.find_user_by(email: invitee_email),
|
invitee <- Plausible.Auth.find_user_by(email: invitee_email),
|
||||||
:ok <- ensure_new_membership(site, invitee, role),
|
:ok <- ensure_new_membership(site, invitee, role),
|
||||||
@ -85,14 +101,21 @@ defmodule Plausible.Sites do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp check_invitation_permissions(site, inviter, requested_role) do
|
defp check_invitation_permissions(site, inviter, requested_role, opts) do
|
||||||
required_roles = if requested_role == :owner, do: [:owner], else: [:admin, :owner]
|
check_permissions? = Keyword.get(opts, :check_permissions, true)
|
||||||
|
|
||||||
membership_query =
|
if check_permissions? do
|
||||||
from m in Plausible.Site.Membership,
|
required_roles = if requested_role == :owner, do: [:owner], else: [:admin, :owner]
|
||||||
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}
|
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}
|
||||||
|
else
|
||||||
|
:ok
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
defp send_invitation_email(invitation, invitee) do
|
defp send_invitation_email(invitation, invitee) do
|
||||||
|
55
test/plausible/site/admin_test.exs
Normal file
55
test/plausible/site/admin_test.exs
Normal file
@ -0,0 +1,55 @@
|
|||||||
|
defmodule Plausible.Site.AdminTest do
|
||||||
|
use Plausible.DataCase, async: true
|
||||||
|
use Bamboo.Test
|
||||||
|
|
||||||
|
setup do
|
||||||
|
admin_user = insert(:user)
|
||||||
|
conn = %Plug.Conn{assigns: %{current_user: admin_user}}
|
||||||
|
action = Plausible.SiteAdmin.list_actions(conn)[:transfer_ownership][:action]
|
||||||
|
|
||||||
|
{:ok,
|
||||||
|
%{
|
||||||
|
action: action,
|
||||||
|
conn: conn
|
||||||
|
}}
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "bulk transferring site ownership" do
|
||||||
|
test "user has to select at least one site", %{conn: conn, action: action} do
|
||||||
|
assert action.(conn, [], %{}) == {:error, "Please select at least one site from the list"}
|
||||||
|
end
|
||||||
|
|
||||||
|
test "new owner must be an existing user", %{conn: conn, action: action} do
|
||||||
|
site = insert(:site)
|
||||||
|
|
||||||
|
assert action.(conn, [site], %{"email" => "random@email.com"}) ==
|
||||||
|
{:error, "User could not be found"}
|
||||||
|
end
|
||||||
|
|
||||||
|
test "initiates ownership transfer for multiple sites in one action", %{
|
||||||
|
conn: conn,
|
||||||
|
action: action
|
||||||
|
} do
|
||||||
|
current_owner = insert(:user)
|
||||||
|
new_owner = insert(:user)
|
||||||
|
|
||||||
|
site1 =
|
||||||
|
insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)])
|
||||||
|
|
||||||
|
site2 =
|
||||||
|
insert(:site, memberships: [build(:site_membership, user: current_owner, role: :owner)])
|
||||||
|
|
||||||
|
assert :ok = action.(conn, [site1, site2], %{"email" => new_owner.email})
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site1.domain}"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site2.domain}"
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
@ -209,6 +209,76 @@ defmodule Plausible.SitesTest do
|
|||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
describe "bulk_transfer_ownership/4" do
|
||||||
|
test "initiates ownership transfer for multiple sites in one action" do
|
||||||
|
admin_user = insert(:user)
|
||||||
|
new_owner = insert(:user)
|
||||||
|
|
||||||
|
site1 =
|
||||||
|
insert(:site, memberships: [build(:site_membership, user: admin_user, role: :owner)])
|
||||||
|
|
||||||
|
site2 =
|
||||||
|
insert(:site, memberships: [build(:site_membership, user: admin_user, role: :owner)])
|
||||||
|
|
||||||
|
assert {:ok, _} = Sites.bulk_transfer_ownership([site1, site2], admin_user, new_owner.email)
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site1.domain}"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert Repo.exists?(
|
||||||
|
from(i in Plausible.Auth.Invitation,
|
||||||
|
where:
|
||||||
|
i.site_id == ^site1.id and i.email == ^new_owner.email and i.role == :owner
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_invitation_exists(site1, new_owner.email, :owner)
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site2.domain}"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_invitation_exists(site2, new_owner.email, :owner)
|
||||||
|
end
|
||||||
|
|
||||||
|
test "initiates ownership transfer for multiple sites in one action skipping permission checks" do
|
||||||
|
superadmin_user = insert(:user)
|
||||||
|
new_owner = insert(:user)
|
||||||
|
|
||||||
|
site1 = insert(:site)
|
||||||
|
site2 = insert(:site)
|
||||||
|
|
||||||
|
assert {:ok, _} =
|
||||||
|
Sites.bulk_transfer_ownership([site1, site2], superadmin_user, new_owner.email,
|
||||||
|
check_permissions: false
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site1.domain}"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert Repo.exists?(
|
||||||
|
from(i in Plausible.Auth.Invitation,
|
||||||
|
where:
|
||||||
|
i.site_id == ^site1.id and i.email == ^new_owner.email and i.role == :owner
|
||||||
|
)
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_invitation_exists(site1, new_owner.email, :owner)
|
||||||
|
|
||||||
|
assert_email_delivered_with(
|
||||||
|
to: [nil: new_owner.email],
|
||||||
|
subject: "[Plausible Analytics] Request to transfer ownership of #{site2.domain}"
|
||||||
|
)
|
||||||
|
|
||||||
|
assert_invitation_exists(site2, new_owner.email, :owner)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
describe "get_for_user/2" do
|
describe "get_for_user/2" do
|
||||||
test "get site for super_admin" do
|
test "get site for super_admin" do
|
||||||
user1 = insert(:user)
|
user1 = insert(:user)
|
||||||
@ -223,4 +293,12 @@ defmodule Plausible.SitesTest do
|
|||||||
assert %{id: ^site_id} = Sites.get_for_user(user2.id, domain, [:super_admin])
|
assert %{id: ^site_id} = Sites.get_for_user(user2.id, domain, [:super_admin])
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
defp assert_invitation_exists(site, email, role) do
|
||||||
|
assert Repo.exists?(
|
||||||
|
from(i in Plausible.Auth.Invitation,
|
||||||
|
where: i.site_id == ^site.id and i.email == ^email and i.role == ^role
|
||||||
|
)
|
||||||
|
)
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
Loading…
Reference in New Issue
Block a user