Merge pull request #89979 from software-mansion-labs/jakubstec/domains/validate-moving-users-between-groups

mountiny · web-flow · commit af06d133beeb · 2026-05-13T10:28:42.000+02:00
[Domain Control] add validation in `MoveUsersBetweenGroupsPage` for Preferred Policy admin restriction
diff --git a/src/languages/de.ts b/src/languages/de.ts
@@ -9225,6 +9225,8 @@ Hier ist ein *Testbeleg*, um dir zu zeigen, wie es funktioniert:`,
                 addMember: 'Dieses Mitglied kann nicht hinzugefügt werden. Bitte versuche es erneut.',
                 vacationDelegate: 'Dieser Benutzer kann nicht als Urlaubsvertretung festgelegt werden. Bitte versuche es erneut.',
                 moveMember: 'Dieses Mitglied kann nicht verschoben werden. Bitte versuchen Sie es erneut.',
+                moveMemberNotPolicyAdmin:
+                    'Das Mitglied kann nicht in die Domänengruppe verschoben werden. Sie müssen Richtlinienadministrator für die bevorzugte Richtlinie sein, die für die Domänengruppe festgelegt ist, in die Sie diesen Benutzer verschieben möchten.',
             },
             reportSuspiciousActivityPrompt: (email: string) =>
                 `Bist du sicher? Dadurch wird das Konto von <strong>${email}</strong> gesperrt. <br /><br /> Unser Team wird das Konto anschließend überprüfen und unbefugten Zugriff entfernen. Um den Zugriff wiederherzustellen, muss die Person mit Concierge zusammenarbeiten.`,
diff --git a/src/languages/en.ts b/src/languages/en.ts
@@ -9230,6 +9230,8 @@ const translations = {
                 removeMember: 'Unable to remove this user. Please try again.',
                 moveMember: 'Unable to move this member. Please try again.',
                 vacationDelegate: 'Unable to set this user as a vacation delegate. Please try again.',
+                moveMemberNotPolicyAdmin:
+                    'Cannot move member to the domain group. You must be a Policy Admin for the Preferred Policy set on the domain group you are trying to move this user to.',
             },
             cannotSetVacationDelegateForMember: (email: string) => `You can't set a vacation delegate for ${email} because they're currently the delegate for the following members:`,
 
diff --git a/src/languages/es.ts b/src/languages/es.ts
@@ -9418,6 +9418,8 @@ ${amount} para ${merchant} - ${date}`,
                 addMember: 'No se pudo añadir este miembro. Por favor, inténtalo de nuevo.',
                 vacationDelegate: 'No se pudo establecer a este usuario como delegado de vacaciones. Por favor, inténtalo de nuevo.',
                 moveMember: 'No se pudo mover este miembro. Por favor, inténtalo de nuevo.',
+                moveMemberNotPolicyAdmin:
+                    'No se puede mover al miembro al grupo de dominio. Debes ser Administrador de Políticas para la Política Preferida establecida en el grupo de dominio al que intentas mover a este usuario.',
             },
             cannotSetVacationDelegateForMember: (email: string) =>
                 `No puedes establecer un delegado de vacaciones para ${email} porque actualmente es el delegado de los siguientes miembros:`,
diff --git a/src/languages/fr.ts b/src/languages/fr.ts
@@ -9246,6 +9246,8 @@ Voici un *reçu test* pour vous montrer comment ça fonctionne :`,
                 addMember: 'Impossible d’ajouter ce membre. Veuillez réessayer.',
                 vacationDelegate: 'Impossible de définir cet utilisateur comme délégué de vacances. Veuillez réessayer.',
                 moveMember: 'Impossible de déplacer ce membre. Veuillez réessayer.',
+                moveMemberNotPolicyAdmin:
+                    'Impossible de déplacer le membre vers le groupe de domaine. Vous devez être Administrateur de Politique pour la Politique Préférée définie sur le groupe de domaine vers lequel vous essayez de déplacer cet utilisateur.',
             },
             cannotSetVacationDelegateForMember: (email: string) =>
                 `Vous ne pouvez pas définir un délégué de vacances pour ${email}, car cette personne est actuellement le délégué des membres suivants :`,
diff --git a/src/languages/it.ts b/src/languages/it.ts
@@ -9217,6 +9217,8 @@ Ecco una *ricevuta di prova* per mostrarti come funziona:`,
                 addMember: 'Impossibile aggiungere questo membro. Riprova.',
                 vacationDelegate: 'Impossibile impostare questo utente come delegato per le ferie. Riprova.',
                 moveMember: 'Impossibile spostare questo membro. Riprova.',
