Skip to content

Commit 4bdf539

Browse files
JinwooHwangJinwooHwang
authored
[GEODE-10618] Make named-query endpoint permissions consistent with region data endpoints (#8043)
1 parent 2127f9e commit 4bdf539

4 files changed

Lines changed: 199 additions & 3 deletions

File tree

‎geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityIntegrationTest.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,8 @@ public void testPostQuery() {
107107
assertResponse(restClient.doPost("/queries?id=0&q=", "user", "user", ""))
108108
.hasStatusCode(403);
109109
assertResponse(restClient.doPost("/queries?id=0&q=", "dataRead", "dataRead", ""))
110+
.hasStatusCode(403);
111+
assertResponse(restClient.doPost("/queries?id=0&q=", "dataWrite", "dataWrite", ""))
110112
.hasStatusCode(500);
111113
}
112114

@@ -127,6 +129,8 @@ public void testPutQuery() {
127129
assertResponse(restClient.doPut("/queries/id", "user", "user", "{\"id\" : \"foo\"}"))
128130
.hasStatusCode(403);
129131
assertResponse(restClient.doPut("/queries/id", "dataRead", "dataRead", "{\"id\" : \"foo\"}"))
132+
.hasStatusCode(403);
133+
assertResponse(restClient.doPut("/queries/id", "dataWrite", "dataWrite", "{\"id\" : \"foo\"}"))
130134
.hasStatusCode(404);
131135
}
132136

‎geode-assembly/src/integrationTest/java/org/apache/geode/rest/internal/web/RestSecurityPostProcessorTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -158,7 +158,7 @@ public void namedQuery() throws Exception {
158158
// Install the named query
159159
assertResponse(
160160
restClient.doPost("/queries?id=selectCustomer&q=" + URLEncoder.encode(namedQuery, "UTF-8"),
161-
"dataReader", "1234567", ""))
161+
"dataUser", "1234567", ""))
162162
.hasStatusCode(201);
163163

164164
// Verify the query has been installed

‎geode-web-api/src/integrationTest/java/org/apache/geode/rest/internal/web/controllers/QueryAccessControllerAuthorizationTest.java‎

