🔄 refactor: Principal Type Handling in Search Principals to use Array

This commit is contained in:
Danny Avila 2025-08-12 16:49:19 -04:00
parent dcd96c29c5
commit 803ade8601
No known key found for this signature in database
GPG key ID: BF31EEB2C5CA0956
9 changed files with 68 additions and 151 deletions

View file

@ -364,7 +364,7 @@ const getUserEffectivePermissions = async (req, res) => {
*/ */
const searchPrincipals = async (req, res) => { const searchPrincipals = async (req, res) => {
try { try {
const { q: query, limit = 20, type } = req.query; const { q: query, limit = 20, types } = req.query;
if (!query || query.trim().length === 0) { if (!query || query.trim().length === 0) {
return res.status(400).json({ return res.status(400).json({
@ -379,22 +379,34 @@ const searchPrincipals = async (req, res) => {
} }
const searchLimit = Math.min(Math.max(1, parseInt(limit) || 10), 50); const searchLimit = Math.min(Math.max(1, parseInt(limit) || 10), 50);
const typeFilter = [PrincipalType.USER, PrincipalType.GROUP, PrincipalType.ROLE].includes(type)
? type
: null;
const localResults = await searchLocalPrincipals(query.trim(), searchLimit, typeFilter); let typeFilters = null;
if (types) {
const typesArray = Array.isArray(types) ? types : types.split(',');
const validTypes = typesArray.filter((t) =>
[PrincipalType.USER, PrincipalType.GROUP, PrincipalType.ROLE].includes(t),
);
typeFilters = validTypes.length > 0 ? validTypes : null;
}
const localResults = await searchLocalPrincipals(query.trim(), searchLimit, typeFilters);
let allPrincipals = [...localResults]; let allPrincipals = [...localResults];
const useEntraId = entraIdPrincipalFeatureEnabled(req.user); const useEntraId = entraIdPrincipalFeatureEnabled(req.user);
if (useEntraId && localResults.length < searchLimit) { if (useEntraId && localResults.length < searchLimit) {
try { try {
const graphTypeMap = { let graphType = 'all';
user: 'users', if (typeFilters && typeFilters.length === 1) {
group: 'groups', const graphTypeMap = {
null: 'all', [PrincipalType.USER]: 'users',
}; [PrincipalType.GROUP]: 'groups',
};
const mappedType = graphTypeMap[typeFilters[0]];
if (mappedType) {
graphType = mappedType;
}
}
const authHeader = req.headers.authorization; const authHeader = req.headers.authorization;
const accessToken = const accessToken =
@ -405,7 +417,7 @@ const searchPrincipals = async (req, res) => {
accessToken, accessToken,
req.user.openidId, req.user.openidId,
query.trim(), query.trim(),
graphTypeMap[typeFilter], graphType,
searchLimit - localResults.length, searchLimit - localResults.length,
); );
@ -436,21 +448,22 @@ const searchPrincipals = async (req, res) => {
_searchScore: calculateRelevanceScore(item, query.trim()), _searchScore: calculateRelevanceScore(item, query.trim()),
})); }));
allPrincipals = sortPrincipalsByRelevance(scoredResults) const finalResults = sortPrincipalsByRelevance(scoredResults)
.slice(0, searchLimit) .slice(0, searchLimit)
.map((result) => { .map((result) => {
const { _searchScore, ...resultWithoutScore } = result; const { _searchScore, ...resultWithoutScore } = result;
return resultWithoutScore; return resultWithoutScore;
}); });
res.status(200).json({ res.status(200).json({
query: query.trim(), query: query.trim(),
limit: searchLimit, limit: searchLimit,
type: typeFilter, types: typeFilters,
results: allPrincipals, results: finalResults,
count: allPrincipals.length, count: finalResults.length,
sources: { sources: {
local: allPrincipals.filter((r) => r.source === 'local').length, local: finalResults.filter((r) => r.source === 'local').length,
entra: allPrincipals.filter((r) => r.source === 'entra').length, entra: finalResults.filter((r) => r.source === 'entra').length,
}, },
}); });
} catch (error) { } catch (error) {

View file

@ -1,103 +0,0 @@
import React, { useState, useMemo } from 'react';
import { PrincipalType } from 'librechat-data-provider';
import type { TPrincipal, PrincipalSearchParams } from 'librechat-data-provider';
import { useSearchPrincipalsQuery } from 'librechat-data-provider/react-query';
import PeoplePickerSearchItem from './PeoplePickerSearchItem';
import SelectedPrincipalsList from './SelectedPrincipalsList';
import { SearchPicker } from './SearchPicker';
import { useLocalize } from '~/hooks';
interface PeoplePickerProps {
onSelectionChange: (principals: TPrincipal[]) => void;
placeholder?: string;
className?: string;
typeFilter?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE | null;
}
export default function PeoplePicker({
onSelectionChange,
placeholder,
className = '',
typeFilter = null,
}: PeoplePickerProps) {
const localize = useLocalize();
const [searchQuery, setSearchQuery] = useState('');
const [selectedShares, setSelectedShares] = useState<TPrincipal[]>([]);
const searchParams: PrincipalSearchParams = useMemo(
() => ({
q: searchQuery,
limit: 30,
...(typeFilter && { type: typeFilter }),
}),
[searchQuery, typeFilter],
);
const {
data: searchResponse,
isLoading: queryIsLoading,
error,
} = useSearchPrincipalsQuery(searchParams, {
enabled: searchQuery.length >= 2,
});
const isLoading = searchQuery.length >= 2 && queryIsLoading;
const selectableResults = useMemo(() => {
const results = searchResponse?.results || [];
return results.filter(
(result) => !selectedShares.some((share) => share.idOnTheSource === result.idOnTheSource),
);
}, [searchResponse?.results, selectedShares]);
if (error) {
console.error('Principal search error:', error);
}
return (
<div className={`space-y-3 ${className}`}>
<div className="relative">
<SearchPicker<TPrincipal & { key: string; value: string }>
options={selectableResults.map((s) => {
const key = s.idOnTheSource || 'unknown' + 'picker_key';
const value = s.idOnTheSource || 'Unknown';
return {
...s,
id: s.id ?? undefined,
key,
value,
};
})}
renderOptions={(o) => <PeoplePickerSearchItem principal={o} />}
placeholder={placeholder || localize('com_ui_search_default_placeholder')}
query={searchQuery}
onQueryChange={(query: string) => {
setSearchQuery(query);
}}
onPick={(principal) => {
console.log('Selected Principal:', principal);
setSelectedShares((prev) => {
const newArray = [...prev, principal];
onSelectionChange([...newArray]);
return newArray;
});
setSearchQuery('');
}}
isLoading={isLoading}
/>
</div>
<SelectedPrincipalsList
principles={selectedShares}
onRemoveHandler={(idOnTheSource: string) => {
setSelectedShares((prev) => {
const newArray = prev.filter((share) => share.idOnTheSource !== idOnTheSource);
onSelectionChange(newArray);
return newArray;
});
}}
/>
</div>
);
}

View file

@ -9,7 +9,7 @@ interface UnifiedPeopleSearchProps {
onAddPeople: (principals: TPrincipal[]) => void; onAddPeople: (principals: TPrincipal[]) => void;
placeholder?: string; placeholder?: string;
className?: string; className?: string;
typeFilter?: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE | null; typeFilter?: Array<PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE> | null;
excludeIds?: (string | undefined)[]; excludeIds?: (string | undefined)[];
} }
@ -27,7 +27,7 @@ export default function UnifiedPeopleSearch({
() => ({ () => ({
q: searchQuery, q: searchQuery,
limit: 30, limit: 30,
...(typeFilter && { type: typeFilter }), ...(typeFilter && typeFilter.length > 0 && { types: typeFilter }),
}), }),
[searchQuery, typeFilter], [searchQuery, typeFilter],
); );

View file

@ -1,4 +1,3 @@
export { default as PeoplePicker } from './PeoplePicker';
export { default as PeoplePickerSearchItem } from './PeoplePickerSearchItem'; export { default as PeoplePickerSearchItem } from './PeoplePickerSearchItem';
export { default as SelectedPrincipalsList } from './SelectedPrincipalsList'; export { default as SelectedPrincipalsList } from './SelectedPrincipalsList';
export { default as UnifiedPeopleSearch } from './UnifiedPeopleSearch'; export { default as UnifiedPeopleSearch } from './UnifiedPeopleSearch';

View file

@ -24,21 +24,28 @@ export const usePeoplePickerPermissions = () => {
const hasPeoplePickerAccess = canViewUsers || canViewGroups || canViewRoles; const hasPeoplePickerAccess = canViewUsers || canViewGroups || canViewRoles;
const peoplePickerTypeFilter: const peoplePickerTypeFilter: Array<
| PrincipalType.USER PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE
| PrincipalType.GROUP > | null = useMemo(() => {
| PrincipalType.ROLE const allowedTypes: Array<PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE> = [];
| null = useMemo(() => {
if (canViewUsers && canViewGroups && canViewRoles) { if (canViewUsers) {
return null; // All types allowed allowedTypes.push(PrincipalType.USER);
} else if (canViewUsers) {
return PrincipalType.USER;
} else if (canViewGroups) {
return PrincipalType.GROUP;
} else if (canViewRoles) {
return PrincipalType.ROLE;
} }
return null; if (canViewGroups) {
allowedTypes.push(PrincipalType.GROUP);
}
if (canViewRoles) {
allowedTypes.push(PrincipalType.ROLE);
}
// Return null if no types are allowed (will show no results)
// or if all types are allowed (no filtering needed)
if (allowedTypes.length === 0 || allowedTypes.length === 3) {
return null;
}
return allowedTypes;
}, [canViewUsers, canViewGroups, canViewRoles]); }, [canViewUsers, canViewGroups, canViewRoles]);
return { return {

View file

@ -315,15 +315,15 @@ export const memory = (key: string) => `${memories()}/${encodeURIComponent(key)}
export const memoryPreferences = () => `${memories()}/preferences`; export const memoryPreferences = () => `${memories()}/preferences`;
export const searchPrincipals = (params: q.PrincipalSearchParams) => { export const searchPrincipals = (params: q.PrincipalSearchParams) => {
const { q: query, limit, type } = params; const { q: query, limit, types } = params;
let url = `/api/permissions/search-principals?q=${encodeURIComponent(query)}`; let url = `/api/permissions/search-principals?q=${encodeURIComponent(query)}`;
if (limit !== undefined) { if (limit !== undefined) {
url += `&limit=${limit}`; url += `&limit=${limit}`;
} }
if (type !== undefined) { if (types && types.length > 0) {
url += `&type=${type}`; url += `&types=${types.join(',')}`;
} }
return url; return url;

View file

@ -129,13 +129,13 @@ export type MemoriesResponse = {
export type PrincipalSearchParams = { export type PrincipalSearchParams = {
q: string; q: string;
limit?: number; limit?: number;
type?: p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE; types?: Array<p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE>;
}; };
export type PrincipalSearchResponse = { export type PrincipalSearchResponse = {
query: string; query: string;
limit: number; limit: number;
type?: p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE; types?: Array<p.PrincipalType.USER | p.PrincipalType.GROUP | p.PrincipalType.ROLE>;
results: p.TPrincipalSearchResult[]; results: p.TPrincipalSearchResult[];
count: number; count: number;
sources: { sources: {

View file

@ -234,7 +234,7 @@ describe('Role-based Permissions Integration', () => {
}); });
test('should filter search results by role type', async () => { test('should filter search results by role type', async () => {
const results = await methods.searchPrincipals('mod', 10, PrincipalType.ROLE); const results = await methods.searchPrincipals('mod', 10, [PrincipalType.ROLE]);
expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true); expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true);
expect(results).toHaveLength(1); expect(results).toHaveLength(1);
@ -247,7 +247,7 @@ describe('Role-based Permissions Integration', () => {
await Role.create({ name: `testrole${i}` }); await Role.create({ name: `testrole${i}` });
} }
const results = await methods.searchPrincipals('testrole', 5, PrincipalType.ROLE); const results = await methods.searchPrincipals('testrole', 5, [PrincipalType.ROLE]);
expect(results).toHaveLength(5); expect(results).toHaveLength(5);
expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true); expect(results.every((r) => r.type === PrincipalType.ROLE)).toBe(true);
@ -275,14 +275,14 @@ describe('Role-based Permissions Integration', () => {
}); });
test('should handle case-insensitive role search', async () => { test('should handle case-insensitive role search', async () => {
const results = await methods.searchPrincipals('ADMIN', 10, PrincipalType.ROLE); const results = await methods.searchPrincipals('ADMIN', 10, [PrincipalType.ROLE]);
expect(results).toHaveLength(1); expect(results).toHaveLength(1);
expect(results[0].name).toBe('admin'); expect(results[0].name).toBe('admin');
}); });
test('should return empty array for no role matches', async () => { test('should return empty array for no role matches', async () => {
const results = await methods.searchPrincipals('nonexistentrole', 10, PrincipalType.ROLE); const results = await methods.searchPrincipals('nonexistentrole', 10, [PrincipalType.ROLE]);
expect(results).toEqual([]); expect(results).toEqual([]);
}); });

View file

@ -495,14 +495,14 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
* Returns combined results in TPrincipalSearchResult format without sorting * Returns combined results in TPrincipalSearchResult format without sorting
* @param searchPattern - The pattern to search for * @param searchPattern - The pattern to search for
* @param limitPerType - Maximum number of results to return * @param limitPerType - Maximum number of results to return
* @param typeFilter - Optional filter: PrincipalType.USER, PrincipalType.GROUP, or null for all * @param typeFilter - Optional array of types to filter by, or null for all types
* @param session - Optional MongoDB session for transactions * @param session - Optional MongoDB session for transactions
* @returns Array of principals in TPrincipalSearchResult format * @returns Array of principals in TPrincipalSearchResult format
*/ */
async function searchPrincipals( async function searchPrincipals(
searchPattern: string, searchPattern: string,
limitPerType: number = 10, limitPerType: number = 10,
typeFilter: PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE | null = null, typeFilter: Array<PrincipalType.USER | PrincipalType.GROUP | PrincipalType.ROLE> | null = null,
session?: ClientSession, session?: ClientSession,
): Promise<TPrincipalSearchResult[]> { ): Promise<TPrincipalSearchResult[]> {
if (!searchPattern || searchPattern.trim().length === 0) { if (!searchPattern || searchPattern.trim().length === 0) {
@ -512,7 +512,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
const trimmedPattern = searchPattern.trim(); const trimmedPattern = searchPattern.trim();
const promises: Promise<TPrincipalSearchResult[]>[] = []; const promises: Promise<TPrincipalSearchResult[]>[] = [];
if (!typeFilter || typeFilter === PrincipalType.USER) { if (!typeFilter || typeFilter.includes(PrincipalType.USER)) {
/** Note: searchUsers is imported from ~/models and needs to be passed in or implemented */ /** Note: searchUsers is imported from ~/models and needs to be passed in or implemented */
const userFields = 'name email username avatar provider idOnTheSource'; const userFields = 'name email username avatar provider idOnTheSource';
/** For now, we'll use a direct query instead of searchUsers */ /** For now, we'll use a direct query instead of searchUsers */
@ -547,7 +547,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
promises.push(Promise.resolve([])); promises.push(Promise.resolve([]));
} }
if (!typeFilter || typeFilter === PrincipalType.GROUP) { if (!typeFilter || typeFilter.includes(PrincipalType.GROUP)) {
promises.push( promises.push(
findGroupsByNamePattern(trimmedPattern, null, limitPerType, session).then((groups) => findGroupsByNamePattern(trimmedPattern, null, limitPerType, session).then((groups) =>
groups.map(transformGroupToTPrincipalSearchResult), groups.map(transformGroupToTPrincipalSearchResult),
@ -557,7 +557,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
promises.push(Promise.resolve([])); promises.push(Promise.resolve([]));
} }
if (!typeFilter || typeFilter === PrincipalType.ROLE) { if (!typeFilter || typeFilter.includes(PrincipalType.ROLE)) {
const Role = mongoose.models.Role as Model<IRole>; const Role = mongoose.models.Role as Model<IRole>;
if (Role) { if (Role) {
const regex = new RegExp(trimmedPattern, 'i'); const regex = new RegExp(trimmedPattern, 'i');
@ -575,6 +575,7 @@ export function createUserGroupMethods(mongoose: typeof import('mongoose')) {
type: PrincipalType.ROLE, type: PrincipalType.ROLE,
name: role.name, name: role.name,
source: 'local' as const, source: 'local' as const,
idOnTheSource: role.name,
})), })),
), ),
); );