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] Identity Module and tokens for internal authentication #5471

Closed

Conversation

cwperks
Copy link
Member

@cwperks cwperks commented Dec 6, 2022

Description

Opening a draft PR to solicit feedback for implementation of internal authentication.

This PR introduces a new sandbox module identity that will use some of the existing extension points that the security plug-in does to authenticate rest requests and pass a token around on the header of the threadcontext of a task that identifies the user and can subsequently be used for authorization.

This new identity module uses a few existing extension points from the ActionPlugin and the NetworkPlugin.

From the ActionPlugin this branch uses:

  • getRestHandlerWrapper to provide a wrapper that handles authentication. As of now, there is only a Basic auth mechanism that uses the internal IdP in this feature branch to authenticate the user and return a 403 if the request cannot be authenticated
  • getActionFilters - This branch introduces an AuthorizationFilter that is intended to be used to perform authorization. This is mostly pass-through at the moment and right now it verifies that a token is present and valid before the TransportRequest performs its doExecute

From the NetworkPlugin this uses:

  • getTransportInterceptors - The transport interceptor intercepts outgoing TransportRequests and can modify the request before its sent to another node. When testing this branch, I ran into problems with how the TransportMessageListener intercepted outgoing requests as the ThreadContext was not available to inspect to ensure that the token that received the RestRequest created a token before sending the transport request to other nodes. When running the test its clear to see that other nodes received it the created token, but the message listener is unable to get it because of how its wrapped in an ActionListener in OutboundHandler:
void sendRequest(...) throws IOException, TransportException {
        Version version = Version.min(this.version, channelVersion);
        OutboundMessage.Request message = new OutboundMessage.Request(
            threadPool.getThreadContext(),
            features,
            request,
            version,
            action,
            requestId,
            isHandshake,
            compressRequest
        );
        ActionListener<Void> listener = ActionListener.wrap(() -> messageListener.onRequestSent(node, requestId, action, request, options));
        sendMessage(channel, message, listener);
    }

The transport interceptor has access to the ThreadContext and the tests will be updated to use the interceptor.

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.

cwperks and others added 15 commits November 21, 2022 15:00
…rt actions to identify the subject

Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
…roject#5439)

Add conditional check on assertNull to fix flaky tests.

