-
-
Notifications
You must be signed in to change notification settings - Fork 94
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
BE: RBAC: Impl Active Directory populator #717
Conversation
@wernerdv wanna take a look? |
@Haarolean Yes, I'll check locally with these changes and report back. |
@Haarolean A few comments: 1 - https://github.com/kafbat/kafka-ui/pull/717/files#diff-af0ab81f6ae459e9fd60e1979d12b70ab8556054fe012848b0e930ade92208a5R52 need 2 - we can make 3 - But most importantly, RBAC with AD still only works correctly if If not, then /api/authorization returns empty permissions for user. When I did that and checked locally - RBAC is working as expected with any parameter value
|
api/src/main/java/io/kafbat/ui/service/rbac/extractor/RbacLdapAuthoritiesExtractor.java
Outdated
Show resolved
Hide resolved
api/src/main/java/io/kafbat/ui/config/auth/LdapSecurityConfig.java
Outdated
Show resolved
Hide resolved
daa488c
to
50bae9f
Compare
@wernerdv hey, I've implemented the changes I've talked about here: #717 (comment) |
...c/main/java/io/kafbat/ui/service/rbac/extractor/RbacActiveDirectoryAuthoritiesExtractor.java
Outdated
Show resolved
Hide resolved
A couple of fixes:
Otherwise there will be an error on startup. My yaml config:
Error at UI startup:
|
api/src/main/java/io/kafbat/ui/config/auth/LdapSecurityConfig.java
Outdated
Show resolved
Hide resolved
@wernerdv thanks, fixed! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked and everything works as expected
LGTM
@wernerdv thanks for the review and trying out the changes! |
What changes did you make? (Give an overview)
Is there anything you'd like reviewers to focus on?
RbacLdapAuthoritiesExtractor
. Resolves RBAC: Support Active Directory #54user
subject type for LDAP & AD RBAC. Fixes BE: RBAC: LDAP: Implementuser
subject type for LDAP & AD #730How Has This Been Tested? (put an "x" (case-sensitive!) next to an item)
Checklist (put an "x" (case-sensitive!) next to all the items, otherwise the build will fail)
Check out Contributing and Code of Conduct
A picture of a cute animal (not mandatory but encouraged)