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

[Backport 2.x] Authorize rest requests (#2753) and Enhance RestLayerPrivilegesEvaluator Code Coverage (#3218) #2956

Conversation

DarshitChanpura
Copy link
Member

@DarshitChanpura DarshitChanpura commented Jul 7, 2023

Description

Manually Backports #2753

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.

* WIP on rest layer authz

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* WIP on rest-layer authz

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Extension handshake

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Extension TLS

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove SecurityRestFilterChanges to isolate extension TLS change

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* WIP for HelloWorld sample extension role

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Initial implementation of authz check in REST layer

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove header

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Create authorizeRequest method

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* small fix

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Change to ProtectedRoute

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Remove extension permissions

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Initial implementation of authz check in REST layer

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Extension TLS

Signed-off-by: Craig Perkins <cwperx@amazon.com>

* Adds dummy roles for testing rest authorization against legacy permissions

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds support for legacy permissions to perform rest authorization

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes white-space changes

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Rebases ConfigConstants with main

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Implements a new logic for rest permissions check to be more flexible

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes spotless errors

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds regex to match against current role permissions when comparing new permission with legacy ones

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Moves legacy permission check logic to ConfigModelV7

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes extra new-lines

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes unused imports

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes out-of-scope white space changes

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes code-ql errors

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes spotless and code-ql errors

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes variable name and remove references to whitelist in javadoc

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds tests for rest layer privilege evaluator

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds license header to the test file

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Updates zstd dependency to fetch from core version.properties

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Updates action name in the regex to be dynamic

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds support for allowing evaluation against multiple actions names for a registered named route

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Updates tests

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds null check

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Makes authorize logic clearer to follow

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds extra check to ensure new actions are also evaluated against transport actions

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes spotless errors

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes security rest filter setup

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Removes extension reference

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* turn on audit logging

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>

* Adds unit tests for restPathMatches method

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Cleans up TODOs

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Organizes demo users and roles for extension

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Address PR feedback

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds more comments

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* add privileges info

Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>

* Makes whoami action a named route and fixes license header check

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds integ tests for whoami route

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Change permissions order in roles.yml

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds developer documentation for authorization in REST layer

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes broken tests

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes checkstyle errors

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Addresses feedback and cleans up logic for super admin check

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Addresses Plugin Install CI failure

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Fixes failing citest task

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Modifies WhoAmI integ tests

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Adds a new endpoint called whoamiprotected and removes changes made to whoami route

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Updates documentation to reflect the new API

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Addresses PR feedback

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

* Renames action0 to actions

Signed-off-by: Darshit Chanpura <dchanp@amazon.com>

---------

Signed-off-by: Craig Perkins <cwperx@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Signed-off-by: Maciej Mierzwa <dev.maciej.mierzwa@gmail.com>
Co-authored-by: Craig Perkins <cwperx@amazon.com>
Co-authored-by: MaciejMierzwa <dev.maciej.mierzwa@gmail.com>
(cherry picked from commit a53a8a6)
@DarshitChanpura
Copy link
Member Author

Waiting for security reviews to be complete before merging.

Copy link
Member

@peternied peternied left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Putting the hold on this feature with a request changes requirement - dismissed with the internal reviews have completed

@DarshitChanpura
Copy link
Member Author

White source check will be unblocked once #2974 is merged.

@codecov
Copy link

codecov bot commented Jul 10, 2023

Codecov Report

Merging #2956 (c459df5) into 2.x (19c6a9d) will increase coverage by 0.02%.
The diff coverage is 74.50%.

Impacted file tree graph

@@             Coverage Diff              @@
##                2.x    #2956      +/-   ##
============================================
+ Coverage     64.32%   64.34%   +0.02%     
- Complexity     3462     3490      +28     
============================================
  Files           256      257       +1     
  Lines         19515    19606      +91     
  Branches       3297     3312      +15     
============================================
+ Hits          12553    12616      +63     
- Misses         5350     5373      +23     
- Partials       1612     1617       +5     
Files Coverage Δ
.../opensearch/security/OpenSearchSecurityPlugin.java 84.53% <100.00%> (+0.07%) ⬆️
...earch/security/auditlog/impl/AbstractAuditLog.java 76.48% <100.00%> (+0.05%) ⬆️
...g/opensearch/security/dlic/rest/support/Utils.java 60.97% <100.00%> (+3.64%) ⬆️
...opensearch/security/rest/SecurityWhoAmIAction.java 81.39% <100.00%> (+2.44%) ⬆️
...urity/privileges/RestLayerPrivilegesEvaluator.java 94.11% <94.11%> (ø)
...opensearch/security/filter/SecurityRestFilter.java 63.80% <50.00%> (-14.32%) ⬇️

... and 3 files with indirect coverage changes

@stephen-crawford stephen-crawford removed the v2.9.0 v2.9.0 label Jul 11, 2023
@stephen-crawford
Copy link
Contributor

Removing 2.9 label

@davidlago davidlago added the v2.10.0 Issues targeting release v2.10.0 label Jul 17, 2023
DarshitChanpura and others added 3 commits August 24, 2023 11:49
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
…t#3218)

Enhance RestLayerPrivilegesEvaluator Code Coverage

Signed-off-by: Stephen Crawford <steecraw@amazon.com>
Signed-off-by: Peter Nied <petern@amazon.com>
Signed-off-by: Darshit Chanpura <dchanp@amazon.com>
Co-authored-by: Peter Nied <petern@amazon.com>
Co-authored-by: Darshit Chanpura <dchanp@amazon.com>
(cherry picked from commit 46dfd84)
@DarshitChanpura DarshitChanpura changed the title [Backport 2.x] Authorize rest requests (#2753) [Backport 2.x] Authorize rest requests (#2753) and Enhance RestLayerPrivilegesEvaluator Code Coverage (#3218) Aug 24, 2023
@peternied peternied removed the v2.10.0 Issues targeting release v2.10.0 label Aug 31, 2023
@peternied
Copy link
Member

Based on where the security review for this feature is, it isn't going to make code freeze for 2.10, removing the tag.

@DarshitChanpura DarshitChanpura added the v2.11.0 Issues targeting the 2.11 release label Sep 29, 2023
@DarshitChanpura
Copy link
Member Author

We are targeting 2.11 for this feature and so tagging as such.

@DarshitChanpura DarshitChanpura self-assigned this Sep 29, 2023
@DarshitChanpura DarshitChanpura dismissed peternied’s stale review October 4, 2023 16:41

This change is ready for 2.11 as internal security testing is complete

@DarshitChanpura DarshitChanpura merged commit 59e0e82 into opensearch-project:2.x Oct 4, 2023
56 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v2.11.0 Issues targeting the 2.11 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants