Identity and access control foundation for OpenSearch #5925

peternied · 2023-01-18T21:06:23Z

Description

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

Outstanding Changes

Integration testing
Unit testing

Issues Resolved

Closes Initial Identity in OpenSearch #5880
Related [META] Identity in OpenSearch #5834

Check List

New functionality includes testing.
- All tests pass
New functionality has been documented.
- New functionality has javadoc added
Commits are signed per the DCO using --signoff
Commit changes are listed out in CHANGELOG.md file (See: Changelog)

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.

peternied · 2023-01-18T21:09:13Z

FYI @opensearch-project/security Please take a look, this is a departure in a couple important ways from what we were doing on the feature branch. I'm going to keep iterating on the todo list, please feel free to comment on things in this pull request or for functionality on the todo list as well.

github-actions · 2023-01-18T21:27:03Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/9556/
CommitID: 930d667
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

peternied · 2023-01-18T22:01:29Z

@dbwiddis @saratvemulapalli What do you think of adding a new plugin interface, this would be notable since the identity plugin should not be supported via the extensions interface performance would be a killer?

github-actions · 2023-01-18T22:11:02Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/9559/
CommitID: 798cd2b
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

dbwiddis · 2023-01-20T16:19:32Z

What do you think of adding a new plugin interface ... extensions interface performance would be a killer?

This seems to overly simplify the trade-offs here. We have three locations we can put things: extensions, plugins, or directly in core. I think some/most of the identity functionality should be present in core. What's the use case for putting it in a plugin rather than core?

There might be room for some extensions interacting with identity providers (providing SSO, etc.). Sign-in would generally be a one-time thing where a few milliseconds may not matter as much, while core/plugin would be better for repeated checks during operation.

cwperks · 2023-01-20T19:29:38Z

@peternied looking at this now. This is a bit of a deviation of what is in feature/identity already, can you elaborate on the differences?

@dbwiddis I'll explain the current thinking and state of the feature/identity branch. So far in feature/identity there are core identity features like an internal IdP (a list of users persisted in an OpenSearch system index), basic and bearer auth handling of requests, APIs for interacting with identity inside of plugins and core and soon-to-be REST APIs for interacting with the internal IdP (create/delete User, change password, update User, whoami) and preliminary work on granting permissions.

The way it is organized is in a library (sandbox/lib/opensearch-authn) and a module (sandbox/modules/identity). These are both sandboxed which is where the developer guide advises to put experimental code: https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#sandbox. Modules are features that are shipped with OpenSearch by default and are not optional the way that plugins are: https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#modules

The library contains the APIs for interacting with security conveniently. This will provide richer APIs for interacting with security than common-utils. This would include methods like Identity.getAuthManager.getSubject() and currentSubject.isPermitted(requestedPermission) which can be used inside of core or plugins to check permissions which currently does not exist.

The identity module is where the internal implementation of security is located and it does use the current plugin extension points of the security plugin to implement REST request authentication via ActionPlugin.getRestHandlerWrapper, register new API actions for interacting with the IdP, register an ActionFilter for authorization, register settings and defining a system index.

This is from the developer guide on modules in OpenSearch:

Features that are shipped with OpenSearch by default but are not built in to the server. We typically separate features from the server because they require permissions that we don't believe all of OpenSearch should have or because they depend on libraries that we don't believe all of OpenSearch should depend on.

We haven't run into any issues building identity as a module, but there's active discussion around leveraging existing extension points vs putting the implementation directly in server.

peternied · 2023-01-20T23:46:27Z

Basic authentication is working

[2023-01-20T23:43:24,741][INFO ][o.o.r.RestController     ] [runTask-0] Logged in as user ShiroSubject(principal=StringPrincipal(name=admin))

github-actions · 2023-01-20T23:48:53Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/9702/
CommitID: 3642a2f
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

peternied · 2023-01-20T23:51:48Z

This is a bit of a deviation of what is in feature/identity already, can you elaborate on the differences?

Yes - we are in heavy-prototyping in the feature branch - which is great. However, following the model established by the extensions team, I want to go directly into what I'd expect to be the final resting place of the identity code and associated module(s). I know this is going to create some complex merge issues in feature/identity, but I've stripped out features that we are not ready to merge into main yet.

