Add email-change permission validation for linked members
All checks were successful
continuous-integration/drone/push Build is passing
All checks were successful
continuous-integration/drone/push Build is passing
Only admins or the linked user may change a linked member's email. - New validation EmailChangePermission (uses Actor.admin?, Loader.get_linked_user). - Register on Member update_member; docs and gettext.
This commit is contained in:
parent
ad02f8914f
commit
4ea31f0f37
7 changed files with 324 additions and 28 deletions
|
|
@ -4,6 +4,7 @@
|
||||||
2. **DB constraints** - Prevent duplicates within same table (users.email, members.email)
|
2. **DB constraints** - Prevent duplicates within same table (users.email, members.email)
|
||||||
3. **Custom validations** - Prevent cross-table conflicts only for linked entities
|
3. **Custom validations** - Prevent cross-table conflicts only for linked entities
|
||||||
4. **Sync is bidirectional**: User ↔ Member (but User always wins on link)
|
4. **Sync is bidirectional**: User ↔ Member (but User always wins on link)
|
||||||
|
5. **Linked member email change** - When a member is linked, only administrators or the linked user may change that member's email (Member resource validation `EmailChangePermission`). This keeps email sync under control and prevents non-admins from changing another user's linked member email.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -25,6 +25,7 @@ defmodule Mv.Membership.Member do
|
||||||
- Postal code format: exactly 5 digits (German format)
|
- Postal code format: exactly 5 digits (German format)
|
||||||
- Date validations: join_date not in future, exit_date after join_date
|
- Date validations: join_date not in future, exit_date after join_date
|
||||||
- Email uniqueness: prevents conflicts with unlinked users
|
- Email uniqueness: prevents conflicts with unlinked users
|
||||||
|
- Linked member email change: only admins or the linked user may change a linked member's email (see `Mv.Membership.Member.Validations.EmailChangePermission`)
|
||||||
|
|
||||||
## Full-Text Search
|
## Full-Text Search
|
||||||
Members have a `search_vector` attribute (tsvector) that is automatically
|
Members have a `search_vector` attribute (tsvector) that is automatically
|
||||||
|
|
@ -381,6 +382,9 @@ defmodule Mv.Membership.Member do
|
||||||
# Validates that member email is not already used by another (unlinked) user
|
# Validates that member email is not already used by another (unlinked) user
|
||||||
validate Mv.Membership.Member.Validations.EmailNotUsedByOtherUser
|
validate Mv.Membership.Member.Validations.EmailNotUsedByOtherUser
|
||||||
|
|
||||||
|
# Only admins or the linked user may change a linked member's email (prevents breaking sync)
|
||||||
|
validate Mv.Membership.Member.Validations.EmailChangePermission, on: [:update]
|
||||||
|
|
||||||
# Prevent linking to a user that already has a member
|
# Prevent linking to a user that already has a member
|
||||||
# This validation prevents "stealing" users from other members by checking
|
# This validation prevents "stealing" users from other members by checking
|
||||||
# if the target user is already linked to a different member
|
# if the target user is already linked to a different member
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,69 @@
|
||||||
|
defmodule Mv.Membership.Member.Validations.EmailChangePermission do
|
||||||
|
@moduledoc """
|
||||||
|
Validates that only admins or the linked user may change a linked member's email.
|
||||||
|
|
||||||
|
This validation runs on member update when the email attribute is changing.
|
||||||
|
It allows the change only if:
|
||||||
|
- The member is not linked to a user, or
|
||||||
|
- The actor has the admin permission set (via `Mv.Authorization.Actor.admin?/1`), or
|
||||||
|
- The actor is the user linked to this member (actor.member_id == member.id).
|
||||||
|
|
||||||
|
This prevents non-admins from changing another user's linked member email,
|
||||||
|
which would sync to that user's account and break email synchronization.
|
||||||
|
|
||||||
|
No system-actor fallback: missing actor is treated as not allowed.
|
||||||
|
"""
|
||||||
|
use Ash.Resource.Validation
|
||||||
|
use Gettext, backend: MvWeb.Gettext, otp_app: :mv
|
||||||
|
|
||||||
|
alias Mv.Authorization.Actor
|
||||||
|
alias Mv.EmailSync.Loader
|
||||||
|
|
||||||
|
@doc """
|
||||||
|
Validates that the actor may change the member's email when the member is linked.
|
||||||
|
|
||||||
|
Only runs when the email attribute is changing (checked inside). Skips when
|
||||||
|
member is not linked. Allows when actor is admin or owns the linked member.
|
||||||
|
"""
|
||||||
|
@impl true
|
||||||
|
def validate(changeset, _opts, context) do
|
||||||
|
if Ash.Changeset.changing_attribute?(changeset, :email) do
|
||||||
|
validate_linked_member_email_change(changeset, context)
|
||||||
|
else
|
||||||
|
:ok
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp validate_linked_member_email_change(changeset, context) do
|
||||||
|
linked_user = Loader.get_linked_user(changeset.data)
|
||||||
|
|
||||||
|
if is_nil(linked_user) do
|
||||||
|
:ok
|
||||||
|
else
|
||||||
|
actor = resolve_actor(changeset, context)
|
||||||
|
member_id = changeset.data.id
|
||||||
|
|
||||||
|
if Actor.admin?(actor) or actor_owns_member?(actor, member_id) do
|
||||||
|
:ok
|
||||||
|
else
|
||||||
|
msg =
|
||||||
|
dgettext("default", "Only administrators can change email for members linked to users")
|
||||||
|
|
||||||
|
{:error, field: :email, message: msg}
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
# Ash stores actor in changeset.context.private.actor; validation context also has .actor
|
||||||
|
defp resolve_actor(changeset, context) do
|
||||||
|
get_in(changeset.context || %{}, [:private, :actor]) ||
|
||||||
|
(context && Map.get(context, :actor))
|
||||||
|
end
|
||||||
|
|
||||||
|
defp actor_owns_member?(nil, _member_id), do: false
|
||||||
|
|
||||||
|
defp actor_owns_member?(actor, member_id) do
|
||||||
|
actor_member_id = Map.get(actor, :member_id) || Map.get(actor, "member_id")
|
||||||
|
actor_member_id == member_id
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
@ -2298,17 +2298,7 @@ msgstr "Verwende die Namen der Datenfelder als Spaltennamen in der CSV Datei. Da
|
||||||
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
||||||
msgstr "Unbekannte Spalte '%{header}' wird ignoriert. Falls dies ein Datenfeld ist, erstellen Sie es in Mila vor dem Import."
|
msgstr "Unbekannte Spalte '%{header}' wird ignoriert. Falls dies ein Datenfeld ist, erstellen Sie es in Mila vor dem Import."
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
#: lib/mv/membership/member/validations/email_change_permission.ex
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
#, elixir-autogen, elixir-format
|
||||||
#~ msgid "Custom Fields in CSV Import"
|
msgid "Only administrators can change email for members linked to users"
|
||||||
#~ msgstr "Benutzerdefinierte Felder"
|
msgstr "Nur Administrator*innen können die E-Mail von Mitgliedern ändern, die mit Benutzer*innen verknüpft sind."
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
|
||||||
#~ msgid "Individual data fields must be created in Mila before importing. Use the field name as the CSV column header. Unknown custom field columns will be ignored with a warning."
|
|
||||||
#~ msgstr "Individuelle Datenfelder müssen in Mila erstellt werden, bevor sie importiert werden können. Verwenden Sie den Namen des Datenfeldes als CSV-Spaltenüberschrift. Unbekannte Spaltenüberschriften werden mit einer Warnung ignoriert."
|
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format
|
|
||||||
#~ msgid "Manage Custom Fields"
|
|
||||||
#~ msgstr "Benutzerdefinierte Felder verwalten"
|
|
||||||
|
|
|
||||||
|
|
@ -2298,3 +2298,8 @@ msgstr ""
|
||||||
#, elixir-autogen, elixir-format
|
#, elixir-autogen, elixir-format
|
||||||
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
||||||
msgstr ""
|
msgstr ""
|
||||||
|
|
||||||
|
#: lib/mv/membership/member/validations/email_change_permission.ex
|
||||||
|
#, elixir-autogen, elixir-format
|
||||||
|
msgid "Only administrators can change email for members linked to users"
|
||||||
|
msgstr ""
|
||||||
|
|
|
||||||
|
|
@ -2299,17 +2299,7 @@ msgstr ""
|
||||||
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
msgid "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
||||||
msgstr "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
msgstr "Unknown column '%{header}' will be ignored. If this is a custom field, create it in Mila before importing."
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
#: lib/mv/membership/member/validations/email_change_permission.ex
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
#, elixir-autogen, elixir-format
|
||||||
#~ msgid "Custom Fields in CSV Import"
|
msgid "Only administrators can change email for members linked to users"
|
||||||
#~ msgstr ""
|
msgstr "Only administrators can change email for members linked to users"
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format, fuzzy
|
|
||||||
#~ msgid "Individual data fields must be created in Mila before importing. Use the field name as the CSV column header. Unknown custom field columns will be ignored with a warning."
|
|
||||||
#~ msgstr ""
|
|
||||||
|
|
||||||
#~ #: lib/mv_web/live/global_settings_live.ex
|
|
||||||
#~ #, elixir-autogen, elixir-format
|
|
||||||
#~ msgid "Manage Custom Fields"
|
|
||||||
#~ msgstr ""
|
|
||||||
|
|
|
||||||
237
test/mv/membership/member_email_validation_test.exs
Normal file
237
test/mv/membership/member_email_validation_test.exs
Normal file
|
|
@ -0,0 +1,237 @@
|
||||||
|
defmodule Mv.Membership.MemberEmailValidationTest do
|
||||||
|
@moduledoc """
|
||||||
|
Tests for Member email-change permission validation.
|
||||||
|
|
||||||
|
When a member is linked to a user, only admins or the linked user may change
|
||||||
|
that member's email. Unlinked members and non-email updates are unaffected.
|
||||||
|
"""
|
||||||
|
use Mv.DataCase, async: false
|
||||||
|
|
||||||
|
alias Mv.Accounts
|
||||||
|
alias Mv.Authorization
|
||||||
|
alias Mv.Helpers.SystemActor
|
||||||
|
alias Mv.Membership
|
||||||
|
|
||||||
|
setup do
|
||||||
|
system_actor = SystemActor.get_system_actor()
|
||||||
|
%{actor: system_actor}
|
||||||
|
end
|
||||||
|
|
||||||
|
defp create_role_with_permission_set(permission_set_name, actor) do
|
||||||
|
role_name = "Test Role #{permission_set_name} #{System.unique_integer([:positive])}"
|
||||||
|
|
||||||
|
case Authorization.create_role(
|
||||||
|
%{
|
||||||
|
name: role_name,
|
||||||
|
description: "Test role for #{permission_set_name}",
|
||||||
|
permission_set_name: permission_set_name
|
||||||
|
},
|
||||||
|
actor: actor
|
||||||
|
) do
|
||||||
|
{:ok, role} -> role
|
||||||
|
{:error, error} -> raise "Failed to create role: #{inspect(error)}"
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
defp create_user_with_permission_set(permission_set_name, actor) do
|
||||||
|
role = create_role_with_permission_set(permission_set_name, actor)
|
||||||
|
|
||||||
|
{:ok, user} =
|
||||||
|
Accounts.User
|
||||||
|
|> Ash.Changeset.for_create(:register_with_password, %{
|
||||||
|
email: "user#{System.unique_integer([:positive])}@example.com",
|
||||||
|
password: "testpassword123"
|
||||||
|
})
|
||||||
|
|> Ash.create(actor: actor)
|
||||||
|
|
||||||
|
{:ok, user} =
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.manage_relationship(:role, role, type: :append_and_remove)
|
||||||
|
|> Ash.update(actor: actor)
|
||||||
|
|
||||||
|
{:ok, user_with_role} = Ash.load(user, :role, domain: Mv.Accounts, actor: actor)
|
||||||
|
user_with_role
|
||||||
|
end
|
||||||
|
|
||||||
|
defp create_admin_user(actor) do
|
||||||
|
create_user_with_permission_set("admin", actor)
|
||||||
|
end
|
||||||
|
|
||||||
|
defp create_linked_member_for_user(user, actor) do
|
||||||
|
admin = create_admin_user(actor)
|
||||||
|
|
||||||
|
{:ok, member} =
|
||||||
|
Membership.create_member(
|
||||||
|
%{
|
||||||
|
first_name: "Linked",
|
||||||
|
last_name: "Member",
|
||||||
|
email: "linked#{System.unique_integer([:positive])}@example.com"
|
||||||
|
},
|
||||||
|
actor: admin
|
||||||
|
)
|
||||||
|
|
||||||
|
user
|
||||||
|
|> Ash.Changeset.for_update(:update, %{})
|
||||||
|
|> Ash.Changeset.force_change_attribute(:member_id, member.id)
|
||||||
|
|> Ash.update(actor: admin, domain: Mv.Accounts, return_notifications?: false)
|
||||||
|
|
||||||
|
member
|
||||||
|
end
|
||||||
|
|
||||||
|
defp create_unlinked_member(actor) do
|
||||||
|
admin = create_admin_user(actor)
|
||||||
|
|
||||||
|
{:ok, member} =
|
||||||
|
Membership.create_member(
|
||||||
|
%{
|
||||||
|
first_name: "Unlinked",
|
||||||
|
last_name: "Member",
|
||||||
|
email: "unlinked#{System.unique_integer([:positive])}@example.com"
|
||||||
|
},
|
||||||
|
actor: admin
|
||||||
|
)
|
||||||
|
|
||||||
|
member
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "unlinked member" do
|
||||||
|
test "normal_user can update email of unlinked member", %{actor: actor} do
|
||||||
|
normal_user = create_user_with_permission_set("normal_user", actor)
|
||||||
|
unlinked_member = create_unlinked_member(actor)
|
||||||
|
|
||||||
|
new_email = "new#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(unlinked_member, %{email: new_email}, actor: normal_user)
|
||||||
|
|
||||||
|
assert updated.email == new_email
|
||||||
|
end
|
||||||
|
|
||||||
|
test "validation does not block when member has no linked user", %{actor: actor} do
|
||||||
|
normal_user = create_user_with_permission_set("normal_user", actor)
|
||||||
|
unlinked_member = create_unlinked_member(actor)
|
||||||
|
|
||||||
|
new_email = "other#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, _} =
|
||||||
|
Membership.update_member(unlinked_member, %{email: new_email}, actor: normal_user)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "linked member – another user's member" do
|
||||||
|
test "normal_user cannot update email of another user's linked member", %{actor: actor} do
|
||||||
|
user_a = create_user_with_permission_set("own_data", actor)
|
||||||
|
linked_member = create_linked_member_for_user(user_a, actor)
|
||||||
|
|
||||||
|
normal_user_b = create_user_with_permission_set("normal_user", actor)
|
||||||
|
new_email = "other#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:error, %Ash.Error.Invalid{} = error} =
|
||||||
|
Membership.update_member(linked_member, %{email: new_email}, actor: normal_user_b)
|
||||||
|
|
||||||
|
error_str = Exception.message(error)
|
||||||
|
assert error_str =~ "administrators"
|
||||||
|
assert error_str =~ "linked to users"
|
||||||
|
end
|
||||||
|
|
||||||
|
test "admin can update email of linked member", %{actor: actor} do
|
||||||
|
user_a = create_user_with_permission_set("own_data", actor)
|
||||||
|
linked_member = create_linked_member_for_user(user_a, actor)
|
||||||
|
admin = create_admin_user(actor)
|
||||||
|
|
||||||
|
new_email = "admin_changed#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(linked_member, %{email: new_email}, actor: admin)
|
||||||
|
|
||||||
|
assert updated.email == new_email
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "linked member – own member" do
|
||||||
|
test "own_data user can update email of their own linked member", %{actor: actor} do
|
||||||
|
own_data_user = create_user_with_permission_set("own_data", actor)
|
||||||
|
linked_member = create_linked_member_for_user(own_data_user, actor)
|
||||||
|
|
||||||
|
{:ok, own_data_user} =
|
||||||
|
Ash.get(Accounts.User, own_data_user.id, domain: Mv.Accounts, load: [:role], actor: actor)
|
||||||
|
|
||||||
|
{:ok, own_data_user} =
|
||||||
|
Ash.load(own_data_user, :member, domain: Mv.Accounts, actor: actor)
|
||||||
|
|
||||||
|
new_email = "own_updated#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(linked_member, %{email: new_email}, actor: own_data_user)
|
||||||
|
|
||||||
|
assert updated.email == new_email
|
||||||
|
end
|
||||||
|
|
||||||
|
test "normal_user with linked member can update email of that same member", %{actor: actor} do
|
||||||
|
normal_user = create_user_with_permission_set("normal_user", actor)
|
||||||
|
linked_member = create_linked_member_for_user(normal_user, actor)
|
||||||
|
|
||||||
|
{:ok, normal_user} =
|
||||||
|
Ash.get(Accounts.User, normal_user.id, domain: Mv.Accounts, load: [:role], actor: actor)
|
||||||
|
|
||||||
|
{:ok, normal_user} = Ash.load(normal_user, :member, domain: Mv.Accounts, actor: actor)
|
||||||
|
|
||||||
|
new_email = "normal_own#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(linked_member, %{email: new_email}, actor: normal_user)
|
||||||
|
|
||||||
|
assert updated.email == new_email
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "no-op / other fields" do
|
||||||
|
test "updating only other attributes on linked member as normal_user does not trigger validation error",
|
||||||
|
%{actor: actor} do
|
||||||
|
user_a = create_user_with_permission_set("own_data", actor)
|
||||||
|
linked_member = create_linked_member_for_user(user_a, actor)
|
||||||
|
normal_user_b = create_user_with_permission_set("normal_user", actor)
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(linked_member, %{first_name: "UpdatedName"},
|
||||||
|
actor: normal_user_b
|
||||||
|
)
|
||||||
|
|
||||||
|
assert updated.first_name == "UpdatedName"
|
||||||
|
assert updated.email == linked_member.email
|
||||||
|
end
|
||||||
|
|
||||||
|
test "updating email of linked member as admin succeeds", %{actor: actor} do
|
||||||
|
user_a = create_user_with_permission_set("own_data", actor)
|
||||||
|
linked_member = create_linked_member_for_user(user_a, actor)
|
||||||
|
admin = create_admin_user(actor)
|
||||||
|
|
||||||
|
new_email = "admin_ok#{System.unique_integer([:positive])}@example.com"
|
||||||
|
|
||||||
|
assert {:ok, updated} =
|
||||||
|
Membership.update_member(linked_member, %{email: new_email}, actor: admin)
|
||||||
|
|
||||||
|
assert updated.email == new_email
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
describe "read_only" do
|
||||||
|
test "read_only cannot update any member (policy rejects before validation)", %{actor: actor} do
|
||||||
|
read_only_user = create_user_with_permission_set("read_only", actor)
|
||||||
|
linked_member = create_linked_member_for_user(read_only_user, actor)
|
||||||
|
|
||||||
|
{:ok, read_only_user} =
|
||||||
|
Ash.get(Accounts.User, read_only_user.id,
|
||||||
|
domain: Mv.Accounts,
|
||||||
|
load: [:role],
|
||||||
|
actor: actor
|
||||||
|
)
|
||||||
|
|
||||||
|
assert {:error, %Ash.Error.Forbidden{}} =
|
||||||
|
Membership.update_member(linked_member, %{email: "changed@example.com"},
|
||||||
|
actor: read_only_user
|
||||||
|
)
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
Loading…
Add table
Add a link
Reference in a new issue