From 5c76f1ec56dea862543c79a06352bbd92ee8afdf Mon Sep 17 00:00:00 2001 From: James Gee <1285296+geemanjs@users.noreply.github.com> Date: Fri, 5 Dec 2025 21:04:12 +0100 Subject: [PATCH] fix: PAY-4074 - Owner registration in multi-main setup (#22520) Signed-off-by: James Gee Signed-off-by: James Gee <1285296+geemanjs@users.noreply.github.com> --- .../src/auth/__tests__/auth.service.test.ts | 29 +++- packages/cli/src/auth/auth.service.ts | 7 +- .../cli/src/commands/user-management/reset.ts | 7 +- packages/cli/src/config/schema.ts | 6 +- packages/cli/src/config/types.ts | 1 - .../__tests__/invitation.controller.test.ts | 139 +++++------------- .../__tests__/owner.controller.test.ts | 87 ++--------- .../cli/src/controllers/e2e.controller.ts | 13 +- .../src/controllers/invitation.controller.ts | 5 +- .../cli/src/controllers/owner.controller.ts | 61 +------- packages/cli/src/server.ts | 8 +- .../__tests__/frontend.service.test.ts | 40 +++-- .../ownership.service.integration.test.ts | 60 ++++++++ .../__tests__/ownership.service.test.ts | 72 ++++++++- packages/cli/src/services/frontend.service.ts | 22 +-- .../cli/src/services/ownership.service.ts | 76 +++++++++- .../cli/test/integration/api-keys.api.test.ts | 1 - .../cli/test/integration/auth.api.test.ts | 1 - .../integration/commands/reset.cmd.test.ts | 12 -- .../external-secrets.api.test.ts | 2 - .../test/integration/ldap/ldap.api.test.ts | 2 - packages/cli/test/integration/me.api.test.ts | 2 - .../cli/test/integration/owner.api.test.ts | 9 +- .../cli/test/integration/shared/db/users.ts | 3 +- .../test/integration/shared/utils/index.ts | 24 +-- .../integration/shared/utils/test-server.ts | 2 - 26 files changed, 343 insertions(+), 348 deletions(-) create mode 100644 packages/cli/src/services/__tests__/ownership.service.integration.test.ts diff --git a/packages/cli/src/auth/__tests__/auth.service.test.ts b/packages/cli/src/auth/__tests__/auth.service.test.ts index d1e89284fc8..573f23ad7dc 100644 --- a/packages/cli/src/auth/__tests__/auth.service.test.ts +++ b/packages/cli/src/auth/__tests__/auth.service.test.ts @@ -6,6 +6,7 @@ import type { InvalidAuthTokenRepository, UserRepository, } from '@n8n/db'; +import { GLOBAL_OWNER_ROLE } from '@n8n/db'; import type { NextFunction, Response } from 'express'; import { mock } from 'jest-mock-extended'; import jwt from 'jsonwebtoken'; @@ -15,6 +16,7 @@ import { AUTH_COOKIE_NAME } from '@/constants'; import type { MfaService } from '@/mfa/mfa.service'; import { JwtService } from '@/services/jwt.service'; import type { UrlService } from '@/services/url.service'; +import type { License } from '@/license'; describe('AuthService', () => { const browserId = 'test-browser-id'; @@ -35,10 +37,11 @@ describe('AuthService', () => { const userRepository = mock(); const invalidAuthTokenRepository = mock(); const mfaService = mock(); + const license = mock(); const authService = new AuthService( globalConfig, mock(), - mock(), + license, jwtService, urlService, userRepository, @@ -61,6 +64,7 @@ describe('AuthService', () => { globalConfig.userManagement.jwtSessionDurationHours = 168; globalConfig.userManagement.jwtRefreshTimeoutHours = 0; globalConfig.auth.cookie = { secure: true, samesite: 'lax' }; + license.isWithinUsersLimit.mockReturnValue(true); }); describe('createJWTHash', () => { @@ -520,6 +524,29 @@ describe('AuthService', () => { }); }); + describe('when user limit is reached', () => { + it('should block issuance if the user is not the global owner', async () => { + license.isWithinUsersLimit.mockReturnValue(false); + expect(() => { + authService.issueCookie(res, user, false, browserId); + }).toThrowError('Maximum number of users reached'); + }); + + it('should allow issuance if the user is the global owner', async () => { + license.isWithinUsersLimit.mockReturnValue(false); + user.role = GLOBAL_OWNER_ROLE; + expect(() => { + authService.issueCookie(res, user, false, browserId); + }).not.toThrowError('Maximum number of users reached'); + expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, { + httpOnly: true, + maxAge: 604800000, + sameSite: 'lax', + secure: true, + }); + }); + }); + it('should issue a cookie with the correct options, when 2FA was used', () => { authService.issueCookie(res, user, true, browserId); diff --git a/packages/cli/src/auth/auth.service.ts b/packages/cli/src/auth/auth.service.ts index ee139c40167..ccf72bd1049 100644 --- a/packages/cli/src/auth/auth.service.ts +++ b/packages/cli/src/auth/auth.service.ts @@ -10,7 +10,6 @@ import type { NextFunction, Response } from 'express'; import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken'; import type { StringValue as TimeUnitValue } from 'ms'; -import config from '@/config'; import { AuthError } from '@/errors/response-errors/auth.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import { License } from '@/license'; @@ -171,11 +170,7 @@ export class AuthService { // TODO: move this check to the login endpoint in AuthController // If the instance has exceeded its user quota, prevent non-owners from logging in const isWithinUsersLimit = this.license.isWithinUsersLimit(); - if ( - config.getEnv('userManagement.isInstanceOwnerSetUp') && - user.role.slug !== GLOBAL_OWNER_ROLE.slug && - !isWithinUsersLimit - ) { + if (user.role.slug !== GLOBAL_OWNER_ROLE.slug && !isWithinUsersLimit) { throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); } diff --git a/packages/cli/src/commands/user-management/reset.ts b/packages/cli/src/commands/user-management/reset.ts index cddd2027d82..91a9289d053 100644 --- a/packages/cli/src/commands/user-management/reset.ts +++ b/packages/cli/src/commands/user-management/reset.ts @@ -3,7 +3,6 @@ import { User, CredentialsRepository, ProjectRepository, - SettingsRepository, SharedCredentialsRepository, SharedWorkflowRepository, UserRepository, @@ -19,6 +18,7 @@ const defaultUserProps = { lastName: null, email: null, password: null, + lastActiveAt: null, role: 'global:owner', }; @@ -53,11 +53,6 @@ export class Reset extends BaseCommand { ); await Container.get(SharedCredentialsRepository).save(newSharedCredentials); - await Container.get(SettingsRepository).update( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: 'false' }, - ); - this.logger.info('Successfully reset the database to default user state.'); } diff --git a/packages/cli/src/config/schema.ts b/packages/cli/src/config/schema.ts index 294f200878a..fe2a30ff312 100644 --- a/packages/cli/src/config/schema.ts +++ b/packages/cli/src/config/schema.ts @@ -7,10 +7,12 @@ import { Container } from '@n8n/di'; export const schema = { userManagement: { /** - * @important Do not remove until after cloud hooks are updated to stop using convict config. + * @important Do not remove isInstanceOwnerSetUp until after cloud hooks (user-management) are updated to stop using + * this property + * @deprecated */ isInstanceOwnerSetUp: { - // n8n loads this setting from DB on startup + // n8n loads this setting from SettingsRepository (DB) on startup doc: "Whether the instance owner's account has been set up", format: Boolean, default: false, diff --git a/packages/cli/src/config/types.ts b/packages/cli/src/config/types.ts index aae7da82f63..314d6fa5931 100644 --- a/packages/cli/src/config/types.ts +++ b/packages/cli/src/config/types.ts @@ -76,7 +76,6 @@ type ToReturnType = T extends NumericPath type ExceptionPaths = { 'queue.bull.redis': RedisOptions; processedDataManager: IProcessedDataConfig; - 'userManagement.isInstanceOwnerSetUp': boolean; 'ui.banners.dismissed': string[] | undefined; easyAIWorkflowOnboarded: boolean | undefined; }; diff --git a/packages/cli/src/controllers/__tests__/invitation.controller.test.ts b/packages/cli/src/controllers/__tests__/invitation.controller.test.ts index 638baf6fa0b..4f3a1211ee7 100644 --- a/packages/cli/src/controllers/__tests__/invitation.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/invitation.controller.test.ts @@ -22,6 +22,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; import config from '@/config'; import type { AuthlessRequest } from '@/requests'; import { v4 as uuidv4 } from 'uuid'; +import { OwnershipService } from '@/services/ownership.service'; describe('InvitationController', () => { const logger: Logger = mockInstance(Logger); @@ -33,22 +34,29 @@ describe('InvitationController', () => { const userRepository: UserRepository = mockInstance(UserRepository); const postHog: PostHogClient = mockInstance(PostHogClient); const eventService: EventService = mockInstance(EventService); + const ownershipService: OwnershipService = mockInstance(OwnershipService); + + function defaultInvitationController() { + return new InvitationController( + logger, + externalHooks, + authService, + userService, + license, + passwordUtility, + userRepository, + postHog, + eventService, + ownershipService, + ); + } describe('inviteUser', () => { it('throws a BadRequestError if SSO is enabled', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(true); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const user = mock({ id: '123', @@ -77,18 +85,9 @@ describe('InvitationController', () => { it('throws a ForbiddenError if the user limit quota has been reached', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false); jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const user = mock({ id: '123', @@ -112,18 +111,9 @@ describe('InvitationController', () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false); jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(true); jest.spyOn(config, 'getEnv').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(false)); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const user = mock({ id: '123', @@ -148,18 +138,9 @@ describe('InvitationController', () => { jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(true); jest.spyOn(config, 'getEnv').mockReturnValue(true); jest.spyOn(license, 'isAdvancedPermissionsLicensed').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const user = mock({ id: '123', @@ -209,17 +190,9 @@ describe('InvitationController', () => { jest.spyOn(config, 'getEnv').mockReturnValue(true); jest.spyOn(license, 'isAdvancedPermissionsLicensed').mockReturnValue(true); jest.spyOn(userService, 'inviteUsers').mockResolvedValue(inviteUsersResult); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); + + const invitationController = defaultInvitationController(); const user = mock({ id: '123', @@ -255,19 +228,11 @@ describe('InvitationController', () => { describe('acceptInvitation', () => { it('throws a BadRequestError if SSO is enabled', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(true); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); + const id = uuidv4(); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const payload = new AcceptInvitationRequestDto({ inviterId: id, @@ -291,19 +256,11 @@ describe('InvitationController', () => { it('throws a BadRequestError if the inviter ID and invitee ID are not found in the database', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); + const id = uuidv4(); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const payload = new AcceptInvitationRequestDto({ inviterId: id, @@ -332,6 +289,8 @@ describe('InvitationController', () => { it('throws a BadRequestError if the invitee already has a password', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); + const invitee = mock({ id: '123', email: 'valid@email.com', @@ -346,17 +305,7 @@ describe('InvitationController', () => { jest.spyOn(userRepository, 'find').mockResolvedValue([inviter, invitee]); const id = uuidv4(); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const payload = new AcceptInvitationRequestDto({ inviterId: id, @@ -379,6 +328,8 @@ describe('InvitationController', () => { it('accepts the invitation successfully', async () => { jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false); + jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true)); + const id = uuidv4(); const inviter = mock({ id: '124', @@ -400,17 +351,7 @@ describe('InvitationController', () => { jest.spyOn(userService, 'toPublic').mockResolvedValue(invitee as unknown as PublicUser); jest.spyOn(externalHooks, 'run').mockResolvedValue(invitee as never); - const invitationController = new InvitationController( - logger, - externalHooks, - authService, - userService, - license, - passwordUtility, - userRepository, - postHog, - eventService, - ); + const invitationController = defaultInvitationController(); const payload = new AcceptInvitationRequestDto({ inviterId: id, diff --git a/packages/cli/src/controllers/__tests__/owner.controller.test.ts b/packages/cli/src/controllers/__tests__/owner.controller.test.ts index ebabd8cb697..1edfbd8af20 100644 --- a/packages/cli/src/controllers/__tests__/owner.controller.test.ts +++ b/packages/cli/src/controllers/__tests__/owner.controller.test.ts @@ -1,103 +1,40 @@ -import type { DismissBannerRequestDto, OwnerSetupRequestDto } from '@n8n/api-types'; -import type { Logger } from '@n8n/backend-common'; -import { - type AuthenticatedRequest, - type User, - type PublicUser, - type SettingsRepository, - type UserRepository, - GLOBAL_OWNER_ROLE, -} from '@n8n/db'; -import type { Response } from 'express'; +import type { DismissBannerRequestDto } from '@n8n/api-types'; import { mock } from 'jest-mock-extended'; import type { AuthService } from '@/auth/auth.service'; -import config from '@/config'; import { OwnerController } from '@/controllers/owner.controller'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import type { EventService } from '@/events/event.service'; import type { BannerService } from '@/services/banner.service'; -import type { PasswordUtility } from '@/services/password.utility'; import type { UserService } from '@/services/user.service'; +import type { OwnershipService } from '@/services/ownership.service'; +import type { PostHogClient } from '@/posthog'; describe('OwnerController', () => { - const configGetSpy = jest.spyOn(config, 'getEnv'); - const configSetSpy = jest.spyOn(config, 'set'); - - const logger = mock(); - const eventService = mock(); const authService = mock(); const bannerService = mock(); const userService = mock(); - const userRepository = mock(); - const settingsRepository = mock(); - const passwordUtility = mock(); + const ownershipService = mock(); + const postHogClient = mock(); const controller = new OwnerController( - logger, - eventService, - settingsRepository, authService, bannerService, userService, - passwordUtility, - mock(), - userRepository, + postHogClient, + ownershipService, ); describe('setupOwner', () => { - it('should throw a BadRequestError if the instance owner is already setup', async () => { - configGetSpy.mockReturnValue(true); + it('should pass on errors from the service', async () => { + jest + .spyOn(ownershipService, 'setupOwner') + .mockRejectedValueOnce(new BadRequestError('Instance owner already setup')); + await expect(controller.setupOwner(mock(), mock(), mock())).rejects.toThrowError( new BadRequestError('Instance owner already setup'), ); - expect(userRepository.findOneOrFail).not.toHaveBeenCalled(); - expect(userRepository.save).not.toHaveBeenCalled(); expect(authService.issueCookie).not.toHaveBeenCalled(); - expect(settingsRepository.update).not.toHaveBeenCalled(); - expect(configSetSpy).not.toHaveBeenCalled(); - expect(eventService.emit).not.toHaveBeenCalled(); - expect(logger.debug).toHaveBeenCalledWith( - 'Request to claim instance ownership failed because instance owner already exists', - ); - }); - - it('should setup the instance owner successfully', async () => { - const user = mock({ - id: 'userId', - role: GLOBAL_OWNER_ROLE, - authIdentities: [], - }); - const browserId = 'test-browser-id'; - const req = mock({ user, browserId, authInfo: { usedMfa: false } }); - const res = mock(); - const payload = mock({ - email: 'valid@email.com', - password: 'NewPassword123', - firstName: 'Jane', - lastName: 'Doe', - }); - configGetSpy.mockReturnValue(false); - userRepository.findOneOrFail.mockResolvedValue(user); - userRepository.save.mockResolvedValue(user); - userService.toPublic.mockResolvedValue(mock({ id: 'newUserId' })); - - const result = await controller.setupOwner(req, res, payload); - - expect(userRepository.findOneOrFail).toHaveBeenCalledWith({ - where: { role: { slug: GLOBAL_OWNER_ROLE.slug } }, - relations: ['role'], - }); - expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false }); - expect(authService.issueCookie).toHaveBeenCalledWith(res, user, false, browserId); - expect(settingsRepository.update).toHaveBeenCalledWith( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: JSON.stringify(true) }, - ); - expect(configSetSpy).toHaveBeenCalledWith('userManagement.isInstanceOwnerSetUp', true); - expect(eventService.emit).toHaveBeenCalledWith('instance-owner-setup', { userId: 'userId' }); - expect(result.id).toEqual('newUserId'); }); }); diff --git a/packages/cli/src/controllers/e2e.controller.ts b/packages/cli/src/controllers/e2e.controller.ts index 3548867b510..8faf4910330 100644 --- a/packages/cli/src/controllers/e2e.controller.ts +++ b/packages/cli/src/controllers/e2e.controller.ts @@ -16,7 +16,6 @@ import { Request } from 'express'; import { v4 as uuid } from 'uuid'; import { ActiveWorkflowManager } from '@/active-workflow-manager'; -import config from '@/config'; import { inE2ETests } from '@/constants'; import { MessageEventBus } from '@/eventbus/message-event-bus/message-event-bus'; import type { FeatureReturnType } from '@/license'; @@ -223,8 +222,7 @@ export class E2EController { @Get('/env-feature-flags', { skipAuth: true }) async getEnvFeatureFlags() { - const currentFlags = this.frontendService.getSettings().envFeatureFlags; - return currentFlags; + return (await this.frontendService.getSettings()).envFeatureFlags; } @Patch('/env-feature-flags', { skipAuth: true }) @@ -254,7 +252,7 @@ export class E2EController { } // Return the current environment feature flags - const currentFlags = this.frontendService.getSettings().envFeatureFlags; + const currentFlags = (await this.frontendService.getSettings()).envFeatureFlags; return { success: true, message: 'Environment feature flags updated', @@ -364,13 +362,6 @@ export class E2EController { mfaRecoveryCodes: encryptedRecoveryCodes, }); } - - await this.settingsRepo.update( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: 'true' }, - ); - - config.set('userManagement.isInstanceOwnerSetUp', true); } private async resetCache() { diff --git a/packages/cli/src/controllers/invitation.controller.ts b/packages/cli/src/controllers/invitation.controller.ts index 2be98a7f2b8..84391593a8b 100644 --- a/packages/cli/src/controllers/invitation.controller.ts +++ b/packages/cli/src/controllers/invitation.controller.ts @@ -6,7 +6,6 @@ import { Post, GlobalScope, RestController, Body, Param } from '@n8n/decorators' import { Response } from 'express'; import { AuthService } from '@/auth/auth.service'; -import config from '@/config'; import { RESPONSE_ERROR_MESSAGES } from '@/constants'; import { BadRequestError } from '@/errors/response-errors/bad-request.error'; import { ForbiddenError } from '@/errors/response-errors/forbidden.error'; @@ -17,6 +16,7 @@ import { PostHogClient } from '@/posthog'; import { AuthlessRequest } from '@/requests'; import { PasswordUtility } from '@/services/password.utility'; import { UserService } from '@/services/user.service'; +import { OwnershipService } from '@/services/ownership.service'; import { isSsoCurrentAuthenticationMethod } from '@/sso.ee/sso-helpers'; @RestController('/invitations') @@ -31,6 +31,7 @@ export class InvitationController { private readonly userRepository: UserRepository, private readonly postHog: PostHogClient, private readonly eventService: EventService, + private readonly ownershipService: OwnershipService, ) {} /** @@ -64,7 +65,7 @@ export class InvitationController { throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED); } - if (!config.getEnv('userManagement.isInstanceOwnerSetUp')) { + if (!(await this.ownershipService.hasInstanceOwner())) { this.logger.debug( 'Request to send email invite(s) to user(s) failed because the owner account is not set up', ); diff --git a/packages/cli/src/controllers/owner.controller.ts b/packages/cli/src/controllers/owner.controller.ts index 6192fed4cb8..1365cbb526b 100644 --- a/packages/cli/src/controllers/owner.controller.ts +++ b/packages/cli/src/controllers/owner.controller.ts @@ -1,82 +1,31 @@ import { DismissBannerRequestDto, OwnerSetupRequestDto } from '@n8n/api-types'; -import { Logger } from '@n8n/backend-common'; -import { - AuthenticatedRequest, - GLOBAL_OWNER_ROLE, - SettingsRepository, - UserRepository, -} from '@n8n/db'; +import { AuthenticatedRequest } from '@n8n/db'; import { Body, GlobalScope, Post, RestController } from '@n8n/decorators'; import { Response } from 'express'; import { AuthService } from '@/auth/auth.service'; -import config from '@/config'; -import { BadRequestError } from '@/errors/response-errors/bad-request.error'; -import { EventService } from '@/events/event.service'; -import { validateEntity } from '@/generic-helpers'; import { PostHogClient } from '@/posthog'; import { BannerService } from '@/services/banner.service'; -import { PasswordUtility } from '@/services/password.utility'; import { UserService } from '@/services/user.service'; +import { OwnershipService } from '@/services/ownership.service'; @RestController('/owner') export class OwnerController { constructor( - private readonly logger: Logger, - private readonly eventService: EventService, - private readonly settingsRepository: SettingsRepository, private readonly authService: AuthService, private readonly bannerService: BannerService, private readonly userService: UserService, - private readonly passwordUtility: PasswordUtility, private readonly postHog: PostHogClient, - private readonly userRepository: UserRepository, + private readonly ownershipService: OwnershipService, ) {} /** - * Promote a shell into the owner of the n8n instance, - * and enable `isInstanceOwnerSetUp` setting. + * Promote a shell into the owner of the n8n instance */ @Post('/setup', { skipAuth: true }) async setupOwner(req: AuthenticatedRequest, res: Response, @Body payload: OwnerSetupRequestDto) { - const { email, firstName, lastName, password } = payload; - - if (config.getEnv('userManagement.isInstanceOwnerSetUp')) { - this.logger.debug( - 'Request to claim instance ownership failed because instance owner already exists', - ); - throw new BadRequestError('Instance owner already setup'); - } - - let owner = await this.userRepository.findOneOrFail({ - where: { role: { slug: GLOBAL_OWNER_ROLE.slug } }, - relations: ['role'], - }); - owner.email = email; - owner.firstName = firstName; - owner.lastName = lastName; - owner.password = await this.passwordUtility.hash(password); - - // TODO: move XSS validation out into the DTO class - await validateEntity(owner); - - owner = await this.userRepository.save(owner, { transaction: false }); - - this.logger.info('Owner was set up successfully'); - - await this.settingsRepository.update( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: JSON.stringify(true) }, - ); - - config.set('userManagement.isInstanceOwnerSetUp', true); - - this.logger.debug('Setting isInstanceOwnerSetUp updated successfully'); - + const owner = await this.ownershipService.setupOwner(payload); this.authService.issueCookie(res, owner, req.authInfo?.usedMfa ?? false, req.browserId); - - this.eventService.emit('instance-owner-setup', { userId: owner.id }); - return await this.userService.toPublic(owner, { posthog: this.postHog, withScopes: true }); } diff --git a/packages/cli/src/server.ts b/packages/cli/src/server.ts index a5fc76eaca8..e237016a263 100644 --- a/packages/cli/src/server.ts +++ b/packages/cli/src/server.ts @@ -200,7 +200,7 @@ export class Server extends AbstractServer { const { frontendService } = this; if (frontendService) { - await this.externalHooks.run('frontend.settings', [frontendService.getSettings()]); + await this.externalHooks.run('frontend.settings', [await frontendService.getSettings()]); } await this.postHogClient.init(); @@ -215,7 +215,7 @@ export class Server extends AbstractServer { const { apiRouters, apiLatestVersion } = await loadPublicApiVersions(publicApiEndpoint); this.app.use(...apiRouters); if (frontendService) { - frontendService.settings.publicApi.latestVersion = apiLatestVersion; + (await frontendService.getSettings()).publicApi.latestVersion = apiLatestVersion; } } @@ -487,7 +487,9 @@ export class Server extends AbstractServer { `/${this.restEndpoint}/settings`, authService.createAuthMiddleware({ allowSkipMFA: false, allowUnauthenticated: true }), ResponseHelper.send(async (req: AuthenticatedRequest) => { - return req.user ? frontendService.getSettings() : frontendService.getPublicSettings(); + return req.user + ? await frontendService.getSettings() + : await frontendService.getPublicSettings(); }), ); } diff --git a/packages/cli/src/services/__tests__/frontend.service.test.ts b/packages/cli/src/services/__tests__/frontend.service.test.ts index 9da6558e4e2..94c061f809f 100644 --- a/packages/cli/src/services/__tests__/frontend.service.test.ts +++ b/packages/cli/src/services/__tests__/frontend.service.test.ts @@ -14,6 +14,7 @@ import type { PushConfig } from '@/push/push.config'; import { FrontendService, type PublicFrontendSettings } from '@/services/frontend.service'; import type { UrlService } from '@/services/url.service'; import type { UserManagementMailer } from '@/user-management/email'; +import type { OwnershipService } from '../ownership.service'; // Mock the workflow history helper functions to avoid DI container issues in tests jest.mock('@/workflows/workflow-history/workflow-history-helper', () => ({ @@ -148,6 +149,10 @@ describe('FrontendService', () => { isMFAEnforced: jest.fn().mockReturnValue(false), }); + const ownershipService = mock({ + hasInstanceOwner: jest.fn().mockReturnValue(false), + }); + const createMockService = () => { Container.set( CommunityPackagesConfig, @@ -173,6 +178,7 @@ describe('FrontendService', () => { licenseState, moduleRegistry, mfaService, + ownershipService, ), license, }; @@ -188,9 +194,9 @@ describe('FrontendService', () => { }); describe('getSettings', () => { - it('should return frontend settings', () => { + it('should return frontend settings', async () => { const { service } = createMockService(); - const settings = service.getSettings(); + const settings = await service.getSettings(); expect(settings).toEqual( expect.objectContaining({ @@ -201,7 +207,7 @@ describe('FrontendService', () => { }); describe('getPublicSettings', () => { - it('should return public settings', () => { + it('should return public settings', async () => { const expectedPublicSettings: PublicFrontendSettings = { settingsMode: 'public', userManagement: { @@ -223,7 +229,7 @@ describe('FrontendService', () => { }; const { service } = createMockService(); - const settings = service.getPublicSettings(); + const settings = await service.getPublicSettings(); expect(settings).toEqual(expectedPublicSettings); }); @@ -282,29 +288,31 @@ describe('FrontendService', () => { }); describe('settings integration', () => { - it('should include envFeatureFlags in initial settings', () => { + it('should include envFeatureFlags in initial settings', async () => { process.env = { N8N_ENV_FEAT_INIT_FLAG: 'true', N8N_ENV_FEAT_ANOTHER_FLAG: 'false', }; const { service } = createMockService(); + const settings = await service.getSettings(); - expect(service.settings.envFeatureFlags).toEqual({ + expect(settings.envFeatureFlags).toEqual({ N8N_ENV_FEAT_INIT_FLAG: 'true', N8N_ENV_FEAT_ANOTHER_FLAG: 'false', }); }); - it('should refresh envFeatureFlags when getSettings is called', () => { + it('should refresh envFeatureFlags when getSettings is called', async () => { process.env = { N8N_ENV_FEAT_INITIAL_FLAG: 'true', }; const { service } = createMockService(); + const initialSettings = await service.getSettings(); // Verify initial state - expect(service.settings.envFeatureFlags).toEqual({ + expect(initialSettings.envFeatureFlags).toEqual({ N8N_ENV_FEAT_INITIAL_FLAG: 'true', }); @@ -315,7 +323,7 @@ describe('FrontendService', () => { }; // getSettings should refresh the flags - const settings = service.getSettings(); + const settings = await service.getSettings(); expect(settings.envFeatureFlags).toEqual({ N8N_ENV_FEAT_INITIAL_FLAG: 'false', @@ -326,33 +334,33 @@ describe('FrontendService', () => { }); describe('aiBuilder setting', () => { - it('should initialize aiBuilder setting as disabled by default', () => { + it('should initialize aiBuilder setting as disabled by default', async () => { const { service } = createMockService(); - - expect(service.settings.aiBuilder).toEqual({ + const initialSettings = await service.getSettings(); + expect(initialSettings.aiBuilder).toEqual({ enabled: false, setup: false, }); }); - it('should set aiBuilder.enabled to true when license has feat:aiBuilder', () => { + it('should set aiBuilder.enabled to true when license has feat:aiBuilder', async () => { const { service, license } = createMockService(); license.isLicensed.mockImplementation((feature) => { return feature === 'feat:aiBuilder'; }); - const settings = service.getSettings(); + const settings = await service.getSettings(); expect(settings.aiBuilder.enabled).toBe(true); }); - it('should keep aiBuilder.enabled as false when license does not have feat:aiBuilder', () => { + it('should keep aiBuilder.enabled as false when license does not have feat:aiBuilder', async () => { const { service, license } = createMockService(); license.isLicensed.mockReturnValue(false); - const settings = service.getSettings(); + const settings = await service.getSettings(); expect(settings.aiBuilder.enabled).toBe(false); }); diff --git a/packages/cli/src/services/__tests__/ownership.service.integration.test.ts b/packages/cli/src/services/__tests__/ownership.service.integration.test.ts new file mode 100644 index 00000000000..30c0fabdfc1 --- /dev/null +++ b/packages/cli/src/services/__tests__/ownership.service.integration.test.ts @@ -0,0 +1,60 @@ +import { testDb } from '@n8n/backend-test-utils'; +import { GLOBAL_OWNER_ROLE } from '@n8n/db'; +import { Container } from '@n8n/di'; + +import { HooksService } from '@/services/hooks.service'; +import { OwnershipService } from '@/services/ownership.service'; +import { createUserShell } from '@test-integration/db/users'; + +let hookService: HooksService; +let ownershipService: OwnershipService; + +// See PAY-4247 - This test case can be deleted when the ticket is complete +describe('Ownership Service integration test', () => { + beforeEach(async () => { + await testDb.truncate(['User']); + await createUserShell(GLOBAL_OWNER_ROLE); + jest.clearAllMocks(); + }); + + beforeAll(async () => { + await testDb.init(); + hookService = Container.get(HooksService); + ownershipService = Container.get(OwnershipService); + }); + + afterAll(async () => { + await testDb.terminate(); + }); + + it('should recognise ownership creation from cloud hooks', async () => { + expect(await ownershipService.hasInstanceOwner()).toBeFalsy(); + const shellOwnerUser = await hookService.findOneUser({ + where: { + role: { + slug: GLOBAL_OWNER_ROLE.slug, + }, + }, + }); + // @ts-expect-error - this is how this function is called in the cloud hook so I match it here + await hookService.saveUser({ + firstName: 'FN', + lastName: 'LN', + email: 'fn@ln.com', + password: '', + id: shellOwnerUser!.id, + }); + expect(await ownershipService.hasInstanceOwner()).toBeTruthy(); + }); + + it('should recognise ownership creation from api', async () => { + expect(await ownershipService.hasInstanceOwner()).toBeFalsy(); + await ownershipService.setupOwner({ + firstName: 'TEST', + lastName: 'LN', + password: 'PW', + email: 'EM@em.com', + }); + expect(await ownershipService.hasInstanceOwner()).toBeTruthy(); + }); +}); diff --git a/packages/cli/src/services/__tests__/ownership.service.test.ts b/packages/cli/src/services/__tests__/ownership.service.test.ts index 824e2756d10..d9e5a278d9b 100644 --- a/packages/cli/src/services/__tests__/ownership.service.test.ts +++ b/packages/cli/src/services/__tests__/ownership.service.test.ts @@ -1,5 +1,5 @@ +import { Logger } from '@n8n/backend-common'; import { mockInstance } from '@n8n/backend-test-utils'; -import type { SharedCredentials } from '@n8n/db'; import { Project, SharedWorkflow, @@ -12,10 +12,15 @@ import { GLOBAL_OWNER_ROLE, PROJECT_OWNER_ROLE, } from '@n8n/db'; +import type { SharedCredentials, SettingsRepository } from '@n8n/db'; import { PROJECT_OWNER_ROLE_SLUG } from '@n8n/permissions'; +import { mock } from 'jest-mock-extended'; import { v4 as uuid } from 'uuid'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import type { EventService } from '@/events/event.service'; import { OwnershipService } from '@/services/ownership.service'; +import { PasswordUtility } from '@/services/password.utility'; import { mockCredential, mockProject } from '@test/mock-objects'; import { CacheService } from '../cache/cache.service'; @@ -25,11 +30,20 @@ describe('OwnershipService', () => { const sharedWorkflowRepository = mockInstance(SharedWorkflowRepository); const projectRelationRepository = mockInstance(ProjectRelationRepository); const cacheService = mockInstance(CacheService); + const passwordUtility = mockInstance(PasswordUtility); + const logger = mockInstance(Logger); + const eventService = mock(); + const settingsRepository = mock(); + const ownershipService = new OwnershipService( cacheService, - userRepository, + eventService, + logger, + passwordUtility, projectRelationRepository, sharedWorkflowRepository, + userRepository, + settingsRepository, ); beforeEach(() => { @@ -67,7 +81,8 @@ describe('OwnershipService', () => { owner.role = GLOBAL_OWNER_ROLE; const projectRelation = new ProjectRelation(); projectRelation.role = PROJECT_OWNER_ROLE; - (projectRelation.project = project), (projectRelation.user = owner); + projectRelation.project = project; + projectRelation.user = owner; projectRelationRepository.getPersonalProjectOwners.mockResolvedValueOnce([projectRelation]); @@ -94,8 +109,9 @@ describe('OwnershipService', () => { owner.id = uuid(); owner.role = GLOBAL_OWNER_ROLE; const projectRelation = new ProjectRelation(); - projectRelation.role = { slug: PROJECT_OWNER_ROLE_SLUG } as any; - (projectRelation.project = project), (projectRelation.user = owner); + projectRelation.role = PROJECT_OWNER_ROLE; + projectRelation.project = project; + projectRelation.user = owner; cacheService.getHashValue.mockResolvedValueOnce(owner); userRepository.create.mockReturnValueOnce(owner); @@ -226,4 +242,50 @@ describe('OwnershipService', () => { }); }); }); + + describe('setupOwner()', () => { + it('should throw a BadRequestError if the instance owner is already setup', async () => { + jest.spyOn(userRepository, 'exists').mockResolvedValueOnce(true); + + await expect(ownershipService.setupOwner(mock())).rejects.toThrowError( + new BadRequestError('Instance owner already setup'), + ); + + expect(userRepository.save).not.toHaveBeenCalled(); + expect(eventService.emit).not.toHaveBeenCalled(); + expect(logger.debug).toHaveBeenCalledWith( + 'Request to claim instance ownership failed because instance owner already exists', + ); + }); + + it('should setup the instance owner successfully', async () => { + const user = mock({ + id: 'userId', + role: GLOBAL_OWNER_ROLE, + authIdentities: [], + }); + + const payload = { + email: 'valid@email.com', + password: 'NewPassword123', + firstName: 'Jane', + lastName: 'Doe', + }; + + // not quite perfect as we hash the password. + const expected = { ...user, ...payload, id: 'newUserId' }; + + userRepository.exists.mockResolvedValueOnce(false); + userRepository.findOneOrFail.mockResolvedValueOnce(user); + userRepository.save.mockResolvedValueOnce(expected); + + const actual = await ownershipService.setupOwner(payload); + + expect(userRepository.save).toHaveBeenCalledWith(user, { transaction: false }); + expect(eventService.emit).toHaveBeenCalledWith('instance-owner-setup', { + userId: 'newUserId', + }); + expect(actual.id).toEqual('newUserId'); + }); + }); }); diff --git a/packages/cli/src/services/frontend.service.ts b/packages/cli/src/services/frontend.service.ts index 36902840555..ff7759f6f5b 100644 --- a/packages/cli/src/services/frontend.service.ts +++ b/packages/cli/src/services/frontend.service.ts @@ -20,6 +20,7 @@ import { getLdapLoginLabel } from '@/ldap.ee/helpers.ee'; import { License } from '@/license'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; import { MfaService } from '@/mfa/mfa.service'; +import { OwnershipService } from '@/services/ownership.service'; import { CommunityPackagesConfig } from '@/modules/community-packages/community-packages.config'; import type { CommunityPackagesService } from '@/modules/community-packages/community-packages.service'; import { isApiEnabled } from '@/public-api'; @@ -93,7 +94,7 @@ export type PublicFrontendSettings = { @Service() export class FrontendService { - settings: FrontendSettings; + private settings: FrontendSettings; private communityPackagesService?: CommunityPackagesService; @@ -113,12 +114,10 @@ export class FrontendService { private readonly licenseState: LicenseState, private readonly moduleRegistry: ModuleRegistry, private readonly mfaService: MfaService, + private readonly ownershipService: OwnershipService, ) { loadNodesAndCredentials.addPostProcessor(async () => await this.generateTypes()); void this.generateTypes(); - - this.initSettings(); - // @TODO: Move to community-packages module if (Container.get(CommunityPackagesConfig).enabled) { void import('@/modules/community-packages/community-packages.service').then( @@ -141,7 +140,7 @@ export class FrontendService { return envFeatureFlags; } - private initSettings() { + private async initSettings() { const instanceBaseUrl = this.urlService.getInstanceBaseUrl(); const restEndpoint = this.globalConfig.endpoints.rest; @@ -230,7 +229,7 @@ export class FrontendService { defaultLocale: this.globalConfig.defaultLocale, userManagement: { quota: this.license.getUsersLimit(), - showSetupOnFirstLoad: !config.getEnv('userManagement.isInstanceOwnerSetUp'), + showSetupOnFirstLoad: !(await this.ownershipService.hasInstanceOwner()), smtpSetup: this.mailer.isEmailSetUp, authenticationMethod: getCurrentAuthenticationMethod(), }, @@ -374,7 +373,10 @@ export class FrontendService { this.writeStaticJSON('credentials', credentials); } - getSettings(): FrontendSettings { + async getSettings(): Promise { + if (!this.settings) { + await this.initSettings(); + } const restEndpoint = this.globalConfig.endpoints.rest; // Update all urls, in case `WEBHOOK_URL` was updated by `--tunnel` @@ -390,7 +392,7 @@ export class FrontendService { Object.assign(this.settings.userManagement, { quota: this.license.getUsersLimit(), authenticationMethod: getCurrentAuthenticationMethod(), - showSetupOnFirstLoad: !config.getEnv('userManagement.isInstanceOwnerSetUp'), + showSetupOnFirstLoad: !(await this.ownershipService.hasInstanceOwner()), }); let dismissedBanners: string[] = []; @@ -517,7 +519,7 @@ export class FrontendService { * Only add settings that are absolutely necessary for non-authenticated pages * @returns Public settings for unauthenticated users */ - getPublicSettings(): PublicFrontendSettings { + async getPublicSettings(): Promise { // Get full settings to ensure all required properties are initialized const { userManagement: { authenticationMethod, showSetupOnFirstLoad, smtpSetup }, @@ -525,7 +527,7 @@ export class FrontendService { authCookie, previewMode, enterprise: { saml, ldap, oidc }, - } = this.getSettings(); + } = await this.getSettings(); const publicSettings: PublicFrontendSettings = { settingsMode: 'public', diff --git a/packages/cli/src/services/ownership.service.ts b/packages/cli/src/services/ownership.service.ts index 196fc39c914..46c44d45181 100644 --- a/packages/cli/src/services/ownership.service.ts +++ b/packages/cli/src/services/ownership.service.ts @@ -7,19 +7,31 @@ import { SharedWorkflowRepository, UserRepository, Role, + SettingsRepository, Scope, } from '@n8n/db'; import { Service } from '@n8n/di'; - +import { Logger } from '@n8n/backend-common'; import { CacheService } from '@/services/cache/cache.service'; +import { OwnerSetupRequestDto } from '@n8n/api-types'; +import { BadRequestError } from '@/errors/response-errors/bad-request.error'; +import { EventService } from '@/events/event.service'; +import { PasswordUtility } from './password.utility'; +import { IsNull } from '@n8n/typeorm/find-options/operator/IsNull'; +import { Not } from '@n8n/typeorm/find-options/operator/Not'; +import config from '@/config'; @Service() export class OwnershipService { constructor( private cacheService: CacheService, - private userRepository: UserRepository, + private eventService: EventService, + private logger: Logger, + private passwordUtility: PasswordUtility, private projectRelationRepository: ProjectRelationRepository, private sharedWorkflowRepository: SharedWorkflowRepository, + private userRepository: UserRepository, + private settingsRepository: SettingsRepository, ) {} // To make use of the cache service we should store POJOs, these @@ -179,4 +191,64 @@ export class OwnershipService { where: { role: { slug: GLOBAL_OWNER_ROLE.slug } }, }); } + + async hasInstanceOwner() { + return await this.userRepository.exists({ + where: [ + { + role: { slug: GLOBAL_OWNER_ROLE.slug }, + // We use this to avoid selecting the "shell" user + lastActiveAt: Not(IsNull()), + }, + // OR + // This condition only exists because of PAY-4247 + { + role: { slug: GLOBAL_OWNER_ROLE.slug }, + // We use this to avoid selecting the "shell" user + password: Not(IsNull()), + }, + ], + relations: ['role'], + }); + } + + async setupOwner(payload: OwnerSetupRequestDto) { + const { email, firstName, lastName, password } = payload; + if (await this.hasInstanceOwner()) { + this.logger.debug( + 'Request to claim instance ownership failed because instance owner already exists', + ); + throw new BadRequestError('Instance owner already setup'); + } + + let shellUser = await this.userRepository.findOneOrFail({ + where: { role: { slug: GLOBAL_OWNER_ROLE.slug } }, + relations: ['role'], + }); + + shellUser.email = email; + shellUser.firstName = firstName; + shellUser.lastName = lastName; + shellUser.lastActiveAt = new Date(); + shellUser.password = await this.passwordUtility.hash(password); + + shellUser = await this.userRepository.save(shellUser, { transaction: false }); + + this.logger.info('Owner was set up successfully'); + this.eventService.emit('instance-owner-setup', { userId: shellUser.id }); + + // The next block needs to be deleted and is temporary for now + // See packages/cli/src/config/schema.ts for more info + // We update the SettingsRepository so when we "startup" next time + // the config state is restored. + // #region Delete me + await this.settingsRepository.update( + { key: 'userManagement.isInstanceOwnerSetUp' }, + { value: JSON.stringify(true) }, + ); + config.set('userManagement.isInstanceOwnerSetUp', true); + // #endregion + + return shellUser; + } } diff --git a/packages/cli/test/integration/api-keys.api.test.ts b/packages/cli/test/integration/api-keys.api.test.ts index 5187c60bedc..1bd4f4d5794 100644 --- a/packages/cli/test/integration/api-keys.api.test.ts +++ b/packages/cli/test/integration/api-keys.api.test.ts @@ -326,7 +326,6 @@ describe('Member', () => { password: memberPassword, role: GLOBAL_MEMBER_ROLE, }); - await utils.setInstanceOwnerSetUp(true); }); test('POST /api-keys should create an api key with no expiration', async () => { diff --git a/packages/cli/test/integration/auth.api.test.ts b/packages/cli/test/integration/auth.api.test.ts index b4f0aa87e50..bc7ae310552 100644 --- a/packages/cli/test/integration/auth.api.test.ts +++ b/packages/cli/test/integration/auth.api.test.ts @@ -31,7 +31,6 @@ beforeAll(async () => { beforeEach(async () => { await testDb.truncate(['User']); config.set('ldap.disabled', true); - await utils.setInstanceOwnerSetUp(true); }); describe('POST /login', () => { diff --git a/packages/cli/test/integration/commands/reset.cmd.test.ts b/packages/cli/test/integration/commands/reset.cmd.test.ts index e1262d5fe3e..728bf72dbbc 100644 --- a/packages/cli/test/integration/commands/reset.cmd.test.ts +++ b/packages/cli/test/integration/commands/reset.cmd.test.ts @@ -7,7 +7,6 @@ import { } from '@n8n/backend-test-utils'; import { CredentialsEntity, - SettingsRepository, CredentialsRepository, SharedCredentialsRepository, SharedWorkflowRepository, @@ -54,12 +53,6 @@ test('user-management:reset should reset DB to default user state', async () => await encryptCredentialData(Object.assign(new CredentialsEntity(), randomCredentialPayload())), ); - // mark instance as set up - await Container.get(SettingsRepository).update( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: 'true' }, - ); - // // ACT // @@ -100,9 +93,4 @@ test('user-management:reset should reset DB to default user state', async () => await expect( Container.get(SharedCredentialsRepository).findBy({ credentialsId: danglingCredential.id }), ).resolves.toMatchObject([{ projectId: ownerProject.id, role: 'credential:owner' }]); - - // the instance is marked as not set up: - await expect( - Container.get(SettingsRepository).findBy({ key: 'userManagement.isInstanceOwnerSetUp' }), - ).resolves.toMatchObject([{ value: 'false' }]); }); diff --git a/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts b/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts index 2f4aa357027..e4328176b7a 100644 --- a/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts +++ b/packages/cli/test/integration/external-secrets/external-secrets.api.test.ts @@ -6,7 +6,6 @@ import { mock } from 'jest-mock-extended'; import { Cipher } from 'n8n-core'; import type { IDataObject } from 'n8n-workflow'; -import config from '@/config'; import { CREDENTIAL_BLANKING_VALUE } from '@/constants'; import type { EventService } from '@/events/event.service'; import { License } from '@/license'; @@ -112,7 +111,6 @@ beforeAll(async () => { authOwnerAgent = testServer.authAgentFor(owner); const member = await createUser(); authMemberAgent = testServer.authAgentFor(member); - config.set('userManagement.isInstanceOwnerSetUp', true); Container.set( ExternalSecretsManager, new ExternalSecretsManager( diff --git a/packages/cli/test/integration/ldap/ldap.api.test.ts b/packages/cli/test/integration/ldap/ldap.api.test.ts index 403c86951ac..cdcfffe1f97 100644 --- a/packages/cli/test/integration/ldap/ldap.api.test.ts +++ b/packages/cli/test/integration/ldap/ldap.api.test.ts @@ -64,8 +64,6 @@ beforeEach(async () => { jest.mock('@/telemetry'); - config.set('userManagement.isInstanceOwnerSetUp', true); - await setCurrentAuthenticationMethod('email'); }); diff --git a/packages/cli/test/integration/me.api.test.ts b/packages/cli/test/integration/me.api.test.ts index df0b2b35c8c..708c53f88b4 100644 --- a/packages/cli/test/integration/me.api.test.ts +++ b/packages/cli/test/integration/me.api.test.ts @@ -144,7 +144,6 @@ describe('Member', () => { role: { slug: 'global:member' }, }); authMemberAgent = testServer.authAgentFor(member); - await utils.setInstanceOwnerSetUp(true); }); test('PATCH /me should succeed with valid inputs', async () => { @@ -286,7 +285,6 @@ describe('Chat User', () => { role: { slug: 'global:chatUser' }, }); authMemberAgent = testServer.authAgentFor(member); - await utils.setInstanceOwnerSetUp(true); }); test('PATCH /me should succeed with valid inputs', async () => { diff --git a/packages/cli/test/integration/owner.api.test.ts b/packages/cli/test/integration/owner.api.test.ts index f1f096e6258..52c1eb8deca 100644 --- a/packages/cli/test/integration/owner.api.test.ts +++ b/packages/cli/test/integration/owner.api.test.ts @@ -7,11 +7,10 @@ import { } from '@n8n/backend-test-utils'; import type { User } from '@n8n/db'; import { GLOBAL_OWNER_ROLE, UserRepository } from '@n8n/db'; +import { OwnershipService } from '@/services/ownership.service'; import { Container } from '@n8n/di'; import validator from 'validator'; -import config from '@/config'; - import { createUserShell } from './shared/db/users'; import * as utils from './shared/utils/'; @@ -21,7 +20,6 @@ let ownerShell: User; beforeEach(async () => { ownerShell = await createUserShell(GLOBAL_OWNER_ROLE); - config.set('userManagement.isInstanceOwnerSetUp', false); }); afterEach(async () => { @@ -71,10 +69,7 @@ describe('POST /owner/setup', () => { expect(storedOwner.firstName).toBe(newOwnerData.firstName); expect(storedOwner.lastName).toBe(newOwnerData.lastName); - const isInstanceOwnerSetUpConfig = config.getEnv('userManagement.isInstanceOwnerSetUp'); - expect(isInstanceOwnerSetUpConfig).toBe(true); - - const isInstanceOwnerSetUpSetting = await utils.isInstanceOwnerSetUp(); + const isInstanceOwnerSetUpSetting = await Container.get(OwnershipService).hasInstanceOwner(); expect(isInstanceOwnerSetUpSetting).toBe(true); }); diff --git a/packages/cli/test/integration/shared/db/users.ts b/packages/cli/test/integration/shared/db/users.ts index 12831513dcc..eb59e4a48e5 100644 --- a/packages/cli/test/integration/shared/db/users.ts +++ b/packages/cli/test/integration/shared/db/users.ts @@ -43,13 +43,14 @@ async function handlePasswordSetup(password: string | null | undefined): Promise /** Store a new user object, defaulting to a `member` */ export async function newUser(attributes: DeepPartial = {}): Promise { - const { email, password, firstName, lastName, role, ...rest } = attributes; + const { email, password, firstName, lastName, role, lastActiveAt, ...rest } = attributes; return Container.get(UserRepository).create({ email: email ?? randomEmail(), password: await handlePasswordSetup(password), firstName: firstName ?? randomName(), lastName: lastName ?? randomName(), role: role ?? GLOBAL_MEMBER_ROLE, + lastActiveAt: lastActiveAt ?? new Date(), ...rest, }); } diff --git a/packages/cli/test/integration/shared/utils/index.ts b/packages/cli/test/integration/shared/utils/index.ts index ce9e02457a8..8157e56f1e5 100644 --- a/packages/cli/test/integration/shared/utils/index.ts +++ b/packages/cli/test/integration/shared/utils/index.ts @@ -1,6 +1,6 @@ import type { Logger } from '@n8n/backend-common'; import { mockInstance } from '@n8n/backend-test-utils'; -import { SettingsRepository, WorkflowEntity } from '@n8n/db'; +import { WorkflowEntity } from '@n8n/db'; import { Container } from '@n8n/di'; import { mock } from 'jest-mock-extended'; import { @@ -25,7 +25,6 @@ import type { INodeTypeData, INode } from 'n8n-workflow'; import type request from 'supertest'; import { v4 as uuid } from 'uuid'; -import config from '@/config'; import { AUTH_COOKIE_NAME } from '@/constants'; import { ExecutionService } from '@/executions/execution.service'; import { LoadNodesAndCredentials } from '@/load-nodes-and-credentials'; @@ -159,27 +158,6 @@ export function getAuthToken(response: request.Response, authCookieName = AUTH_C return match.groups.token; } -// ---------------------------------- -// settings -// ---------------------------------- - -export async function isInstanceOwnerSetUp() { - const { value } = await Container.get(SettingsRepository).findOneByOrFail({ - key: 'userManagement.isInstanceOwnerSetUp', - }); - - return Boolean(value); -} - -export const setInstanceOwnerSetUp = async (value: boolean) => { - config.set('userManagement.isInstanceOwnerSetUp', value); - - await Container.get(SettingsRepository).update( - { key: 'userManagement.isInstanceOwnerSetUp' }, - { value: JSON.stringify(value) }, - ); -}; - // ---------------------------------- // community nodes // ---------------------------------- diff --git a/packages/cli/test/integration/shared/utils/test-server.ts b/packages/cli/test/integration/shared/utils/test-server.ts index 5a6a8ed9239..e10388f23d6 100644 --- a/packages/cli/test/integration/shared/utils/test-server.ts +++ b/packages/cli/test/integration/shared/utils/test-server.ts @@ -10,7 +10,6 @@ import request from 'supertest'; import { URL } from 'url'; import { AuthService } from '@/auth/auth.service'; -import config from '@/config'; import { AUTH_COOKIE_NAME } from '@/constants'; import { ControllerRegistry } from '@/controller.registry'; import { License } from '@/license'; @@ -129,7 +128,6 @@ export const setupTestServer = ({ await testDb.init(); Container.get(GlobalConfig).userManagement.jwtSecret = 'My JWT secret'; - config.set('userManagement.isInstanceOwnerSetUp', true); testServer.license.mock(Container.get(License)); testServer.license.mockLicenseState(Container.get(LicenseState));