[FEATURE/IDENTITY] Add bearer authentication to feature/identity #5611

stephen-crawford · 2022-12-21T15:48:46Z

Signed-off-by: Stephen Crawford [email protected]

Description

Adds bearer authentication support to the feature/identity branch of OpenSearch core. This implementation is built off of the existing Basic Authentication framework and includes testing.

Issues Resolved

[List any issues this PR will resolve]

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 · 2022-12-21T15:54:47Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8309/
CommitID: ac66566
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 · 2022-12-21T16:00:55Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8310/
CommitID: 9fd9d7a
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 · 2022-12-21T16:13:24Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8311/
CommitID: 9682a84
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 · 2022-12-21T17:28:57Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8312/
CommitID: c340120
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 · 2022-12-21T17:43:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8313/
CommitID: 87106b7
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 · 2022-12-21T17:55:51Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8315/
CommitID: 8683c54
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 · 2022-12-21T18:21:06Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8316/
CommitID: 303a24e
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 · 2022-12-28T15:55:28Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8552/
CommitID: b22d663
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 · 2022-12-28T16:14:20Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8553/
CommitID: 735334d
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 · 2022-12-28T17:02:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8554/
CommitID: 524c05f
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 · 2022-12-28T17:35:55Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8555/
CommitID: a5c9db0

stephen-crawford · 2022-12-28T17:42:23Z

@DarshitChanpura @cwperks This finally passed, but I am not thrilled about the license setup that I had to do. I thought that we could share licenses across modules so that authn and identity could use the same license but I could not get that to work so eventually just did it manually. If either of you know how to fix the sharing configuration let me know. Otherwise this should be all set. I can also squash the commits when doing this.

cwperks · 2022-12-28T18:05:18Z

@scrawfor99 It depends on which build.gradle file the dependency is in. The sha, LICENSE and NOTICE files should be in the build.gradle file of the project where the corresponding implementation or api dependency line is located.

In our case the identity module depends on the opensearch-authn library so since the cxf dependencies are in identity that's where the files should be so what you have done looks good to me.

For anyone reading this and wondering why there is both identity and opensearch-authn, its first had to do with how feature/identity has changed over time as we figure out a way to sandbox the changes and get ready for a merge with main down the line. feature/identity started as classes in an identity package under server, but then moved to sandbox where experimental code should go before being promoted to a main feature. Some code in the package migrated to the sandbox/opensearch-authn lib and the lib was added as a dependency to server so that the classes were importable. Most recently, the identity module was introduced and the opensearch-authn dependency on server was removed to keep the changes in this feature branch isolated from the server. opensearch-authn may be removed in the future and the classes there moved into the identity module. There may be a library introduced in the future for other modules and plugins to use identity features conveniently.

stephen-crawford · 2022-12-28T20:23:57Z

Hi @cwperks, thank you for the explanation. I was not sure what the difference between the two modules was myself so I appreciate you clearing that up. I will say that the dependencies are listed in both authn and identity--which seems like a mistake on my part given your explanation. I am going to remove them from identity and swap the api tag in the identity build.gradle over to an implementation as this should make the module inherit the transitive dependencies in authn.

github-actions · 2022-12-28T20:28:26Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8565/
CommitID: 874d100
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?

Add bearer authentication to feature/identity Signed-off-by: Stephen Crawford <[email protected]> Update documentation Signed-off-by: Stephen Crawford <[email protected]> Spotless apply Jenkins fix Signed-off-by: Stephen Crawford <[email protected]> Add license file Signed-off-by: Stephen Crawford <[email protected]> Add sha Signed-off-by: Stephen Crawford <[email protected]> Rename License File Signed-off-by: Stephen Crawford <[email protected]> Add empty notice file Signed-off-by: Stephen Crawford <[email protected]>

Signed-off-by: Stephen Crawford <[email protected]>

