Skip to content

Commit

Permalink
Merge branch 'main' into identity-aware-plugin-request-perms
Browse files Browse the repository at this point in the history
  • Loading branch information
cwperks committed Sep 20, 2024
2 parents 394c47a + f0ea056 commit fde386d
Show file tree
Hide file tree
Showing 60 changed files with 1,698 additions and 385 deletions.
10 changes: 5 additions & 5 deletions .github/benchmark-configs.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@
"SINGLE_NODE_CLUSTER": "true",
"MIN_DISTRIBUTION": "true",
"TEST_WORKLOAD": "big5",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard\"}",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard_ordered\"}",
"CAPTURE_NODE_STAT": "true",
"TEST_PROCEDURE": "restore-from-snapshot"
},
Expand Down Expand Up @@ -126,7 +126,7 @@
"SINGLE_NODE_CLUSTER": "true",
"MIN_DISTRIBUTION": "true",
"TEST_WORKLOAD": "big5",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots\",\"snapshot_name\":\"big5_1_shard\"}",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots\",\"snapshot_name\":\"big5_1_shard_ordered\"}",
"CAPTURE_NODE_STAT": "true",
"TEST_PROCEDURE": "restore-from-snapshot"
},
Expand Down Expand Up @@ -176,7 +176,7 @@
"MIN_DISTRIBUTION": "true",
"TEST_WORKLOAD": "big5",
"ADDITIONAL_CONFIG": "search.concurrent_segment_search.enabled:true",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard\"}",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard_ordered\"}",
"CAPTURE_NODE_STAT": "true",
"TEST_PROCEDURE": "restore-from-snapshot"
},
Expand All @@ -194,7 +194,7 @@
"MIN_DISTRIBUTION": "true",
"TEST_WORKLOAD": "big5",
"ADDITIONAL_CONFIG": "search.concurrent_segment_search.mode:all",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard\"}",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard_ordered\"}",
"CAPTURE_NODE_STAT": "true",
"TEST_PROCEDURE": "restore-from-snapshot"
},
Expand All @@ -212,7 +212,7 @@
"MIN_DISTRIBUTION": "true",
"TEST_WORKLOAD": "big5",
"ADDITIONAL_CONFIG": "search.concurrent_segment_search.mode:auto",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard\"}",
"WORKLOAD_PARAMS": "{\"snapshot_repo_name\":\"benchmark-workloads-repo-300\",\"snapshot_bucket_name\":\"benchmark-workload-snapshots\",\"snapshot_region\":\"us-east-1\",\"snapshot_base_path\":\"workload-snapshots-300\",\"snapshot_name\":\"big5_1_shard_ordered\"}",
"CAPTURE_NODE_STAT": "true",
"TEST_PROCEDURE": "restore-from-snapshot"
},
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/delete_backport_branch.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
contents: write
if: startsWith(github.event.pull_request.head.ref,'backport/')
steps:
- name: Delete merged branch
- name: Delete merged branch
uses: actions/github-script@v5
with:
script: |
Expand Down
4 changes: 4 additions & 0 deletions .github/workflows/gradle-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ on:
pull_request_target:
types: [opened, synchronize, reopened]

concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number }}
cancel-in-progress: true

permissions:
contents: read # to fetch code (actions/checkout)

Expand Down
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),

