[$500] BA - No validation error when emoji and certain symbols are entered in name field

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number: 1.4.6.2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: Applause - Internal Team
Slack conversation:

Issue found when executing PR #30674

Action Performed:

1. Navigate to staging.new.expensify.com
2. Go to Settings > Workspaces > any workspace
3. Go to Bank account > Connect manually
4. Proceed to Step 3
5. Add @, +, (), [] and emoji in the name field
6. Submit the form

Expected Result:

Error will show up on the name field with invalid symbols and emoji

Actual Result:

Error does not show up on the name field with invalid symbols and emoji. Only certain symbols pass the validation

Workaround:

Unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

• Android: Native
• Android: mWeb Chrome
• iOS: Native
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

Bug6296280_1701365098247.20231130_214539.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01fdad690a07b3c07f
• Upwork Job ID: 1740328309374783488
• Last Price Increase: 2024-01-10
• Automatic offers:
• 0xmiroslav | Reviewer | 28109979
• Krishna2323 | Contributor | 28109980

[melvin-bot]

Triggered auto assignment to @trjExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

[melvin-bot]

Bug0 Triage Checklist (Main S/O)

• This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
• This bug is not a duplicate report (check E/App issues and #expensify-bugs)
  • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
• This bug is reproducible using the reproduction steps in the OP. S/O
  • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
  • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
• This issue is filled out as thoroughly and clearly as possible
  • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
• I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

[shubham1206agra]

Regression from linked PR

[0xmiros]

Not regression. This validation was never happened before #30674.
Btw, happy to take this as C+

[Krishna2323]

Proposal

Please re-state the problem that we are trying to solve in this issue.

BA - No validation error when emoji and certain symbols are entered in name field.

What is the root cause of that problem?

We only check validation to determine if the name is empty, there is no validation in place to ensure that the name contains only Latin characters.

What changes do you think we should make in order to solve the problem?

We should add an 'if' statement to check whether the name contains only valid firstName & lastName or not. We can use the new function isValidPersonName inside ValidationUtils.

App/src/libs/ValidationUtils.ts

Lines 303 to 305 in 1682be4

	function isValidPersonName(value: string) {
	return /^[^\d^!#$%*=<>;{}"]+$/.test(value);
	}

Updated code should look like:

    // Inside `validate` function in `RequestorStep.js`
    if (!ValidationUtils.isValidPersonName(values.firstName)) {
        errors.firstName = 'bankAccount.error.firstName';
    }

    if (!ValidationUtils.isValidPersonName(values.lastName)) {
        errors.lastName = 'bankAccount.error.lastName';
    }

Result

What alternative solutions did you explore? (Optional)

Or we can use isValidLegalName instead of isValidPersonName which only permits Latin characters.

App/src/libs/ValidationUtils.ts

Lines 326 to 329 in 50865ef

	function isValidLegalName(name: string): boolean {
	const hasAccentedChars = Boolean(name.match(CONST.REGEX.ACCENT_LATIN_CHARS));
	return CONST.REGEX.ALPHABETIC_AND_LATIN_CHARS.test(name) && !hasAccentedChars;
	}

[trjExpensify]

We only check validation to determine if the name is empty, there is no validation in place to ensure that the name contains only Latin characters.

So looking at the linked PR we explicitly decided not to exclude special characters in the name field as per here. It's unclear if that matched Onfido's requirements or not? CC: @stitesExpensify as you reviewed that PR as well.

@0xmiroslav assigning you for the time being.

[stitesExpensify]

We don't check for onfido's requirements on the front end or back end currently. I posted the backend logic on the other issue and we just strip out a specific set of characters. This seems like a nice to have, but honestly if you're putting random random characters and emojis in your name and it fails, I'm not too concerned by that.

[stitesExpensify]

If we want to add @ or + we can, but I'm not aware of any full list of forbidden characters that we have anywhere

[trjExpensify]

if you're putting random random characters and emojis in your name and it fails, I'm not too concerned by that.

Right, but do you agree there should be an error message then? Otherwise there's no feedback at all to indicate why the form won't submit, or am I not following something?

[stitesExpensify]

Oh, you can't submit? My understanding was that you could submit but would get an error message

[trjExpensify]

Nah, look at the screenie below of the video:

Imagine you filled out everything apart from the first name field correctly. That would be weird, right?

[stitesExpensify]

Right, so that screenshot shows that you can submit with the first name wrong, and then you would get an error afterwards which isn't really a big deal if you're putting things like @ and emojis in the first name.

Regardless, it should probably be fixed to match the rest of them.

[stitesExpensify]

@0xmiroslav feel free to check out proposals here

[trjExpensify]

Right, so that screenshot shows that you can submit with the first name wrong

Right, our forms design never prevents you from clicking the submit button on a form.

and then you would get an error afterwards which isn't really a big deal if you're putting things like @ and emojis in the first name.

There's no error being shown on the first name field, that's the problem.

[shubham1206agra]

There's no error being shown on the first name field, that's the problem.

Does BE show any errors if we send emojis in the name?

[dukenv0307]

Proposal

Please re-state the problem that we are trying to solve in this issue.

Error does not show up on the name field with invalid symbols and emoji. Only certain symbols pass the validation

What is the root cause of that problem?

In here, we're only restrict some symbols so other symbols and emojis are still allowed in the first name and last name field.

What changes do you think we should make in order to solve the problem?

Extend the regex here to exclude other symbols and also exclude emojis. The emoji regex can be taken from CONST.REGEX.EMOJIS

What alternative solutions did you explore? (Optional)

We can use isValidLegalName for consistency, which only permits Latin characters (other character types aside from this like Greek or Cyrillic alphabets can be added to the Regex for it if necessary). I think it makes sense to make it consistent because legal names also have first and last name field, it's a bit weird when a name can be considered valid when use in Legal name but invalid if used in bank account flow.

Or legal name check can also follow the same as bank account first and last name check.

Regarding the Greek and Cyrillic alphabets here #32291 (comment), I think both legal name and bank account name should follow the same rule and can use the same regex. But if only bank account name needs to support those alphabets but legal name doesn't, then we'll make another regex for the bank account name.

[trjExpensify]

@0xmiroslav over to you for the proposal review, please!

[0xmiros]

There're various solutions to invalidate emoji but I'm inclined to the solution which only validates language alphabets.
We intentionally enabled numbers in #30674 but I am still not sure person name can include numbers.

[stitesExpensify]

On hold, moving this to weekly while we wait.

[trjExpensify]

Ah sorry, forgot to drop it to weekly. Thanks!

Still held on #34498

[trjExpensify]

Okay so #34498 has merged. Taking this off hold. We should be good to proceed here now @Krishna2323.

[Krishna2323]

Raising PR today.

[Krishna2323]

@Ollyws PR ready for review :)

[trjExpensify]

PR hit staging 15 hours ago.

[Krishna2323]

@NikkiWines friendly bump for payments here :), PR was deployed to production on 29th Feb.

[NikkiWines]

@trjExpensify can you issue payment, please? Thank you 🙇

[trjExpensify]

Payment summary:

@Krishna2323 paid $500 for the fix
@Ollyws offer sent for $500 for the review

[Ollyws]

Accepted, thanks.

[trjExpensify]

Paid, closing. Thanks, everyone!