[Awaiting checklist] [$250] Workspace - Invited members can open import members page by navigating to ../members/import

[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: 9.0.51-1
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 a workspace.

1. Go to staging.new.expensify.com
2. Go to workspace settings of the invited workspace.
3. Go to Members.
4. Add /import to the end of URL and navigate to it.

Expected Result:

Not here page will show up.

Actual Result:

Import member modal shows up.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Bug6639642_1729342061093.20241019_204451.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021849229257554464797
• Upwork Job ID: 1849229257554464797
• Last Price Increase: 2024-11-06
• Automatic offers:
• BhuvaneshPatil | Contributor | 104773620

Issue Owner

Current Issue Owner: @Pujan92

[melvin-bot]

Triggered auto assignment to @twisterdotcom (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]

@twisterdotcom 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

[BhuvaneshPatil]

Edited by proposal-police: This proposal was edited at 2024-10-21 03:53:04 UTC.

Proposal

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

Workspace - Invited members can open import members page by navigating to ../members/import

What is the root cause of that problem?

We are not wrapping this component

App/src/pages/workspace/members/ImportMembersPage.tsx

Lines 10 to 19 in 66cf824

	function ImportMembersPage({route}: ImportMembersPageProps) {
	const policyID = route.params.policyID;
	return (
	<ImportSpreedsheet
	backTo={ROUTES.WORKSPACE_MEMBERS.getRoute(policyID)}
	goTo={ROUTES.WORKSPACE_MEMBERS_IMPORTED.getRoute(policyID)}
	/>
	);
	}

with AccessOrNotFoundWrapper component.
This allows users to view page without any access check if they open with URL.
This issue is with other pages as well.

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

We shall wrap the component ImportMembersPage with AccessOrNotFoundWrapper Like we have done here -

App/src/pages/workspace/WorkspaceInvitePage.tsx

Lines 310 to 314 in 66cf824

	<AccessOrNotFoundWrapper
	policyID={route.params.policyID}
	accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN]}
	fullPageNotFoundViewProps={{subtitleKey: isEmptyObject(policy) ? undefined : 'workspace.common.notAuthorized', onLinkPress: PolicyUtils.goBackFromInvalidPolicy}}
	>

This way we ensure only people who can invite can import members.

If business needs are other then we can change the accessVariants prop to manage role based permissions.

We can change the other props as required
Similar to this we should find and modify the pages where we don't want access using deep link

What alternative solutions did you explore? (Optional)

[Shahidullah-Muffakir]

Edited by proposal-police: This proposal was edited at 2024-10-19 18:22:02 UTC.

Proposal

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

invited workspace members can import members spreadsheet using url manipulation

What is the root cause of that problem?

we did not protect this page : ImportMembersPage

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

in the ImportMembersPage page
add this:

    const policy = PolicyUtils.getPolicy(policyID);
    const isPolicyAdmin = PolicyUtils.isPolicyAdmin(policy);
return (
        <FullPageNotFoundView shouldShow={!isPolicyAdmin}>
        <ImportSpreedsheet
            backTo={ROUTES.WORKSPACE_MEMBERS.getRoute(policyID)}
            goTo={ROUTES.WORKSPACE_MEMBERS_IMPORTED.getRoute(policyID)}
        />
        </FullPageNotFoundView>
    );

We can pass shouldForceFullScreen prop as true to FullPageNotFoundView, it will take the full screen. the default value for shouldForceFullScreen is false.
Note: The same issue can happen in the ImportCategoriesPage page as well, we have to protect ImportCategoriesPage, because we are only showing Import spreadsheet option in this condition:

App/src/pages/workspace/categories/WorkspaceCategoriesPage.tsx

Lines 310 to 321 in 66cf824

	if (!PolicyUtils.hasAccountingConnections(policy)) {
	menuItems.push({
	icon: Expensicons.Table,
	text: translate('spreadsheet.importSpreadsheet'),
	onSelected: () => {
	if (isOffline) {
	Modal.close(() => setIsOfflineModalVisible(true));
	return;
	}
	Navigation.navigate(ROUTES.WORKSPACE_CATEGORIES_IMPORT.getRoute(policyId));
	},
	});

, if that condition is false, then show the FullPageNotFoundView page

in this page: here
add this change:

return (
        <FullPageNotFoundView shouldShow={PolicyUtils.hasAccountingConnections(policy)}>
        <ImportSpreedsheet
            backTo={ROUTES.WORKSPACE_CATEGORIES.getRoute(policyID)}
            goTo={ROUTES.WORKSPACE_CATEGORIES_IMPORTED.getRoute(policyID)}
        />
        </FullPageNotFoundView>
    );

Screen.20Recording.202024-10-19.20at.2011.mp4

[ChavdaSachin]

Proposal

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

Invited members can open import members page using deeplink.

What is the root cause of that problem?

Invite members page is not wrapped with AccessOrNotFoundWrapper.
Additionally importedMembers page is also accessible using deeplink ( 'settings/workspaces/:policyID/members/imported')
Similar could be observed for ImportTagsPage, ImportedTagsPage, ImportCategoriesPage and ImportedCategoriesPage

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

Wrap all of the following pages with AccessOrNotFoundWrapper.
ImportMembersPage
ImportedMembersPage
ImportCategoriesPage
ImportedCategoriesPage
ImportTagsPage
ImportedTagsPage

What alternative solutions did you explore? (Optional)

Alternatively ImportSpreadsheet could be wrapped with AccessOrNotFoundWrapper along with ImportedMembersPage, ImportedCategoriesPage and ImportedTagsPage

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

@twisterdotcom Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

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

[melvin-bot]

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

[Pujan92]

@BhuvaneshPatil's proposal looks good to me which suggested wrapping ImportMembersPage and other required pages with AccessOrNotFoundWrapper.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[BhuvaneshPatil]

@deetergp 👀

[melvin-bot]

@deetergp, @twisterdotcom, @Pujan92 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[melvin-bot]

@deetergp @twisterdotcom @Pujan92 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[twisterdotcom]

@deetergp we need you to review and assign please.

[melvin-bot]

@deetergp, @twisterdotcom, @Pujan92 Still overdue 6 days?! Let's take care of this!

[melvin-bot]

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

[melvin-bot]

📣 @BhuvaneshPatil 🎉 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 📖

[deetergp]

Apologies for the late assignment, I just got back from being OOO.

[melvin-bot]

@deetergp, @twisterdotcom, @Pujan92, @BhuvaneshPatil Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@deetergp, @twisterdotcom, @Pujan92, @BhuvaneshPatil Still overdue 6 days?! Let's take care of this!

[twisterdotcom]

How we doing @BhuvaneshPatil now?

[BhuvaneshPatil]

I will be raising the PR tonight. Thank you for your patience

[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 9.0.66-8 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• add not found page for page accessed through link #52593

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

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

• @Pujan92 requires payment through NewDot Manual Requests
• @BhuvaneshPatil requires payment automatic offer (Contributor)

[melvin-bot]

@Pujan92 @twisterdotcom @Pujan92 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]

[twisterdotcom]

Payment Summary:

• @Pujan92 paid $250 via NewDot Manual Request
• @BhuvaneshPatil paid $250 here (Contributor)

[Pujan92]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production
• 2b. Reported on staging (deploy blocker)
• 2c. Reported on both staging and production
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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/48876/files#r1869197328

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

1. Try to access link https://staging.new.expensify.com/settings/workspaces/{workspaceId}/members/import in the URL directly by replacing the workspaceId where you are a member and not an owner
2. Verify the no access page will be shown

Do we agree 👍 or 👎

[garrettmknight]

$250 approved for @Pujan92