You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by ji...@apache.org on 2026/08/26 14:35:10 UTC
(geode) branch support/1.15 updated: [GEODE-10620] Make named-query endpoint permissions consistent with region data endpoints (#8045)
This is an automated email from the ASF dual-hosted git repository.
JinwooHwang pushed a commit to branch support/1.15
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/support/1.15 by this push:
new 99a04bd842 [GEODE-10620] Make named-query endpoint permissions consistent with region data endpoints (#8045)
99a04bd842 is described below
commit 99a04bd8426b0d917ccd0ff092688ba7a05c7c2f
Author: Jinwoo Hwang <92...@users.noreply.github.com>
AuthorDate: Wed Aug 26 10:35:04 2026 -0400
[GEODE-10620] Make named-query endpoint permissions consistent with region data endpoints (#8045)
---
.../internal/web/RestSecurityIntegrationTest.java | 4 +
.../web/RestSecurityPostProcessorTest.java | 2 +-
.../QueryAccessControllerAuthorizationTest.java | 193 +++++++++++++++++++++
.../web/controllers/QueryAccessController.java | 4 +-
4 files changed, 200 insertions(+), 3 deletions(-)
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
index 2a9bc83f35..7a0a97fe72 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java
@@ -107,6 +107,8 @@ public class RestSecurityIntegrationTest {
assertResponse(restClient.doPost("/queries?id=0&q=", "user", "user", ""))
.hasStatusCode(403);
assertResponse(restClient.doPost("/queries?id=0&q=", "dataRead", "dataRead", ""))
+ .hasStatusCode(403);
+ assertResponse(restClient.doPost("/queries?id=0&q=", "dataWrite", "dataWrite", ""))
.hasStatusCode(500);
}
@@ -127,6 +129,8 @@ public class RestSecurityIntegrationTest {
assertResponse(restClient.doPut("/queries/id", "user", "user", "{\"id\" : \"foo\"}"))
.hasStatusCode(403);
assertResponse(restClient.doPut("/queries/id", "dataRead", "dataRead", "{\"id\" : \"foo\"}"))
+ .hasStatusCode(403);
+ assertResponse(restClient.doPut("/queries/id", "dataWrite", "dataWrite", "{\"id\" : \"foo\"}"))
.hasStatusCode(404);
}
diff --git a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java
index a30855ac36..cd5a3fd730 100644
--- a/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java
+++ b/geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java
@@ -158,7 +158,7 @@ public class RestSecurityPostProcessorTest {
// Install the named query
assertResponse(
restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"),
- "dataReader", "1234567", ""))
+ "dataUser", "1234567", ""))
.hasStatusCode(201);
// Verify the query has been installed
diff --git a/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java
new file mode 100644
index 0000000000..1600799106
--- /dev/null
+++ b/geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java
@@ -0,0 +1,193 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more contributor license
+ * agreements. See the NOTICE file distributed with this work for additional information regarding
+ * copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License. You may obtain a
+ * copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software distributed under the License
+ * is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
+ * or implied. See the License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.geode.rest.internal.web.controllers;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
+import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
+import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
+import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
+
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.ClassRule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.http.HttpHeaders;
+import org.springframework.http.MediaType;
+import org.springframework.mock.web.MockHttpServletRequest;
+import org.springframework.test.context.ContextConfiguration;
+import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
+import org.springframework.test.context.web.GenericXmlWebContextLoader;
+import org.springframework.test.context.web.WebAppConfiguration;
+import org.springframework.test.context.web.WebMergedContextConfiguration;
+import org.springframework.test.web.servlet.MockMvc;
+import org.springframework.test.web.servlet.request.RequestPostProcessor;
+import org.springframework.test.web.servlet.setup.MockMvcBuilders;
+import org.springframework.web.context.WebApplicationContext;
+import org.springframework.web.context.support.GenericWebApplicationContext;
+
+import org.apache.geode.cache.Region;
+import org.apache.geode.cache.RegionShortcut;
+import org.apache.geode.cache.internal.HttpService;
+import org.apache.geode.examples.SimpleSecurityManager;
+import org.apache.geode.management.internal.RestAgent;
+import org.apache.geode.test.junit.rules.ServerStarterRule;
+
+/**
+ * Verifies the permissions the named-query endpoints of {@link QueryAccessController} require.
+ *
+ * <p>
+ * The endpoints that read query state require {@code DATA:READ}; the endpoints that create, update
+ * or remove a stored named query all require {@code DATA:WRITE}, matching the permission required
+ * for the equivalent operations on ordinary region data.
+ */
+@RunWith(SpringJUnit4ClassRunner.class)
+@ContextConfiguration(locations = {"classpath*:WEB-INF/geode-servlet.xml"},
+ loader = SecuredTestContextLoader.class)
+@WebAppConfiguration
+public class QueryAccessControllerAuthorizationTest {
+
+ private static final String QUERY_STORE = "__ParameterizedQueries__";
+ private static final String REGION_NAME = "customers";
+
+ private static final String READ_USER = "dataRead";
+ private static final String WRITE_USER = "dataWrite";
+
+ private static final String OQL = "SELECT * FROM " + Region.SEPARATOR + REGION_NAME;
+ private static final String OTHER_OQL =
+ "SELECT c.name FROM " + Region.SEPARATOR + REGION_NAME + " c";
+
+ private static final RequestPostProcessor JSON = new JsonRequestPostProcessor();
+
+ @ClassRule
+ public static ServerStarterRule rule = new ServerStarterRule()
+ .withProperty("log-level", "warn")
+ .withSecurityManager(SimpleSecurityManager.class)
+ .withRegion(RegionShortcut.REPLICATE, REGION_NAME);
+
+ @Autowired
+ private WebApplicationContext webApplicationContext;
+
+ private MockMvc mockMvc;
+
+ @BeforeClass
+ public static void createQueryStore() {
+ RestAgent.createParameterizedQueryRegion();
+ }
+
+ @Before
+ public void setUp() {
+ mockMvc = MockMvcBuilders.webAppContextSetup(webApplicationContext)
+ .apply(springSecurity())
+ .build();
+ queryStore().clear();
+ }
+
+ @SuppressWarnings("unchecked")
+ private static Region<String, String> queryStore() {
+ return rule.getCache().getInternalRegionByPath(Region.SEPARATOR + QUERY_STORE);
+ }
+
+ @Test
+ public void createIsRefusedForAUserWithoutWritePermission() throws Exception {
+ mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL)
+ .with(httpBasic(READ_USER, READ_USER))
+ .with(JSON))
+ .andExpect(status().isForbidden());
+
+ assertThat(queryStore()).doesNotContainKey("q1");
+ }
+
+ @Test
+ public void updateIsRefusedForAUserWithoutWritePermission() throws Exception {
+ queryStore().put("q1", OQL);
+
+ mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL)
+ .with(httpBasic(READ_USER, READ_USER))
+ .with(JSON))
+ .andExpect(status().isForbidden());
+
+ assertThat(queryStore().get("q1")).isEqualTo(OQL);
+ }
+
+ @Test
+ public void deleteIsRefusedForAUserWithoutWritePermission() throws Exception {
+ queryStore().put("q1", OQL);
+
+ mockMvc.perform(delete("/v1/queries/q1")
+ .with(httpBasic(READ_USER, READ_USER))
+ .with(JSON))
+ .andExpect(status().isForbidden());
+
+ assertThat(queryStore()).containsKey("q1");
+ }
+
+ @Test
+ public void createAndUpdateAreAllowedForAUserWithWritePermission() throws Exception {
+ mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL)
+ .with(httpBasic(WRITE_USER, WRITE_USER))
+ .with(JSON))
+ .andExpect(status().isCreated());
+
+ assertThat(queryStore().get("q1")).isEqualTo(OQL);
+
+ mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL)
+ .with(httpBasic(WRITE_USER, WRITE_USER))
+ .with(JSON))
+ .andExpect(status().isOk());
+
+ assertThat(queryStore().get("q1")).isEqualTo(OTHER_OQL);
+ }
+
+ @Test
+ public void listIsAllowedForAUserWithReadPermission() throws Exception {
+ queryStore().put("q1", OQL);
+
+ mockMvc.perform(get("/v1/queries")
+ .with(httpBasic(READ_USER, READ_USER))
+ .with(JSON))
+ .andExpect(status().isOk());
+ }
+
+ private static class JsonRequestPostProcessor implements RequestPostProcessor {
+
+ @SuppressWarnings("deprecation")
+ private static final MediaType APPLICATION_JSON_UTF8 = MediaType.APPLICATION_JSON_UTF8;
+
+ @Override
+ public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
+ request.addHeader(HttpHeaders.ACCEPT, APPLICATION_JSON_UTF8);
+ request.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON_UTF8);
+ return request;
+ }
+ }
+}
+
+
+class SecuredTestContextLoader extends GenericXmlWebContextLoader {
+ @Override
+ protected void loadBeanDefinitions(GenericWebApplicationContext context,
+ WebMergedContextConfiguration webMergedConfig) {
+ super.loadBeanDefinitions(context, webMergedConfig);
+ context.getServletContext().setAttribute(
+ HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
+ QueryAccessControllerAuthorizationTest.rule.getCache().getSecurityService());
+ }
+}
diff --git a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
index 35fab2f290..1bab9a255e 100644
--- a/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
+++ b/geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java
@@ -124,7 +124,7 @@ public class QueryAccessController extends AbstractBaseController {
@ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."),
@ApiResponse(responseCode = "409", description = "QueryId already assigned to other query."),
@ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")})
- @PreAuthorize("@securityService.authorize('DATA', 'READ')")
+ @PreAuthorize("@securityService.authorize('DATA', 'WRITE')")
public ResponseEntity<?> create(@RequestParam("id") final String queryId,
@RequestParam(value = "q", required = false) String oqlInUrl,
@RequestBody(required = false) final String oqlInBody) {
@@ -306,7 +306,7 @@ public class QueryAccessController extends AbstractBaseController {
@ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."),
@ApiResponse(responseCode = "404", description = "queryId does not exist."),
@ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")})
- @PreAuthorize("@securityService.authorize('DATA', 'READ')")
+ @PreAuthorize("@securityService.authorize('DATA', 'WRITE')")
public ResponseEntity<?> update(@PathVariable("query") final String queryId,
@RequestParam(value = "q", required = false) String oqlInUrl,
@RequestBody(required = false) final String oqlInBody) {