Setting aside package naming and file placement, I've created the IdentityPlugin interface to describe the extensibility model, IdentityModule handles picking up the plugin and accessing the functionality throughout the server codebase.

DarshitChanpura · 2023-01-23T18:01:57Z

@peternied This PR is a good segue into "identity as core feature". Is that the long term vision for this feature? If so how should the task priorities be considered for future development.

peternied · 2023-01-23T18:19:02Z

Is that the long term vision for this feature?

Yes! This is a refinement of the work done in features/identity [^1] to a digestible chunk of functionality. I'm borrowing heavily from the work recently done by @ryanbogan to integrate extensions which have a similar trajectory.

If so how should the task priorities be considered for future development.

This is a good topic for the meta issue [^2] around this work, but I would say the project board's readme captures the current status the best [^3]

peternied · 2023-01-23T18:35:34Z

@dbwiddis This seems to overly simplify the trade-offs here. We have three locations we can put things: extensions, plugins, or directly in core. I think some/most of the identity functionality should be present in core. What's the use case for putting it in a plugin rather than core?

Security evolved outside of core for many years - there are many external implementations of identity and access controls built as plugins. While we own the Security plugin in this project, there are others customers that want to keep using alternative implementations.

By having the a plugin layer it will protect us from ourselves from too much coupling as we iterate. I recently pushed an implementation of this interface I'd be curious what you think of the module and its separation of responsibility.

cwperks · 2023-01-23T21:23:00Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroIdentityPlugin.java

+    public ShiroIdentityPlugin(final Settings settings) {
+        this.settings = settings;
+
+        SecurityManager securityManager = new DefaultSecurityManager(InternalRealm.INSTANCE);


The DefaultSecurityManager will create sessions by default. This will not be stateless as basic auth should be.

stephen-crawford

I left some comments places with general notes and a couple of things to check.

stephen-crawford · 2023-01-23T21:29:04Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+            String username = ((UsernamePasswordToken) token).getUsername();
+            // Look up the user by the provide username
+            User userRecord = getInternalUser(username);
+            // Check for other things, like a locked account, expired password, etc.


Is this supposed to be a TODO or is that part of the getInternalUser call?

stephen-crawford · 2023-01-23T21:29:59Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+                throw new IncorrectCredentialsException();
+            }
+        }
+        // Don't know what to do with this token


Should we be including this into main? It seems like this section is still a work in progress.

stephen-crawford · 2023-01-23T21:32:24Z

modules/identity-shiro/src/test/java/org/opensearch/identity/AuthTokenHandlerTests.java

+    }
+
+    // TODO: Fix this test
+    // public void testShouldExtractBasicAuthTokenSuccessfully_twoSemiColonPassword() {


Should this be removed?

I need to fix these tests before I can merge, they should be uncommented before then. The TODO above captures

modules/identity-shiro/src/test/java/org/opensearch/identity/AuthTokenHandlerTests.java

modules/identity-shiro/src/test/java/org/opensearch/identity/BasicAuthTests.java

server/src/main/java/org/opensearch/identity/tokens/AuthToken.java

server/src/main/java/org/opensearch/identity/tokens/UnsupportedHttpAuthenticationToken.java

server/src/main/java/org/opensearch/rest/RestController.java

server/src/main/java/org/opensearch/identity/IdentityModule.java

github-actions · 2023-01-23T23:45:48Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/9798/
CommitID: d444768
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

RyanL1997

Do we have the lint check for this PR? As Stephen mentioned above, it seems like we are missing some of the additional line space at end of couple files.

peternied · 2023-01-24T17:56:57Z

Do we have the lint check for this PR? As Stephen mentioned above, it seems like we are missing some of the additional line space at end of couple files.

This pull request is in draft and fixing all CR checks will be required to merge. I'll get around to these, but the functionality is where I am concentrating my efforts.

server/src/main/java/org/opensearch/identity/IdentityModule.java

modules/identity-shiro/build.gradle

