Skip to content

Conversation

Rishav9852Kumar
Copy link
Contributor

@Rishav9852Kumar Rishav9852Kumar commented May 14, 2025

Description

Currently, our system only allows for the invalidation of the entire user authentication cache, which can lead to numerous cache misses and inefficiencies. When dealing with LDAP user updates, this global cache invalidation affects all users unnecessarily.

Solution

This PR introduces a new REST endpoint that enables selective cache invalidation at the individual user level. This allows for more precise cache management, particularly useful when handling LDAP user updates where only specific user entries become stale.

Key Changes

  • Introduces new REST endpoint /api/security/authcache/users/{username} for selective cache invalidation
  • Adds integration tests to verify the behavior with LDAP authentication
  • Implements proper authentication and security controls for the new endpoint

Benefits

  • Prevents unnecessary cache misses for unaffected users
  • Improves system efficiency by maintaining cache integrity for other users
  • Provides better handling of LDAP user updates without impacting the entire cache

Developer Note

This issue was aimed as an excellent introduction to the plugin development workflow, touching all crucial aspects including REST API development, integration testing, unit testing, and authentication handling with external providers - making it an ideal "good first issue" for new contributors.

Acknowledgments

This work builds upon the contributions of:

Related Work

Issues Resolved

#2829

Is this a backport? If so, please add backport PR # and/or commits #, and remove backport-failed label 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

IntegrationTest

LdapAuthenticationCacheTest.java

FlushCacheApiIntegrationTest.java

Unit tesing

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.

@Rishav9852Kumar Rishav9852Kumar changed the title add flush cache for individual user Add flush cache for individual user May 14, 2025
@Rishav9852Kumar Rishav9852Kumar changed the title Add flush cache for individual user Add flush cache endpoint for individual user May 14, 2025
Copy link

codecov bot commented May 16, 2025

Codecov Report

Attention: Patch coverage is 67.16418% with 22 lines in your changes missing coverage. Please review.

Project coverage is 72.08%. Comparing base (a1717ee) to head (f1ef7d4).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
...ch/security/dlic/rest/api/FlushCacheApiAction.java 60.71% 9 Missing and 2 partials ⚠️
...urity/action/configupdate/ConfigUpdateRequest.java 69.23% 1 Missing and 3 partials ⚠️
...tion/configupdate/TransportConfigUpdateAction.java 69.23% 0 Missing and 4 partials ⚠️
.../org/opensearch/security/auth/BackendRegistry.java 76.92% 2 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5337      +/-   ##
==========================================
+ Coverage   72.06%   72.08%   +0.01%     
==========================================
  Files         381      381              
  Lines       23608    23654      +46     
  Branches     3621     3632      +11     
==========================================
+ Hits        17014    17050      +36     
- Misses       4798     4799       +1     
- Partials     1796     1805       +9     
Files with missing lines Coverage Δ
.../org/opensearch/security/auth/BackendRegistry.java 77.51% <76.92%> (-0.03%) ⬇️
...urity/action/configupdate/ConfigUpdateRequest.java 83.33% <69.23%> (-10.79%) ⬇️
...tion/configupdate/TransportConfigUpdateAction.java 87.09% <69.23%> (-12.91%) ⬇️
...ch/security/dlic/rest/api/FlushCacheApiAction.java 69.23% <60.71%> (+1.37%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Signed-off-by: Rishav Kumar <[email protected]>
@Rishav9852Kumar Rishav9852Kumar changed the title Add flush cache endpoint for individual user Selective User Cache Invalidation Enhancement May 16, 2025
Signed-off-by: Rishav Kumar <[email protected]>
Rishav9852Kumar and others added 6 commits May 21, 2025 11:32
cwperks
cwperks previously approved these changes May 23, 2025
Copy link
Member

@cwperks cwperks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Rishav9852Kumar. This change looks good to me! From an end-user perspective, the /_plugins/_security/cache/user/{username} makes sense to me.

I know there is still some discussion on the organization around the internal transport action, but since that's not user facing I think that can be addressed in a separate PR and I see that there's already a comment in the code explaining the re-use of the action.

Rishav Kumar added 3 commits May 26, 2025 14:33
Signed-off-by: Rishav Kumar <[email protected]>
Signed-off-by: Rishav Kumar <[email protected]>
Signed-off-by: Rishav Kumar <[email protected]>
@Rishav9852Kumar Rishav9852Kumar requested a review from cwperks May 26, 2025 10:12
Copy link
Collaborator

@shikharj05 shikharj05 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Rishav9852Kumar - as a follow-up, can you try and improve coverage for the changes? For e.g. see https://github.com/opensearch-project/security/pull/5337/checks?check_run_id=42888662886 for missing coverage

@cwperks cwperks merged commit a23b35c into opensearch-project:main Jun 5, 2025
69 of 71 checks passed
@Rishav9852Kumar Rishav9852Kumar deleted the flush-catche-indi-user branch June 16, 2025 09:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants