From 16cb84037b5697bf7f201f7ce2af51863e0a6471 Mon Sep 17 00:00:00 2001 From: Nils Bandener Date: Thu, 4 Jul 2024 10:24:27 +0200 Subject: [PATCH] Made PrivilegesEvaluationContext.mappedRoles non-final, moved OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION handling back to evaluate() Signed-off-by: Nils Bandener --- .../security/filter/SecurityFilter.java | 11 +---- .../PrivilegesEvaluationContext.java | 14 ++++++- .../privileges/PrivilegesEvaluator.java | 40 ++++++++++--------- .../PrivilegesEvaluatorResponse.java | 23 ----------- 4 files changed, 37 insertions(+), 51 deletions(-) diff --git a/src/main/java/org/opensearch/security/filter/SecurityFilter.java b/src/main/java/org/opensearch/security/filter/SecurityFilter.java index 509a407196..a2c16eb549 100644 --- a/src/main/java/org/opensearch/security/filter/SecurityFilter.java +++ b/src/main/java/org/opensearch/security/filter/SecurityFilter.java @@ -379,15 +379,8 @@ private void ap log.trace("Evaluate permissions for user: {}", user.getName()); } - PrivilegesEvaluationContext context = null; - PrivilegesEvaluatorResponse pres; - - try { - context = eval.createContext(user, action, request, task, injectedRoles); - pres = eval.evaluate(context); - } catch (PrivilegesEvaluatorResponse.NotAllowedException e) { - pres = e.getResponse(); - } + PrivilegesEvaluationContext context = eval.createContext(user, action, request, task, injectedRoles); + PrivilegesEvaluatorResponse pres = eval.evaluate(context); if (log.isDebugEnabled()) { log.debug(pres.toString()); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java index bee5ad90e8..98ffddb3d3 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java @@ -32,7 +32,7 @@ public class PrivilegesEvaluationContext { private final ActionRequest request; private IndexResolverReplacer.Resolved resolvedRequest; private final Task task; - private final ImmutableSet mappedRoles; + private ImmutableSet mappedRoles; private final IndexResolverReplacer indexResolverReplacer; public PrivilegesEvaluationContext( @@ -82,4 +82,16 @@ public ImmutableSet getMappedRoles() { return mappedRoles; } + /** + * Note: Ideally, mappedRoles would be an unmodifiable attribute. PrivilegesEvaluator however contains logic + * related to OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION which first validates roles and afterwards modifies + * them again. Thus, we need to be able to set this attribute. + * + * However, this method should be only used for this one particular phase. Normally, all roles should be determined + * upfront and stay constant during the whole privilege evaluation process. + */ + void setMappedRoles(ImmutableSet mappedRoles) { + this.mappedRoles = mappedRoles; + } + } diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java index d8985f9f90..f7f8fb66a9 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java @@ -221,29 +221,19 @@ private void setUserInfoInThreadContext(User user) { } } - public PrivilegesEvaluationContext createContext(User user, String action0, ActionRequest request, Task task, Set injectedRoles) - throws PrivilegesEvaluatorResponse.NotAllowedException { + public PrivilegesEvaluationContext createContext( + User user, + String action0, + ActionRequest request, + Task task, + Set injectedRoles + ) { if (!isInitialized()) { throw new OpenSearchSecurityException("OpenSearch Security is not initialized."); } TransportAddress caller = threadContext.getTransient(ConfigConstants.OPENDISTRO_SECURITY_REMOTE_ADDRESS); ImmutableSet mappedRoles = ImmutableSet.copyOf((injectedRoles == null) ? mapRoles(user, caller) : injectedRoles); - final String injectedRolesValidationString = threadContext.getTransient( - ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION - ); - if (injectedRolesValidationString != null) { - HashSet injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(","))); - if (!mappedRoles.containsAll(injectedRolesValidationSet)) { - PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse(); - presponse.allowed = false; - presponse.missingSecurityRoles.addAll(injectedRolesValidationSet); - presponse.reason("Injected roles validation failed"); - log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user); - throw new PrivilegesEvaluatorResponse.NotAllowedException(presponse); - } - mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet); - } return new PrivilegesEvaluationContext(user, mappedRoles, action0, request, task, irr); } @@ -272,8 +262,22 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context) action0 = PutMappingAction.NAME; } - PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse(); + final PrivilegesEvaluatorResponse presponse = new PrivilegesEvaluatorResponse(); + final String injectedRolesValidationString = threadContext.getTransient( + ConfigConstants.OPENDISTRO_SECURITY_INJECTED_ROLES_VALIDATION + ); + if (injectedRolesValidationString != null) { + HashSet injectedRolesValidationSet = new HashSet<>(Arrays.asList(injectedRolesValidationString.split(","))); + if (!mappedRoles.containsAll(injectedRolesValidationSet)) { + presponse.allowed = false; + presponse.missingSecurityRoles.addAll(injectedRolesValidationSet); + log.info("Roles {} are not mapped to the user {}", injectedRolesValidationSet, user); + return presponse; + } + mappedRoles = ImmutableSet.copyOf(injectedRolesValidationSet); + context.setMappedRoles(mappedRoles); + } presponse.resolvedSecurityRoles.addAll(mappedRoles); final SecurityRoles securityRoles = getSecurityRoles(mappedRoles); diff --git a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java index 4c259871a5..1da88f62b7 100644 --- a/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java +++ b/src/main/java/org/opensearch/security/privileges/PrivilegesEvaluatorResponse.java @@ -96,27 +96,4 @@ public static enum PrivilegesEvaluatorResponseState { PENDING, COMPLETE; } - - /** - * This exception can be used to indicate that a method denies a user access to an OpenSearch action. - * - * Note: As exceptions take their performance toll, please use this exception only when there is - * no other way. Prefer to use PrivilegesEvaluatorResponse directly as a return value. - */ - public static class NotAllowedException extends Exception { - private final PrivilegesEvaluatorResponse response; - - public NotAllowedException(PrivilegesEvaluatorResponse response) { - super(response.reason); - this.response = response; - if (response.allowed) { - throw new IllegalArgumentException("Only possible for PrivilegesEvaluatorResponse with allowed=false"); - } - } - - public PrivilegesEvaluatorResponse getResponse() { - return response; - } - } - }