mirror of
https://github.com/Abdulazizzn/n8n-enterprise-unlocked.git
synced 2025-12-16 17:46:45 +00:00
chore(core): Enforce email format for user entity, remove unused user validators (#18534)
This commit is contained in:
@@ -38,7 +38,8 @@
|
|||||||
"p-lazy": "3.1.0",
|
"p-lazy": "3.1.0",
|
||||||
"reflect-metadata": "catalog:",
|
"reflect-metadata": "catalog:",
|
||||||
"uuid": "catalog:",
|
"uuid": "catalog:",
|
||||||
"xss": "catalog:"
|
"xss": "catalog:",
|
||||||
|
"zod": "catalog:"
|
||||||
},
|
},
|
||||||
"devDependencies": {
|
"devDependencies": {
|
||||||
"@n8n/typescript-config": "workspace:*",
|
"@n8n/typescript-config": "workspace:*",
|
||||||
|
|||||||
@@ -32,4 +32,85 @@ describe('User Entity', () => {
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('preUpsertHook email validation', () => {
|
||||||
|
describe('valid email scenarios', () => {
|
||||||
|
it('should allow null email', () => {
|
||||||
|
const user = new User();
|
||||||
|
(user as { email: string | null }).email = null;
|
||||||
|
|
||||||
|
expect(() => user.preUpsertHook()).not.toThrow();
|
||||||
|
expect(user.email).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should allow undefined email', () => {
|
||||||
|
const user = new User();
|
||||||
|
(user as { email: string | undefined }).email = undefined;
|
||||||
|
|
||||||
|
expect(() => user.preUpsertHook()).not.toThrow();
|
||||||
|
expect(user.email).toBeNull();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should lowercase valid emails', () => {
|
||||||
|
const user = new User();
|
||||||
|
user.email = 'TEST@EXAMPLE.COM';
|
||||||
|
|
||||||
|
expect(() => user.preUpsertHook()).not.toThrow();
|
||||||
|
expect(user.email).toBe('test@example.com');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should accept standard valid email formats', () => {
|
||||||
|
const validEmails = [
|
||||||
|
'test@example.com',
|
||||||
|
'user.name@example.com',
|
||||||
|
'user+tag@example.com',
|
||||||
|
'user123@example123.com',
|
||||||
|
'test@sub.example.com',
|
||||||
|
'a@b.co',
|
||||||
|
];
|
||||||
|
|
||||||
|
validEmails.forEach((email) => {
|
||||||
|
const user = new User();
|
||||||
|
user.email = email;
|
||||||
|
|
||||||
|
expect(() => user.preUpsertHook()).not.toThrow();
|
||||||
|
expect(user.email).toBe(email.toLowerCase());
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('invalid email scenarios', () => {
|
||||||
|
it.each([
|
||||||
|
['test..email@example.com'],
|
||||||
|
['test@'],
|
||||||
|
['@example.com'],
|
||||||
|
['test@@example.com'],
|
||||||
|
['test @example.com'],
|
||||||
|
['test@ example.com'],
|
||||||
|
['.test@example.com'],
|
||||||
|
['test.@example.com'],
|
||||||
|
['test@example.'],
|
||||||
|
['test@.example.com'],
|
||||||
|
['test<>@example.com'],
|
||||||
|
['test[]@example.com'],
|
||||||
|
['test()@example.com'],
|
||||||
|
['test,@example.com'],
|
||||||
|
['test;@example.com'],
|
||||||
|
['test:@example.com'],
|
||||||
|
['test"@example.com'],
|
||||||
|
['test\\@example.com'],
|
||||||
|
[''],
|
||||||
|
[' '],
|
||||||
|
['a'.repeat(300) + '@invalid'],
|
||||||
|
])('should throw Error for invalid email <%s>', (email) => {
|
||||||
|
const user = new User();
|
||||||
|
user.email = email;
|
||||||
|
|
||||||
|
expect(() => user.preUpsertHook()).toThrow(Error);
|
||||||
|
expect(() => user.preUpsertHook()).toThrow(
|
||||||
|
`Cannot save user <${email}>: Provided email is invalid`,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -10,7 +10,6 @@ import {
|
|||||||
PrimaryGeneratedColumn,
|
PrimaryGeneratedColumn,
|
||||||
BeforeInsert,
|
BeforeInsert,
|
||||||
} from '@n8n/typeorm';
|
} from '@n8n/typeorm';
|
||||||
import { IsEmail, IsString, Length } from 'class-validator';
|
|
||||||
import type { IUser, IUserSettings } from 'n8n-workflow';
|
import type { IUser, IUserSettings } from 'n8n-workflow';
|
||||||
|
|
||||||
import { JsonColumn, WithTimestamps } from './abstract-entity';
|
import { JsonColumn, WithTimestamps } from './abstract-entity';
|
||||||
@@ -20,9 +19,8 @@ import type { ProjectRelation } from './project-relation';
|
|||||||
import type { SharedCredentials } from './shared-credentials';
|
import type { SharedCredentials } from './shared-credentials';
|
||||||
import type { SharedWorkflow } from './shared-workflow';
|
import type { SharedWorkflow } from './shared-workflow';
|
||||||
import type { IPersonalizationSurveyAnswers } from './types-db';
|
import type { IPersonalizationSurveyAnswers } from './types-db';
|
||||||
|
import { isValidEmail } from '../utils/is-valid-email';
|
||||||
import { lowerCaser, objectRetriever } from '../utils/transformers';
|
import { lowerCaser, objectRetriever } from '../utils/transformers';
|
||||||
import { NoUrl } from '../utils/validators/no-url.validator';
|
|
||||||
import { NoXss } from '../utils/validators/no-xss.validator';
|
|
||||||
|
|
||||||
@Entity()
|
@Entity()
|
||||||
export class User extends WithTimestamps implements IUser, AuthPrincipal {
|
export class User extends WithTimestamps implements IUser, AuthPrincipal {
|
||||||
@@ -35,25 +33,15 @@ export class User extends WithTimestamps implements IUser, AuthPrincipal {
|
|||||||
transformer: lowerCaser,
|
transformer: lowerCaser,
|
||||||
})
|
})
|
||||||
@Index({ unique: true })
|
@Index({ unique: true })
|
||||||
@IsEmail()
|
|
||||||
email: string;
|
email: string;
|
||||||
|
|
||||||
@Column({ length: 32, nullable: true })
|
@Column({ length: 32, nullable: true })
|
||||||
@NoXss()
|
|
||||||
@NoUrl()
|
|
||||||
@IsString({ message: 'First name must be of type string.' })
|
|
||||||
@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
|
|
||||||
firstName: string;
|
firstName: string;
|
||||||
|
|
||||||
@Column({ length: 32, nullable: true })
|
@Column({ length: 32, nullable: true })
|
||||||
@NoXss()
|
|
||||||
@NoUrl()
|
|
||||||
@IsString({ message: 'Last name must be of type string.' })
|
|
||||||
@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
|
|
||||||
lastName: string;
|
lastName: string;
|
||||||
|
|
||||||
@Column({ type: String, nullable: true })
|
@Column({ type: String, nullable: true })
|
||||||
@IsString({ message: 'Password must be of type string.' })
|
|
||||||
password: string | null;
|
password: string | null;
|
||||||
|
|
||||||
@JsonColumn({
|
@JsonColumn({
|
||||||
@@ -90,6 +78,14 @@ export class User extends WithTimestamps implements IUser, AuthPrincipal {
|
|||||||
@BeforeUpdate()
|
@BeforeUpdate()
|
||||||
preUpsertHook(): void {
|
preUpsertHook(): void {
|
||||||
this.email = this.email?.toLowerCase() ?? null;
|
this.email = this.email?.toLowerCase() ?? null;
|
||||||
|
|
||||||
|
// Validate email if present (including empty strings)
|
||||||
|
if (this.email !== null && this.email !== undefined) {
|
||||||
|
const result = isValidEmail(this.email);
|
||||||
|
if (!result) {
|
||||||
|
throw new Error(`Cannot save user <${this.email}>: Provided email is invalid`);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@Column({ type: Boolean, default: false })
|
@Column({ type: Boolean, default: false })
|
||||||
|
|||||||
@@ -11,6 +11,7 @@ export {
|
|||||||
|
|
||||||
export { generateNanoId } from './utils/generators';
|
export { generateNanoId } from './utils/generators';
|
||||||
export { isStringArray } from './utils/is-string-array';
|
export { isStringArray } from './utils/is-string-array';
|
||||||
|
export { isValidEmail } from './utils/is-valid-email';
|
||||||
export { separate } from './utils/separate';
|
export { separate } from './utils/separate';
|
||||||
export { sql } from './utils/sql';
|
export { sql } from './utils/sql';
|
||||||
export { idStringifier, lowerCaser, objectRetriever, sqlite } from './utils/transformers';
|
export { idStringifier, lowerCaser, objectRetriever, sqlite } from './utils/transformers';
|
||||||
|
|||||||
5
packages/@n8n/db/src/utils/is-valid-email.ts
Normal file
5
packages/@n8n/db/src/utils/is-valid-email.ts
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
import { z } from 'zod';
|
||||||
|
|
||||||
|
export function isValidEmail(email: string): boolean {
|
||||||
|
return z.string().email().safeParse(email).success;
|
||||||
|
}
|
||||||
@@ -2,7 +2,7 @@ import { Logger } from '@n8n/backend-common';
|
|||||||
import { GlobalConfig } from '@n8n/config';
|
import { GlobalConfig } from '@n8n/config';
|
||||||
import type { LdapConfig } from '@n8n/constants';
|
import type { LdapConfig } from '@n8n/constants';
|
||||||
import { LDAP_FEATURE_NAME } from '@n8n/constants';
|
import { LDAP_FEATURE_NAME } from '@n8n/constants';
|
||||||
import { SettingsRepository } from '@n8n/db';
|
import { isValidEmail, SettingsRepository } from '@n8n/db';
|
||||||
import type { User, RunningMode, SyncStatus } from '@n8n/db';
|
import type { User, RunningMode, SyncStatus } from '@n8n/db';
|
||||||
import { Service, Container } from '@n8n/di';
|
import { Service, Container } from '@n8n/di';
|
||||||
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
|
// eslint-disable-next-line n8n-local-rules/misplaced-n8n-typeorm-import
|
||||||
@@ -349,9 +349,25 @@ export class LdapService {
|
|||||||
localAdUsers,
|
localAdUsers,
|
||||||
);
|
);
|
||||||
|
|
||||||
|
const filteredUsersToCreate = usersToCreate.filter(([id, user]) => {
|
||||||
|
if (!isValidEmail(user.email)) {
|
||||||
|
this.logger.warn(`LDAP - Invalid email format for user ${id}`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
|
||||||
|
const filteredUsersToUpdate = usersToUpdate.filter(([id, user]) => {
|
||||||
|
if (!isValidEmail(user.email)) {
|
||||||
|
this.logger.warn(`LDAP - Invalid email format for user ${id}`);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
return true;
|
||||||
|
});
|
||||||
|
|
||||||
this.logger.debug('LDAP - Users to process', {
|
this.logger.debug('LDAP - Users to process', {
|
||||||
created: usersToCreate.length,
|
created: filteredUsersToCreate.length,
|
||||||
updated: usersToUpdate.length,
|
updated: filteredUsersToUpdate.length,
|
||||||
disabled: usersToDisable.length,
|
disabled: usersToDisable.length,
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -361,7 +377,7 @@ export class LdapService {
|
|||||||
|
|
||||||
try {
|
try {
|
||||||
if (mode === 'live') {
|
if (mode === 'live') {
|
||||||
await processUsers(usersToCreate, usersToUpdate, usersToDisable);
|
await processUsers(filteredUsersToCreate, filteredUsersToUpdate, usersToDisable);
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
if (error instanceof QueryFailedError) {
|
if (error instanceof QueryFailedError) {
|
||||||
@@ -373,8 +389,8 @@ export class LdapService {
|
|||||||
await saveLdapSynchronization({
|
await saveLdapSynchronization({
|
||||||
startedAt,
|
startedAt,
|
||||||
endedAt,
|
endedAt,
|
||||||
created: usersToCreate.length,
|
created: filteredUsersToCreate.length,
|
||||||
updated: usersToUpdate.length,
|
updated: filteredUsersToUpdate.length,
|
||||||
disabled: usersToDisable.length,
|
disabled: usersToDisable.length,
|
||||||
scanned: adUsers.length,
|
scanned: adUsers.length,
|
||||||
runMode: mode,
|
runMode: mode,
|
||||||
@@ -385,7 +401,8 @@ export class LdapService {
|
|||||||
this.eventService.emit('ldap-general-sync-finished', {
|
this.eventService.emit('ldap-general-sync-finished', {
|
||||||
type: !this.syncTimer ? 'scheduled' : `manual_${mode}`,
|
type: !this.syncTimer ? 'scheduled' : `manual_${mode}`,
|
||||||
succeeded: true,
|
succeeded: true,
|
||||||
usersSynced: usersToCreate.length + usersToUpdate.length + usersToDisable.length,
|
usersSynced:
|
||||||
|
filteredUsersToCreate.length + filteredUsersToUpdate.length + usersToDisable.length,
|
||||||
error: errorMessage,
|
error: errorMessage,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import { GlobalConfig } from '@n8n/config';
|
|||||||
import {
|
import {
|
||||||
AuthIdentity,
|
AuthIdentity,
|
||||||
AuthIdentityRepository,
|
AuthIdentityRepository,
|
||||||
|
isValidEmail,
|
||||||
SettingsRepository,
|
SettingsRepository,
|
||||||
type User,
|
type User,
|
||||||
UserRepository,
|
UserRepository,
|
||||||
@@ -104,6 +105,10 @@ export class OidcService {
|
|||||||
throw new BadRequestError('An email is required');
|
throw new BadRequestError('An email is required');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!isValidEmail(userInfo.email)) {
|
||||||
|
throw new BadRequestError('Invalid email format');
|
||||||
|
}
|
||||||
|
|
||||||
const openidUser = await this.authIdentityRepository.findOne({
|
const openidUser = await this.authIdentityRepository.findOne({
|
||||||
where: { providerId: claims.sub, providerType: 'oidc' },
|
where: { providerId: claims.sub, providerType: 'oidc' },
|
||||||
relations: ['user'],
|
relations: ['user'],
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
import type { SamlPreferences } from '@n8n/api-types';
|
import type { SamlPreferences } from '@n8n/api-types';
|
||||||
import { Logger } from '@n8n/backend-common';
|
import { Logger } from '@n8n/backend-common';
|
||||||
import type { Settings, User } from '@n8n/db';
|
import type { Settings, User } from '@n8n/db';
|
||||||
import { SettingsRepository, UserRepository } from '@n8n/db';
|
import { isValidEmail, SettingsRepository, UserRepository } from '@n8n/db';
|
||||||
import { Service } from '@n8n/di';
|
import { Service } from '@n8n/di';
|
||||||
import axios from 'axios';
|
import axios from 'axios';
|
||||||
import type express from 'express';
|
import type express from 'express';
|
||||||
@@ -197,6 +197,11 @@ export class SamlService {
|
|||||||
const attributes = await this.getAttributesFromLoginResponse(req, binding);
|
const attributes = await this.getAttributesFromLoginResponse(req, binding);
|
||||||
if (attributes.email) {
|
if (attributes.email) {
|
||||||
const lowerCasedEmail = attributes.email.toLowerCase();
|
const lowerCasedEmail = attributes.email.toLowerCase();
|
||||||
|
|
||||||
|
if (!isValidEmail(lowerCasedEmail)) {
|
||||||
|
throw new BadRequestError('Invalid email format');
|
||||||
|
}
|
||||||
|
|
||||||
const user = await this.userRepository.findOne({
|
const user = await this.userRepository.findOne({
|
||||||
where: { email: lowerCasedEmail },
|
where: { email: lowerCasedEmail },
|
||||||
relations: ['authIdentities'],
|
relations: ['authIdentities'],
|
||||||
|
|||||||
@@ -382,7 +382,7 @@ describe('GET /resolve-signup-token', () => {
|
|||||||
|
|
||||||
// cause inconsistent DB state
|
// cause inconsistent DB state
|
||||||
owner.email = '';
|
owner.email = '';
|
||||||
await Container.get(UserRepository).save(owner);
|
await Container.get(UserRepository).save(owner, { listeners: false });
|
||||||
const fifth = await authOwnerAgent
|
const fifth = await authOwnerAgent
|
||||||
.get('/resolve-signup-token')
|
.get('/resolve-signup-token')
|
||||||
.query({ inviterId: owner.id })
|
.query({ inviterId: owner.id })
|
||||||
|
|||||||
@@ -435,6 +435,94 @@ describe('POST /ldap/sync', () => {
|
|||||||
const response = await testServer.authAgentFor(member).get('/login');
|
const response = await testServer.authAgentFor(member).get('/login');
|
||||||
expect(response.status).toBe(401);
|
expect(response.status).toBe(401);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('should filter out users with invalid email addresses during sync', async () => {
|
||||||
|
const validLdapUser = {
|
||||||
|
mail: randomEmail(),
|
||||||
|
dn: '',
|
||||||
|
sn: randomName(),
|
||||||
|
givenName: randomName(),
|
||||||
|
uid: uniqueId(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const invalidLdapUser = {
|
||||||
|
mail: 'invalid-email',
|
||||||
|
dn: '',
|
||||||
|
sn: randomName(),
|
||||||
|
givenName: randomName(),
|
||||||
|
uid: uniqueId(),
|
||||||
|
};
|
||||||
|
|
||||||
|
const ldapUsers = [validLdapUser, invalidLdapUser];
|
||||||
|
|
||||||
|
const loggerSpy = jest.spyOn(Container.get(LdapService)['logger'], 'warn');
|
||||||
|
|
||||||
|
const synchronization = await runTest(ldapUsers);
|
||||||
|
|
||||||
|
// Should only create 1 user (the valid one)
|
||||||
|
expect(synchronization.created).toBe(1);
|
||||||
|
|
||||||
|
// Should log error for invalid email
|
||||||
|
expect(loggerSpy).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining(`LDAP - Invalid email format for user ${invalidLdapUser.uid}`),
|
||||||
|
);
|
||||||
|
|
||||||
|
loggerSpy.mockReset();
|
||||||
|
loggerSpy.mockRestore();
|
||||||
|
|
||||||
|
// Verify only valid user was created
|
||||||
|
const allUsers = await getAllUsers();
|
||||||
|
expect(allUsers.length).toBe(2); // owner + valid user
|
||||||
|
|
||||||
|
const memberUser = allUsers.find((u) => u.email !== owner.email)!;
|
||||||
|
expect(memberUser.email).toBe(validLdapUser.mail);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should filter out users with invalid email addresses during update', async () => {
|
||||||
|
const originalEmail = randomEmail();
|
||||||
|
const originalUserId = uniqueId();
|
||||||
|
|
||||||
|
// Create user with valid email first
|
||||||
|
await createLdapUser(
|
||||||
|
{
|
||||||
|
role: 'global:member',
|
||||||
|
email: originalEmail,
|
||||||
|
firstName: randomName(),
|
||||||
|
lastName: randomName(),
|
||||||
|
},
|
||||||
|
originalUserId,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Now try to update with invalid email
|
||||||
|
const invalidLdapUser = {
|
||||||
|
mail: 'not-an-email',
|
||||||
|
dn: '',
|
||||||
|
sn: randomName(),
|
||||||
|
givenName: randomName(),
|
||||||
|
uid: originalUserId,
|
||||||
|
};
|
||||||
|
|
||||||
|
const loggerSpy = jest.spyOn(Container.get(LdapService)['logger'], 'warn');
|
||||||
|
|
||||||
|
const synchronization = await runTest([invalidLdapUser]);
|
||||||
|
|
||||||
|
// Should not update any users
|
||||||
|
expect(synchronization.updated).toBe(0);
|
||||||
|
|
||||||
|
// Should log error for invalid email
|
||||||
|
expect(loggerSpy).toHaveBeenCalledWith(
|
||||||
|
expect.stringContaining(`LDAP - Invalid email format for user ${originalUserId}`),
|
||||||
|
);
|
||||||
|
|
||||||
|
loggerSpy.mockReset();
|
||||||
|
loggerSpy.mockRestore();
|
||||||
|
|
||||||
|
// Verify user still has original email
|
||||||
|
const localLdapIdentities = await getLdapIdentities();
|
||||||
|
const localLdapUsers = localLdapIdentities.map(({ user }) => user);
|
||||||
|
expect(localLdapUsers.length).toBe(1);
|
||||||
|
expect(localLdapUsers[0].email).toBe(originalEmail);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
|||||||
@@ -353,6 +353,79 @@ describe('OIDC service', () => {
|
|||||||
await expect(oidcService.loginUser(callbackUrl)).rejects.toThrowError(BadRequestError);
|
await expect(oidcService.loginUser(callbackUrl)).rejects.toThrowError(BadRequestError);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it('should throw `BadRequestError` if OIDC Idp provides an invalid email format', async () => {
|
||||||
|
const callbackUrl = new URL(
|
||||||
|
'http://localhost:5678/rest/sso/oidc/callback?code=valid-code&state=valid-state',
|
||||||
|
);
|
||||||
|
|
||||||
|
const mockTokens: mocked_oidc_client.TokenEndpointResponse &
|
||||||
|
mocked_oidc_client.TokenEndpointResponseHelpers = {
|
||||||
|
access_token: 'mock-access-token-invalid',
|
||||||
|
id_token: 'mock-id-token-invalid',
|
||||||
|
token_type: 'bearer',
|
||||||
|
claims: () => {
|
||||||
|
return {
|
||||||
|
sub: 'mock-subject-invalid',
|
||||||
|
iss: 'https://example.com/auth/realms/n8n',
|
||||||
|
aud: 'test-client-id',
|
||||||
|
iat: Math.floor(Date.now() / 1000) - 1000,
|
||||||
|
exp: Math.floor(Date.now() / 1000) + 3600,
|
||||||
|
} as mocked_oidc_client.IDToken;
|
||||||
|
},
|
||||||
|
expiresIn: () => 3600,
|
||||||
|
} as mocked_oidc_client.TokenEndpointResponse &
|
||||||
|
mocked_oidc_client.TokenEndpointResponseHelpers;
|
||||||
|
|
||||||
|
authorizationCodeGrantMock.mockResolvedValueOnce(mockTokens);
|
||||||
|
|
||||||
|
// Provide an invalid email format
|
||||||
|
fetchUserInfoMock.mockResolvedValueOnce({
|
||||||
|
email_verified: true,
|
||||||
|
email: 'invalid-email-format',
|
||||||
|
});
|
||||||
|
|
||||||
|
const error = await oidcService.loginUser(callbackUrl).catch((e) => e);
|
||||||
|
expect(error.message).toBe('Invalid email format');
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([
|
||||||
|
['not-an-email'],
|
||||||
|
['@missinglocal.com'],
|
||||||
|
['missing@.com'],
|
||||||
|
['spaces in@email.com'],
|
||||||
|
['double@@domain.com'],
|
||||||
|
])('should throw `BadRequestError` for invalid email <%s>', async (invalidEmail) => {
|
||||||
|
const callbackUrl = new URL(
|
||||||
|
'http://localhost:5678/rest/sso/oidc/callback?code=valid-code&state=valid-state',
|
||||||
|
);
|
||||||
|
|
||||||
|
const mockTokens: mocked_oidc_client.TokenEndpointResponse &
|
||||||
|
mocked_oidc_client.TokenEndpointResponseHelpers = {
|
||||||
|
access_token: 'mock-access-token-multi',
|
||||||
|
id_token: 'mock-id-token-multi',
|
||||||
|
token_type: 'bearer',
|
||||||
|
claims: () => {
|
||||||
|
return {
|
||||||
|
sub: 'mock-subject-multi',
|
||||||
|
iss: 'https://example.com/auth/realms/n8n',
|
||||||
|
aud: 'test-client-id',
|
||||||
|
iat: Math.floor(Date.now() / 1000) - 1000,
|
||||||
|
exp: Math.floor(Date.now() / 1000) + 3600,
|
||||||
|
} as mocked_oidc_client.IDToken;
|
||||||
|
},
|
||||||
|
expiresIn: () => 3600,
|
||||||
|
} as mocked_oidc_client.TokenEndpointResponse &
|
||||||
|
mocked_oidc_client.TokenEndpointResponseHelpers;
|
||||||
|
|
||||||
|
authorizationCodeGrantMock.mockResolvedValueOnce(mockTokens);
|
||||||
|
fetchUserInfoMock.mockResolvedValueOnce({
|
||||||
|
email_verified: true,
|
||||||
|
email: invalidEmail,
|
||||||
|
});
|
||||||
|
|
||||||
|
await expect(oidcService.loginUser(callbackUrl)).rejects.toThrowError(BadRequestError);
|
||||||
|
});
|
||||||
|
|
||||||
it('should throw `ForbiddenError` if OIDC token does not provide claims', async () => {
|
it('should throw `ForbiddenError` if OIDC token does not provide claims', async () => {
|
||||||
const callbackUrl = new URL(
|
const callbackUrl = new URL(
|
||||||
'http://localhost:5678/rest/sso/oidc/callback?code=valid-code&state=valid-state',
|
'http://localhost:5678/rest/sso/oidc/callback?code=valid-code&state=valid-state',
|
||||||
|
|||||||
@@ -20,7 +20,7 @@ describe('sso/saml/samlHelpers', () => {
|
|||||||
const samlUserAttributes: SamlUserAttributes = {
|
const samlUserAttributes: SamlUserAttributes = {
|
||||||
firstName: 'Nathan',
|
firstName: 'Nathan',
|
||||||
lastName: 'Nathaniel',
|
lastName: 'Nathaniel',
|
||||||
email: 'n@8.n',
|
email: 'nathan@n8n.io',
|
||||||
userPrincipalName: 'Huh?',
|
userPrincipalName: 'Huh?',
|
||||||
};
|
};
|
||||||
|
|
||||||
|
|||||||
@@ -2,8 +2,11 @@ import { randomEmail, randomName, randomValidPassword } from '@n8n/backend-test-
|
|||||||
import { GlobalConfig } from '@n8n/config';
|
import { GlobalConfig } from '@n8n/config';
|
||||||
import type { User } from '@n8n/db';
|
import type { User } from '@n8n/db';
|
||||||
import { Container } from '@n8n/di';
|
import { Container } from '@n8n/di';
|
||||||
|
import type express from 'express';
|
||||||
|
|
||||||
|
import { BadRequestError } from '@/errors/response-errors/bad-request.error';
|
||||||
import { setSamlLoginEnabled } from '@/sso.ee/saml/saml-helpers';
|
import { setSamlLoginEnabled } from '@/sso.ee/saml/saml-helpers';
|
||||||
|
import { SamlService } from '@/sso.ee/saml/saml.service.ee';
|
||||||
import {
|
import {
|
||||||
getCurrentAuthenticationMethod,
|
getCurrentAuthenticationMethod,
|
||||||
setCurrentAuthenticationMethod,
|
setCurrentAuthenticationMethod,
|
||||||
@@ -283,3 +286,86 @@ describe('Check endpoint permissions', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
describe('SAML email validation', () => {
|
||||||
|
let samlService: SamlService;
|
||||||
|
|
||||||
|
beforeAll(async () => {
|
||||||
|
samlService = Container.get(SamlService);
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('handleSamlLogin', () => {
|
||||||
|
test('should throw BadRequestError for invalid email format', async () => {
|
||||||
|
// Mock getAttributesFromLoginResponse to return invalid email
|
||||||
|
jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({
|
||||||
|
email: 'invalid-email-format',
|
||||||
|
firstName: 'John',
|
||||||
|
lastName: 'Doe',
|
||||||
|
userPrincipalName: 'john.doe',
|
||||||
|
});
|
||||||
|
|
||||||
|
const mockRequest = {} as express.Request;
|
||||||
|
|
||||||
|
await expect(samlService.handleSamlLogin(mockRequest, 'post')).rejects.toThrow(
|
||||||
|
new BadRequestError('Invalid email format'),
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
test.each([['not-an-email'], ['@missinglocal.com'], ['missing@.com'], ['spaces in@email.com']])(
|
||||||
|
'should throw BadRequestError for invalid email <%s>',
|
||||||
|
async (invalidEmail) => {
|
||||||
|
jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({
|
||||||
|
email: invalidEmail,
|
||||||
|
firstName: 'John',
|
||||||
|
lastName: 'Doe',
|
||||||
|
userPrincipalName: 'john.doe',
|
||||||
|
});
|
||||||
|
|
||||||
|
const mockRequest = {} as express.Request;
|
||||||
|
|
||||||
|
await expect(samlService.handleSamlLogin(mockRequest, 'post')).rejects.toThrow(
|
||||||
|
new BadRequestError('Invalid email format'),
|
||||||
|
);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
test.each([
|
||||||
|
['user@example.com'],
|
||||||
|
['test.email@domain.org'],
|
||||||
|
['user+tag@example.com'],
|
||||||
|
['user123@test-domain.com'],
|
||||||
|
])('should handle valid email <%s> successfully', async (validEmail) => {
|
||||||
|
const mockRequest = {} as express.Request;
|
||||||
|
|
||||||
|
jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({
|
||||||
|
email: validEmail,
|
||||||
|
firstName: 'John',
|
||||||
|
lastName: 'Doe',
|
||||||
|
userPrincipalName: 'john.doe',
|
||||||
|
});
|
||||||
|
|
||||||
|
// Should not throw an error for valid emails
|
||||||
|
const result = await samlService.handleSamlLogin(mockRequest, 'post');
|
||||||
|
expect(result).toBeDefined();
|
||||||
|
expect(result.attributes.email).toBe(validEmail);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('should convert email to lowercase before validation', async () => {
|
||||||
|
const upperCaseEmail = 'USER@EXAMPLE.COM';
|
||||||
|
|
||||||
|
jest.spyOn(samlService, 'getAttributesFromLoginResponse').mockResolvedValue({
|
||||||
|
email: upperCaseEmail,
|
||||||
|
firstName: 'John',
|
||||||
|
lastName: 'Doe',
|
||||||
|
userPrincipalName: 'john.doe',
|
||||||
|
});
|
||||||
|
|
||||||
|
const mockRequest = {} as express.Request;
|
||||||
|
|
||||||
|
// Should not throw an error as the email is valid when converted to lowercase
|
||||||
|
const result = await samlService.handleSamlLogin(mockRequest, 'post');
|
||||||
|
expect(result).toBeDefined();
|
||||||
|
expect(result.attributes.email).toBe(upperCaseEmail); // Original email should be preserved in attributes
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
15
pnpm-lock.yaml
generated
15
pnpm-lock.yaml
generated
@@ -698,6 +698,9 @@ importers:
|
|||||||
xss:
|
xss:
|
||||||
specifier: 'catalog:'
|
specifier: 'catalog:'
|
||||||
version: 1.0.15
|
version: 1.0.15
|
||||||
|
zod:
|
||||||
|
specifier: 'catalog:'
|
||||||
|
version: 3.25.67
|
||||||
devDependencies:
|
devDependencies:
|
||||||
'@n8n/typescript-config':
|
'@n8n/typescript-config':
|
||||||
specifier: workspace:*
|
specifier: workspace:*
|
||||||
@@ -25552,7 +25555,7 @@ snapshots:
|
|||||||
|
|
||||||
eslint-import-resolver-node@0.3.9:
|
eslint-import-resolver-node@0.3.9:
|
||||||
dependencies:
|
dependencies:
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
is-core-module: 2.16.1
|
is-core-module: 2.16.1
|
||||||
resolve: 1.22.10
|
resolve: 1.22.10
|
||||||
transitivePeerDependencies:
|
transitivePeerDependencies:
|
||||||
@@ -25576,7 +25579,7 @@ snapshots:
|
|||||||
|
|
||||||
eslint-module-utils@2.12.1(@typescript-eslint/parser@8.35.0(eslint@9.29.0(jiti@1.21.7))(typescript@5.9.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@4.4.3)(eslint@9.29.0(jiti@1.21.7)):
|
eslint-module-utils@2.12.1(@typescript-eslint/parser@8.35.0(eslint@9.29.0(jiti@1.21.7))(typescript@5.9.2))(eslint-import-resolver-node@0.3.9)(eslint-import-resolver-typescript@4.4.3)(eslint@9.29.0(jiti@1.21.7)):
|
||||||
dependencies:
|
dependencies:
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
optionalDependencies:
|
optionalDependencies:
|
||||||
'@typescript-eslint/parser': 8.35.0(eslint@9.29.0(jiti@1.21.7))(typescript@5.9.2)
|
'@typescript-eslint/parser': 8.35.0(eslint@9.29.0(jiti@1.21.7))(typescript@5.9.2)
|
||||||
eslint: 9.29.0(jiti@1.21.7)
|
eslint: 9.29.0(jiti@1.21.7)
|
||||||
@@ -25615,7 +25618,7 @@ snapshots:
|
|||||||
array.prototype.findlastindex: 1.2.6
|
array.prototype.findlastindex: 1.2.6
|
||||||
array.prototype.flat: 1.3.3
|
array.prototype.flat: 1.3.3
|
||||||
array.prototype.flatmap: 1.3.3
|
array.prototype.flatmap: 1.3.3
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
doctrine: 2.1.0
|
doctrine: 2.1.0
|
||||||
eslint: 9.29.0(jiti@1.21.7)
|
eslint: 9.29.0(jiti@1.21.7)
|
||||||
eslint-import-resolver-node: 0.3.9
|
eslint-import-resolver-node: 0.3.9
|
||||||
@@ -26574,7 +26577,7 @@ snapshots:
|
|||||||
array-parallel: 0.1.3
|
array-parallel: 0.1.3
|
||||||
array-series: 0.1.5
|
array-series: 0.1.5
|
||||||
cross-spawn: 7.0.6
|
cross-spawn: 7.0.6
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
transitivePeerDependencies:
|
transitivePeerDependencies:
|
||||||
- supports-color
|
- supports-color
|
||||||
|
|
||||||
@@ -29979,7 +29982,7 @@ snapshots:
|
|||||||
|
|
||||||
pdf-parse@1.1.1:
|
pdf-parse@1.1.1:
|
||||||
dependencies:
|
dependencies:
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
node-ensure: 0.0.0
|
node-ensure: 0.0.0
|
||||||
transitivePeerDependencies:
|
transitivePeerDependencies:
|
||||||
- supports-color
|
- supports-color
|
||||||
@@ -30967,7 +30970,7 @@ snapshots:
|
|||||||
|
|
||||||
rhea@1.0.24:
|
rhea@1.0.24:
|
||||||
dependencies:
|
dependencies:
|
||||||
debug: 3.2.7(supports-color@8.1.1)
|
debug: 3.2.7(supports-color@5.5.0)
|
||||||
transitivePeerDependencies:
|
transitivePeerDependencies:
|
||||||
- supports-color
|
- supports-color
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user