Adds support for BearerToken for InternalRealm and skips credential matching for jwtToken Signed-off-by: Darshit Chanpura <[email protected]> Modifies bearer auth tests to test correct response Signed-off-by: Darshit Chanpura <[email protected]> Push before rebase Signed-off-by: Stephen Crawford <[email protected]> Fix public change Signed-off-by: Stephen Crawford <[email protected]> rename test case Signed-off-by: Stephen Crawford <[email protected]> Fix exceptions Signed-off-by: Stephen Crawford <[email protected]> remove unneeded method Signed-off-by: Stephen Crawford <[email protected]> Spotless Signed-off-by: Stephen Crawford <[email protected]> Remove old exception throw Signed-off-by: Stephen Crawford <[email protected]> Fix failing test Signed-off-by: Stephen Crawford <[email protected]> Remove throw from Subject Signed-off-by: Stephen Crawford <[email protected]> Rename fail statement Signed-off-by: Stephen Crawford <[email protected]> spotless Signed-off-by: Stephen Crawford <[email protected]> Remove dead throw and split into utility class Signed-off-by: Stephen Crawford <[email protected]> Add javadocs for utility methods Signed-off-by: Stephen Crawford <[email protected]> Changes requested Signed-off-by: Stephen Crawford <[email protected]> retry Signed-off-by: Stephen Crawford <[email protected]> Try to resolve build issue Signed-off-by: Stephen Crawford <[email protected]> Update shas Signed-off-by: Stephen Crawford <[email protected]> Update licenses Signed-off-by: Stephen Crawford <[email protected]> Retry license fix Signed-off-by: Stephen Crawford <[email protected]> Retry licenses Signed-off-by: Stephen Crawford <[email protected]> Spotless Signed-off-by: Stephen Crawford <[email protected]> Remove old sha Signed-off-by: Stephen Crawford <[email protected]> Add basic into identity dependencies Signed-off-by: Stephen Crawford <[email protected]> Update SHAs Signed-off-by: Stephen Crawford <[email protected]> Add licenses Signed-off-by: Stephen Crawford <[email protected]>

Signed-off-by: Stephen Crawford <[email protected]>

github-actions · 2022-12-28T20:36:59Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8566/
CommitID: 891b9f1
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 · 2022-12-28T20:46:04Z

Gradle Check (Jenkins) Run Completed with:

RESULT: FAILURE ❌
URL: https://build.ci.opensearch.org/job/gradle-check/8567/
CommitID: f19fdac
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?

Signed-off-by: Stephen Crawford <[email protected]>

github-actions · 2022-12-28T21:33:05Z

Gradle Check (Jenkins) Run Completed with:

RESULT: SUCCESS ✅
URL: https://build.ci.opensearch.org/job/gradle-check/8570/
CommitID: 15e3019

DarshitChanpura

@scrawfor99 Thank you for addressing PR comments. Looks like there are some unintended changes in identity/build.gradle. Can you please address those?

sandbox/libs/authn/src/main/java/org/opensearch/authn/internal/InternalSubject.java

sandbox/libs/authn/src/main/java/org/opensearch/authn/realm/InternalRealm.java

sandbox/modules/identity/build.gradle

sandbox/modules/identity/src/main/java/org/opensearch/identity/SecurityRestFilter.java

DarshitChanpura · 2022-12-31T09:24:30Z

sandbox/modules/identity/src/test/java/org/opensearch/identity/BearerAuthTests.java

+            MatcherAssert.assertThat(e.getResponse().getStatusLine().getStatusCode(), equalTo(401));
+        }
+    }
+}


@cwperks @scrawfor99 This test suite seems like a mix of Unit (lines 32-91) and Integration (lines 93-159) tests. Should we separate them out into two classes. Thoughts?

Whatever people think is best seems fine with me. I see why you would separate them by test type, but I also get why keeping all the tests related to Bearer Auth stuff together is nice. Whichever is preferred I can do.

Upon further review, I think it is best to leave things as-is for now. The primary reason being the dependency handling between authn and identity. Right now, identity is dependent on authn but all of the JwtVendor* classes are in identity. In order to move the unit tests over to the AuthenticationTokenHandlerTests class we would need to make authn dependent on identity or move the AuthenticationTokenHandlerTests.
If we end up combining authn and identity then this will no longer be an issue but for the time being it seems best to merge as-is and revisit down the road.

Signed-off-by: Stephen Crawford <[email protected]>

github-actions · 2023-01-03T15:28:27Z

Gradle Check (Jenkins) Run Completed with:

RESULT: UNSTABLE ❕
TEST FAILURES:

      1 org.opensearch.action.admin.indices.create.ShrinkIndexIT.testCreateShrinkIndex

