Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OnBehalfOf tokens odds and ends #3593

Merged
merged 4 commits into from
Oct 27, 2023
Merged

OnBehalfOf tokens odds and ends #3593

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
48eb41e
OnBehalfOf tokens odds and ends
peternied Oct 24, 2023
eb1b555
Spotless fixes
peternied Oct 24, 2023
2ed0d7c
Fix test failures due to parameter inversion
peternied Oct 26, 2023
413b2a2
Merge remote-tracking branch 'origin/main' into obo-odds-and-ends
peternied Oct 26, 2023
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
2 changes: 1 addition & 1 deletion src/integrationTest/java/org/opensearch/security/DefaultConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ public void shouldLoadDefaultConfiguration() {
Awaitility.await().alias("Load default configuration").until(() -> client.getAuthInfo().getStatusCode(), equalTo(200));
}
try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
client.assertCorrectCredentials(ADMIN_USER_NAME);
client.confirmCorrectCredentials(ADMIN_USER_NAME);
HttpResponse response = client.get("/_plugins/_security/api/internalusers");
response.assertStatusCode(200);
Map<String, Object> users = response.getBodyAs(Map.class);
Expand Down
12 changes: 6 additions & 6 deletions src/integrationTest/java/org/opensearch/security/SecurityConfigurationTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,10 +97,10 @@ public void shouldCreateUserViaRestApi_success() {
assertThat(httpResponse.getStatusCode(), equalTo(201));
}
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
client.assertCorrectCredentials(USER_ADMIN.getName());
client.confirmCorrectCredentials(USER_ADMIN.getName());
}
try (TestRestClient client = cluster.getRestClient(ADDITIONAL_USER_1, ADDITIONAL_PASSWORD_1)) {
client.assertCorrectCredentials(ADDITIONAL_USER_1);
client.confirmCorrectCredentials(ADDITIONAL_USER_1);
}
}

Expand Down Expand Up @@ -160,10 +160,10 @@ public void shouldCreateUserViaRestApiWhenAdminIsAuthenticatedViaCertificate_pos
httpResponse.assertStatusCode(201);
}
try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
client.assertCorrectCredentials(USER_ADMIN.getName());
client.confirmCorrectCredentials(USER_ADMIN.getName());
}
try (TestRestClient client = cluster.getRestClient(ADDITIONAL_USER_2, ADDITIONAL_PASSWORD_2)) {
client.assertCorrectCredentials(ADDITIONAL_USER_2);
client.confirmCorrectCredentials(ADDITIONAL_USER_2);
}
}

