security icon indicating copy to clipboard operation
security copied to clipboard

[FEATURE] Endpoint to purge cache entries for specific users

Open prabhask5 opened this issue 1 year ago • 4 comments

Description

Finishing previous PR here: https://github.com/opensearch-project/security/pull/4314

Currently, our system only allows for the invalidation of the entire user authentication cache, which can lead to numerous cache misses and inefficiencies. This change is required to allow for more precise cache management, specifically targeting stale cache entries at the individual user level without disrupting the cache state of other users

Category: New feature Why these changes are required? Currently, our system only allows for the invalidation of the entire user authentication cache. This change is required to allow for more precise cache management, specifically targeting stale cache entries at the individual user level without disrupting the cache state of other users. What is the old behavior before changes and new behavior after changes? Previously, invalidating a user's cache required clearing the entire cache, affecting all users. The new behavior introduces an endpoint that allows for the invalidation of cache entries on a per-user basis, thereby maintaining cache integrity for other users and reducing unnecessary cache misses.

Issues Resolved

https://github.com/opensearch-project/security/issues/2829

Is this a backport? If so, please add backport PR # and/or commits # no

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

Unit testing was written in FlushCacheApiTest.java

Check List

  • [x] New functionality includes testing
  • [ ] New functionality has been documented
  • [ ] New Roles/Permissions have a corresponding security dashboards plugin PR
  • [ ] API changes companion pull request created
  • [x] Commits are signed per the DCO using --signoff

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.

prabhask5 avatar Jul 16 '24 14:07 prabhask5

@cwperks This should be a better PR, I'm not sure why there's integration test errors I don't change them, can you help me out with that? It should be the same test errors as in https://github.com/opensearch-project/security/pull/4568

prabhask5 avatar Jul 16 '24 14:07 prabhask5

Thank you @prabhask5 ! Looking at the integ test failures now.

cwperks avatar Jul 16 '24 14:07 cwperks

@prabhask5 Can you rebase this PR with the latest from main that has a fix for the compilation error?

cwperks avatar Jul 17 '24 14:07 cwperks

@prabhask5 For the issue with FlushCacheApiIntegrationTest.testFlushCache there are 2 problems:

  1. In FlushCacheApiAction you need to modify the response message if the param username is non-null. i.e.
if (username != null) {
    LOGGER.debug("Cache invalidated for user: " + username);
    ok(channel, "Cache invalidated for user: " + username);
} else {
    LOGGER.debug("cache flushed successfully");
    ok(channel, "Cache flushed successfully.");
}

You would also need to move the extraction of this param up one level in scope.

  1. This line is using the wrong response object. It should be referencing deleteUserCacheResponse instead of deleteAllCacheResponse.

cwperks avatar Jul 25 '24 13:07 cwperks

@prabhask5 Closing this PR since there hasn't been activity since 1 month. Please feel free to open a new PR if you intend to continue working on this.

DarshitChanpura avatar Aug 22 '24 18:08 DarshitChanpura