mirror of
https://github.com/n8n-io/n8n.git
synced 2025-02-21 02:56:40 -08:00
fix(core): All calls to plainToInstance
should exclude extraneous values (no-changelog) (#9338)
This commit is contained in:
parent
9003c15811
commit
5025d209ca
|
@ -41,7 +41,7 @@ export class MeController {
|
|||
@Patch('/')
|
||||
async updateCurrentUser(req: MeRequest.UserUpdate, res: Response): Promise<PublicUser> {
|
||||
const { id: userId, email: currentEmail } = req.user;
|
||||
const payload = plainToInstance(UserUpdatePayload, req.body);
|
||||
const payload = plainToInstance(UserUpdatePayload, req.body, { excludeExtraneousValues: true });
|
||||
|
||||
const { email } = payload;
|
||||
if (!email) {
|
||||
|
@ -227,7 +227,9 @@ export class MeController {
|
|||
*/
|
||||
@Patch('/settings')
|
||||
async updateCurrentUserSettings(req: MeRequest.UserSettingsUpdate): Promise<User['settings']> {
|
||||
const payload = plainToInstance(UserSettingsUpdatePayload, req.body);
|
||||
const payload = plainToInstance(UserSettingsUpdatePayload, req.body, {
|
||||
excludeExtraneousValues: true,
|
||||
});
|
||||
const { id } = req.user;
|
||||
|
||||
await this.userService.updateSettings(id, payload);
|
||||
|
|
|
@ -117,7 +117,9 @@ export class UsersController {
|
|||
@Patch('/:id/settings')
|
||||
@GlobalScope('user:update')
|
||||
async updateUserSettings(req: UserRequest.UserSettingsUpdate) {
|
||||
const payload = plainToInstance(UserSettingsUpdatePayload, req.body);
|
||||
const payload = plainToInstance(UserSettingsUpdatePayload, req.body, {
|
||||
excludeExtraneousValues: true,
|
||||
});
|
||||
|
||||
const id = req.params.id;
|
||||
|
||||
|
@ -293,7 +295,9 @@ export class UsersController {
|
|||
const { NO_ADMIN_ON_OWNER, NO_USER, NO_OWNER_ON_OWNER } =
|
||||
UsersController.ERROR_MESSAGES.CHANGE_ROLE;
|
||||
|
||||
const payload = plainToInstance(UserRoleChangePayload, req.body);
|
||||
const payload = plainToInstance(UserRoleChangePayload, req.body, {
|
||||
excludeExtraneousValues: true,
|
||||
});
|
||||
await validateEntity(payload);
|
||||
|
||||
const targetUser = await this.userRepository.findOne({
|
||||
|
|
|
@ -115,7 +115,7 @@ export class User extends WithTimestamps implements IUser {
|
|||
@AfterLoad()
|
||||
@AfterUpdate()
|
||||
computeIsPending(): void {
|
||||
this.isPending = this.password === null;
|
||||
this.isPending = this.password === null && this.role !== 'global:owner';
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
|
@ -10,6 +10,7 @@ import type {
|
|||
IUser,
|
||||
} from 'n8n-workflow';
|
||||
|
||||
import { Expose } from 'class-transformer';
|
||||
import { IsBoolean, IsEmail, IsIn, IsOptional, IsString, Length } from 'class-validator';
|
||||
import { NoXss } from '@db/utils/customValidators';
|
||||
import type { PublicUser, SecretsProvider, SecretsProviderState } from '@/Interfaces';
|
||||
|
@ -20,14 +21,17 @@ import type { CredentialsEntity } from '@db/entities/CredentialsEntity';
|
|||
import type { WorkflowHistory } from '@db/entities/WorkflowHistory';
|
||||
|
||||
export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'lastName'> {
|
||||
@Expose()
|
||||
@IsEmail()
|
||||
email: string;
|
||||
|
||||
@Expose()
|
||||
@NoXss()
|
||||
@IsString({ message: 'First name must be of type string.' })
|
||||
@Length(1, 32, { message: 'First name must be $constraint1 to $constraint2 characters long.' })
|
||||
firstName: string;
|
||||
|
||||
@Expose()
|
||||
@NoXss()
|
||||
@IsString({ message: 'Last name must be of type string.' })
|
||||
@Length(1, 32, { message: 'Last name must be $constraint1 to $constraint2 characters long.' })
|
||||
|
@ -35,16 +39,19 @@ export class UserUpdatePayload implements Pick<User, 'email' | 'firstName' | 'la
|
|||
}
|
||||
|
||||
export class UserSettingsUpdatePayload {
|
||||
@Expose()
|
||||
@IsBoolean({ message: 'userActivated should be a boolean' })
|
||||
@IsOptional()
|
||||
userActivated: boolean;
|
||||
|
||||
@Expose()
|
||||
@IsBoolean({ message: 'allowSSOManualLogin should be a boolean' })
|
||||
@IsOptional()
|
||||
allowSSOManualLogin?: boolean;
|
||||
}
|
||||
|
||||
export class UserRoleChangePayload {
|
||||
@Expose()
|
||||
@IsIn(['global:admin', 'global:member'])
|
||||
newRoleName: AssignableRole;
|
||||
}
|
||||
|
|
|
@ -356,13 +356,11 @@ const VALID_PATCH_ME_PAYLOADS = [
|
|||
email: randomEmail(),
|
||||
firstName: randomName(),
|
||||
lastName: randomName(),
|
||||
password: randomValidPassword(),
|
||||
},
|
||||
{
|
||||
email: randomEmail().toUpperCase(),
|
||||
firstName: randomName(),
|
||||
lastName: randomName(),
|
||||
password: randomValidPassword(),
|
||||
},
|
||||
];
|
||||
|
||||
|
|
|
@ -87,27 +87,28 @@ describe('MeController', () => {
|
|||
id: '123',
|
||||
password: 'password',
|
||||
authIdentities: [],
|
||||
role: 'global:owner',
|
||||
role: 'global:member',
|
||||
});
|
||||
const reqBody = { email: 'valid@email.com', firstName: 'John', lastName: 'Potato' };
|
||||
const req = mock<MeRequest.UserUpdate>({ user, body: reqBody, browserId });
|
||||
const req = mock<MeRequest.UserUpdate>({ user, browserId });
|
||||
req.body = reqBody;
|
||||
const res = mock<Response>();
|
||||
userRepository.findOneOrFail.mockResolvedValue(user);
|
||||
jest.spyOn(jwt, 'sign').mockImplementation(() => 'signed-token');
|
||||
|
||||
// Add invalid data to the request payload
|
||||
Object.assign(reqBody, { id: '0', role: '42' });
|
||||
Object.assign(reqBody, { id: '0', role: 'global:owner' });
|
||||
|
||||
await controller.updateCurrentUser(req, res);
|
||||
|
||||
expect(userService.update).toHaveBeenCalled();
|
||||
|
||||
const updatedUser = userService.update.mock.calls[0][1];
|
||||
expect(updatedUser.email).toBe(reqBody.email);
|
||||
expect(updatedUser.firstName).toBe(reqBody.firstName);
|
||||
expect(updatedUser.lastName).toBe(reqBody.lastName);
|
||||
expect(updatedUser.id).not.toBe('0');
|
||||
expect(updatedUser.role).not.toBe('42');
|
||||
const updatePayload = userService.update.mock.calls[0][1];
|
||||
expect(updatePayload.email).toBe(reqBody.email);
|
||||
expect(updatePayload.firstName).toBe(reqBody.firstName);
|
||||
expect(updatePayload.lastName).toBe(reqBody.lastName);
|
||||
expect(updatePayload.id).toBeUndefined();
|
||||
expect(updatePayload.role).toBeUndefined();
|
||||
});
|
||||
|
||||
it('should throw BadRequestError if beforeUpdate hook throws BadRequestError', async () => {
|
||||
|
|
Loading…
Reference in a new issue