Skip to content

[Fix-18276][API] Fix alert plugin instance permission check #18279

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SbloodyS merged 1 commit into from
May 22, 2026
Merged

[Fix-18276][API] Fix alert plugin instance permission check #18279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...c/main/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ public Result<AlertPluginInstance> getAlertPluginInstance(@Parameter(hidden = tr
@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);
}

Expand All @@ -210,7 +210,7 @@ public Result<List<AlertPluginInstanceVO>> getAlertPluginInstance(@Parameter(hid
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);
Expand Down
4 changes: 2 additions & 2 deletions ...api/src/main/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceService.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,14 +76,14 @@ AlertPluginInstance updateById(User loginUser,
*
* @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
Expand Down
18 changes: 13 additions & 5 deletions ...ain/java/org/apache/dolphinscheduler/api/service/impl/AlertPluginInstanceServiceImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -181,26 +181,28 @@ public void deleteById(User loginUser, int alertPluginInstanceId) {
*/
@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);
Expand All @@ -211,6 +213,12 @@ public PageInfo<AlertPluginInstanceVO> listPaging(User loginUser, String searchV
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)) {
Expand Down
6 changes: 3 additions & 3 deletions ...st/java/org/apache/dolphinscheduler/api/controller/AlertPluginInstanceControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ public void testGetAlertPluginInstance() throws Exception {
@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")
Expand All @@ -219,7 +219,7 @@ public void testVerifyGroupName() throws Exception {
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(
Expand Down Expand Up @@ -247,7 +247,7 @@ public void testVerifyGroupNamePluginInstanceNameExist() throws Exception {
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(
Expand Down
90 changes: 75 additions & 15 deletions ...src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
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;
Expand Down Expand Up @@ -261,8 +260,8 @@
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));

Expand All @@ -281,8 +280,8 @@

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"));

Expand All @@ -291,12 +290,29 @@

@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() {

Check warning on line 299 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_S&open=AZ5KAdSTEn5I1ZphW9_S&pullRequest=18279
assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
() -> alertPluginInstanceService.checkExistPluginInstanceName(noPermUser, "test"));
Mockito.verify(alertPluginInstanceMapper, Mockito.never()).existInstanceName(Mockito.anyString());
}

@Test
public void testCheckExistPluginInstanceNameWithOperationPermission() {

Check warning on line 306 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_T&open=AZ5KAdSTEn5I1ZphW9_T&pullRequest=18279
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);
Expand All @@ -307,15 +323,38 @@
assertDoesNotThrow(() -> alertPluginInstanceService.listPaging(user, "test", 1, 1));
}

@Test
public void testListPagingWithoutPermission() {

Check warning on line 327 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_U&open=AZ5KAdSTEn5I1ZphW9_U&pullRequest=18279
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() {

Check warning on line 335 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_V&open=AZ5KAdSTEn5I1ZphW9_V&pullRequest=18279
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);
Expand All @@ -324,7 +363,27 @@
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() {

Check warning on line 370 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_W&open=AZ5KAdSTEn5I1ZphW9_W&pullRequest=18279
assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
() -> alertPluginInstanceService.queryAll(noPermUser));
Mockito.verify(alertPluginInstanceMapper, Mockito.never()).queryAllAlertPluginInstanceList();
}

@Test
public void testQueryAllWithOperationPermission() {

Check warning on line 377 in dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AlertPluginInstanceServiceTest.java

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Remove this 'public' modifier. 


See more on https://sonarcloud.io/project/issues?id=apache-dolphinscheduler&issues=AZ5KAdSTEn5I1ZphW9_X&open=AZ5KAdSTEn5I1ZphW9_X&pullRequest=18279
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) {
Expand All @@ -336,11 +395,12 @@
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);
}

}
Loading