RockteMQ-AI commented on code in PR #338:
URL:
https://github.com/apache/rocketmq-dashboard/pull/338#discussion_r3918417897
##########
frontend-new/src/pages/Acl/acl.jsx:
##########
@@ -132,6 +133,16 @@ const Acl = () => {
}, [activeTab]); // Dependencies for useEffect
+ useEffect(() => {
+ const userPermission = localStorage.getItem('userrole');
+ console.log(userPermission);
Review Comment:
Uses loose equality `userPermission == 2` — `localStorage.getItem()` returns
a string `"2"`, so this relies on JS type coercion. Use `=== '2'` or
`Number(userPermission) === 2` for clarity and safety.
##########
frontend-new/src/pages/Acl/acl.jsx:
##########
@@ -132,6 +133,16 @@ const Acl = () => {
}, [activeTab]); // Dependencies for useEffect
+ useEffect(() => {
+ const userPermission = localStorage.getItem('userrole');
+ console.log(userPermission);
Review Comment:
**[Security]** Client-side permission check using
`localStorage.getItem('userrole')` is cosmetic only — it can be trivially
bypassed by editing localStorage in DevTools. The real authorization must be
enforced on the backend (the `MQAdminAspect` additions in this PR are good, but
they only cover MQ admin operations, not the ACL user management API endpoints
like `updateUser`/`deleteUser`). Please ensure the backend ACL controller
endpoints also validate the caller's role before executing write operations.
##########
frontend-new/src/pages/Acl/acl.jsx:
##########
@@ -132,6 +133,16 @@ const Acl = () => {
}, [activeTab]); // Dependencies for useEffect
+ useEffect(() => {
+ const userPermission = localStorage.getItem('userrole');
Review Comment:
`console.log(userPermission)` is left in — please remove before merge.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
