From 82d04f1924692db83461b8c7800f2ab7228825c1 Mon Sep 17 00:00:00 2001 From: Sunny Lei <56599784+SunnyZ-L@users.noreply.github.com> Date: Sat, 15 Jan 2022 09:08:03 +0800 Subject: [PATCH] [DS-6820][feat] Ordinary users can also share resource data source projects with others (#7929) Admin's entire contents of non-selected users Non-admins can only authorize content they create This closes #6820 --- .../service/impl/DataSourceServiceImpl.java | 29 ++-- .../api/service/impl/ProjectServiceImpl.java | 15 +- .../service/impl/ResourcesServiceImpl.java | 46 ++--- .../api/service/impl/UsersServiceImpl.java | 24 +-- .../api/service/DataSourceServiceTest.java | 73 ++++++-- .../api/service/ProjectServiceTest.java | 45 +++-- .../api/service/ResourcesServiceTest.java | 160 ++++++++++++++---- .../api/service/UsersServiceTest.java | 53 +++--- 8 files changed, 297 insertions(+), 148 deletions(-) diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java index c775e2a66a..80a9b3118a 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/DataSourceServiceImpl.java @@ -38,6 +38,7 @@ import org.apache.commons.lang.StringUtils; import java.sql.Connection; import java.util.ArrayList; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -389,30 +390,27 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource */ @Override public Map unauthDatasource(User loginUser, Integer userId) { - Map result = new HashMap<>(); - //only admin operate - if (!isAdmin(loginUser)) { - putMsg(result, Status.USER_NO_OPERATION_PERM); - return result; - } - /** - * query all data sources except userId - */ + List datasourceList; + if (isAdmin(loginUser)) { + // admin gets all data sources except userId + datasourceList = dataSourceMapper.queryDatasourceExceptUserId(userId); + } else { + // non-admins users get their own data sources + datasourceList = dataSourceMapper.selectByMap(Collections.singletonMap("user_id", loginUser.getId())); + } List resultList = new ArrayList<>(); - List datasourceList = dataSourceMapper.queryDatasourceExceptUserId(userId); - Set datasourceSet = null; + Set datasourceSet; if (datasourceList != null && !datasourceList.isEmpty()) { datasourceSet = new HashSet<>(datasourceList); List authedDataSourceList = dataSourceMapper.queryAuthedDatasource(userId); - Set authedDataSourceSet = null; + Set authedDataSourceSet; if (authedDataSourceList != null && !authedDataSourceList.isEmpty()) { authedDataSourceSet = new HashSet<>(authedDataSourceList); datasourceSet.removeAll(authedDataSourceSet); - } resultList = new ArrayList<>(datasourceSet); } @@ -432,11 +430,6 @@ public class DataSourceServiceImpl extends BaseServiceImpl implements DataSource public Map authedDatasource(User loginUser, Integer userId) { Map result = new HashMap<>(); - if (!isAdmin(loginUser)) { - putMsg(result, Status.USER_NO_OPERATION_PERM); - return result; - } - List authedDatasourceList = dataSourceMapper.queryAuthedDatasource(userId); result.put(Constants.DATA_LIST, authedDatasourceList); putMsg(result, Status.SUCCESS); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java index f608af31c0..d35507fe5a 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ProjectServiceImpl.java @@ -344,11 +344,14 @@ public class ProjectServiceImpl extends BaseServiceImpl implements ProjectServic @Override public Map queryUnauthorizedProject(User loginUser, Integer userId) { Map result = new HashMap<>(); - if (loginUser.getId() != userId && isNotAdmin(loginUser, result)) { - return result; + + List projectList; + if (isAdmin(loginUser)) { + // admin gets all projects except userId + projectList = projectMapper.queryProjectExceptUserId(userId); + } else { + projectList = projectMapper.queryProjectCreatedByUser(loginUser.getId()); } - // query all project list except specified userId - List projectList = projectMapper.queryProjectExceptUserId(userId); List resultList = new ArrayList<>(); Set projectSet; if (projectList != null && !projectList.isEmpty()) { @@ -393,10 +396,6 @@ public class ProjectServiceImpl extends BaseServiceImpl implements ProjectServic public Map queryAuthorizedProject(User loginUser, Integer userId) { Map result = new HashMap<>(); - if (loginUser.getId() != userId && isNotAdmin(loginUser, result)) { - return result; - } - List projects = projectMapper.queryAuthedProjectListByUserId(userId); result.put(Constants.DATA_LIST, projects); putMsg(result, Status.SUCCESS); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java index 0ebf546c01..9892dd527b 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/ResourcesServiceImpl.java @@ -59,6 +59,7 @@ import java.io.IOException; import java.text.MessageFormat; import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -1130,12 +1131,16 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe */ @Override public Map authorizeResourceTree(User loginUser, Integer userId) { - Map result = new HashMap<>(); - if (isNotAdmin(loginUser, result)) { - return result; + + List resourceList; + if (isAdmin(loginUser)) { + // admin gets all resources except userId + resourceList = resourcesMapper.queryResourceExceptUserId(userId); + } else { + // non-admins users get their own resources + resourceList = resourcesMapper.queryResourceListAuthored(loginUser.getId(), -1); } - List resourceList = resourcesMapper.queryResourceExceptUserId(userId); List list; if (CollectionUtils.isNotEmpty(resourceList)) { Visitor visitor = new ResourceTreeVisitor(resourceList); @@ -1158,12 +1163,16 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe */ @Override public Map unauthorizedFile(User loginUser, Integer userId) { - Map result = new HashMap<>(); - if (isNotAdmin(loginUser, result)) { - return result; + + List resourceList; + if (isAdmin(loginUser)) { + // admin gets all resources except userId + resourceList = resourcesMapper.queryResourceExceptUserId(userId); + } else { + // non-admins users get their own resources + resourceList = resourcesMapper.queryResourceListAuthored(loginUser.getId(), -1); } - List resourceList = resourcesMapper.queryResourceExceptUserId(userId); List list; if (resourceList != null && !resourceList.isEmpty()) { Set resourceSet = new HashSet<>(resourceList); @@ -1189,12 +1198,15 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe @Override public Map unauthorizedUDFFunction(User loginUser, Integer userId) { Map result = new HashMap<>(); - //only admin can operate - if (isNotAdmin(loginUser, result)) { - return result; - } - List udfFuncList = udfFunctionMapper.queryUdfFuncExceptUserId(userId); + List udfFuncList; + if (isAdmin(loginUser)) { + // admin gets all udfs except userId + udfFuncList = udfFunctionMapper.queryUdfFuncExceptUserId(userId); + } else { + // non-admins users get their own udfs + udfFuncList = udfFunctionMapper.selectByMap(Collections.singletonMap("user_id", loginUser.getId())); + } List resultList = new ArrayList<>(); Set udfFuncSet; if (CollectionUtils.isNotEmpty(udfFuncList)) { @@ -1220,9 +1232,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe @Override public Map authorizedUDFFunction(User loginUser, Integer userId) { Map result = new HashMap<>(); - if (isNotAdmin(loginUser, result)) { - return result; - } + List udfFuncs = udfFunctionMapper.queryAuthedUdfFunc(userId); result.put(Constants.DATA_LIST, udfFuncs); putMsg(result, Status.SUCCESS); @@ -1239,9 +1249,7 @@ public class ResourcesServiceImpl extends BaseServiceImpl implements ResourcesSe @Override public Map authorizedFile(User loginUser, Integer userId) { Map result = new HashMap<>(); - if (isNotAdmin(loginUser, result)) { - return result; - } + List authedResources = queryResourceList(userId, Constants.AUTHORIZE_WRITABLE_PERM); Visitor visitor = new ResourceTreeVisitor(authedResources); String visit = JSONUtils.toJsonString(visitor.visit(), SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS); diff --git a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java index 7790b86a92..b057bae81d 100644 --- a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java +++ b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java @@ -548,11 +548,6 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { Map result = new HashMap<>(); result.put(Constants.STATUS, false); - //only admin can operate - if (check(result, !isAdmin(loginUser), Status.USER_NO_OPERATION_PERM)) { - return result; - } - //check exist User tempUser = userMapper.selectById(userId); if (tempUser == null) { @@ -573,7 +568,7 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { ProjectUser projectUser = new ProjectUser(); projectUser.setUserId(userId); projectUser.setProjectId(Integer.parseInt(projectId)); - projectUser.setPerm(7); + projectUser.setPerm(Constants.AUTHORIZE_WRITABLE_PERM); projectUser.setCreateTime(now); projectUser.setUpdateTime(now); projectUserMapper.insert(projectUser); @@ -680,10 +675,7 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { @Transactional(rollbackFor = RuntimeException.class) public Map grantResources(User loginUser, int userId, String resourceIds) { Map result = new HashMap<>(); - //only admin can operate - if (check(result, !isAdmin(loginUser), Status.USER_NO_OPERATION_PERM)) { - return result; - } + User user = userMapper.selectById(userId); if (user == null) { putMsg(result, Status.USER_NOT_EXIST, userId); @@ -778,10 +770,6 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { public Map grantUDFFunction(User loginUser, int userId, String udfIds) { Map result = new HashMap<>(); - //only admin can operate - if (check(result, !isAdmin(loginUser), Status.USER_NO_OPERATION_PERM)) { - return result; - } User user = userMapper.selectById(userId); if (user == null) { putMsg(result, Status.USER_NOT_EXIST, userId); @@ -801,7 +789,7 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { UDFUser udfUser = new UDFUser(); udfUser.setUserId(userId); udfUser.setUdfId(Integer.parseInt(udfId)); - udfUser.setPerm(7); + udfUser.setPerm(Constants.AUTHORIZE_WRITABLE_PERM); udfUser.setCreateTime(now); udfUser.setUpdateTime(now); udfUserMapper.insert(udfUser); @@ -826,10 +814,6 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { Map result = new HashMap<>(); result.put(Constants.STATUS, false); - //only admin can operate - if (check(result, !isAdmin(loginUser), Status.USER_NO_OPERATION_PERM)) { - return result; - } User user = userMapper.selectById(userId); if (user == null) { putMsg(result, Status.USER_NOT_EXIST, userId); @@ -850,7 +834,7 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService { DatasourceUser datasourceUser = new DatasourceUser(); datasourceUser.setUserId(userId); datasourceUser.setDatasourceId(Integer.parseInt(datasourceId)); - datasourceUser.setPerm(7); + datasourceUser.setPerm(Constants.AUTHORIZE_WRITABLE_PERM); datasourceUser.setCreateTime(now); datasourceUser.setUpdateTime(now); datasourceUserMapper.insert(datasourceUser); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java index 3a5c026680..cd3493646c 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/DataSourceServiceTest.java @@ -17,6 +17,7 @@ package org.apache.dolphinscheduler.api.service; +import org.apache.commons.collections.CollectionUtils; import org.apache.dolphinscheduler.api.enums.Status; import org.apache.dolphinscheduler.api.service.impl.DataSourceServiceImpl; import org.apache.dolphinscheduler.api.utils.Result; @@ -42,6 +43,7 @@ import org.apache.dolphinscheduler.spi.utils.PropertyUtils; import java.sql.Connection; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -56,6 +58,8 @@ import org.powermock.api.mockito.PowerMockito; import org.powermock.core.classloader.annotations.PowerMockIgnore; import org.powermock.core.classloader.annotations.PrepareForTest; import org.powermock.modules.junit4.PowerMockRunner; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * data source service test @@ -65,6 +69,8 @@ import org.powermock.modules.junit4.PowerMockRunner; @PrepareForTest({DataSourceUtils.class, CommonUtils.class, DataSourceClientProvider.class, PasswordUtils.class}) public class DataSourceServiceTest { + private static final Logger logger = LoggerFactory.getLogger(DataSourceServiceTest.class); + @InjectMocks private DataSourceServiceImpl dataSourceService; @@ -224,30 +230,47 @@ public class DataSourceServiceTest { @Test public void unauthDatasourceTest() { User loginUser = getAdminUser(); - int userId = -1; - - //user no operation perm - Map noOperationPerm = dataSourceService.unauthDatasource(loginUser, userId); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, noOperationPerm.get(Constants.STATUS)); - - //success + loginUser.setId(1); loginUser.setUserType(UserType.ADMIN_USER); - Map success = dataSourceService.unauthDatasource(loginUser, userId); - Assert.assertEquals(Status.SUCCESS, success.get(Constants.STATUS)); + int userId = 3; + + // test admin user + Mockito.when(dataSourceMapper.queryAuthedDatasource(userId)).thenReturn(getSingleDataSourceList()); + Mockito.when(dataSourceMapper.queryDatasourceExceptUserId(userId)).thenReturn(getDataSourceList()); + Map result = dataSourceService.unauthDatasource(loginUser, userId); + logger.info(result.toString()); + List dataSources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(dataSources)); + + // test non-admin user + loginUser.setId(2); + loginUser.setUserType(UserType.GENERAL_USER); + Mockito.when(dataSourceMapper.selectByMap(Collections.singletonMap("user_id", loginUser.getId()))).thenReturn(getDataSourceList()); + result = dataSourceService.unauthDatasource(loginUser, userId); + logger.info(result.toString()); + dataSources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(dataSources)); } @Test public void authedDatasourceTest() { User loginUser = getAdminUser(); - int userId = -1; + loginUser.setId(1); + loginUser.setUserType(UserType.ADMIN_USER); + int userId = 3; - //user no operation perm - Map noOperationPerm = dataSourceService.authedDatasource(loginUser, userId); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, noOperationPerm.get(Constants.STATUS)); + // test admin user + Mockito.when(dataSourceMapper.queryAuthedDatasource(userId)).thenReturn(getSingleDataSourceList()); + Map result = dataSourceService.authedDatasource(loginUser, userId); + logger.info(result.toString()); + List dataSources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(dataSources)); - //success - loginUser.setUserType(UserType.ADMIN_USER); + // test non-admin user + loginUser.setId(2); + loginUser.setUserType(UserType.GENERAL_USER); Map success = dataSourceService.authedDatasource(loginUser, userId); + logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, success.get(Constants.STATUS)); } @@ -283,10 +306,16 @@ public class DataSourceServiceTest { private List getDataSourceList() { List dataSources = new ArrayList<>(); - dataSources.add(getOracleDataSource()); + dataSources.add(getOracleDataSource(1)); + dataSources.add(getOracleDataSource(2)); + dataSources.add(getOracleDataSource(3)); return dataSources; } + private List getSingleDataSourceList() { + return Collections.singletonList(getOracleDataSource(3)); + } + private DataSource getOracleDataSource() { DataSource dataSource = new DataSource(); dataSource.setName("test"); @@ -298,6 +327,18 @@ public class DataSourceServiceTest { return dataSource; } + private DataSource getOracleDataSource(int dataSourceId) { + DataSource dataSource = new DataSource(); + dataSource.setId(dataSourceId); + dataSource.setName("test"); + dataSource.setNote("Note"); + dataSource.setType(DbType.ORACLE); + dataSource.setConnectionParams("{\"connectType\":\"ORACLE_SID\",\"address\":\"jdbc:oracle:thin:@192.168.xx.xx:49161\",\"database\":\"XE\"," + + "\"jdbcUrl\":\"jdbc:oracle:thin:@192.168.xx.xx:49161/XE\",\"user\":\"system\",\"password\":\"oracle\"}"); + + return dataSource; + } + @Test public void buildParameter() { OracleDataSourceParamDTO oracleDatasourceParamDTO = new OracleDataSourceParamDTO(); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProjectServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProjectServiceTest.java index 9cfa94eddb..0c5579b685 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProjectServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ProjectServiceTest.java @@ -254,24 +254,21 @@ public class ProjectServiceTest { @Test public void testQueryAuthorizedProject() { + Mockito.when(projectMapper.queryAuthedProjectListByUserId(2)).thenReturn(getList()); User loginUser = getLoginUser(); - Mockito.when(projectMapper.queryAuthedProjectListByUserId(1)).thenReturn(getList()); - //USER_NO_OPERATION_PERM - Map result = projectService.queryAuthorizedProject(loginUser, 3); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); - - //success + // test admin user loginUser.setUserType(UserType.ADMIN_USER); - result = projectService.queryAuthorizedProject(loginUser, 1); + Map result = projectService.queryAuthorizedProject(loginUser, 2); logger.info(result.toString()); List projects = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(projects)); + // test non-admin user loginUser.setUserType(UserType.GENERAL_USER); - result = projectService.queryAuthorizedProject(loginUser, loginUser.getId()); + loginUser.setId(3); + result = projectService.queryAuthorizedProject(loginUser, 2); projects = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(projects)); } @@ -359,16 +356,25 @@ public class ProjectServiceTest { @Test public void testQueryUnauthorizedProject() { - // Mockito.when(projectMapper.queryAuthedProjectListByUserId(1)).thenReturn(getList()); Mockito.when(projectMapper.queryProjectExceptUserId(2)).thenReturn(getList()); + Mockito.when(projectMapper.queryProjectCreatedByUser(2)).thenReturn(getList()); + Mockito.when(projectMapper.queryAuthedProjectListByUserId(2)).thenReturn(getSingleList()); + // test admin user User loginUser = new User(); loginUser.setUserType(UserType.ADMIN_USER); - Map result = projectService.queryUnauthorizedProject(loginUser, 2); logger.info(result.toString()); List projects = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(projects)); + + // test non-admin user + loginUser.setId(2); + loginUser.setUserType(UserType.GENERAL_USER); + result = projectService.queryUnauthorizedProject(loginUser, 3); + logger.info(result.toString()); + projects = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(projects)); } private Project getProject() { @@ -380,12 +386,27 @@ public class ProjectServiceTest { return project; } + private Project getProject(int projectId) { + Project project = new Project(); + project.setId(projectId); + project.setCode(1L); + project.setName(projectName); + project.setUserId(1); + return project; + } + private List getList() { List list = new ArrayList<>(); - list.add(getProject()); + list.add(getProject(1)); + list.add(getProject(2)); + list.add(getProject(3)); return list; } + private List getSingleList() { + return Collections.singletonList(getProject(2)); + } + /** * create admin user */ diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java index 6a7d7df81f..f438f63e0f 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/ResourcesServiceTest.java @@ -42,6 +42,7 @@ import org.apache.commons.collections.CollectionUtils; import java.io.IOException; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -553,80 +554,143 @@ public class ResourcesServiceTest { } @Test - public void testUnauthorizedFile() { + public void testAuthorizeResourceTree() { User user = getUser(); - //USER_NO_OPERATION_PERM - Map result = resourcesService.unauthorizedFile(user, 1); + user.setId(1); + user.setUserType(UserType.ADMIN_USER); + int userId = 3; + + // test admin user + List resIds = new ArrayList<>(); + resIds.add(1); + Mockito.when(resourcesMapper.queryResourceExceptUserId(userId)).thenReturn(getResourceList()); + Map result = resourcesService.authorizeResourceTree(user, userId); + logger.info(result.toString()); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + List resources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); + + // test non-admin user + user.setId(2); + user.setUserType(UserType.GENERAL_USER); + Mockito.when(resourcesMapper.queryResourceListAuthored(user.getId(), -1)).thenReturn(getResourceList()); + result = resourcesService.authorizeResourceTree(user, userId); logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + resources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); + } - //SUCCESS + @Test + public void testUnauthorizedFile() { + User user = getUser(); + user.setId(1); user.setUserType(UserType.ADMIN_USER); - Mockito.when(resourcesMapper.queryResourceExceptUserId(1)).thenReturn(getResourceList()); - result = resourcesService.unauthorizedFile(user, 1); + int userId = 3; + + // test admin user + List resIds = new ArrayList<>(); + resIds.add(1); + Mockito.when(resourcesMapper.queryResourceExceptUserId(userId)).thenReturn(getResourceList()); + Mockito.when(resourceUserMapper.queryResourcesIdListByUserIdAndPerm(Mockito.anyInt(), Mockito.anyInt())).thenReturn(resIds); + Mockito.when(resourcesMapper.queryResourceListById(Mockito.any())).thenReturn(getSingleResourceList()); + Map result = resourcesService.unauthorizedFile(user, userId); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); List resources = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); + // test non-admin user + user.setId(2); + user.setUserType(UserType.GENERAL_USER); + Mockito.when(resourcesMapper.queryResourceListAuthored(user.getId(), -1)).thenReturn(getResourceList()); + result = resourcesService.unauthorizedFile(user, userId); + logger.info(result.toString()); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + resources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); } @Test public void testUnauthorizedUDFFunction() { - User user = getUser(); - //USER_NO_OPERATION_PERM - Map result = resourcesService.unauthorizedUDFFunction(user, 1); + user.setId(1); + user.setUserType(UserType.ADMIN_USER); + int userId = 3; + + // test admin user + Mockito.when(udfFunctionMapper.queryUdfFuncExceptUserId(userId)).thenReturn(getUdfFuncList()); + Mockito.when(udfFunctionMapper.queryAuthedUdfFunc(userId)).thenReturn(getSingleUdfFuncList()); + Map result = resourcesService.unauthorizedUDFFunction(user, userId); logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + List udfFuncs = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(udfFuncs)); - //SUCCESS - user.setUserType(UserType.ADMIN_USER); - Mockito.when(udfFunctionMapper.queryUdfFuncExceptUserId(1)).thenReturn(getUdfFuncList()); - result = resourcesService.unauthorizedUDFFunction(user, 1); + // test non-admin user + user.setId(2); + user.setUserType(UserType.GENERAL_USER); + Mockito.when(udfFunctionMapper.selectByMap(Collections.singletonMap("user_id", user.getId()))).thenReturn(getUdfFuncList()); + result = resourcesService.unauthorizedUDFFunction(user, userId); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); - List udfFuncs = (List) result.get(Constants.DATA_LIST); + udfFuncs = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(udfFuncs)); } @Test public void testAuthorizedUDFFunction() { User user = getUser(); - //USER_NO_OPERATION_PERM - Map result = resourcesService.authorizedUDFFunction(user, 1); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); - //SUCCESS + user.setId(1); user.setUserType(UserType.ADMIN_USER); - Mockito.when(udfFunctionMapper.queryAuthedUdfFunc(1)).thenReturn(getUdfFuncList()); - result = resourcesService.authorizedUDFFunction(user, 1); + int userId = 3; + + // test admin user + Mockito.when(udfFunctionMapper.queryAuthedUdfFunc(userId)).thenReturn(getUdfFuncList()); + Map result = resourcesService.authorizedUDFFunction(user, userId); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); List udfFuncs = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(udfFuncs)); + + // test non-admin user + user.setUserType(UserType.GENERAL_USER); + user.setId(2); + Mockito.when(udfFunctionMapper.queryAuthedUdfFunc(userId)).thenReturn(getUdfFuncList()); + result = resourcesService.authorizedUDFFunction(user, userId); + logger.info(result.toString()); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + udfFuncs = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(udfFuncs)); } @Test public void testAuthorizedFile() { - User user = getUser(); - //USER_NO_OPERATION_PERM - Map result = resourcesService.authorizedFile(user, 1); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); - //SUCCESS + user.setId(1); user.setUserType(UserType.ADMIN_USER); + int userId = 3; + // test admin user List resIds = new ArrayList<>(); resIds.add(1); Mockito.when(resourceUserMapper.queryResourcesIdListByUserIdAndPerm(Mockito.anyInt(), Mockito.anyInt())).thenReturn(resIds); Mockito.when(resourcesMapper.queryResourceListById(Mockito.any())).thenReturn(getResourceList()); - result = resourcesService.authorizedFile(user, 1); + Map result = resourcesService.authorizedFile(user, userId); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); List resources = (List) result.get(Constants.DATA_LIST); Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); + + // test non-admin user + user.setId(2); + user.setUserType(UserType.GENERAL_USER); + Mockito.when(resourceUserMapper.queryResourcesIdListByUserIdAndPerm(Mockito.anyInt(), Mockito.anyInt())).thenReturn(resIds); + Mockito.when(resourcesMapper.queryResourceListById(Mockito.any())).thenReturn(getResourceList()); + result = resourcesService.authorizedFile(user, userId); + logger.info(result.toString()); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + resources = (List) result.get(Constants.DATA_LIST); + Assert.assertTrue(CollectionUtils.isNotEmpty(resources)); } @Test @@ -650,10 +714,16 @@ public class ResourcesServiceTest { private List getResourceList() { List resources = new ArrayList<>(); - resources.add(getResource()); + resources.add(getResource(1)); + resources.add(getResource(2)); + resources.add(getResource(3)); return resources; } + private List getSingleResourceList() { + return Collections.singletonList(getResource(1)); + } + private Tenant getTenant() { Tenant tenant = new Tenant(); tenant.setTenantCode("123"); @@ -672,6 +742,19 @@ public class ResourcesServiceTest { return resource; } + private Resource getResource(int resourceId) { + + Resource resource = new Resource(); + resource.setId(resourceId); + resource.setPid(-1); + resource.setUserId(1); + resource.setDescription("ResourcesServiceTest.jar"); + resource.setAlias("ResourcesServiceTest.jar"); + resource.setFullName("/ResourcesServiceTest.jar"); + resource.setType(ResourceType.FILE); + return resource; + } + private Resource getUdfResource() { Resource resource = new Resource(); @@ -690,13 +773,26 @@ public class ResourcesServiceTest { return udfFunc; } + private UdfFunc getUdfFunc(int udfId) { + + UdfFunc udfFunc = new UdfFunc(); + udfFunc.setId(udfId); + return udfFunc; + } + private List getUdfFuncList() { List udfFuncs = new ArrayList<>(); - udfFuncs.add(getUdfFunc()); + udfFuncs.add(getUdfFunc(1)); + udfFuncs.add(getUdfFunc(2)); + udfFuncs.add(getUdfFunc(3)); return udfFuncs; } + private List getSingleUdfFuncList() { + return Collections.singletonList(getUdfFunc(3)); + } + private User getUser() { User user = new User(); user.setId(1); diff --git a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java index 2b17955a3a..36a313bedc 100644 --- a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java +++ b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java @@ -321,19 +321,21 @@ public class UsersServiceTest { @Test public void testGrantProject() { - when(userMapper.selectById(1)).thenReturn(getUser()); - User loginUser = new User(); String projectIds = "100000,120000"; - Map result = usersService.grantProject(loginUser, 1, projectIds); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + User loginUser = new User(); + int userId = 3; + //user not exist + loginUser.setId(1); loginUser.setUserType(UserType.ADMIN_USER); - result = usersService.grantProject(loginUser, 2, projectIds); + when(userMapper.selectById(userId)).thenReturn(null); + Map result = usersService.grantProject(loginUser, userId, projectIds); logger.info(result.toString()); Assert.assertEquals(Status.USER_NOT_EXIST, result.get(Constants.STATUS)); - //success - result = usersService.grantProject(loginUser, 1, projectIds); + + //SUCCESS + when(userMapper.selectById(userId)).thenReturn(getUser()); + result = usersService.grantProject(loginUser, userId, projectIds); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); } @@ -411,12 +413,10 @@ public class UsersServiceTest { String resourceIds = "100000,120000"; when(userMapper.selectById(1)).thenReturn(getUser()); User loginUser = new User(); - Map result = usersService.grantResources(loginUser, 1, resourceIds); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + //user not exist loginUser.setUserType(UserType.ADMIN_USER); - result = usersService.grantResources(loginUser, 2, resourceIds); + Map result = usersService.grantResources(loginUser, 2, resourceIds); logger.info(result.toString()); Assert.assertEquals(Status.USER_NOT_EXIST, result.get(Constants.STATUS)); //success @@ -433,12 +433,10 @@ public class UsersServiceTest { String udfIds = "100000,120000"; when(userMapper.selectById(1)).thenReturn(getUser()); User loginUser = new User(); - Map result = usersService.grantUDFFunction(loginUser, 1, udfIds); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + //user not exist loginUser.setUserType(UserType.ADMIN_USER); - result = usersService.grantUDFFunction(loginUser, 2, udfIds); + Map result = usersService.grantUDFFunction(loginUser, 2, udfIds); logger.info(result.toString()); Assert.assertEquals(Status.USER_NOT_EXIST, result.get(Constants.STATUS)); //success @@ -451,19 +449,28 @@ public class UsersServiceTest { @Test public void testGrantDataSource() { String datasourceIds = "100000,120000"; - when(userMapper.selectById(1)).thenReturn(getUser()); User loginUser = new User(); - Map result = usersService.grantDataSource(loginUser, 1, datasourceIds); - logger.info(result.toString()); - Assert.assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS)); + int userId = 3; + //user not exist + loginUser.setId(1); loginUser.setUserType(UserType.ADMIN_USER); - result = usersService.grantDataSource(loginUser, 2, datasourceIds); + when(userMapper.selectById(userId)).thenReturn(null); + Map result = usersService.grantDataSource(loginUser, userId, datasourceIds); logger.info(result.toString()); Assert.assertEquals(Status.USER_NOT_EXIST, result.get(Constants.STATUS)); - //success + + // test admin user + when(userMapper.selectById(userId)).thenReturn(getUser()); when(datasourceUserMapper.deleteByUserId(Mockito.anyInt())).thenReturn(1); - result = usersService.grantDataSource(loginUser, 1, datasourceIds); + result = usersService.grantDataSource(loginUser, userId, datasourceIds); + logger.info(result.toString()); + Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS)); + + // test non-admin user + loginUser.setId(2); + loginUser.setUserType(UserType.GENERAL_USER); + result = usersService.grantDataSource(loginUser, userId, datasourceIds); logger.info(result.toString()); Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));