[Fix-11031] AccessToken can only be used by the creator. (#11032)

* AccessToken can only be used by the creator.

* supplement ut

* checkstyle fix

dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AccessTokenServiceImpl.java

@@ -27,6 +27,7 @@ import org.apache.dolphinscheduler.api.utils.PageInfo;
 import org.apache.dolphinscheduler.api.utils.Result;
 import org.apache.dolphinscheduler.common.Constants;
 import org.apache.dolphinscheduler.common.enums.AuthorizationType;
+import org.apache.dolphinscheduler.common.enums.UserType;
 import org.apache.dolphinscheduler.common.utils.DateUtils;
 import org.apache.dolphinscheduler.common.utils.EncryptionUtils;
 import org.apache.dolphinscheduler.dao.entity.AccessToken;
@@ -35,13 +36,10 @@ import org.apache.dolphinscheduler.dao.mapper.AccessTokenMapper;
 
 import org.apache.commons.lang3.StringUtils;
 
-import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -75,13 +73,14 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
     public Result queryAccessTokenList(User loginUser, String searchVal, Integer pageNo, Integer pageSize) {
         Result result = new Result();
         PageInfo<AccessToken> pageInfo = new PageInfo<>(pageNo, pageSize);
-        Set<Integer> ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ACCESS_TOKEN, loginUser.getId(), logger);
+        Page<AccessToken> page = new Page<>(pageNo, pageSize);
-        if (!ids.isEmpty()) {
+        int userId = loginUser.getId();
-            Page<AccessToken> page = new Page<>(pageNo, pageSize);
+        if (loginUser.getUserType() == UserType.ADMIN_USER) {
-            IPage<AccessToken> accessTokenList = accessTokenMapper.selectAccessTokenPage(page, new ArrayList<>(ids), searchVal);
+            userId = 0;
-            pageInfo.setTotal((int) accessTokenList.getTotal());
-            pageInfo.setTotalList(accessTokenList.getRecords());
         }
+        IPage<AccessToken> accessTokenList = accessTokenMapper.selectAccessTokenPage(page, searchVal, userId);
+        pageInfo.setTotal((int) accessTokenList.getTotal());
+        pageInfo.setTotalList(accessTokenList.getRecords());
         result.setData(pageInfo);
         putMsg(result, Status.SUCCESS);
         return result;
@@ -98,11 +97,14 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
     public Map<String, Object> queryAccessTokenByUser(User loginUser, Integer userId) {
         Map<String, Object> result = new HashMap<>();
         result.put(Constants.STATUS, false);
-        List<AccessToken> accessTokenList = Collections.EMPTY_LIST;
+        // no permission
-        Set<Integer> ids = resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ACCESS_TOKEN, loginUser.getId(), logger);
+        if (loginUser.getUserType().equals(UserType.GENERAL_USER) && loginUser.getId() != userId) {
-        if (!ids.isEmpty()) {
+            putMsg(result, Status.USER_NO_OPERATION_PERM);
-            accessTokenList = this.accessTokenMapper.selectBatchIds(ids);
+            return result;
         }
+        userId = loginUser.getUserType().equals(UserType.ADMIN_USER) ? 0 : userId;
+        // query access token for specified user
+        List<AccessToken> accessTokenList = this.accessTokenMapper.queryAccessTokenByUser(userId);
         result.put(Constants.DATA_LIST, accessTokenList);
         this.putMsg(result, Status.SUCCESS);
         return result;
@@ -111,7 +113,7 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
     /**
      * create token
      *
-     * @param loginUser
+     * @param loginUser loginUser
      * @param userId token for user
      * @param expireTime token expire time
      * @param token token string (if it is absent, it will be automatically generated)
@@ -154,7 +156,6 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
         if (insert > 0) {
             result.setData(accessToken);
             putMsg(result, Status.SUCCESS);
-            resourcePermissionCheckService.postHandle(AuthorizationType.ACCESS_TOKEN, loginUser.getId(), new ArrayList<>(accessToken.getId()), logger);
         } else {
             putMsg(result, Status.CREATE_ACCESS_TOKEN_ERROR);
         }
@@ -189,19 +190,23 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
     @Override
     public Map<String, Object> delAccessTokenById(User loginUser, int id) {
         Map<String, Object> result = new HashMap<>();
-        if (!canOperatorPermissions(loginUser,new Object[]{id},AuthorizationType.ACCESS_TOKEN,ACCESS_TOKEN_DELETE)) {
+        if (!canOperatorPermissions(loginUser, null, AuthorizationType.ACCESS_TOKEN,ACCESS_TOKEN_DELETE)) {
             putMsg(result, Status.USER_NO_OPERATION_PERM);
             return result;
         }
 
         AccessToken accessToken = accessTokenMapper.selectById(id);
-
         if (accessToken == null) {
             logger.error("access token not exist,  access token id {}", id);
             putMsg(result, Status.ACCESS_TOKEN_NOT_EXIST);
             return result;
         }
 
+        // admin can operate all, non-admin can operate their own
+        if (accessToken.getUserId() != loginUser.getId() && !loginUser.getUserType().equals(UserType.ADMIN_USER)) {
+            putMsg(result, Status.USER_NO_OPERATION_PERM);
+            return result;
+        }
         accessTokenMapper.deleteById(id);
         putMsg(result, Status.SUCCESS);
         return result;
@@ -221,7 +226,7 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
         Map<String, Object> result = new HashMap<>();
 
         // 1. check permission
-        if (!canOperatorPermissions(loginUser,new Object[]{id},AuthorizationType.ACCESS_TOKEN,ACCESS_TOKEN_UPDATE)) {
+        if (!canOperatorPermissions(loginUser, null,AuthorizationType.ACCESS_TOKEN,ACCESS_TOKEN_UPDATE)) {
             putMsg(result, Status.USER_NO_OPERATION_PERM);
             return result;
         }
@@ -233,6 +238,11 @@ public class AccessTokenServiceImpl extends BaseServiceImpl implements AccessTok
             putMsg(result, Status.ACCESS_TOKEN_NOT_EXIST);
             return result;
         }
+        // admin can operate all, non-admin can operate their own
+        if (accessToken.getUserId() != loginUser.getId() && !loginUser.getUserType().equals(UserType.ADMIN_USER)) {
+            putMsg(result, Status.USER_NO_OPERATION_PERM);
+            return result;
+        }
 
         // 3. generate access token if absent
         if (StringUtils.isBlank(token)) {

dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AccessTokenServiceTest.java

@@ -22,6 +22,7 @@ import static org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationCon
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.when;
 
 import org.apache.dolphinscheduler.api.enums.Status;
@@ -41,10 +42,8 @@ import org.apache.dolphinscheduler.dao.mapper.AccessTokenMapper;
 import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Date;
-import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.assertj.core.util.Lists;
 import org.junit.Assert;
@@ -54,7 +53,6 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.Mockito;
 import org.mockito.junit.MockitoJUnitRunner;
-import org.powermock.api.mockito.PowerMockito;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -68,7 +66,6 @@ import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
 public class AccessTokenServiceTest {
     private static final Logger baseServiceLogger = LoggerFactory.getLogger(BaseServiceImpl.class);
     private static final Logger logger = LoggerFactory.getLogger(AccessTokenServiceTest.class);
-    private static final Logger serviceLogger = LoggerFactory.getLogger(AccessTokenServiceImpl.class);
 
     @InjectMocks
     private AccessTokenServiceImpl accessTokenService;
@@ -84,20 +81,16 @@ public class AccessTokenServiceTest {
     public void testQueryAccessTokenList() {
         IPage<AccessToken> tokenPage = new Page<>();
         tokenPage.setRecords(getList());
-        tokenPage.setTotal(1L);
         User user = new User();
         user.setId(1);
         user.setUserType(UserType.ADMIN_USER);
-        Set<Integer> tokenIds = new HashSet<>();
+        when(accessTokenMapper.selectAccessTokenPage(any(Page.class), eq("zhangsan"), eq(0))).thenReturn(tokenPage);
-        tokenIds.add(1);
-        when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ACCESS_TOKEN, user.getId(), serviceLogger)).thenReturn(new HashSet());
         Result result = accessTokenService.queryAccessTokenList(user, "zhangsan", 1, 10);
         PageInfo<AccessToken> pageInfo = (PageInfo<AccessToken>) result.getData();
         assertEquals(0, (int) pageInfo.getTotal());
 
-        PowerMockito.when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ACCESS_TOKEN, user.getId(), serviceLogger)).thenReturn(tokenIds);
+        tokenPage.setTotal(1L);
-        Mockito.when(accessTokenMapper.selectAccessTokenPage(Mockito.any(Page.class), Mockito.anyList(),Mockito.eq("zhangsan"))).thenReturn(tokenPage);
+        when(accessTokenMapper.selectAccessTokenPage(any(Page.class), eq("zhangsan"), eq(0))).thenReturn(tokenPage);
-
         result = accessTokenService.queryAccessTokenList(user, "zhangsan", 1, 10);
         pageInfo = (PageInfo<AccessToken>) result.getData();
         logger.info(result.toString());
@@ -109,10 +102,7 @@ public class AccessTokenServiceTest {
         User user = this.getLoginUser();
         user.setUserType(UserType.ADMIN_USER);
         List<AccessToken> accessTokenList = Lists.newArrayList(this.getEntity());
-
+        Mockito.when(this.accessTokenMapper.queryAccessTokenByUser(Mockito.anyInt())).thenReturn(accessTokenList);
-        Set<Integer> tokenIds = new HashSet<>();
-        tokenIds.add(1);
-        Mockito.when(resourcePermissionCheckService.userOwnedResourceIdsAcquisition(AuthorizationType.ACCESS_TOKEN, user.getId(), serviceLogger)).thenReturn(tokenIds);
         Map<String, Object> result = this.accessTokenService.queryAccessTokenByUser(user, 1);
         logger.info(result.toString());
         assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
@@ -152,19 +142,20 @@ public class AccessTokenServiceTest {
         userLogin.setId(1);
         userLogin.setUserType(UserType.ADMIN_USER);
         Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ACCESS_TOKEN, 1, ACCESS_TOKEN_DELETE, baseServiceLogger)).thenReturn(true);
-        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, new Object[]{0}, 0, baseServiceLogger)).thenReturn(true);
+        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, null, 0, baseServiceLogger)).thenReturn(true);
         // not exist
         Map<String, Object> result = accessTokenService.delAccessTokenById(userLogin, 0);
         logger.info(result.toString());
         assertEquals(Status.ACCESS_TOKEN_NOT_EXIST, result.get(Constants.STATUS));
         // no operate
+        userLogin.setId(2);
         result = accessTokenService.delAccessTokenById(userLogin, 1);
         logger.info(result.toString());
         assertEquals(Status.USER_NO_OPERATION_PERM, result.get(Constants.STATUS));
         //success
         userLogin.setId(1);
         userLogin.setUserType(UserType.ADMIN_USER);
-        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, new Object[]{1}, 0, baseServiceLogger)).thenReturn(true);
+        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, null, 0, baseServiceLogger)).thenReturn(true);
         result = accessTokenService.delAccessTokenById(userLogin, 1);
         logger.info(result.toString());
         assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
@@ -176,7 +167,7 @@ public class AccessTokenServiceTest {
         user.setId(1);
         user.setUserType(UserType.ADMIN_USER);
         Mockito.when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ACCESS_TOKEN, 1, ACCESS_TOKEN_UPDATE, baseServiceLogger)).thenReturn(true);
-        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, new Object[]{1}, 0, baseServiceLogger)).thenReturn(true);
+        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, null, 0, baseServiceLogger)).thenReturn(true);
         // Given Token
         when(accessTokenMapper.selectById(1)).thenReturn(getEntity());
         Map<String, Object> result = accessTokenService.updateToken(getLoginUser(), 1,Integer.MAX_VALUE,getDate(),"token");
@@ -191,7 +182,7 @@ public class AccessTokenServiceTest {
         Assert.assertNotNull(result.get(Constants.DATA_LIST));
 
         // ACCESS_TOKEN_NOT_EXIST
-        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, new Object[]{2}, 0, baseServiceLogger)).thenReturn(true);
+        Mockito.when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN, null, 0, baseServiceLogger)).thenReturn(true);
         result = accessTokenService.updateToken(getLoginUser(), 2,Integer.MAX_VALUE,getDate(),"token");
         logger.info(result.toString());
         assertEquals(Status.ACCESS_TOKEN_NOT_EXIST, result.get(Constants.STATUS));

dolphinscheduler-dao/src/main/java/org/apache/dolphinscheduler/dao/mapper/AccessTokenMapper.java

@@ -37,13 +37,13 @@ public interface AccessTokenMapper extends BaseMapper<AccessToken> {
      * access token page
      *
      * @param page page
-     * @param tokenIds tokenIds
+     * @param userId userId
      * @param userName userName
      * @return access token Ipage
      */
     IPage<AccessToken> selectAccessTokenPage(Page page,
-                                             @Param("ids") List<Integer> tokenIds,
+                                             @Param("userName") String userName,
-                                             @Param("userName") String userName
+                                             @Param("userId") int userId
     );
 
     /**

dolphinscheduler-dao/src/main/resources/org/apache/dolphinscheduler/dao/mapper/AccessTokenMapper.xml

@@ -26,11 +26,8 @@
         <if test="userName != null and userName != ''">
             and u.user_name like concat ('%', #{userName}, '%')
         </if>
-        <if test="ids != null and ids.size() > 0">
+        <if test="userId != 0">
-            and t.id  in
+            and t.user_id = #{userId}
-            <foreach item="id" index="index" collection="ids" open="(" separator="," close=")">
-                #{id}
-            </foreach>
         </if>
         order by t.update_time desc
     </select>

dolphinscheduler-dao/src/test/java/org/apache/dolphinscheduler/dao/mapper/AccessTokenMapperTest.java

@@ -30,9 +30,10 @@ import org.apache.dolphinscheduler.dao.BaseDaoTest;
 import org.apache.dolphinscheduler.dao.entity.AccessToken;
 import org.apache.dolphinscheduler.dao.entity.User;
 
+import java.util.ArrayList;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.concurrent.ThreadLocalRandom;
 import java.util.stream.Collectors;
 
@@ -86,7 +87,6 @@ public class AccessTokenMapperTest extends BaseDaoTest {
         Assert.assertEquals(insertCount, deleteCount);
     }
 
-
     /**
      * test select by id
      *
@@ -139,17 +139,26 @@ public class AccessTokenMapperTest extends BaseDaoTest {
         Integer size = 2;
 
         Map<Integer, AccessToken> accessTokenMap = createAccessTokens(count, userName);
+        Set<Integer> userIds = accessTokenMap.values().stream().map(AccessToken::getUserId).collect(Collectors.toSet());
+        Integer createTokenUserId = new ArrayList<>(userIds).get(0);
 
+        // general user and create token user
         Page page = new Page(offset, size);
-        List<Integer> tokenIds = accessTokenMap.values().stream().map(AccessToken::getId).collect(Collectors.toList());
+        IPage<AccessToken> accessTokenPage = accessTokenMapper.selectAccessTokenPage(page, userName, createTokenUserId);
-        IPage<AccessToken> accessTokenPage = accessTokenMapper.selectAccessTokenPage(page, tokenIds, userName);
-
         assertEquals(Integer.valueOf(accessTokenPage.getRecords().size()), size);
 
-        for (AccessToken accessToken : accessTokenPage.getRecords()) {
+        // admin user
+        IPage<AccessToken> adminAccessTokenPage = accessTokenMapper.selectAccessTokenPage(page, userName, 0);
+        assertEquals(Integer.valueOf(adminAccessTokenPage.getRecords().size()), size);
+        for (AccessToken accessToken : adminAccessTokenPage.getRecords()) {
             AccessToken resultAccessToken = accessTokenMap.get(accessToken.getId());
             assertEquals(accessToken, resultAccessToken);
         }
+
+        // general user
+        Integer emptySize = 0;
+        IPage<AccessToken> generalAccessTokenPage = accessTokenMapper.selectAccessTokenPage(page, userName, 1);
+        assertEquals(Integer.valueOf(generalAccessTokenPage.getRecords().size()), emptySize);
     }