[HOLD for payment 2024-02-07] [$500] Display Name - User can save Expensify and Concierge as a last name while error is thrown

[lanitochka17]

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!

---

Version Number: 1.4.24-0
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:

Action Performed:

1. Navigate to Settings > Profile > Display Name
2. Enter either 'Expensify' or 'Concierge' as the last name
3. Click on 'Save' button

Expected Result:

User shouldn't be able to save the 'Expensify' or 'Concierge' as last name

Actual Result:

User is able to save the 'Expensify' or 'Concierge' as last name

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

Bug6338345_1704929835247.Screen_Recording_2024-01-10_at_10.34.33_at_night.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01afa29712aeb0580b
• Upwork Job ID: 1745234409296494592
• Last Price Increase: 2024-01-18
• Automatic offers:
• fedirjh | Reviewer | 28119205
• dukenv0307 | Contributor | 28119206

[melvin-bot]

Job added to Upwork: https://www.upwork.com/jobs/~01afa29712aeb0580b

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @fedirjh (External)

[neonbhai]

Proposal

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

Display Name - User can save Expensify and Concierge as a last name while error is thrown

What is the root cause of that problem?

This happens as we are not checking the lastName for reserved words on this page:

App/src/pages/settings/Profile/DisplayNamePage.js

Lines 56 to 72 in 8a559fd

	const validate = (values) => {
	const errors = {};
	// First we validate the first name field
	if (!ValidationUtils.isValidDisplayName(values.firstName)) {
	ErrorUtils.addErrorMessage(errors, 'firstName', 'personalDetails.error.hasInvalidCharacter');
	}
	if (ValidationUtils.doesContainReservedWord(values.firstName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) {
	ErrorUtils.addErrorMessage(errors, 'firstName', 'personalDetails.error.containsReservedWord');
	}
	// Then we validate the last name field
	if (!ValidationUtils.isValidDisplayName(values.lastName)) {
	errors.lastName = 'personalDetails.error.hasInvalidCharacter';
	}
	return errors;
	};

What changes do you think we should make in order

We should add error checking like we did for first Name

if (ValidationUtils.doesContainReservedWord(values.lastName, CONST.DISPLAY_NAME.RESERVED_LAST_NAMES)) { 
   ErrorUtils.addErrorMessage(errors, 'firstName', 'personalDetails.error.containsReservedWord'); 
}

Alternatively

We may add failure data for this request here:

App/src/libs/actions/PersonalDetails.ts

Line 134 in c8631be

function updateDisplayName(firstName: string, lastName: string) {

[abzokhattab]

Proposal

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

Display Name - User can save Expensify and Concierge as a last name while error is thrown

What is the root cause of that problem?

We dont check if the last name is one of the reserved words similar to first name

App/src/pages/settings/Profile/DisplayNamePage.js

Lines 63 to 65 in 9208c95

	if (ValidationUtils.doesContainReservedWord(values.firstName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) {
	ErrorUtils.addErrorMessage(errors, 'firstName', 'personalDetails.error.containsReservedWord');
	}

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

we need to add a validation here for the last name as well

 if (ValidationUtils.doesContainReservedWord(values. lastName, CONST.DISPLAY_NAME.RESERVED_FIRST_NAMES)) { 
         ErrorUtils.addErrorMessage(errors, 'lastName', 'personalDetails.error.containsReservedWord'); 
     } 

[dukenv0307]

Proposal

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

User is able to save the 'Expensify' or 'Concierge' as last name

What is the root cause of that problem?

1. In here, we're not validating that the lastName doesn't contain the reserved words similar to what we did with the first name
2. When we save the legal name (also has first name and last name), if the display names are not set, it will also set the displayName to same as legal name. So the user can save reserved words like Expensify and Concierge as legal name and it will be shown as the display name, and then if the user goes to the display name page and click Save it will show error. We don't currently have any reserved words validation for legal name.

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

1. Validate reserved words for lastName here similar to for first name. We might want to change RESERVED_FIRST_NAMES to RESERVED_NAMES because it doesn't just apply to first name
2. Add reserved words validation for both first name and last name, to the legal name page. We can optionally fix the back-end to reject legal names that contain reserved words as well

What alternative solutions did you explore? (Optional)

We should additionally evaluate other names like workspace names, room names, personal bank information first & last names, ... to see if we should apply the same reserved words restriction, I think it makes sense to restrict in those places as well to reduce frauds.

Another approach for 2, is to remove the linkage between Legal name and Display name, so if Display name is not already set, when we set Legal name, it should not set Display name to the same values.

[mkhutornyi]

I think it's intentional to validate only first name but not last name. And it's what backend already does.
Originally implemented in #14873 to fix #13779
cc: @puneetlath

[fedirjh]

I think it's intentional to validate only first name but not last name. And it's what backend already does.

@mkhutornyi It seems that the backend also validates the last name and that’s why it throws this error :

[fedirjh]

@puneetlath Should we add the name validation for the legal name page ? As currently user can save legal name as Expensify and Concierge

[puneetlath]

Does the API call fail? If not, I don't think it's necessary.

[neonbhai]

@puneetlath hi, the name seems to reset on logout and login.

Also not validating last name would allow the user to impersonate Expensify or Concierge:

Video

ce48befa-09b9-46bb-b008-a731c677d4b7.mp4

And the backend does return an error. Shown next time we visit the page:

Video

Screen.Recording.2024-01-12.at.4.25.53.AM.mov

The name will be reset on next login. We can save this optimistically as there is no front end validation.

I think we should we fix this

[dukenv0307]

When we save the legal name (also has first name and last name), if the display names are not set, it will also set the displayName to same as legal name. So the user can save reserved words like Expensify and Concierge as legal name and it will be shown as the display name, and then if the user goes to the display name page and click Save it will show error. We don't currently have any reserved words validation for legal name.

@puneetlath if we allow legal name to contains, Expensify and Concierge, that means the user will be able to set the display name to those, because when updating legal name, it will also update display name to the same values if display name doesn't exist yet.

So the user can use that as a workaround to impersonate Expensify and Concierge

[fedirjh]

Does the API call fail? If not, I don't think it's necessary.

@puneetlath The api call returns an error (which means req failure), I think we have missing failureData that should resets the display name when the req fails.

[puneetlath]

Ok got it! In that case, yes, I think we should have front-end validation for that.

[fedirjh]

@dukenv0307 It seems that the backend does not have any validation rules for the legal name. I think we should stick with that and just implement the necessary validation solely for the display name.

[dukenv0307]

When we save the legal name (also has first name and last name), if the display names are not set, it will also set the displayName to same as legal name. So the user can save reserved words like Expensify and Concierge as legal name and it will be shown as the display name, and then if the user goes to the display name page and click Save it will show error. We don't currently have any reserved words validation for legal name.

@fedirjh What do you think? If we don't fix in legal name, I'll still be able to set the forbidden words as the display name.

[fedirjh]

@dukenv0307 When display name is not set, it will fallback to the email address instead of the legal name. I guess the legal name is only used in the add bank account flow.

[dukenv0307]

@fedirjh Please see the below video:

1. Initially there's no display name
2. I go to legal name page and set the First name to Concierge and Last name to Expensify and save
3. Reload the page
4. Go to display name, now the display name first name is Concierge, last name is Expensify, which violates our rules, and when clicking save, it shows error.

So the legal name will become display name if the display name is not already set

Screen.Recording.2024-01-15.at.7.00.04.PM.mp4

[fedirjh]

@dukenv0307 Interesting find, I could reproduce with a new account as well. It seems that we should add both front End and backend validation to the legal name.

cc @puneetlath your thoughts on this bug? It seems that it will require a backend fix.

[puneetlath]

Hm, I'm not eager to do anything then if it requires a back-end update. We can solve it if/when it becomes a real problem.

[dukenv0307]

Hm, I'm not eager to do anything then if it requires a back-end update. We can solve it if/when it becomes a real problem.

@puneetlath this is a security issue if we don't fix it, the user can impersonate Expensify/Concierge which is what we tried to prevent in the first place.

I'd say at least we should fix in the front-end to not allow that to happen, so regular users cannot do such things. The back-end doesn't need to be updated for now.

[zanyrenney]

bump @puneetlath for your thoughts please?

[zanyrenney]

have sent @puneetlath a DM asking him to take a look.

[zanyrenney]

After discussing with @puneetlath in DM, he agreed that we should fix this.

I think we should just focus on the front-end fix for now though @dukenv0307 - please keep pressing ahead if @fedirjh agrees your proposal is preferable.

[dukenv0307]

Thanks @zanyrenney !

@fedirjh do you think we should move forward with my proposal?

[fedirjh]

@zanyrenney Thanks for the feedback.

In this case, it makes sense to proceed with @dukenv0307's proposal. Let’s add the required front-end validation to both last name and legal name fields.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

Triggered auto assignment to @Julesssss, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[Julesssss]

Sounds good

[melvin-bot]

📣 @fedirjh 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

[melvin-bot]

📣 @dukenv0307 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.34-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: add validate to last name and legal name #35227

If no regressions arise, payment will be issued on 2024-02-07. 🎊

For reference, here are some details about the assignees on this issue:

• @fedirjh requires payment automatic offer (Reviewer)
• @dukenv0307 requires payment automatic offer (Contributor)

[melvin-bot]

BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:

• [@fedirjh] The PR that introduced the bug has been identified. Link to the PR:
• [@fedirjh] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
• [@fedirjh] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
• [@fedirjh] Determine if we should create a regression test for this bug.
• [@fedirjh] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
• [@zanyrenney] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

[zanyrenney]

Payment summary

@fedirjh requires payment automatic offer (Reviewer) - PAID $500
@dukenv0307 requires payment automatic offer (Contributor) PAID $500