This is an automated email from the ASF dual-hosted git repository.
journey pushed a commit to branch dev-1.2.1
in repository https://gitbox.apache.org/repos/asf/incubator-dolphinscheduler.git
The following commit(s) were added to refs/heads/dev-1.2.1 by this push:
new d75485a [Fix #1828]check whether has permission to download udf file
or delete udf function (#1858)
d75485a is described below
commit d75485a1672af2e3b0f00c3466f094295a52354e
Author: lgcareer <[email protected]>
AuthorDate: Sun Jan 19 12:00:58 2020 +0800
[Fix #1828]check whether has permission to download udf file or delete udf
function (#1858)
* fix issue 1828:get the udf resource path error when create udf function
* update grantResources
* first verify whether udf resource is bound by udf function
* update grantResources
* update testListAuthorizedUdfFunc
* update getUserInfo in order to run success
* check whether has permission to download udf file or delete udf file
* update listAuthorizedResourceById in ResourceMapper.xml
---
.../api/controller/ResourcesController.java | 2 +-
.../api/service/UdfFuncService.java | 17 +++++++--
.../dolphinscheduler/api/service/UsersService.java | 29 ++++++++++------
.../api/service/UsersServiceTest.java | 8 +++--
.../common/enums/AuthorizationType.java | 6 ++--
.../apache/dolphinscheduler/dao/ProcessDao.java | 4 +++
.../dao/mapper/ResourceMapper.java | 8 +++++
.../dolphinscheduler/dao/mapper/UdfFuncMapper.java | 7 ++++
.../dolphinscheduler/dao/mapper/ResourceMapper.xml | 12 +++++++
.../dolphinscheduler/dao/mapper/UdfFuncMapper.xml | 13 +++++++
.../dao/mapper/UdfFuncMapperTest.java | 3 ++
.../server/worker/task/sql/SqlTask.java | 40 +++++++++++++++-------
12 files changed, 117 insertions(+), 32 deletions(-)
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
index 7bac661..43a44bb 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/controller/ResourcesController.java
@@ -620,7 +620,7 @@ public class ResourcesController extends BaseController{
try{
logger.info("login user {}, delete udf function id: {}",
loginUser.getUserName(),udfFuncId);
- return udfFuncService.delete(udfFuncId);
+ return udfFuncService.delete(loginUser,udfFuncId);
}catch (Exception e){
logger.error(DELETE_UDF_FUNCTION_ERROR.getMsg(),e);
return error(Status.DELETE_UDF_FUNCTION_ERROR.getCode(),
Status.DELETE_UDF_FUNCTION_ERROR.getMsg());
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
index adff29d..37717bb 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UdfFuncService.java
@@ -301,13 +301,24 @@ public class UdfFuncService extends BaseService{
/**
* delete udf function
*
- * @param id udf function id
+ * @param id udf function id
+ * @param loginUser login user
* @return delete result code
*/
@Transactional(rollbackFor = Exception.class)
- public Result delete(int id) {
+ public Result delete(User loginUser,int id) {
Result result = new Result();
-
+
+ UdfFunc udfFunc = udfFuncMapper.selectUdfById(id);
+ if(udfFunc == null){
+ putMsg(result,Status.UDF_FUNCTION_NOT_EXIST);
+ return result;
+ }
+
+ if (!hasPerm(loginUser,udfFunc.getUserId())){
+ putMsg(result,Status.USER_NO_OPERATION_PERM);
+ return result;
+ }
udfFuncMapper.deleteById(id);
udfUserMapper.deleteByUdfFuncId(id);
putMsg(result, Status.SUCCESS);
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UsersService.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UsersService.java
index dd04700..e9f6e0a 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UsersService.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/UsersService.java
@@ -438,21 +438,30 @@ public class UsersService extends BaseService {
return result;
}
String[] resourcesIdArr = resourceIds.split(",");
- //if resource type is UDF,need check whether it is bound by UDF functon
Set<Integer> needAuthorizedIds = new HashSet<>();
if (StringUtils.isNotEmpty(resourceIds)) {
needAuthorizedIds =
Arrays.stream(resourcesIdArr).map(t->Integer.parseInt(t)).collect(Collectors.toSet());
}
- List<Resource> udfResourceList = resourceMapper.queryResourceList("",
0, ResourceType.UDF.ordinal());
- Set<Integer> allUdfResIds = udfResourceList.stream().map(t ->
t.getId()).collect(Collectors.toSet());
- allUdfResIds.removeAll(needAuthorizedIds);
- List<UdfFunc> udfFuncs =
udfFuncMapper.listUdfByResourceId(ArrayUtils.toPrimitive(allUdfResIds.toArray(new
Integer[allUdfResIds.size()])));
- if (CollectionUtils.isNotEmpty(udfFuncs)) {
- logger.error("can't be deleted,because it is bound by UDF
functions:{}",udfFuncs.toString());
- putMsg(result, Status.UDF_RESOURCE_IS_BOUND,
udfFuncs.get(0).getFuncName());
- return result;
- }
+ //if resource type is UDF,need check whether it is bound by UDF functon
+
+ //get the authorized resource id list by user id and resource type
+ List<Resource> oldAuthorizedUdfRes =
resourceMapper.queryResourceListAuthored(userId, ResourceType.UDF.ordinal());
+ Set<Integer> oldAuthorizedUdfResIds =
oldAuthorizedUdfRes.stream().map(t -> t.getId()).collect(Collectors.toSet());
+
+ //get the unauthorized resource id list
+ oldAuthorizedUdfResIds.removeAll(needAuthorizedIds);
+
+ if (CollectionUtils.isNotEmpty(oldAuthorizedUdfResIds)) {
+ int[] unauthorizedResIds =
ArrayUtils.toPrimitive(oldAuthorizedUdfResIds.toArray(new
Integer[oldAuthorizedUdfResIds.size()]));
+ List<UdfFunc> authorizedUdfFuncs =
udfFuncMapper.listAuthorizedUdfByResourceId(userId, unauthorizedResIds);
+
+ if (CollectionUtils.isNotEmpty(authorizedUdfFuncs)) {
+ logger.error("can't be deleted,because it is bound by UDF
functions:{}",authorizedUdfFuncs.toString());
+ putMsg(result, Status.UDF_RESOURCE_IS_BOUND,
authorizedUdfFuncs.get(0).getFuncName());
+ return result;
+ }
+ }
resourcesUserMapper.deleteResourceUser(userId, 0);
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 fd51f91..991daef 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
@@ -297,18 +297,19 @@ public class UsersServiceTest {
logger.info(result.toString());
Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
- List<Resource> udfResourceList = new ArrayList<Resource>() {{
+ //the udf resources needAuthorizedUser has the permission to use
+ List<Resource> oldAuthorizedUdfRes = new ArrayList<Resource>() {{
add(createResource(getAdminUser(), ResourceType.UDF, 100000));
add(createResource(getAdminUser(), ResourceType.UDF, 120000));
}};
- when(resourceMapper.queryResourceList("", 0,
ResourceType.UDF.ordinal())).thenReturn(udfResourceList);
+
when(resourceMapper.queryResourceListAuthored(needAuthorizedUser.getId(),
ResourceType.UDF.ordinal())).thenReturn(oldAuthorizedUdfRes);
//mock udf function list
UdfFunc udfFunc = createUdfFunc(getAdminUser(), 100000);
List<UdfFunc> udfFuncs = new ArrayList<>();
udfFuncs.add(udfFunc);
- when(udfFuncMapper.listUdfByResourceId(new
int[]{100000})).thenReturn(udfFuncs);
+
when(udfFuncMapper.listAuthorizedUdfByResourceId(needAuthorizedUser.getId(),new
int[]{100000})).thenReturn(udfFuncs);
//fail if udf resource is already bound by the udf function
result = usersService.grantResources(adminUser,
needAuthorizedUser.getId(), "120000");
@@ -316,6 +317,7 @@ public class UsersServiceTest {
result = usersService.grantResources(adminUser,
needAuthorizedUser.getId(), "100000");
Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
+ logger.info(result.toString());
}
diff --git
a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuthorizationType.java
b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuthorizationType.java
index 1c371e7..4c13134 100644
---
a/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuthorizationType.java
+++
b/dolphinscheduler-common/src/main/java/org/apache/dolphinscheduler/common/enums/AuthorizationType.java
@@ -24,12 +24,14 @@ import com.baomidou.mybatisplus.annotation.EnumValue;
public enum AuthorizationType {
/**
* 0 RESOURCE_FILE;
+ * 1 UDF_FILE;
* 1 DATASOURCE;
* 2 UDF;
*/
RESOURCE_FILE(0, "resource file"),
- DATASOURCE(1, "data source"),
- UDF(2, "udf function");
+ UDF_FILE(1, "udf file"),
+ DATASOURCE(2, "data source"),
+ UDF(3, "udf function");
AuthorizationType(int code, String descp){
this.code = code;
diff --git
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/ProcessDao.java
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/ProcessDao.java
index aabd6b4..19e87f8 100644
---
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/ProcessDao.java
+++
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/ProcessDao.java
@@ -1789,6 +1789,10 @@ public class ProcessDao {
Set<String> authorizedResources =
resourceMapper.listAuthorizedResource(userId, needChecks).stream().map(t ->
t.getAlias()).collect(toSet());
originResSet.removeAll(authorizedResources);
break;
+ case UDF_FILE:
+ Set<Integer> authorizedUdfFiles =
resourceMapper.listAuthorizedResourceById(userId, needChecks).stream().map(t ->
t.getId()).collect(toSet());
+ originResSet.removeAll(authorizedUdfFiles);
+ break;
case DATASOURCE:
Set<Integer> authorizedDatasources =
dataSourceMapper.listAuthorizedDataSource(userId,needChecks).stream().map(t ->
t.getId()).collect(toSet());
originResSet.removeAll(authorizedDatasources);
diff --git
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java
index c407811..3d0b183 100644
---
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java
+++
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.java
@@ -99,4 +99,12 @@ public interface ResourceMapper extends BaseMapper<Resource>
{
* @return resource list
*/
<T> List<Resource> listAuthorizedResource(@Param("userId") int
userId,@Param("resNames")T[] resNames);
+
+ /**
+ * list authorized resource
+ * @param userId userId
+ * @param resIds resource ids
+ * @return resource list
+ */
+ <T> List<Resource> listAuthorizedResourceById(@Param("userId") int
userId,@Param("resIds")T[] resIds);
}
diff --git
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.java
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.java
index 7e7850c..2411c9b 100644
---
a/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.java
+++
b/dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.java
@@ -93,5 +93,12 @@ public interface UdfFuncMapper extends BaseMapper<UdfFunc> {
*/
List<UdfFunc> listUdfByResourceId(@Param("resourceIds") int[] resourceIds);
+ /**
+ * list authorized UDF by resource id
+ * @param resourceIds resource id array
+ * @return UDF function list
+ */
+ List<UdfFunc> listAuthorizedUdfByResourceId(@Param("userId") int
userId,@Param("resourceIds") int[] resourceIds);
+
}
diff --git
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml
index b451d62..c9dc2b3 100644
---
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml
+++
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/ResourceMapper.xml
@@ -86,5 +86,17 @@
</foreach>
</if>
</select>
+ <select id="listAuthorizedResourceById"
resultType="org.apache.dolphinscheduler.dao.entity.Resource">
+ select *
+ from t_ds_resources
+ where id in (select resources_id from t_ds_relation_resources_user
where user_id=#{userId}
+ union select id as resources_id from t_ds_resources where
user_id=#{userId})
+ <if test="resIds != null and resIds != ''">
+ and id in
+ <foreach collection="resIds" item="i" open="(" close=")"
separator=",">
+ #{i}
+ </foreach>
+ </if>
+ </select>
</mapper>
diff --git
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.xml
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.xml
index 512312a..e38d163 100644
---
a/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.xml
+++
b/dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapper.xml
@@ -98,4 +98,17 @@
</foreach>
</if>
</select>
+ <select id="listAuthorizedUdfByResourceId"
resultType="org.apache.dolphinscheduler.dao.entity.UdfFunc">
+ select *
+ from t_ds_udfs
+ where
+ id in (select udf_id from t_ds_relation_udfs_user where
user_id=#{userId}
+ union select id as udf_id from t_ds_udfs where user_id=#{userId})
+ <if test="resourceIds != null and resourceIds != ''">
+ and resource_id in
+ <foreach collection="resourceIds" item="i" open="(" close=")"
separator=",">
+ #{i}
+ </foreach>
+ </if>
+ </select>
</mapper>
\ No newline at end of file
diff --git
a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapperTest.java
b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapperTest.java
index c4d2927..b0704b7 100644
---
a/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapperTest.java
+++
b/dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/UdfFuncMapperTest.java
@@ -23,6 +23,7 @@ import org.apache.commons.lang3.ArrayUtils;
import org.apache.dolphinscheduler.common.enums.ResourceType;
import org.apache.dolphinscheduler.common.enums.UdfType;
import org.apache.dolphinscheduler.common.enums.UserType;
+import org.apache.dolphinscheduler.common.utils.CollectionUtils;
import org.apache.dolphinscheduler.dao.entity.Resource;
import org.apache.dolphinscheduler.dao.entity.UDFUser;
import org.apache.dolphinscheduler.dao.entity.UdfFunc;
@@ -39,6 +40,8 @@ import
org.springframework.transaction.annotation.Transactional;
import java.util.Arrays;
import java.util.Date;
import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
import static java.util.stream.Collectors.toList;
diff --git
a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
index 9702e66..5cb7239 100644
---
a/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
+++
b/dolphinscheduler-server/src/main/java/org/apache/dolphinscheduler/server/worker/task/sql/SqlTask.java
@@ -53,6 +53,7 @@ import java.sql.*;
import java.util.*;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import java.util.stream.Collector;
import java.util.stream.Collectors;
import static org.apache.dolphinscheduler.common.Constants.*;
@@ -123,8 +124,11 @@ public class SqlTask extends AbstractTask {
return;
}
int dataSourceId = sqlParameters.getDatasource();
+ // process instance
+ ProcessInstance processInstance =
processDao.findProcessInstanceByTaskId(taskProps.getTaskInstId());
+ int userId = processInstance.getExecutorId();
// check data source permission
- checkDataSourcePermission(dataSourceId);
+ checkDataSourcePermission(userId,dataSourceId);
dataSource= processDao.findDataSourceById(dataSourceId);
logger.info("datasource name : {} , type : {} , desc : {} , user_id :
{} , parameter : {}",
dataSource.getName(),
@@ -171,8 +175,11 @@ public class SqlTask extends AbstractTask {
idsArray[i]=Integer.parseInt(ids[i]);
}
// check udf permission
- checkUdfPermission(ArrayUtils.toObject(idsArray));
+ checkUdfPermission(userId,ArrayUtils.toObject(idsArray));
List<UdfFunc> udfFuncList =
processDao.queryUdfFunListByids(idsArray);
+ // check whether has permission to download udf file
+ checkUdfFilePermission(userId,udfFuncList);
+
Map<String,UdfFunc> udfFuncMap = new HashMap<String,UdfFunc>();
for(UdfFunc udfFunc : udfFuncList) {
String tenantCode =
processDao.queryTenantCodeByResName(udfFunc.getResourceName(),
ResourceType.UDF);
@@ -466,29 +473,36 @@ public class SqlTask extends AbstractTask {
}
/**
- * check udf function permission
- * @param udfFunIds udf functions
+ * check udf file permission
+ * @param userId login user id
+ * @param udfFuncList udf function list
* @return if has download permission return true else false
*/
- private void checkUdfPermission(Integer[] udfFunIds) throws Exception{
- // process instance
- ProcessInstance processInstance =
processDao.findProcessInstanceByTaskId(taskProps.getTaskInstId());
- int userId = processInstance.getExecutorId();
+ private void checkUdfFilePermission(int userId,List<UdfFunc> udfFuncList)
throws Exception{
+ Integer[] resourceIds = udfFuncList.stream().map(t ->
t.getResourceId()).collect(Collectors.toList()).toArray(new
Integer[udfFuncList.size()]);
+ PermissionCheck<Integer> permissionCheckUdfFile = new
PermissionCheck<Integer>(AuthorizationType.UDF_FILE,processDao,resourceIds,userId,logger);
+ permissionCheckUdfFile.checkPermission();
+ }
+
+ /**
+ * check udf function permission
+ * @param userId login user id
+ * @param udfFunIds udf functions
+ * @return if has permission return true else false
+ */
+ private void checkUdfPermission(int userId,Integer[] udfFunIds) throws
Exception{
PermissionCheck<Integer> permissionCheckUdf = new
PermissionCheck<Integer>(AuthorizationType.UDF,processDao,udfFunIds,userId,logger);
permissionCheckUdf.checkPermission();
}
/**
* check data source permission
+ * @param userId login user id
* @param dataSourceId data source id
* @return if has download permission return true else false
*/
- private void checkDataSourcePermission(int dataSourceId) throws Exception{
- // process instance
- ProcessInstance processInstance =
processDao.findProcessInstanceByTaskId(taskProps.getTaskInstId());
- int userId = processInstance.getExecutorId();
-
+ private void checkDataSourcePermission(int userId,int dataSourceId) throws
Exception{
PermissionCheck<Integer> permissionCheckDataSource = new
PermissionCheck<Integer>(AuthorizationType.DATASOURCE,processDao,new
Integer[]{dataSourceId},userId,logger);
permissionCheckDataSource.checkPermission();
}
