Skip to content

Commit

Permalink
Add more extensive tests for authenticator, switch to list of indexPe…
Browse files Browse the repository at this point in the history
…rmissions

Signed-off-by: Derek Ho <[email protected]>
  • Loading branch information
derek-ho committed Dec 27, 2024
1 parent 92d4e60 commit 22cfbe8
Show file tree
Hide file tree
Showing 7 changed files with 156 additions and 135 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,7 @@ public class SecurityRestFilter {

protected final Logger log = LogManager.getLogger(this.getClass());
public static final String API_TOKEN_CLUSTERPERM_KEY = "security.api_token.clusterperm";
public static final String API_TOKEN_INDEXACTIONS_KEY = "security.api_token.indexactions";
public static final String API_TOKEN_INDICES_KEY = "security.api_token.indices";
public static final String API_TOKEN_INDEXPERM_KEY = "security.api_token.indexactions";
private final BackendRegistry registry;
private final RestLayerPrivilegesEvaluator evaluator;
private final AuditLog auditLog;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,8 @@
import java.security.AccessController;
import java.security.PrivilegedAction;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

Expand All @@ -36,7 +33,6 @@
import org.opensearch.core.xcontent.DeprecationHandler;
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.core.xcontent.XContentParser;
import org.opensearch.security.DefaultObjectMapper;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.action.apitokens.ApiTokenIndexListenerCache;
import org.opensearch.security.auth.HTTPAuthenticator;
Expand All @@ -55,8 +51,7 @@
import static org.opensearch.security.OpenSearchSecurityPlugin.LEGACY_OPENDISTRO_PREFIX;
import static org.opensearch.security.OpenSearchSecurityPlugin.PLUGINS_PREFIX;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_CLUSTERPERM_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXACTIONS_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDICES_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXPERM_KEY;
import static org.opensearch.security.util.AuthTokenUtils.isAccessToRestrictedEndpoints;

public class ApiTokenAuthenticator implements HTTPAuthenticator {
Expand All @@ -65,7 +60,7 @@ public class ApiTokenAuthenticator implements HTTPAuthenticator {
private static final String REGEX_PATH_PREFIX = "/(" + LEGACY_OPENDISTRO_PREFIX + "|" + PLUGINS_PREFIX + ")/" + "(.*)";
private static final Pattern PATTERN_PATH_PREFIX = Pattern.compile(REGEX_PATH_PREFIX);

protected final Logger log = LogManager.getLogger(this.getClass());
public Logger log = LogManager.getLogger(this.getClass());

private static final Pattern BEARER = Pattern.compile("^\\s*Bearer\\s.*", Pattern.CASE_INSENSITIVE);
private static final String BEARER_PREFIX = "bearer ";
Expand Down Expand Up @@ -131,7 +126,7 @@ private String extractClusterPermissionsFromClaims(Claims claims) {
return clusterPermissions;
}

private String extractAllowedActionsFromClaims(Claims claims) throws IOException {
private List<ApiToken.IndexPermission> extractIndexPermissionFromClaims(Claims claims) throws IOException {
Object ip = claims.get("ip");

if (ip != null) {
Expand All @@ -150,56 +145,15 @@ private String extractAllowedActionsFromClaims(Claims claims) throws IOException
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
permissions.add(ApiToken.IndexPermission.fromXContent(parser));
}
// Get first permission's actions
if (!permissions.isEmpty() && !permissions.get(0).getAllowedActions().isEmpty()) {
return permissions.get(0).getAllowedActions().get(0);
}

return "";
} catch (Exception e) {
log.error("Error extracting allowed actions", e);
return "";
}

}

return "";
}

private String extractIndicesFromClaims(Claims claims) throws IOException {
Object ip = claims.get("ip");

if (ip != null) {
String decryptedPermissions = encryptionUtil.decrypt(ip.toString());

try (
XContentParser parser = XContentType.JSON.xContent()
.createParser(NamedXContentRegistry.EMPTY, DeprecationHandler.THROW_UNSUPPORTED_OPERATION, decryptedPermissions)
) {

// Use built-in array parsing
List<ApiToken.IndexPermission> permissions = new ArrayList<>();

// Move to start of array
parser.nextToken(); // START_ARRAY
while (parser.nextToken() != XContentParser.Token.END_ARRAY) {
permissions.add(ApiToken.IndexPermission.fromXContent(parser));
}

// Get first permission's actions
if (!permissions.isEmpty() && !permissions.get(0).getIndexPatterns().isEmpty()) {
return permissions.get(0).getIndexPatterns().get(0);
}

return "";
return permissions;
} catch (Exception e) {
log.error("Error extracting indices", e);
return "";
log.error("Error extracting index permissions", e);
return List.of();

Check warning on line 151 in src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java#L149-L151

Added lines #L149 - L151 were not covered by tests
}

}

return "";
return List.of();

Check warning on line 156 in src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java#L156

Added line #L156 was not covered by tests
}

@Override
Expand Down Expand Up @@ -253,46 +207,19 @@ private AuthCredentials extractCredentials0(final SecurityRequest request, final
return null;

Check warning on line 207 in src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/http/ApiTokenAuthenticator.java#L206-L207

Added lines #L206 - L207 were not covered by tests
}

final Set<String> audience = claims.getAudience();
if (audience == null || audience.isEmpty()) {
log.error("Valid jwt api token with no audience");
return null;
}

final String issuer = claims.getIssuer();
if (!clusterName.equals(issuer)) {
log.error("The issuer of this api token does not match the current cluster identifier");
return null;
}

String clusterPermissions = extractClusterPermissionsFromClaims(claims);
String allowedActions = extractAllowedActionsFromClaims(claims);
String indices = extractIndicesFromClaims(claims);

final AuthCredentials ac = new AuthCredentials(subject, List.of(), new String[0]).markComplete();

for (Entry<String, Object> claim : claims.entrySet()) {
String key = "attr.jwt." + claim.getKey();
Object value = claim.getValue();

if (value instanceof Collection<?>) {
try {
// Convert the list to a JSON array string
String jsonValue = DefaultObjectMapper.writeValueAsString(value, false);
ac.addAttribute(key, jsonValue);
} catch (Exception e) {
log.warn("Failed to convert list claim to JSON for key: " + key, e);
// Fallback to string representation
ac.addAttribute(key, String.valueOf(value));
}
} else {
ac.addAttribute(key, String.valueOf(value));
}
}
List<ApiToken.IndexPermission> indexPermissions = extractIndexPermissionFromClaims(claims);

final AuthCredentials ac = new AuthCredentials(subject, List.of(), "").markComplete();

context.putTransient(API_TOKEN_CLUSTERPERM_KEY, clusterPermissions);
context.putTransient(API_TOKEN_INDEXACTIONS_KEY, allowedActions);
context.putTransient(API_TOKEN_INDICES_KEY, indices);
context.putTransient(API_TOKEN_INDEXPERM_KEY, indexPermissions);

return ac;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import org.opensearch.common.settings.Settings;
import org.opensearch.core.common.unit.ByteSizeUnit;
import org.opensearch.core.common.unit.ByteSizeValue;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.securityconf.FlattenedActionGroups;
import org.opensearch.security.securityconf.impl.SecurityDynamicConfiguration;
Expand Down Expand Up @@ -164,8 +165,16 @@ public PrivilegesEvaluatorResponse hasIndexPrivilege(

// API Token Authz
// TODO: this is very naive implementation
if (context.getIndices() != null && new HashSet<>(context.getIndices()).containsAll(resolvedIndices.getAllIndices())) {
if (new HashSet<>(context.getAllowedActions()).containsAll(actions)) {
if (context.getIndexPermissions() != null) {
List<ApiToken.IndexPermission> indexPermissions = context.getIndexPermissions();

Check warning on line 169 in src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/ActionPrivileges.java#L169

Added line #L169 was not covered by tests

boolean hasPermission = indexPermissions.stream().anyMatch(permission -> {
boolean hasAllActions = new HashSet<>(permission.getAllowedActions()).containsAll(actions);
boolean hasAllIndices = new HashSet<>(permission.getIndexPatterns()).containsAll(resolvedIndices.getAllIndices());

Check warning on line 173 in src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/ActionPrivileges.java#L171-L173

Added lines #L171 - L173 were not covered by tests
return hasAllActions && hasAllIndices;
});

if (hasPermission) {
return PrivilegesEvaluatorResponse.ok();

Check warning on line 178 in src/main/java/org/opensearch/security/privileges/ActionPrivileges.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/ActionPrivileges.java#L178

Added line #L178 was not covered by tests
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import org.opensearch.cluster.ClusterState;
import org.opensearch.cluster.metadata.IndexAbstraction;
import org.opensearch.cluster.metadata.IndexNameExpressionResolver;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.resolver.IndexResolverReplacer;
import org.opensearch.security.support.WildcardMatcher;
import org.opensearch.security.user.User;
Expand All @@ -47,8 +48,7 @@ public class PrivilegesEvaluationContext {
private final IndexNameExpressionResolver indexNameExpressionResolver;
private final Supplier<ClusterState> clusterStateSupplier;
private List<String> clusterPermissions;
private List<String> allowedActions;
private List<String> indices;
private List<ApiToken.IndexPermission> indexPermissions;

/**
* This caches the ready to use WildcardMatcher instances for the current request. Many index patterns have
Expand Down Expand Up @@ -185,19 +185,11 @@ public List<String> getClusterPermissions() {
return clusterPermissions;
}

public List<String> getAllowedActions() {
return allowedActions;
public List<ApiToken.IndexPermission> getIndexPermissions() {
return indexPermissions;
}

public void setAllowedActions(List<String> allowedActions) {
this.allowedActions = allowedActions;
}

public List<String> getIndices() {
return indices;
}

public void setIndices(List<String> indices) {
this.indices = indices;
public void setIndexPermissions(List<ApiToken.IndexPermission> indexPermissions) {
this.indexPermissions = indexPermissions;
}

Check warning on line 194 in src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluationContext.java#L193-L194

Added lines #L193 - L194 were not covered by tests
}
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@
import org.opensearch.core.xcontent.NamedXContentRegistry;
import org.opensearch.index.reindex.ReindexAction;
import org.opensearch.script.mustache.RenderSearchTemplateAction;
import org.opensearch.security.action.apitokens.ApiToken;
import org.opensearch.security.auditlog.AuditLog;
import org.opensearch.security.configuration.ClusterInfoHolder;
import org.opensearch.security.configuration.ConfigurationRepository;
Expand All @@ -110,8 +111,7 @@

import static org.opensearch.security.OpenSearchSecurityPlugin.traceAction;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_CLUSTERPERM_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXACTIONS_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDICES_KEY;
import static org.opensearch.security.filter.SecurityRestFilter.API_TOKEN_INDEXPERM_KEY;
import static org.opensearch.security.support.ConfigConstants.OPENDISTRO_SECURITY_USER_INFO_THREAD_CONTEXT;

public class PrivilegesEvaluator {
Expand Down Expand Up @@ -354,21 +354,11 @@ public PrivilegesEvaluatorResponse evaluate(PrivilegesEvaluationContext context)
context.setClusterPermissions(clusterPermissions);

Check warning on line 354 in src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java#L353-L354

Added lines #L353 - L354 were not covered by tests
}

final String apiTokenIndexAllowedActions = threadContext.getTransient(API_TOKEN_INDEXACTIONS_KEY);
if (apiTokenIndexAllowedActions != null) {
List<String> allowedactions = Arrays.asList(apiTokenIndexAllowedActions.split(","));
context.setAllowedActions(allowedactions);
final List<ApiToken.IndexPermission> apiTokenIndexPermissions = threadContext.getTransient(API_TOKEN_INDEXPERM_KEY);
if (apiTokenIndexPermissions != null) {
context.setIndexPermissions(apiTokenIndexPermissions);

Check warning on line 359 in src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/privileges/PrivilegesEvaluator.java#L359

Added line #L359 was not covered by tests
}

final String apiTokenIndices = threadContext.getTransient(API_TOKEN_INDICES_KEY);
if (apiTokenIndices != null) {
List<String> indices = Arrays.asList(apiTokenIndices.split(","));
context.setIndices(indices);
}

log.info("API Tokens actions" + apiTokenIndexAllowedActions);
log.info("API Tokens indices" + apiTokenIndices);

// Add the security roles for this user so that they can be used for DLS parameter substitution.
user.addSecurityRoles(mappedRoles);
setUserInfoInThreadContext(user);
Expand Down
Loading

0 comments on commit 22cfbe8

Please sign in to comment.