mirror
/
analytics
mirror of https://github.com/plausible/analytics.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Remove Editor->Admin site role conversion and limit permissions (#5210) 

* Remove editor->admin mapping from Site Settings > People

* Restrict editor role and allow admin to update site role

* Disable actions in Site Settings > People for roles other than admin and owner

* Fix test

* Update labeling of guest members in CRM for clarity

* Update phrasing in other spots of UI

* Update CHANGELONG.md

* Revise team related banners displayed under Site Settings > People

* Fix permissions check for role update
This commit is contained in:
Adrian Gruntkowski 2025-04-03 13:04:19 +02:00 committed by GitHub
parent 6ee4f57cf7
commit a715ed2f32
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
17 changed files with 160 additions and 121 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
CHANGELOG.md
View File

@ -42,6 +42,7 @@ All notable changes to this project will be documented in this file.
- Main graph now shows revenue with relevant currency symbol when hovering a data point
- Main graph now shows `-` instead of `0` for visit duration, scroll depth when hovering a data point with no visit data
- Make Stats and Sites API keys scoped to teams they are created in
- Remove permissions to manage sites guests and run destructive actions from team editor and guest editor roles in favour of team admin role
### Fixed

4
lib/plausible/site/admin.ex
View File

@ -82,7 +82,7 @@ defmodule Plausible.SiteAdmin do
public: nil,
team: %{value: &get_team/1},
owners: %{value: &get_owners/1},
other_members: %{value: &get_other_members/1},
other_guest_members: %{value: &get_other_members/1},
limits: %{
value: fn site ->
rate_limiting_status =
@ -251,7 +251,7 @@ defmodule Plausible.SiteAdmin do
role = html_escape(m.role)
"""
<a href="/crm/auth/user/#{id}">#{email} (#{role})</a>
<a href="/crm/auth/user/#{id}">#{email} (guest #{role})</a>
"""
end)
|> Phoenix.HTML.raw()

24
lib/plausible/sites.ex
View File

@ -96,17 +96,7 @@ defmodule Plausible.Sites do
where: gm.site_id == ^site.id,
select: %{
user: u,
role:
fragment(
"""
CASE
WHEN ? = 'editor' THEN 'admin'
ELSE ?
END
""",
gm.role,
gm.role
)
role: gm.role
}
)
|> Repo.all()
@ -121,17 +111,7 @@ defmodule Plausible.Sites do
select: %{
invitation_id: gi.invitation_id,
email: ti.email,
role:
fragment(
"""
CASE
WHEN ? = 'editor' THEN 'admin'
ELSE ?
END
""",
gi.role,
gi.role
)
role: gi.role
}
)
|> Repo.all()

8
lib/plausible/teams/memberships.ex
View File

@ -110,7 +110,7 @@ defmodule Plausible.Teams.Memberships do
{:ok, guest_membership} ->
can_grant_role? =
if guest_membership.team_membership.user_id == current_user.id do
can_grant_role_to_self?(current_user_role, new_role)
false
else
can_grant_role_to_other?(current_user_role, new_role)
end
@ -152,16 +152,10 @@ defmodule Plausible.Teams.Memberships do
end
end
defp can_grant_role_to_self?(:editor, :viewer), do: true
defp can_grant_role_to_self?(_, _), do: false
defp can_grant_role_to_other?(:owner, :editor), do: true
defp can_grant_role_to_other?(:owner, :admin), do: true
defp can_grant_role_to_other?(:owner, :viewer), do: true
defp can_grant_role_to_other?(:admin, :editor), do: true
defp can_grant_role_to_other?(:admin, :viewer), do: true
defp can_grant_role_to_other?(:editor, :editor), do: true
defp can_grant_role_to_other?(:editor, :viewer), do: true
defp can_grant_role_to_other?(_, _), do: false
defp send_site_member_removed_email(guest_membership) do

29
lib/plausible_web/components/team/notice.ex
View File