Expand All @@ -189,10 +189,10 @@ public void shouldStillWorkAfterUpdateOfSecurityConfig() {
cluster.updateUserConfiguration(users);

try (TestRestClient client = cluster.getRestClient(USER_ADMIN)) {
client.assertCorrectCredentials(USER_ADMIN.getName());
client.confirmCorrectCredentials(USER_ADMIN.getName());
}
try (TestRestClient client = cluster.getRestClient(newUser)) {
client.assertCorrectCredentials(newUser.getName());
client.confirmCorrectCredentials(newUser.getName());
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/integrationTest/java/org/opensearch/security/http/CertificateAuthenticationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ public void shouldAuthenticateUserWithCertificate_positiveUserSpoke() {
CertificateData userSpockCertificate = TEST_CERTIFICATES.issueUserCertificate(BACKEND_ROLE_BRIDGE, USER_SPOCK);
try (TestRestClient client = cluster.getRestClient(userSpockCertificate)) {

client.assertCorrectCredentials(USER_SPOCK);
client.confirmCorrectCredentials(USER_SPOCK);
}
}

Expand All @@ -98,7 +98,7 @@ public void shouldAuthenticateUserWithCertificate_positiveUserKirk() {
CertificateData userSpockCertificate = TEST_CERTIFICATES.issueUserCertificate(BACKEND_ROLE_BRIDGE, USER_KIRK);
try (TestRestClient client = cluster.getRestClient(userSpockCertificate)) {

client.assertCorrectCredentials(USER_KIRK);
client.confirmCorrectCredentials(USER_KIRK);
}
}

Expand Down
47 changes: 22 additions & 25 deletions src/integrationTest/java/org/opensearch/security/http/OnBehalfOfJwtAuthenticationTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
import java.util.Base64;
import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;

import com.carrotsearch.randomizedtesting.annotations.ThreadLeakScope;
Expand All @@ -24,7 +25,6 @@
import org.apache.hc.core5.http.Header;
import org.apache.hc.core5.http.HttpStatus;
import org.apache.hc.core5.http.message.BasicHeader;
import org.junit.Assert;
import org.junit.ClassRule;
import org.junit.Test;
import org.junit.runner.RunWith;
Expand All @@ -38,11 +38,10 @@
import org.opensearch.test.framework.cluster.TestRestClient;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.aMapWithSize;
import static org.hamcrest.Matchers.allOf;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.hasKey;
import static org.junit.Assert.assertTrue;
import static org.hamcrest.Matchers.notNullValue;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.contains;
import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_RESTAPI_ROLES_ENABLED;
import static org.opensearch.test.framework.TestSecurityConfig.AuthcDomain.AUTHC_HTTPBASIC_INTERNAL;
Expand All @@ -56,6 +55,7 @@ public class OnBehalfOfJwtAuthenticationTest {

static final TestSecurityConfig.User ADMIN_USER = new TestSecurityConfig.User("admin").roles(ALL_ACCESS);

private static final String CREATE_OBO_TOKEN_PATH = "_plugins/_security/api/generateonbehalfoftoken";
private static Boolean oboEnabled = true;
private static final String signingKey = Base64.getEncoder()
.encodeToString(
Expand Down Expand Up @@ -139,7 +139,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessOBOEndpoint() {
Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken);

try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) {
TestRestClient.HttpResponse response = client.getOnBehalfOfToken(OBO_DESCRIPTION, adminOboAuthHeader);
TestRestClient.HttpResponse response = client.postJson(CREATE_OBO_TOKEN_PATH, OBO_DESCRIPTION);
response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED);
}
}
Expand All @@ -150,7 +150,7 @@ public void shouldNotAuthenticateForUsingOBOTokenToAccessAccountEndpoint() {
Header adminOboAuthHeader = new BasicHeader("Authorization", "Bearer " + oboToken);

try (TestRestClient client = cluster.getRestClient(adminOboAuthHeader)) {
TestRestClient.HttpResponse response = client.changeInternalUserPassword(CURRENT_AND_NEW_PASSWORDS, adminOboAuthHeader);
TestRestClient.HttpResponse response = client.putJson("_plugins/_security/api/account", CURRENT_AND_NEW_PASSWORDS);
response.assertStatusCode(HttpStatus.SC_UNAUTHORIZED);
}
}
Expand All @@ -173,52 +173,49 @@ public void shouldNotAuthenticateForNonAdminUserWithoutOBOPermission() {
public void shouldNotIncludeRolesFromHostMappingInOBOToken() {
String oboToken = generateOboToken(OBO_USER_NAME_WITH_HOST_MAPPING, DEFAULT_PASSWORD);

Claims claims = Jwts.parserBuilder().setSigningKey(signingKey).build().parseClaimsJws(oboToken).getBody();
Claims claims = Jwts.parserBuilder().setSigningKey(Base64.getDecoder().decode(signingKey)).build().parseClaimsJws(oboToken).getBody();

Object er = claims.get("er");
EncryptionDecryptionUtil encryptionDecryptionUtil = new EncryptionDecryptionUtil(encryptionKey);
String rolesClaim = encryptionDecryptionUtil.decrypt(er.toString());
List<String> roles = Arrays.stream(rolesClaim.split(","))
Set<String> roles = Arrays.stream(rolesClaim.split(","))
.map(String::trim)
.filter(s -> !s.isEmpty())
.collect(Collectors.toUnmodifiableList());
.collect(Collectors.toSet());

Assert.assertFalse(roles.contains("host_mapping_role"));
assertThat(roles, equalTo(HOST_MAPPING_OBO_USER.getRoleNames()));
assertThat(roles, not(contains("host_mapping_role")));
}

@Test
public void shouldNotAuthenticateWithInvalidDurationSeconds() {
try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
client.assertCorrectCredentials(ADMIN_USER_NAME);
client.confirmCorrectCredentials(ADMIN_USER_NAME);
TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_DESCRIPTION_WITH_INVALID_DURATIONSECONDS);
response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
assertTrue(oboEndPointResponse.containsValue("durationSeconds must be an integer."));
assertThat(response.getTextFromJsonBody("/error"), equalTo("durationSeconds must be an integer."));
}
}

