Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
- Github workflow for changelog verification ([#5318](https://github.com/opensearch-project/security/pull/5318))
- Register cluster settings listener for `plugins.security.cache.ttl_minutes` ([#5324](https://github.com/opensearch-project/security/pull/5324))
- Add flush cache endpoint for individual user ([#5337](https://github.com/opensearch-project/security/pull/5337))
- Handle roles in nested claim for JWT auth backends ([#5355](https://github.com/opensearch-project/security/pull/5355))

### Changed
- Use extendedPlugins in integrationTest framework for sample resource plugin testing ([#5322](https://github.com/opensearch-project/security/pull/5322))
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
/*
* Copyright OpenSearch Contributors
* 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.security.http;

import java.security.KeyPair;
import java.util.Arrays;
import java.util.Base64;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
import org.apache.hc.core5.http.Header;
import org.junit.ClassRule;
import org.junit.Rule;
import org.junit.Test;
import org.junit.runner.RunWith;

import org.opensearch.test.framework.JwtConfigBuilder;
import org.opensearch.test.framework.TestSecurityConfig;
import org.opensearch.test.framework.cluster.ClusterManager;
import org.opensearch.test.framework.cluster.LocalCluster;
import org.opensearch.test.framework.cluster.TestRestClient;
import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse;
import org.opensearch.test.framework.log.LogsRule;

import io.jsonwebtoken.SignatureAlgorithm;
import io.jsonwebtoken.security.Keys;

import static java.nio.charset.StandardCharsets.US_ASCII;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasSize;
import static org.opensearch.security.http.JwtAuthenticationTests.POINTER_BACKEND_ROLES;
import static org.opensearch.security.http.JwtAuthenticationTests.POINTER_USERNAME;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.BASIC_AUTH_DOMAIN_ORDER;

@RunWith(com.carrotsearch.randomizedtesting.RandomizedRunner.class)
@ThreadLeakScope(ThreadLeakScope.Scope.NONE)
public class JwtAuthenticationNestedClaimsTests {

public static final String CLAIM_USERNAME = "preferred-username";
public static final List<String> CLAIM_ROLES = List.of("attributes", "roles");

public static final String USER_SUPERHERO = "superhero";
private static final KeyPair KEY_PAIR1 = Keys.keyPairFor(SignatureAlgorithm.RS256);
private static final String PUBLIC_KEY1 = new String(Base64.getEncoder().encode(KEY_PAIR1.getPublic().getEncoded()), US_ASCII);
private static final String JWT_AUTH_HEADER = "jwt-auth";

private static final JwtAuthorizationHeaderFactory tokenFactory1 = new JwtAuthorizationHeaderFactory(
KEY_PAIR1.getPrivate(),
CLAIM_USERNAME,
CLAIM_ROLES,
JWT_AUTH_HEADER
);
public static final TestSecurityConfig.AuthcDomain JWT_AUTH_DOMAIN = new TestSecurityConfig.AuthcDomain(
"jwt",
BASIC_AUTH_DOMAIN_ORDER - 1
).jwtHttpAuthenticator(
new JwtConfigBuilder().jwtHeader(JWT_AUTH_HEADER).signingKey(List.of(PUBLIC_KEY1)).subjectKey(CLAIM_USERNAME).rolesKey(CLAIM_ROLES)
).backend("noop");

@ClassRule
public static final LocalCluster cluster = new LocalCluster.Builder().clusterManager(ClusterManager.SINGLENODE)
.anonymousAuth(false)
.authc(JWT_AUTH_DOMAIN)
.build();

@Rule
public LogsRule logsRule = new LogsRule("org.opensearch.security.auth.http.jwt.HTTPJwtAuthenticator");

// TODO write tests for scenarios where roles are in nested claim. i.e. rolesKey: ['attributes', 'roles']
@Test
public void shouldAuthenticateWithNestedRolesClaim() {
// Create nested claims structure
Map<String, Object> attributes = new HashMap<>();
List<String> rolesClaim = Arrays.asList("all_access", "securitymanager");
attributes.put("roles", rolesClaim);

Map<String, Object> nestedClaims = new HashMap<>();
nestedClaims.put("attributes", attributes);

// Generate token with nested claims
Header header = tokenFactory1.generateValidTokenWithCustomClaims(USER_SUPERHERO, null, nestedClaims);

try (TestRestClient client = cluster.getRestClient(header)) {
HttpResponse response = client.getAuthInfo();

response.assertStatusCode(200);
String username = response.getTextFromJsonBody(POINTER_USERNAME);
assertThat(username, equalTo(USER_SUPERHERO));
List<String> roles = response.getTextArrayFromJsonBody(POINTER_BACKEND_ROLES);
assertThat(roles, hasSize(2));
assertThat(roles, containsInAnyOrder("all_access", "securitymanager"));
}
}

@Test
public void shouldHandleMissingNestedRolesClaim() {
// Create invalid nested claims structure
Map<String, Object> attributes = new HashMap<>();
attributes.put("wrong", "missing"); // Invalid format - should be a list

Map<String, Object> nestedClaims = new HashMap<>();
nestedClaims.put("attributes", attributes);

Header header = tokenFactory1.generateValidTokenWithCustomClaims(USER_SUPERHERO, null, nestedClaims);

try (TestRestClient client = cluster.getRestClient(header)) {
HttpResponse response = client.getAuthInfo();

response.assertStatusCode(200);
String username = response.getTextFromJsonBody(POINTER_USERNAME);
assertThat(username, equalTo(USER_SUPERHERO));
List<String> roles = response.getTextArrayFromJsonBody(POINTER_BACKEND_ROLES);
assertThat(roles, hasSize(0));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@
public class JwtAuthenticationTests {

public static final String CLAIM_USERNAME = "preferred-username";
public static final String CLAIM_ROLES = "backend-user-roles";
public static final List<String> CLAIM_ROLES = List.of("backend-user-roles");

public static final String USER_SUPERHERO = "superhero";
public static final String USERNAME_ROOT = "root";
Expand Down Expand Up @@ -305,5 +305,4 @@ public void secondKeypairShouldAuthenticateWithJwtToken_positiveWithAnotherUsern
assertThat(username, equalTo(USERNAME_ROOT));
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
public class JwtAuthenticationWithUrlParamTests {

public static final String CLAIM_USERNAME = "preferred-username";
public static final String CLAIM_ROLES = "backend-user-roles";
public static final List<String> CLAIM_ROLES = List.of("backend-user-roles");
public static final String POINTER_USERNAME = "/user_name";

private static final KeyPair KEY_PAIR = Keys.keyPairFor(SignatureAlgorithm.RS256);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,10 @@
package org.opensearch.security.http;

import java.security.PrivateKey;
import java.util.Arrays;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import com.google.common.collect.ImmutableMap;
import org.apache.commons.lang3.StringUtils;
Expand All @@ -33,11 +32,11 @@ class JwtAuthorizationHeaderFactory {

private final String usernameClaimName;

private final String rolesClaimName;
private final List<String> rolesClaimName;

private final String headerName;

public JwtAuthorizationHeaderFactory(PrivateKey privateKey, String usernameClaimName, String rolesClaimName, String headerName) {
public JwtAuthorizationHeaderFactory(PrivateKey privateKey, String usernameClaimName, List<String> rolesClaimName, String headerName) {
this.privateKey = requireNonNull(privateKey, "Private key is required");
this.usernameClaimName = requireNonNull(usernameClaimName, "Username claim name is required");
this.rolesClaimName = requireNonNull(rolesClaimName, "Roles claim name is required.");
Expand All @@ -64,8 +63,28 @@ private Map<String, Object> customClaimsMap(String username, String[] roles) {
if (StringUtils.isNoneEmpty(username)) {
builder.put(usernameClaimName, username);
}
if ((roles != null) && (roles.length > 0)) {
builder.put(rolesClaimName, Arrays.stream(roles).collect(Collectors.joining(",")));
if (roles != null && roles.length > 0) {
if (rolesClaimName.size() == 1) {
// Simple case - no nesting
builder.put(rolesClaimName.get(0), String.join(",", roles));
} else {
// Handle nested claims
Map<String, Object> nestedMap = new HashMap<>();
Map<String, Object> currentMap = nestedMap;

// Build the nested structure
for (int i = 0; i < rolesClaimName.size() - 1; i++) {
Map<String, Object> nextMap = new HashMap<>();
currentMap.put(rolesClaimName.get(i), nextMap);
currentMap = nextMap;
}

// Add the roles array at the deepest level
currentMap.put(rolesClaimName.get(rolesClaimName.size() - 1), String.join(",", roles));

// Add the entire nested structure to the builder
builder.putAll(nestedMap);
}
}
return builder.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class JwtConfigBuilder {
private String jwtUrlParameter;
private List<String> signingKeys;
private String subjectKey;
private String rolesKey;
private List<String> rolesKey;

public JwtConfigBuilder jwtHeader(String jwtHeader) {
this.jwtHeader = jwtHeader;
Expand All @@ -45,6 +45,11 @@ public JwtConfigBuilder subjectKey(String subjectKey) {
}

public JwtConfigBuilder rolesKey(String rolesKey) {
this.rolesKey = List.of(rolesKey);
return this;
}

public JwtConfigBuilder rolesKey(List<String> rolesKey) {
this.rolesKey = rolesKey;
return this;
}
Expand All @@ -64,7 +69,7 @@ public Map<String, Object> build() {
if (isNoneBlank(subjectKey)) {
builder.put("subject_key", subjectKey);
}
if (isNoneBlank(rolesKey)) {
if (rolesKey != null && !rolesKey.isEmpty()) {
builder.put("roles_key", rolesKey);
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@
private final boolean isDefaultAuthHeader;
private final String jwtUrlParameter;
private final String subjectKey;
private final String rolesKey;
private final List<String> rolesKey;
private final List<String> requiredAudience;
private final String requiredIssuer;

Expand All @@ -72,7 +72,7 @@
jwtUrlParameter = settings.get("jwt_url_parameter");
jwtHeaderName = settings.get("jwt_header", AUTHORIZATION);
isDefaultAuthHeader = AUTHORIZATION.equalsIgnoreCase(jwtHeaderName);
rolesKey = settings.get("roles_key");
rolesKey = settings.getAsList("roles_key");
subjectKey = settings.get("subject_key");
clockSkewToleranceSeconds = settings.getAsInt("jwt_clock_skew_tolerance_seconds", DEFAULT_CLOCK_SKEW_TOLERANCE_SECONDS);
requiredAudience = settings.getAsList("required_audience");
Expand Down Expand Up @@ -219,7 +219,21 @@
return new String[0];
}

Object rolesObject = claims.getClaim(rolesKey);
Object rolesObject = null;
Map<String, Object> claimsMap = claims.getClaims();
for (int i = 0; i < rolesKey.size(); i++) {
if (i == rolesKey.size() - 1) {
rolesObject = claimsMap.get(rolesKey.get(i));
} else if (claimsMap.get(rolesKey.get(i)) instanceof Map) {
claimsMap = (Map<String, Object>) claimsMap.get(rolesKey.get(i));
} else {
log.warn(

Check warning on line 230 in src/main/java/org/opensearch/security/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L230

Added line #L230 was not covered by tests
"Failed to get roles from JWT claims with roles_key '{}'. Check if this key is correct and available in the JWT payload.",
rolesKey
);
return new String[0];

Check warning on line 234 in src/main/java/org/opensearch/security/auth/http/jwt/AbstractHTTPJwtAuthenticator.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/auth/http/jwt/AbstractHTTPJwtAuthenticator.java#L234

Added line #L234 was not covered by tests
}
}

if (rolesObject == null) {
log.warn(
Expand Down
Loading
Loading