+                moveMemberNotPolicyAdmin:
+                    'Impossibile spostare il membro nel gruppo di dominio. Devi essere Amministratore di Criteri per il Criterio Preferito impostato nel gruppo di dominio in cui stai cercando di spostare questo utente.',
             },
             cannotSetVacationDelegateForMember: (email: string) => `Non puoi impostare un delegato per le vacanze per ${email} perché al momento è il delegato per i seguenti membri:`,
             reportSuspiciousActivityPrompt: (email: string) =>
diff --git a/src/languages/ja.ts b/src/languages/ja.ts
@@ -9100,6 +9100,8 @@ ${reportName}
                 addMember: 'このメンバーを追加できませんでした。もう一度お試しください。',
                 vacationDelegate: 'このユーザーを休暇代理人として設定できませんでした。もう一度お試しください。',
                 moveMember: 'このメンバーを移動できませんでした。もう一度お試しください。',
+                moveMemberNotPolicyAdmin:
+                    'メンバーをドメイングループに移動できません。このユーザーを移動しようとしているドメイングループに設定された優先ポリシーのポリシー管理者である必要があります。',
             },
             cannotSetVacationDelegateForMember: (email: string) => `${email} に休暇代理人を設定できません。現在、このユーザーは次のメンバーの代理人になっています。`,
             reportSuspiciousActivityPrompt: (email: string) =>
diff --git a/src/languages/nl.ts b/src/languages/nl.ts
@@ -9184,6 +9184,8 @@ Hier is een *proefbon* om je te laten zien hoe het werkt:`,
                 addMember: 'Kan dit lid niet toevoegen. Probeer het opnieuw.',
                 vacationDelegate: 'Kan deze gebruiker niet als vakantiemandataris instellen. Probeer het opnieuw.',
                 moveMember: 'Kan dit lid niet verplaatsen. Probeer het opnieuw.',
+                moveMemberNotPolicyAdmin:
+                    'Kan het lid niet naar de domeingroep verplaatsen. U moet Beleidsbeheerder zijn voor het Voorkeursbeleid dat is ingesteld op de domeingroep waarnaar u deze gebruiker probeert te verplaatsen.',
             },
             cannotSetVacationDelegateForMember: (email: string) =>
                 `Je kunt geen vakantiemandataris instellen voor ${email} omdat die persoon momenteel gedelegeerde is voor de volgende leden:`,
diff --git a/src/languages/pl.ts b/src/languages/pl.ts
@@ -9169,6 +9169,8 @@ Oto *paragon testowy*, żeby pokazać Ci, jak to działa:`,
                 addMember: 'Nie można dodać tego członka. Spróbuj ponownie.',
                 vacationDelegate: 'Nie można ustawić tego użytkownika jako zastępującego na czas nieobecności. Spróbuj ponownie.',
                 moveMember: 'Nie można przenieść tego członka. Spróbuj ponownie.',
+                moveMemberNotPolicyAdmin:
+                    'Nie można przenieść członka do grupy domeny. Musisz być Administratorem Zasad dla Preferowanych Zasad ustawionych dla grupy domeny, do której próbujesz przenieść tego użytkownika.',
             },
             cannotSetVacationDelegateForMember: (email: string) => `Nie możesz ustawić zastępstwa urlopowego dla ${email}, ponieważ jest on/ona obecnie zastępcą dla następujących członków:`,
             reportSuspiciousActivityPrompt: (email: string) =>