@Test
public void shouldNotAuthenticateWithInvalidAPIParameter() {
try (TestRestClient client = cluster.getRestClient(ADMIN_USER_NAME, DEFAULT_PASSWORD)) {
client.assertCorrectCredentials(ADMIN_USER_NAME);
client.confirmCorrectCredentials(ADMIN_USER_NAME);
TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_DESCRIPTION_WITH_INVALID_PARAMETERS);
response.assertStatusCode(HttpStatus.SC_BAD_REQUEST);
Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
assertTrue(oboEndPointResponse.containsValue("Unrecognized parameter: invalidParameter"));
}
assertThat(response.getTextFromJsonBody("/error"), equalTo("Unrecognized parameter: invalidParameter"));
}
}

private String generateOboToken(String username, String password) {
try (TestRestClient client = cluster.getRestClient(username, password)) {
client.assertCorrectCredentials(username);
client.confirmCorrectCredentials(username);
TestRestClient.HttpResponse response = client.postJson(OBO_ENDPOINT_PREFIX, OBO_TOKEN_REASON);
response.assertStatusCode(HttpStatus.SC_OK);
Map<String, Object> oboEndPointResponse = (Map<String, Object>) response.getBodyAs(Map.class);
assertThat(
oboEndPointResponse,
allOf(aMapWithSize(3), hasKey("user"), hasKey("authenticationToken"), hasKey("durationSeconds"))
);
return oboEndPointResponse.get("authenticationToken").toString();
assertThat(response.getTextFromJsonBody("/user"), notNullValue());
assertThat(response.getTextFromJsonBody("/authenticationToken"), notNullValue());
assertThat(response.getTextFromJsonBody("/durationSeconds"), notNullValue());
return response.getTextFromJsonBody("/authenticationToken").toString();
}
}

Expand Down
24 changes: 1 addition & 23 deletions src/integrationTest/java/org/opensearch/test/framework/cluster/TestRestClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,29 +125,7 @@ public HttpResponse getAuthInfo(Header... headers) {
return executeRequest(new HttpGet(getHttpServerUri() + "/_opendistro/_security/authinfo?pretty"), headers);
}

public HttpResponse getOnBehalfOfToken(String jsonData, Header... headers) {
try {
HttpPost httpPost = new HttpPost(
new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/generateonbehalfoftoken?pretty").build()
);
httpPost.setEntity(new StringEntity(jsonData));
return executeRequest(httpPost, mergeHeaders(CONTENT_TYPE_JSON, headers));
} catch (URISyntaxException ex) {
throw new RuntimeException("Incorrect URI syntax", ex);
}
}

public HttpResponse changeInternalUserPassword(String jsonData, Header... headers) {
try {
HttpPut httpPut = new HttpPut(new URIBuilder(getHttpServerUri() + "/_plugins/_security/api/account?pretty").build());
httpPut.setEntity(new StringEntity(jsonData));
return executeRequest(httpPut, mergeHeaders(CONTENT_TYPE_JSON, headers));
} catch (URISyntaxException ex) {
throw new RuntimeException("Incorrect URI syntax", ex);
}
}