@ -4,12 +4,35 @@ defmodule PlausibleWeb.Team.Notice do
"""
use PlausibleWeb, :component
def inviting_banner(assigns) do
def owner_cta_banner(assigns) do
~H"""
<aside class="mt-4 mb-4">
<.notice title="Inviting people to your team" class="shadow-md dark:shadow-none mt-4">
<.notice
title="A Better Way of Inviting People to Your Team"
class="shadow-md dark:shadow-none mt-4"
>
<p>
You can also invite people to your team and give them different roles like admin, editor, viewer or billing. Team members can have full access to all sites.
You can now create a team and assign different roles to team members, such as admin,
editor, viewer or billing. Team members will gain access to all your sites. <a href={
Routes.team_setup_path(PlausibleWeb.Endpoint, :setup)
}>Create your team here</a>.
</p>
</.notice>
</aside>
"""
end
def guest_cta_banner(assigns) do
~H"""
<aside class="mt-4 mb-4">
<.notice
title="A Better Way of Inviting People to a Team"
class="shadow-md dark:shadow-none mt-4"
>
<p>
It is now possible to create a team and assign different roles to team members, such as
admin, editor, viewer or billing. Team members can gain access to all the sites. Please
contact the site owner to create your team.
</p>
</.notice>
</aside>

2
lib/plausible_web/controllers/invitation_controller.ex
View File

@ -4,7 +4,7 @@ defmodule PlausibleWeb.InvitationController do
plug PlausibleWeb.RequireAccountPlug
plug PlausibleWeb.Plugs.AuthorizeSiteAccess,
[:owner, :editor, :admin] when action in [:remove_invitation]
[:owner, :admin] when action in [:remove_invitation]
def accept_invitation(conn, %{"invitation_id" => invitation_id}) do
current_user = conn.assigns.current_user

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