...es/identity-shiro/src/main/java/org/opensearch/identity/shiro/OpenSearchSecurityManager.java

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

modules/identity-shiro/src/main/plugin-metadata/plugin-security.policy

server/src/main/java/org/opensearch/identity/tokens/BasicAuthToken.java

server/src/main/java/org/opensearch/rest/RestController.java

github-actions · 2023-01-27T04:12:32Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10018/
CommitID: 57f495b
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

github-actions · 2023-02-03T20:31:46Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/10609/
CommitID: 81af972
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

peternied · 2023-02-13T23:50:32Z

@reta @nknize @saratvemulapalli I haven't seen any comments - I'm going to flush out the all test and get gradle check passing without waiting on any more feedback.

reta · 2023-02-14T00:02:47Z

modules/identity-shiro/build.gradle

+
+apply plugin: 'opensearch.internal-cluster-test'
+
+opensearchplugin {


I think this module will be an ideal candidate for sandbox ?

There are a couple of ways to introduce non-GA features into OpenSearch, sandbox changes the build output which is easy to test locally, but difficult from the distribution channels. Using feature flags was recently introduced and is easier to control outside this repository. What do you think of this tradeoff to include the module but have it disabled via feature flag?

OpenSearch/server/src/main/java/org/opensearch/common/util/FeatureFlags.java

Line 60 in 81af972

public static final String IDENTITY = "opensearch.experimental.feature.identity.enabled";

I believe there are 2 parts to it:

the core support (new plugin interfaces), this could be good candidate for feature flags

the new experimental plugin (like this one), this could go to sandbox

👍 I'll move the module into sandbox

sandbox changes the build output which is easy to test locally, but difficult from the distribution channels

@peternied Feel free to weigh in on #3677

reta · 2023-02-14T00:05:04Z

...hiro/src/internalClusterTest/java/org/opensearch/identity/HttpSmokeTestCaseWithIdentity.java

+ * @opensearch.experimental
+ */
+// TODO not sure why ThreadLeakScope.NONE is required
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)


We probably could remove the @ThreadLeakScope(ThreadLeakScope.Scope.NONE), the thread leaks have to be detected