diff --git a/src/languages/pt-BR.ts b/src/languages/pt-BR.ts
@@ -9177,6 +9177,8 @@ Aqui está um *comprovante de teste* para mostrar como funciona:`,
                 addMember: 'Não foi possível adicionar este membro. Tente novamente.',
                 vacationDelegate: 'Não foi possível definir este usuário como delegado de férias. Tente novamente.',
                 moveMember: 'Não foi possível mover este membro. Tente novamente.',
+                moveMemberNotPolicyAdmin:
+                    'Não é possível mover o membro para o grupo de domínio. Você deve ser Administrador de Política para a Política Preferencial definida no grupo de domínio para o qual está tentando mover este usuário.',
             },
             cannotSetVacationDelegateForMember: (email: string) => `Você não pode definir um procurador de férias para ${email} porque esta pessoa já é procuradora dos seguintes membros:`,
             reportSuspiciousActivityPrompt: (email: string) =>
diff --git a/src/languages/zh-hans.ts b/src/languages/zh-hans.ts
@@ -8949,6 +8949,7 @@ ${reportName}
                 addMember: '无法添加此成员。请重试。',
                 vacationDelegate: '无法将此用户设置为休假代理人。请重试。',
                 moveMember: '无法移动此成员。请重试。',
+                moveMemberNotPolicyAdmin: '无法将成员移动到域组。您必须是您尝试将此用户移动到的域组上设置的首选策略的策略管理员。',
             },
             cannotSetVacationDelegateForMember: (email: string) => `您无法为 ${email} 设置休假代理人，因为 TA 当前是以下成员的代理人：`,
             reportSuspiciousActivityPrompt: (email: string) =>
diff --git a/src/libs/actions/Domain.ts b/src/libs/actions/Domain.ts
@@ -2083,6 +2083,7 @@ function setDefaultSecurityGroup(domainAccountID: number, groupID: string, previ
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`,
+            // backend API uses snake_case for domain_defaultSecurityGroupID
             // eslint-disable-next-line @typescript-eslint/naming-convention
             value: {domain_defaultSecurityGroupID: groupID},
         },
@@ -2110,6 +2111,7 @@ function setDefaultSecurityGroup(domainAccountID: number, groupID: string, previ
         {
             onyxMethod: Onyx.METHOD.MERGE,
             key: `${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`,
+            // backend API uses snake_case for domain_defaultSecurityGroupID
             // eslint-disable-next-line @typescript-eslint/naming-convention
             value: {domain_defaultSecurityGroupID: previousGroupID},
         },
diff --git a/src/pages/domain/Members/MoveUserBetweenGroupsPage.tsx b/src/pages/domain/Members/MoveUserBetweenGroupsPage.tsx
@@ -23,6 +23,7 @@ import ROUTES from '@src/ROUTES';
 import type SCREENS from '@src/SCREENS';
 import {personalDetailsLoginSelector} from '@src/selectors/PersonalDetails';
 import type {Domain} from '@src/types/onyx';
+import useDomainGroupMoveValidation from './useDomainGroupMoveValidation';
 
 type SecurityGroupItem = ListItem & {
     value: string;
@@ -34,8 +35,8 @@ function MoveUserBetweenGroupsPage({route}: MoveUserBetweenGroupsPageProps) {
     const {domainAccountID, accountID} = route.params;
     const styles = useThemeStyles();
     const {translate} = useLocalize();
-
     const [selectedGroupId, setSelectedGroupId] = useState<string | undefined>();
+    const {isBlocked, showBlockedModal} = useDomainGroupMoveValidation(domainAccountID, selectedGroupId);
     const [domainName] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: domainNameSelector});
     const [securityGroups] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: groupsSelector});
 
@@ -68,6 +69,11 @@ function MoveUserBetweenGroupsPage({route}: MoveUserBetweenGroupsPageProps) {
             return;
         }
 
+        if (isBlocked) {
+            showBlockedModal();
+            return;
+        }
+
         const newSecurityGroupKey: `${typeof CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}${string}` = `${CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}${selectedGroupId}`;
         changeDomainSecurityGroup(domainAccountID, domainName, memberLogin, accountID, userSecurityGroup.key, userSecurityGroup.securityGroup, newSecurityGroupKey);
         Navigation.goBack(ROUTES.DOMAIN_MEMBER_DETAILS.getRoute(domainAccountID, accountID));
diff --git a/src/pages/domain/Members/MoveUsersBetweenGroupsPage.tsx b/src/pages/domain/Members/MoveUsersBetweenGroupsPage.tsx
@@ -21,6 +21,7 @@ import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
 import type SCREENS from '@src/SCREENS';
+import useDomainGroupMoveValidation from './useDomainGroupMoveValidation';
 
 type SecurityGroupItem = ListItem & {
     value: string;
@@ -32,8 +33,8 @@ function MoveUsersBetweenGroupsPage({route}: MoveUsersBetweenGroupsPageProps) {
     const {domainAccountID} = route.params;
     const {translate} = useLocalize();
     const styles = useThemeStyles();
-
     const [selectedGroupId, setSelectedGroupId] = useState<string | undefined>();
+    const {isBlocked, showBlockedModal} = useDomainGroupMoveValidation(domainAccountID, selectedGroupId);
     const [domainName] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: domainNameSelector});
     const [securityGroups] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: groupsSelector});
     const [selectedMemberAccountIDs] = useOnyx(ONYXKEYS.DOMAIN_MEMBERS_SELECTED_FOR_MOVE);
