Change API token index actions to use action listeners and limit to 100 tokens outstanding #5147
Conversation
Signed-off-by: Derek Ho <[email protected]>
…curity into actionget-limit
Signed-off-by: Derek Ho <[email protected]>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/api-tokens #5147 +/- ##
======================================================
- Coverage 71.46% 71.37% -0.09%
======================================================
Files 334 355 +21
Lines 22552 23475 +923
Branches 3590 3686 +96
======================================================
+ Hits 16117 16756 +639
- Misses 4642 4883 +241
- Partials 1793 1836 +43
🚀 New features to boost your workflow:
|
| for (ApiToken token : tokens.values()) { | ||
| builder.startObject(); | ||
| builder.field(NAME_FIELD, token.getName()); | ||
| builder.field(CREATION_TIME_FIELD, token.getCreationTime().toEpochMilli()); |
There was a problem hiding this comment.
Can we call this issued_at to be aligned with the rfc? https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.6
| try { | ||
| XContentBuilder builder = channel.newBuilder(); | ||
| builder.startObject(); | ||
| builder.field("Api Token: ", token); |
There was a problem hiding this comment.
Let's just have the field name be token. No need to include the colon here because this gets serialized to json and will get included.
| // If count is ok, create the token | ||
| apiTokenRepository.createApiToken(name, clusterPermissions, indexPermissions, expiration, ActionListener.wrap(token -> { | ||
| // After successful creation, trigger the update action | ||
| ApiTokenUpdateRequest updateRequest = new ApiTokenUpdateRequest(); |
There was a problem hiding this comment.
This logic should be centralized so its applied after any change like issuance of a new token or revocation to make sure each node of the cluster synchronizes with the change as it occurs.
| if (deletedDocs == 0) { | ||
| listener.onFailure(new ApiTokenException("No token found with name " + name)); | ||
| } else { | ||
| listener.onResponse(null); |
There was a problem hiding this comment.
No synchronization after delete? Will this ensure that the token is no longer able to be used regardless of which node a request is made against?
Signed-off-by: Derek Ho <[email protected]>
Description
Changes index operations on the api token index to be action listeners instead of action get. Also implements 100 api token limit.
Issues Resolved
[List any issues this PR will resolve]
Is this a backport? If so, please add backport PR # and/or commits #, and remove
backport-failedlabel from the original PR.Do these changes introduce new permission(s) to be displayed in the static dropdown on the front-end? If so, please open a draft PR in the security dashboards plugin and link the draft PR here
Testing
[Please provide details of testing done: unit testing, integration testing and manual testing]
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.