diff --git a/packages/cli/src/Interfaces.ts b/packages/cli/src/Interfaces.ts index 83c2664332af0..b6bd20270dbc2 100644 --- a/packages/cli/src/Interfaces.ts +++ b/packages/cli/src/Interfaces.ts @@ -772,9 +772,7 @@ export interface PublicUser { disabled: boolean; settings?: IUserSettings | null; inviteAcceptUrl?: string; -} - -export interface CurrentUser extends PublicUser { + isOwner?: boolean; featureFlags?: FeatureFlags; } diff --git a/packages/cli/src/UserManagement/UserManagementHelper.ts b/packages/cli/src/UserManagement/UserManagementHelper.ts index 23e4b21ace82f..6ec733e675863 100644 --- a/packages/cli/src/UserManagement/UserManagementHelper.ts +++ b/packages/cli/src/UserManagement/UserManagementHelper.ts @@ -4,13 +4,12 @@ import { Container } from 'typedi'; import * as Db from '@/Db'; import * as ResponseHelper from '@/ResponseHelper'; -import type { CurrentUser, PublicUser, WhereClause } from '@/Interfaces'; +import type { WhereClause } from '@/Interfaces'; import type { User } from '@db/entities/User'; import { MAX_PASSWORD_LENGTH, MIN_PASSWORD_LENGTH } from '@db/entities/User'; import config from '@/config'; import { License } from '@/License'; import { getWebhookBaseUrl } from '@/WebhookHelpers'; -import type { PostHogClient } from '@/posthog'; import { RoleService } from '@/services/role.service'; export function isEmailSetUp(): boolean { @@ -84,64 +83,6 @@ export function validatePassword(password?: string): string { return password; } -/** - * Remove sensitive properties from the user to return to the client. - */ -export function sanitizeUser(user: User, withoutKeys?: string[]): PublicUser { - const { password, updatedAt, apiKey, authIdentities, mfaSecret, mfaRecoveryCodes, ...rest } = - user; - if (withoutKeys) { - withoutKeys.forEach((key) => { - // @ts-ignore - delete rest[key]; - }); - } - - const sanitizedUser: PublicUser = { - ...rest, - signInType: 'email', - hasRecoveryCodesLeft: !!user.mfaRecoveryCodes?.length, - }; - - const ldapIdentity = authIdentities?.find((i) => i.providerType === 'ldap'); - if (ldapIdentity) { - sanitizedUser.signInType = 'ldap'; - } - - return sanitizedUser; -} - -export async function withFeatureFlags( - postHog: PostHogClient | undefined, - user: CurrentUser, -): Promise { - if (!postHog) { - return user; - } - - // native PostHog implementation has default 10s timeout and 3 retries.. which cannot be updated without affecting other functionality - // https://github.com/PostHog/posthog-js-lite/blob/a182de80a433fb0ffa6859c10fb28084d0f825c2/posthog-core/src/index.ts#L67 - const timeoutPromise = new Promise((resolve) => { - setTimeout(() => { - resolve(user); - }, 1500); - }); - - const fetchPromise = new Promise(async (resolve) => { - user.featureFlags = await postHog.getFeatureFlags(user); - resolve(user); - }); - - return Promise.race([fetchPromise, timeoutPromise]); -} - -export function addInviteLinkToUser(user: PublicUser, inviterId: string): PublicUser { - if (user.isPending) { - user.inviteAcceptUrl = generateUserInviteUrl(inviterId, user.id); - } - return user; -} - export async function getUserById(userId: string): Promise { const user = await Db.collections.User.findOneOrFail({ where: { id: userId }, diff --git a/packages/cli/src/controllers/auth.controller.ts b/packages/cli/src/controllers/auth.controller.ts index a02be95cdb0d7..c7beb64a222f2 100644 --- a/packages/cli/src/controllers/auth.controller.ts +++ b/packages/cli/src/controllers/auth.controller.ts @@ -8,16 +8,15 @@ import { InternalServerError, UnauthorizedError, } from '@/ResponseHelper'; -import { sanitizeUser, withFeatureFlags } from '@/UserManagement/UserManagementHelper'; import { issueCookie, resolveJwt } from '@/auth/jwt'; import { AUTH_COOKIE_NAME, RESPONSE_ERROR_MESSAGES } from '@/constants'; import { Request, Response } from 'express'; import { ILogger } from 'n8n-workflow'; import type { User } from '@db/entities/User'; import { LoginRequest, UserRequest } from '@/requests'; +import type { PublicUser } from '@/Interfaces'; import { Config } from '@/config'; import { IInternalHooksClass } from '@/Interfaces'; -import type { PublicUser, CurrentUser } from '@/Interfaces'; import { handleEmailLogin, handleLdapLogin } from '@/auth'; import { PostHogClient } from '@/posthog'; import { @@ -98,7 +97,8 @@ export class AuthController { user, authenticationMethod: usedAuthenticationMethod, }); - return withFeatureFlags(this.postHog, sanitizeUser(user)); + + return this.userService.toPublic(user, { posthog: this.postHog }); } void Container.get(InternalHooks).onUserLoginFailed({ user: email, @@ -112,7 +112,7 @@ export class AuthController { * Manually check the `n8n-auth` cookie. */ @Get('/login') - async currentUser(req: Request, res: Response): Promise { + async currentUser(req: Request, res: Response): Promise { // Manually check the existing cookie. // eslint-disable-next-line @typescript-eslint/no-unsafe-member-access const cookieContents = req.cookies?.[AUTH_COOKIE_NAME] as string | undefined; @@ -123,7 +123,7 @@ export class AuthController { try { user = await resolveJwt(cookieContents); - return await withFeatureFlags(this.postHog, sanitizeUser(user)); + return await this.userService.toPublic(user, { posthog: this.postHog }); } catch (error) { res.clearCookie(AUTH_COOKIE_NAME); } @@ -146,7 +146,7 @@ export class AuthController { } await issueCookie(res, user); - return withFeatureFlags(this.postHog, sanitizeUser(user)); + return this.userService.toPublic(user, { posthog: this.postHog }); } /** @@ -183,7 +183,10 @@ export class AuthController { } } - const users = await this.userService.findMany({ where: { id: In([inviterId, inviteeId]) } }); + const users = await this.userService.findMany({ + where: { id: In([inviterId, inviteeId]) }, + relations: ['globalRole'], + }); if (users.length !== 2) { this.logger.debug( 'Request to resolve signup token failed because the ID of the inviter and/or the ID of the invitee were not found in database', diff --git a/packages/cli/src/controllers/me.controller.ts b/packages/cli/src/controllers/me.controller.ts index ad8e4ed393c1f..8026e67bdb4ea 100644 --- a/packages/cli/src/controllers/me.controller.ts +++ b/packages/cli/src/controllers/me.controller.ts @@ -1,12 +1,7 @@ import validator from 'validator'; import { plainToInstance } from 'class-transformer'; import { Authorized, Delete, Get, Patch, Post, RestController } from '@/decorators'; -import { - compareHash, - hashPassword, - sanitizeUser, - validatePassword, -} from '@/UserManagement/UserManagementHelper'; +import { compareHash, hashPassword, validatePassword } from '@/UserManagement/UserManagementHelper'; import { BadRequestError } from '@/ResponseHelper'; import { validateEntity } from '@/GenericHelpers'; import { issueCookie } from '@/auth/jwt'; @@ -89,9 +84,11 @@ export class MeController { fields_changed: updatedKeys, }); - await this.externalHooks.run('user.profile.update', [currentEmail, sanitizeUser(user)]); + const publicUser = await this.userService.toPublic(user); + + await this.externalHooks.run('user.profile.update', [currentEmail, publicUser]); - return sanitizeUser(user); + return publicUser; } /** diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index 362b298761c98..3a33388fd5ec3 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -2,12 +2,7 @@ import validator from 'validator'; import { validateEntity } from '@/GenericHelpers'; import { Authorized, Post, RestController } from '@/decorators'; import { BadRequestError } from '@/ResponseHelper'; -import { - hashPassword, - sanitizeUser, - validatePassword, - withFeatureFlags, -} from '@/UserManagement/UserManagementHelper'; +import { hashPassword, validatePassword } from '@/UserManagement/UserManagementHelper'; import { issueCookie } from '@/auth/jwt'; import { Response } from 'express'; import { ILogger } from 'n8n-workflow'; @@ -106,7 +101,7 @@ export class OwnerController { void this.internalHooks.onInstanceOwnerSetup({ user_id: userId }); - return withFeatureFlags(this.postHog, sanitizeUser(owner)); + return this.userService.toPublic(owner, { posthog: this.postHog }); } @Post('/dismiss-banner') diff --git a/packages/cli/src/controllers/users.controller.ts b/packages/cli/src/controllers/users.controller.ts index 518a11d90bd86..e77774c9ecfdf 100644 --- a/packages/cli/src/controllers/users.controller.ts +++ b/packages/cli/src/controllers/users.controller.ts @@ -1,19 +1,17 @@ import validator from 'validator'; -import { In } from 'typeorm'; +import type { FindManyOptions } from 'typeorm'; +import { In, Not } from 'typeorm'; import { ILogger, ErrorReporterProxy as ErrorReporter } from 'n8n-workflow'; import { User } from '@db/entities/User'; import { SharedCredentials } from '@db/entities/SharedCredentials'; import { SharedWorkflow } from '@db/entities/SharedWorkflow'; import { Authorized, NoAuthRequired, Delete, Get, Post, RestController, Patch } from '@/decorators'; import { - addInviteLinkToUser, generateUserInviteUrl, getInstanceBaseUrl, hashPassword, isEmailSetUp, - sanitizeUser, validatePassword, - withFeatureFlags, } from '@/UserManagement/UserManagementHelper'; import { issueCookie } from '@/auth/jwt'; import { @@ -23,12 +21,12 @@ import { UnauthorizedError, } from '@/ResponseHelper'; import { Response } from 'express'; -import { Config } from '@/config'; -import { UserRequest, UserSettingsUpdatePayload } from '@/requests'; +import { ListQuery, UserRequest, UserSettingsUpdatePayload } from '@/requests'; import { UserManagementMailer } from '@/UserManagement/email'; +import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; +import { Config } from '@/config'; import { IExternalHooksClass, IInternalHooksClass } from '@/Interfaces'; import type { PublicUser, ITelemetryUserDeletionData } from '@/Interfaces'; -import { ActiveWorkflowRunner } from '@/ActiveWorkflowRunner'; import { AuthIdentity } from '@db/entities/AuthIdentity'; import { PostHogClient } from '@/posthog'; import { isSamlLicensedAndEnabled } from '../sso/saml/samlHelpers'; @@ -40,6 +38,7 @@ import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { JwtService } from '@/services/jwt.service'; import { RoleService } from '@/services/role.service'; import { UserService } from '@/services/user.service'; +import { listQueryMiddleware } from '@/middlewares'; @Authorized(['global', 'owner']) @RestController('/users') @@ -131,6 +130,7 @@ export class UsersController { // remove/exclude existing users from creation const existingUsers = await this.userService.findMany({ where: { email: In(Object.keys(createUsers)) }, + relations: ['globalRole'], }); existingUsers.forEach((user) => { if (user.password) { @@ -306,20 +306,98 @@ export class UsersController { was_disabled_ldap_user: false, }); - await this.externalHooks.run('user.profile.update', [invitee.email, sanitizeUser(invitee)]); + const publicInvitee = await this.userService.toPublic(invitee); + + await this.externalHooks.run('user.profile.update', [invitee.email, publicInvitee]); await this.externalHooks.run('user.password.update', [invitee.email, invitee.password]); - return withFeatureFlags(this.postHog, sanitizeUser(updatedUser)); + return this.userService.toPublic(updatedUser, { posthog: this.postHog }); + } + + private async toFindManyOptions(listQueryOptions?: ListQuery.Options) { + const findManyOptions: FindManyOptions = {}; + + if (!listQueryOptions) { + findManyOptions.relations = ['globalRole', 'authIdentities']; + return findManyOptions; + } + + const { filter, select, take, skip } = listQueryOptions; + + if (select) findManyOptions.select = select; + if (take) findManyOptions.take = take; + if (skip) findManyOptions.skip = skip; + + if (take && !select) { + findManyOptions.relations = ['globalRole', 'authIdentities']; + } + + if (take && select && !select?.id) { + findManyOptions.select = { ...findManyOptions.select, id: true }; // pagination requires id + } + + if (filter) { + const { isOwner, ...otherFilters } = filter; + + findManyOptions.where = otherFilters; + + if (isOwner !== undefined) { + const ownerRole = await this.roleService.findGlobalOwnerRole(); + + findManyOptions.relations = ['globalRole']; + findManyOptions.where.globalRole = { id: isOwner ? ownerRole.id : Not(ownerRole.id) }; + } + } + + return findManyOptions; + } + + removeSupplementaryFields( + publicUsers: Array>, + listQueryOptions: ListQuery.Options, + ) { + const { take, select, filter } = listQueryOptions; + + // remove fields added to satisfy query + + if (take && select && !select?.id) { + for (const user of publicUsers) delete user.id; + } + + if (filter?.isOwner) { + for (const user of publicUsers) delete user.globalRole; + } + + // remove computed fields (unselectable) + + if (select) { + for (const user of publicUsers) { + delete user.isOwner; + delete user.isPending; + delete user.signInType; + delete user.hasRecoveryCodesLeft; + } + } + + return publicUsers; } @Authorized('any') - @Get('/') - async listUsers(req: UserRequest.List) { - const users = await this.userService.findMany({ relations: ['globalRole', 'authIdentities'] }); - return users.map( - (user): PublicUser => - addInviteLinkToUser(sanitizeUser(user, ['personalizationAnswers']), req.user.id), + @Get('/', { middlewares: listQueryMiddleware }) + async listUsers(req: ListQuery.Request) { + const { listQueryOptions } = req; + + const findManyOptions = await this.toFindManyOptions(listQueryOptions); + + const users = await this.userService.findMany(findManyOptions); + + const publicUsers: Array> = await Promise.all( + users.map(async (u) => this.userService.toPublic(u, { withInviteUrl: true })), ); + + return listQueryOptions + ? this.removeSupplementaryFields(publicUsers, listQueryOptions) + : publicUsers; } @Authorized(['global', 'owner']) @@ -393,6 +471,7 @@ export class UsersController { const users = await this.userService.findMany({ where: { id: In([transferId, idToDelete]) }, + relations: ['globalRole'], }); if (!users.length || (transferId && users.length !== 2)) { @@ -483,7 +562,7 @@ export class UsersController { telemetryData, publicApi: false, }); - await this.externalHooks.run('user.deleted', [sanitizeUser(userToDelete)]); + await this.externalHooks.run('user.deleted', [await this.userService.toPublic(userToDelete)]); return { success: true }; } @@ -521,7 +600,7 @@ export class UsersController { publicApi: false, }); - await this.externalHooks.run('user.deleted', [sanitizeUser(userToDelete)]); + await this.externalHooks.run('user.deleted', [await this.userService.toPublic(userToDelete)]); return { success: true }; } diff --git a/packages/cli/src/middlewares/listQuery/dtos/base.filter.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/base.filter.dto.ts new file mode 100644 index 0000000000000..47a7273b5b2df --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/base.filter.dto.ts @@ -0,0 +1,30 @@ +/* eslint-disable @typescript-eslint/naming-convention */ + +import { isObjectLiteral } from '@/utils'; +import { plainToInstance, instanceToPlain } from 'class-transformer'; +import { validate } from 'class-validator'; +import { jsonParse } from 'n8n-workflow'; + +export class BaseFilter { + protected static async toFilter(rawFilter: string, Filter: typeof BaseFilter) { + const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); + + if (!isObjectLiteral(dto)) throw new Error('Filter must be an object literal'); + + const instance = plainToInstance(Filter, dto, { + excludeExtraneousValues: true, // remove fields not in class + }); + + await instance.validate(); + + return instanceToPlain(instance, { + exposeUnsetFields: false, // remove in-class undefined fields + }); + } + + private async validate() { + const result = await validate(this); + + if (result.length > 0) throw new Error('Parsed filter does not fit the schema'); + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/base.select.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/base.select.dto.ts new file mode 100644 index 0000000000000..da7ba6752d17b --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/base.select.dto.ts @@ -0,0 +1,19 @@ +/* eslint-disable @typescript-eslint/naming-convention */ + +import { isStringArray } from '@/utils'; +import { jsonParse } from 'n8n-workflow'; + +export class BaseSelect { + static selectableFields: Set; + + protected static toSelect(rawFilter: string, Select: typeof BaseSelect) { + const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); + + if (!isStringArray(dto)) throw new Error('Parsed select is not a string array'); + + return dto.reduce>((acc, field) => { + if (!Select.selectableFields.has(field)) return acc; + return (acc[field] = true), acc; + }, {}); + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/user.filter.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/user.filter.dto.ts new file mode 100644 index 0000000000000..6c7e8a01312ba --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/user.filter.dto.ts @@ -0,0 +1,29 @@ +import { IsOptional, IsString, IsBoolean } from 'class-validator'; +import { Expose } from 'class-transformer'; +import { BaseFilter } from './base.filter.dto'; + +export class UserFilter extends BaseFilter { + @IsString() + @IsOptional() + @Expose() + email?: string; + + @IsString() + @IsOptional() + @Expose() + firstName?: string; + + @IsString() + @IsOptional() + @Expose() + lastName?: string; + + @IsBoolean() + @IsOptional() + @Expose() + isOwner?: boolean; + + static async fromString(rawFilter: string) { + return this.toFilter(rawFilter, UserFilter); + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/user.select.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/user.select.dto.ts new file mode 100644 index 0000000000000..7da40be3e8975 --- /dev/null +++ b/packages/cli/src/middlewares/listQuery/dtos/user.select.dto.ts @@ -0,0 +1,11 @@ +import { BaseSelect } from './base.select.dto'; + +export class UserSelect extends BaseSelect { + static get selectableFields() { + return new Set(['id', 'email', 'firstName', 'lastName']); + } + + static fromString(rawFilter: string) { + return this.toSelect(rawFilter, UserSelect); + } +} diff --git a/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts index 4eb9da41e0c94..389246cf6cf42 100644 --- a/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts +++ b/packages/cli/src/middlewares/listQuery/dtos/workflow.filter.dto.ts @@ -1,9 +1,9 @@ -import { IsOptional, IsString, IsBoolean, IsArray, validate } from 'class-validator'; -import { Expose, instanceToPlain, plainToInstance } from 'class-transformer'; -import { jsonParse } from 'n8n-workflow'; -import { isObjectLiteral } from '@/utils'; +import { IsOptional, IsString, IsBoolean, IsArray } from 'class-validator'; +import { Expose } from 'class-transformer'; -export class WorkflowFilter { +import { BaseFilter } from './base.filter.dto'; + +export class WorkflowFilter extends BaseFilter { @IsString() @IsOptional() @Expose() @@ -21,23 +21,6 @@ export class WorkflowFilter { tags?: string[]; static async fromString(rawFilter: string) { - const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); - - if (!isObjectLiteral(dto)) throw new Error('Filter must be an object literal'); - - const instance = plainToInstance(WorkflowFilter, dto, { - excludeExtraneousValues: true, // remove fields not in class - exposeUnsetFields: false, // remove in-class undefined fields - }); - - await instance.validate(); - - return instanceToPlain(instance); - } - - private async validate() { - const result = await validate(this); - - if (result.length > 0) throw new Error('Parsed filter does not fit the schema'); + return this.toFilter(rawFilter, WorkflowFilter); } } diff --git a/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts b/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts index dd74e81269d7a..0a905604623c0 100644 --- a/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts +++ b/packages/cli/src/middlewares/listQuery/dtos/workflow.select.dto.ts @@ -1,9 +1,6 @@ -import { isStringArray } from '@/utils'; -import { jsonParse } from 'n8n-workflow'; - -export class WorkflowSelect { - fields: string[]; +import { BaseSelect } from './base.select.dto'; +export class WorkflowSelect extends BaseSelect { static get selectableFields() { return new Set([ 'id', // always included downstream @@ -18,13 +15,6 @@ export class WorkflowSelect { } static fromString(rawFilter: string) { - const dto = jsonParse(rawFilter, { errorMessage: 'Failed to parse filter JSON' }); - - if (!isStringArray(dto)) throw new Error('Parsed select is not a string array'); - - return dto.reduce>((acc, field) => { - if (!WorkflowSelect.selectableFields.has(field)) return acc; - return (acc[field] = true), acc; - }, {}); + return this.toSelect(rawFilter, WorkflowSelect); } } diff --git a/packages/cli/src/middlewares/listQuery/filter.ts b/packages/cli/src/middlewares/listQuery/filter.ts index 6ca297ef909fb..cb189a5042cc5 100644 --- a/packages/cli/src/middlewares/listQuery/filter.ts +++ b/packages/cli/src/middlewares/listQuery/filter.ts @@ -2,6 +2,7 @@ import * as ResponseHelper from '@/ResponseHelper'; import { WorkflowFilter } from './dtos/workflow.filter.dto'; +import { UserFilter } from './dtos/user.filter.dto'; import { toError } from '@/utils'; import type { NextFunction, Response } from 'express'; @@ -20,6 +21,8 @@ export const filterListQueryMiddleware = async ( if (req.baseUrl.endsWith('workflows')) { Filter = WorkflowFilter; + } else if (req.baseUrl.endsWith('users')) { + Filter = UserFilter; } else { return next(); } diff --git a/packages/cli/src/middlewares/listQuery/pagination.ts b/packages/cli/src/middlewares/listQuery/pagination.ts index 9a16ee3a5a28e..2438292be7517 100644 --- a/packages/cli/src/middlewares/listQuery/pagination.ts +++ b/packages/cli/src/middlewares/listQuery/pagination.ts @@ -11,9 +11,13 @@ export const paginationListQueryMiddleware: RequestHandler = ( ) => { const { take: rawTake, skip: rawSkip = '0' } = req.query; - if (!rawTake) return next(); - try { + if (!rawTake && req.query.skip) { + throw new Error('Please specify `take` when using `skip`'); + } + + if (!rawTake) return next(); + const { take, skip } = Pagination.fromString(rawTake, rawSkip); req.listQueryOptions = { ...req.listQueryOptions, skip, take }; diff --git a/packages/cli/src/middlewares/listQuery/select.ts b/packages/cli/src/middlewares/listQuery/select.ts index e5813bbc0e0a8..4ff4ac525af66 100644 --- a/packages/cli/src/middlewares/listQuery/select.ts +++ b/packages/cli/src/middlewares/listQuery/select.ts @@ -1,6 +1,7 @@ /* eslint-disable @typescript-eslint/naming-convention */ import { WorkflowSelect } from './dtos/workflow.select.dto'; +import { UserSelect } from './dtos/user.select.dto'; import * as ResponseHelper from '@/ResponseHelper'; import { toError } from '@/utils'; @@ -16,6 +17,8 @@ export const selectListQueryMiddleware: RequestHandler = (req: ListQuery.Request if (req.baseUrl.endsWith('workflows')) { Select = WorkflowSelect; + } else if (req.baseUrl.endsWith('users')) { + Select = UserSelect; } else { return next(); } diff --git a/packages/cli/src/requests.ts b/packages/cli/src/requests.ts index fea69cd652539..fe7bb2c421897 100644 --- a/packages/cli/src/requests.ts +++ b/packages/cli/src/requests.ts @@ -280,8 +280,6 @@ export declare namespace PasswordResetRequest { // ---------------------------------- export declare namespace UserRequest { - export type List = AuthenticatedRequest; - export type Invite = AuthenticatedRequest<{}, {}, Array<{ email: string }>>; export type ResolveSignUp = AuthlessRequest< diff --git a/packages/cli/src/services/user.service.ts b/packages/cli/src/services/user.service.ts index 5e01ef6a85901..6d6c9c15f528b 100644 --- a/packages/cli/src/services/user.service.ts +++ b/packages/cli/src/services/user.service.ts @@ -4,6 +4,9 @@ import { In } from 'typeorm'; import { User } from '@db/entities/User'; import type { IUserSettings } from 'n8n-workflow'; import { UserRepository } from '@/databases/repositories'; +import { getInstanceBaseUrl } from '@/UserManagement/UserManagementHelper'; +import type { PublicUser } from '@/Interfaces'; +import type { PostHogClient } from '@/posthog'; @Service() export class UserService { @@ -18,7 +21,7 @@ export class UserService { } async findMany(options: FindManyOptions) { - return this.userRepository.find({ relations: ['globalRole'], ...options }); + return this.userRepository.find(options); } async findOneBy(options: FindOptionsWhere) { @@ -59,4 +62,54 @@ export class UserService { return url.toString(); } + + async toPublic(user: User, options?: { withInviteUrl?: boolean; posthog?: PostHogClient }) { + const { password, updatedAt, apiKey, authIdentities, ...rest } = user; + + const ldapIdentity = authIdentities?.find((i) => i.providerType === 'ldap'); + + let publicUser: PublicUser = { + ...rest, + signInType: ldapIdentity ? 'ldap' : 'email', + hasRecoveryCodesLeft: !!user.mfaRecoveryCodes?.length, + }; + + if (options?.withInviteUrl && publicUser.isPending) { + publicUser = this.addInviteUrl(publicUser, user.id); + } + + if (options?.posthog) { + publicUser = await this.addFeatureFlags(publicUser, options.posthog); + } + + return publicUser; + } + + private addInviteUrl(user: PublicUser, inviterId: string) { + const url = new URL(getInstanceBaseUrl()); + url.pathname = '/signup'; + url.searchParams.set('inviterId', inviterId); + url.searchParams.set('inviteeId', user.id); + + user.inviteAcceptUrl = url.toString(); + + return user; + } + + private async addFeatureFlags(publicUser: PublicUser, posthog: PostHogClient) { + // native PostHog implementation has default 10s timeout and 3 retries.. which cannot be updated without affecting other functionality + // https://github.com/PostHog/posthog-js-lite/blob/a182de80a433fb0ffa6859c10fb28084d0f825c2/posthog-core/src/index.ts#L67 + const timeoutPromise = new Promise((resolve) => { + setTimeout(() => { + resolve(publicUser); + }, 1500); + }); + + const fetchPromise = new Promise(async (resolve) => { + publicUser.featureFlags = await posthog.getFeatureFlags(publicUser); + resolve(publicUser); + }); + + return Promise.race([fetchPromise, timeoutPromise]); + } } diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 65d1c080d5b7d..8b00566d1187d 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -11,7 +11,6 @@ import { LdapManager } from '@/Ldap/LdapManager.ee'; import { LdapService } from '@/Ldap/LdapService.ee'; import { encryptPassword, saveLdapSynchronization } from '@/Ldap/helpers'; import type { LdapConfig } from '@/Ldap/types'; -import { sanitizeUser } from '@/UserManagement/UserManagementHelper'; import { getCurrentAuthenticationMethod, setCurrentAuthenticationMethod } from '@/sso/ssoHelpers'; import { randomEmail, randomName, uniqueId } from './../shared/random'; @@ -570,24 +569,3 @@ describe('Instance owner should able to delete LDAP users', () => { await authOwnerAgent.post(`/users/${member.id}?transferId=${owner.id}`); }); }); - -test('Sign-type should be returned when listing users', async () => { - const ldapConfig = await createLdapConfig(); - LdapManager.updateConfig(ldapConfig); - - await testDb.createLdapUser( - { - globalRole: globalMemberRole, - }, - uniqueId(), - ); - - const allUsers = await testDb.getAllUsers(); - expect(allUsers.length).toBe(2); - - const ownerUser = allUsers.find((u) => u.email === owner.email)!; - expect(sanitizeUser(ownerUser).signInType).toStrictEqual('email'); - - const memberUser = allUsers.find((u) => u.email !== owner.email)!; - expect(sanitizeUser(memberUser).signInType).toStrictEqual('ldap'); -}); diff --git a/packages/cli/test/integration/shared/testDb.ts b/packages/cli/test/integration/shared/testDb.ts index ff8f598eca8b4..51a9085511258 100644 --- a/packages/cli/test/integration/shared/testDb.ts +++ b/packages/cli/test/integration/shared/testDb.ts @@ -246,6 +246,10 @@ export async function createOwner() { return createUser({ globalRole: await getGlobalOwnerRole() }); } +export async function createMember() { + return createUser({ globalRole: await getGlobalMemberRole() }); +} + export async function createUserShell(globalRole: Role): Promise { if (globalRole.scope !== 'global') { throw new Error(`Invalid role received: ${JSON.stringify(globalRole)}`); diff --git a/packages/cli/test/integration/users.api.test.ts b/packages/cli/test/integration/users.api.test.ts index 0f56258883a15..3f3457d5cb194 100644 --- a/packages/cli/test/integration/users.api.test.ts +++ b/packages/cli/test/integration/users.api.test.ts @@ -60,49 +60,6 @@ beforeEach(async () => { config.set('userManagement.emails.smtp.host', ''); }); -describe('GET /users', () => { - test('should return all users (for owner)', async () => { - await testDb.createUser({ globalRole: globalMemberRole }); - - const response = await authOwnerAgent.get('/users'); - - expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(2); - - response.body.data.map((user: User) => { - const { - id, - email, - firstName, - lastName, - personalizationAnswers, - globalRole, - password, - isPending, - apiKey, - } = user; - - expect(validator.isUUID(id)).toBe(true); - expect(email).toBeDefined(); - expect(firstName).toBeDefined(); - expect(lastName).toBeDefined(); - expect(personalizationAnswers).toBeUndefined(); - expect(password).toBeUndefined(); - expect(isPending).toBe(false); - expect(globalRole).toBeDefined(); - expect(apiKey).not.toBeDefined(); - }); - }); - - test('should return all users (for member)', async () => { - const member = await testDb.createUser({ globalRole: globalMemberRole }); - const response = await testServer.authAgentFor(member).get('/users'); - - expect(response.statusCode).toBe(200); - expect(response.body.data.length).toBe(2); - }); -}); - describe('DELETE /users/:id', () => { test('should delete the user', async () => { const userToDelete = await testDb.createUser({ globalRole: globalMemberRole }); diff --git a/packages/cli/test/integration/users.controller.test.ts b/packages/cli/test/integration/users.controller.test.ts new file mode 100644 index 0000000000000..0dc5d41a28daa --- /dev/null +++ b/packages/cli/test/integration/users.controller.test.ts @@ -0,0 +1,248 @@ +import * as testDb from './shared/testDb'; +import { setupTestServer } from './shared/utils/'; +import type { User } from '@/databases/entities/User'; +import type { PublicUser } from '@/Interfaces'; + +const { any } = expect; + +const testServer = setupTestServer({ endpointGroups: ['users'] }); + +let owner: User; +let member: User; + +beforeEach(async () => { + await testDb.truncate(['User']); + owner = await testDb.createOwner(); + member = await testDb.createMember(); +}); + +const validatePublicUser = (user: PublicUser) => { + expect(typeof user.id).toBe('string'); + expect(user.email).toBeDefined(); + expect(user.firstName).toBeDefined(); + expect(user.lastName).toBeDefined(); + expect(typeof user.isOwner).toBe('boolean'); + expect(user.isPending).toBe(false); + expect(user.signInType).toBe('email'); + expect(user.settings).toBe(null); + expect(user.personalizationAnswers).toBeNull(); + expect(user.password).toBeUndefined(); + expect(user.globalRole).toBeDefined(); +}; + +describe('GET /users', () => { + test('should return all users', async () => { + const response = await testServer.authAgentFor(owner).get('/users').expect(200); + + expect(response.body.data).toHaveLength(2); + + response.body.data.forEach(validatePublicUser); + + const _response = await testServer.authAgentFor(member).get('/users').expect(200); + + expect(_response.body.data).toHaveLength(2); + + _response.body.data.forEach(validatePublicUser); + }); + + describe('filter', () => { + test('should filter users by field: email', async () => { + const secondMember = await testDb.createMember(); + + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query(`filter={ "email": "${secondMember.email}" }`) + .expect(200); + + expect(response.body.data).toHaveLength(1); + + const [user] = response.body.data; + + expect(user.email).toBe(secondMember.email); + + const _response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "email": "non@existing.com" }') + .expect(200); + + expect(_response.body.data).toHaveLength(0); + }); + + test('should filter users by field: firstName', async () => { + const secondMember = await testDb.createMember(); + + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query(`filter={ "firstName": "${secondMember.firstName}" }`) + .expect(200); + + expect(response.body.data).toHaveLength(1); + + const [user] = response.body.data; + + expect(user.email).toBe(secondMember.email); + + const _response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "firstName": "Non-Existing" }') + .expect(200); + + expect(_response.body.data).toHaveLength(0); + }); + + test('should filter users by field: lastName', async () => { + const secondMember = await testDb.createMember(); + + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query(`filter={ "lastName": "${secondMember.lastName}" }`) + .expect(200); + + expect(response.body.data).toHaveLength(1); + + const [user] = response.body.data; + + expect(user.email).toBe(secondMember.email); + + const _response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "lastName": "Non-Existing" }') + .expect(200); + + expect(_response.body.data).toHaveLength(0); + }); + + test('should filter users by computed field: isOwner', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "isOwner": true }') + .expect(200); + + expect(response.body.data).toHaveLength(1); + + const [user] = response.body.data; + + expect(user.isOwner).toBe(true); + + const _response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "isOwner": false }') + .expect(200); + + expect(_response.body.data).toHaveLength(1); + + const [_user] = _response.body.data; + + expect(_user.isOwner).toBe(false); + }); + }); + + describe('select', () => { + test('should select user field: id', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('select=["id"]') + .expect(200); + + expect(response.body).toEqual({ + data: [{ id: any(String) }, { id: any(String) }], + }); + }); + + test('should select user field: email', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('select=["email"]') + .expect(200); + + expect(response.body).toEqual({ + data: [{ email: any(String) }, { email: any(String) }], + }); + }); + + test('should select user field: firstName', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('select=["firstName"]') + .expect(200); + + expect(response.body).toEqual({ + data: [{ firstName: any(String) }, { firstName: any(String) }], + }); + }); + + test('should select user field: lastName', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('select=["lastName"]') + .expect(200); + + expect(response.body).toEqual({ + data: [{ lastName: any(String) }, { lastName: any(String) }], + }); + }); + }); + + describe('take', () => { + test('should return n users or less, without skip', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('take=2') + .expect(200); + + expect(response.body.data).toHaveLength(2); + + response.body.data.forEach(validatePublicUser); + + const _response = await testServer + .authAgentFor(owner) + .get('/users') + .query('take=1') + .expect(200); + + expect(_response.body.data).toHaveLength(1); + + _response.body.data.forEach(validatePublicUser); + }); + + test('should return n users or less, with skip', async () => { + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('take=1&skip=1') + .expect(200); + + expect(response.body.data).toHaveLength(1); + + response.body.data.forEach(validatePublicUser); + }); + }); + + describe('combinations', () => { + test('should support options that require auxiliary fields', async () => { + // isOwner requires globalRole + // id-less select with take requires id + + const response = await testServer + .authAgentFor(owner) + .get('/users') + .query('filter={ "isOwner": true }&select=["firstName"]&take=10') + .expect(200); + + expect(response.body).toEqual({ data: [{ firstName: any(String) }] }); + }); + }); +}); diff --git a/packages/cli/test/unit/controllers/me.controller.test.ts b/packages/cli/test/unit/controllers/me.controller.test.ts index f45c2a1256f9d..e2970be7d7f8b 100644 --- a/packages/cli/test/unit/controllers/me.controller.test.ts +++ b/packages/cli/test/unit/controllers/me.controller.test.ts @@ -2,7 +2,7 @@ import type { CookieOptions, Response } from 'express'; import jwt from 'jsonwebtoken'; import { mock, anyObject, captor } from 'jest-mock-extended'; import type { ILogger } from 'n8n-workflow'; -import type { IExternalHooksClass, IInternalHooksClass } from '@/Interfaces'; +import type { IExternalHooksClass, IInternalHooksClass, PublicUser } from '@/Interfaces'; import type { User } from '@db/entities/User'; import { MeController } from '@/controllers'; import { AUTH_COOKIE_NAME } from '@/constants'; @@ -45,6 +45,7 @@ describe('MeController', () => { const res = mock(); userService.findOneOrFail.mockResolvedValue(user); jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token'); + userService.toPublic.mockResolvedValue({} as unknown as PublicUser); await controller.updateCurrentUser(req, res); diff --git a/packages/cli/test/unit/middleware/listQuery.test.ts b/packages/cli/test/unit/middleware/listQuery.test.ts index dc119d1ed4ff6..218c7d25123f7 100644 --- a/packages/cli/test/unit/middleware/listQuery.test.ts +++ b/packages/cli/test/unit/middleware/listQuery.test.ts @@ -134,12 +134,12 @@ describe('List query middleware', () => { expect(nextFn).toBeCalledTimes(1); }); - test('should ignore skip without take', () => { + test('should throw on skip without take', () => { mockReq.query = { skip: '1' }; paginationListQueryMiddleware(...args); expect(mockReq.listQueryOptions).toBeUndefined(); - expect(nextFn).toBeCalledTimes(1); + expect(sendErrorResponse).toHaveBeenCalledTimes(1); }); test('should default skip to 0', () => {