Repository navigation
[CP Staging] Revert "Fix: Prevent "From" link display when user lacks access to parent report" - #71775
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| import type {SearchFullscreenNavigatorParamList} from '@libs/Navigation/types'; | ||
| import {getReportAction, shouldReportActionBeVisible} from '@libs/ReportActionsUtils'; | ||
| import {canCurrentUserOpenReport, canUserPerformWriteAction as canUserPerformWriteActionReportUtils} from '@libs/ReportUtils'; | ||
| import {canUserPerformWriteAction as canUserPerformWriteActionReportUtils} from '@libs/ReportUtils'; |
There was a problem hiding this comment.
⚠️ Potential Security/UX Issue
This revert removes the canCurrentUserOpenReport import and its usage, which was previously used to prevent displaying the "From" link when users lack access to the parent report.
Impact: Users might now see "From" links to reports they cannot actually access, leading to:
- Poor user experience when clicking non-functional links
- Potential information disclosure by showing report names users shouldn't see
Recommendation: Consider re-implementing the access check with a different approach or ensure this revert addresses a more critical issue that outweighs the security/UX concerns.
| // We should not display the parent navigation subtitle if the user does not have access to the parent chat (the reportName is empty or user lacks access) | ||
| if (!reportName || !canCurrentUserOpenReport(report, isReportArchived)) { | ||
| // We should not display the parent navigation subtitle if the user does not have access to the parent chat (the reportName is empty in this case) | ||
| if (!reportName) { |
There was a problem hiding this comment.
⚠️ Logic Regression
This simplified condition removes the access control check that was previously preventing users from seeing links to reports they cannot access.
Previous logic: !reportName || !canCurrentUserOpenReport(report, isReportArchived)
Current logic: !reportName
Issue: The component will now render the "From" link even when canCurrentUserOpenReport(report, isReportArchived) returns false, potentially exposing report names and creating non-functional navigation links.
Recommendation: If this revert is necessary due to issues with canCurrentUserOpenReport, consider:
- Adding proper error handling for the access check
- Implementing an alternative access validation method
- Adding user feedback when links are non-functional
|
@arosiclair looks like this was merged without a test passing. Please add a note explaining why this was done and remove the |
|
@carlosmiceli Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
Straight revert |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
mountiny
changed the title
…ink-inaccessible-parent-reports Revert "Fix: Prevent "From" link display when user lacks access to parent report" (cherry picked from commit 7a98595) (cherry-picked to staging by mountiny)
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.2.22-4 🚀
|
|
🚀 Deployed to production by https://github.com/Julesssss in version: 9.2.22-6 🚀
|
|
🚀 Cherry-picked to staging by https://github.com/mountiny in version: 9.2.23-0 🚀
|
|
🚀 Deployed to production by https://github.com/lakchote in version: 9.2.23-3 🚀
|
Reverts #71443
$ #71727