[NV-2074] 🐛 Bug Report: Validation problems

[michaldziuba03]

📜 Description

Unbound length

User can create entities with veery long unpractical names/identifiers. I think we shouldn't allow for names with 88886 length.

Lack of trim

Basically in every string input (like Organization name) I can "bypass" any kind of length validation (even if we fix unbound length issue) by using bunch of whitespaces (whitespace counts as character).

👟 Reproduction steps

Unbound length

1. Generate very long string
2. Paste this string to the input
3. You created entity with veeeery long name

Lack of trim

1. Type something like " " in any string input.
2. You created entity with empty name :)

I consider this as visual bug, because I tried to break application/flow and everything works fine (but just looks weird).

👍 Expected behavior

Unbound length

I suggest to apply decorators like @Length(MIN, MAX) for each DTO. Every input should have defined practical boundaries.

Fix can be tricky because it's hard to determine practical length and can break application for existing users (who have for some reason long names/identifiers).

Lack of trim

I think inputs for things like names and identifiers should be transformed by using .trim() String method.

import { Transform } from 'class-transformer';

export const Trim = () => Transform(({ value }) => value?.trim());

We can apply this for each DTO.

Obviously not every string input should be trimmed - I would leave password as it is and fields where whitespace matters (html content, descriptions).

The fix should be implemented carefully, because it can break the application for users who previously created names with leading whitespaces. I will "think" about them in my PR.

👎 Actual Behavior with Screenshots

Unbound length

I generated 88886 bytes name.

Lack of Trim

📃 Provide any additional context for the Bug.

No response

👀 Have you spent some time to check if this bug has been raised before?

• [X] I checked and didn't find similar issue

🏢 Have you read the Contributing Guidelines?

• [X] I have read the Contributing Guidelines

Are you willing to submit PR?

Yes I am willing to submit a PR!

_NV-2074

[scopsy]

@michaldziuba03 thank you for the submission, got the error on sentry and was curious about what you found out :)

I think we can maybe start with some obvious things like org name, user first name and last name, and general things we display in the UI. As you mentioned it would be tricky to control everywhere for all DTO's in my opinion. wdyt?

[michaldziuba03]

@scopsy I agree we can start with these obvious things like:

• organization name
• first and last names
• notification name, identifier

I personally manage things like lengths in file <module/entity/whatever-you-create>.constants.ts. It's easy to control lengths from single place, right? We can even mantain constants in shared module to easily provide consistency when we decide to implement validation on frontend as well (I always implement on both sides).

user.constants.ts

export const PASSWORD_MIN = 8;
export const PASSWORD_MAX = 128;

export const NAME_MIN = 2; export const NAME_MAX = 32;

> login.dto.ts
```ts
import { IsEmail, IsString, Length } from 'class-validator';
import { PASSWORD_MAX, PASSWORD_MIN } from '../../user/user.constants';

export class LoginDTO {
  @IsEmail()
  email: string;

  @IsString()
  @Length(PASSWORD_MIN, PASSWORD_MAX)
  password: string;
}

reset-password.dto.ts

import { IsString, Length } from 'class-validator';
import { PASSWORD_MAX, PASSWORD_MIN } from '../../user.constants';

export class ResetPasswordDTO { @IsString() token: string;

@IsString() @Length(PASSWORD_MIN, PASSWORD_MAX) password: string; }

[scopsy]

Yep! Love the idea of having constant for those

[gitstart]

Are you still working on this? @michaldziuba03 We would like to pick it up

[gitstart]

Are you still working on this? @michaldziuba03 We would like to pick it up

Cc @scopsy

[scopsy]

@gitstart we have quite a few open PR's already on your end, I would love to merge all the open one's first 🙏