-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
[Extensions] Introduce Identity Plugin to Core #7246
[Extensions] Introduce Identity Plugin to Core #7246
Conversation
Adding a new module and plugin interface that provides identity and access control inside of OpenSearch. This is the founding building block, see more high level thoughts here on our recent blog. https://opensearch.org/blog/Introducing-Identity/ The new extension point, IdentityPlugin, is added with IdentityService handling that plugin interface. IdentitySevice authenticates users and enables access control systems. Adding HTTP basic authentication in the RestController the default NoopIdentityPlugin changes no behavior. When the identity feature flag is enabled the identity-shiro module is loaded into the IdentityService. This implementations includes an admin user backed through the Apache Shiro system. By partitioning these features through a plugin interface it isolate the dependencies if we ever switch out the underlying library. Project tracking addition features and functionality - https://github.com/orgs/opensearch-project/projects/89?pane=info Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Peter Nied <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
af4f452
to
9a0ad21
Compare
Gradle Check (Jenkins) Run Completed with:
|
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more @@ Coverage Diff @@
## main #7246 +/- ##
============================================
+ Coverage 70.54% 70.57% +0.03%
- Complexity 59387 59450 +63
============================================
Files 4859 4873 +14
Lines 285370 285542 +172
Branches 41134 41151 +17
============================================
+ Hits 201317 201535 +218
+ Misses 67428 67413 -15
+ Partials 16625 16594 -31
|
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
Go through the code and get it to a minimum PR. Let me know when this is ready to review, I was tracking the previous one. |
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
identityPlugin = identityPlugins.get(0); | ||
} else { | ||
throw new OpenSearchException( | ||
"Multiple identity plugins are not supported, found: " |
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.
Since this PR introduces a default module and this file enforces at most 1 IdentityPlugin
is installed at a time there is no way to install an IdentityPlugin with this. The opensearch node fails with this error:
OpenSearchException[Multiple identity plugins are not supported, found: org.opensearch.identity.shiro.ShiroIdentityPlugin,org.opensearch.security.OpenSearchSecurityPlugin]
at org.opensearch.identity.IdentityService.<init>(IdentityService.java:38)
at org.opensearch.node.Node.<init>(Node.java:464)
at org.opensearch.node.Node.<init>(Node.java:377)
at org.opensearch.bootstrap.Bootstrap$5.<init>(Bootstrap.java:242)
at org.opensearch.bootstrap.Bootstrap.setup(Bootstrap.java:242)
at org.opensearch.bootstrap.Bootstrap.init(Bootstrap.java:404)
at org.opensearch.bootstrap.OpenSearch.init(OpenSearch.java:180)
at org.opensearch.bootstrap.OpenSearch.execute(OpenSearch.java:171)
at org.opensearch.cli.EnvironmentAwareCommand.execute(EnvironmentAwareCommand.java:104)
at org.opensearch.cli.Command.mainWithoutErrorHandling(Command.java:138)
at org.opensearch.cli.Command.main(Command.java:101)
at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:137)
at org.opensearch.bootstrap.OpenSearch.main(OpenSearch.java:103)
For complete error details, refer to the log at /Users/cwperx/Desktop/distributions/opensearch-stephen-identity/opensearch-3.0.0-SNAPSHOT/logs/opensearch.log
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.
Hi @cwperks, great catch. This can be resolved in a couple minor changes--making the identity plugin used configurable or allowing shiro to be turned off--but it may be best to wait until @peternied is available and ask him what he thinks considering much of this was his original work.
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.
Since moving forward the we are not intending on using [1] security plugin build out of core, I think we shouldn't make it harder for identity plugin developers to get started by requiring them to remove/uninstall the identity-shiro plugin. I think we can revisit how this works as we get feedback from other security implementations, until then we should default to use the 'no op' provider.
To support this seems like we might need to move the location from module to plugin. @scrawfor99 I think your question [2] framed this well and we can figure out how to best move forward with a recommendation from the maintainers.
Hi @reta, @dblock, and @saratvemulapalli. I am tagging you because we are trying to move this PR forward but wanted your input. As mentioned by @cwperks, in this comment, the way that this PR is configured will need to be modified so that the IdentityPlugin being used is configurable. Currently, we cannot install any other IdentityPlugins since Shiro will always be installed by virtue of being a module. Before I change this, I wanted to get the opinion of some core maintainers about whether we could move forward with this PR afterwards. An alternative approach to getting our changes into Core would be to start with a barebones Identity Interface and then merging smaller changes. This may be easier to review, but will not show the same functional demonstration that the Shiro implementation @peternied setup would. Which direction do you prefer I take? |
I think this is a good point but do we want it to be configurable? I mean if in 99.9% use cases the default is enough, the other 0.1% of the users could always uninstall it and replace with the custom plugin. Also, do we always want to bundle it? May be we could just move it to |
Hi @reta, I think moving it from modules to a bundled plugin would be an appropriate solution. The reason we are planning on bundling it would be so that users could test the functionality of the Identity Interface and potentially create their own implementations. Basically, we are going a similar route as the SDK's HelloWorld extension where we doubt anyone will choose to use Shiro but this lets us demonstrate how the interface works. Moving the code from a module to a plugin should allow us to ignore the installation issue and we could look into further configuration changes as needed. |
+1 |
Gradle Check (Jenkins) Run Completed with:
|
Signed-off-by: Stephen Crawford <[email protected]>
logger.info("Identity on so found plugins implementing: " + pluginsService.filterPlugins(IdentityPlugin.class).toString()); | ||
identityPlugins.addAll(pluginsService.filterPlugins(IdentityPlugin.class)); | ||
} | ||
logger.info("Creating identity service in Node.java with identity plugins: " + identityPlugins.toString()); |
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.
Oops, @scrawfor99 my apologies, I missed that, probably Node.java
is too precise:
logger.info("Creating identity service in Node.java with identity plugins: " + identityPlugins.toString()); | |
logger.info("Creating identity service for node with identity plugins: " + identityPlugins.toString()); |
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.
Oops, I did not mean to include that print. I will remove it.
Signed-off-by: Stephen Crawford <[email protected]>
Gradle Check (Jenkins) Run Completed with:
|
Gradle Check (Jenkins) Run Completed with:
|
The backport to
To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-7246-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 1151308ed4ac22f17eb0167746b062d04f755cf4
# Push it to GitHub
git push --set-upstream origin backport/backport-7246-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/backport-2.x Then, create a pull request where the |
* Identity and access control for OpenSearch Adding a new service and plugin interface that provides identity and access control inside of OpenSearch. This is the founding building block, see more high level thoughts here on our recent blog. https://opensearch.org/blog/Introducing-Identity/ The new extension point, IdentityPlugin, is added with IdentityService handling that plugin interface. IdentityService authenticates users and enables access control systems. Adding HTTP basic authentication in the RestController the default NoopIdentityPlugin changes no behavior. Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Peter Nied <[email protected]> Co-authored-by: Andriy Redko <[email protected]>
@@ -395,6 +403,11 @@ private void tryAllHandlers(final RestRequest request, final RestChannel channel | |||
return; | |||
} | |||
} else { | |||
if (FeatureFlags.isEnabled(FeatureFlags.IDENTITY)) { | |||
if (!handleAuthenticateUser(request, channel)) { |
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.
@peternied @scrawfor99 Is the idea of this change in the RestController to use this to authenticate the request instead of ActionPlugin.getRestHandlerWrapper
? If identity is enabled to you envision that the security plugin would make the rest wrapper a noop and the logic would be executed in handleAuthenticateUser
instead?
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.
Yes, I imagine at some point the Rest Wrapper would be completely removed from the security plugin as there will be tighter interfaces that better support audit logging or www-authentication features.
…7246) (#7393) * [Extensions] Introduce Identity Plugin to Core (#7246) * Identity and access control for OpenSearch Adding a new service and plugin interface that provides identity and access control inside of OpenSearch. This is the founding building block, see more high level thoughts here on our recent blog. https://opensearch.org/blog/Introducing-Identity/ The new extension point, IdentityPlugin, is added with IdentityService handling that plugin interface. IdentityService authenticates users and enables access control systems. Adding HTTP basic authentication in the RestController the default NoopIdentityPlugin changes no behavior. Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Peter Nied <[email protected]> Co-authored-by: Andriy Redko <[email protected]> * swap changelog Signed-off-by: Stephen Crawford <[email protected]> * Swap plugin name Signed-off-by: Stephen Crawford <[email protected]> --------- Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Peter Nied <[email protected]> Co-authored-by: Andriy Redko <[email protected]>
* Identity and access control for OpenSearch Adding a new service and plugin interface that provides identity and access control inside of OpenSearch. This is the founding building block, see more high level thoughts here on our recent blog. https://opensearch.org/blog/Introducing-Identity/ The new extension point, IdentityPlugin, is added with IdentityService handling that plugin interface. IdentityService authenticates users and enables access control systems. Adding HTTP basic authentication in the RestController the default NoopIdentityPlugin changes no behavior. Signed-off-by: Stephen Crawford <[email protected]> Signed-off-by: Peter Nied <[email protected]> Co-authored-by: Peter Nied <[email protected]> Co-authored-by: Andriy Redko <[email protected]> Signed-off-by: Shivansh Arora <[email protected]>
Description
Takes over for #5925 since Peter is out.
Per the origing PR:
Adding a new module and plugin interface that provides identity and access control inside of OpenSearch. This is the founding building block, see more high level thoughts here on our recent blog.
The new extension point, IdentityPlugin, is added with IdentityService handling that plugin interface. IdentitySevice authenticates users and enables access control systems. Adding HTTP basic authentication in the RestController the default NoopIdentityPlugin changes no behavior.
When the identity feature flag is enabled the identity-shiro module is loaded into the IdentityService. This implementations includes an admin user backed through the Apache Shiro system. By partitioning these features through a plugin interface it isolate the dependencies if we ever switch out the underlying library.
Project tracking addition features and functionality: https://github.com/orgs/opensearch-project/projects/89?pane=info
Issues Resolved:
#5880
Check List