URL: https://build.ci.opensearch.org/job/gradle-check/8735/
CommitID: 518848b
Please review all flaky tests that succeeded after retry and create an issue if one does not already exist to track the flaky failure.

DarshitChanpura · 2023-01-03T19:35:53Z

@peternied Can you please review and merge this change once you get a chance?

peternied · 2023-01-03T22:04:44Z

@cwperks I'm not sure if all your comments were fully addressed, happy to review follow up PRs or follow up issues if you'd still like to see more changes.

stephen-crawford · 2023-01-03T22:13:52Z

@peternied, I believe all of @cwperks have been addressed. The only unresolved one that I am aware of is the potential merging of the identity and authn substructures which will require a separate initiative.

stephen-crawford changed the title ~~Add bearer authentication to feature/identity~~ [FEATURE/IDENTITY] Add bearer authentication to feature/identity Dec 21, 2022

stephen-crawford mentioned this pull request Dec 21, 2022

[FEATURE/IDENTITY] Bearer Authentication based on Internal tokens model #5528

Closed

6 tasks

stephen-crawford force-pushed the bearer-auth branch from 874d100 to 891b9f1 Compare December 28, 2022 20:31

stephen-crawford and others added 4 commits December 28, 2022 15:31

All but one test passing

70cec79

Signed-off-by: Stephen Crawford <[email protected]>

Remove dependencies from identity

891b9f1

Signed-off-by: Stephen Crawford <[email protected]>

Corrected formatting

f19fdac

Signed-off-by: Stephen Crawford <[email protected]>

Correct dependecy tree

15e3019

Signed-off-by: Stephen Crawford <[email protected]>

DarshitChanpura suggested changes Dec 31, 2022

View reviewed changes

Add Bearer Auth to Feature/Identity

518848b

Signed-off-by: Stephen Crawford <[email protected]>

DarshitChanpura approved these changes Jan 3, 2023

View reviewed changes

peternied approved these changes Jan 3, 2023

View reviewed changes

peternied merged commit c39a92b into opensearch-project:feature/identity Jan 3, 2023

DarshitChanpura mentioned this pull request Jan 11, 2023

[META] Identity in OpenSearch #5834

Open

35 tasks

peternied mentioned this pull request Jan 16, 2023

Support http bearer tokens for authentication #5886

Open

stephen-crawford deleted the bearer-auth branch January 31, 2023 20:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE/IDENTITY] Add bearer authentication to feature/identity #5611

[FEATURE/IDENTITY] Add bearer authentication to feature/identity #5611

stephen-crawford commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 21, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

stephen-crawford commented Dec 28, 2022

cwperks commented Dec 28, 2022 •

edited

Loading

stephen-crawford commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

DarshitChanpura left a comment

DarshitChanpura Dec 31, 2022

stephen-crawford Jan 3, 2023

stephen-crawford Jan 3, 2023

github-actions bot commented Jan 3, 2023

DarshitChanpura commented Jan 3, 2023

peternied commented Jan 3, 2023

stephen-crawford commented Jan 3, 2023

[FEATURE/IDENTITY] Add bearer authentication to feature/identity #5611

[FEATURE/IDENTITY] Add bearer authentication to feature/identity #5611

Conversation

stephen-crawford commented Dec 21, 2022

Description

Issues Resolved

Check List

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 21, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

stephen-crawford commented Dec 28, 2022

cwperks commented Dec 28, 2022 • edited Loading

stephen-crawford commented Dec 28, 2022

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

github-actions bot commented Dec 28, 2022

Gradle Check (Jenkins) Run Completed with:

DarshitChanpura left a comment

Choose a reason for hiding this comment

DarshitChanpura Dec 31, 2022

Choose a reason for hiding this comment

stephen-crawford Jan 3, 2023

Choose a reason for hiding this comment

stephen-crawford Jan 3, 2023

Choose a reason for hiding this comment

github-actions bot commented Jan 3, 2023

Gradle Check (Jenkins) Run Completed with:

DarshitChanpura commented Jan 3, 2023

peternied commented Jan 3, 2023

stephen-crawford commented Jan 3, 2023

cwperks commented Dec 28, 2022 •

edited

Loading