[WEB-3597] fix: guest work item view access when hyper mode is enabled #6785
[WEB-3597] fix: guest work item view access when hyper mode is enabled #6785sriramveeraghanta merged 2 commits intopreviewfrom
Conversation
WalkthroughThe changes add a new field, Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Storage
participant RootStore
User->>Storage: Request issues for a project
Storage->>RootStore: Retrieve project details & user role
RootStore-->>Storage: Return project details and role
alt User is guest and guest view is disallowed
Storage-->>User: Return empty results
else
Storage-->>User: Return issues data
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
…all_features is disabled
henit-chobisa
changed the title
henit-chobisa
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
web/core/local-db/utils/query-sanitizer.ts.ts (1)
28-30: Consider adding a comment explaining the shouldApplyFilter condition.The logic for determining when to apply the filter is correct, but slightly complex. A comment explaining when we should or shouldn't override the existing filter would improve maintainability.
const shouldApplyFilter = !existingCreatedByFilter || (typeof existingCreatedByFilter === "string" && existingCreatedByFilter.includes(currentUserId)); + // Apply filter when no created_by filter exists or when the current filter already includes this user
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
web/core/local-db/storage.sqlite.ts(5 hunks)web/core/local-db/utils/query-sanitizer.ts.ts(1 hunks)web/core/services/issue/issue.service.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- web/core/local-db/storage.sqlite.ts
🔇 Additional comments (5)
web/core/local-db/utils/query-sanitizer.ts.ts (2)
7-41: Solid implementation of guest access restrictions.The
sanitizeWorkItemQueriesfunction effectively implements the security fix for guest work item access by:
- Checking if the current user is a guest and if the project has disabled the
guest_view_all_featuressetting- Ensuring guests can only see work items they've created by enforcing the
created_byfilterThis implementation correctly addresses the security issue mentioned in the PR objectives.
21-25:Details
❓ Verification inconclusive
Verify the behavior when currentProject is undefined.
The code checks if
currentProject?.guest_view_all_features === false, but doesn't handle the case wherecurrentProjectis undefined. Consider what happens if project data hasn't loaded yet.
🏁 Script executed:
#!/bin/bash # Check how currentProject is initialized in the codebase # Find where projectRoot.project.getProjectById is defined rg -A 10 "projectRoot\s*=\s*{" --type ts # Check if the getProjectById method handles undefined cases rg -A 5 "getProjectById\s*\(" --type tsLength of output: 51546
Attention: Verify Handling of Undefined Project Data for Guest Users
WhencurrentProjectis undefined, the optional chaining incurrentProject?.guest_view_all_features === falseevaluates to
undefined === false(i.e. false), which means the restriction block won’t run. This behavior is consistent with many usages ofgetProjectByIdin the codebase; however, please confirm that it’s acceptable in cases where project data hasn’t loaded yet. If the design requires applying guest restrictions even while the project data is pending, an explicit check for an undefinedcurrentProjectmight be necessary.
- Review Point: Confirm that skipping the branch when
currentProjectis undefined is intended.- Suggestion: If guest users should be restricted by default when project data is not yet available, consider adding an explicit undefined check.
web/core/services/issue/issue.service.ts (3)
4-4: Good addition of the TIssueParams import.Adding this import supports the type safety improvements in the getIssues method.
79-84: Improved type safety for getIssues method.The updated type signature (
Partial<Record<TIssueParams, string | boolean>>) provides better type safety compared to the previousanytype. This aligns well with the sanitization function implemented in query-sanitizer.ts.
89-89:Details
✅ Verification successful
Verify that persistence.getIssues is updated to use the sanitizeWorkItemQueries function.
This line calls
persistence.getIssueswith the queries, but we need to ensure this function is properly utilizing the new sanitization function to enforce guest access restrictions.
🏁 Script executed:
#!/bin/bash # Check how persistence.getIssues is implemented and if it uses sanitizeWorkItemQueries # Find the persistence.getIssues method definition rg -A 15 "getIssues\s*\(" --type ts # Check if sanitizeWorkItemQueries is imported and used in the file rg "sanitizeWorkItemQueries" --type tsLength of output: 27027
Persistence Layer Query Sanitization Confirmed
After verification, it appears that the persistence layer handles query sanitization. Although the service file (
web/core/services/issue/issue.service.ts) invokespersistence.getIssueswithout directly applyingsanitizeWorkItemQueries, the implementation in the persistence layer (e.g., inweb/core/local-db/storage.sqlite.ts) shows thatsanitizeWorkItemQueriesis imported and used appropriately. No additional changes are required here.
henit-chobisa
changed the title
|
Pull Request Linked with Plane Work Items Comment Automatically Generated by Plane |
#6785) * [WEB-3597] fix: guest work item view access when hyper mode is enabled * fix: only show work item created by the guest user if the guest_view_all_features is disabled
2 participants
Description
This PR resolves an issue where guest were able to view work items even when
view access was disabled for guests.Type of Change
Summary by CodeRabbit
New Features
Refactor