Lines changed: 192 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,192 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one or more contributor license
3+
* agreements. See the NOTICE file distributed with this work for additional information regarding
4+
* copyright ownership. The ASF licenses this file to You under the Apache License, Version 2.0 (the
5+
* "License"); you may not use this file except in compliance with the License. You may obtain a
6+
* copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software distributed under the License
11+
* is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express
12+
* or implied. See the License for the specific language governing permissions and limitations under
13+
* the License.
14+
*/
15+
package org.apache.geode.rest.internal.web.controllers;
16+
17+
import static org.assertj.core.api.Assertions.assertThat;
18+
import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic;
19+
import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
20+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete;
21+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get;
22+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post;
23+
import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.put;
24+
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
25+
26+
import org.junit.Before;
27+
import org.junit.BeforeClass;
28+
import org.junit.ClassRule;
29+
import org.junit.Test;
30+
import org.junit.runner.RunWith;
31+
import org.springframework.beans.factory.annotation.Autowired;
32+
import org.springframework.http.HttpHeaders;
33+
import org.springframework.http.MediaType;
34+
import org.springframework.mock.web.MockHttpServletRequest;
35+
import org.springframework.test.context.ContextConfiguration;
36+
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
37+
import org.springframework.test.context.web.GenericXmlWebContextLoader;
38+
import org.springframework.test.context.web.WebAppConfiguration;
39+
import org.springframework.test.context.web.WebMergedContextConfiguration;
40+
import org.springframework.test.web.servlet.MockMvc;
41+
import org.springframework.test.web.servlet.request.RequestPostProcessor;
42+
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
43+
import org.springframework.web.context.WebApplicationContext;
44+
import org.springframework.web.context.support.GenericWebApplicationContext;
45+
46+
import org.apache.geode.cache.Region;
47+
import org.apache.geode.cache.RegionShortcut;
48+
import org.apache.geode.cache.internal.HttpService;
49+
import org.apache.geode.examples.SimpleSecurityManager;
50+
import org.apache.geode.management.internal.RestAgent;
51+
import org.apache.geode.test.junit.rules.ServerStarterRule;
52+
53+
/**
54+
* Verifies the permissions the named-query endpoints of {@link QueryAccessController} require.
55+
*
56+
* <p>
57+
* The endpoints that read query state require {@code DATA:READ}; the endpoints that create, update
58+
* or remove a stored named query all require {@code DATA:WRITE}, matching the permission required
59+
* for the equivalent operations on ordinary region data.
60+
*/
61+
@RunWith(SpringJUnit4ClassRunner.class)
62+
@ContextConfiguration(locations = {"classpath*:WEB-INF/geode-servlet.xml"},
63+
loader = SecuredTestContextLoader.class)
64+
@WebAppConfiguration
65+
public class QueryAccessControllerAuthorizationTest {
66+
67+
private static final String QUERY_STORE = "__ParameterizedQueries__";
68+
private static final String REGION_NAME = "customers";
69+
70+
private static final String READ_USER = "dataRead";
71+
private static final String WRITE_USER = "dataWrite";
72+
73+
private static final String OQL = "SELECT * FROM " + Region.SEPARATOR + REGION_NAME;
74+
private static final String OTHER_OQL =
75+
"SELECT c.name FROM " + Region.SEPARATOR + REGION_NAME + " c";
76+
77+
private static final RequestPostProcessor JSON = new JsonRequestPostProcessor();
78+
79+
@ClassRule
80+
public static ServerStarterRule rule = new ServerStarterRule()
81+
.withProperty("log-level", "warn")
82+
.withSecurityManager(SimpleSecurityManager.class)
83+
.withRegion(RegionShortcut.REPLICATE, REGION_NAME);
84+
85+
@Autowired
86+
private WebApplicationContext webApplicationContext;
87+
88+
private MockMvc mockMvc;
89+
90+
@BeforeClass
91+
public static void createQueryStore() {
92+
RestAgent.createParameterizedQueryRegion();
93+
}
94+
95+
@Before
96+
public void setUp() {
97+
mockMvc = MockMvcBuilders.webAppContextSetup(webApplicationContext)
98+
.apply(springSecurity())
99+
.build();
100+
queryStore().clear();
101+
}
102+
103+
private static Region<String, String> queryStore() {
104+
return rule.getCache().getInternalRegionByPath(Region.SEPARATOR + QUERY_STORE);
105+
}
106+
107+
@Test
108+
public void createIsRefusedForAUserWithoutWritePermission() throws Exception {
109+
mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL)
110+
.with(httpBasic(READ_USER, READ_USER))
111+
.with(JSON))
112+
.andExpect(status().isForbidden());
113+
114+
assertThat(queryStore()).doesNotContainKey("q1");
115+
}
116+
117+
@Test
118+
public void updateIsRefusedForAUserWithoutWritePermission() throws Exception {
119+
queryStore().put("q1", OQL);
120+
121+
mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL)
122+
.with(httpBasic(READ_USER, READ_USER))
123+
.with(JSON))
124+
.andExpect(status().isForbidden());
125+
126+
assertThat(queryStore().get("q1")).isEqualTo(OQL);
127+
}
128+
129+
@Test
130+
public void deleteIsRefusedForAUserWithoutWritePermission() throws Exception {
131+
queryStore().put("q1", OQL);
132+
133+
mockMvc.perform(delete("/v1/queries/q1")
134+
.with(httpBasic(READ_USER, READ_USER))
135+
.with(JSON))
136+
.andExpect(status().isForbidden());
137+
138+
assertThat(queryStore()).containsKey("q1");
139+
}
140+
141+
@Test
142+
public void createAndUpdateAreAllowedForAUserWithWritePermission() throws Exception {
143+
mockMvc.perform(post("/v1/queries?id=q1&q=" + OQL)
144+
.with(httpBasic(WRITE_USER, WRITE_USER))
145+
.with(JSON))
146+
.andExpect(status().isCreated());
147+
148+
assertThat(queryStore().get("q1")).isEqualTo(OQL);
149+
150+
mockMvc.perform(put("/v1/queries/q1?q=" + OTHER_OQL)
151+
.with(httpBasic(WRITE_USER, WRITE_USER))
152+
.with(JSON))
153+
.andExpect(status().isOk());
154+
155+
assertThat(queryStore().get("q1")).isEqualTo(OTHER_OQL);
156+
}
157+
158+
@Test
159+
public void listIsAllowedForAUserWithReadPermission() throws Exception {
160+
queryStore().put("q1", OQL);
161+
162+
mockMvc.perform(get("/v1/queries")
163+
.with(httpBasic(READ_USER, READ_USER))
164+
.with(JSON))
165+
.andExpect(status().isOk());
166+
}
167+
168+
private static class JsonRequestPostProcessor implements RequestPostProcessor {
169+
170+
@SuppressWarnings("deprecation")
171+
private static final MediaType APPLICATION_JSON_UTF8 = MediaType.APPLICATION_JSON_UTF8;
172+
173+
@Override
174+
public MockHttpServletRequest postProcessRequest(MockHttpServletRequest request) {
175+
request.addHeader(HttpHeaders.ACCEPT, APPLICATION_JSON_UTF8);
176+
request.addHeader(HttpHeaders.CONTENT_TYPE, APPLICATION_JSON_UTF8);
177+
return request;
178+
}
179+
}
180+
}
181+
182+
183+
class SecuredTestContextLoader extends GenericXmlWebContextLoader {
184+
@Override
185+
protected void loadBeanDefinitions(GenericWebApplicationContext context,
186+
WebMergedContextConfiguration webMergedConfig) {
187+
super.loadBeanDefinitions(context, webMergedConfig);
188+
context.getServletContext().setAttribute(
189+
HttpService.SECURITY_SERVICE_SERVLET_CONTEXT_PARAM,
190+
QueryAccessControllerAuthorizationTest.rule.getCache().getSecurityService());
191+
}
192+
}

