-
Notifications
You must be signed in to change notification settings - Fork 291
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
Implement new extension points in IdentityPlugin and add ContextProvidingPluginSubject #4896
Implement new extension points in IdentityPlugin and add ContextProvidingPluginSubject #4896
Conversation
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
Show resolved
Hide resolved
...est/java/org/opensearch/security/plugin/RestBulkIndexDocumentIntoMixOfSystemIndexAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/filter/SecurityFilter.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/privileges/ActionPrivileges.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java
Show resolved
Hide resolved
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
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.
Thank you @cwperks. Left a few comments but generally looks good to me. Will there be a follow-up PR to clean up pluginActions matcher store once the PR in core gets merged?
src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java
Show resolved
Hide resolved
src/test/java/org/opensearch/security/auth/SecurityUserTests.java
Outdated
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/test/framework/cluster/LocalCluster.java
Show resolved
Hide resolved
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Yes there would be one more PR to implement index actions (for completeness) if opensearch-project/OpenSearch#15778 is merged. There's been a lot of discussion and I left a comment here to outline some reasons why I don't recommend system index protection to be a core feature. This gist is that the security plugin has a notion of connecting with the admin certificate that can be used as a backdoor for an operator to perform invasive actions on system indices if necessary. The admin certificate requires that a cluster has |
src/integrationTest/java/org/opensearch/security/SystemIndexTests.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/auth/BackendRegistry.java
Outdated
Show resolved
Hide resolved
...ionTest/java/org/opensearch/security/plugin/TransportIndexDocumentIntoSystemIndexAction.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/identity/PluginContextSwitcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java
Show resolved
Hide resolved
src/main/java/org/opensearch/security/privileges/SystemIndexAccessEvaluator.java
Show resolved
Hide resolved
src/integrationTest/java/org/opensearch/security/plugin/SystemIndexPlugin1.java
Outdated
Show resolved
Hide resolved
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Craig Perkins <cwperx@amazon.com>
The backport to
To backport manually, run these commands in your terminal: # Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-4896-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1924e410f80c4b958f6ec48c9691f8ae7a0af102
# Push it to GitHub
git push --set-upstream origin backport/backport-4896-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x Then, create a pull request where the |
Description
Re-opening this previously closed PR to rebase on top of Optimized Privilege Evaluation
Companion PR in core: opensearch-project/OpenSearch#14630
This PR by itself does not add additional functionality, it simply implements the new extension points in core and introduces a new class called
ContextProvidingPluginSubject
which populates a header in the ThreadContext with the canonical class name of the plugin that is executing code usingpluginSystemSubject.runAs(() -> { ... })
. See./gradlew integrationTest --tests SystemIndexTests
for an example that verifies that a block of code run withpluginSystemSubject.runAs(() -> { ... })
has contextual info in the thread context.Enhancement
Issues Resolved
Related to #4439
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.