@ -2,9 +2,8 @@ defmodule PlausibleWeb.Site.MembershipController do
@moduledoc """
This controller deals with user management via the UI in Site Settings -> People. It's important to enforce permissions in this controller.
Owner - Can manage users, can trigger a 'transfer ownership' request
Admin and Editor - Can manage users
Viewer - Can not access user management settings
Owner and Admin - Can manage users, can trigger a 'transfer ownership' request
Editor and Viewer - Can not access user management settings
Anyone - Can accept invitations
Everything else should be explicitly disallowed.
@ -15,20 +14,9 @@ defmodule PlausibleWeb.Site.MembershipController do
use Plausible
alias Plausible.Site.Memberships
@only_owner_and_admin_is_allowed_to [
:transfer_ownership_form,
:transfer_ownership,
:change_team,
:change_team_form
]
plug PlausibleWeb.RequireAccountPlug
plug PlausibleWeb.Plugs.AuthorizeSiteAccess,
[:owner, :admin] when action in @only_owner_and_admin_is_allowed_to
plug PlausibleWeb.Plugs.AuthorizeSiteAccess,
[:owner, :admin, :editor] when action not in @only_owner_and_admin_is_allowed_to
plug PlausibleWeb.Plugs.AuthorizeSiteAccess, [:owner, :admin]
def invite_member_form(conn, _params) do
site =

15
lib/plausible_web/controllers/site_controller.ex
View File

@ -5,11 +5,22 @@ defmodule PlausibleWeb.SiteController do
alias Plausible.Sites
@unrestricted_actions [:new, :create_site]
@destructive_actions [:settings_danger_zone, :reset_stats, :delete_site]
@special_cased_actions @unrestricted_actions ++ @destructive_actions
plug(PlausibleWeb.RequireAccountPlug)
plug(
PlausibleWeb.Plugs.AuthorizeSiteAccess,
[:owner, :admin, :editor, :super_admin] when action not in [:new, :create_site]
[:owner, :admin, :editor, :super_admin]
when action not in @special_cased_actions
)
plug(
PlausibleWeb.Plugs.AuthorizeSiteAccess,
[:owner, :admin, :super_admin] when action in @destructive_actions
)
def new(conn, params) do
@ -134,7 +145,7 @@ defmodule PlausibleWeb.SiteController do
end
def settings_people(conn, _params) do
site = conn.assigns.site
site = Repo.preload(conn.assigns.site, :team)
%{memberships: memberships, invitations: invitations} =
Sites.list_people(site)

2
lib/plausible_web/templates/email/ownership_transfer_accepted.html.heex
View File

@ -1,6 +1,6 @@
{@new_owner_email} has accepted the ownership transfer of {@site.domain}. They will be responsible for billing of it going
forward<%= if @initiator_as_guest? and @initiator_as_editor? do %>
and your role has been changed to <b>admin</b>
and your role has been changed to <b>guest editor</b>
<% end %>.
<%= if @initiator_as_guest? do %>
<a href={Routes.site_url(PlausibleWeb.Endpoint, :settings_general, @site.domain) <> "?__team=none"}>

14
lib/plausible_web/templates/site/membership/invite_member_form.html.heex
View File

@ -38,12 +38,12 @@
<div class="mt-1 bg-white rounded-md -space-y-px dark:bg-gray-800">
<label
class="border-gray-200 dark:border-gray-500 rounded-tl-md rounded-tr-md relative border p-4 flex cursor-pointer"
x-class="{'bg-indigo-50 border-indigo-200 dark:bg-indigo-500 dark:border-indigo-800 z-10': selectedOption === 'admin', 'border-gray-200': selectedOption !== 'admin'}"
x-class="{'bg-indigo-50 border-indigo-200 dark:bg-indigo-500 dark:border-indigo-800 z-10': selectedOption === 'editor', 'border-gray-200': selectedOption !== 'editor'}"
>
<.input
type="radio"
field={f[:role]}
id="role_admin"
id="role_editor"
value="editor"
x-model="selectedOption"
required="true"
@ -53,15 +53,15 @@
<div class="ml-3 flex flex-col">
<span
class="text-gray-900 dark:text-gray-100 block text-sm font-medium"
x-class="{'text-indigo-900 dark:text-white': selectedOption === 'admin', 'text-gray-900 dark:text-gray-100': selectedOption !== 'admin'}"
x-class="{'text-indigo-900 dark:text-white': selectedOption === 'editor', 'text-gray-900 dark:text-gray-100': selectedOption !== 'editor'}"
>
Guest Admin
Guest Editor
</span>
<span
class="text-gray-500 dark:text-gray-400 text-sm block"
x-class="{'text-indigo-700 dark:text-gray-100': selectedOption === 'admin', 'text-gray-500 dark:text-gray-200': selectedOption !== 'admin'}"
x-class="{'text-indigo-700 dark:text-gray-100': selectedOption === 'editor', 'text-gray-500 dark:text-gray-200': selectedOption !== 'editor'}"
>
Can view stats, change site settings and invite other members
Can view stats and change site settings
</span>
</div>
</label>
@ -91,7 +91,7 @@
class="text-gray-500 dark:text-gray-400 text-sm block"
x-class="{'text-indigo-700 dark:text-gray-100': selectedOption === 'viewer', 'text-gray-500 dark:text-gray-200': selectedOption !== 'viewer'}"
>
Can view stats but cannot access settings or invite members
Can view stats but cannot access settings
</span>
</div>
</label>

2
lib/plausible_web/templates/site/membership/transfer_ownership_form.html.heex
View File

@ -8,7 +8,7 @@
hours, the request will expire automatically. <br /><br />
Do note that a subscription plan is not transferred alongside the site. If
they accept the transfer request, the new owner will need to have an active
subscription. Your access will be downgraded to <b>admin</b>
subscription. Your access will be downgraded to <b>guest editor</b>
and any other
member roles will stay the same.
</:subtitle>

18
lib/plausible_web/templates/site/settings_people.html.heex
View File

@ -1,13 +1,17 @@
<.settings_tiles>
<PlausibleWeb.Team.Notice.inviting_banner :if={
@site_role in [:owner, :admin] and Plausible.Teams.setup?(@current_team)
<PlausibleWeb.Team.Notice.owner_cta_banner :if={
@site_role == :owner and not Plausible.Teams.setup?(@current_team)
} />
<PlausibleWeb.Team.Notice.guest_cta_banner :if={
@site_role != :owner and not Plausible.Teams.setup?(@site.team)
} />
<.tile docs="users-roles">
<:title>People</:title>
<:subtitle>Invite your friends or coworkers</:subtitle>
<.filter_bar filtering_enabled?={false}>
<.filter_bar :if={@site_role in [:owner, :admin]} filtering_enabled?={false}>
<.button_link
mt?={false}
href={Routes.membership_path(@conn, :invite_member_form, @site.domain)}
@ -89,9 +93,9 @@
)
}
method="put"
disabled={membership.role == "admin"}
disabled={@site_role not in [:owner, :admin] or membership.role == "editor"}
>
<div>Guest Admin</div>
<div>Guest Editor</div>
<div class="text-gray-500 dark:text-gray-400 text-xs/5">
View stats and edit site settings
</div>
@ -107,7 +111,7 @@
)
}
method="put"
disabled={membership.role == "viewer"}
disabled={@site_role not in [:owner, :admin] or membership.role == "viewer"}
>
<div>Guest Viewer</div>
<div class="text-gray-500 dark:text-gray-400 text-xs/5">
@ -126,6 +130,7 @@
}
class="text-red-600 hover:text-red-600"
method="delete"
disabled={@site_role not in [:owner, :admin]}
>
Remove member
</.dropdown_item>
@ -157,6 +162,7 @@
<.td hide_on_mobile>{Phoenix.Naming.humanize(invitation.role)}</.td>
<.td actions>
<.delete_button
:if={@site_role in [:owner, :admin]}
href={
Routes.invitation_path(
@conn,

6
lib/plausible_web/views/site_view.ex
View File

@ -26,12 +26,12 @@ defmodule PlausibleWeb.SiteView do
end
end
def site_role(%{role: "viewer"}) do
def site_role(%{role: :viewer}) do
"Guest Viewer"
end
def site_role(%{role: "admin"}) do
"Guest Admin"
def site_role(%{role: :editor}) do
"Guest Editor"
end
def site_role(%{role: role}) do

2
test/plausible_web/controllers/api/external_sites_controller_test.exs
View File

@ -1039,7 +1039,7 @@ defmodule PlausibleWeb.Api.ExternalSitesControllerTest do
assert %{"role" => "viewer", "status" => "invited"} = json_response(conn2, 200)
assert %{memberships: [_], invitations: [%{role: "viewer"}]} =
assert %{memberships: [_], invitations: [%{role: :viewer}]} =
Plausible.Sites.list_people(site)
assert_no_emails_delivered()

11
test/plausible_web/controllers/invitation_controller_test.exs
View File

@ -231,7 +231,8 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do
test "removes the invitation", %{conn: conn, user: user} do
owner = new_user()
site = new_site(owner: owner)
add_guest(site, user: user, role: :editor)
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :admin)
invitation = invite_guest(site, "jane@example.com", inviter: owner, role: :editor)
conn =
@ -248,7 +249,8 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do
test "removes the invitation for ownership transfer", %{conn: conn, user: user} do
owner = new_user()
site = new_site(owner: owner)
add_guest(site, user: user, role: :editor)
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :admin)
transfer = invite_transfer(site, "jane@example.com", inviter: owner)
conn =
@ -265,7 +267,7 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do
test "fails to remove an invitation with insufficient permission", %{conn: conn, user: user} do
owner = new_user()
site = new_site(owner: owner)
add_guest(site, user: user, role: :viewer)
add_guest(site, user: user, role: :editor)
invitation = invite_guest(site, "jane@example.com", inviter: owner, role: :editor)
@ -302,7 +304,8 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do
test "renders error for non-existent invitation", %{conn: conn, user: user} do
site = new_site()
add_guest(site, user: user, role: :editor)
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :admin)
remove_invitation_path =
Routes.invitation_path(

104
test/plausible_web/controllers/site/membership_controller_test.exs
View File

@ -5,6 +5,7 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
use Bamboo.Test
use Plausible.Teams.Test
import Plausible.Teams.Test
import Plausible.Test.Support.HTML
@subject_prefix if ee?(), do: "[Plausible Analytics] ", else: "[Plausible CE] "
@ -300,7 +301,10 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
assert_guest_membership(site.team, site, collaborator, :viewer)
end
test "team admin can update a site member's role by user id", %{conn: conn, user: user} do
test "team admin can update a site member's role by user id (from editor to viewer)", %{
conn: conn,
user: user
} do
site = new_site()
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :admin)
@ -314,7 +318,40 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
assert_guest_membership(team, site, collaborator, :viewer)
end
test "can downgrade yourself from admin to viewer, redirects to stats", %{
test "team admin can update a site member's role by user id (from viewer to editor)", %{
conn: conn,
user: user
} do
site = new_site()
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :admin)
collaborator = add_guest(site, role: :viewer)
assert_guest_membership(team, site, collaborator, :viewer)
conn = set_current_team(conn, team)
put(conn, "/sites/#{site.domain}/memberships/u/#{collaborator.id}/role/editor")
assert_guest_membership(team, site, collaborator, :editor)
end
test "team editor can't update site member's role", %{conn: conn, user: user} do
site = new_site()
team = Plausible.Teams.complete_setup(site.team)
add_member(team, user: user, role: :editor)
collaborator = add_guest(site, role: :editor)
assert_guest_membership(team, site, collaborator, :editor)
conn = set_current_team(conn, team)
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{collaborator.id}/role/viewer")
assert html_response(conn, 404)
assert_guest_membership(team, site, collaborator, :editor)
end
test "can't update role when an editor", %{
conn: conn,
user: user
} do
@ -323,21 +360,37 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{user.id}/role/viewer")
assert_guest_membership(site.team, site, user, :viewer)
assert_guest_membership(site.team, site, user, :editor)
assert redirected_to(conn) == "/#{URI.encode_www_form(site.domain)}"
assert html_response(conn, 404)
end
test "can't update role when a viewer", %{
conn: conn,
user: user
} do
site = new_site()
add_guest(site, user: user, role: :viewer)
another_guest = add_guest(site, role: :editor)
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{another_guest.id}/role/viewer")
assert_guest_membership(site.team, site, another_guest, :editor)
assert html_response(conn, 404)
end
test "owner cannot make anyone else owner", %{
conn: conn,
user: user
} do
site = new_site()
admin = add_guest(site, user: user, role: :editor)
site = new_site(owner: user)
editor = new_user()
add_guest(site, user: editor, role: :editor)
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{admin.id}/role/owner")
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{editor.id}/role/owner")
assert_guest_membership(site.team, site, user, :editor)
assert_guest_membership(site.team, site, editor, :editor)
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
"You are not allowed to grant the owner role"
@ -358,33 +411,6 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
"You are not allowed to grant the admin role"
end
test "admin can make another user admin", %{
conn: conn,
user: user
} do
site = new_site()
add_guest(site, user: user, role: :editor)
viewer = add_guest(site, user: new_user(), role: :viewer)
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{viewer.id}/role/editor")
assert_guest_membership(site.team, site, viewer, :editor)
assert redirected_to(conn) == "/#{URI.encode_www_form(site.domain)}/settings/people"
end
test "admin can't make themselves an owner", %{conn: conn, user: user} do
site = new_site()
add_guest(site, user: user, role: :editor)
conn = put(conn, "/sites/#{site.domain}/memberships/u/#{user.id}/role/owner")
assert_guest_membership(site.team, site, user, :editor)
assert Phoenix.Flash.get(conn.assigns.flash, :error) ==
"You are not allowed to grant the owner role"
end
end
describe "DELETE /sites/:domain/memberships/:id" do
@ -453,13 +479,13 @@ defmodule PlausibleWeb.Site.MembershipControllerTest do
end
test "notifies the user who has been removed via email", %{conn: conn, user: user} do
site = new_site()
admin = add_guest(site, user: user, role: :editor)
site = new_site(owner: user)
editor = add_guest(site, role: :editor)
delete(conn, "/sites/#{site.domain}/memberships/u/#{admin.id}")
delete(conn, "/sites/#{site.domain}/memberships/u/#{editor.id}")
assert_email_delivered_with(
to: [nil: admin.email],
to: [nil: editor.email],
subject: @subject_prefix <> "Your access to #{site.domain} has been revoked"
)
end

21
test/plausible_web/controllers/site_controller_test.exs
View File

@ -552,7 +552,7 @@ defmodule PlausibleWeb.SiteControllerTest do
assert owner_row =~ "Owner"
assert editor_row =~ editor.email
assert editor_row_button == "Guest Admin"
assert editor_row_button == "Guest Editor"
refute editor_row =~ "Owner"
assert viewer_row =~ viewer.email
@ -567,7 +567,8 @@ defmodule PlausibleWeb.SiteControllerTest do
conn = get(conn, "/#{site.domain}/settings/people")
resp = html_response(conn, 200)
assert text_of_element(resp, "#invitation-#{i1.invitation_id}") == "admin@example.com Admin"
assert text_of_element(resp, "#invitation-#{i1.invitation_id}") ==
"admin@example.com Editor"
assert text_of_element(resp, "#invitation-#{i2.invitation_id}") ==
"viewer@example.com Viewer"
@ -586,11 +587,12 @@ defmodule PlausibleWeb.SiteControllerTest do
"#{new_owner.email} Owner"
end
test "renders team management notices", %{conn: conn, user: user} do
test "renders distinct team management notices to owner", %{conn: conn, user: user} do
site = new_site(owner: user)
resp = conn |> get("/#{site.domain}/settings/people") |> html_response(200)
refute resp =~ "You can also invite people to your team"
refute resp =~ "A Better Way of Inviting People to a Team"
assert resp =~ "A Better Way of Inviting People to Your Team"
refute resp =~ "Team members automatically have access to this site."
team = team_of(user)
@ -598,22 +600,27 @@ defmodule PlausibleWeb.SiteControllerTest do
conn = set_current_team(conn, team)
resp = conn |> get("/#{site.domain}/settings/people") |> html_response(200)
assert resp =~ "You can also invite people to your team"
refute resp =~ "A Better Way of Inviting People to Your Team"
assert resp =~ "Team members automatically have access to this site."
end
test "does not render team management notices to editors", %{conn: conn, user: user} do
test "renders distinct team management notices to editors", %{conn: conn, user: user} do
# this can go away once we support multiple teams
user |> team_of() |> Repo.delete!()
owner = new_user()
site = new_site(owner: owner)
add_member(team_of(owner), user: user, role: :editor)
resp = conn |> get("/#{site.domain}/settings/people") |> html_response(200)
assert resp =~ "A Better Way of Inviting People to a Team"
refute resp =~ "A Better Way of Inviting People to Your Team"
owner |> team_of() |> Teams.Team.setup_changeset() |> Repo.update!()
resp = conn |> get("/#{site.domain}/settings/people") |> html_response(200)
refute resp =~ "You can also invite people to your team"
refute resp =~ "A Better Way of Inviting People to a Team"
refute resp =~ "Team members automatically have access to this site."
end
end