## [Unreleased 2.x]
### Added
- Add support for async deletion in S3BlobContainer ([#15621](https:/opensearch-project/OpenSearch/pull/15621))
- MultiTermQueries in keyword fields now default to `indexed` approach and gated behind cluster setting ([#15637](https:/opensearch-project/OpenSearch/pull/15637))
- [Workload Management] QueryGroup resource cancellation framework changes ([#15651](https:/opensearch-project/OpenSearch/pull/15651))
- Fallback to Remote cluster-state on Term-Version check mismatch - ([#15424](https:/opensearch-project/OpenSearch/pull/15424))
- Implement WithFieldName interface in ValuesSourceAggregationBuilder & FieldSortBuilder ([#15916](https:/opensearch-project/OpenSearch/pull/15916))
- Add successfulSearchShardIndices in searchRequestContext ([#15967](https:/opensearch-project/OpenSearch/pull/15967))
- Remove identity-related feature flagged code from the RestController ([#15430](https:/opensearch-project/OpenSearch/pull/15430))

### Dependencies
- Bump `com.azure:azure-identity` from 1.13.0 to 1.13.2 ([#15578](https:/opensearch-project/OpenSearch/pull/15578))
Expand All @@ -18,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Bump `com.nimbusds:oauth2-oidc-sdk` from 11.9.1 to 11.19.1 ([#15862](https:/opensearch-project/OpenSearch/pull/15862))
- Bump `com.microsoft.azure:msal4j` from 1.17.0 to 1.17.1 ([#15945](https:/opensearch-project/OpenSearch/pull/15945))
- Bump `ch.qos.logback:logback-core` from 1.5.6 to 1.5.8 ([#15946](https:/opensearch-project/OpenSearch/pull/15946))
- Update protobuf from 3.25.4 to 3.25.5 ([#16011](https:/opensearch-project/OpenSearch/pull/16011))

### Changed

Expand All @@ -30,6 +34,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Fix wildcard query containing escaped character ([#15737](https:/opensearch-project/OpenSearch/pull/15737))
- Fix case-insensitive query on wildcard field ([#15882](https:/opensearch-project/OpenSearch/pull/15882))
- Add validation for the search backpressure cancellation settings ([#15501](https:/opensearch-project/OpenSearch/pull/15501))
- Fix infinite loop in nested agg ([#15931](https:/opensearch-project/OpenSearch/pull/15931))

### Security

Expand Down
2 changes: 1 addition & 1 deletion buildSrc/version.properties
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ woodstox = 6.4.0
kotlin = 1.7.10
antlr4 = 4.13.1
guava = 32.1.1-jre
protobuf = 3.25.4
protobuf = 3.25.5
jakarta_annotation = 1.3.5
google_http_client = 1.44.1
tdigest = 3.3
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,36 +13,42 @@
import org.apache.shiro.SecurityUtils;
import org.apache.shiro.mgt.SecurityManager;
import org.opensearch.client.Client;
import org.opensearch.client.node.NodeClient;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.cluster.service.ClusterService;
import org.opensearch.common.annotation.ExperimentalApi;
import org.opensearch.common.settings.Settings;
import org.opensearch.common.util.concurrent.ThreadContext;
import org.opensearch.core.common.io.stream.NamedWriteableRegistry;
import org.opensearch.core.rest.RestStatus;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.env.Environment;
import org.opensearch.env.NodeEnvironment;
import org.opensearch.identity.PluginSubject;
import org.opensearch.identity.Subject;
import org.opensearch.identity.tokens.AuthToken;
import org.opensearch.identity.tokens.TokenManager;
import org.opensearch.plugins.ActionPlugin;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.plugins.Plugin;
import org.opensearch.plugins.PluginInfo;
import org.opensearch.repositories.RepositoriesService;
import org.opensearch.rest.BytesRestResponse;
import org.opensearch.rest.RestChannel;
import org.opensearch.rest.RestHandler;
import org.opensearch.rest.RestRequest;
import org.opensearch.script.ScriptService;
import org.opensearch.threadpool.ThreadPool;
import org.opensearch.watcher.ResourceWatcherService;

import java.util.Collection;
import java.util.Collections;
import java.util.function.Supplier;
import java.util.function.UnaryOperator;

/**
* Identity implementation with Shiro
*
* @opensearch.experimental
*/
@ExperimentalApi
public final class ShiroIdentityPlugin extends Plugin implements IdentityPlugin {
public final class ShiroIdentityPlugin extends Plugin implements IdentityPlugin, ActionPlugin {
private Logger log = LogManager.getLogger(this.getClass());

private final Settings settings;
Expand Down Expand Up @@ -101,6 +107,38 @@ public TokenManager getTokenManager() {
return this.authTokenHandler;
}

@Override
public UnaryOperator<RestHandler> getRestHandlerWrapper(ThreadContext threadContext) {
return AuthcRestHandler::new;
}

class AuthcRestHandler extends RestHandler.Wrapper {

public AuthcRestHandler(RestHandler original) {
super(original);
}

@Override
public void handleRequest(RestRequest request, RestChannel channel, NodeClient client) throws Exception {
try {
final AuthToken token = ShiroTokenExtractor.extractToken(request);
// If no token was found, continue executing the request
if (token == null) {
// Authentication did not fail so return true. Authorization is handled at the action level.
super.handleRequest(request, channel, client);
return;
}
ShiroSubject shiroSubject = (ShiroSubject) getCurrentSubject();
shiroSubject.authenticate(token);
// Caller was authorized, forward the request to the handler
super.handleRequest(request, channel, client);
} catch (final Exception e) {
final BytesRestResponse bytesRestResponse = new BytesRestResponse(RestStatus.UNAUTHORIZED, e.getMessage());
channel.sendResponse(bytesRestResponse);
}
}
}

@Override
public PluginSubject getPluginSubject(PluginInfo pluginInfo) {
return new ShiroPluginSubject(threadPool);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@

/**
* OpenSearch specific security manager implementation
*
* @opensearch.experimental
*/
public class ShiroSecurityManager extends DefaultSecurityManager {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@

/**
* Subject backed by Shiro
*
* @opensearch.experimental
*/
public class ShiroSubject implements UserSubject {
private final ShiroTokenManager authTokenHandler;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*/
package org.opensearch.identity.tokens;
package org.opensearch.identity.shiro;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.opensearch.core.common.Strings;
import org.opensearch.identity.tokens.AuthToken;
import org.opensearch.identity.tokens.BasicAuthToken;
import org.opensearch.rest.RestRequest;

import java.util.Collections;
Expand All @@ -18,9 +20,9 @@
/**
* Extracts tokens from RestRequests used for authentication
*/
public class RestTokenExtractor {
public class ShiroTokenExtractor {

private static final Logger logger = LogManager.getLogger(RestTokenExtractor.class);
private static final Logger logger = LogManager.getLogger(ShiroTokenExtractor.class);

public final static String AUTH_HEADER_NAME = "Authorization";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@

/**
* Extracts Shiro's {@link AuthenticationToken} from different types of auth headers
*
* @opensearch.experimental
*/
class ShiroTokenManager implements TokenManager {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@

/**
* Password matcher for BCrypt
*
* @opensearch.experimental
*/
public class BCryptPasswordMatcher implements CredentialsMatcher {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,6 @@

/**
* Internal Realm is a custom realm using the internal OpenSearch IdP
*
* @opensearch.experimental
*/
public class OpenSearchRealm extends AuthenticatingRealm {
private static final String DEFAULT_REALM_NAME = "internal";
Expand Down Expand Up @@ -93,7 +91,7 @@ public OpenSearchRealm build() {
public User getInternalUser(final String principalIdentifier) throws UnknownAccountException {
final User userRecord = internalUsers.get(principalIdentifier);
if (userRecord == null) {
throw new UnknownAccountException();
throw new UnknownAccountException("Incorrect credentials");
}
return userRecord;
}
Expand Down Expand Up @@ -131,7 +129,7 @@ protected AuthenticationInfo doGetAuthenticationInfo(final AuthenticationToken t
return sai;
} else {
// Bad password
throw new IncorrectCredentialsException();
throw new IncorrectCredentialsException("Incorrect credentials");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@

/**
* A non-volatile and immutable object in the storage.
*
* @opensearch.experimental
*/
public class User {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,35 +13,35 @@
import org.opensearch.identity.IdentityService;
import org.opensearch.plugins.IdentityPlugin;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.threadpool.TestThreadPool;
import org.opensearch.threadpool.ThreadPool;

import java.util.List;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
import static org.junit.Assert.assertThrows;
import static org.mockito.Mockito.mock;

public class ShiroIdentityPluginTests extends OpenSearchTestCase {

public void testSingleIdentityPluginSucceeds() {
TestThreadPool threadPool = new TestThreadPool(getTestName());
IdentityPlugin identityPlugin1 = new ShiroIdentityPlugin(Settings.EMPTY);
List<IdentityPlugin> pluginList1 = List.of(identityPlugin1);
IdentityService identityService1 = new IdentityService(Settings.EMPTY, threadPool, pluginList1);
IdentityService identityService1 = new IdentityService(Settings.EMPTY, mock(ThreadPool.class), pluginList1);
assertThat(identityService1.getTokenManager(), is(instanceOf(ShiroTokenManager.class)));
terminate(threadPool);
}

public void testMultipleIdentityPluginsFail() {
TestThreadPool threadPool = new TestThreadPool(getTestName());
IdentityPlugin identityPlugin1 = new ShiroIdentityPlugin(Settings.EMPTY);
IdentityPlugin identityPlugin2 = new ShiroIdentityPlugin(Settings.EMPTY);
IdentityPlugin identityPlugin3 = new ShiroIdentityPlugin(Settings.EMPTY);
List<IdentityPlugin> pluginList = List.of(identityPlugin1, identityPlugin2, identityPlugin3);
Exception ex = assertThrows(OpenSearchException.class, () -> new IdentityService(Settings.EMPTY, threadPool, pluginList));
Exception ex = assertThrows(
OpenSearchException.class,
() -> new IdentityService(Settings.EMPTY, mock(ThreadPool.class), pluginList)
);
assert (ex.getMessage().contains("Multiple identity plugins are not supported,"));
terminate(threadPool);
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* 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.
*/

package org.opensearch.identity.shiro;

import org.opensearch.identity.tokens.AuthToken;
import org.opensearch.identity.tokens.BasicAuthToken;
import org.opensearch.rest.RestRequest;
import org.opensearch.test.OpenSearchTestCase;
import org.opensearch.test.rest.FakeRestRequest;

import java.nio.charset.StandardCharsets;
import java.util.Base64;
import java.util.List;
import java.util.Map;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;

public class ShiroTokenExtractorTests extends OpenSearchTestCase {

public void testAuthorizationHeaderExtractionWithBasicAuthToken() {
String basicAuthHeader = Base64.getEncoder().encodeToString("foo:bar".getBytes(StandardCharsets.UTF_8));
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(
Map.of(ShiroTokenExtractor.AUTH_HEADER_NAME, List.of(BasicAuthToken.TOKEN_IDENTIFIER + " " + basicAuthHeader))
).build();
AuthToken extractedToken = ShiroTokenExtractor.extractToken(fakeRequest);
assertThat(extractedToken, instanceOf(BasicAuthToken.class));
assertThat(extractedToken.asAuthHeaderValue(), equalTo(basicAuthHeader));
}

public void testAuthorizationHeaderExtractionWithUnknownToken() {
String authHeader = "foo";
RestRequest fakeRequest = new FakeRestRequest.Builder(xContentRegistry()).withHeaders(
Map.of(ShiroTokenExtractor.AUTH_HEADER_NAME, List.of(authHeader))
).build();
AuthToken extractedToken = ShiroTokenExtractor.extractToken(fakeRequest);
assertNull(extractedToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ protected Settings nodeSettings(int nodeOrdinal) {
// Disable request throttling because some random values in tests might generate too many failures for the S3 client
.put(S3ClientSettings.USE_THROTTLE_RETRIES_SETTING.getConcreteSettingForNamespace("test").getKey(), false)
.put(S3ClientSettings.PROXY_TYPE_SETTING.getConcreteSettingForNamespace("test").getKey(), ProxySettings.ProxyType.DIRECT)
.put(BlobStoreRepository.SNAPSHOT_ASYNC_DELETION_ENABLE_SETTING.getKey(), false)
.put(super.nodeSettings(nodeOrdinal))
.setSecureSettings(secureSettings);

Expand Down
Loading

0 comments on commit fde386d

Please sign in to comment.