@@ -56,6 +57,11 @@ function MoveUsersBetweenGroupsPage({route}: MoveUsersBetweenGroupsPageProps) {
             return;
         }
 
+        if (isBlocked) {
+            showBlockedModal();
+            return;
+        }
+
         for (const accountIDString of selectedMemberAccountIDs) {
             const accountID = Number(accountIDString);
             const memberLogin = getLoginByAccountID(accountID);
diff --git a/src/pages/domain/Members/useDomainGroupMoveValidation.ts b/src/pages/domain/Members/useDomainGroupMoveValidation.ts
@@ -0,0 +1,32 @@
+import {selectRestrictedPrimaryPolicyID} from '@selectors/Domain';
+import {isAdminForPolicyByIDSelector} from '@selectors/Policy';
+import useConfirmModal from '@hooks/useConfirmModal';
+import useLocalize from '@hooks/useLocalize';
+import useOnyx from '@hooks/useOnyx';
+import ONYXKEYS from '@src/ONYXKEYS';
+
+function useDomainGroupMoveValidation(domainAccountID: number, targetGroupId: string | undefined) {
+    const {translate} = useLocalize();
+    const {showConfirmModal} = useConfirmModal();
+
+    const [targetPolicyID] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {
+        selector: selectRestrictedPrimaryPolicyID(targetGroupId),
+    });
+
+    const [isAdminForTargetPolicy] = useOnyx(ONYXKEYS.COLLECTION.POLICY, {
+        selector: isAdminForPolicyByIDSelector(targetPolicyID),
+    });
+
+    const showBlockedModal = () => {
+        showConfirmModal({
+            title: translate('workspace.distanceRates.oopsNotSoFast'),
+            prompt: translate('domain.members.error.moveMemberNotPolicyAdmin'),
+            confirmText: translate('common.buttonConfirm'),
+            shouldShowCancelButton: false,
+        });
+    };
+
+    return {isBlocked: !isAdminForTargetPolicy, showBlockedModal};
+}
+
+export default useDomainGroupMoveValidation;
diff --git a/src/selectors/Domain.ts b/src/selectors/Domain.ts
@@ -182,6 +182,16 @@ function groupsSelector(domain: OnyxEntry<Domain>): DomainSecurityGroupWithID[]
 
 const accountLockSelector = (accountID: number) => (domain: OnyxEntry<Domain>) => domain?.[`${CONST.DOMAIN.PRIVATE_LOCKED_ACCOUNT_PREFIX}${accountID}`];
 
