Expanding Authentication with SecurityRequest Abstraction by peternied · Pull Request #3430 · opensearch-project/security

peternied · 2023-10-01T23:18:37Z

Description

Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture.

graph TD
    A[SecurityRequestFactory] -->|Creates| E[SecurityRequest]
    A -->|Creates| B[SecurityRequestChannel]
    C[Netty Channel Handler] -->|Provides data to| A
    D[OpenSearch Rest Controller] -->|Provides data to| A
    
    subgraph SecurityRequest Objects
        E --> B[SecurityRequestChannel]
    end
    
    B --> |Used in| SecurityFilter
    B --> |Used in| BackendRegistry
    B --> |Used in| HttpAuthenticator

    style A fill:#f9d,stroke:#333,stroke-width:2px;
    style B fill:#afd,stroke:#333,stroke-width:2px;
    style C fill:#fea,stroke:#333,stroke-width:2px;
    style D fill:#fea,stroke:#333,stroke-width:2px;
    style E fill:#afd,stroke:#333,stroke-width:2px;

To Do items:

Unit tests for new components
Check code coverage on impacted changes, add / update tests

Issues Resolved

Related [Feature] Improve compressed request handling OpenSearch#10260

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

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.

Signed-off-by: Peter Nied <petern@amazon.com>

…equest Signed-off-by: Peter Nied <petern@amazon.com>

Signed-off-by: Peter Nied <petern@amazon.com>

codecov · 2023-10-01T23:22:03Z

Codecov Report

Merging #3430 (e7b0916) into main (bfba97a) will increase coverage by 0.02%.
The diff coverage is 64.85%.

@@             Coverage Diff              @@
##               main    #3430      +/-   ##
============================================
+ Coverage     64.57%   64.60%   +0.02%     
- Complexity     3549     3573      +24     
============================================
  Files           269      275       +6     
  Lines         20365    20451      +86     
  Branches       3377     3382       +5     
============================================
+ Hits          13151    13212      +61     
- Misses         5526     5544      +18     
- Partials       1688     1695       +7

Files	Coverage Δ
...ava/org/opensearch/security/auditlog/AuditLog.java	`100.00% <ø> (ø)`
...security/auditlog/AuditLogSslExceptionHandler.java	`57.14% <ø> (ø)`
...org/opensearch/security/auditlog/NullAuditLog.java	`0.00% <ø> (ø)`
...earch/security/auditlog/impl/AbstractAuditLog.java	`76.59% <ø> (ø)`
...pensearch/security/auditlog/impl/AuditLogImpl.java	`88.09% <ø> (ø)`
...ava/org/opensearch/security/auth/UserInjector.java	`90.66% <ø> (ø)`
...arch/security/dlic/rest/api/AbstractApiAction.java	`88.61% <100.00%> (ø)`
...earch/security/dlic/rest/api/NodesDnApiAction.java	`89.74% <ø> (ø)`
...rity/dlic/rest/api/RestApiPrivilegesEvaluator.java	`69.23% <100.00%> (+0.15%)`	⬆️
...curity/dlic/rest/validation/EndpointValidator.java	`94.20% <ø> (ø)`
... and 32 more

Signed-off-by: Peter Nied <petern@amazon.com>

RyanL1997

Hi @peternied, great change. I just left some comments

src/main/java/org/opensearch/security/http/RemoteIpDetector.java

src/main/java/org/opensearch/security/http/XFFResolver.java

src/main/java/org/opensearch/security/ssl/util/SSLRequestHelper.java

src/main/java/com/amazon/dlic/auth/http/saml/HTTPSamlAuthenticator.java

src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java

opensearch-trigger-bot · 2023-10-06T00:58:48Z

The backport to 2.x failed:

The process '/usr/bin/git' failed with exit code 128

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.x 2.x
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.x
# Create a new branch
git switch --create backport/backport-3430-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f435c052a826b15683b8a6a6f2ccd9dd107070f9
# Push it to GitHub
git push --set-upstream origin backport/backport-3430-to-2.x
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.x

Then, create a pull request where the base branch is 2.x and the compare/head branch is backport/backport-3430-to-2.x.

opensearch-trigger-bot · 2023-10-06T00:58:50Z

