From a45bc1c963d06be076298ee4241358c99c307d58 Mon Sep 17 00:00:00 2001 From: Adrian Gruntkowski Date: Mon, 20 Jan 2025 15:24:21 +0100 Subject: [PATCH] Implement team member and invitation management actions (#4977) * Implement scaffolding for team member and invite mgmt actions * Implement updating team role * Prevent changing role if the subject is the only remaining owner * Implement removing team membership * Fix only remaining owner removal checks * Fix remove team membership service * Fix and clean up imports * Implement team invitation removal * Fix errors surfaced by dialyzer * Test and fix removing team invitations * Make accept invitation action work for team invitations * Test rejecting team invitation * Test team membership role update and removal actions * Fix flash message interpolation and missing team in transfer result * Implement migration adding UUID identifier to team * Set UUID identifier on team creation * Implement get team by identifier * Display team invitations on /sites * Test rendering team invitations on /sites * Add team management notices on /settings/people * Test showing team management notices on /settings/people * Stop drawing double horizontal rule * Add modueldoc * Handle guest member trying to call team membership endpoints gracefully --------- Co-authored-by: Adam Rutkowski --- .../site/memberships/accept_invitation.ex | 2 +- lib/plausible/teams.ex | 14 +- lib/plausible/teams/invitations.ex | 27 ++- lib/plausible/teams/invitations/remove.ex | 31 +++ lib/plausible/teams/memberships.ex | 24 +++ lib/plausible/teams/memberships/remove.ex | 43 ++++ .../teams/memberships/update_role.ex | 89 ++++++++ lib/plausible/teams/team.ex | 10 + lib/plausible_web/components/team/notice.ex | 64 ++++++ .../controllers/invitation_controller.ex | 42 +++- .../controllers/team_controller.ex | 71 ++++++ lib/plausible_web/email.ex | 13 ++ lib/plausible_web/live/sites.ex | 26 ++- lib/plausible_web/router.ex | 5 + .../email/team_member_removed.html.heex | 4 + .../templates/site/settings_people.html.heex | 8 + .../invitation_controller_test.exs | 130 +++++++++++ .../controllers/site_controller_test.exs | 30 +++ .../controllers/team_controller_test.exs | 203 ++++++++++++++++++ test/plausible_web/live/sites_test.exs | 41 ++++ test/support/teams/test.ex | 7 + 21 files changed, 863 insertions(+), 21 deletions(-) create mode 100644 lib/plausible/teams/invitations/remove.ex create mode 100644 lib/plausible/teams/memberships/remove.ex create mode 100644 lib/plausible/teams/memberships/update_role.ex create mode 100644 lib/plausible_web/components/team/notice.ex create mode 100644 lib/plausible_web/controllers/team_controller.ex create mode 100644 lib/plausible_web/templates/email/team_member_removed.html.heex create mode 100644 test/plausible_web/controllers/team_controller_test.exs diff --git a/lib/plausible/site/memberships/accept_invitation.ex b/lib/plausible/site/memberships/accept_invitation.ex index b03ec97ec3a7..dec687387edd 100644 --- a/lib/plausible/site/memberships/accept_invitation.ex +++ b/lib/plausible/site/memberships/accept_invitation.ex @@ -103,7 +103,7 @@ defmodule Plausible.Site.Memberships.AcceptInvitation do site = site |> Repo.reload!() |> Repo.preload(ownership: :user) - {:ok, %{team_membership: site.ownership, site: site}} + {:ok, %{team: team, team_membership: site.ownership, site: site}} end end diff --git a/lib/plausible/teams.ex b/lib/plausible/teams.ex index 7b0dee4bfbee..39f21a53760c 100644 --- a/lib/plausible/teams.ex +++ b/lib/plausible/teams.ex @@ -16,11 +16,15 @@ defmodule Plausible.Teams do not is_nil(team) and FunWithFlags.enabled?(:teams, for: team) end - @spec get!(pos_integer()) :: Teams.Team.t() - def get!(team_id) do + @spec get!(pos_integer() | binary()) :: Teams.Team.t() + def get!(team_id) when is_integer(team_id) do Repo.get!(Teams.Team, team_id) end + def get!(team_identifier) when is_binary(team_identifier) do + Repo.get_by!(Teams.Team, identifier: team_identifier) + end + @spec get_owner(Teams.Team.t()) :: {:ok, Plausible.Auth.User.t()} | {:error, :no_owner | :multiple_owners} def get_owner(team) do @@ -68,10 +72,11 @@ defmodule Plausible.Teams do def owned_sites_ids(team) do Repo.all( - from s in Plausible.Site, + from(s in Plausible.Site, where: s.team_id == ^team.id, select: s.id, order_by: [desc: s.id] + ) ) end @@ -81,9 +86,10 @@ defmodule Plausible.Teams do def owned_sites_locked?(team) do Repo.exists?( - from s in Plausible.Site, + from(s in Plausible.Site, where: s.team_id == ^team.id, where: s.locked == true + ) ) end diff --git a/lib/plausible/teams/invitations.ex b/lib/plausible/teams/invitations.ex index 9d3cd32d60d1..041f47812c2c 100644 --- a/lib/plausible/teams/invitations.ex +++ b/lib/plausible/teams/invitations.ex @@ -9,6 +9,16 @@ defmodule Plausible.Teams.Invitations do alias Plausible.Repo alias Plausible.Teams + def get_team_invitation(team, invitation_id) do + invitation = Repo.get_by(Teams.Invitation, team_id: team.id, invitation_id: invitation_id) + + if invitation do + {:ok, invitation} + else + {:error, :invitation_not_found} + end + end + def find_for_user(invitation_or_transfer_id, user) do with {:error, :invitation_not_found} <- find_team_invitation_for_user(invitation_or_transfer_id, user), @@ -25,6 +35,17 @@ defmodule Plausible.Teams.Invitations do end end + def find_team_invitations(user) do + Repo.all( + from ti in Teams.Invitation, + inner_join: inviter in assoc(ti, :inviter), + inner_join: team in assoc(ti, :team), + where: ti.email == ^user.email, + where: ti.role != :guest, + preload: [inviter: inviter, team: team] + ) + end + defp find_team_invitation_for_user(team_invitation_id, user) do invitation_query = from ti in Teams.Invitation, @@ -280,7 +301,11 @@ defmodule Plausible.Teams.Invitations do send_invitation_accepted_email(team_invitation, guest_invitations) end - %{team_membership: team_membership, guest_memberships: guest_memberships} + %{ + team: team_invitation.team, + team_membership: team_membership, + guest_memberships: guest_memberships + } else {:error, changeset} -> Repo.rollback(changeset) end diff --git a/lib/plausible/teams/invitations/remove.ex b/lib/plausible/teams/invitations/remove.ex new file mode 100644 index 000000000000..e6b8aec556b1 --- /dev/null +++ b/lib/plausible/teams/invitations/remove.ex @@ -0,0 +1,31 @@ +defmodule Plausible.Teams.Invitations.Remove do + @moduledoc """ + Service for removing a team invitation. + """ + + alias Plausible.Repo + alias Plausible.Teams.Invitations + alias Plausible.Teams.Memberships + + def remove(nil, _invitation_id, _current_user) do + {:error, :permission_denied} + end + + def remove(team, invitation_id, current_user) do + with {:ok, team_invitation} <- Invitations.get_team_invitation(team, invitation_id), + {:ok, current_user_role} <- Memberships.team_role(team, current_user), + :ok <- check_can_remove_invitation(current_user_role) do + Repo.delete!(team_invitation) + + {:ok, team_invitation} + end + end + + defp check_can_remove_invitation(role) do + if role in [:owner, :admin] do + :ok + else + {:error, :permission_denied} + end + end +end diff --git a/lib/plausible/teams/memberships.ex b/lib/plausible/teams/memberships.ex index ee1492cc3973..41d4c5fee670 100644 --- a/lib/plausible/teams/memberships.ex +++ b/lib/plausible/teams/memberships.ex @@ -13,6 +13,13 @@ defmodule Plausible.Teams.Memberships do |> Repo.all() end + def owners_count(team) do + Repo.aggregate( + from(tm in Teams.Membership, where: tm.team_id == ^team.id and tm.role == :owner), + :count + ) + end + def team_role(team, user) do result = from(u in Auth.User, @@ -145,6 +152,23 @@ defmodule Plausible.Teams.Memberships do :ok end + def get_team_membership(team, %Auth.User{} = user) do + get_team_membership(team, user.id) + end + + def get_team_membership(team, user_id) do + query = + from( + tm in Teams.Membership, + where: tm.team_id == ^team.id and tm.user_id == ^user_id + ) + + case Repo.one(query) do + nil -> {:error, :membership_not_found} + membership -> {:ok, membership} + end + end + defp get_guest_membership(site_id, user_id) do query = from( diff --git a/lib/plausible/teams/memberships/remove.ex b/lib/plausible/teams/memberships/remove.ex new file mode 100644 index 000000000000..9aa1562e2c43 --- /dev/null +++ b/lib/plausible/teams/memberships/remove.ex @@ -0,0 +1,43 @@ +defmodule Plausible.Teams.Memberships.Remove do + @moduledoc """ + Service for removing a team member. + """ + + alias Plausible.Repo + alias Plausible.Teams.Memberships + + def remove(nil, _, _), do: {:error, :permission_denied} + + def remove(team, user_id, current_user) do + with {:ok, team_membership} <- Memberships.get_team_membership(team, user_id), + {:ok, current_user_role} <- Memberships.team_role(team, current_user), + :ok <- check_can_remove_membership(current_user_role, team_membership.role), + :ok <- check_owner_can_get_removed(team, team_membership.role) do + team_membership = Repo.preload(team_membership, [:team, :user]) + Repo.delete!(team_membership) + send_team_member_removed_email(team_membership) + + {:ok, team_membership} + end + end + + defp check_can_remove_membership(:owner, _), do: :ok + defp check_can_remove_membership(:admin, role) when role != :owner, do: :ok + defp check_can_remove_membership(_, _), do: {:error, :permission_denied} + + defp check_owner_can_get_removed(team, :owner) do + if Memberships.owners_count(team) > 1 do + :ok + else + {:error, :only_one_owner} + end + end + + defp check_owner_can_get_removed(_team, _role), do: :ok + + defp send_team_member_removed_email(team_membership) do + team_membership + |> PlausibleWeb.Email.team_member_removed() + |> Plausible.Mailer.send() + end +end diff --git a/lib/plausible/teams/memberships/update_role.ex b/lib/plausible/teams/memberships/update_role.ex new file mode 100644 index 000000000000..2dccfd18981e --- /dev/null +++ b/lib/plausible/teams/memberships/update_role.ex @@ -0,0 +1,89 @@ +defmodule Plausible.Teams.Memberships.UpdateRole do + @moduledoc """ + Service for updating role of a team member. + """ + + alias Plausible.Repo + alias Plausible.Teams + alias Plausible.Teams.Memberships + + def update(nil, _, _, _), do: {:error, :permission_denied} + + def update(team, user_id, new_role_str, current_user) do + new_role = String.to_existing_atom(new_role_str) + + with :ok <- check_valid_role(new_role), + {:ok, team_membership} <- Memberships.get_team_membership(team, user_id), + {:ok, current_user_role} <- Memberships.team_role(team, current_user), + granting_to_self? = team_membership.user_id == user_id, + :ok <- + check_can_grant_role( + current_user_role, + team_membership.role, + new_role, + granting_to_self? + ), + :ok <- check_owner_can_get_demoted(team, team_membership.role, new_role) do + team_membership = + team_membership + |> Ecto.Changeset.change(role: new_role) + |> Repo.update!() + |> Repo.preload(:user) + + {:ok, team_membership} + end + end + + defp check_valid_role(role) do + if role in (Teams.Membership.roles() -- [:guest]) do + :ok + else + {:error, :invalid_role} + end + end + + defp check_owner_can_get_demoted(team, :owner, new_role) when new_role != :owner do + if Memberships.owners_count(team) > 1 do + :ok + else + {:error, :only_one_owner} + end + end + + defp check_owner_can_get_demoted(_team, _current_role, _new_role), do: :ok + + defp check_can_grant_role(user_role, _from_role, to_role, true) do + if can_grant_role_to_self?(user_role, to_role) do + :ok + else + {:error, :permission_denied} + end + end + + defp check_can_grant_role(user_role, from_role, to_role, false) do + if can_grant_role_to_other?(user_role, from_role, to_role) do + :ok + else + {:error, :permission_denied} + end + end + + defp can_grant_role_to_self?(:owner, :admin), do: true + defp can_grant_role_to_self?(:owner, :editor), do: true + defp can_grant_role_to_self?(:owner, :viewer), do: true + defp can_grant_role_to_self?(:admin, :editor), do: true + defp can_grant_role_to_self?(:admin, :viewer), do: true + defp can_grant_role_to_self?(_, _), do: false + + defp can_grant_role_to_other?(:owner, _, _), do: true + defp can_grant_role_to_other?(:admin, :admin, :admin), do: true + defp can_grant_role_to_other?(:admin, :admin, :editor), do: true + defp can_grant_role_to_other?(:admin, :admin, :viewer), do: true + defp can_grant_role_to_other?(:admin, :editor, :admin), do: true + defp can_grant_role_to_other?(:admin, :editor, :editor), do: true + defp can_grant_role_to_other?(:admin, :editor, :viewer), do: true + defp can_grant_role_to_other?(:admin, :viewer, :admin), do: true + defp can_grant_role_to_other?(:admin, :viewer, :editor), do: true + defp can_grant_role_to_other?(:admin, :viewer, :viewer), do: true + defp can_grant_role_to_other?(_, _, _), do: false +end diff --git a/lib/plausible/teams/team.ex b/lib/plausible/teams/team.ex index ba8116b0a0aa..d17e337f084f 100644 --- a/lib/plausible/teams/team.ex +++ b/lib/plausible/teams/team.ex @@ -20,6 +20,7 @@ defmodule Plausible.Teams.Team do @subscription_accept_traffic_until_offset_days 30 schema "teams" do + field :identifier, Ecto.UUID field :name, :string field :trial_expiry_date, :date field :accept_traffic_until, :date @@ -53,6 +54,7 @@ defmodule Plausible.Teams.Team do |> validate_required(:name) |> start_trial(today) |> maybe_bump_accept_traffic_until() + |> maybe_set_identifier() end def name_changeset(team, attrs \\ %{}) do @@ -84,6 +86,14 @@ defmodule Plausible.Teams.Team do change(team, trial_expiry_date: Date.utc_today() |> Date.shift(day: -1)) end + defp maybe_set_identifier(changeset) do + if get_field(changeset, :identifier) do + changeset + else + put_change(changeset, :identifier, Ecto.UUID.generate()) + end + end + defp maybe_bump_accept_traffic_until(changeset) do expiry_change = get_change(changeset, :trial_expiry_date) diff --git a/lib/plausible_web/components/team/notice.ex b/lib/plausible_web/components/team/notice.ex new file mode 100644 index 000000000000..a2140b5c9e92 --- /dev/null +++ b/lib/plausible_web/components/team/notice.ex @@ -0,0 +1,64 @@ +defmodule PlausibleWeb.Team.Notice do + @moduledoc """ + Components with teams related notices. + """ + use PlausibleWeb, :component + + def inviting_banner(assigns) do + ~H""" + + """ + end + + def team_members_notice(assigns) do + ~H""" + + """ + end + + def team_invitations(assigns) do + ~H""" + + """ + end +end diff --git a/lib/plausible_web/controllers/invitation_controller.ex b/lib/plausible_web/controllers/invitation_controller.ex index 34a45b05a500..8bc3186b12bf 100644 --- a/lib/plausible_web/controllers/invitation_controller.ex +++ b/lib/plausible_web/controllers/invitation_controller.ex @@ -9,18 +9,29 @@ defmodule PlausibleWeb.InvitationController do def accept_invitation(conn, %{"invitation_id" => invitation_id}) do case Plausible.Site.Memberships.accept_invitation(invitation_id, conn.assigns.current_user) do {:ok, result} -> + team = result.team + site = case result do %{guest_memberships: [guest_membership]} -> Plausible.Repo.preload(guest_membership, :site).site + %{guest_memberships: []} -> + nil + %{site: site} -> site end - conn - |> put_flash(:success, "You now have access to #{site.domain}") - |> redirect(external: "/#{URI.encode_www_form(site.domain)}") + if site do + conn + |> put_flash(:success, "You now have access to #{site.domain}") + |> redirect(external: "/#{URI.encode_www_form(site.domain)}") + else + conn + |> put_flash(:success, "You now have access to \"#{team.name}\" team") + |> redirect(external: "/sites") + end {:error, :invitation_not_found} -> conn @@ -54,9 +65,9 @@ defmodule PlausibleWeb.InvitationController do def reject_invitation(conn, %{"invitation_id" => invitation_id}) do case Plausible.Site.Memberships.reject_invitation(invitation_id, conn.assigns.current_user) do - {:ok, invitation} -> + {:ok, _invitation} -> conn - |> put_flash(:success, "You have rejected the invitation to #{invitation.site.domain}") + |> put_flash(:success, "You have rejected the invitation") |> redirect(to: "/sites") {:error, :invitation_not_found} -> @@ -88,4 +99,25 @@ defmodule PlausibleWeb.InvitationController do |> redirect(external: Routes.site_path(conn, :settings_people, conn.assigns.site.domain)) end end + + def remove_team_invitation(conn, %{"invitation_id" => invitation_id}) do + %{my_team: team, current_user: current_user} = conn.assigns + + case Plausible.Teams.Invitations.Remove.remove(team, invitation_id, current_user) do + {:ok, invitation} -> + conn + |> put_flash(:success, "You have removed the invitation for #{invitation.email}") + |> redirect(external: Routes.settings_path(conn, :team_general)) + + {:error, :invitation_not_found} -> + conn + |> put_flash(:error, "Invitation missing or already removed") + |> redirect(external: Routes.settings_path(conn, :team_general)) + + {:error, :permission_denied} -> + conn + |> put_flash(:error, "You are not allowed to remove invitations") + |> redirect(to: Routes.settings_path(conn, :team_general)) + end + end end diff --git a/lib/plausible_web/controllers/team_controller.ex b/lib/plausible_web/controllers/team_controller.ex new file mode 100644 index 000000000000..ccf9585eb05c --- /dev/null +++ b/lib/plausible_web/controllers/team_controller.ex @@ -0,0 +1,71 @@ +defmodule PlausibleWeb.TeamController do + use PlausibleWeb, :controller + + alias Plausible.Teams + + plug PlausibleWeb.RequireAccountPlug + + def update_member_role(conn, %{"id" => user_id, "new_role" => new_role_str}) do + %{my_team: team, current_user: current_user} = conn.assigns + + case Teams.Memberships.UpdateRole.update(team, user_id, new_role_str, current_user) do + {:ok, team_membership} -> + redirect_target = + if team_membership.user_id == current_user.id and team_membership.role == :viewer do + Routes.site_path(conn, :index) + else + Routes.settings_path(conn, :team_general) + end + + conn + |> put_flash( + :success, + "#{team_membership.user.name} is now #{PlausibleWeb.SiteView.with_indefinite_article(to_string(team_membership.role))}" + ) + |> redirect(to: redirect_target) + + {:error, :only_one_owner} -> + conn + |> put_flash(:error, "User is the only owner and can't be changed") + |> redirect(to: Routes.settings_path(conn, :team_general)) + + {:error, _} -> + conn + |> put_flash(:error, "You are not allowed to grant role to that member") + |> redirect(to: Routes.settings_path(conn, :team_general)) + end + end + + def remove_member(conn, %{"id" => user_id}) do + %{my_team: team, current_user: current_user} = conn.assigns + + case Teams.Memberships.Remove.remove(team, user_id, current_user) do + {:ok, team_membership} -> + redirect_target = + if team_membership.user_id == current_user.id do + Routes.site_path(conn, :index) + else + Routes.settings_path(conn, :team_general) + end + + conn + |> put_flash(:success, "User has been removed from the team") + |> redirect(external: redirect_target) + + {:error, :only_one_owner} -> + conn + |> put_flash(:error, "User is the only owner and can't be changed") + |> redirect(to: Routes.settings_path(conn, :team_general)) + + {:error, :membership_not_found} -> + conn + |> put_flash(:success, "User has been removed from \"#{team.name}\" team") + |> redirect(external: Routes.settings_path(conn, :team_general)) + + {:error, :permission_denied} -> + conn + |> put_flash(:error, "You are not allowed to remove that member") + |> redirect(to: Routes.settings_path(conn, :team_general)) + end + end +end diff --git a/lib/plausible_web/email.ex b/lib/plausible_web/email.ex index 6f7691209166..0da3118e56b0 100644 --- a/lib/plausible_web/email.ex +++ b/lib/plausible_web/email.ex @@ -388,6 +388,19 @@ defmodule PlausibleWeb.Email do ) end + def team_member_removed(team_membership) do + priority_email() + |> to(team_membership.user.email) + |> tag("team-member-removed") + |> subject( + "[#{Plausible.product_name()}] Your access to \"#{team_membership.team.name}\" team has been revoked" + ) + |> render("team_member_removed.html", + user: team_membership.user, + team_membership: team_membership + ) + end + def import_success(site_import, user) do import_api = Plausible.Imported.ImportSources.by_name(site_import.source) label = import_api.label() diff --git a/lib/plausible_web/live/sites.ex b/lib/plausible_web/live/sites.ex index 25c8d0d1b683..3eaa466770cc 100644 --- a/lib/plausible_web/live/sites.ex +++ b/lib/plausible_web/live/sites.ex @@ -16,6 +16,10 @@ defmodule PlausibleWeb.Live.Sites do socket = socket |> assign(:uri, uri) + |> assign( + :team_invitations, + Plausible.Teams.Invitations.find_team_invitations(socket.assigns.current_user) + ) |> assign(:filter_text, params["filter_text"] || "") {:ok, socket} @@ -57,6 +61,8 @@ defmodule PlausibleWeb.Live.Sites do + +
<.search_form :if={@has_sites?} filter_text={@filter_text} uri={@uri} />

@@ -145,9 +151,9 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :site, Plausible.Site, required: true - attr :invitation, :map, required: true - attr :hourly_stats, :map, required: true + attr(:site, Plausible.Site, required: true) + attr(:invitation, :map, required: true) + attr(:hourly_stats, :map, required: true) def invitation(assigns) do ~H""" @@ -180,8 +186,8 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :site, Plausible.Site, required: true - attr :hourly_stats, :map, required: true + attr(:site, Plausible.Site, required: true) + attr(:hourly_stats, :map, required: true) def site(assigns) do ~H""" @@ -272,7 +278,7 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :rest, :global + attr(:rest, :global) def icon_pin(assigns) do ~H""" @@ -289,7 +295,7 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :hourly_stats, :map, required: true + attr(:hourly_stats, :map, required: true) def site_stats(assigns) do ~H""" @@ -321,7 +327,7 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :change, :integer, required: true + attr(:change, :integer, required: true) # Related React component: def percentage_change(assigns) do @@ -517,8 +523,8 @@ defmodule PlausibleWeb.Live.Sites do """ end - attr :filter_text, :string, default: "" - attr :uri, URI, required: true + attr(:filter_text, :string, default: "") + attr(:uri, URI, required: true) def search_form(assigns) do ~H""" diff --git a/lib/plausible_web/router.ex b/lib/plausible_web/router.ex index 5b03d77b5903..bb03581d6c5e 100644 --- a/lib/plausible_web/router.ex +++ b/lib/plausible_web/router.ex @@ -389,6 +389,11 @@ defmodule PlausibleWeb.Router do get "/team/general", SettingsController, :team_general post "/team/general/name", SettingsController, :update_team_name + put "/team/memberships/u/:id/role/:new_role", TeamController, :update_member_role + delete "/team/memberships/u/:id", TeamController, :remove_member + post "/team/invitations/:invitation_id/accept", InvitationController, :accept_invitation + post "/team/invitations/:invitation_id/reject", InvitationController, :reject_invitation + delete "/team/invitations/:invitation_id", InvitationController, :remove_team_invitation end scope "/", PlausibleWeb do diff --git a/lib/plausible_web/templates/email/team_member_removed.html.heex b/lib/plausible_web/templates/email/team_member_removed.html.heex new file mode 100644 index 000000000000..a4ba474eb0d4 --- /dev/null +++ b/lib/plausible_web/templates/email/team_member_removed.html.heex @@ -0,0 +1,4 @@ +An administrator of \"{@team_membership.team.name}\" team has removed you as a member. +

+Click here +to view your sites. diff --git a/lib/plausible_web/templates/site/settings_people.html.heex b/lib/plausible_web/templates/site/settings_people.html.heex index aabdac734f40..d5c540942ac4 100644 --- a/lib/plausible_web/templates/site/settings_people.html.heex +++ b/lib/plausible_web/templates/site/settings_people.html.heex @@ -1,4 +1,8 @@ <.settings_tiles> + + <.tile docs="users-roles"> <:title>People <:subtitle>Invite your friends or coworkers @@ -12,6 +16,10 @@ + +

    <%= for membership <- @memberships do %> diff --git a/test/plausible_web/controllers/invitation_controller_test.exs b/test/plausible_web/controllers/invitation_controller_test.exs index c4ecdeca5204..5f060ede15b4 100644 --- a/test/plausible_web/controllers/invitation_controller_test.exs +++ b/test/plausible_web/controllers/invitation_controller_test.exs @@ -4,6 +4,8 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do use Plausible.Teams.Test use Bamboo.Test + alias Plausible.Teams + setup [:create_user, :log_in] describe "POST /sites/invitations/:invitation_id/accept" do @@ -26,6 +28,26 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do assert_guest_membership(site.team, site, user, :editor) end + test "converts the team invitation into a team membership", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + + invitation = + invite_member(team, user.email, inviter: owner, role: :editor) + + conn = post(conn, "/settings/team/invitations/#{invitation.invitation_id}/accept") + + assert Phoenix.Flash.get(conn.assigns.flash, :success) == + "You now have access to \"#{team.name}\" team" + + assert redirected_to(conn) == "/sites" + + refute Repo.get_by(Teams.Invitation, email: user.email) + + assert_team_membership(user, team, :editor) + end + test "does not crash if clicked for the 2nd time in another tab", %{conn: conn, user: user} do owner = new_user() site = new_site(owner: owner) @@ -119,6 +141,20 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do refute Repo.reload(invitation) end + test "rejects the team invitation", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + + invitation = invite_member(team, user.email, inviter: owner, role: :editor) + + conn = post(conn, "/settings/team/invitations/#{invitation.invitation_id}/reject") + + assert redirected_to(conn, 302) == "/sites" + + refute Repo.reload(invitation) + end + test "renders error for non-existent invitation", %{conn: conn} do conn = post(conn, "/sites/invitations/does-not-exist/reject") @@ -222,4 +258,98 @@ defmodule PlausibleWeb.Site.InvitationControllerTest do "Invitation missing or already removed" end end + + describe "DELETE /team/invitations/:invitation_id" do + for role <- [:owner, :admin], invitee_role <- Teams.Membership.roles() do + test "#{role} removes the #{invitee_role} team invitation", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + add_member(team, user: user, role: unquote(role)) + + invitation = + invite_member(team, "jane@example.com", inviter: owner, role: unquote(invitee_role)) + + conn = + delete( + conn, + Routes.invitation_path(conn, :remove_team_invitation, invitation.invitation_id) + ) + + assert redirected_to(conn, 302) == "/settings/team/general" + + refute Repo.reload(invitation) + end + end + + for role <- Teams.Membership.roles() -- [:owner, :admin] do + test "#{role} can't remove a team invitation", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + add_member(team, user: user, role: unquote(role)) + + invitation = + invite_member(team, "jane@example.com", inviter: owner, role: :viewer) + + conn = + delete( + conn, + Routes.invitation_path(conn, :remove_team_invitation, invitation.invitation_id) + ) + + assert redirected_to(conn, 302) == "/settings/team/general" + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "You are not allowed to remove invitations" + + assert Repo.reload(invitation) + end + end + + test "fails to remove a team invitation from the outside", %{conn: my_conn, user: me} do + new_site(owner: me) + other_user = new_user() + _other_site = new_site(owner: other_user) + other_team = team_of(other_user) + + invitation = + invite_member(other_team, "jane@example.com", role: :editor, inviter: other_user) + + remove_invitation_path = + Routes.invitation_path( + my_conn, + :remove_team_invitation, + invitation.invitation_id + ) + + my_conn = delete(my_conn, remove_invitation_path) + + assert Phoenix.Flash.get(my_conn.assigns.flash, :error) == + "Invitation missing or already removed" + + assert Repo.reload(invitation) + end + + test "renders error for non-existent team invitation", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + add_member(team, user: user, role: :editor) + + remove_invitation_path = + Routes.invitation_path( + conn, + :remove_team_invitation, + "does_not_exist" + ) + + conn = delete(conn, remove_invitation_path) + + assert redirected_to(conn, 302) == "/settings/team/general" + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "Invitation missing or already removed" + end + end end diff --git a/test/plausible_web/controllers/site_controller_test.exs b/test/plausible_web/controllers/site_controller_test.exs index f79cce7ec8b8..a55fd1e54570 100644 --- a/test/plausible_web/controllers/site_controller_test.exs +++ b/test/plausible_web/controllers/site_controller_test.exs @@ -10,6 +10,7 @@ defmodule PlausibleWeb.SiteControllerTest do import Plausible.Test.Support.HTML alias Plausible.Imported.SiteImport + alias Plausible.Teams require Plausible.Imported.SiteImport @@ -567,6 +568,35 @@ defmodule PlausibleWeb.SiteControllerTest do assert text_of_element(resp, "#invitation-#{st.transfer_id}") == "#{new_owner.email} Owner" end + + test "renders team management notices", %{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 =~ "Team members automatically have access to this site." + + user |> team_of() |> Teams.Team.setup_changeset() |> Repo.update!() + + resp = conn |> get("/#{site.domain}/settings/people") |> html_response(200) + assert resp =~ "You can also invite 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 + # 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) + + 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 =~ "Team members automatically have access to this site." + end end describe "GET /:domain/settings/goals" do diff --git a/test/plausible_web/controllers/team_controller_test.exs b/test/plausible_web/controllers/team_controller_test.exs new file mode 100644 index 000000000000..f74963526be5 --- /dev/null +++ b/test/plausible_web/controllers/team_controller_test.exs @@ -0,0 +1,203 @@ +defmodule PlausibleWeb.TeamControllerTest do + use PlausibleWeb.ConnCase, async: true + use Plausible.Repo + use Plausible.Teams.Test + use Bamboo.Test + + alias Plausible.Teams + + @subject_prefix if ee?(), do: "[Plausible Analytics] ", else: "[Plausible CE] " + + setup [:create_user, :log_in] + + describe "PUT /team/memberships/u/:id/role/:new_role" do + test "updates a team member's role by user id", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + collaborator = add_member(team, role: :editor) + assert_team_membership(collaborator, team, :editor) + + put(conn, "/settings/team/memberships/u/#{collaborator.id}/role/viewer") + + assert_team_membership(collaborator, team, :viewer) + end + + test "can demote self when an owner", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + _collaborator = add_member(team, role: :owner) + + conn = put(conn, "/settings/team/memberships/u/#{user.id}/role/viewer") + + assert redirected_to(conn, 302) == Routes.site_path(conn, :index) + + assert_team_membership(user, team, :viewer) + end + + test "can't demote self when the only owner", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + + conn = put(conn, "/settings/team/memberships/u/#{user.id}/role/viewer") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "User is the only owner and can't be changed" + + assert_team_membership(user, team, :owner) + end + + test "can demote self when an admin", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + _admin = add_member(team, user: user, role: :admin) + + conn = put(conn, "/settings/team/memberships/u/#{user.id}/role/viewer") + + assert redirected_to(conn, 302) == Routes.site_path(conn, :index) + + assert_team_membership(user, team, :viewer) + end + + test "admin can't update role of an owner", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + _admin = add_member(team, user: user, role: :admin) + _another_owner = add_member(team, role: :owner) + + conn = put(conn, "/settings/team/memberships/u/#{owner.id}/role/viewer") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "You are not allowed to grant role to that member" + + assert_team_membership(owner, team, :owner) + end + + for role <- Teams.Membership.roles() -- [:owner, :admin] do + test "#{role} can't update role of a member", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + _member = add_member(team, user: user, role: unquote(role)) + another_member = add_member(team, role: :viewer) + + conn = put(conn, "/settings/team/memberships/u/#{another_member.id}/role/editor") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "You are not allowed to grant role to that member" + + assert_team_membership(another_member, team, :viewer) + end + end + end + + describe "DELETE /team/memberships/u/:id" do + test "removes a member from a team", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + collaborator = add_member(team, role: :editor) + assert_team_membership(collaborator, team, :editor) + + conn = delete(conn, "/settings/team/memberships/u/#{collaborator.id}") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :success) == + "User has been removed from the team" + + refute_team_member(collaborator, team) + + assert_email_delivered_with( + to: [nil: collaborator.email], + subject: @subject_prefix <> "Your access to \"#{team.name}\" team has been revoked" + ) + end + + test "can't remove the only owner", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + + conn = delete(conn, "/settings/team/memberships/u/#{user.id}") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "User is the only owner and can't be changed" + + assert_team_membership(user, team, :owner) + end + + test "can remove self (the owner) when there's more than one owner", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + _another_owner = add_member(team, role: :owner) + + conn = delete(conn, "/settings/team/memberships/u/#{user.id}") + + assert redirected_to(conn, 302) == Routes.site_path(conn, :index) + + assert Phoenix.Flash.get(conn.assigns.flash, :success) == + "User has been removed from the team" + + refute_team_member(user, team) + end + + test "can remove another owner when there's more than one owner", %{conn: conn, user: user} do + _site = new_site(owner: user) + team = team_of(user) + another_owner = add_member(team, role: :owner) + + conn = delete(conn, "/settings/team/memberships/u/#{another_owner.id}") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :success) == + "User has been removed from the team" + + refute_team_member(another_owner, team) + end + + test "admin can't remove owner", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + _another_owner = add_member(team, role: :owner) + _admin = add_member(team, user: user, role: :admin) + + conn = delete(conn, "/settings/team/memberships/u/#{owner.id}") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "You are not allowed to remove that member" + + assert_team_membership(owner, team, :owner) + end + + for role <- Teams.Membership.roles() -- [:owner, :admin] do + test "#{role} can't update role of a member", %{conn: conn, user: user} do + owner = new_user() + _site = new_site(owner: owner) + team = team_of(owner) + _member = add_member(team, user: user, role: unquote(role)) + another_member = add_member(team, role: :viewer) + + conn = delete(conn, "/settings/team/memberships/u/#{another_member.id}") + + assert redirected_to(conn, 302) == Routes.settings_path(conn, :team_general) + + assert Phoenix.Flash.get(conn.assigns.flash, :error) == + "You are not allowed to remove that member" + + assert_team_membership(another_member, team, :viewer) + end + end + end +end diff --git a/test/plausible_web/live/sites_test.exs b/test/plausible_web/live/sites_test.exs index 1038da2ea48d..2aacfd45f2fa 100644 --- a/test/plausible_web/live/sites_test.exs +++ b/test/plausible_web/live/sites_test.exs @@ -16,6 +16,47 @@ defmodule PlausibleWeb.Live.SitesTest do assert text(html) =~ "You don't have any sites yet" end + test "renders team invitations", %{user: user, conn: conn} do + owner1 = new_user(name: "G.I. Joe") + new_site(owner: owner1) + team1 = team_of(owner1) + + owner2 = new_user(name: "G.I. Jane") + new_site(owner: owner2) + team2 = team_of(owner2) + + invitation1 = invite_member(team1, user, inviter: owner1, role: :viewer) + invitation2 = invite_member(team2, user, inviter: owner2, role: :editor) + + {:ok, _lv, html} = live(conn, "/sites") + + assert text_of_element(html, "#invitation-#{invitation1.invitation_id}") =~ + "G.I. Joe has invited you to join the \"My Team\" as viewer member." + + assert text_of_element(html, "#invitation-#{invitation2.invitation_id}") =~ + "G.I. Jane has invited you to join the \"My Team\" as editor member." + + assert find( + html, + "#invitation-#{invitation1.invitation_id} a[href=#{Routes.invitation_path(PlausibleWeb.Endpoint, :accept_invitation, invitation1.invitation_id)}]" + ) + + assert find( + html, + "#invitation-#{invitation1.invitation_id} a[href=#{Routes.invitation_path(PlausibleWeb.Endpoint, :reject_invitation, invitation1.invitation_id)}]" + ) + + assert find( + html, + "#invitation-#{invitation2.invitation_id} a[href=#{Routes.invitation_path(PlausibleWeb.Endpoint, :accept_invitation, invitation2.invitation_id)}]" + ) + + assert find( + html, + "#invitation-#{invitation2.invitation_id} a[href=#{Routes.invitation_path(PlausibleWeb.Endpoint, :reject_invitation, invitation2.invitation_id)}]" + ) + end + test "renders metadata for invitation", %{ conn: conn, user: user diff --git a/test/support/teams/test.ex b/test/support/teams/test.ex index 3e744fbdb965..3ac8c6d553c4 100644 --- a/test/support/teams/test.ex +++ b/test/support/teams/test.ex @@ -291,6 +291,13 @@ defmodule Plausible.Teams.Test do membership end + def refute_team_member(user, team) do + refute Repo.get_by(Teams.Membership, + team_id: team.id, + user_id: user.id + ) + end + def assert_team_attached(site, team_id \\ nil) do assert site = %{team: team} = site |> Repo.reload!() |> Repo.preload([:team, :owner])