From b85cfd9ac5b2a02bc375847607fbb9433169a0df Mon Sep 17 00:00:00 2001 From: Vishnutheep B Date: Thu, 18 Jun 2026 22:52:03 +0530 Subject: [PATCH 1/3] Allow super-admins to create roles with restapi:admin/* permissions Signed-off-by: Vishnutheep B --- .../api/RestApiAuthorizationEvaluator.java | 5 ++ .../rest/validation/EndpointValidator.java | 7 +++ .../api/RolesApiActionValidationTest.java | 47 +++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAuthorizationEvaluator.java b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAuthorizationEvaluator.java index fdb7bc11af..454bc95678 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAuthorizationEvaluator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/api/RestApiAuthorizationEvaluator.java @@ -255,6 +255,11 @@ public boolean isCurrentUserAdminFor(final Endpoint endpoint, final String actio return hasAccess && restapiAdminEnabled; } + public boolean isCurrentUserSuperAdmin() { + final Pair userAndRemoteAddress = Utils.userAndRemoteAddressFrom(threadContext); + return userAndRemoteAddress.getLeft() != null && adminDNs.isAdmin(userAndRemoteAddress.getLeft()); + } + public boolean isCurrentUserAdminFor(final Endpoint endpoint) { return isCurrentUserAdminFor(endpoint, null); } diff --git a/src/main/java/org/opensearch/security/dlic/rest/validation/EndpointValidator.java b/src/main/java/org/opensearch/security/dlic/rest/validation/EndpointValidator.java index c60926846c..1683e278e2 100644 --- a/src/main/java/org/opensearch/security/dlic/rest/validation/EndpointValidator.java +++ b/src/main/java/org/opensearch/security/dlic/rest/validation/EndpointValidator.java @@ -30,6 +30,10 @@ public interface EndpointValidator { RestApiAuthorizationEvaluator restApiAuthorizationEvaluator(); + default boolean isCurrentUserSuperAdmin() { + return restApiAuthorizationEvaluator().isCurrentUserSuperAdmin(); + } + private String resourceName() { if (Objects.isNull(endpoint())) { return ""; @@ -156,6 +160,9 @@ default ValidationResult> validateRoles( default ValidationResult isAllowedToChangeEntityWithRestAdminPermissions( final SecurityConfiguration securityConfiguration ) throws IOException { + if (isCurrentUserSuperAdmin()) { + return ValidationResult.success(securityConfiguration); + } final var configuration = securityConfiguration.configuration(); if (securityConfiguration.entityExists()) { final var existingEntity = configuration.getCEntry(securityConfiguration.entityName()); diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java index a5b6e4b267..1d57005ded 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java @@ -14,6 +14,7 @@ import org.junit.Test; import org.opensearch.core.rest.RestStatus; +import org.opensearch.security.securityconf.impl.CType; import org.opensearch.security.securityconf.impl.v7.RoleV7; import org.mockito.Mockito; @@ -39,6 +40,52 @@ public void isAllowedToChangeImmutableEntity() throws Exception { assertTrue(result.isValid()); } + @Test + public void superAdminIsAllowedToCreateRoleWithRestAdminPermissions() throws Exception { + when(restApiAuthorizationEvaluator.isCurrentUserSuperAdmin()).thenReturn(true); + + final var role = objectMapper.createObjectNode(); + final var clusterPermissions = objectMapper.createArrayNode(); + clusterPermissions.add("restapi:admin/actiongroups"); + clusterPermissions.add("restapi:admin/roles"); + clusterPermissions.add("restapi:admin/rolesmapping"); + role.set("cluster_permissions", clusterPermissions); + final var rolesApiActionEndpointValidator = new RolesApiAction(clusterService, threadPool, securityApiDependencies) + .createEndpointValidator(); + assertTrue(rolesApiActionEndpointValidator.isCurrentUserSuperAdmin()); + + final var result = rolesApiActionEndpointValidator.isAllowedToChangeImmutableEntity( + SecurityConfiguration.of(role, "test_role", configuration) + ); + + assertTrue(result.isValid()); + } + + @Test + public void nonSuperAdminIsNotAllowedToCreateRoleWithRestAdminPermissions() throws Exception { + when(restApiAuthorizationEvaluator.isCurrentUserSuperAdmin()).thenReturn(false); + Mockito.doReturn(CType.ROLES).when(configuration).getCType(); + when(configuration.getImplementingClass()).thenCallRealMethod(); + when(restApiAuthorizationEvaluator.containsRestApiAdminPermissions(any(Object.class))).thenCallRealMethod(); + + final var role = objectMapper.createObjectNode(); + final var clusterPermissions = objectMapper.createArrayNode(); + clusterPermissions.add("restapi:admin/actiongroups"); + clusterPermissions.add("restapi:admin/roles"); + clusterPermissions.add("restapi:admin/rolesmapping"); + role.set("cluster_permissions", clusterPermissions); + final var rolesApiActionEndpointValidator = new RolesApiAction(clusterService, threadPool, securityApiDependencies) + .createEndpointValidator(); + assertFalse(rolesApiActionEndpointValidator.isCurrentUserSuperAdmin()); + + final var result = rolesApiActionEndpointValidator.isAllowedToChangeImmutableEntity( + SecurityConfiguration.of(role, "test_role", configuration) + ); + + assertFalse(result.isValid()); + assertThat(result.status(), is(RestStatus.FORBIDDEN)); + } + @Test public void isNotAllowedRightsToChangeImmutableEntity() throws Exception { final var role = new RoleV7(); From d4caeab0a51b10b656017a8b57c2b8e97f6c21ef Mon Sep 17 00:00:00 2001 From: Vishnutheep B Date: Thu, 18 Jun 2026 23:09:26 +0530 Subject: [PATCH 2/3] Fix spotless check Signed-off-by: Vishnutheep B --- .../security/dlic/rest/api/RolesApiActionValidationTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java index 1d57005ded..ebdc539ee7 100644 --- a/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java +++ b/src/test/java/org/opensearch/security/dlic/rest/api/RolesApiActionValidationTest.java @@ -60,7 +60,7 @@ public void superAdminIsAllowedToCreateRoleWithRestAdminPermissions() throws Exc assertTrue(result.isValid()); } - + @Test public void nonSuperAdminIsNotAllowedToCreateRoleWithRestAdminPermissions() throws Exception { when(restApiAuthorizationEvaluator.isCurrentUserSuperAdmin()).thenReturn(false); From e97094c7be375a24b2c629b9d2a93635491f2117 Mon Sep 17 00:00:00 2001 From: Vishnutheep B Date: Mon, 27 Jul 2026 22:34:23 +0530 Subject: [PATCH 3/3] Fix and add integration tests Signed-off-by: Vishnutheep B --- ...bstractConfigEntityApiIntegrationTest.java | 26 +++++++++-- .../ActionGroupsRestApiIntegrationTest.java | 19 ++++++++ .../RolesMappingRestApiIntegrationTest.java | 27 ++++++++++- .../api/RolesRestApiIntegrationTest.java | 45 ++++++++++++++++++- 4 files changed, 112 insertions(+), 5 deletions(-) diff --git a/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java index 910ec08b49..0d69dbd36b 100644 --- a/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/AbstractConfigEntityApiIntegrationTest.java @@ -141,17 +141,33 @@ Pair predefinedHiddenAndReservedConfigEntities(LocalCluster loca public void availableForTLSAdminUser(LocalCluster localCluster) throws Exception { try (TestRestClient client = localCluster.getAdminCertRestClient()) { - availableForSuperAdminUser(client); + verifySuperAdminCanManageRestAdminPermissions(client); } } public void availableForRESTAdminUser(LocalCluster localCluster) throws Exception { try (TestRestClient client = localCluster.getRestClient(REST_ADMIN_USER)) { - availableForSuperAdminUser(client); + verifyRestAdminCannotManageRestAdminPermissions(client); } } - void availableForSuperAdminUser(final TestRestClient client) throws Exception { + // mTLS super-admin bypasses the restapi:admin/* permission guard — can create/modify/delete freely + private void verifySuperAdminCanManageRestAdminPermissions(final TestRestClient client) throws Exception { + creationOfReadOnlyEntityForbidden( + randomAlphanumericString(), + client, + (builder, params) -> testDescriptor.staticEntityPayload().toXContent(builder, params) + ); + verifyCrudOperations(true, null, client); + verifyCrudOperations(null, true, client); + verifyCrudOperations(null, null, client); + verifyBadRequestOperations(client); + verifySuperAdminCanCreateEntityWithRestAdminPermissions(client); + verifySuperAdminCanUpdateAndDeleteEntityWithRestAdminPermissions(client); + } + + // REST admin user does NOT bypass the guard — still blocked from touching restapi:admin/* entities + private void verifyRestAdminCannotManageRestAdminPermissions(final TestRestClient client) throws Exception { creationOfReadOnlyEntityForbidden( randomAlphanumericString(), client, @@ -276,6 +292,10 @@ void forbiddenToCreateEntityWithRestAdminPermissions(final TestRestClient client void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {} + void verifySuperAdminCanCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {} + + void verifySuperAdminCanUpdateAndDeleteEntityWithRestAdminPermissions(final TestRestClient client) throws Exception {} + abstract void verifyBadRequestOperations(final TestRestClient client) throws Exception; abstract void verifyCrudOperations(final Boolean hidden, final Boolean reserved, final TestRestClient client) throws Exception; diff --git a/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java index 269d8a7503..f790c6abaf 100644 --- a/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/ActionGroupsRestApiIntegrationTest.java @@ -163,6 +163,25 @@ void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final Test assertThat(client.delete(apiPath(REST_ADMIN_PERMISSION_ACTION_GROUP)), isForbidden()); } + @Override + void verifySuperAdminCanCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { + assertThat(client.putJson(apiPath("new_rest_admin_action_group"), actionGroup(randomRestAdminPermission())), isCreated()); + assertThat( + client.patch(apiPath(), patch(addOp("new_rest_admin_action_group_2", actionGroup(randomRestAdminPermission())))), + isOk() + ); + } + + @Override + void verifySuperAdminCanUpdateAndDeleteEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { + final var tempGroup = "temp_rest_admin_action_group"; + assertThat(client.putJson(apiPath(tempGroup), actionGroup(randomRestAdminPermission())), isCreated()); + assertThat(client.putJson(apiPath(tempGroup), actionGroup("a", "b")), isOk()); + assertThat(client.patch(apiPath(), patch(replaceOp(tempGroup, actionGroup("a", "b")))), isOk()); + assertThat(client.patch(apiPath(tempGroup), patch(replaceOp("allowed_actions", configJsonArray("c", "d")))), isOk()); + assertThat(client.delete(apiPath(tempGroup)), isOk()); + } + @Override void verifyCrudOperations(final Boolean hidden, final Boolean reserved, final TestRestClient client) throws Exception { // create diff --git a/src/integrationTest/java/org/opensearch/security/api/RolesMappingRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/RolesMappingRestApiIntegrationTest.java index 748781f6de..b07aba9df8 100644 --- a/src/integrationTest/java/org/opensearch/security/api/RolesMappingRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/RolesMappingRestApiIntegrationTest.java @@ -25,6 +25,7 @@ import org.opensearch.security.DefaultObjectMapper; import org.opensearch.security.dlic.rest.api.Endpoint; import org.opensearch.test.framework.TestSecurityConfig; +import org.opensearch.test.framework.TestSecurityConfig.Role; import org.opensearch.test.framework.cluster.LocalCluster; import org.opensearch.test.framework.cluster.TestRestClient; import org.opensearch.test.framework.cluster.TestRestClient.HttpResponse; @@ -38,7 +39,6 @@ import static org.opensearch.security.api.PatchPayloadHelper.patch; import static org.opensearch.security.api.PatchPayloadHelper.removeOp; import static org.opensearch.security.api.PatchPayloadHelper.replaceOp; -import static org.opensearch.test.framework.TestSecurityConfig.Role; import static org.opensearch.test.framework.matcher.RestMatchers.isBadRequest; import static org.opensearch.test.framework.matcher.RestMatchers.isCreated; import static org.opensearch.test.framework.matcher.RestMatchers.isForbidden; @@ -498,6 +498,31 @@ void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(TestRestCl assertThat(client.delete(apiPath(REST_ADMIN_ROLE_WITH_MAPPING)), isForbidden()); } + @Override + void verifySuperAdminCanCreateEntityWithRestAdminPermissions(TestRestClient client) throws Exception { + final var users = arrayOptions(false).get(0); + assertThat(client.putJson(apiPath(REST_ADMIN_ROLE), roleMappingWithUsers(users)), isCreated()); + assertThat(client.patch(apiPath(), patch(replaceOp(REST_ADMIN_ROLE, roleMappingWithUsers(users)))), isOk()); + } + + @Override + void verifySuperAdminCanUpdateAndDeleteEntityWithRestAdminPermissions(TestRestClient client) throws Exception { + final var users = arrayOptions(false).get(0); + assertThat(client.putJson(apiPath(REST_ADMIN_ROLE_WITH_MAPPING), roleMapping(users, users, users, users)), isOk()); + assertThat( + client.patch(apiPath(), patch(replaceOp(REST_ADMIN_ROLE_WITH_MAPPING, roleMapping(users, users, users, users)))), + isOk() + ); + assertThat(client.patch(apiPath(REST_ADMIN_ROLE_WITH_MAPPING), patch(replaceOp("users", users))), isOk()); + assertThat( + client.putJson( + apiPath(REST_ADMIN_ROLE_WITH_MAPPING), + roleMapping(configJsonArray(), configJsonArray(), configJsonArray(), configJsonArray()) + ), + isOk() + ); + } + List jsonProperties() { return List.of("backend_roles", "hosts", "users", "and_backend_roles"); } diff --git a/src/integrationTest/java/org/opensearch/security/api/RolesRestApiIntegrationTest.java b/src/integrationTest/java/org/opensearch/security/api/RolesRestApiIntegrationTest.java index d8ad1a357e..801e693fb8 100644 --- a/src/integrationTest/java/org/opensearch/security/api/RolesRestApiIntegrationTest.java +++ b/src/integrationTest/java/org/opensearch/security/api/RolesRestApiIntegrationTest.java @@ -268,7 +268,7 @@ void verifyBadRequestOperations(TestRestClient client) throws Exception { void forbiddenToCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { assertThat(client.putJson(apiPath("new_rest_admin_role"), roleWithClusterPermissions(randomRestAdminPermission())), isForbidden()); assertThat( - client.patch(apiPath(), patch(addOp("new_rest_admin_action_group", roleWithClusterPermissions(randomRestAdminPermission())))), + client.patch(apiPath(), patch(addOp("new_rest_admin_role", roleWithClusterPermissions(randomRestAdminPermission())))), isForbidden() ); } @@ -312,6 +312,49 @@ void forbiddenToUpdateAndDeleteExistingEntityWithRestAdminPermissions(final Test assertThat(client.delete(apiPath(REST_ADMIN_PERMISSION_ROLE)), isForbidden()); } + @Override + void verifySuperAdminCanCreateEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { + assertThat(client.putJson(apiPath("new_rest_admin_role"), roleWithClusterPermissions(randomRestAdminPermission())), isCreated()); + assertThat( + client.patch(apiPath(), patch(addOp("new_rest_admin_role_2", roleWithClusterPermissions(randomRestAdminPermission())))), + isOk() + ); + } + + @Override + void verifySuperAdminCanUpdateAndDeleteEntityWithRestAdminPermissions(final TestRestClient client) throws Exception { + final var tempRole = "temp_rest_admin_role"; + assertThat(client.putJson(apiPath(tempRole), roleWithClusterPermissions(randomRestAdminPermission())), isCreated()); + assertThat( + client.putJson( + apiPath(tempRole), + role(clusterPermissionsOptions(false).get(0), indexPermissionsOptions(false).get(0), tenantPermissionsOptions(false).get(0)) + ), + isOk() + ); + assertThat( + client.patch( + apiPath(), + patch( + replaceOp( + tempRole, + role( + clusterPermissionsOptions(false).get(0), + indexPermissionsOptions(false).get(0), + tenantPermissionsOptions(false).get(0) + ) + ) + ) + ), + isOk() + ); + assertThat( + client.patch(apiPath(tempRole), patch(replaceOp("cluster_permissions", clusterPermissionsOptions(false).get(0)))), + isOk() + ); + assertThat(client.delete(apiPath(tempRole)), isOk()); + } + void assertRole( final TestRestClient.HttpResponse response, final String roleName,