The backport to 2.11 failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Navigate to the root of your repository
cd $(git rev-parse --show-toplevel)
# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add ../.worktrees/security/backport-2.11 2.11
# Navigate to the new working tree
pushd ../.worktrees/security/backport-2.11
# Create a new branch
git switch --create backport/backport-3430-to-2.11
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 f435c052a826b15683b8a6a6f2ccd9dd107070f9
# Push it to GitHub
git push --set-upstream origin backport/backport-3430-to-2.11
# Go back to the original working tree
popd
# Delete the working tree
git worktree remove ../.worktrees/security/backport-2.11

Then, create a pull request where the base branch is 2.11 and the compare/head branch is backport/backport-3430-to-2.11.

…ion (opensearch-project#3430) Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit f435c05)

…tion (opensearch-project#3430) Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit f435c05)

…tion (#3430) (#3488) Backport of f435c05 from #3430 Signed-off-by: David Osorno <daosgu27@gmail.com> Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: opensearch-trigger-bot[bot] <98922864+opensearch-trigger-bot[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com> Co-authored-by: Andrey Pleskach <ples@aiven.io>

…ion (#3430) (#3487) Backport of f435c05 from #3430 Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit f435c05)

…ion (opensearch-project#3430) Backport of f20cc68 from opensearch-project#3430 Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit f20cc68)

…ion (#3487) (#3670) Backport of f20cc68 from #3430 Introduced a new abstraction, SecurityRequest & SecurityRequestChannel, to streamline and secure the authentication process in the OpenSearch Security plugin. By isolating the essential request components needed for authentication, we minimize potential risks associated with previous designs and provide a more maintainable architecture. Signed-off-by: Peter Nied <petern@amazon.com> (cherry picked from commit f20cc68) Signed-off-by: Peter Nied <petern@amazon.com>

peternied added 12 commits September 29, 2023 13:46

Removed http channel

ab693c6

Signed-off-by: Peter Nied <petern@amazon.com>

Add request source from the netty channel

6abd939

Signed-off-by: Peter Nied <petern@amazon.com>

Plumb SecurityRequest into HTTPAuthenticator interface

da8d0cb

Signed-off-by: Peter Nied <petern@amazon.com>

Fix all tests

26c6b83

Signed-off-by: Peter Nied <petern@amazon.com>

Prevent build break for generalized testing associated with SecurityR…

a825d83

…equest Signed-off-by: Peter Nied <petern@amazon.com>

Spotless

095d4e5

Signed-off-by: Peter Nied <petern@amazon.com>

Disable createHeaderVerifier so builds can pass

6580e7f

Signed-off-by: Peter Nied <petern@amazon.com>

Implement SecurityRequest gaps

8c44278

Signed-off-by: Peter Nied <petern@amazon.com>

Fix spotless

9b5fe42

Signed-off-by: Peter Nied <petern@amazon.com>

Redo how reRequestAuthentication works

9f369cb

Signed-off-by: Peter Nied <petern@amazon.com>

Nix that idea

46358a2

Signed-off-by: Peter Nied <petern@amazon.com>

Cleaning up some

fa98ba9

Signed-off-by: Peter Nied <petern@amazon.com>

This was referenced Oct 2, 2023

Add header verification netty channel handler opensearch-project/OpenSearch#10290

Closed

[Feature] Improve compressed request handling opensearch-project/OpenSearch#10260

Closed

peternied added 15 commits October 2, 2023 15:31

Mild switch to RequestChannel

9844df9

Signed-off-by: Peter Nied <petern@amazon.com>

Handle unsupported scenarios

cafe1c7

Signed-off-by: Peter Nied <petern@amazon.com>

Tests compile again

2beb434

Signed-off-by: Peter Nied <petern@amazon.com>

Fix spotless issues

2ef4c3f

Signed-off-by: Peter Nied <petern@amazon.com>

Merge remote-tracking branch 'origin/main' into headerverifier

4970a49

Signed-off-by: Peter Nied <petern@amazon.com>

Fix merging from main changes

87646d7

Signed-off-by: Peter Nied <petern@amazon.com>

Restore default build.gradle settings

547218b

Signed-off-by: Peter Nied <petern@amazon.com>

Fix bug in authn/z stages of handler wrapper

d44d277

Signed-off-by: Peter Nied <petern@amazon.com>

Focus channel implementation

b960aac

Signed-off-by: Peter Nied <petern@amazon.com>

Merge remote-tracking branch 'origin/main' into headerverifier

4d9506c

Signed-off-by: Peter Nied <petern@amazon.com>

Fix merge conflict related issues

3111665

Signed-off-by: Peter Nied <petern@amazon.com>

Seperate read-only vs read-write scenarios

2aad250

Signed-off-by: Peter Nied <petern@amazon.com>

Spotless

9a915e7

Signed-off-by: Peter Nied <petern@amazon.com>

more cleanup

e749a8e

Signed-off-by: Peter Nied <petern@amazon.com>

Rename concrete classes

20c9ea7

Signed-off-by: Peter Nied <petern@amazon.com>

peternied added 3 commits October 5, 2023 21:33

Merge remote-tracking branch 'origin/main' into headerverifier

7940233

Signed-off-by: Peter Nied <petern@amazon.com>

switch from complete to queue requests

8701359

Signed-off-by: Peter Nied <petern@amazon.com>

Fix checkstyle issue

e7b0916

Signed-off-by: Peter Nied <petern@amazon.com>

peternied added backport 2.x backport 2.11 Backport to 2.11 branch labels Oct 5, 2023

RyanL1997 reviewed Oct 5, 2023

View reviewed changes

RyanL1997 approved these changes Oct 6, 2023

View reviewed changes

cwperks reviewed Oct 6, 2023

View reviewed changes

src/main/java/com/amazon/dlic/auth/http/jwt/AbstractHTTPJwtAuthenticator.java Show resolved Hide resolved

cwperks reviewed Oct 6, 2023

View reviewed changes

src/main/java/org/opensearch/security/filter/SecurityRequestChannel.java Show resolved Hide resolved

cwperks approved these changes Oct 6, 2023

View reviewed changes

peternied mentioned this pull request Oct 6, 2023

[Refactor] Remove the notion of SecurityRequestChannel #3476

Open

3 tasks

peternied merged commit f435c05 into opensearch-project:main Oct 6, 2023

opensearch-trigger-bot bot added the backport-failed label Oct 6, 2023

This was referenced Oct 6, 2023

[Backport 2.x] Expanding Authentication with SecurityRequest Abstraction (#3430) #3487

Merged

[Backport 2.11] Expanding Authentication with SecurityRequest Abstraction (#3430) #3488

Merged

peternied removed the backport-failed label Oct 6, 2023

RyanL1997 mentioned this pull request Oct 6, 2023

Add early rejection from RestHandler for unauthorized requests #3418

Merged

3 tasks

peternied deleted the headerverifier branch October 17, 2023 09:38

peternied mentioned this pull request Oct 17, 2023

[Backport 1.3] Add early rejection from RestHandler for unauthorized requests #3559

Closed

12 tasks

peternied mentioned this pull request Oct 26, 2023

SecurityResponse only accept single header values #3608

Closed

5 tasks

peternied mentioned this pull request Nov 8, 2023

[Backport 1.3] Expanding Authentication with SecurityRequest Abstraction (#3487) #3670

Merged

3 tasks

cwperks mentioned this pull request Nov 15, 2023

[BUG] Response content-type changed in 2.11 for some endpoints #3718

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expanding Authentication with SecurityRequest Abstraction#3430

Expanding Authentication with SecurityRequest Abstraction#3430
peternied merged 53 commits intoopensearch-project:mainfrom
peternied:headerverifier

peternied commented Oct 1, 2023 •

edited

Loading

Uh oh!

codecov bot commented Oct 1, 2023 •

edited

Loading

Uh oh!

RyanL1997 left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 6, 2023

Uh oh!

opensearch-trigger-bot bot commented Oct 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

peternied commented Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

To Do items:

Issues Resolved

Check List

Uh oh!

codecov bot commented Oct 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

RyanL1997 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

opensearch-trigger-bot bot commented Oct 6, 2023

Uh oh!

opensearch-trigger-bot bot commented Oct 6, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

peternied commented Oct 1, 2023 •

edited

Loading

codecov bot commented Oct 1, 2023 •

edited

Loading