Skip to content
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

[Feature/Identity] Prototype Internal IdP #4659

Merged

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Sep 30, 2022

Description

As part of the identity feature branch, this adds a foundation for building an internal IdP inside of core. This PR includes a working prototype of an IdP similar to the one in the security plugin. I'm creating this as a draft to solicit feedback, there is currently no way to interact with the internal realm via actions in OpenSearch.

This branch shows a custom realm built with shiro called the internal realm and uses BCrypt (salt + hash) for authenticating subjects. I had to add a line in security.policy to get the ObjectMapper working for jackson-databind and also needed to add a dependency on slf4j for shiro, but I'd like to see if there are ways not to include this since opensearch uses log4j for logging.

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.

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@cwperks
Copy link
Member Author

cwperks commented Oct 3, 2022

@peternied Do you think we should consider making the work we're doing a native module of OpenSearch?

https://github.com/opensearch-project/OpenSearch/blob/main/DEVELOPER_GUIDE.md#modules

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

@peternied
Copy link
Member

@peternied Do you think we should consider making the work we're doing a native module of OpenSearch?

We spoke about this briefly offline, recapping from the discussion. While we could refactor this to reduce the number of default dependencies on OpenSearch, I think we at the minimum need security to be accessible without installing anything else.

I think this is a good motivation to get a pull request into main sooner so we can see the point for or against it. I might be a good idea to add documentation of any concerns you have in the code so when we do the feature/identity -> main pull request those ideas are surfaced

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2022

Gradle Check (Jenkins) Run Completed with:

Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2022

Gradle Check (Jenkins) Run Completed with:

@cwperks
Copy link
Member Author

cwperks commented Oct 7, 2022

@peternied I wanted to leave an update on this branch. I took the work I was doing in the org.opensearch.identity package and created a new library under sandbox called opensearch-authn with the top level package being org.opensearch.authn to match the library name. I think this is the right place to put this work for now until it can be promoted as a lib of opensearch. It also makes it easy to create a build with native authn included or not by specifying ./gradlew assemble -Dsandbox.enabled=false.

One of the problems I faced in this branch is that the build was failing on ./gradlew dependencyLicenses because server got the new transitive dependencies introduced in opensearch-authn. A lot of the deleted files on this PR were no longer needed since the licenses and jars were already in the opensearch-authn library.

There are currently CI issues with jarHell in bouncy castle >.<

class: org.bouncycastle.LICENSE
jar1: /Users/cwperx/.gradle/caches/modules-2/files-2.1/org.bouncycastle/bc-fips/1.0.2.3/da62b32cb72591f5b4d322e6ab0ce7de3247b534/bc-fips-1.0.2.3.jar
jar2: /Users/cwperx/.gradle/caches/modules-2/files-2.1/org.bouncycastle/bcprov-jdk15on/1.70/4636a0d01f74acaf28082fb62b317f1080118371/bcprov-jdk15on-1.70.jar

Bouncy castle is needed by authn for its BCryptHash capabilities which will not be released in Shiro until 2.0.

@peternied
Copy link
Member

There are currently CI issues with jarHell in bouncy castle >.<

How would you feel about disabling jarhell? We did this in security because of issues with the Kafka test libraries [1], then we can make an issue and circle back.

It might be as easy as adding the following to server/build.gradle

jarHell.enabled = false

[1] https://github.com/opensearch-project/security/blob/main/build.gradle#L103

@peternied
Copy link
Member

Note; I saw that you disabled jar hell in the authz plugin, it looks like the failure is coming out in :distribution:tools:plugin-cli:jarHell is it just that one that is causing issues, otherwise we could unregister the gradle tasks for the time being

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2022

Gradle Check (Jenkins) Run Completed with:

@cwperks
Copy link
Member Author

cwperks commented Oct 7, 2022

There are currently CI issues with jarHell in bouncy castle >.<

How would you feel about disabling jarhell? We did this in security because of issues with the Kafka test libraries [1], then we can make an issue and circle back.

It might be as easy as adding the following to server/build.gradle

jarHell.enabled = false

[1] https://github.com/opensearch-project/security/blob/main/build.gradle#L103

Thank you @peternied. I disabled the jar hell check for plugin-cli and would like to revisit to see if there is another solution to resolve the jar hell problem.

@cwperks cwperks marked this pull request as ready for review October 7, 2022 21:58
@cwperks cwperks requested review from a team and reta as code owners October 7, 2022 21:58
@peternied
Copy link
Member

@cwperks Note; I am happy to merge so you can address in follow up PRs

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@cwperks
Copy link
Member Author

cwperks commented Oct 11, 2022

@peternied CI is passing again - issues this last time with security policy with jackson-databind and the integration test suite. In a follow-up PR I plan to see if there's another way of transforming yaml -> POJOs to eliminate the dependency on jackson-databind and jackson-annotations for this new library.

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.

3 participants