-
Notifications
You must be signed in to change notification settings - Fork 24.7k
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
[HLRC] Add support for get application privileges API #35556
Conversation
Pinging @elastic/es-security |
Pinging @elastic/es-core-infra |
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.
Left some comments, otherwise lgtm.
...nt/rest-high-level/src/main/java/org/elasticsearch/client/security/GetPrivilegesRequest.java
Outdated
Show resolved
Hide resolved
...t/rest-high-level/src/main/java/org/elasticsearch/client/security/GetPrivilegesResponse.java
Outdated
Show resolved
Hide resolved
...el/src/main/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilege.java
Outdated
Show resolved
Hide resolved
...t/rest-high-level/src/test/java/org/elasticsearch/client/SecurityRequestConvertersTests.java
Show resolved
Hide resolved
...nt/rest-high-level/src/main/java/org/elasticsearch/client/security/GetPrivilegesRequest.java
Outdated
Show resolved
Hide resolved
...high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java
Outdated
Show resolved
Hide resolved
...el/src/main/java/org/elasticsearch/client/security/user/privileges/ApplicationPrivilege.java
Show resolved
Hide resolved
...high-level/src/test/java/org/elasticsearch/client/documentation/SecurityDocumentationIT.java
Outdated
Show resolved
Hide resolved
...t-high-level/src/test/java/org/elasticsearch/client/security/GetPrivilegesResponseTests.java
Outdated
Show resolved
Hide resolved
@bizybot Thanks for the feedback and the quick turnaround, I have addressed your comments and this is ready for another look |
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.
other than a small correction in the docs, LGTM, Thank you.
client/rest-high-level/src/main/java/org/elasticsearch/client/SecurityRequestConverters.java
Outdated
Show resolved
Hide resolved
...t/rest-high-level/src/test/java/org/elasticsearch/client/SecurityRequestConvertersTests.java
Show resolved
Hide resolved
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've noticed a bug that should be fixed, but other than that LGTM .
@albertzaharovits will you take another look please at the implemented changes for the bug you caught? |
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.
LGTM Thanks!
This commits adds support for the Get Application Privileges API to the HLRC Relates: #29827
This commits adds support for the Get Application Privileges
API to the HLRC
Relates: #29827