[$250] Upgrade - Invited user can access upgrade page via /upgrade/report-fields path

[IuliiaHerets]

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: v9.0.24-0 Reproducible in staging?: Y Reproducible in production?: Y Email or phone of affected tester (no customers): [email protected] Issue reported by: Applause Internal Team

Action Performed:

Precondition:

• User is invited to workspace.
• Steps below will be performed by the invited user.

1. Go to staging.new.expensify.com
2. Go to the invited workspace settings.
3. Add /upgrade/report-fields to the URL (eg, staging.new.expensify.com/settings/workspaces/ID/upgrade/report-fields).

Expected Result:

Not here page will open because the invited user should not be able to access or upgrade the workspace.

Actual Result:

Upgrade page opens although the workspace cannot be upgraded by the invited user.

Workaround:

Unknown

Platforms:

• [x] Android: Native
• [x] Android: mWeb Chrome
• [x] iOS: Native
• [x] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [x] MacOS: Desktop

Screenshots/Videos

https://github.com/user-attachments/assets/99e809e6-8a14-4364-ab8b-4a312c66bf02

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~01819a1e2aa1e41f2a
• Upwork Job ID: 1828745121652626422
• Last Price Increase: 2024-08-28
• Automatic offers:
• paultsimura | Reviewer | 103720683

Issue Owner

Current Issue Owner: @paultsimura

[melvin-bot[bot]]

Triggered auto assignment to @dylanexpensify (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.

[IuliiaHerets]

We think that this bug might be related to #wave-control

[IuliiaHerets]

@dylanexpensify FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

[excile1]

Edited by proposal-police: This proposal was edited at 2024-08-23T11:20:13Z.

Proposal

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

The invited user, who is not an admin, is able to access the upgrade page.

What is the root cause of that problem?

The root cause of this problem is this check in WorkspaceUpgradePage.tsx: https://github.com/Expensify/App/blob/259a9b781a3904c7246d4317f5ef37ff986afacb/src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx#L69-L71 which only returns the not found page if the policy or feature doesn't exist.

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

Updating this check to be something like: if (!feature || !policy || !PolicyUtils.isPolicyAdmin(policy)) { should fix the issue, and it will only allow the workspace administrator to access the upgrade page. To make sure the upgrade doesn't fire, probably also worth updating that same check in the upgradeToCorporate function: https://github.com/Expensify/App/blob/6488908562131e5e411275059bb73b578a2cf420/src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx#L34-L40

What alternative solutions did you explore? (Optional)

An alternative solution I explored was wrapping the page with the AccessOrNotFoundWrapper component, while specifying the accessVariants parameter as CONST.POLICY.ACCESS_VARIANTS.ADMIN. This also works, but I believe my initial solution is a little bit cleaner and produces the same result.

Result: (invited user)

(workspace admin)

[mkzie2]

Proposal

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

Upgrade page opens although the workspace cannot be upgraded by the invited user.

What is the root cause of that problem?

We only display the not found page if the feature or policy is empty. For the invited user, the policy isn't empty which leads the user with role is user still can access the upgrade page via deep link.

https://github.com/Expensify/App/blob/6488908562131e5e411275059bb73b578a2cf420/src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx#L69-L72

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

We should only show this page for the admin of the policy and we also need to prevent calling the API from confirmUpgrade function here If the member user accesses the upgrade page of the control policy and closes this page.

To cleaner we can create a variable that checks whether the not found page should show or not

shouldShowNotFoundPage = !feature  || !policy || !PolicyUtils.isPolicyAdmin(policy)

And use this variable to show the not found page here and prevent calling confirmUpgrade function here

if (!isUpgraded || shouldShowNotFoundPage) {
    return;
}
confirmUpgrade();

What alternative solutions did you explore? (Optional)

For the case showing not found page, we can use AccessOrNotFoundWrapper as we did on WorkspaceMoreFeaturesPage here

[excile1]

Proposal

Updated

[melvin-bot[bot]]

@dylanexpensify Whoops! This issue is 2 days overdue. Let's get this updated quick!

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[paultsimura]

Thanks for the proposals. @mkzie2's proposal was the first to include the access check inside the functions, which is vital for this case. Therefore, we should proceed with it.

🎀👀🎀 C+ reviewed

[melvin-bot[bot]]

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

[melvin-bot[bot]]

📣 @paultsimura 🎉 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[bot]]

📣 @mkzie2 You have been assigned to this job! Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs! Keep in mind: Code of Conduct | Contributing 📖

[paultsimura]

Deployed to production: https://github.com/Expensify/App/pull/48256#issuecomment-2325378563 Payment is due on Sep 10.

[paultsimura]

• The PR that introduced the bug has been identified. Link to the PR: https://github.com/Expensify/App/pull/46617
• 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: https://github.com/Expensify/App/pull/46617/files#r1748934169
• 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: N/A
• Determine if we should create a regression test for this bug: Yes
• 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.

Regression Test Proposal

1. Login as a workspace's invited user (neither admin nor auditor)
2. Go to the Workspace Upgrade page by entering the URL: /settings/workspaces/:workspaceID/upgrade/report-fields
3. Verify the "Not found" page is shown

Do we agree 👍 or 👎

[dylanexpensify]

Payment summary:

Contributor: @mkzie2 $250 Contributor+: @paultsimura $250

Please apply/request!

[dylanexpensify]

@mkzie2 please apply here!!

[dylanexpensify]

bump @mkzie2 !

[mkzie2]

@dylanexpensify Sorry for missing this. Could you help send an offer to my account at https://www.upwork.com/freelancers/~019f73367b03c6d784 ?

[dylanexpensify]

Ah all good! Invite sent @mkzie2!!

[mkzie2]

@dylanexpensify Thank you, I accepted the invite

[melvin-bot[bot]]

This issue has not been updated in over 15 days. @tgolen, @paultsimura, @dylanexpensify, @mkzie2 eroding to Monthly issue.

P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do!

[paultsimura]

@dylanexpensify can we close this one?

[dylanexpensify]

@mkzie2 pending offer accepting

[mkzie2]

@dylanexpensify Hey thanks! I accepted