‎geode-web-api/src/main/java/org/apache/geode/rest/internal/web/controllers/QueryAccessController.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ public ResponseEntity<?> list() {
129129
@ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."),
130130
@ApiResponse(responseCode = "409", description = "QueryId already assigned to other query."),
131131
@ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")})
132-
@PreAuthorize("@securityService.authorizeBoolean('DATA', 'READ')")
132+
@PreAuthorize("@securityService.authorizeBoolean('DATA', 'WRITE')")
133133
public ResponseEntity<?> create(@RequestParam("id") final String queryId,
134134
@RequestParam(value = "q", required = false) String oqlInUrl,
135135
@RequestBody(required = false) final String oqlInBody) {
@@ -311,7 +311,7 @@ public ResponseEntity<String> runNamedQuery(@PathVariable("query") String queryI
311311
@ApiResponse(responseCode = "403", description = "Insufficient privileges for operation."),
312312
@ApiResponse(responseCode = "404", description = "queryId does not exist."),
313313
@ApiResponse(responseCode = "500", description = "GemFire throws an error or exception.")})
314-
@PreAuthorize("@securityService.authorizeBoolean('DATA', 'READ')")
314+
@PreAuthorize("@securityService.authorizeBoolean('DATA', 'WRITE')")
315315
public ResponseEntity<?> update(@PathVariable("query") final String queryId,
316316
@RequestParam(value = "q", required = false) String oqlInUrl,
317317
@RequestBody(required = false) final String oqlInBody) {

0 commit comments

Comments
 (0)