public void assertCorrectCredentials(String expectedUserName) {
public void confirmCorrectCredentials(String expectedUserName) {
HttpResponse response = getAuthInfo();
assertThat(response, notNullValue());
response.assertStatusCode(200);
Expand Down
32 changes: 16 additions & 16 deletions src/main/java/org/opensearch/security/action/onbehalf/CreateOnBehalfOfTokenAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,28 +58,20 @@ public class CreateOnBehalfOfTokenAction extends BaseRestHandler {

private ConfigModel configModel;

private DynamicConfigModel dcm;

public static final Integer OBO_DEFAULT_EXPIRY_SECONDS = 5 * 60;
public static final Integer OBO_MAX_EXPIRY_SECONDS = 10 * 60;

public static final String DEFAULT_SERVICE = "self-issued";

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

private static final Set<String> RECOGNIZED_PARAMS = new HashSet<>(
Arrays.asList("durationSeconds", "description", "roleSecurityMode", "service")
);

@Subscribe
public void onConfigModelChanged(ConfigModel configModel) {
this.configModel = configModel;
}

@Subscribe
public void onDynamicConfigModelChanged(DynamicConfigModel dcm) {
this.dcm = dcm;

Settings settings = dcm.getDynamicOnBehalfOfSettings();

Boolean enabled = Boolean.parseBoolean(settings.get("enabled"));
Expand Down Expand Up @@ -146,10 +138,6 @@ public void accept(RestChannel channel) throws Exception {

final String description = (String) requestBody.getOrDefault("description", null);

final Boolean roleSecurityMode = Optional.ofNullable(requestBody.get("roleSecurityMode"))
.map(value -> (Boolean) value)
.orElse(true); // Default to false if null

final String service = (String) requestBody.getOrDefault("service", DEFAULT_SERVICE);
final User user = threadPool.getThreadContext().getTransient(ConfigConstants.OPENDISTRO_SECURITY_USER);
Set<String> mappedRoles = mapRoles(user);
Expand All @@ -164,7 +152,7 @@ public void accept(RestChannel channel) throws Exception {
tokenDuration,
mappedRoles.stream().collect(Collectors.toList()),
user.getRoles().stream().collect(Collectors.toList()),
roleSecurityMode
false
);
builder.field("authenticationToken", token);
builder.field("durationSeconds", tokenDuration);
Expand All @@ -187,15 +175,27 @@ public void accept(RestChannel channel) throws Exception {
};
}

private enum InputParameters {
DURATION("durationSeconds"),
DESCRIPTION("description"),
SERVICE("service");

final String paramName;
private InputParameters(final String paramName) {
this.paramName = paramName;
}
}

private Set<String> mapRoles(final User user) {
return this.configModel.mapSecurityRoles(user, null);
}

private void validateRequestParameters(Map<String, Object> requestBody) throws IllegalArgumentException {
for (String key : requestBody.keySet()) {
if (!RECOGNIZED_PARAMS.contains(key)) {
throw new IllegalArgumentException("Unrecognized parameter: " + key);
}
Arrays.stream(InputParameters.values())
.filter(param -> param.paramName.equalsIgnoreCase(key))
.findAny()
.orElseThrow(() -> new IllegalArgumentException("Unrecognized parameter: " + key));
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/opensearch/security/authtoken/jwt/JwtVendor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public String createJwt(
Integer expirySeconds,
List<String> roles,
List<String> backendRoles,
boolean roleSecurityMode
boolean includeBackendRoles
) throws Exception {
final long nowAsMillis = timeProvider.getAsLong();
final Instant nowAsInstant = Instant.ofEpochMilli(timeProvider.getAsLong());
Expand Down Expand Up @@ -148,7 +148,7 @@ public String createJwt(
throw new Exception("Roles cannot be null");
}

if (!roleSecurityMode && backendRoles != null) {
if (includeBackendRoles && backendRoles != null) {
String listOfBackendRoles = String.join(",", backendRoles);
jwtClaims.setProperty("br", listOfBackendRoles);
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/java/org/opensearch/security/authtoken/jwt/JwtVendorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ public void testCreateJwtWithRoles() throws Exception {
}

@Test
public void testCreateJwtWithRoleSecurityMode() throws Exception {
public void testCreateJwtWithBackendRolesIncluded() throws Exception {
String issuer = "cluster_0";
String subject = "admin";
String audience = "audience_0";
Expand Down