[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

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

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

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

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());
 
     }
 

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;

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

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

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);
+
 
 }

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>

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

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;
 

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();
     }