Fix(publisher): Inconsistent role based behaviour in comment reply functionality#1391
Conversation
In the Publisher portal comment reply functionality, currently only the Admins and Publishers were allowed to make a reply to a comment regarding their user scopes for comments. This fixes it by instead of checking the user role check whether the user has necessary scopes for adding a comment (apim:comment_write or apim:comment_manage) Fixes #4881
📝 WalkthroughWalkthroughComment creation and reply authorization now use user comment scopes. API-type restrictions and creator/administrator role checks were replaced with scope checks in the publisher comment components. ChangesComment scope authorization
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
ShavinAnjithaAlpha
changed the title
| return isRestricted(['apim:api_create', 'apim:api_publish'], api); | ||
| } | ||
| const user = AuthManager.getUser(); | ||
| return !user.scopes.includes("apim:comment_write") && !user.scopes.includes("apim:comment_manage"); |
There was a problem hiding this comment.
The original isRestricted() utility handles the user being null other cases of having publish permissions and lifecycle states. any reason to remove that?
There was a problem hiding this comment.
In the backend carbon-apimgt logic for addComment, there were no consideration in the API lifecycle state and publish permission checks, therefore rather than using existing generic isRestricted helper method, a separate method has been used to check the permissions for the Add Comment in the frontend.
| const username = Utils.getUserNameWithoutDomain(user.name); | ||
| const canDelete = (comment.createdBy === username) || user.isAdmin(); | ||
| const canReply = !user.isCreator() || user.isAdmin(); | ||
| const canReply = user.scopes.includes("apim:comment_write") || user.scopes.includes("apim:comment_manage"); |
There was a problem hiding this comment.
cant we use the isRestricted same util method here
| return isRestricted(['apim:api_create', 'apim:api_publish'], api); | ||
| } | ||
| const user = AuthManager.getUser(); | ||
| return !user.scopes.includes("apim:comment_write") && !user.scopes.includes("apim:comment_manage"); |
There was a problem hiding this comment.
Also logic here is an AND. but our scope validation logic behaves in OR in REST API
There was a problem hiding this comment.
Yes, the REST API allows access if the user has either scope (OR). Since this frontend method calculates whether to restrict the user, it needs to return true only when the user is missing both scopes. So !write && !manage exactly mirrors the backend's write || manage logic"
2 participants
Purpose
In the Publisher portal's comment reply functionality, currently only users with publisher and admin roles can reply to a comment. For other roles, the
Replybutton andAdd Commenttext field are disabled, even if those roles have the default scopes to create a comment. The current implementation only checks the user's role instead of checking their scopes for the necessary permissions.This PR fixes this issue by replacing that logic with proper user scope checking for both the
Replybutton's visibility and theAdd Commenttext field's editability.Fixes #4881
Public Issue Link
Approach
File Modified:
CommentOptions.jsx,CommentAdd.jsxCommentOptions.jsx: Updated thecanReplyboolean variable to determine its value using the user's scopes instead of user roles.CommentAdd.jsx: Updated theisAccessRestrictedfunction to check the user's scopes for adding comments. (Note: The existing implementation used the defaultAuthManager.isRestrictedfunction, which is not suitable for this scenario).