Skip to content

Commit f258aec

Browse files
Aias00Aias00
authored
Fix missing RBAC on selector, rule, and data-permission endpoints (#6319)
1 parent 0e1cc3c commit f258aec

6 files changed

Lines changed: 99 additions & 2 deletions

File tree

‎shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/DataPermissionController.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import org.apache.shenyu.admin.service.DataPermissionService;
3030
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
3131
import org.apache.shenyu.admin.validation.annotation.Existed;
32+
import org.apache.shiro.authz.annotation.RequiresPermissions;
3233
import org.springframework.web.bind.annotation.DeleteMapping;
3334
import org.springframework.web.bind.annotation.GetMapping;
3435
import org.springframework.web.bind.annotation.PostMapping;
@@ -62,6 +63,7 @@ public DataPermissionController(final DataPermissionService dataPermissionServic
6263
* @return {@linkplain ShenyuAdminResult}
6364
*/
6465
@GetMapping("/selector")
66+
@RequiresPermissions("system:manager:configureDataPermission")
6567
public ShenyuAdminResult listPageSelectorDataPermissions(@RequestParam("currentPage") final Integer currentPage,
6668
@RequestParam("pageSize") final Integer pageSize,
6769
@RequestParam("userId") final String userId,
@@ -86,6 +88,7 @@ public ShenyuAdminResult listPageSelectorDataPermissions(@RequestParam("currentP
8688
* @return {@linkplain ShenyuAdminResult}
8789
*/
8890
@GetMapping("/rules")
91+
@RequiresPermissions("system:manager:configureDataPermission")
8992
public ShenyuAdminResult listPageRuleDataPermissions(@RequestParam("currentPage") final Integer currentPage,
9093
@RequestParam("pageSize") final Integer pageSize,
9194
@RequestParam("userId") final String userId,
@@ -104,6 +107,7 @@ public ShenyuAdminResult listPageRuleDataPermissions(@RequestParam("currentPage"
104107
* @return effect rows count
105108
*/
106109
@PostMapping("/selector")
110+
@RequiresPermissions("system:manager:configureDataPermission")
107111
public ShenyuAdminResult saveSelector(@RequestBody @Valid @NotNull final DataPermissionDTO dataPermissionDTO) {
108112
return ShenyuAdminResult.success(ShenyuResultMessage.SAVE_SUCCESS, dataPermissionService.createSelector(dataPermissionDTO));
109113
}
@@ -115,28 +119,31 @@ public ShenyuAdminResult saveSelector(@RequestBody @Valid @NotNull final DataPer
115119
* @return effect rows count
116120
*/
117121
@DeleteMapping("/selector")
122+
@RequiresPermissions("system:manager:configureDataPermission")
118123
public ShenyuAdminResult deleteSelector(@RequestBody @Valid @NotNull final DataPermissionDTO dataPermissionDTO) {
119124
return ShenyuAdminResult.success(ShenyuResultMessage.DELETE_SUCCESS, dataPermissionService.deleteSelector(dataPermissionDTO));
120125
}
121126

122127
/**
123-
* Delete rule data permission.
128+
* Create rule data permission.
124129
*
125130
* @param dataPermissionDTO {@linkplain DataPermissionDTO}
126131
* @return effect rows count
127132
*/
128133
@PostMapping("/rule")
134+
@RequiresPermissions("system:manager:configureDataPermission")
129135
public ShenyuAdminResult saveRule(@RequestBody @Valid @NotNull final DataPermissionDTO dataPermissionDTO) {
130136
return ShenyuAdminResult.success(ShenyuResultMessage.SAVE_SUCCESS, dataPermissionService.createRule(dataPermissionDTO));
131137
}
132138

133139
/**
134-
* Delete selector data permission.
140+
* Delete rule data permission.
135141
*
136142
* @param dataPermissionDTO {@linkplain DataPermissionDTO}
137143
* @return effect rows count
138144
*/
139145
@DeleteMapping("/rule")
146+
@RequiresPermissions("system:manager:configureDataPermission")
140147
public ShenyuAdminResult deleteRule(@RequestBody @Valid @NotNull final DataPermissionDTO dataPermissionDTO) {
141148
return ShenyuAdminResult.success(ShenyuResultMessage.DELETE_SUCCESS, dataPermissionService.deleteRule(dataPermissionDTO));
142149
}

‎shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/RuleController.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
3838
import org.apache.shenyu.admin.validation.annotation.Existed;
3939
import org.apache.shenyu.common.utils.ListUtil;
40+
import org.apache.shiro.authz.annotation.RequiresPermissions;
4041
import org.springframework.web.bind.annotation.DeleteMapping;
4142
import org.springframework.web.bind.annotation.GetMapping;
4243
import org.springframework.web.bind.annotation.PathVariable;
@@ -101,6 +102,7 @@ public ShenyuAdminResult detailRule(@Valid @PathVariable("id")
101102
* @return {@linkplain ShenyuAdminResult}
102103
*/
103104
@PostMapping
105+
@RequiresPermissions("system:plugin:edit")
104106
public ShenyuAdminResult createRule(@Valid @RequestBody final RuleDTO ruleDTO) {
105107
Integer createCount = ruleService.createOrUpdate(ruleDTO);
106108
return ShenyuAdminResult.success(ShenyuResultMessage.CREATE_SUCCESS, createCount);
@@ -114,6 +116,7 @@ public ShenyuAdminResult createRule(@Valid @RequestBody final RuleDTO ruleDTO) {
114116
* @return {@linkplain ShenyuAdminResult}
115117
*/
116118
@PutMapping("/{id}")
119+
@RequiresPermissions("system:plugin:edit")
117120
public ShenyuAdminResult updateRule(@PathVariable("id") @Valid
118121
@Existed(provider = RuleMapper.class,
119122
message = "rule is not existed") final String id,
@@ -130,6 +133,7 @@ public ShenyuAdminResult updateRule(@PathVariable("id") @Valid
130133
* @return the shenyu result
131134
*/
132135
@PostMapping("/batchEnabled")
136+
@RequiresPermissions("system:plugin:disable")
133137
public ShenyuAdminResult batchEnabled(@Valid @RequestBody final BatchCommonDTO batchCommonDTO) {
134138
if (!ruleService.enabledByIdsAndNamespaceId(batchCommonDTO.getIds(), batchCommonDTO.getEnabled(), batchCommonDTO.getNamespaceId())) {
135139
return ShenyuAdminResult.error(ShenyuResultMessage.NOT_FOUND_EXCEPTION);
@@ -144,6 +148,7 @@ public ShenyuAdminResult batchEnabled(@Valid @RequestBody final BatchCommonDTO b
144148
* @return {@linkplain ShenyuAdminResult}
145149
*/
146150
@DeleteMapping("/batch")
151+
@RequiresPermissions("system:plugin:delete")
147152
public ShenyuAdminResult deleteRules(@Valid @RequestBody final BatchNamespaceCommonDTO batchNamespaceCommonDTO) {
148153
Integer deleteCount = ruleService.deleteByIdsAndNamespaceId(batchNamespaceCommonDTO.getIds(), batchNamespaceCommonDTO.getNamespaceId());
149154
return ShenyuAdminResult.success(ShenyuResultMessage.DELETE_SUCCESS, deleteCount);

‎shenyu-admin/src/main/java/org/apache/shenyu/admin/controller/SelectorController.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
3737
import org.apache.shenyu.admin.validation.annotation.Existed;
3838
import org.apache.shenyu.common.utils.ListUtil;
39+
import org.apache.shiro.authz.annotation.RequiresPermissions;
3940
import org.springframework.web.bind.annotation.DeleteMapping;
4041
import org.springframework.web.bind.annotation.GetMapping;
4142
import org.springframework.web.bind.annotation.PathVariable;
@@ -104,6 +105,7 @@ public ShenyuAdminResult detailSelector(@PathVariable("id") @Valid
104105
* @return {@linkplain ShenyuAdminResult}
105106
*/
106107
@PostMapping
108+
@RequiresPermissions("system:plugin:edit")
107109
public ShenyuAdminResult createSelector(@Valid @RequestBody final SelectorDTO selectorDTO) {
108110
selectorService.createOrUpdate(selectorDTO);
109111
return ShenyuAdminResult.success(ShenyuResultMessage.CREATE_SUCCESS, selectorDTO.getId());
@@ -117,6 +119,7 @@ public ShenyuAdminResult createSelector(@Valid @RequestBody final SelectorDTO se
117119
* @return {@linkplain ShenyuAdminResult}
118120
*/
119121
@PutMapping("/{id}")
122+
@RequiresPermissions("system:plugin:edit")
120123
public ShenyuAdminResult updateSelector(@PathVariable("id") @Valid
121124
@Existed(provider = SelectorMapper.class, message = "selector is not existed") final String id,
122125
@Valid @RequestBody final SelectorDTO selectorDTO) {
@@ -132,6 +135,7 @@ public ShenyuAdminResult updateSelector(@PathVariable("id") @Valid
132135
* @return the shenyu result
133136
*/
134137
@PostMapping("/batchEnabled")
138+
@RequiresPermissions("system:plugin:disable")
135139
public ShenyuAdminResult batchEnabled(@Valid @RequestBody final BatchCommonDTO batchCommonDTO) {
136140
if (!selectorService.enabledByIdsAndNamespaceId(batchCommonDTO.getIds(), batchCommonDTO.getEnabled(), batchCommonDTO.getNamespaceId())) {
137141
return ShenyuAdminResult.error(ShenyuResultMessage.NOT_FOUND_EXCEPTION);
@@ -146,6 +150,7 @@ public ShenyuAdminResult batchEnabled(@Valid @RequestBody final BatchCommonDTO b
146150
* @return {@linkplain ShenyuAdminResult}
147151
*/
148152
@DeleteMapping("/batch")
153+
@RequiresPermissions("system:plugin:delete")
149154
public ShenyuAdminResult deleteSelector(@Valid @RequestBody final BatchNamespaceCommonDTO batchNamespaceCommonDTO) {
150155
Integer deleteCount = selectorService.deleteByNamespaceId(batchNamespaceCommonDTO.getIds(), batchNamespaceCommonDTO.getNamespaceId());
151156
return ShenyuAdminResult.success(ShenyuResultMessage.DELETE_SUCCESS, deleteCount);

‎shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/DataPermissionControllerTest.java‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,20 +26,27 @@
2626
import org.apache.shenyu.admin.service.DataPermissionService;
2727
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
2828
import org.apache.shenyu.common.utils.GsonUtils;
29+
import org.apache.shiro.authz.annotation.RequiresPermissions;
2930
import org.junit.jupiter.api.BeforeEach;
3031
import org.junit.jupiter.api.Test;
3132
import org.junit.jupiter.api.extension.ExtendWith;
3233
import org.mockito.InjectMocks;
3334
import org.mockito.Mock;
3435
import org.mockito.junit.jupiter.MockitoExtension;
36+
import org.springframework.core.annotation.AnnotationUtils;
3537
import org.springframework.http.MediaType;
3638
import org.springframework.test.web.servlet.MockMvc;
3739
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
3840
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
41+
42+
import java.lang.reflect.Method;
3943
import java.util.Collections;
44+
import java.util.Objects;
4045

4146
import static org.apache.shenyu.common.constant.Constants.SYS_DEFAULT_NAMESPACE_ID;
4247
import static org.hamcrest.core.Is.is;
48+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
49+
import static org.junit.jupiter.api.Assertions.assertNotNull;
4350
import static org.mockito.BDDMockito.given;
4451
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath;
4552
import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status;
@@ -175,4 +182,31 @@ public void deleteRule() throws Exception {
175182
.andExpect(jsonPath("$.data", is(1)))
176183
.andReturn();
177184
}
185+
186+
@Test
187+
public void shouldRequireConfigureDataPermissionForAllEndpoints() throws NoSuchMethodException {
188+
assertPermissions(DataPermissionController.class.getMethod("listPageSelectorDataPermissions",
189+
Integer.class, Integer.class, String.class, String.class, String.class, String.class),
190+
"system:manager:configureDataPermission");
191+
assertPermissions(DataPermissionController.class.getMethod("listPageRuleDataPermissions",
192+
Integer.class, Integer.class, String.class, String.class, String.class),
193+
"system:manager:configureDataPermission");
194+
assertPermissions(DataPermissionController.class.getMethod("saveSelector", DataPermissionDTO.class),
195+
"system:manager:configureDataPermission");
196+
assertPermissions(DataPermissionController.class.getMethod("deleteSelector", DataPermissionDTO.class),
197+
"system:manager:configureDataPermission");
198+
assertPermissions(DataPermissionController.class.getMethod("saveRule", DataPermissionDTO.class),
199+
"system:manager:configureDataPermission");
200+
assertPermissions(DataPermissionController.class.getMethod("deleteRule", DataPermissionDTO.class),
201+
"system:manager:configureDataPermission");
202+
}
203+
204+
private void assertPermissions(final Method method, final String... expectedPermissions) {
205+
RequiresPermissions permissions = AnnotationUtils.findAnnotation(method, RequiresPermissions.class);
206+
if (Objects.isNull(permissions)) {
207+
permissions = AnnotationUtils.findAnnotation(method.getDeclaringClass(), RequiresPermissions.class);
208+
}
209+
assertNotNull(permissions, method.getName() + " should declare @RequiresPermissions");
210+
assertArrayEquals(expectedPermissions, permissions.value(), method.getName() + " should declare the expected permissions");
211+
}
178212
}

‎shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/RuleControllerTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.shenyu.admin.utils.ShenyuResultMessage;
3737
import org.apache.shenyu.common.utils.DateUtils;
3838
import org.apache.shenyu.common.utils.GsonUtils;
39+
import org.apache.shiro.authz.annotation.RequiresPermissions;
3940
import org.junit.jupiter.api.BeforeEach;
4041
import org.junit.jupiter.api.Test;
4142
import org.junit.jupiter.api.extension.ExtendWith;
@@ -45,19 +46,24 @@
4546
import org.mockito.junit.jupiter.MockitoSettings;
4647
import org.mockito.quality.Strictness;
4748
import org.springframework.context.ConfigurableApplicationContext;
49+
import org.springframework.core.annotation.AnnotationUtils;
4850
import org.springframework.http.MediaType;
4951
import org.springframework.test.web.servlet.MockMvc;
5052
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
5153
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
5254

55+
import java.lang.reflect.Method;
5356
import java.time.LocalDateTime;
5457
import java.util.ArrayList;
5558
import java.util.Arrays;
5659
import java.util.Collections;
5760
import java.util.List;
61+
import java.util.Objects;
5862

5963
import static org.apache.shenyu.common.constant.Constants.SYS_DEFAULT_NAMESPACE_ID;
6064
import static org.hamcrest.core.Is.is;
65+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
66+
import static org.junit.jupiter.api.Assertions.assertNotNull;
6167
import static org.mockito.ArgumentMatchers.any;
6268
import static org.mockito.BDDMockito.given;
6369
import static org.mockito.Mockito.mock;
@@ -273,4 +279,21 @@ public void testEnableRule() throws Exception {
273279
.andReturn();
274280
}
275281

282+
@Test
283+
public void shouldRequirePluginPermissionsForMutatingEndpoints() throws NoSuchMethodException {
284+
assertPermissions(RuleController.class.getMethod("createRule", RuleDTO.class), "system:plugin:edit");
285+
assertPermissions(RuleController.class.getMethod("updateRule", String.class, RuleDTO.class), "system:plugin:edit");
286+
assertPermissions(RuleController.class.getMethod("batchEnabled", BatchCommonDTO.class), "system:plugin:disable");
287+
assertPermissions(RuleController.class.getMethod("deleteRules", BatchNamespaceCommonDTO.class), "system:plugin:delete");
288+
}
289+
290+
private void assertPermissions(final Method method, final String... expectedPermissions) {
291+
RequiresPermissions permissions = AnnotationUtils.findAnnotation(method, RequiresPermissions.class);
292+
if (Objects.isNull(permissions)) {
293+
permissions = AnnotationUtils.findAnnotation(method.getDeclaringClass(), RequiresPermissions.class);
294+
}
295+
assertNotNull(permissions, method.getName() + " should declare @RequiresPermissions");
296+
assertArrayEquals(expectedPermissions, permissions.value(), method.getName() + " should declare the expected permissions");
297+
}
298+
276299
}

‎shenyu-admin/src/test/java/org/apache/shenyu/admin/controller/SelectorControllerTest.java‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
import org.apache.shenyu.common.enums.SelectorTypeEnum;
3737
import org.apache.shenyu.common.utils.DateUtils;
3838
import org.apache.shenyu.common.utils.GsonUtils;
39+
import org.apache.shiro.authz.annotation.RequiresPermissions;
3940
import org.junit.jupiter.api.BeforeEach;
4041
import org.junit.jupiter.api.Test;
4142
import org.junit.jupiter.api.extension.ExtendWith;
@@ -45,17 +46,22 @@
4546
import org.mockito.junit.jupiter.MockitoSettings;
4647
import org.mockito.quality.Strictness;
4748
import org.springframework.context.ConfigurableApplicationContext;
49+
import org.springframework.core.annotation.AnnotationUtils;
4850
import org.springframework.http.MediaType;
4951
import org.springframework.test.web.servlet.MockMvc;
5052
import org.springframework.test.web.servlet.request.MockMvcRequestBuilders;
5153
import org.springframework.test.web.servlet.setup.MockMvcBuilders;
5254

55+
import java.lang.reflect.Method;
5356
import java.time.LocalDateTime;
5457
import java.util.Arrays;
5558
import java.util.Collections;
59+
import java.util.Objects;
5660

5761
import static org.apache.shenyu.common.constant.Constants.SYS_DEFAULT_NAMESPACE_ID;
5862
import static org.hamcrest.core.Is.is;
63+
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
64+
import static org.junit.jupiter.api.Assertions.assertNotNull;
5965
import static org.mockito.ArgumentMatchers.any;
6066
import static org.mockito.BDDMockito.given;
6167
import static org.mockito.Mockito.mock;
@@ -241,4 +247,21 @@ public void enableSelector() throws Exception {
241247
.andExpect(jsonPath("$.message", is(ShenyuResultMessage.ENABLE_SUCCESS)))
242248
.andReturn();
243249
}
250+
251+
@Test
252+
public void shouldRequirePluginPermissionsForMutatingEndpoints() throws NoSuchMethodException {
253+
assertPermissions(SelectorController.class.getMethod("createSelector", SelectorDTO.class), "system:plugin:edit");
254+
assertPermissions(SelectorController.class.getMethod("updateSelector", String.class, SelectorDTO.class), "system:plugin:edit");
255+
assertPermissions(SelectorController.class.getMethod("batchEnabled", BatchCommonDTO.class), "system:plugin:disable");
256+
assertPermissions(SelectorController.class.getMethod("deleteSelector", BatchNamespaceCommonDTO.class), "system:plugin:delete");
257+
}
258+
259+
private void assertPermissions(final Method method, final String... expectedPermissions) {
260+
RequiresPermissions permissions = AnnotationUtils.findAnnotation(method, RequiresPermissions.class);
261+
if (Objects.isNull(permissions)) {
262+
permissions = AnnotationUtils.findAnnotation(method.getDeclaringClass(), RequiresPermissions.class);
263+
}
264+
assertNotNull(permissions, method.getName() + " should declare @RequiresPermissions");
265+
assertArrayEquals(expectedPermissions, permissions.value(), method.getName() + " should declare the expected permissions");
266+
}
244267
}

0 commit comments

Comments
 (0)