Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Role with no access to topic data - getting error in browser console [0.25.0] #1784

Closed
sverrehu opened this issue May 22, 2024 · 4 comments · Fixed by #1785
Closed

Role with no access to topic data - getting error in browser console [0.25.0] #1784

sverrehu opened this issue May 22, 2024 · 4 comments · Fixed by #1785

Comments

@sverrehu
Copy link
Contributor

sverrehu commented May 22, 2024

I'm trying to create a role that can list topics, but not see the data on the topics. For that I copied the reader role, but removed the TOPIC_DATA resource:

reader-no-data-access:
  - resources: [ "TOPIC", "CONSUMER_GROUP", "CONNECT_CLUSTER", "CONNECTOR", "SCHEMA", "NODE", "ACL", "KSQLDB" ]
    actions: [ "READ" ]

However, now the browser console shows an error:

TypeError: Cannot read properties of undefined (reading 'includes')
    at Qf.render (AkhqRoutes.jsx:162:57)

I notice the logic in AkhqRoutes.jsx:

roles && roles.TOPIC && roles.TOPIC_DATA.includes('CREATE')

It doesn't look entirely correct. I would expect it to be either TOPIC or TOPIC_DATA on both the boolean operands, in order not to call includes on a non-existing roles key.

@AlexisSouquiere
Copy link
Collaborator

Good catch, thank you !
I think you PR can be merged. After checking the code, I'm seeing that the increase partition action should require TOPIC/UPDATE instead of TOPIC_DATA/CREATE but it requires another update in the Topic.jsx. I'll take it into account after the merge

@sverrehu
Copy link
Contributor Author

sverrehu commented May 23, 2024

Thanks for your kind comment, @AlexisSouquiere!

But I really think your excellent rewrite of the ACL system should have been marked as a breaking change in the release notes, since I did not realize it was breaking until it broke, and then in the docs I found that it was a breaking change. :-) But that's what you get when there is more than a year between releases, and you eagerly try to upgrade. :-) Thanks anyway, and keep up the good work!

Just to add my 2 cents: I would much more prefer to take part in a somewhat flaky 0.25.0, .1, .2 ... .156 patch level approach instead of having to deal with a dev branch that most businesses forbid people like me to use, rather than to have a big surprise later on. But you're the bosses, and I guess @tchiotludo is the one who decides eventually.

Bottom line, not small letters: We really enjoy your product! Thanks a lot!

@tchiotludo
Copy link
Owner

will try to release more often, just need to have reviewer for the release 🎉

@sverrehu
Copy link
Contributor Author

will try to release more often, just need to have reviewer for the release 🎉

I am not a UI person, but I can help with the Java stuff.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants