Skip to content

Commit 3be1361

Browse files
nibixnibix
committed
fewer resolved indices
1 parent 396601d commit 3be1361

18 files changed

+1415
-770
lines changed

‎src/integrationTest/java/org/opensearch/security/privileges/int_tests/IndexAuthorizationReadOnlyIntTests.java‎

Lines changed: 500 additions & 375 deletions
Large diffs are not rendered by default.

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -657,7 +657,7 @@ public List<RestHandler> getRestHandlers(
657657
settings,
658658
restController,
659659
Objects.requireNonNull(backendRegistry),
660-
Objects.requireNonNull(evaluator)
660+
Objects.requireNonNull(privilegesConfiguration)
661661
)
662662
);
663663
handlers.add(

‎src/main/java/org/opensearch/security/configuration/PrivilegesInterceptorImpl.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ public ReplaceResult replaceDashboardsIndex(
106106
final ActionRequest request,
107107
final String action,
108108
final User user,
109-
final OptionallyResolvedIndices optionallyResolvedIndices,
110109
final PrivilegesEvaluationContext context
111110
) {
112111
DashboardsMultiTenancyConfiguration config = this.multiTenancyConfigurationSupplier.get();
@@ -117,6 +116,7 @@ public ReplaceResult replaceDashboardsIndex(
117116
return CONTINUE_EVALUATION_REPLACE_RESULT;
118117
}
119118

119+
OptionallyResolvedIndices optionallyResolvedIndices = context.getResolvedRequest();
120120
TenantPrivileges tenantPrivileges = this.tenantPrivilegesSupplier.get();
121121

122122
if (!(optionallyResolvedIndices instanceof ResolvedIndices resolvedIndices)) {

‎src/main/java/org/opensearch/security/filter/SecurityRestFilter.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -264,14 +264,13 @@ void authorizeRequest(RestHandler original, SecurityRequestChannel request, User
264264
.findFirst();
265265
final boolean routeSupportsRestAuthorization = handler.isPresent() && handler.get() instanceof NamedRoute;
266266
if (routeSupportsRestAuthorization) {
267-
PrivilegesEvaluatorResponse pres = new PrivilegesEvaluatorResponse();
268267
NamedRoute route = ((NamedRoute) handler.get());
269268
// Check both route.actionNames() and route.name(). The presence of either is sufficient.
270269
Set<String> actionNames = ImmutableSet.<String>builder()
271270
.addAll(route.actionNames() != null ? route.actionNames() : Collections.emptySet())
272271
.add(route.name())
273272
.build();
274-
pres = evaluator.evaluate(user, route.name(), actionNames);
273+
PrivilegesEvaluatorResponse pres = evaluator.evaluate(user, route.name(), actionNames);
275274

276275
if (log.isDebugEnabled()) {
277276
log.debug(pres.toString());

‎src/main/java/org/opensearch/security/privileges/IndicesRequestModifier.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import org.opensearch.action.ActionRequest;
1818
import org.opensearch.action.IndicesRequest;
19+
import org.opensearch.action.admin.indices.segments.PitSegmentsRequest;
1920
import org.opensearch.cluster.metadata.ResolvedIndices;
2021

2122
/**
@@ -32,7 +33,10 @@ public boolean setLocalIndices(ActionRequest targetRequest, ResolvedIndices reso
3233
return setLocalIndicesToEmpty(targetRequest, resolvedIndices);
3334
}
3435

35-
if (targetRequest instanceof IndicesRequest.Replaceable) {
36+
if (targetRequest instanceof PitSegmentsRequest) {
37+
// PitSegmentsRequest implements IndicesRequest.Replaceable, but ignores all specified indices
38+
return false;
39+
} else if (targetRequest instanceof IndicesRequest.Replaceable) {
3640
((IndicesRequest.Replaceable) targetRequest).indices(concat(newIndices, resolvedIndices.remote().asRawExpressions()));
3741
return true;
3842
} else {
@@ -41,7 +45,10 @@ public boolean setLocalIndices(ActionRequest targetRequest, ResolvedIndices reso
4145
}
4246

4347
public boolean setLocalIndicesToEmpty(ActionRequest targetRequest, ResolvedIndices resolvedIndices) {
44-
if (targetRequest instanceof IndicesRequest.Replaceable replaceable) {
48+
if (targetRequest instanceof PitSegmentsRequest) {
49+
// PitSegmentsRequest implements IndicesRequest.Replaceable, but ignores all specified indices
50+
return false;
51+
} else if (targetRequest instanceof IndicesRequest.Replaceable replaceable) {
4552
if (resolvedIndices.remote().isEmpty()) {
4653
if (replaceable.indicesOptions().expandWildcardsOpen() || replaceable.indicesOptions().expandWildcardsClosed()) {
4754
// If the request expands wildcards, we use an index expression which resolves to no indices

‎src/main/java/org/opensearch/security/privileges/PrivilegesConfiguration.java‎

Lines changed: 32 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -117,10 +117,7 @@ public PrivilegesConfiguration(
117117
PrivilegesEvaluationType privilegesEvaluationType = PrivilegesEvaluationType.getFrom(
118118
configurationRepository.getConfiguration(CType.CONFIG)
119119
);
120-
PrivilegesEvaluationType currentEvaluationType = currentPrivilegesEvaluator == null ? null
121-
: currentPrivilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.legacy.PrivilegesEvaluator
122-
? PrivilegesEvaluationType.LEGACY
123-
: PrivilegesEvaluationType.NEXT_GEN;
120+
PrivilegesEvaluationType currentEvaluationType = PrivilegesEvaluationType.typeOf(currentPrivilegesEvaluator);
124121

125122
if (privilegesEvaluationType != currentEvaluationType) {
126123
if (privilegesEvaluationType == PrivilegesEvaluationType.LEGACY) {
@@ -147,23 +144,24 @@ public PrivilegesConfiguration(
147144
}
148145
} else {
149146
PrivilegesEvaluator oldInstance = privilegesEvaluator.getAndSet(
150-
new org.opensearch.security.privileges.actionlevel.nextgen.PrivilegesEvaluator(
151-
clusterService,
152-
clusterStateSupplier,
153-
roleMapper,
154-
threadPool,
155-
threadPool.getThreadContext(),
156-
resolver,
157-
auditLog,
158-
settings,
159-
privilegesInterceptor,
160-
flattenedActionGroups,
161-
staticActionGroups,
162-
rolesConfiguration,
163-
generalConfiguration,
164-
pluginIdToRolePrivileges,
165-
new RuntimeOptimizedActionPrivileges.SpecialIndexProtection(this.specialIndices::isUniversallyDeniedIndex, this.specialIndices::isSystemIndex)
147+
new org.opensearch.security.privileges.actionlevel.nextgen.PrivilegesEvaluator(
148+
clusterStateSupplier,
149+
roleMapper,
150+
threadPool,
151+
threadPool.getThreadContext(),
152+
resolver,
153+
settings,
154+
privilegesInterceptor,
155+
flattenedActionGroups,
156+
staticActionGroups,
157+
rolesConfiguration,
158+
generalConfiguration,
159+
pluginIdToRolePrivileges,
160+
new RuntimeOptimizedActionPrivileges.SpecialIndexProtection(
161+
this.specialIndices::isUniversallyDeniedIndex,
162+
this.specialIndices::isSystemIndex
166163
)
164+
)
167165
);
168166
if (oldInstance != null) {
169167
oldInstance.shutdown();
@@ -239,6 +237,10 @@ public void updatePluginToActionPrivileges(String pluginIdentifier, RoleV7 plugi
239237
pluginIdToRolePrivileges.put(pluginIdentifier, pluginPermissions);
240238
}
241239

240+
public boolean isInitialized() {
241+
return this.privilegesEvaluator().isInitialized();
242+
}
243+
242244
/**
243245
* TODO: Think about better names
244246
*/
@@ -263,6 +265,16 @@ static PrivilegesEvaluationType getFrom(SecurityDynamicConfiguration<ConfigV7> c
263265
return LEGACY;
264266
}
265267
}
268+
269+
static PrivilegesEvaluationType typeOf(PrivilegesEvaluator privilegesEvaluator) {
270+
if (privilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.legacy.PrivilegesEvaluator) {
271+
return PrivilegesEvaluationType.LEGACY;
272+
} else if (privilegesEvaluator instanceof org.opensearch.security.privileges.actionlevel.nextgen.PrivilegesEvaluator) {
273+
return PrivilegesEvaluationType.NEXT_GEN;
274+
} else {
275+
return null;
276+
}
277+
}
266278
}
267279

268280
private static FlattenedActionGroups buildStaticActionGroups() {

‎src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@ void updateConfiguration(
7373

7474
boolean notFailOnForbiddenEnabled();
7575

76+
boolean isInitialized();
77+
7678
/**
7779
* A PrivilegesEvaluator implementation that just throws "not initialized" exceptions.
7880
* Used initially by PrivilegesConfiguration.
@@ -129,6 +131,11 @@ public boolean notFailOnForbiddenEnabled() {
129131
return false;
130132
}
131133

134+
@Override
135+
public boolean isInitialized() {
136+
return false;
137+
}
138+
132139
private OpenSearchSecurityException exception() {
133140
StringBuilder error = new StringBuilder("OpenSearch Security is not initialized");
134141
String reason = this.unavailablityReasonSupplier.get();

‎src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java‎

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -127,7 +127,24 @@ public String getPrivilegeMatrix() {
127127
String result = this.privilegeMatrix;
128128

129129
if (result == null) {
130-
result = this.indexToActionCheckTable.toTableString("ok", "MISSING");
130+
String topLevelMatrix;
131+
132+
if (this.indexToActionCheckTable != null) {
133+
topLevelMatrix = this.indexToActionCheckTable.toTableString("ok", "MISSING");
134+
} else {
135+
topLevelMatrix = "n/a";
136+
}
137+
138+
if (subResults.isEmpty()) {
139+
result = topLevelMatrix;
140+
} else {
141+
StringBuilder resultBuilder = new StringBuilder(topLevelMatrix);
142+
for (PrivilegesEvaluatorResponse subResult : subResults) {
143+
resultBuilder.append("\n");
144+
resultBuilder.append(subResult.getPrivilegeMatrix());
145+
}
146+
result = resultBuilder.toString();
147+
}
131148
this.privilegeMatrix = result;
132149
}
133150
return result;
@@ -141,8 +158,40 @@ public CreateIndexRequestBuilder getCreateIndexRequestBuilder() {
141158
return createIndexRequestBuilder;
142159
}
143160

144-
public PrivilegesEvaluatorResponse markComplete() {
145-
this.state = PrivilegesEvaluatorResponseState.COMPLETE;
161+
public PrivilegesEvaluatorResponse originalResult() {
162+
return this.originalResult;
163+
}
164+
165+
public boolean privilegesAreComplete() {
166+
if (originalResult != null && !originalResult.privilegesAreComplete()) {
167+
return false;
168+
} else if (indexToActionCheckTable != null && !indexToActionCheckTable.isComplete()) {
169+
return false;
170+
} else if (!subResults.isEmpty() && subResults.stream().anyMatch(subResult -> !subResult.privilegesAreComplete())) {
171+
return false;
172+
} else {
173+
return this.allowed;
174+
}
175+
}
176+
177+
public PrivilegesEvaluatorResponse insufficient(List<PrivilegesEvaluatorResponse> subResults) {
178+
String reason = this.reason;
179+
if (reason == null) {
180+
reason = subResults.stream().map(result -> result.reason).filter(Objects::nonNull).findFirst().orElse(null);
181+
}
182+
PrivilegesEvaluatorResponse result = new PrivilegesEvaluatorResponse();
183+
result.allowed = false;
184+
result.indexToActionCheckTable = this.indexToActionCheckTable;
185+
result.subResults = ImmutableList.copyOf(subResults);
186+
result.reason = reason;
187+
return result;
188+
}
189+
190+
public PrivilegesEvaluatorResponse originalResult(PrivilegesEvaluatorResponse originalResult) {
191+
if (originalResult != null && !originalResult.evaluationExceptions.isEmpty()) {
192+
this.originalResult = originalResult;
193+
this.evaluationExceptions.addAll(originalResult.evaluationExceptions);
194+
}
146195
return this;
147196
}
148197

‎src/main/java/org/opensearch/security/privileges/PrivilegesInterceptor.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import org.opensearch.action.ActionRequest;
3030
import org.opensearch.action.admin.indices.create.CreateIndexRequestBuilder;
3131
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
32-
import org.opensearch.cluster.metadata.OptionallyResolvedIndices;
3332
import org.opensearch.cluster.service.ClusterService;
3433
import org.opensearch.common.util.concurrent.ThreadContext;
3534
import org.opensearch.security.user.User;
@@ -79,7 +78,6 @@ public ReplaceResult replaceDashboardsIndex(
7978
final ActionRequest request,
8079
final String action,
8180
final User user,
82-
final OptionallyResolvedIndices requestedResolved,
8381
final PrivilegesEvaluationContext context
8482
) {
8583
throw new RuntimeException("not implemented");

‎src/main/java/org/opensearch/security/privileges/actionlevel/RuntimeOptimizedActionPrivileges.java‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.apache.logging.log4j.Logger;
2626

2727
import org.opensearch.cluster.metadata.IndexAbstraction;
28+
import org.opensearch.cluster.metadata.IndexMetadata;
2829
import org.opensearch.cluster.metadata.OptionallyResolvedIndices;
2930
import org.opensearch.security.privileges.ActionPrivileges;
3031
import org.opensearch.security.privileges.IndexPattern;
@@ -414,19 +415,19 @@ protected void checkPrivilegesForNonWellKnownActions(
414415
}
415416
}
416417

418+
/**
419+
* When we have finished the "normal" privilege evaluation, which is based on index_permissions in roles.yml,
420+
* we have to pass the CheckTable with the available privileges through this method in order to have specially
421+
* protected indices and actions removed again from the CheckTable.
422+
*/
417423
protected PrivilegesEvaluatorResponse finalizeResult(PrivilegesEvaluationContext context, IntermediateResult intermediateResult) {
418424
CheckTable<String, String> checkTable = intermediateResult.indexToActionCheckTable;
419425
List<PrivilegesEvaluationException> exceptions = new ArrayList<>(intermediateResult.exceptions);
420426
if (this.universallyDeniedIndices != null) {
421427
checkTable.uncheckIf(this.universallyDeniedIndices, checkTable.getColumns());
422428
}
423429
if (this.indicesNeedingSystemIndexPrivileges != null) {
424-
// TODO aliases
425-
checkTable.uncheckIf(
426-
index -> this.indicesNeedingSystemIndexPrivileges.test(index)
427-
&& !providesExplicitPrivilege(context, index, ConfigConstants.SYSTEM_INDEX_PERMISSION, exceptions),
428-
checkTable.getColumns()
429-
);
430+
checkTable.uncheckIf(index -> this.isUnauthorizedSystemIndex(context, index, exceptions), checkTable.getColumns());
430431
}
431432

432433
if (checkTable.isComplete()) {
@@ -451,6 +452,36 @@ protected PrivilegesEvaluatorResponse finalizeResult(PrivilegesEvaluationContext
451452
return PrivilegesEvaluatorResponse.insufficient(checkTable).reason(reason).evaluationExceptions(exceptions);
452453

453454
}
455+
456+
/**
457+
* Returns true if the given indexOrAlias is a system index or an alias containing a system index AND if
458+
* the current user does not have the necessary explicit privilege to access this system index.
459+
*/
460+
private boolean isUnauthorizedSystemIndex(
461+
PrivilegesEvaluationContext context,
462+
String indexOrAlias,
463+
List<PrivilegesEvaluationException> exceptions
464+
) {
465+
if (this.indicesNeedingSystemIndexPrivileges.test(indexOrAlias)) {
466+
return !providesExplicitPrivilege(context, indexOrAlias, ConfigConstants.SYSTEM_INDEX_PERMISSION, exceptions);
467+
}
468+
469+
IndexAbstraction indexAbstraction = context.getIndicesLookup().get(indexOrAlias);
470+
if (indexAbstraction instanceof IndexAbstraction.Alias alias) {
471+
for (IndexMetadata index : alias.getIndices()) {
472+
if (this.indicesNeedingSystemIndexPrivileges.test(index.getIndex().getName())) {
473+
return !providesExplicitPrivilege(
474+
context,
475+
index.getIndex().getName(),
476+
ConfigConstants.SYSTEM_INDEX_PERMISSION,
477+
exceptions
478+
);
479+
}
480+
}
481+
}
482+
483+
return false;
484+
}
454485
}
455486

456487
/**

0 commit comments

Comments
 (0)