This is an automated email from the ASF dual-hosted git repository.
SbloodyS pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new a08de7bb4f [Fix-18276][API] Fix alert plugin instance permission check
(#18279)
a08de7bb4f is described below
commit a08de7bb4feecf625735d87d8e57d70591061ec1
Author: Wenjun Ruan <[email protected]>
AuthorDate: Fri May 22 09:40:51 2026 +0800
[Fix-18276][API] Fix alert plugin instance permission check (#18279)
---
.../controller/AlertPluginInstanceController.java | 4 +-
.../api/service/AlertPluginInstanceService.java | 4 +-
.../impl/AlertPluginInstanceServiceImpl.java | 18 +++--
.../AlertPluginInstanceControllerTest.java | 6 +-
.../service/AlertPluginInstanceServiceTest.java | 90 ++++++++++++++++++----
5 files changed, 95 insertions(+), 27 deletions(-)
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
index 133791533e..1ae628b4bf 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
@@ -190,7 +190,7 @@ public class AlertPluginInstanceController extends
BaseController {
@ResponseStatus(HttpStatus.OK)
@ApiException(QUERY_ALL_ALERT_PLUGIN_INSTANCE_ERROR)
public Result<List<AlertPluginInstanceVO>>
getAlertPluginInstance(@Parameter(hidden = true) @RequestAttribute(value =
Constants.SESSION_USER) User loginUser) {
- List<AlertPluginInstanceVO> alertPluginInstanceVOS =
alertPluginInstanceService.queryAll();
+ List<AlertPluginInstanceVO> alertPluginInstanceVOS =
alertPluginInstanceService.queryAll(loginUser);
return Result.success(alertPluginInstanceVOS);
}
@@ -210,7 +210,7 @@ public class AlertPluginInstanceController extends
BaseController {
public Result verifyGroupName(@Parameter(hidden = true)
@RequestAttribute(value = Constants.SESSION_USER) User loginUser,
@RequestParam(value = "alertInstanceName")
String alertInstanceName) {
- boolean exist =
alertPluginInstanceService.checkExistPluginInstanceName(alertInstanceName);
+ boolean exist =
alertPluginInstanceService.checkExistPluginInstanceName(loginUser,
alertInstanceName);
if (exist) {
log.error("alert plugin instance {} has exist, can't create
again.", alertInstanceName);
return Result.error(Status.PLUGIN_INSTANCE_ALREADY_EXISTS);
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceService.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceService.java
index 0297c13acb..718a088d8b 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceService.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceService.java
@@ -76,14 +76,14 @@ public interface AlertPluginInstanceService {
*
* @return alert plugins
*/
- List<AlertPluginInstanceVO> queryAll();
+ List<AlertPluginInstanceVO> queryAll(User loginUser);
/**
* checkExistPluginInstanceName
* @param pluginName plugin name
* @return isExist
*/
- boolean checkExistPluginInstanceName(String pluginName);
+ boolean checkExistPluginInstanceName(User loginUser, String pluginName);
/**
* queryPluginPage
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
index 3633fa04e5..765ef9b425 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
@@ -17,11 +17,11 @@
package org.apache.dolphinscheduler.api.service.impl;
+import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ALARM_INSTANCE_MANAGE;
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ALERT_INSTANCE_CREATE;
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ALERT_PLUGIN_DELETE;
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ALERT_PLUGIN_UPDATE;
-import org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant;
import org.apache.dolphinscheduler.api.enums.Status;
import org.apache.dolphinscheduler.api.exceptions.ServiceException;
import org.apache.dolphinscheduler.api.service.AlertPluginInstanceService;
@@ -181,26 +181,28 @@ public class AlertPluginInstanceServiceImpl extends
BaseServiceImpl implements A
*/
@Override
public AlertPluginInstance getById(User loginUser, int id) {
- if (!canOperatorPermissions(loginUser, null,
AuthorizationType.ALERT_PLUGIN_INSTANCE,
- ApiFuncIdentificationConstant.ALARM_INSTANCE_MANAGE)) {
+ if (!canOperatorPermissions(loginUser, null,
AuthorizationType.ALERT_PLUGIN_INSTANCE, ALARM_INSTANCE_MANAGE)) {
throw new ServiceException(Status.USER_NO_OPERATION_PERM);
}
return alertPluginInstanceMapper.selectById(id);
}
@Override
- public List<AlertPluginInstanceVO> queryAll() {
+ public List<AlertPluginInstanceVO> queryAll(User loginUser) {
+ checkAlertPluginInstanceViewPermission(loginUser);
List<AlertPluginInstance> alertPluginInstances =
alertPluginInstanceMapper.queryAllAlertPluginInstanceList();
return buildPluginInstanceVOList(alertPluginInstances);
}
@Override
- public boolean checkExistPluginInstanceName(String pluginInstanceName) {
+ public boolean checkExistPluginInstanceName(User loginUser, String
pluginInstanceName) {
+ checkAlertPluginInstanceViewPermission(loginUser);
return alertPluginInstanceMapper.existInstanceName(pluginInstanceName)
== Boolean.TRUE;
}
@Override
public PageInfo<AlertPluginInstanceVO> listPaging(User loginUser, String
searchVal, int pageNo, int pageSize) {
+ checkAlertPluginInstanceViewPermission(loginUser);
IPage<AlertPluginInstance> alertPluginInstanceIPage =
alertPluginInstanceMapper.queryByInstanceNamePage(new
Page<>(pageNo, pageSize), searchVal);
@@ -211,6 +213,12 @@ public class AlertPluginInstanceServiceImpl extends
BaseServiceImpl implements A
return pageInfo;
}
+ private void checkAlertPluginInstanceViewPermission(User loginUser) {
+ if (!canOperatorPermissions(loginUser, null,
AuthorizationType.ALERT_PLUGIN_INSTANCE, ALARM_INSTANCE_MANAGE)) {
+ throw new ServiceException(Status.USER_NO_OPERATION_PERM);
+ }
+ }
+
private List<AlertPluginInstanceVO>
buildPluginInstanceVOList(List<AlertPluginInstance> alertPluginInstances) {
List<AlertPluginInstanceVO> alertPluginInstanceVOS = new ArrayList<>();
if (CollectionUtils.isEmpty(alertPluginInstances)) {
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceControllerTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceControllerTest.java
index 3cb50c5a31..67f777f82c 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceControllerTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceControllerTest.java
@@ -196,7 +196,7 @@ public class AlertPluginInstanceControllerTest extends
AbstractControllerTest {
@Test
public void testGetAlertPluginInstanceList() throws Exception {
// Given
- when(alertPluginInstanceService.queryAll()).thenReturn(null);
+
when(alertPluginInstanceService.queryAll(any(User.class))).thenReturn(null);
// When
final MvcResult mvcResult =
mockMvc.perform(get("/alert-plugin-instances/list")
@@ -219,7 +219,7 @@ public class AlertPluginInstanceControllerTest extends
AbstractControllerTest {
paramsMap.add("pluginDefineId", String.valueOf(pluginDefineId));
paramsMap.add("alertInstanceName", instanceName);
-
when(alertPluginInstanceService.checkExistPluginInstanceName(eq(instanceName)))
+
when(alertPluginInstanceService.checkExistPluginInstanceName(any(User.class),
eq(instanceName)))
.thenReturn(false);
Result expectResponseContent = JSONUtils.parseObject(
@@ -247,7 +247,7 @@ public class AlertPluginInstanceControllerTest extends
AbstractControllerTest {
paramsMap.add("pluginDefineId", String.valueOf(pluginDefineId));
paramsMap.add("alertInstanceName", instanceName);
-
when(alertPluginInstanceService.checkExistPluginInstanceName(eq(instanceName)))
+
when(alertPluginInstanceService.checkExistPluginInstanceName(any(User.class),
eq(instanceName)))
.thenReturn(true);
Result expectResponseContent = JSONUtils.parseObject(
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java
index 429a12abb8..bf3b44fd44 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java
@@ -33,7 +33,6 @@ import
org.apache.dolphinscheduler.api.service.impl.BaseServiceImpl;
import org.apache.dolphinscheduler.common.enums.AuthorizationType;
import org.apache.dolphinscheduler.common.enums.UserType;
import org.apache.dolphinscheduler.common.model.Server;
-import org.apache.dolphinscheduler.dao.entity.AlertGroup;
import org.apache.dolphinscheduler.dao.entity.AlertPluginInstance;
import org.apache.dolphinscheduler.dao.entity.PluginDefine;
import org.apache.dolphinscheduler.dao.entity.User;
@@ -261,8 +260,8 @@ public class AlertPluginInstanceServiceTest {
when(alertPluginInstanceMapper.updateById(Mockito.any())).thenReturn(0);
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
1,
ALERT_PLUGIN_UPDATE, baseServiceLogger)).thenReturn(true);
-
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
null, 0,
- baseServiceLogger)).thenReturn(true);
+
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
+ null, 0, baseServiceLogger)).thenReturn(true);
assertThrowsServiceException(Status.SAVE_ERROR,
() -> alertPluginInstanceService.updateById(user, 1,
"testUpdate", uiParams));
@@ -281,8 +280,8 @@ public class AlertPluginInstanceServiceTest {
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
user.getId(), ALARM_INSTANCE_MANAGE,
baseServiceLogger)).thenReturn(true);
-
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
null, 0,
- baseServiceLogger)).thenReturn(true);
+
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
+ null, 0, baseServiceLogger)).thenReturn(true);
when(alertPluginInstanceMapper.selectById(1))
.thenReturn(getAlertPluginInstance(1, "test_get_instance"));
@@ -291,12 +290,29 @@ public class AlertPluginInstanceServiceTest {
@Test
public void testCheckExistPluginInstanceName() {
+ grantAlertPluginInstanceViewPermission(user);
when(alertPluginInstanceMapper.existInstanceName(Mockito.any(String.class))).thenReturn(false);
- Assertions.assertEquals(false,
alertPluginInstanceService.checkExistPluginInstanceName("test"));
+ Assertions.assertEquals(false,
alertPluginInstanceService.checkExistPluginInstanceName(user, "test"));
+ }
+
+ @Test
+ public void testCheckExistPluginInstanceNameWithoutPermission() {
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () ->
alertPluginInstanceService.checkExistPluginInstanceName(noPermUser, "test"));
+ Mockito.verify(alertPluginInstanceMapper,
Mockito.never()).existInstanceName(Mockito.anyString());
+ }
+
+ @Test
+ public void testCheckExistPluginInstanceNameWithOperationPermission() {
+ grantAlertPluginInstanceViewPermission(noPermUser);
+
when(alertPluginInstanceMapper.existInstanceName("test")).thenReturn(true);
+
+
Assertions.assertTrue(alertPluginInstanceService.checkExistPluginInstanceName(noPermUser,
"test"));
}
@Test
public void testListPaging() {
+ grantAlertPluginInstanceViewPermission(user);
IPage<AlertPluginInstance> page = new Page<>();
page.setRecords(Collections.singletonList(alertPluginInstance));
page.setTotal(1);
@@ -307,15 +323,38 @@ public class AlertPluginInstanceServiceTest {
assertDoesNotThrow(() -> alertPluginInstanceService.listPaging(user,
"test", 1, 1));
}
+ @Test
+ public void testListPagingWithoutPermission() {
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () -> alertPluginInstanceService.listPaging(noPermUser,
"test", 1, 1));
+ Mockito.verify(alertPluginInstanceMapper,
Mockito.never()).queryByInstanceNamePage(Mockito.any(Page.class),
+ Mockito.anyString());
+ }
+
+ @Test
+ public void testListPagingWithOperationPermission() {
+ grantAlertPluginInstanceViewPermission(noPermUser);
+
+ IPage<AlertPluginInstance> page = new Page<>();
+ page.setRecords(Collections.singletonList(alertPluginInstance));
+ page.setTotal(1);
+ page.setPages(1);
+
when(alertPluginInstanceMapper.queryByInstanceNamePage(Mockito.any(Page.class),
Mockito.eq("test")))
+ .thenReturn(page);
+
+ Assertions.assertEquals(1,
alertPluginInstanceService.listPaging(noPermUser, "test", 1, 1).getTotal());
+ }
+
@Test
public void testQueryAll() {
+ grantAlertPluginInstanceViewPermission(user);
when(alertPluginInstanceMapper.queryAllAlertPluginInstanceList()).thenReturn(Collections.emptyList());
- Assertions.assertEquals(0,
alertPluginInstanceService.queryAll().size());
+ Assertions.assertEquals(0,
alertPluginInstanceService.queryAll(user).size());
when(alertPluginInstanceMapper.queryAllAlertPluginInstanceList())
.thenReturn(Collections.singletonList(alertPluginInstance));
when(pluginDefineMapper.queryAllPluginDefineList()).thenReturn(Collections.emptyList());
- Assertions.assertEquals(0,
alertPluginInstanceService.queryAll().size());
+ Assertions.assertEquals(0,
alertPluginInstanceService.queryAll(user).size());
AlertPluginInstance alertPluginInstance = getAlertPluginInstance(1,
"test");
PluginDefine pluginDefine = new PluginDefine("script", "script",
uiParams);
@@ -324,7 +363,27 @@ public class AlertPluginInstanceServiceTest {
List<AlertPluginInstance> pluginInstanceList =
Collections.singletonList(alertPluginInstance);
when(alertPluginInstanceMapper.queryAllAlertPluginInstanceList()).thenReturn(pluginInstanceList);
when(pluginDefineMapper.queryAllPluginDefineList()).thenReturn(pluginDefines);
- Assertions.assertDoesNotThrow(() ->
alertPluginInstanceService.queryAll());
+ Assertions.assertDoesNotThrow(() ->
alertPluginInstanceService.queryAll(user));
+ }
+
+ @Test
+ public void testQueryAllWithoutPermission() {
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () -> alertPluginInstanceService.queryAll(noPermUser));
+ Mockito.verify(alertPluginInstanceMapper,
Mockito.never()).queryAllAlertPluginInstanceList();
+ }
+
+ @Test
+ public void testQueryAllWithOperationPermission() {
+ grantAlertPluginInstanceViewPermission(noPermUser);
+
+ PluginDefine pluginDefine = new PluginDefine("script", "script",
uiParams);
+ pluginDefine.setId(1);
+
when(alertPluginInstanceMapper.queryAllAlertPluginInstanceList()).thenReturn(Collections.singletonList(
+ alertPluginInstance));
+
when(pluginDefineMapper.queryAllPluginDefineList()).thenReturn(Collections.singletonList(pluginDefine));
+
+ Assertions.assertEquals(1,
alertPluginInstanceService.queryAll(noPermUser).size());
}
private AlertPluginInstance getAlertPluginInstance(int id, String
instanceName) {
@@ -336,11 +395,12 @@ public class AlertPluginInstanceServiceTest {
return alertPluginInstance;
}
- private AlertGroup getGlobalAlertGroup(String... alertPluginInstanceIds) {
- AlertGroup globalAlertGroup = new AlertGroup();
- globalAlertGroup.setId(2);
- globalAlertGroup.setAlertInstanceIds(String.join(",",
alertPluginInstanceIds));
-
- return globalAlertGroup;
+ private void grantAlertPluginInstanceViewPermission(User loginUser) {
+
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
+ loginUser.getId(), ALARM_INSTANCE_MANAGE,
baseServiceLogger)).thenReturn(true);
+
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ALERT_PLUGIN_INSTANCE,
null,
+ loginUser.getUserType().equals(UserType.ADMIN_USER) ? 0 :
loginUser.getId(), baseServiceLogger))
+ .thenReturn(true);
}
+
}
