-
Notifications
You must be signed in to change notification settings - Fork 349
Authorize rest requests #2753
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
Merged
DarshitChanpura
merged 71 commits into
opensearch-project:main
from
DarshitChanpura:authorize-rest-requests
Jul 7, 2023
Merged
Authorize rest requests #2753
Changes from all commits
Commits
Show all changes
71 commits
Select commit
Hold shift + click to select a range
a8a1660
WIP on rest layer authz
cwperks 0fedc03
WIP on rest-layer authz
cwperks 1793a5b
Extension handshake
cwperks 32d8763
Extension TLS
cwperks a5a44b8
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks 99bfd94
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks f62f590
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks 7596b7e
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks 1fb5316
Remove SecurityRestFilterChanges to isolate extension TLS change
cwperks 25fda77
WIP for HelloWorld sample extension role
cwperks aea8ede
Initial implementation of authz check in REST layer
cwperks 15eb6b9
Remove header
cwperks f2cba87
Create authorizeRequest method
cwperks 77700e5
small fix
cwperks d1d9ed1
Change to ProtectedRoute
cwperks ccab6ce
Remove extension permissions
cwperks 174a142
Initial implementation of authz check in REST layer
cwperks 2042a60
Extension TLS
cwperks 0c70700
Adds dummy roles for testing rest authorization against legacy permis…
DarshitChanpura 9837378
Adds support for legacy permissions to perform rest authorization
DarshitChanpura a34c3a4
Fixes white-space changes
DarshitChanpura f5afd62
Rebases ConfigConstants with main
DarshitChanpura 918ab51
Implements a new logic for rest permissions check to be more flexible
DarshitChanpura 98791a2
Fixes spotless errors
DarshitChanpura b639544
Adds regex to match against current role permissions when comparing n…
DarshitChanpura 4288235
Moves legacy permission check logic to ConfigModelV7
DarshitChanpura 93a2c66
Fixes extra new-lines
DarshitChanpura 6e9e83d
Fixes unused imports
DarshitChanpura 5ee7b12
Fixes out-of-scope white space changes
DarshitChanpura 34092f9
Fixes code-ql errors
DarshitChanpura c41023b
Fixes spotless and code-ql errors
DarshitChanpura d746435
Fixes variable name and remove references to whitelist in javadoc
DarshitChanpura d68c76e
Adds tests for rest layer privilege evaluator
DarshitChanpura 2814283
Adds license header to the test file
DarshitChanpura 3b41475
Merge branch 'main' into authorize-rest-requests
DarshitChanpura d5392e4
Updates zstd dependency to fetch from core version.properties
DarshitChanpura 425c22b
Updates action name in the regex to be dynamic
DarshitChanpura b1ed481
Adds support for allowing evaluation against multiple actions names f…
DarshitChanpura 1e3efe2
Updates tests
DarshitChanpura 590f55a
Adds null check
DarshitChanpura 6918a80
Merge branch 'main' into authorize-rest-requests
DarshitChanpura 427460c
Makes authorize logic clearer to follow
DarshitChanpura f3c4a77
Adds extra check to ensure new actions are also evaluated against tra…
DarshitChanpura 8557fce
Fixes spotless errors
DarshitChanpura d66294d
Fixes security rest filter setup
DarshitChanpura 4a33b33
Removes extension reference
DarshitChanpura 6a1c25a
Merge branch 'main' into authorize-rest-requests
DarshitChanpura 8503358
turn on audit logging
MaciejMierzwa afc8c81
Adds unit tests for restPathMatches method
DarshitChanpura 14fe152
Cleans up TODOs
DarshitChanpura 5bae5e8
Organizes demo users and roles for extension
DarshitChanpura 24be564
Address PR feedback
DarshitChanpura 2edd319
Adds more comments
DarshitChanpura 3c397e2
add privileges info
MaciejMierzwa e7d10c7
Audit Logging for NamedRoutes
DarshitChanpura 54cfcd9
Merge remote-tracking branch 'upstream/main' into authorize-rest-requ…
DarshitChanpura 8bfeb25
Makes whoami action a named route and fixes license header check
DarshitChanpura ccd3d2d
Adds integ tests for whoami route
DarshitChanpura c68083c
Merge branch 'main' into authorize-rest-requests
DarshitChanpura dc0c1e2
Change permissions order in roles.yml
DarshitChanpura 39314d7
Adds developer documentation for authorization in REST layer
DarshitChanpura ebc5486
Fixes broken tests
DarshitChanpura a78582f
Fixes checkstyle errors
DarshitChanpura a325d8e
Addresses feedback and cleans up logic for super admin check
DarshitChanpura 0e905a0
Addresses Plugin Install CI failure
DarshitChanpura 25a41b6
Fixes failing citest task
DarshitChanpura 1b1f519
Modifies WhoAmI integ tests
DarshitChanpura ffdd927
Adds a new endpoint called whoamiprotected and removes changes made t…
DarshitChanpura 9b05d44
Updates documentation to reflect the new API
DarshitChanpura a21c4c0
Addresses PR feedback
DarshitChanpura 6c5ee8e
Renames action0 to actions
DarshitChanpura File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| # Authorization at REST Layer for plugins | ||
|
|
||
| This feature is introduced as an added layer of security on top of existing TransportLayer authorization framework. In order to leverage these feature some core changes need to be made at Route registration level. This document talks about how you can achieve this. | ||
|
|
||
| **NOTE:** This doesn't replace Transport Layer Authorization. Plugin developers may choose to skip creating transport actions for APIs that do not need interaction with the Transport Layer. | ||
|
|
||
| ## Pre-requisites | ||
|
|
||
| The security plugin must be installed and operational in your OpenSearch cluster for this feature to work. | ||
|
|
||
| ### How does NamedRoute authorization work? | ||
|
|
||
| Once the routes are defined as NamedRoute, they, along-with their handlers, will be registered the same way as Route objects. When a request comes in, `SecurityRestFilter.java` applies an authorization check which extracts information about the NamedRoute. | ||
| Next we get the unique name and actionNames associated with that route and evaluate these against existing `cluster_permissions` across all roles of the requesting user. If the authorization check succeeds, the request chain proceeds as normal. If it fails, a 401 response is returned to the user. | ||
|
|
||
| NOTE: | ||
| 1. The action names defined in roles must exactly match the names of registered routes, or else, the request would be deemed unauthorized. | ||
| 2. This check will not be implemented for plugins who do not use NamedRoutes. | ||
|
|
||
|
|
||
|
|
||
| ### How to translate an existing Route to be a NamedRoute? | ||
|
|
||
| Here is a sample of an existing route converted to a named route: | ||
| Before: | ||
| ``` | ||
| public List<Route> routes() { | ||
| return ImmutableList.of( | ||
| new Route(GET, "/uri") | ||
| ); | ||
| } | ||
| ``` | ||
| With new scheme: | ||
| ``` | ||
| public List<NamedRoute> routes() { | ||
| return ImmutableList.of( | ||
| new NamedRoute.Builder().method(GET).path("/uri").uniqueName("plugin:uri").actionNames(Set.of("cluster:admin/opensearch/plugin/uri")).build() | ||
| ); | ||
| } | ||
| ``` | ||
|
|
||
| `actionNames()` are optional. They correspond to any current actions defined as permissions in roles. | ||
| Ensure that these name-to-route mappings are easily accessible to the cluster admins to allow granting access to these APIs. | ||
|
|
||
| ### How does authorization in the REST Layer work? | ||
|
|
||
| We will continue on the above example of translating `/uri` from Route to NamedRoute. | ||
|
|
||
| Consider these roles are defined in the cluster: | ||
| ```yaml | ||
| plugin_role: | ||
| reserved: true | ||
| cluster_permissions: | ||
| - 'plugin:uri' | ||
|
|
||
| plugin_role_legacy: | ||
| reserved: true | ||
| cluster_permissions: | ||
| - 'cluster:admin/opensearch/plugin/uri' | ||
| ``` | ||
|
|
||
| Successful authz scenarios for a user: | ||
| 1. The user is mapped either to `plugin_role` OR `plugin_role_legacy`. | ||
| 2. The user is mapped to both of these roles. | ||
| 3. The user is mapped to `plugin_role` even if no `actionNames()` were registered for this route. | ||
|
|
||
| Unsuccessful authz scenarios for a user: | ||
| 1. The user is not mapped any roles. | ||
| 2. The user is mapped to a different role which doesn't grant the cluster permissions: `plugin:uri` OR `cluster:admin/opensearch/plugin/uri`/ | ||
| 3. The user is mapped to a role `plugin_role_other` which has a typo in action name, i.e.`plugin:uuri`. | ||
|
|
||
|
|
||
| ### Sample API in Security Plugin | ||
|
|
||
| As part of this effort a new uri `GET /whoamiprotected` was introduced as a NamedRoute version of `GET /whoami`. Here is how you can test it: | ||
|
|
||
| #### roles.yml | ||
| ```yaml | ||
| who_am_i_role: | ||
| reserved: true | ||
| cluster_permissions: | ||
| - 'security:whoamiprotected' | ||
|
|
||
| who_am_i_role_legacy: | ||
| reserved: true | ||
| cluster_permissions: | ||
| - 'cluster:admin/opendistro_security/whoamiprotected' | ||
|
|
||
| who_am_i_role_no_perm: | ||
| reserved: true | ||
| cluster_permissions: | ||
| - 'some_invalid_perm' | ||
|
|
||
| ``` | ||
|
|
||
| #### internal_users.yml | ||
| ```yaml | ||
| who_am_i-user: | ||
DarshitChanpura marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG" #admin | ||
| reserved: true | ||
| description: "Demo user for ext-test" | ||
|
|
||
| who_am_i_legacy-user: | ||
| hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG" | ||
| reserved: true | ||
| description: "Demo user for ext-test" | ||
|
|
||
| who_am_i_no_perm-user: | ||
| hash: "$2a$12$VcCDgh2NDk07JGN0rjGbM.Ad41qVR/YFJcgHp0UGns5JDymv..TOG" | ||
| reserved: true | ||
| description: "Demo user for ext-test" | ||
| ``` | ||
|
|
||
| #### roles_mapping.yml | ||
| ```yaml | ||
| who_am_i_role: | ||
| reserved: true | ||
| users: | ||
| - "who_am_i-user" | ||
|
|
||
| who_am_i_role_legacy: | ||
| reserved: true | ||
| users: | ||
| - "who_am_i_legacy-user" | ||
|
|
||
| who_am_i_role_no_perm: | ||
| reserved: true | ||
| users: | ||
| - "who_am_i_no_perm-user" | ||
| ``` | ||
|
|
||
| Follow [DEVELOPER_GUIDE](DEVELOPER_GUIDE.md) to setup OpenSearch cluster and initialize security plugin. Once you have verified that security plugin is installed correctly and OpenSearch is running, execute following curl requests: | ||
| 1. `curl -XGET https://who_am_i-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should succeed. | ||
| 2. `curl -XGET https://who_am_i_legacy-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should succeed. | ||
| 3. `curl -XGET https://who_am_i_no-perm-user:admin@localhost:9200/_plugins/_security/whoami --insecure` should fail. | ||
DarshitChanpura marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| 4. `curl -XPOST ` to `/whoami` with all 3 users should succeed. This is because POST route is not a NamedRoute and hence no authorization check was made. | ||
107 changes: 107 additions & 0 deletions
107
src/integrationTest/java/org/opensearch/security/rest/WhoAmITests.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,107 @@ | ||
| /* | ||
| * SPDX-License-Identifier: Apache-2.0 | ||
| * | ||
| * The OpenSearch Contributors require contributions made to | ||
| * this file be licensed under the Apache-2.0 license or a | ||
| * compatible open source license. | ||
| * | ||
| * Modifications Copyright OpenSearch Contributors. See | ||
| * GitHub history for details. | ||
| */ | ||
|
|
||
| package org.opensearch.security.rest; | ||
|
|
||
| import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope; | ||
| import org.apache.hc.core5.http.HttpStatus; | ||
| import org.junit.ClassRule; | ||
| import org.junit.Test; | ||
| import org.junit.runner.RunWith; | ||
| import org.opensearch.test.framework.TestSecurityConfig; | ||
| import org.opensearch.test.framework.TestSecurityConfig.Role; | ||
| import org.opensearch.test.framework.cluster.ClusterManager; | ||
| import org.opensearch.test.framework.cluster.LocalCluster; | ||
| import org.opensearch.test.framework.cluster.TestRestClient; | ||
|
|
||
| import static org.hamcrest.MatcherAssert.assertThat; | ||
| import static org.hamcrest.Matchers.equalTo; | ||
| import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL; | ||
|
|
||
| @RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class) | ||
| @ThreadLeakScope(ThreadLeakScope.Scope.NONE) | ||
| public class WhoAmITests { | ||
DarshitChanpura marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| protected final static TestSecurityConfig.User WHO_AM_I = new TestSecurityConfig.User("who_am_i_user").roles( | ||
| new Role("who_am_i_role").clusterPermissions("security:whoamiprotected") | ||
| ); | ||
|
|
||
| protected final static TestSecurityConfig.User WHO_AM_I_LEGACY = new TestSecurityConfig.User("who_am_i_user_legacy").roles( | ||
| new Role("who_am_i_role_legacy").clusterPermissions("cluster:admin/opendistro_security/whoamiprotected") | ||
| ); | ||
|
|
||
| protected final static TestSecurityConfig.User WHO_AM_I_NO_PERM = new TestSecurityConfig.User("who_am_i_user_no_perm").roles( | ||
| new Role("who_am_i_role_no_perm") | ||
| ); | ||
|
|
||
| protected final static TestSecurityConfig.User WHO_AM_I_UNREGISTERED = new TestSecurityConfig.User("who_am_i_user_no_perm"); | ||
|
|
||
| public static final String WHOAMI_ENDPOINT = "_plugins/_security/whoami"; | ||
| public static final String WHOAMI_PROTECTED_ENDPOINT = "_plugins/_security/whoamiprotected"; | ||
|
|
||
| @ClassRule | ||
| public static LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.THREE_CLUSTER_MANAGERS) | ||
| .authc(AUTHC_HTTPBASIC_INTERNAL) | ||
| .users(WHO_AM_I, WHO_AM_I_LEGACY, WHO_AM_I_NO_PERM) | ||
| .build(); | ||
|
|
||
| @Test | ||
| public void testWhoAmIWithGetPermissions() throws Exception { | ||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) { | ||
| assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) { | ||
| assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testWhoAmIWithGetPermissionsLegacy() throws Exception { | ||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) { | ||
| assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) { | ||
| assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testWhoAmIWithoutGetPermissions() throws Exception { | ||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) { | ||
| assertThat(client.get(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) { | ||
| assertThat(client.get(WHOAMI_PROTECTED_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_UNAUTHORIZED)); | ||
| } | ||
| } | ||
|
|
||
| @Test | ||
| public void testWhoAmIPost() throws Exception { | ||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I)) { | ||
| assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_LEGACY)) { | ||
| assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_NO_PERM)) { | ||
| assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| try (TestRestClient client = cluster.getRestClient(WHO_AM_I_UNREGISTERED)) { | ||
| assertThat(client.post(WHOAMI_ENDPOINT).getStatusCode(), equalTo(HttpStatus.SC_OK)); | ||
| } | ||
|
|
||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.