Skip to content

Commit a8b4ac1

Browse files
derek-hoderek-ho
authored
Subset of permissions check on creation (#5012)
Signed-off-by: Derek Ho <[email protected]>
1 parent 79f0c46 commit a8b4ac1

File tree

11 files changed

+263
-91
lines changed

11 files changed

+263
-91
lines changed

‎src/integrationTest/java/org/opensearch/security/privileges/ActionPrivilegesTest.java

+2-4
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import org.opensearch.core.common.unit.ByteSizeUnit;
4040
import org.opensearch.core.common.unit.ByteSizeValue;
4141
import org.opensearch.security.action.apitokens.ApiToken;
42-
import org.opensearch.security.action.apitokens.ApiTokenRepository;
4342
import org.opensearch.security.action.apitokens.Permissions;
4443
import org.opensearch.security.resolver.IndexResolverReplacer;
4544
import org.opensearch.security.securityconf.FlattenedActionGroups;
@@ -50,9 +49,8 @@
5049
import org.opensearch.security.user.User;
5150
import org.opensearch.security.util.MockIndexMetadataBuilder;
5251

53-
import org.mockito.Mockito;
54-
5552
import static org.hamcrest.MatcherAssert.assertThat;
53+
import static org.opensearch.security.privileges.ActionPrivilegesTest.IndexPrivileges.IndicesAndAliases.resolved;
5654
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isAllowed;
5755
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isForbidden;
5856
import static org.opensearch.security.privileges.PrivilegeEvaluatorResponseMatcher.isPartiallyOk;
@@ -342,6 +340,7 @@ public void apiToken_succeedsWithActionGroupsExapnded() throws Exception {
342340
"apitoken:" + token,
343341
new Permissions(List.of("CLUSTER_ALL"), List.of())
344342
);
343+
345344
// Explicit succeeds
346345
assertThat(subject.hasExplicitClusterPrivilege(context, "cluster:whatever"), isAllowed());
347346
// Not explicit succeeds
@@ -1152,7 +1151,6 @@ static RoleBasedPrivilegesEvaluationContext ctx(String... roles) {
11521151
static RoleBasedPrivilegesEvaluationContext ctxWithUserName(String userName, String... roles) {
11531152
User user = new User(userName);
11541153
user.addAttributes(ImmutableMap.of("attrs.dept_no", "a11"));
1155-
ApiTokenRepository mockRepository = Mockito.mock(ApiTokenRepository.class);
11561154
return new RoleBasedPrivilegesEvaluationContext(
11571155
user,
11581156
ImmutableSet.copyOf(roles),

‎src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

+25-7
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
259259
private volatile UserService userService;
260260
private volatile RestLayerPrivilegesEvaluator restLayerEvaluator;
261261
private volatile ConfigurationRepository cr;
262-
private volatile ApiTokenRepository ar;
262+
private volatile ApiTokenRepository apiTokenRepository;
263263
private volatile AdminDNs adminDns;
264264
private volatile ClusterService cs;
265265
private volatile AtomicReference<DiscoveryNode> localNode = new AtomicReference<>();
@@ -649,7 +649,21 @@ public List<RestHandler> getRestHandlers(
649649
)
650650
);
651651
handlers.add(new CreateOnBehalfOfTokenAction(tokenManager));
652-
handlers.add(new ApiTokenAction(ar));
652+
handlers.add(
653+
new ApiTokenAction(
654+
Objects.requireNonNull(threadPool),
655+
cr,
656+
evaluator,
657+
settings,
658+
adminDns,
659+
auditLog,
660+
configPath,
661+
principalExtractor,
662+
apiTokenRepository,
663+
cs,
664+
indexNameExpressionResolver
665+
)
666+
);
653667
handlers.addAll(
654668
SecurityRestApiActions.getHandler(
655669
settings,
@@ -1111,7 +1125,7 @@ public Collection<Object> createComponents(
11111125
final XFFResolver xffResolver = new XFFResolver(threadPool);
11121126
backendRegistry = new BackendRegistry(settings, adminDns, xffResolver, auditLog, threadPool);
11131127
tokenManager = new SecurityTokenManager(cs, threadPool, userService);
1114-
ar = new ApiTokenRepository(localClient, clusterService, tokenManager);
1128+
apiTokenRepository = new ApiTokenRepository(localClient, clusterService, tokenManager);
11151129

11161130
final CompatConfig compatConfig = new CompatConfig(environment, transportPassiveAuthSetting);
11171131

@@ -1128,7 +1142,7 @@ public Collection<Object> createComponents(
11281142
cih,
11291143
irr,
11301144
namedXContentRegistry.get(),
1131-
ar
1145+
apiTokenRepository
11321146
);
11331147

11341148
dlsFlsBaseContext = new DlsFlsBaseContext(evaluator, threadPool.getThreadContext(), adminDns);
@@ -1170,7 +1184,7 @@ public Collection<Object> createComponents(
11701184
configPath,
11711185
compatConfig
11721186
);
1173-
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher, ar);
1187+
dcf = new DynamicConfigFactory(cr, settings, configPath, localClient, threadPool, cih, passwordHasher, apiTokenRepository);
11741188
dcf.registerDCFListener(backendRegistry);
11751189
dcf.registerDCFListener(compatConfig);
11761190
dcf.registerDCFListener(irr);
@@ -1220,7 +1234,7 @@ public Collection<Object> createComponents(
12201234
components.add(dcf);
12211235
components.add(userService);
12221236
components.add(passwordHasher);
1223-
components.add(ar);
1237+
components.add(apiTokenRepository);
12241238

12251239
components.add(sslSettingsManager);
12261240
if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) {
@@ -2143,7 +2157,11 @@ public Collection<SystemIndexDescriptor> getSystemIndexDescriptors(Settings sett
21432157
ConfigConstants.OPENDISTRO_SECURITY_DEFAULT_CONFIG_INDEX
21442158
);
21452159
final SystemIndexDescriptor systemIndexDescriptor = new SystemIndexDescriptor(indexPattern, "Security index");
2146-
return Collections.singletonList(systemIndexDescriptor);
2160+
final SystemIndexDescriptor apiTokenSystemIndexDescriptor = new SystemIndexDescriptor(
2161+
ConfigConstants.OPENSEARCH_API_TOKENS_INDEX,
2162+
"Security API token index"
2163+
);
2164+
return List.of(systemIndexDescriptor, apiTokenSystemIndexDescriptor);
21472165
}
21482166

21492167
@Override

‎src/main/java/org/opensearch/security/action/apitokens/ApiTokenAction.java

+84-25
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
package org.opensearch.security.action.apitokens;
1313

1414
import java.io.IOException;
15+
import java.nio.file.Path;
1516
import java.time.Instant;
1617
import java.util.Collections;
1718
import java.util.List;
@@ -24,14 +25,29 @@
2425
import org.apache.logging.log4j.Logger;
2526

2627
import org.opensearch.client.node.NodeClient;
28+
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
29+
import org.opensearch.cluster.service.ClusterService;
30+
import org.opensearch.common.settings.Settings;
31+
import org.opensearch.common.util.concurrent.ThreadContext;
2732
import org.opensearch.core.action.ActionListener;
2833
import org.opensearch.core.rest.RestStatus;
2934
import org.opensearch.core.xcontent.XContentBuilder;
3035
import org.opensearch.rest.BaseRestHandler;
3136
import org.opensearch.rest.BytesRestResponse;
3237
import org.opensearch.rest.RestChannel;
33-
import org.opensearch.rest.RestHandler;
3438
import org.opensearch.rest.RestRequest;
39+
import org.opensearch.security.auditlog.AuditLog;
40+
import org.opensearch.security.configuration.AdminDNs;
41+
import org.opensearch.security.configuration.ConfigurationRepository;
42+
import org.opensearch.security.dlic.rest.api.Endpoint;
43+
import org.opensearch.security.dlic.rest.api.RestApiAdminPrivilegesEvaluator;
44+
import org.opensearch.security.dlic.rest.api.RestApiPrivilegesEvaluator;
45+
import org.opensearch.security.dlic.rest.api.SecurityApiDependencies;
46+
import org.opensearch.security.dlic.rest.support.Utils;
47+
import org.opensearch.security.privileges.PrivilegesEvaluator;
48+
import org.opensearch.security.ssl.transport.PrincipalExtractor;
49+
import org.opensearch.security.support.ConfigConstants;
50+
import org.opensearch.threadpool.ThreadPool;
3551

3652
import static org.opensearch.rest.RestRequest.Method.DELETE;
3753
import static org.opensearch.rest.RestRequest.Method.GET;
@@ -44,23 +60,57 @@
4460
import static org.opensearch.security.action.apitokens.ApiToken.INDEX_PERMISSIONS_FIELD;
4561
import static org.opensearch.security.action.apitokens.ApiToken.NAME_FIELD;
4662
import static org.opensearch.security.dlic.rest.support.Utils.addRoutesPrefix;
63+
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ADMIN_ENABLED;
4764
import static org.opensearch.security.util.ParsingUtils.safeMapList;
4865
import static org.opensearch.security.util.ParsingUtils.safeStringList;
4966

5067
public class ApiTokenAction extends BaseRestHandler {
51-
private ApiTokenRepository apiTokenRepository;
68+
private final ApiTokenRepository apiTokenRepository;
5269
public Logger log = LogManager.getLogger(this.getClass());
53-
54-
private static final List<RestHandler.Route> ROUTES = addRoutesPrefix(
55-
ImmutableList.of(
56-
new RestHandler.Route(POST, "/apitokens"),
57-
new RestHandler.Route(DELETE, "/apitokens"),
58-
new RestHandler.Route(GET, "/apitokens")
59-
)
70+
private final ThreadPool threadPool;
71+
private final ConfigurationRepository configurationRepository;
72+
private final PrivilegesEvaluator privilegesEvaluator;
73+
private final SecurityApiDependencies securityApiDependencies;
74+
private final ClusterService clusterService;
75+
private final IndexNameExpressionResolver indexNameExpressionResolver;
76+
77+
private static final List<Route> ROUTES = addRoutesPrefix(
78+
ImmutableList.of(new Route(POST, "/apitokens"), new Route(DELETE, "/apitokens"), new Route(GET, "/apitokens"))
6079
);
6180

62-
public ApiTokenAction(ApiTokenRepository apiTokenRepository) {
81+
public ApiTokenAction(
82+
ThreadPool threadpool,
83+
ConfigurationRepository configurationRepository,
84+
PrivilegesEvaluator privilegesEvaluator,
85+
Settings settings,
86+
AdminDNs adminDns,
87+
AuditLog auditLog,
88+
Path configPath,
89+
PrincipalExtractor principalExtractor,
90+
ApiTokenRepository apiTokenRepository,
91+
ClusterService clusterService,
92+
IndexNameExpressionResolver indexNameExpressionResolver
93+
) {
6394
this.apiTokenRepository = apiTokenRepository;
95+
this.threadPool = threadpool;
96+
this.configurationRepository = configurationRepository;
97+
this.privilegesEvaluator = privilegesEvaluator;
98+
this.securityApiDependencies = new SecurityApiDependencies(
99+
adminDns,
100+
configurationRepository,
101+
privilegesEvaluator,
102+
new RestApiPrivilegesEvaluator(settings, adminDns, privilegesEvaluator, principalExtractor, configPath, threadPool),
103+
new RestApiAdminPrivilegesEvaluator(
104+
threadPool.getThreadContext(),
105+
privilegesEvaluator,
106+
adminDns,
107+
settings.getAsBoolean(SECURITY_RESTAPI_ADMIN_ENABLED, false)
108+
),
109+
auditLog,
110+
settings
111+
);
112+
this.clusterService = clusterService;
113+
this.indexNameExpressionResolver = indexNameExpressionResolver;
64114
}
65115

66116
@Override
@@ -69,22 +119,28 @@ public String getName() {
69119
}
70120

71121
@Override
72-
public List<RestHandler.Route> routes() {
122+
public List<Route> routes() {
73123
return ROUTES;
74124
}
75125

76126
@Override
77127
protected RestChannelConsumer prepareRequest(final RestRequest request, final NodeClient client) throws IOException {
78-
// TODO: Authorize this API properly
79-
switch (request.method()) {
80-
case POST:
81-
return handlePost(request, client);
82-
case DELETE:
83-
return handleDelete(request, client);
84-
case GET:
85-
return handleGet(request, client);
86-
default:
87-
throw new IllegalArgumentException(request.method() + " not supported");
128+
authorizeSecurityAccess(request);
129+
return doPrepareRequest(request, client);
130+
}
131+
132+
RestChannelConsumer doPrepareRequest(RestRequest request, NodeClient client) {
133+
final var originalUserAndRemoteAddress = Utils.userAndRemoteAddressFrom(client.threadPool().getThreadContext());
134+
try (final ThreadContext.StoredContext ctx = client.threadPool().getThreadContext().stashContext()) {
135+
client.threadPool()
136+
.getThreadContext()
137+
.putTransient(ConfigConstants.OPENDISTRO_SECURITY_USER, originalUserAndRemoteAddress.getLeft());
138+
return switch (request.method()) {
139+
case POST -> handlePost(request, client);
140+
case DELETE -> handleDelete(request, client);
141+
case GET -> handleGet(request, client);
142+
default -> throw new IllegalArgumentException(request.method() + " not supported");
143+
};
88144
}
89145
}
90146

@@ -119,8 +175,6 @@ private RestChannelConsumer handleGet(RestRequest request, NodeClient client) {
119175

120176
private RestChannelConsumer handlePost(RestRequest request, NodeClient client) {
121177
return channel -> {
122-
final XContentBuilder builder = channel.newBuilder();
123-
BytesRestResponse response;
124178
try {
125179
final Map<String, Object> requestBody = request.contentOrSourceParamParser().map();
126180
validateRequestParameters(requestBody);
@@ -245,8 +299,6 @@ void validateIndexPermissionsList(List<Map<String, Object>> indexPermsList) {
245299

246300
private RestChannelConsumer handleDelete(RestRequest request, NodeClient client) {
247301
return channel -> {
248-
final XContentBuilder builder = channel.newBuilder();
249-
BytesRestResponse response;
250302
try {
251303
final Map<String, Object> requestBody = request.contentOrSourceParamParser().map();
252304

@@ -295,4 +347,11 @@ private void sendErrorResponse(RestChannel channel, RestStatus status, String er
295347
}
296348
}
297349

350+
protected void authorizeSecurityAccess(RestRequest request) throws IOException {
351+
// Check if user has security API access
352+
if (!(securityApiDependencies.restApiAdminPrivilegesEvaluator().isCurrentUserAdminFor(Endpoint.APITOKENS)
353+
|| securityApiDependencies.restApiPrivilegesEvaluator().checkAccessPermissions(request, Endpoint.APITOKENS) == null)) {
354+
throw new SecurityException("User does not have required security API access");
355+
}
356+
}
298357
}

0 commit comments

Comments
 (0)