[$500] Web - Tasks - Assignee can mark task as completed after task has been

[kbecciv]

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.23-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:

Issue found when executing PR #33656

Action Performed:

1. Sign into account A and assign a task to account B
2. Sign into account B and open the assigned task
3. Sign back into account A and re assign the task to account C
4. Sign into account B and open the assigned task and observe the assignee has changed
5. Click the task as completed from account B

Expected Result:

Once the task is re assigned to account C account B should not be able to complete the task

Actual Result:

Even if the task is re assigned to account C, account B is still able to mark the task as completed

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

Bug6336939_1704821571346.assigneeChange.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~011401c8786c4cfbb8
• Upwork Job ID: 1744785613189410816
• Last Price Increase: 2024-01-09

[melvin-bot]

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

[melvin-bot]

Triggered auto assignment to @CortneyOfstad (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 - @Santhosh-Sellavel (External)

[dummy-1111]

Proposal

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

An account can modify task when he isn't the assignee or the creator

What is the root cause of that problem?

We check the modifiability in the canModifyTask function
Please check the below code

App/src/libs/actions/Task.js

Lines 907 to 909 in 35a6916

	if (sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport)) {
	return true;
	}

As you can see, we return true if the current user is the owner or the assignee. Even when the user isn't the owner and isn't the assignee, we allow the modification when the parent report is allowed to comment

App/src/libs/actions/Task.js

Line 920 in 35a6916

return ReportUtils.isAllowedToComment(parentReport);

This is the root cause

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

The main idea is that we should return false if the user isn't the creator nor the assignee instead of returning true if the user is the creator or the assignee

Updating

App/src/libs/actions/Task.js

Lines 907 to 909 in 35a6916

	if (sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport)) {
	return true;
	}

to

    if (sessionAccountID !== getTaskOwnerAccountID(taskReport) && sessionAccountID !== getTaskAssigneeAccountID(taskReport)) {
        return false;
    }

would solve the issue

Note: If we need to allow modification of the task for policy admins in the workspace chat or chat room, we would need to add those conditions with &&

What alternative solutions did you explore? (Optional)

[Santhosh-Sellavel]

Proposal looks good to need ensure whether this issues comes from any recent PR which is under regression period

[paultsimura]

Proposal looks good

This change will break the ability for public room admins to modify tasks

[dummy-1111]

This change will break the ability for public room admins to modify tasks

Yeah, we need to add those conditions as well if admins should be able to modify tasks.

[paultsimura]

Proposal

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

What is the root cause of that problem?

The root cause is that when checking for the user's ability to modify the task, neither of early return conditions are met, and we get all the way to the last return, which checks if the user can comment on the parent chat (their 1:1 chat).

App/src/libs/actions/Task.js

Lines 894 to 921 in 382a096

	/**
	* Check if you're allowed to modify the task - anyone that has write access to the report can modify the task
	* @param {Object} taskReport
	* @param {Number} sessionAccountID
	* @param {String} policyRole
	*
	* @returns {Boolean}
	*/
	function canModifyTask(taskReport, sessionAccountID, policyRole = '') {
	if (ReportUtils.isCanceledTaskReport(taskReport)) {
	return false;
	}
	if (sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport)) {
	return true;
	}
	const parentReport = ReportUtils.getParentReport(taskReport);
	if (policyRole && (ReportUtils.isChatRoom(parentReport) || ReportUtils.isPolicyExpenseChat(parentReport)) && policyRole !== CONST.POLICY.ROLE.ADMIN) {
	return false;
	}
	// If you don't have access to the task report (maybe haven't opened it yet), check if you can access the parent report
	// - If the parent report is an #admins only room
	// - If you are a policy admin
	return ReportUtils.isAllowedToComment(parentReport);
	}

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

In this function, we should early return false if the parent report is a chat report:

    const parentReport = ReportUtils.getParentReport(taskReport);
+    if (ReportUtils.isChatReport(parentReport)) {
+        return false;
+    }

This is safe to add because:

1. We have verified earlier that the current user is neither the task owner nor the task assignee:
   

   App/src/libs/actions/Task.js

   Lines 907 to 909 in 382a096

   	if (sessionAccountID === getTaskOwnerAccountID(taskReport) || sessionAccountID === getTaskAssigneeAccountID(taskReport)) {
   	return true;
   	}

2. This is not a room or workspace, but a chat, so the user should have no right to modify this task if they are neither the owner nor an assignee of this task

What alternative solutions did you explore? (Optional)

[dukenv0307]

Seem like it's expected cc @thienlnam

[thienlnam]

Anyone can modify the task if you can access it - this is intended

[thienlnam]

@kbecciv Can you please post the TR that created this issue? I believe we fixed a bunch but there might be more that we are missing

[kbecciv]

This issue discovered when executing PR #33656 @thienlnam

[thienlnam]

Ah yeah - the expected behavior is that anyone that can view the task can modify the task