mirror of
https://github.com/n8n-io/n8n.git
synced 2026-05-12 16:10:30 +02:00
fix(core): Restore peer project discovery in share dropdowns (#29537)
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.14.1) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (25.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
Some checks are pending
CI: Master (Build, Test, Lint) / Build for Github Cache (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (22.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (24.14.1) (push) Waiting to run
CI: Master (Build, Test, Lint) / Unit tests (25.x) (push) Waiting to run
CI: Master (Build, Test, Lint) / Lint (push) Waiting to run
CI: Master (Build, Test, Lint) / Performance (push) Waiting to run
CI: Master (Build, Test, Lint) / Notify Slack on failure (push) Blocked by required conditions
This commit is contained in:
parent
5f93b48e79
commit
2a0e2fb47a
|
|
@ -53,6 +53,11 @@ export class ProjectRepository extends Repository<Project> {
|
|||
return await query.getManyAndCount();
|
||||
}
|
||||
|
||||
// Strict semantics: returns only projects the user has a relation to
|
||||
// (their personal project + projects they are explicitly a member of).
|
||||
// Do not broaden — peer-personal-project discovery for the share modal lives
|
||||
// in `getShareableProjectsAndCount` below; conflating the two has regressed
|
||||
// the share dropdown before (see IAM-591).
|
||||
async getAccessibleProjectsAndCount(
|
||||
userId: string,
|
||||
options: ProjectListOptions,
|
||||
|
|
@ -62,6 +67,40 @@ export class ProjectRepository extends Repository<Project> {
|
|||
.innerJoin('p.projectRelations', 'pr')
|
||||
.where('pr.userId = :userId', { userId });
|
||||
|
||||
this.applyIdsQueryFilters(idsQuery, options);
|
||||
return await this.runProjectListByIdsQuery(idsQuery, options);
|
||||
}
|
||||
|
||||
// Wide semantics: returns peer personal projects in addition to projects
|
||||
// the user has a relation to. Used only by the sharing-discovery endpoint
|
||||
// (`GET /rest/projects/sharing-candidates`) so the workflow / credential
|
||||
// share dropdowns can list other users as share targets.
|
||||
async getShareableProjectsAndCount(
|
||||
userId: string,
|
||||
options: ProjectListOptions,
|
||||
): Promise<[Project[], number]> {
|
||||
// DISTINCT + LEFT JOIN avoids duplicate rows from the relation join
|
||||
// while still allowing personal projects with no caller relation to match.
|
||||
const idsQuery = this.createQueryBuilder('p')
|
||||
.select('DISTINCT p.id', 'id')
|
||||
.leftJoin('p.projectRelations', 'pr')
|
||||
.where(
|
||||
new Brackets((qb) => {
|
||||
qb.where('p.type = :personalType', { personalType: 'personal' }).orWhere(
|
||||
'pr.userId = :userId',
|
||||
{ userId },
|
||||
);
|
||||
}),
|
||||
);
|
||||
|
||||
this.applyIdsQueryFilters(idsQuery, options);
|
||||
return await this.runProjectListByIdsQuery(idsQuery, options);
|
||||
}
|
||||
|
||||
private applyIdsQueryFilters(
|
||||
idsQuery: SelectQueryBuilder<Project>,
|
||||
options: ProjectListOptions,
|
||||
): void {
|
||||
if (options.search) {
|
||||
idsQuery.andWhere('LOWER(p.name) LIKE LOWER(:search)', {
|
||||
search: `%${options.search}%`,
|
||||
|
|
@ -81,7 +120,12 @@ export class ProjectRepository extends Repository<Project> {
|
|||
}),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
private async runProjectListByIdsQuery(
|
||||
idsQuery: SelectQueryBuilder<Project>,
|
||||
options: ProjectListOptions,
|
||||
): Promise<[Project[], number]> {
|
||||
const query = this.createQueryBuilder('project')
|
||||
.leftJoin('project.creator', 'creator')
|
||||
.where(`project.id IN (${idsQuery.getQuery()})`);
|
||||
|
|
|
|||
|
|
@ -78,6 +78,45 @@ describe('ProjectController', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('getSharingCandidates', () => {
|
||||
it('calls service with query options and returns enriched { count, data }', async () => {
|
||||
const projects = [
|
||||
{ id: 'p1', name: 'Project 1' },
|
||||
{ id: 'p2', name: 'Peer personal project' },
|
||||
];
|
||||
const enriched = projects.map((p) => ({
|
||||
...p,
|
||||
role: 'global:member',
|
||||
scopes: ['user:list'],
|
||||
}));
|
||||
(projectsService.getShareableProjectsAndCount as jest.Mock).mockResolvedValue([projects, 2]);
|
||||
(projectsService.addUserScopes as jest.Mock).mockResolvedValue(enriched);
|
||||
|
||||
const res = makeRes();
|
||||
const query = { skip: 0, take: 50, search: '' };
|
||||
|
||||
await controller.getSharingCandidates(req, res, query as any);
|
||||
|
||||
expect(projectsService.getShareableProjectsAndCount).toHaveBeenCalledWith(req.user, query);
|
||||
expect(projectsService.addUserScopes).toHaveBeenCalledWith(req.user, projects);
|
||||
expect(res.json).toHaveBeenCalledWith({ count: 2, data: enriched });
|
||||
});
|
||||
|
||||
it('always returns the { count, data } envelope (no bare-array path)', async () => {
|
||||
(projectsService.getShareableProjectsAndCount as jest.Mock).mockResolvedValue([[], 0]);
|
||||
(projectsService.addUserScopes as jest.Mock).mockResolvedValue([]);
|
||||
|
||||
const res = makeRes();
|
||||
const parsed = ListProjectsQueryDto.safeParse({});
|
||||
expect(parsed.success).toBe(true);
|
||||
const query = parsed.data!;
|
||||
|
||||
await controller.getSharingCandidates(req, res, query);
|
||||
|
||||
expect(res.json).toHaveBeenCalledWith({ count: 0, data: [] });
|
||||
});
|
||||
});
|
||||
|
||||
it('emits team-project-updated with full members list on addProjectUsers', async () => {
|
||||
// Arrange
|
||||
const projectId = 'p1';
|
||||
|
|
|
|||
|
|
@ -75,6 +75,26 @@ export class ProjectController {
|
|||
return await this.projectsService.getProjectCounts();
|
||||
}
|
||||
|
||||
// Lists projects a caller can pick as share targets, including peer
|
||||
// personal projects so the workflow / credential share dropdowns can
|
||||
// surface other users. Gated on `user:list` (the same boundary that
|
||||
// `GET /rest/users` enforces) — restricted roles without that scope
|
||||
// (e.g. chat-only users) cannot enumerate peer personal projects here.
|
||||
@Get('/sharing-candidates')
|
||||
@GlobalScope('user:list')
|
||||
async getSharingCandidates(
|
||||
req: AuthenticatedRequest,
|
||||
res: Response,
|
||||
@Query payload: ListProjectsQueryDto,
|
||||
) {
|
||||
const [data, count] = await this.projectsService.getShareableProjectsAndCount(
|
||||
req.user,
|
||||
payload,
|
||||
);
|
||||
const enriched = await this.projectsService.addUserScopes(req.user, data);
|
||||
return res.json({ count, data: enriched });
|
||||
}
|
||||
|
||||
@Post('/')
|
||||
@GlobalScope('project:create')
|
||||
// Using admin as all plans that contain projects should allow admins at the very least
|
||||
|
|
|
|||
|
|
@ -278,6 +278,20 @@ export class ProjectService {
|
|||
return await this.projectRepository.getAccessibleProjectsAndCount(user.id, options);
|
||||
}
|
||||
|
||||
// Returns the projects a caller can pick as share targets, including peer
|
||||
// personal projects. Admins (project:read) still see everything; non-admin
|
||||
// callers also see all personal projects so the share dropdown can surface
|
||||
// other users. See `ProjectRepository.getShareableProjectsAndCount`.
|
||||
async getShareableProjectsAndCount(
|
||||
user: User,
|
||||
options: ProjectListOptions,
|
||||
): Promise<[Project[], number]> {
|
||||
if (hasGlobalScope(user, 'project:read')) {
|
||||
return await this.projectRepository.findAllProjectsAndCount(options);
|
||||
}
|
||||
return await this.projectRepository.getShareableProjectsAndCount(user.id, options);
|
||||
}
|
||||
|
||||
async getPersonalProjectOwners(projectIds: string[]): Promise<ProjectRelation[]> {
|
||||
return await this.projectRelationRepository.getPersonalProjectOwners(projectIds);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -34,7 +34,7 @@ import {
|
|||
saveCredential,
|
||||
shareCredentialWithProjects,
|
||||
} from './shared/db/credentials';
|
||||
import { createMember, createOwner, createUser } from './shared/db/users';
|
||||
import { createChatUser, createMember, createOwner, createUser } from './shared/db/users';
|
||||
import * as utils from './shared/utils/';
|
||||
|
||||
import { ActiveWorkflowManager } from '@/active-workflow-manager';
|
||||
|
|
@ -143,6 +143,133 @@ describe('GET /projects/', () => {
|
|||
});
|
||||
});
|
||||
|
||||
describe('GET /projects/sharing-candidates', () => {
|
||||
test('member sees own personal project plus all peer personal projects', async () => {
|
||||
const [member1, member2, member3] = await Promise.all([
|
||||
createMember(),
|
||||
createMember(),
|
||||
createMember(),
|
||||
]);
|
||||
const [teamProject1, teamProject2] = await Promise.all([
|
||||
createTeamProject(undefined, member1),
|
||||
createTeamProject(),
|
||||
]);
|
||||
const [personal1, personal2, personal3] = await Promise.all([
|
||||
getPersonalProject(member1),
|
||||
getPersonalProject(member2),
|
||||
getPersonalProject(member3),
|
||||
]);
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(member1)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0 });
|
||||
expect(resp.status).toBe(200);
|
||||
const respProjects = resp.body.data as Project[];
|
||||
|
||||
// Own + peer personal projects appear (3 personal projects total)
|
||||
expect(respProjects.find((p) => p.id === personal1.id)).not.toBeUndefined();
|
||||
expect(respProjects.find((p) => p.id === personal2.id)).not.toBeUndefined();
|
||||
expect(respProjects.find((p) => p.id === personal3.id)).not.toBeUndefined();
|
||||
|
||||
// Team project the caller is a member of appears
|
||||
expect(respProjects.find((p) => p.id === teamProject1.id)).not.toBeUndefined();
|
||||
|
||||
// Team project the caller is NOT a member of does not appear
|
||||
expect(respProjects.find((p) => p.id === teamProject2.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
test('member does not see peer team projects they are not a member of', async () => {
|
||||
const [member1, member2] = await Promise.all([createMember(), createMember()]);
|
||||
const peerOnlyTeam = await createTeamProject(undefined, member2);
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(member1)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0 });
|
||||
expect(resp.status).toBe(200);
|
||||
const respProjects = resp.body.data as Project[];
|
||||
|
||||
expect(respProjects.find((p) => p.id === peerOnlyTeam.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
test('search filter narrows results across personal and relation branches', async () => {
|
||||
const [member1, peer] = await Promise.all([
|
||||
createUser({ firstName: 'Alice', lastName: 'Anderson' }),
|
||||
createUser({ firstName: 'Bob', lastName: 'Banana' }),
|
||||
]);
|
||||
const matchingTeam = await createTeamProject('Banana Republic', member1);
|
||||
const nonMatchingTeam = await createTeamProject('Other Project', member1);
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(member1)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0, search: 'banana' });
|
||||
expect(resp.status).toBe(200);
|
||||
const respProjects = resp.body.data as Project[];
|
||||
const peerPersonal = await getPersonalProject(peer);
|
||||
|
||||
// Matches by team name
|
||||
expect(respProjects.find((p) => p.id === matchingTeam.id)).not.toBeUndefined();
|
||||
// Matches peer personal project (name contains "Banana")
|
||||
expect(respProjects.find((p) => p.id === peerPersonal.id)).not.toBeUndefined();
|
||||
// Non-matching team does not appear
|
||||
expect(respProjects.find((p) => p.id === nonMatchingTeam.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
test('type=team filter excludes peer personal projects', async () => {
|
||||
const [member1, member2] = await Promise.all([createMember(), createMember()]);
|
||||
const team = await createTeamProject(undefined, member1);
|
||||
const peerPersonal = await getPersonalProject(member2);
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(member1)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0, type: 'team' });
|
||||
expect(resp.status).toBe(200);
|
||||
const respProjects = resp.body.data as Project[];
|
||||
|
||||
expect(respProjects.find((p) => p.id === team.id)).not.toBeUndefined();
|
||||
expect(respProjects.find((p) => p.id === peerPersonal.id)).toBeUndefined();
|
||||
});
|
||||
|
||||
test('owner sees all projects via the admin path', async () => {
|
||||
const [owner, peer1, peer2] = await Promise.all([
|
||||
createOwner(),
|
||||
createMember(),
|
||||
createMember(),
|
||||
]);
|
||||
const [team1, team2] = await Promise.all([createTeamProject(), createTeamProject()]);
|
||||
const [ownerPersonal, peer1Personal, peer2Personal] = await Promise.all([
|
||||
getPersonalProject(owner),
|
||||
getPersonalProject(peer1),
|
||||
getPersonalProject(peer2),
|
||||
]);
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(owner)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0 });
|
||||
expect(resp.status).toBe(200);
|
||||
const respProjects = resp.body.data as Project[];
|
||||
|
||||
// All five projects accessible to the admin
|
||||
for (const expected of [ownerPersonal, peer1Personal, peer2Personal, team1, team2]) {
|
||||
expect(respProjects.find((p) => p.id === expected.id)).not.toBeUndefined();
|
||||
}
|
||||
});
|
||||
|
||||
test('caller without user:list global scope receives 403', async () => {
|
||||
const chatUser = await createChatUser();
|
||||
|
||||
const resp = await testServer
|
||||
.authAgentFor(chatUser)
|
||||
.get('/projects/sharing-candidates')
|
||||
.query({ take: 50, skip: 0 });
|
||||
expect(resp.status).toBe(403);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Project members endpoints', () => {
|
||||
test('POST /projects/:projectId/users adds a member and emits telemetry', async () => {
|
||||
const owner = await createOwner();
|
||||
|
|
|
|||
|
|
@ -89,7 +89,7 @@ describe('WorkflowShareModal.ee.vue', () => {
|
|||
settingsStore.settings.enterprise = { sharing: true } as FrontendSettings['enterprise'];
|
||||
workflowsEEStore.getWorkflowOwnerName = vi.fn(() => 'Owner Name');
|
||||
projectsStore.personalProjects = [createProjectListItem()];
|
||||
projectsStore.searchProjects.mockResolvedValue({
|
||||
projectsStore.searchShareableProjects.mockResolvedValue({
|
||||
count: projectsStore.personalProjects.length,
|
||||
data: projectsStore.personalProjects,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -21,6 +21,27 @@ export const searchProjects = async (
|
|||
return await getFullApiResponse<ProjectListItem[]>(context, 'GET', '/projects', params);
|
||||
};
|
||||
|
||||
// Returns projects the caller can pick as share targets, including peer
|
||||
// personal projects so the workflow / credential share dropdowns can list
|
||||
// other users. Backed by `GET /rest/projects/sharing-candidates`.
|
||||
export const searchShareableProjects = async (
|
||||
context: IRestApiContext,
|
||||
params: {
|
||||
search?: string;
|
||||
take?: number;
|
||||
skip?: number;
|
||||
type?: 'personal' | 'team';
|
||||
activated?: boolean;
|
||||
},
|
||||
): Promise<{ count: number; data: ProjectListItem[] }> => {
|
||||
return await getFullApiResponse<ProjectListItem[]>(
|
||||
context,
|
||||
'GET',
|
||||
'/projects/sharing-candidates',
|
||||
params,
|
||||
);
|
||||
};
|
||||
|
||||
export const getMyProjects = async (context: IRestApiContext): Promise<ProjectListItem[]> => {
|
||||
return await makeRestApiRequest(context, 'GET', '/projects/my-projects');
|
||||
};
|
||||
|
|
|
|||
|
|
@ -126,6 +126,16 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
|
|||
return await projectsApi.searchProjects(rootStore.restApiContext, params);
|
||||
};
|
||||
|
||||
const searchShareableProjects = async (params: {
|
||||
search?: string;
|
||||
take?: number;
|
||||
skip?: number;
|
||||
type?: 'personal' | 'team';
|
||||
activated?: boolean;
|
||||
}) => {
|
||||
return await projectsApi.searchShareableProjects(rootStore.restApiContext, params);
|
||||
};
|
||||
|
||||
const fetchProject = async (id: string) =>
|
||||
await projectsApi.getProject(rootStore.restApiContext, id);
|
||||
|
||||
|
|
@ -345,6 +355,7 @@ export const useProjectsStore = defineStore(STORES.PROJECTS, () => {
|
|||
projectNavActiveId,
|
||||
setCurrentProject,
|
||||
searchProjects,
|
||||
searchShareableProjects,
|
||||
getAllProjects,
|
||||
getMyProjects,
|
||||
getPersonalProject,
|
||||
|
|
|
|||
|
|
@ -1,4 +1,10 @@
|
|||
import { splitName } from './projects.utils';
|
||||
import { createPinia, setActivePinia } from 'pinia';
|
||||
import {
|
||||
splitName,
|
||||
useRemoteProjectSearch,
|
||||
DEFAULT_PROJECT_SEARCH_PAGE_SIZE,
|
||||
} from './projects.utils';
|
||||
import { useProjectsStore } from './projects.store';
|
||||
|
||||
describe('splitName', () => {
|
||||
test.each([
|
||||
|
|
@ -76,3 +82,24 @@ describe('splitName', () => {
|
|||
expect(splitName(input)).toEqual(result);
|
||||
});
|
||||
});
|
||||
|
||||
describe('useRemoteProjectSearch', () => {
|
||||
beforeEach(() => {
|
||||
setActivePinia(createPinia());
|
||||
});
|
||||
|
||||
it('routes to the sharing-candidates endpoint via store.searchShareableProjects', async () => {
|
||||
const store = useProjectsStore();
|
||||
const spy = vi
|
||||
.spyOn(store, 'searchShareableProjects')
|
||||
.mockResolvedValue({ count: 0, data: [] });
|
||||
|
||||
const search = useRemoteProjectSearch();
|
||||
await search('alice');
|
||||
|
||||
expect(spy).toHaveBeenCalledWith({
|
||||
search: 'alice',
|
||||
take: DEFAULT_PROJECT_SEARCH_PAGE_SIZE,
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -8,13 +8,15 @@ export type ProjectSearchResult = { count: number; data: ProjectListItem[] };
|
|||
export type ProjectSearchFn = (query: string) => Promise<ProjectSearchResult>;
|
||||
|
||||
/**
|
||||
* Remote search for Group 1 consumers (sharing/transfer modals).
|
||||
* Always searches via GET /projects?search=&take= for ALL roles.
|
||||
* Remote search for Group 1 consumers (sharing / transfer / user-deletion modals).
|
||||
* Hits `GET /projects/sharing-candidates` so non-admin callers receive peer
|
||||
* personal projects in addition to projects they have a relation to — without
|
||||
* that, the share dropdown would be empty for `global:member` users.
|
||||
*/
|
||||
export function useRemoteProjectSearch(): ProjectSearchFn {
|
||||
const store = useProjectsStore();
|
||||
return async (query: string) => {
|
||||
return await store.searchProjects({
|
||||
return await store.searchShareableProjects({
|
||||
search: query,
|
||||
take: DEFAULT_PROJECT_SEARCH_PAGE_SIZE,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -100,7 +100,7 @@ describe('CredentialSharing.ee', () => {
|
|||
// Mock store methods
|
||||
vi.spyOn(usersStore, 'fetchUsers').mockResolvedValue();
|
||||
vi.spyOn(projectsStore, 'getAllProjects').mockResolvedValue();
|
||||
vi.spyOn(projectsStore, 'searchProjects').mockResolvedValue({
|
||||
vi.spyOn(projectsStore, 'searchShareableProjects').mockResolvedValue({
|
||||
count: testProjects.length,
|
||||
data: testProjects,
|
||||
});
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ describe('DeleteUserModal', () => {
|
|||
usersStore = mockedStore(useUsersStore);
|
||||
|
||||
const projectsStore = mockedStore(useProjectsStore);
|
||||
projectsStore.searchProjects.mockResolvedValue({
|
||||
projectsStore.searchShareableProjects.mockResolvedValue({
|
||||
count: initialState[STORES.PROJECTS].projects.length,
|
||||
data: initialState[STORES.PROJECTS].projects,
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user