Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for payment 2024-12-05] [$250] Require magic code validation for SetPersonalDetailsAndShipExpensifyCards #52316

Closed
MarioExpensify opened this issue Nov 11, 2024 · 14 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor

Comments

@MarioExpensify
Copy link
Contributor

MarioExpensify commented Nov 11, 2024

Problem

Someone can issue multiple physical expensify cards on behalf of a domain without verifying they are actually the owner of the account. This relates to an internal security issue.

Why this is important to solve

This is a security vulnerability that can be taken advantage of if an account is compromised.

Solution

Collect a magic code when requesting a physical Expensify card. In a little more detail:

  • Update this API command here
  • It should have a new parameter called validateCode that is passed to the server
  • In order to do this, anything calling updatePersonalDetailsAndShipExpensifyCards needs a new step to gather a magic code from the user with the ValidateCodeActionModal component from this PR.
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021856002885580948055
  • Upwork Job ID: 1856002885580948055
  • Last Price Increase: 2024-11-11
  • Automatic offers:
    • nkdengineer | Contributor | 104859255
Issue OwnerCurrent Issue Owner: @JmillsExpensify
@MarioExpensify MarioExpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor labels Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

Triggered auto assignment to @JmillsExpensify (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot changed the title Require magic code validation for SetPersonalDetailsAndShipExpensifyCards [$250] Require magic code validation for SetPersonalDetailsAndShipExpensifyCards Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 11, 2024
Copy link

melvin-bot bot commented Nov 11, 2024

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

@nkdengineer
Copy link
Contributor

nkdengineer commented Nov 11, 2024

Edited by proposal-police: This proposal was edited at 2024-11-11 16:15:02 UTC.

Proposal

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

Require magic code validation for SetPersonalDetailsAndShipExpensifyCards

What is the root cause of that problem?

This is a new feature request

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

  1. We should to add ValidateCodeActionModal to MissingPersonalDetailsContent component

function MissingPersonalDetailsContent({privatePersonalDetails, draftValues}: MissingPersonalDetailsContentProps) {

  1. Update function handleFinishStep to open magic code modal then onSubmit of magic code modal call API get validateCode and send it to updatePersonalDetailsAndShipExpensifyCards API

  2. Add validateCode to this parameters

    const parameters: SetPersonalDetailsAndShipExpensifyCardsParams = {

What alternative solutions did you explore? (Optional)

@rushatgabhane
Copy link
Member

This is straight forward implementation issue. let's hire @nkdengineer

🎀 👀 🎀

Copy link

melvin-bot bot commented Nov 12, 2024

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

@MarioExpensify
Copy link
Contributor Author

Moving forward with @nkdengineer, thank you @rushatgabhane!!

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 12, 2024
Copy link

melvin-bot bot commented Nov 12, 2024

📣 @nkdengineer 🎉 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 melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 14, 2024
@melvin-bot melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Nov 28, 2024
@melvin-bot melvin-bot bot changed the title [$250] Require magic code validation for SetPersonalDetailsAndShipExpensifyCards [HOLD for payment 2024-12-05] [$250] Require magic code validation for SetPersonalDetailsAndShipExpensifyCards Nov 28, 2024
@melvin-bot melvin-bot bot removed the Reviewing Has a PR in review label Nov 28, 2024
Copy link

melvin-bot bot commented Nov 28, 2024

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

Copy link

melvin-bot bot commented Nov 28, 2024

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

If no regressions arise, payment will be issued on 2024-12-05. 🎊

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

Copy link

melvin-bot bot commented Nov 28, 2024

@rushatgabhane @JmillsExpensify @rushatgabhane The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

@rushatgabhane
Copy link
Member

Checklist

Not a bug. New feature. So we should add a regression test

Prerequisites:
Workspace is connected with a bank account.
Workspace has a member invited.
Workspace has enabled Expensify card.
Admin has issued a physical card for employee.

1. Employee clicks to the "Add shipping detail" button
2. Finish this flow
3. Verify that: At the end of this flow, require magic code modal is displayed

@melvin-bot melvin-bot bot added Daily KSv2 Overdue and removed Weekly KSv2 labels Dec 5, 2024
@JmillsExpensify
Copy link

Payment summary:

@melvin-bot melvin-bot bot removed the Overdue label Dec 9, 2024
@JmillsExpensify
Copy link

@rushatgabhane please submit via New Expensify. Contributor paid via Upwork and regression test created.

@garrettmknight
Copy link
Contributor

$250 approved for @rushatgabhane

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

6 participants