reta · 2023-02-14T00:17:08Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+        }
+
+        public InternalRealm build() {
+            // TODO: Replace hardcoded admin user / user map with an external provider


👍 totally agree, we should not hardcode

reta · 2023-02-14T00:26:56Z

@reta @nknize @saratvemulapalli I haven't seen any comments - I'm going to flush out the all test and get gradle check passing without waiting on any more feedback.

My apologies @peternied , didn't have time today, did a quick pass but will take a closer look tomorrow, thank you

reta · 2023-02-14T14:34:05Z

CHANGELOG.md


 ### Dependencies
+- Bumps `wiremock-jre8-standalone` from 2.33.2 to 2.35.0


These changes are not related, right?

Bad merge, cleaning up...

reta · 2023-02-14T14:45:58Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

+     * @return the shiro auth token to be for login
+     */
+    public AuthenticationToken translateAuthToken(org.opensearch.identity.tokens.AuthToken authenticationToken) {
+        final AuthenticationToken authToken = null;


The authToken is not used here

Good call, cleaning this up.

reta · 2023-02-14T15:00:48Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroSecurityManager.java

+
+        // By default shiro stores session information into a cache, there were performance
+        // issues with this sessions cache and so are defaulting to a stateless configuration
+        this.subjectDAO = new StatelessDAO();


I don't think you need StatelessDAO class, you could just disable session storage (and consequently, the cache if needed):

final DefaultSessionStorageEvaluator evaluator = new DefaultSessionStorageEvaluator(); evaluator.setSessionStorageEnabled(false); final DefaultSubjectDAO subjectDAO = new DefaultSubjectDAO(); subjectDAO.setSessionStorageEvaluator(evaluator); setSubjectDAO(subjectDAO);

Thanks for the example, using this.

reta · 2023-02-14T15:02:16Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroSubject.java

+
+    public ShiroSubject(final AuthTokenHandler authTokenHandler, final org.apache.shiro.subject.Subject subject) {
+        this.authTokenHandler = authTokenHandler;
+        this.shiroSubject = subject;


Could either authTokenHandler or subject be null?

They shouldn't be, adding Objects.requireNonNull(...) to ensure this

reta · 2023-02-14T15:13:01Z

.../identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/BCryptPasswordMatcher.java

+    @Override
+    public boolean doCredentialsMatch(AuthenticationToken token, AuthenticationInfo info) {
+        final UsernamePasswordToken userToken = (UsernamePasswordToken) token;
+        final String password = new String(userToken.getPassword());


The password is not needed, you convert from char[] to String and back to char[], just use userToken.getPassword() directly

reta · 2023-02-14T15:14:14Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+                realmName
+            );
+
+            // TODO: Doesn't appear to check the password


is this comment still valid?

Nope, removing

reta · 2023-02-14T15:14:54Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+        }
+
+        // If the token was not handled, it was unsupported
+        throw new UnsupportedTokenException("Unable to support authentication token " + token.getClass().getName());


token could be null here, better to check

reta · 2023-02-14T15:21:56Z

modules/identity-shiro/src/test/java/org/opensearch/identity/AbstractIdentityTestCase.java

+ * Base test case for integration tests against the identity plugin.
+ */
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
+public abstract class AbstractIdentityTestCase extends OpenSearchIntegTestCase {


Where this test case is used?

reta · 2023-02-14T15:28:56Z

server/src/main/java/org/opensearch/identity/Principals.java

+    /**
+     * Represents a principal which has not been authenticated
+     */
+    UNAUTHENTICATED(new StringPrincipal("Unauthenticated"));


This seems to be very limited set of possible principals, I think we should not model it as an enumeration but constants:

public final class BuiltinPrincipals { public static final Principal UNAUTHENTICATED = new StringPrincipal("Unauthenticated"); }

I think enums are much more expressive and flexible. I'd like to leave it in, we can clean it up if its cumbersome with further implementation.

Enums imply closed set of choices - this is not the case here

reta · 2023-02-14T15:31:24Z

server/src/main/java/org/opensearch/identity/StringPrincipal.java

+ *
+ * @opensearch.experimental
+ */
+public class StringPrincipal implements Principal {


Should we rename it to UsernamePrincipal instead? StringPrincipal is definitely not giving the context what this principal is about.

To me user implies human, I think we will be using this object to represent segments of the OpenSearch runtime. I believe we will replace many of the ThreadPool.Names with these principals to limit the kinds of access those thread have.

Ok, what about NamedPrincipal?

reta · 2023-02-14T15:42:47Z

server/src/main/java/org/opensearch/identity/tokens/RestTokenExtractor.java

+                return new BasicAuthToken(authHeaderValueStr);
+            } else {
+                if (logger.isDebugEnabled()) {
+                    String tokenTypeTruncated = Strings.substring(authHeaderValueStr, 0, 5);


I think we should remove the tokenTypeTruncated, it would output weird token types (for example, imagine Digest ... here)

In a debugging scenario, I think seeing those weird token times would be useful if you were expecting the request to authenticated and wasn't sure why. What do you think?

I mean, in debugging you could always say that the auth header was not Basic (if you ever need to do that).

reta · 2023-02-14T15:45:18Z

server/src/main/java/org/opensearch/plugins/IdentityPlugin.java

+    /**
+     * Get the current subject
+     * */
+    public Subject getSubject();


Could getSubject() return null vaues? If yes, we have to reconsider the API and return Optional<Subject> instead

Subject should never be null.

@NotNull on return type?

saratvemulapalli · 2023-02-14T17:37:43Z

@reta @nknize @saratvemulapalli I haven't seen any comments - I'm going to flush out the all test and get gradle check passing without waiting on any more feedback.

Sorry for not getting back on this, I am resurfacing from hibernation :).
Looking now..

saratvemulapalli

Lot of barebones and learning to understand Shiro.
These 2 resources helped me learn:

Couple of questions and dropped few comments:

What are your thoughts on re-using existing security plugins authn/authz mechanisms.
Can we write realms to use all of our business logic we have already built
I dont see compliance built into the framework, how do you plan to handle it (atleast to start with audit logging).

saratvemulapalli · 2023-02-14T17:49:29Z

CHANGELOG.md

@@ -9,8 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add getter for path field in NestedQueryBuilder ([#4636](https://github.com/opensearch-project/OpenSearch/pull/4636))
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459))
+- Add support to disallow search request with preference parameter with strict weighted shard routing([#5874](https://github.com/opensearch-project/OpenSearch/pull/5874))
+- Add identity and access control extension point ([5925](https://github.com/opensearch-project/OpenSearch/pull/5925))


This is assuming changes will not be available in OpenSearch 2.x.
Is that the intention?

Thanks for noticing, I've moved this into the Unreleased 2.x section.

This document is confusing to work with for the first time.

saratvemulapalli · 2023-02-14T18:17:47Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

+ *
+ * @opensearch.experimental
+ */
+class AuthTokenHandler {


No opinions, trying to understand: Looks like a utility class. This class doesn't have any state, probably can we go static?
Are there more implementations which will end up here?

Non static so this class can be dependency injected easily.

saratvemulapalli · 2023-02-15T00:47:47Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

+    private static final Logger logger = LogManager.getLogger(AuthTokenHandler.class);
+
+    /**
+     * Translates shiro auth token from the given header token


super nit:

Suggested change

* Translates shiro auth token from the given header token

* Translates into shiro auth token from the given header token

saratvemulapalli · 2023-02-15T01:01:54Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/ShiroSubject.java

+    private final org.apache.shiro.subject.Subject shiroSubject;
+
+    public ShiroSubject(final AuthTokenHandler authTokenHandler, final org.apache.shiro.subject.Subject subject) {
+        this.authTokenHandler = authTokenHandler;


Isn't authTokenHandler for all subjects the same for OpenSearch?
Does it have to be a member for each subject?

Passing via constructor for dependency injection / testing - is there a convention to follow for classes that aren't guice injected but use DI?

saratvemulapalli · 2023-02-15T01:04:55Z

.../identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/BCryptPasswordMatcher.java

+    @Override
+    public boolean doCredentialsMatch(AuthenticationToken token, AuthenticationInfo info) {
+        final UsernamePasswordToken userToken = (UsernamePasswordToken) token;
+        final String password = new String(userToken.getPassword());


saratvemulapalli · 2023-02-15T01:10:41Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+            adminUser.setUsername(new StringPrincipal("admin"));
+            adminUser.setBcryptHash("$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG"); // Password 'admin'
+            final Map<String, User> internalUsers = Map.of("admin", adminUser);
+            return new InternalRealm(name, internalUsers);


How do we plan to integrate this with existing security plugin users:

For new clusters, read through the yml configs and populate them.

For existing clusters which already has loaded yml configs, load it from security system index.

We've got tasks for building up the UserService and management APIs - when we work on #5883 all these pieces will be included.

saratvemulapalli · 2023-02-15T01:14:45Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/realm/InternalRealm.java

+            final SimpleAuthenticationInfo sai = new SimpleAuthenticationInfo(
+                userRecord.getUsername(),
+                userRecord.getBcryptHash(),
+                realmName


Do you plan to have multiple realms for OpenSearch (one for authn and one for authz)?

No plans for multiple realms.

saratvemulapalli · 2023-02-15T01:18:52Z

server/src/main/java/org/opensearch/identity/IdentityService.java

+    public Subject getSubject() {
+        return identityPlugin.getSubject();
+    }
+}


This is clean!!!

saratvemulapalli · 2023-02-15T01:23:07Z

server/src/main/java/org/opensearch/rest/RestController.java

+        if (!FeatureFlags.isEnabled(FeatureFlags.IDENTITY)) {
+            return true;
+        }


I would prefer moving this to L406, lets not get to handleAuthenticateUser if the feature is not enabled.

saratvemulapalli · 2023-02-15T01:25:30Z

server/src/main/java/org/opensearch/rest/RestController.java

+            return false;
+        }
+
+        return true;


By default we should always false and return true only when the subject is authenticated (at L537)

Authentication didn't fail so this is accurate, see the other commment thread in this file https://github.com/opensearch-project/OpenSearch/pull/5925/files#r1089137958. Adding a comment in the code that clarifies.

Signed-off-by: Peter Nied <[email protected]>

peternied

I'll do the migration to sandbox and the test fixes fast following this most recent push

peternied · 2023-02-16T22:33:19Z

CHANGELOG.md

@@ -9,8 +9,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Add getter for path field in NestedQueryBuilder ([#4636](https://github.com/opensearch-project/OpenSearch/pull/4636))
 - Allow mmap to use new JDK-19 preview APIs in Apache Lucene 9.4+ ([#5151](https://github.com/opensearch-project/OpenSearch/pull/5151))
 - Add support for ppc64le architecture ([#5459](https://github.com/opensearch-project/OpenSearch/pull/5459))
+- Add support to disallow search request with preference parameter with strict weighted shard routing([#5874](https://github.com/opensearch-project/OpenSearch/pull/5874))
+- Add identity and access control extension point ([5925](https://github.com/opensearch-project/OpenSearch/pull/5925))


Thanks for noticing, I've moved this into the Unreleased 2.x section.

This document is confusing to work with for the first time.

peternied · 2023-02-16T22:33:23Z

CHANGELOG.md


 ### Dependencies
+- Bumps `wiremock-jre8-standalone` from 2.33.2 to 2.35.0


Bad merge, cleaning up...

peternied · 2023-02-16T22:34:17Z

...hiro/src/internalClusterTest/java/org/opensearch/identity/HttpSmokeTestCaseWithIdentity.java

+ * @opensearch.experimental
+ */
+// TODO not sure why ThreadLeakScope.NONE is required
+@ThreadLeakScope(ThreadLeakScope.Scope.NONE)


peternied · 2023-02-16T22:35:45Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

+ *
+ * @opensearch.experimental
+ */
+class AuthTokenHandler {


Non static so this class can be dependency injected easily.

peternied · 2023-02-16T22:36:04Z

modules/identity-shiro/src/main/java/org/opensearch/identity/shiro/AuthTokenHandler.java

+    private static final Logger logger = LogManager.getLogger(AuthTokenHandler.class);
+
+    /**
+     * Translates shiro auth token from the given header token


peternied · 2023-02-17T21:30:22Z

server/src/main/java/org/opensearch/plugins/IdentityPlugin.java

+    /**
+     * Get the current subject
+     * */
+    public Subject getSubject();


Subject should never be null.

peternied · 2023-02-17T21:32:36Z

server/src/main/java/org/opensearch/rest/RestController.java

+        if (!FeatureFlags.isEnabled(FeatureFlags.IDENTITY)) {
+            return true;
+        }


peternied · 2023-02-17T21:43:15Z

server/src/main/java/org/opensearch/rest/RestController.java

+
+        try {
+            final AuthToken token = RestTokenExtractor.extractToken(request);
+            // If no token was found, continue executing the request


This method is authenticate - not authorize. Consider anonymous scenarios, maybe calls like cluster health would be allowed for anonymous requests. These authorizing calls would happen at lower layer where details about the resource associated with the request is against can factor in 403 or allow.

Note; authorization is not included in this change.

peternied · 2023-02-17T21:45:56Z

server/src/main/java/org/opensearch/rest/RestController.java

+
+        try {
+            final AuthToken token = RestTokenExtractor.extractToken(request);
+            // If no token was found, continue executing the request


Adding this as a comment in the code

peternied · 2023-02-17T21:46:50Z

server/src/main/java/org/opensearch/rest/RestController.java

+            return false;
+        }
+
+        return true;


Authentication didn't fail so this is accurate, see the other commment thread in this file https://github.com/opensearch-project/OpenSearch/pull/5925/files#r1089137958. Adding a comment in the code that clarifies.

github-actions · 2023-02-17T22:57:56Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/11329/
CommitID: a9b4f0d
Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green.
Is the failure a flaky test unrelated to your change?

peternied · 2023-05-03T07:01:50Z

This change was merged as part of the following pull request

[Extensions] Introduce Identity Plugin to Core #7246

peternied added the Identity PR/Issues associated with Authentication or Authorization label Jan 18, 2023

mch2 mentioned this pull request Jan 18, 2023

[Backport 2.x] Muting TranslogTransferManagerTests on Windows. #5926

Merged

cwperks mentioned this pull request Jan 20, 2023

[Feature/Identity] Select AuthenticationManager via configuration #5953

Closed

6 tasks

cwperks reviewed Jan 23, 2023

View reviewed changes

stephen-crawford reviewed Jan 23, 2023

View reviewed changes

cwperks reviewed Jan 23, 2023

View reviewed changes

server/src/main/java/org/opensearch/rest/RestController.java Outdated Show resolved Hide resolved

cwperks reviewed Jan 23, 2023

View reviewed changes

server/src/main/java/org/opensearch/identity/IdentityModule.java Outdated Show resolved Hide resolved

RyanL1997 reviewed Jan 24, 2023

View reviewed changes

peternied commented Jan 24, 2023

View reviewed changes

server/src/main/java/org/opensearch/identity/IdentityModule.java Outdated Show resolved Hide resolved

DarshitChanpura reviewed Jan 26, 2023

View reviewed changes

peternied force-pushed the core-identity branch 2 times, most recently from b2162a7 to 403cdec Compare January 27, 2023 04:40

peternied marked this pull request as ready for review January 27, 2023 04:40

peternied requested review from reta and anasalkouz as code owners January 27, 2023 04:40

reta reviewed Feb 14, 2023

View reviewed changes

saratvemulapalli reviewed Feb 15, 2023

View reviewed changes

peternied added 2 commits February 17, 2023 22:00

partial review feedback

4d81495

Signed-off-by: Peter Nied <[email protected]>

More feedback

a9b4f0d

Signed-off-by: Peter Nied <[email protected]>

peternied commented Feb 17, 2023

View reviewed changes

peternied self-assigned this Mar 29, 2023

peternied mentioned this pull request Apr 3, 2023

[META] Security for Extensions opensearch-project/security#2524

Open

stephen-crawford mentioned this pull request Apr 19, 2023

[Extensions] Introduce Identity Plugin to Core #7246

Merged

6 tasks

stephen-crawford self-assigned this Apr 19, 2023

peternied closed this May 3, 2023


		apply plugin: 'opensearch.internal-cluster-test'

		opensearchplugin {


		### Dependencies
		- Bumps `wiremock-jre8-standalone` from 2.33.2 to 2.35.0

	* Translates shiro auth token from the given header token
	* Translates into shiro auth token from the given header token

Identity and access control foundation for OpenSearch #5925

Identity and access control foundation for OpenSearch #5925

Conversation

peternied commented Jan 18, 2023 • edited Loading

Description

Outstanding Changes

Issues Resolved

Check List

peternied commented Jan 18, 2023

github-actions bot commented Jan 18, 2023

Gradle Check (Jenkins) Run Completed with:

peternied commented Jan 18, 2023

github-actions bot commented Jan 18, 2023

Gradle Check (Jenkins) Run Completed with:

dbwiddis commented Jan 20, 2023

cwperks commented Jan 20, 2023

peternied commented Jan 20, 2023

github-actions bot commented Jan 20, 2023

Gradle Check (Jenkins) Run Completed with:

peternied commented Jan 20, 2023

DarshitChanpura commented Jan 23, 2023

peternied commented Jan 23, 2023

peternied commented Jan 23, 2023

Choose a reason for hiding this comment

stephen-crawford left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 23, 2023

Gradle Check (Jenkins) Run Completed with:

RyanL1997 left a comment

Choose a reason for hiding this comment

peternied commented Jan 24, 2023

github-actions bot commented Jan 27, 2023

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Feb 3, 2023

Gradle Check (Jenkins) Run Completed with:

peternied commented Feb 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta commented Feb 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reta Feb 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternied commented Jan 18, 2023 •

edited

Loading

reta Feb 14, 2023 •

edited

Loading

reta Feb 14, 2023 •

edited

Loading

reta Feb 14, 2023 •

edited

Loading