+function selectRestrictedPrimaryPolicyID(groupID?: string) {
+    return (domain: OnyxEntry<Domain>): string | undefined => {
+        const targetGroup = groupsSelector(domain)?.find((g) => g.id === groupID);
+        if (!targetGroup?.details.enableRestrictedPrimaryPolicy || !targetGroup?.details.restrictedPrimaryPolicyID) {
+            return undefined;
+        }
+        return targetGroup.details.restrictedPrimaryPolicyID;
+    };
+}
+
 /**
  * Creates a selector that checks if a given account ID is an admin of the domain.
  * It checks whether the account ID appears as a value in any expensify_adminPermissions_* entry.
@@ -225,6 +235,7 @@ function isSecurityGroupPendingDeleteSelector(groupID?: string) {
 
 export {
     domainMemberSettingsSelector,
+    selectRestrictedPrimaryPolicyID,
     domainSettingsPrimaryContactSelector,
     domainSamlSettingsStateSelector,
     domainNameSelector,
diff --git a/src/selectors/Policy.ts b/src/selectors/Policy.ts
@@ -4,6 +4,7 @@ import {hasSynchronizationErrorMessage, isConnectionUnverified} from '@libs/acti
 import {getDisplayNameForWorkspace} from '@libs/actions/Policy/Policy';
 import {getActiveAdminWorkspaces, getOwnedPaidPolicies, isPaidGroupPolicy, shouldShowPolicy} from '@libs/PolicyUtils';
 import CONST from '@src/CONST';
+import ONYXKEYS from '@src/ONYXKEYS';
 import type {Policy, PolicyReportField} from '@src/types/onyx';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 
@@ -214,6 +215,16 @@ function lastWorkspaceNumberSelector(policies: OnyxCollection<Policy>, email: st
 
 const policyNameSelector = (policy: OnyxEntry<Policy>) => policy?.name;
 
+function isAdminForPolicyByIDSelector(policyID?: string) {
+    return (policies: OnyxCollection<Policy> | null): boolean => {
+        if (!policyID) {
+            return true;
+        }
+        const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
+        return !!policy && policy.role === CONST.POLICY.ROLE.ADMIN;
+    };
+}
+
 const createAdminPoliciesSelector =
     (currentPolicyID: string | undefined = undefined) =>
     (policies: OnyxCollection<Policy>) => {
@@ -253,5 +264,6 @@ export {
     hasOnlyPersonalPoliciesSelector,
     policyNameSelector,
     createAdminPoliciesSelector,
+    isAdminForPolicyByIDSelector,
 };
 export type {ReusablePolicyConnectionName};
diff --git a/tests/unit/DomainSelectorsTest.ts b/tests/unit/DomainSelectorsTest.ts
@@ -12,6 +12,7 @@ import {
     isSecurityGroupEntry,
     isSecurityGroupPendingDeleteSelector,
     memberAccountIDsSelector,
+    selectRestrictedPrimaryPolicyID,
     selectSecurityGroupForAccount,
     technicalContactSettingsSelector,
     vacationDelegateSelector,
@@ -814,4 +815,51 @@ describe('domainSelectors', () => {
             expect(accountLockSelector(accountID)(domain)).toBeUndefined();
         });
     });
+
+    describe('selectRestrictedPrimaryPolicyID', () => {
+        const makeGroup = (enableRestrictedPrimaryPolicy: boolean, restrictedPrimaryPolicyID?: string): DomainSecurityGroup =>
+            ({enableRestrictedPrimaryPolicy, restrictedPrimaryPolicyID, shared: {}}) as unknown as DomainSecurityGroup;
+
+        const makeDomain = (groups: Record<string, DomainSecurityGroup>): OnyxEntry<Domain> => {
+            const entries = Object.fromEntries(Object.entries(groups).map(([id, g]) => [`${CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}${id}`, g]));
+            return entries as unknown as Domain;
+        };
+
+        it('returns undefined when domain is undefined', () => {
+            expect(selectRestrictedPrimaryPolicyID('g1')(undefined)).toBeUndefined();
+        });
+
+        it('returns undefined when groupID is undefined', () => {
+            const domain = makeDomain({g1: makeGroup(true, 'p1')});
+            expect(selectRestrictedPrimaryPolicyID(undefined)(domain)).toBeUndefined();
+        });
+
+        it('returns undefined when the group does not exist', () => {
+            const domain = makeDomain({g1: makeGroup(true, 'p1')});
+            expect(selectRestrictedPrimaryPolicyID('missing')(domain)).toBeUndefined();
+        });
+
+        it('returns undefined when enableRestrictedPrimaryPolicy is false', () => {
+            const domain = makeDomain({g1: makeGroup(false, 'p1')});
+            expect(selectRestrictedPrimaryPolicyID('g1')(domain)).toBeUndefined();
+        });
+
+        it('returns undefined when restrictedPrimaryPolicyID is not set', () => {
+            const domain = makeDomain({g1: makeGroup(true, undefined)});
+            expect(selectRestrictedPrimaryPolicyID('g1')(domain)).toBeUndefined();
+        });
+
+        it('returns the policyID when the group has restriction enabled', () => {
+            const domain = makeDomain({g1: makeGroup(true, 'p42')});
+            expect(selectRestrictedPrimaryPolicyID('g1')(domain)).toBe('p42');
+        });
+
+        it('returns the correct policyID when multiple groups exist', () => {
+            const domain = makeDomain({
+                g1: makeGroup(true, 'pA'),
+                g2: makeGroup(true, 'pB'),
+            });
+            expect(selectRestrictedPrimaryPolicyID('g2')(domain)).toBe('pB');
+        });
+    });
 });
diff --git a/tests/unit/PolicySelectorTest.ts b/tests/unit/PolicySelectorTest.ts
diff --git a/tests/unit/hooks/useDomainGroupMoveValidation.test.ts b/tests/unit/hooks/useDomainGroupMoveValidation.test.ts
