Files
sure/app/controllers/settings/profiles_controller.rb
dripsmvcp ab52b2b144 fix(family-sharing): prevent silent data loss when rehoming or removing users (#1896)
* fix(family-sharing): prevent silent data loss when rehoming or removing users

Fixes #1689.

Two destructive paths could strand a pre-existing user's family and accounts:

1. Invitation#accept_for unconditionally overwrote user.family_id, orphaning
   the prior family + its accounts with no user able to reach them.
2. Settings::ProfilesController#destroy then called @user.destroy when an admin
   removed the rehomed member, destroying the only login path back to the
   now-orphaned data.

Add hard-block guards on both paths. accept_for refuses when the invitee
already belongs to a family with accounts; ProfilesController#destroy refuses
when the member owns accounts in another family (legacy state from the old
flow). InvitationsController#create surfaces a specific, actionable flash so
the admin understands why the auto-accept was refused.

No automatic recovery of already-orphaned data — that needs a separate
one-shot script per dosubot's analysis on the issue.

* fix(family-sharing): scope invite orphan-guard to invitee-owned accounts (#1896 review)

Codex flagged (P1) and the maintainer review independently raised that
would_orphan_existing_family? keyed off user.family.accounts.exists? —
any account in the invitee's current family — which wrongly blocked a
non-owner member from leaving a multi-user household.

Rename to would_orphan_owned_accounts? and key off
user.owned_accounts.where.not(family_id: family_id), making the invite
guard symmetric with the destroy-path guard in
Settings::ProfilesController. A member who owns no accounts now orphans
nothing by moving and is free to accept the invitation; an owner is
still blocked.

Add a regression test for the non-owner case and update the existing
tests to give the invitee explicit account ownership.

* Remove extra comments per project conventions

---------

Co-authored-by: Juan José Mata <jjmata@jjmata.com>
2026-05-27 23:25:46 +02:00

52 lines
1.4 KiB
Ruby

class Settings::ProfilesController < ApplicationController
layout :layout_for_settings_profile
def show
@user = Current.user
@users = Current.family.users.order(:created_at)
@pending_invitations = Current.family.invitations.pending
@breadcrumbs = [
[ t("breadcrumbs.home"), root_path ],
[ t("breadcrumbs.profile"), nil ]
]
end
def destroy
unless Current.user.admin?
flash[:alert] = t("settings.profiles.destroy.not_authorized")
redirect_to settings_profile_path
return
end
@user = Current.family.users.find(params[:user_id])
if @user == Current.user
flash[:alert] = t("settings.profiles.destroy.cannot_remove_self")
redirect_to settings_profile_path
return
end
if @user.owned_accounts.where.not(family_id: Current.family.id).exists?
flash[:alert] = t(".member_owns_other_family_data")
redirect_to settings_profile_path
return
end
if @user.destroy
# Also destroy the invitation associated with this user for this family
Current.family.invitations.find_by(email: @user.email)&.destroy
flash[:notice] = t(".member_removed")
else
flash[:alert] = t(".member_removal_failed")
end
redirect_to settings_profile_path
end
private
def layout_for_settings_profile
Current.user&.ui_layout_intro? ? "application" : "settings"
end
end