Signed-off-by: Rishikesh1159 <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
…he Mime4j 0.8.8, Apache Poi 5.2.3, Apache PdfBox 2.0.27 (opensearch-project#5448)

Signed-off-by: Andriy Redko <[email protected]>

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

github-actions bot commented Dec 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

Gradle Check (Jenkins) Run Completed with:

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

github-actions bot commented Dec 7, 2022

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

Gradle Check (Jenkins) Run Completed with:

ashking94 and others added 2 commits December 7, 2022 23:01
…ect#5282)

* CheckpointState enhanced to support no-op replication

Signed-off-by: Ashish Singh <[email protected]>
Co-authored-by: Bukhtawar Khan<[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Dec 7, 2022

Gradle Check (Jenkins) Run Completed with:

noCharger and others added 3 commits December 14, 2022 14:39
* Refactor Object to Fuzziness type for all query builders

Signed-off-by: noCharger <[email protected]>

* Revise on bwc

Signed-off-by: noCharger <[email protected]>

* Update change log

Signed-off-by: noCharger <[email protected]>

Signed-off-by: noCharger <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

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

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

cwperks and others added 2 commits December 14, 2022 17:08
* Added bwc version 2.4.2

Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>

* Added 2.4.2.

Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>

* Update Lucene snapshot to 9.5.0-snapshot-d5cef1c

Signed-off-by: Suraj Singh <[email protected]>

* Update changelog entry

Signed-off-by: Suraj Singh <[email protected]>

* Add 2.4.2 bwc version

Signed-off-by: Suraj Singh <[email protected]>

* Internal changes post lucene upgrade

Signed-off-by: Suraj Singh <[email protected]>

Signed-off-by: Daniel (dB.) Doubrovkine <[email protected]>
Signed-off-by: Suraj Singh <[email protected]>
Co-authored-by: opensearch-ci-bot <[email protected]>
Co-authored-by: Daniel (dB.) Doubrovkine <[email protected]>
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@dreamer-89
Copy link
Member

dreamer-89 commented Dec 14, 2022

Gradle Check (Jenkins) Run Completed with:

Failing due to version bump after 2.4.1 release. #5560.

* What went wrong:
Execution failed for task ':distribution:bwc:bugfix:buildBwcLinuxTar'.
> Building 2.4.1 didn't generate expected file /var/jenkins/workspace/gradle-check/search/distribution/bwc/bugfix/build/bwc/checkout-2.4/distribution/archives/linux-tar/build/distributions/opensearch-min-2.4.1-SNAPSHOT-linux-x64.tar.gz

@dreamer-89
Copy link
Member

@cwperks : Can you please rebase your changes against latest changes in main ?

@cwperks
Copy link
Member Author

cwperks commented Dec 14, 2022

@dreamer-89 Thank you for checking on that. Was that build issue resolved with this PR (#5570)? I will merge the latest from main into the identity feature branch and rebase this branch.

@dreamer-89
Copy link
Member

@dreamer-89 Thank you for checking on that. Was that build issue resolved with this PR (#5570)? I will merge the latest from main into the identity feature branch and rebase this branch.

Thanks @cwperks. Yes, issue is resolved on latest main. Please let know if you see any other issue on main.

* Add CI bundle pattern for ivy repo

Signed-off-by: Zelin Hao <[email protected]>

* Gradle update

Signed-off-by: Zelin Hao <[email protected]>

* Extract path

Signed-off-by: Zelin Hao <[email protected]>

* Change with customDistributionDownloadType

Signed-off-by: Zelin Hao <[email protected]>

* Add default for exception handle

Signed-off-by: Zelin Hao <[email protected]>

* Add documentations

Signed-off-by: Zelin Hao <[email protected]>

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

Gradle Check (Jenkins) Run Completed with:

@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

@codecov-commenter
Copy link

codecov-commenter commented Dec 14, 2022

Codecov Report

Merging #5471 (1d56fa3) into feature/identity (31bcda6) will decrease coverage by 0.00%.
The diff coverage is 60.78%.

@@                  Coverage Diff                   @@
##             feature/identity    #5471      +/-   ##
======================================================
- Coverage               71.01%   71.00%   -0.01%     
- Complexity              58149    58509     +360     
======================================================
  Files                    4711     4768      +57     
  Lines                  277573   278956    +1383     
  Branches                40180    40296     +116     
======================================================
+ Hits                   197122   198079     +957     
- Misses                  64293    64783     +490     
+ Partials                16158    16094      -64     
Impacted Files Coverage Δ
...search/transport/Netty4NioServerSocketChannel.java 0.00% <0.00%> (ø)
.../java/org/opensearch/transport/NettyAllocator.java 45.45% <0.00%> (ø)
...rch/authn/internal/InternalAccessTokenManager.java 0.00% <0.00%> (ø)
.../opensearch/authn/jwt/BadCredentialsException.java 0.00% <0.00%> (ø)
.../opensearch/authn/noop/NoopAccessTokenManager.java 0.00% <0.00%> (ø)
.../java/org/opensearch/authn/tokens/AccessToken.java 0.00% <0.00%> (ø)
.../java/org/opensearch/identity/ConfigConstants.java 0.00% <0.00%> (ø)
...rg/opensearch/identity/ThreadContextConstants.java 0.00% <0.00%> (ø)
.../main/java/org/opensearch/action/ActionModule.java 99.11% <ø> (+1.47%) ⬆️
...tion/admin/cluster/state/ClusterStateResponse.java 80.00% <0.00%> (-2.36%) ⬇️
... and 590 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

dependabot bot and others added 3 commits December 14, 2022 15:32
…opensearch-project#5519)

* Bump protobuf-java from 3.21.9 to 3.21.11 in /plugins/repository-hdfs

Bumps [protobuf-java](https://github.com/protocolbuffers/protobuf) from 3.21.9 to 3.21.11.
- [Release notes](https://github.com/protocolbuffers/protobuf/releases)
- [Changelog](https://github.com/protocolbuffers/protobuf/blob/main/generate_changelog.py)
- [Commits](protocolbuffers/protobuf@v3.21.9...v3.21.11)

---
updated-dependencies:
- dependency-name: com.google.protobuf:protobuf-java
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>

* Updating SHAs

Signed-off-by: dependabot[bot] <[email protected]>

* Updated changelog

Signed-off-by: Owais Kazi <[email protected]>

Signed-off-by: dependabot[bot] <[email protected]>
Signed-off-by: Owais Kazi <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com>
Co-authored-by: Owais Kazi <[email protected]>
Co-authored-by: Suraj Singh <[email protected]>
Signed-off-by: Craig Perkins <[email protected]>
@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 Dec 15, 2022

I opened a separate PR after a recent merge with main into feature/identity to squash these commits and simplify. Closing this PR in favor of: #5583

@cwperks cwperks closed this Dec 15, 2022
@github-actions
Copy link
Contributor

Gradle Check (Jenkins) Run Completed with:

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.