Browse Source

[Fix-5701]When deleting a user, the accessToken associated with the user should also be deleted (#5697)

* update

* fix the codestyle error

* fix the compile error

* support rollback
kyoty 3 years ago
parent
commit
8d7d3a8166

+ 7 - 0
dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/UsersServiceImpl.java

@@ -43,6 +43,7 @@ import org.apache.dolphinscheduler.dao.entity.ResourcesUser;
 import org.apache.dolphinscheduler.dao.entity.Tenant;
 import org.apache.dolphinscheduler.dao.entity.UDFUser;
 import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.AccessTokenMapper;
 import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
 import org.apache.dolphinscheduler.dao.mapper.DataSourceUserMapper;
 import org.apache.dolphinscheduler.dao.mapper.ProcessDefinitionMapper;
@@ -83,6 +84,9 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService {
 
     private static final Logger logger = LoggerFactory.getLogger(UsersServiceImpl.class);
 
+    @Autowired
+    private AccessTokenMapper accessTokenMapper;
+
     @Autowired
     private UserMapper userMapper;
 
@@ -482,6 +486,7 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService {
      * @throws Exception exception when operate hdfs
      */
     @Override
+    @Transactional(rollbackFor = RuntimeException.class)
     public Map<String, Object> deleteUserById(User loginUser, int id) throws IOException {
         Map<String, Object> result = new HashMap<>();
         //only admin can operate
@@ -514,6 +519,8 @@ public class UsersServiceImpl extends BaseServiceImpl implements UsersService {
             }
         }
 
+        accessTokenMapper.deleteAccessTokenByUserId(id);
+        
         userMapper.deleteById(id);
         putMsg(result, Status.SUCCESS);
 

+ 8 - 10
dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/UsersServiceTest.java

@@ -35,6 +35,7 @@ import org.apache.dolphinscheduler.dao.entity.Project;
 import org.apache.dolphinscheduler.dao.entity.Resource;
 import org.apache.dolphinscheduler.dao.entity.Tenant;
 import org.apache.dolphinscheduler.dao.entity.User;
+import org.apache.dolphinscheduler.dao.mapper.AccessTokenMapper;
 import org.apache.dolphinscheduler.dao.mapper.AlertGroupMapper;
 import org.apache.dolphinscheduler.dao.mapper.DataSourceUserMapper;
 import org.apache.dolphinscheduler.dao.mapper.ProjectMapper;
@@ -79,6 +80,9 @@ public class UsersServiceTest {
     @Mock
     private UserMapper userMapper;
 
+    @Mock
+    private AccessTokenMapper accessTokenMapper;
+
     @Mock
     private TenantMapper tenantMapper;
 
@@ -221,7 +225,6 @@ public class UsersServiceTest {
         Assert.assertEquals(user.getId(), userExistId);
     }
 
-
     @Test
     public void testQueryUserList() {
         User user = new User();
@@ -265,13 +268,13 @@ public class UsersServiceTest {
         String userPassword = "userTest0001";
         try {
             //user not exist
-            Map<String, Object> result = usersService.updateUser(getLoginUser(), 0,userName,userPassword,"3443@qq.com",1,"13457864543","queue", 1);
+            Map<String, Object> result = usersService.updateUser(getLoginUser(), 0, userName, userPassword, "3443@qq.com", 1, "13457864543", "queue", 1);
             Assert.assertEquals(Status.USER_NOT_EXIST, result.get(Constants.STATUS));
             logger.info(result.toString());
 
             //success
             when(userMapper.selectById(1)).thenReturn(getUser());
-            result = usersService.updateUser(getLoginUser(), 1,userName,userPassword,"32222s@qq.com",1,"13457864543","queue", 1);
+            result = usersService.updateUser(getLoginUser(), 1, userName, userPassword, "32222s@qq.com", 1, "13457864543", "queue", 1);
             logger.info(result.toString());
             Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
         } catch (Exception e) {
@@ -286,7 +289,7 @@ public class UsersServiceTest {
         try {
             when(userMapper.queryTenantCodeByUserId(1)).thenReturn(getUser());
             when(userMapper.selectById(1)).thenReturn(getUser());
-
+            when(accessTokenMapper.deleteAccessTokenByUserId(1)).thenReturn(0);
             //no operate
             Map<String, Object> result = usersService.deleteUserById(loginUser, 3);
             logger.info(result.toString());
@@ -356,7 +359,6 @@ public class UsersServiceTest {
 
     }
 
-
     @Test
     public void testGrantUDFFunction() {
         String udfIds = "100000,120000";
@@ -398,7 +400,7 @@ public class UsersServiceTest {
 
     }
 
-    private User getLoginUser(){
+    private User getLoginUser() {
         User loginUser = new User();
         loginUser.setId(1);
         loginUser.setUserType(UserType.ADMIN_USER);
@@ -431,7 +433,6 @@ public class UsersServiceTest {
         Assert.assertEquals("userTest0001", tempUser.getUserName());
     }
 
-
     @Test
     public void testQueryAllGeneralUsers() {
         User loginUser = new User();
@@ -478,7 +479,6 @@ public class UsersServiceTest {
         Assert.assertEquals(Status.SUCCESS, result.get(Constants.STATUS));
     }
 
-
     @Test
     public void testAuthorizedUser() {
         User loginUser = new User();
@@ -535,7 +535,6 @@ public class UsersServiceTest {
         }
     }
 
-
     @Test
     public void testActivateUser() {
         User user = new User();
@@ -618,7 +617,6 @@ public class UsersServiceTest {
         return user;
     }
 
-
     /**
      * get user
      */

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

@@ -14,13 +14,16 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.apache.dolphinscheduler.dao.mapper;
 
 import org.apache.dolphinscheduler.dao.entity.AccessToken;
+
+import org.apache.ibatis.annotations.Param;
+
 import com.baomidou.mybatisplus.core.mapper.BaseMapper;
 import com.baomidou.mybatisplus.core.metadata.IPage;
 import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
-import org.apache.ibatis.annotations.Param;
 
 /**
  * accesstoken mapper interface
@@ -30,6 +33,7 @@ public interface AccessTokenMapper extends BaseMapper<AccessToken> {
 
     /**
      * access token page
+     *
      * @param page page
      * @param userName userName
      * @param userId userId
@@ -39,4 +43,12 @@ public interface AccessTokenMapper extends BaseMapper<AccessToken> {
                                              @Param("userName") String userName,
                                              @Param("userId") int userId
     );
+
+    /**
+     * delete by userId
+     *
+     * @param userId userId
+     * @return delete result
+     */
+    int deleteAccessTokenByUserId(@Param("userId") int userId);
 }

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

@@ -31,4 +31,8 @@
         </if>
         order by t.update_time desc
     </select>
+    <delete id="deleteAccessTokenByUserId">
+        delete from t_ds_access_token
+        where user_id = #{userId}
+    </delete>
 </mapper>

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

@@ -16,12 +16,22 @@
  */
 package org.apache.dolphinscheduler.dao.mapper;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.greaterThan;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+import static org.junit.Assert.assertTrue;
+
 import org.apache.dolphinscheduler.common.enums.UserType;
 import org.apache.dolphinscheduler.common.utils.DateUtils;
 import org.apache.dolphinscheduler.dao.entity.AccessToken;
-import com.baomidou.mybatisplus.core.metadata.IPage;
-import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
 import org.apache.dolphinscheduler.dao.entity.User;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.ThreadLocalRandom;
+
 import org.junit.Assert;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -31,14 +41,8 @@ import org.springframework.test.annotation.Rollback;
 import org.springframework.test.context.junit4.SpringRunner;
 import org.springframework.transaction.annotation.Transactional;
 
-import javax.annotation.Resource;
-import java.text.SimpleDateFormat;
-import java.util.*;
-import java.util.concurrent.ThreadLocalRandom;
-
-import static org.hamcrest.MatcherAssert.assertThat;
-import static org.hamcrest.Matchers.greaterThan;
-import static org.junit.Assert.*;
+import com.baomidou.mybatisplus.core.metadata.IPage;
+import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
 
 /**
  * AccessToken mapper test
@@ -57,8 +61,6 @@ public class AccessTokenMapperTest {
 
     /**
      * test insert
-     *
-     * @throws Exception
      */
     @Test
     public void testInsert() throws Exception {
@@ -68,6 +70,27 @@ public class AccessTokenMapperTest {
         assertThat(accessToken.getId(), greaterThan(0));
     }
 
+    /**
+     * test delete AccessToken By UserId
+     */
+    @Test
+    public void testDeleteAccessTokenByUserId() throws Exception {
+        Integer userId = 1;
+        int insertCount = 0;
+
+        for (int i = 0; i < 10; i++) {
+            try {
+                createAccessToken(userId);
+                insertCount++;
+            } catch (Exception e) {
+                e.printStackTrace();
+            }
+        }
+
+        int deleteCount = accessTokenMapper.deleteAccessTokenByUserId(userId);
+        Assert.assertEquals(insertCount, deleteCount);
+    }
+
 
     /**
      * test select by id