Skip to content

Commit 5c76f1e

Browse files
geemanjsgeemanjs
authored
fix: PAY-4074 - Owner registration in multi-main setup (#22520)
Signed-off-by: James Gee <[email protected]> Signed-off-by: James Gee <[email protected]>
1 parent 68693b5 commit 5c76f1e

26 files changed

+343
-348
lines changed

‎packages/cli/src/auth/__tests__/auth.service.test.ts‎

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import type {
66
InvalidAuthTokenRepository,
77
UserRepository,
88
} from '@n8n/db';
9+
import { GLOBAL_OWNER_ROLE } from '@n8n/db';
910
import type { NextFunction, Response } from 'express';
1011
import { mock } from 'jest-mock-extended';
1112
import jwt from 'jsonwebtoken';
@@ -15,6 +16,7 @@ import { AUTH_COOKIE_NAME } from '@/constants';
1516
import type { MfaService } from '@/mfa/mfa.service';
1617
import { JwtService } from '@/services/jwt.service';
1718
import type { UrlService } from '@/services/url.service';
19+
import type { License } from '@/license';
1820

1921
describe('AuthService', () => {
2022
const browserId = 'test-browser-id';
@@ -35,10 +37,11 @@ describe('AuthService', () => {
3537
const userRepository = mock<UserRepository>();
3638
const invalidAuthTokenRepository = mock<InvalidAuthTokenRepository>();
3739
const mfaService = mock<MfaService>();
40+
const license = mock<License>();
3841
const authService = new AuthService(
3942
globalConfig,
4043
mock(),
41-
mock(),
44+
license,
4245
jwtService,
4346
urlService,
4447
userRepository,
@@ -61,6 +64,7 @@ describe('AuthService', () => {
6164
globalConfig.userManagement.jwtSessionDurationHours = 168;
6265
globalConfig.userManagement.jwtRefreshTimeoutHours = 0;
6366
globalConfig.auth.cookie = { secure: true, samesite: 'lax' };
67+
license.isWithinUsersLimit.mockReturnValue(true);
6468
});
6569

6670
describe('createJWTHash', () => {
@@ -520,6 +524,29 @@ describe('AuthService', () => {
520524
});
521525
});
522526

527+
describe('when user limit is reached', () => {
528+
it('should block issuance if the user is not the global owner', async () => {
529+
license.isWithinUsersLimit.mockReturnValue(false);
530+
expect(() => {
531+
authService.issueCookie(res, user, false, browserId);
532+
}).toThrowError('Maximum number of users reached');
533+
});
534+
535+
it('should allow issuance if the user is the global owner', async () => {
536+
license.isWithinUsersLimit.mockReturnValue(false);
537+
user.role = GLOBAL_OWNER_ROLE;
538+
expect(() => {
539+
authService.issueCookie(res, user, false, browserId);
540+
}).not.toThrowError('Maximum number of users reached');
541+
expect(res.cookie).toHaveBeenCalledWith('n8n-auth', validToken, {
542+
httpOnly: true,
543+
maxAge: 604800000,
544+
sameSite: 'lax',
545+
secure: true,
546+
});
547+
});
548+
});
549+
523550
it('should issue a cookie with the correct options, when 2FA was used', () => {
524551
authService.issueCookie(res, user, true, browserId);
525552

‎packages/cli/src/auth/auth.service.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import type { NextFunction, Response } from 'express';
1010
import { JsonWebTokenError, TokenExpiredError } from 'jsonwebtoken';
1111
import type { StringValue as TimeUnitValue } from 'ms';
1212

13-
import config from '@/config';
1413
import { AuthError } from '@/errors/response-errors/auth.error';
1514
import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
1615
import { License } from '@/license';
@@ -171,11 +170,7 @@ export class AuthService {
171170
// TODO: move this check to the login endpoint in AuthController
172171
// If the instance has exceeded its user quota, prevent non-owners from logging in
173172
const isWithinUsersLimit = this.license.isWithinUsersLimit();
174-
if (
175-
config.getEnv('userManagement.isInstanceOwnerSetUp') &&
176-
user.role.slug !== GLOBAL_OWNER_ROLE.slug &&
177-
!isWithinUsersLimit
178-
) {
173+
if (user.role.slug !== GLOBAL_OWNER_ROLE.slug && !isWithinUsersLimit) {
179174
throw new ForbiddenError(RESPONSE_ERROR_MESSAGES.USERS_QUOTA_REACHED);
180175
}
181176

‎packages/cli/src/commands/user-management/reset.ts‎

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import {
33
User,
44
CredentialsRepository,
55
ProjectRepository,
6-
SettingsRepository,
76
SharedCredentialsRepository,
87
SharedWorkflowRepository,
98
UserRepository,
@@ -19,6 +18,7 @@ const defaultUserProps = {
1918
lastName: null,
2019
email: null,
2120
password: null,
21+
lastActiveAt: null,
2222
role: 'global:owner',
2323
};
2424

@@ -53,11 +53,6 @@ export class Reset extends BaseCommand {
5353
);
5454
await Container.get(SharedCredentialsRepository).save(newSharedCredentials);
5555

56-
await Container.get(SettingsRepository).update(
57-
{ key: 'userManagement.isInstanceOwnerSetUp' },
58-
{ value: 'false' },
59-
);
60-
6156
this.logger.info('Successfully reset the database to default user state.');
6257
}
6358

‎packages/cli/src/config/schema.ts‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,12 @@ import { Container } from '@n8n/di';
77
export const schema = {
88
userManagement: {
99
/**
10-
* @important Do not remove until after cloud hooks are updated to stop using convict config.
10+
* @important Do not remove isInstanceOwnerSetUp until after cloud hooks (user-management) are updated to stop using
11+
* this property
12+
* @deprecated
1113
*/
1214
isInstanceOwnerSetUp: {
13-
// n8n loads this setting from DB on startup
15+
// n8n loads this setting from SettingsRepository (DB) on startup
1416
doc: "Whether the instance owner's account has been set up",
1517
format: Boolean,
1618
default: false,

‎packages/cli/src/config/types.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ type ToReturnType<T extends ConfigOptionPath> = T extends NumericPath
7676
type ExceptionPaths = {
7777
'queue.bull.redis': RedisOptions;
7878
processedDataManager: IProcessedDataConfig;
79-
'userManagement.isInstanceOwnerSetUp': boolean;
8079
'ui.banners.dismissed': string[] | undefined;
8180
easyAIWorkflowOnboarded: boolean | undefined;
8281
};

‎packages/cli/src/controllers/__tests__/invitation.controller.test.ts‎

Lines changed: 40 additions & 99 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import { ForbiddenError } from '@/errors/response-errors/forbidden.error';
2222
import config from '@/config';
2323
import type { AuthlessRequest } from '@/requests';
2424
import { v4 as uuidv4 } from 'uuid';
25+
import { OwnershipService } from '@/services/ownership.service';
2526

2627
describe('InvitationController', () => {
2728
const logger: Logger = mockInstance(Logger);
@@ -33,22 +34,29 @@ describe('InvitationController', () => {
3334
const userRepository: UserRepository = mockInstance(UserRepository);
3435
const postHog: PostHogClient = mockInstance(PostHogClient);
3536
const eventService: EventService = mockInstance(EventService);
37+
const ownershipService: OwnershipService = mockInstance(OwnershipService);
38+
39+
function defaultInvitationController() {
40+
return new InvitationController(
41+
logger,
42+
externalHooks,
43+
authService,
44+
userService,
45+
license,
46+
passwordUtility,
47+
userRepository,
48+
postHog,
49+
eventService,
50+
ownershipService,
51+
);
52+
}
3653

3754
describe('inviteUser', () => {
3855
it('throws a BadRequestError if SSO is enabled', async () => {
3956
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(true);
57+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
4058

41-
const invitationController = new InvitationController(
42-
logger,
43-
externalHooks,
44-
authService,
45-
userService,
46-
license,
47-
passwordUtility,
48-
userRepository,
49-
postHog,
50-
eventService,
51-
);
59+
const invitationController = defaultInvitationController();
5260

5361
const user = mock<User>({
5462
id: '123',
@@ -77,18 +85,9 @@ describe('InvitationController', () => {
7785
it('throws a ForbiddenError if the user limit quota has been reached', async () => {
7886
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false);
7987
jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(false);
88+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
8089

81-
const invitationController = new InvitationController(
82-
logger,
83-
externalHooks,
84-
authService,
85-
userService,
86-
license,
87-
passwordUtility,
88-
userRepository,
89-
postHog,
90-
eventService,
91-
);
90+
const invitationController = defaultInvitationController();
9291

9392
const user = mock<User>({
9493
id: '123',
@@ -112,18 +111,9 @@ describe('InvitationController', () => {
112111
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false);
113112
jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(true);
114113
jest.spyOn(config, 'getEnv').mockReturnValue(false);
114+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(false));
115115

116-
const invitationController = new InvitationController(
117-
logger,
118-
externalHooks,
119-
authService,
120-
userService,
121-
license,
122-
passwordUtility,
123-
userRepository,
124-
postHog,
125-
eventService,
126-
);
116+
const invitationController = defaultInvitationController();
127117

128118
const user = mock<User>({
129119
id: '123',
@@ -148,18 +138,9 @@ describe('InvitationController', () => {
148138
jest.spyOn(license, 'isWithinUsersLimit').mockReturnValue(true);
149139
jest.spyOn(config, 'getEnv').mockReturnValue(true);
150140
jest.spyOn(license, 'isAdvancedPermissionsLicensed').mockReturnValue(false);
141+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
151142

152-
const invitationController = new InvitationController(
153-
logger,
154-
externalHooks,
155-
authService,
156-
userService,
157-
license,
158-
passwordUtility,
159-
userRepository,
160-
postHog,
161-
eventService,
162-
);
143+
const invitationController = defaultInvitationController();
163144

164145
const user = mock<User>({
165146
id: '123',
@@ -209,17 +190,9 @@ describe('InvitationController', () => {
209190
jest.spyOn(config, 'getEnv').mockReturnValue(true);
210191
jest.spyOn(license, 'isAdvancedPermissionsLicensed').mockReturnValue(true);
211192
jest.spyOn(userService, 'inviteUsers').mockResolvedValue(inviteUsersResult);
212-
const invitationController = new InvitationController(
213-
logger,
214-
externalHooks,
215-
authService,
216-
userService,
217-
license,
218-
passwordUtility,
219-
userRepository,
220-
postHog,
221-
eventService,
222-
);
193+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
194+
195+
const invitationController = defaultInvitationController();
223196

224197
const user = mock<User>({
225198
id: '123',
@@ -255,19 +228,11 @@ describe('InvitationController', () => {
255228
describe('acceptInvitation', () => {
256229
it('throws a BadRequestError if SSO is enabled', async () => {
257230
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(true);
231+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
232+
258233
const id = uuidv4();
259234

260-
const invitationController = new InvitationController(
261-
logger,
262-
externalHooks,
263-
authService,
264-
userService,
265-
license,
266-
passwordUtility,
267-
userRepository,
268-
postHog,
269-
eventService,
270-
);
235+
const invitationController = defaultInvitationController();
271236

272237
const payload = new AcceptInvitationRequestDto({
273238
inviterId: id,
@@ -291,19 +256,11 @@ describe('InvitationController', () => {
291256

292257
it('throws a BadRequestError if the inviter ID and invitee ID are not found in the database', async () => {
293258
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false);
259+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
260+
294261
const id = uuidv4();
295262

296-
const invitationController = new InvitationController(
297-
logger,
298-
externalHooks,
299-
authService,
300-
userService,
301-
license,
302-
passwordUtility,
303-
userRepository,
304-
postHog,
305-
eventService,
306-
);
263+
const invitationController = defaultInvitationController();
307264

308265
const payload = new AcceptInvitationRequestDto({
309266
inviterId: id,
@@ -332,6 +289,8 @@ describe('InvitationController', () => {
332289

333290
it('throws a BadRequestError if the invitee already has a password', async () => {
334291
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false);
292+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
293+
335294
const invitee = mock<User>({
336295
id: '123',
337296
email: '[email protected]',
@@ -346,17 +305,7 @@ describe('InvitationController', () => {
346305
jest.spyOn(userRepository, 'find').mockResolvedValue([inviter, invitee]);
347306
const id = uuidv4();
348307

349-
const invitationController = new InvitationController(
350-
logger,
351-
externalHooks,
352-
authService,
353-
userService,
354-
license,
355-
passwordUtility,
356-
userRepository,
357-
postHog,
358-
eventService,
359-
);
308+
const invitationController = defaultInvitationController();
360309

361310
const payload = new AcceptInvitationRequestDto({
362311
inviterId: id,
@@ -379,6 +328,8 @@ describe('InvitationController', () => {
379328

380329
it('accepts the invitation successfully', async () => {
381330
jest.spyOn(ssoHelpers, 'isSsoCurrentAuthenticationMethod').mockReturnValue(false);
331+
jest.spyOn(ownershipService, 'hasInstanceOwner').mockReturnValue(Promise.resolve(true));
332+
382333
const id = uuidv4();
383334
const inviter = mock<User>({
384335
id: '124',
@@ -400,17 +351,7 @@ describe('InvitationController', () => {
400351
jest.spyOn(userService, 'toPublic').mockResolvedValue(invitee as unknown as PublicUser);
401352
jest.spyOn(externalHooks, 'run').mockResolvedValue(invitee as never);
402353

403-
const invitationController = new InvitationController(
404-
logger,
405-
externalHooks,
406-
authService,
407-
userService,
408-
license,
409-
passwordUtility,
410-
userRepository,
411-
postHog,
412-
eventService,
413-
);
354+
const invitationController = defaultInvitationController();
414355

415356
const payload = new AcceptInvitationRequestDto({
416357
inviterId: id,

0 commit comments

Comments
 (0)