This is an automated email from the ASF dual-hosted git repository.
ruanwenjun pushed a commit to branch dev
in repository https://gitbox.apache.org/repos/asf/dolphinscheduler.git
The following commit(s) were added to refs/heads/dev by this push:
new 25c6a87999 [Fix-18299][API] Enforce access token owner checks (#18300)
25c6a87999 is described below
commit 25c6a879991a69a29dc2148f276fa0a902252e57
Author: Wenjun Ruan <[email protected]>
AuthorDate: Thu May 28 14:32:57 2026 +0800
[Fix-18299][API] Enforce access token owner checks (#18300)
---
.../api/service/impl/AccessTokenServiceImpl.java | 28 +++++-----
.../api/service/AccessTokenServiceTest.java | 64 ++++++++++++----------
2 files changed, 50 insertions(+), 42 deletions(-)
diff --git
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AccessTokenServiceImpl.java
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AccessTokenServiceImpl.java
index e9bba19af7..572f1cd148 100644
---
a/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AccessTokenServiceImpl.java
+++
b/dolphinscheduler-api/src/main/java/org/apache/dolphinscheduler/api/service/impl/AccessTokenServiceImpl.java
@@ -17,9 +17,7 @@
package org.apache.dolphinscheduler.api.service.impl;
-import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ACCESS_TOKEN_CREATE;
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ACCESS_TOKEN_DELETE;
-import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ACCESS_TOKEN_UPDATE;
import org.apache.dolphinscheduler.api.enums.Status;
import org.apache.dolphinscheduler.api.exceptions.ServiceException;
@@ -110,10 +108,7 @@ public class AccessTokenServiceImpl extends
BaseServiceImpl implements AccessTok
public AccessToken createToken(User loginUser, int userId, String
expireTime, String token) {
// 1. check permission
- if (!(canOperatorPermissions(loginUser, null,
AuthorizationType.ACCESS_TOKEN, ACCESS_TOKEN_CREATE)
- || loginUser.getId() == userId)) {
- throw new ServiceException(Status.USER_NO_OPERATION_PERM);
- }
+ checkAccessTokenTargetUserIsLoginUserOrAdmin(loginUser, userId);
// 2. check if user is existed
if (userId <= 0) {
@@ -193,9 +188,7 @@ public class AccessTokenServiceImpl extends BaseServiceImpl
implements AccessTok
public AccessToken updateToken(User loginUser, int id, int userId, String
expireTime, String token) {
// 1. check permission
- if (!canOperatorPermissions(loginUser, null,
AuthorizationType.ACCESS_TOKEN, ACCESS_TOKEN_UPDATE)) {
- throw new ServiceException(Status.USER_NO_OPERATION_PERM);
- }
+ checkAccessTokenTargetUserIsLoginUserOrAdmin(loginUser, userId);
// 2. check if token is existed
AccessToken accessToken = accessTokenDao.queryById(id);
@@ -203,10 +196,7 @@ public class AccessTokenServiceImpl extends
BaseServiceImpl implements AccessTok
log.error("Access token does not exist, accessTokenId:{}.", id);
throw new ServiceException(Status.ACCESS_TOKEN_NOT_EXIST, id);
}
- // admin can operate all, non-admin can operate their own
- if (accessToken.getUserId() != loginUser.getId() &&
!loginUser.getUserType().equals(UserType.ADMIN_USER)) {
- throw new ServiceException(Status.USER_NO_OPERATION_PERM);
- }
+ checkAccessTokenOwnerIsLoginUserOrAdmin(loginUser, accessToken);
// 3. generate access token if absent
if (StringUtils.isBlank(token)) {
@@ -224,4 +214,16 @@ public class AccessTokenServiceImpl extends
BaseServiceImpl implements AccessTok
}
return accessToken;
}
+
+ private void checkAccessTokenTargetUserIsLoginUserOrAdmin(User loginUser,
int userId) {
+ if (!isAdmin(loginUser) && loginUser.getId() != userId) {
+ throw new ServiceException(Status.USER_NO_OPERATION_PERM);
+ }
+ }
+
+ private void checkAccessTokenOwnerIsLoginUserOrAdmin(User loginUser,
AccessToken accessToken) {
+ if (!isAdmin(loginUser) && accessToken.getUserId() !=
loginUser.getId()) {
+ throw new ServiceException(Status.USER_NO_OPERATION_PERM);
+ }
+ }
}
diff --git
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AccessTokenServiceTest.java
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AccessTokenServiceTest.java
index 9320c865ba..ba196f9643 100644
---
a/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AccessTokenServiceTest.java
+++
b/dolphinscheduler-api/src/test/java/org/apache/dolphinscheduler/api/service/AccessTokenServiceTest.java
@@ -20,7 +20,6 @@ package org.apache.dolphinscheduler.api.service;
import static
org.apache.dolphinscheduler.api.AssertionsHelper.assertDoesNotThrow;
import static
org.apache.dolphinscheduler.api.AssertionsHelper.assertThrowsServiceException;
import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ACCESS_TOKEN_DELETE;
-import static
org.apache.dolphinscheduler.api.constants.ApiFuncIdentificationConstant.ACCESS_TOKEN_UPDATE;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
@@ -40,7 +39,6 @@ import org.apache.dolphinscheduler.dao.entity.AccessToken;
import org.apache.dolphinscheduler.dao.entity.User;
import org.apache.dolphinscheduler.dao.repository.AccessTokenDao;
-import java.util.ArrayList;
import java.util.Calendar;
import java.util.Date;
import java.util.List;
@@ -78,7 +76,7 @@ public class AccessTokenServiceTest {
@SuppressWarnings("unchecked")
public void testQueryAccessTokenList() {
IPage<AccessToken> tokenPage = new Page<>();
- tokenPage.setRecords(getList());
+ tokenPage.setRecords(Lists.newArrayList(getEntity()));
User user = new User();
user.setId(1);
user.setUserType(UserType.ADMIN_USER);
@@ -109,10 +107,14 @@ public class AccessTokenServiceTest {
public void testCreateToken() {
User user = getLoginUser();
- // Throw ServiceException when user has no permission
+ user.setUserType(UserType.GENERAL_USER);
+
+ // Throw ServiceException when general user creates token for another
user
assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
() -> accessTokenService.createToken(user, 2, getDate(),
"AccessTokenServiceTest"));
+ Mockito.verify(accessTokenDao,
Mockito.never()).insert(any(AccessToken.class));
+ user.setUserType(UserType.ADMIN_USER);
user.setId(0);
// Throw ServiceException when user is invalid
@@ -137,6 +139,16 @@ public class AccessTokenServiceTest {
() -> accessTokenService.createToken(user, 1, getDate(),
"AccessTokenServiceTest"));
}
+ @Test
+ public void testCreateTokenForOtherUserDeniedForGeneralUser() {
+ User user = getGeneralUser();
+ when(accessTokenDao.insert(any(AccessToken.class))).thenReturn(1);
+
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () -> accessTokenService.createToken(user, 2, getDate(),
"AccessTokenServiceTest"));
+ Mockito.verify(accessTokenDao,
Mockito.never()).insert(any(AccessToken.class));
+ }
+
@Test
public void testGenerateToken() {
User user = new User();
@@ -184,16 +196,6 @@ public class AccessTokenServiceTest {
@Test
public void testUpdateToken() {
- // operation perm check
-
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ACCESS_TOKEN,
1,
- ACCESS_TOKEN_UPDATE, baseServiceLogger)).thenReturn(false);
- assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
- () -> accessTokenService.updateToken(getLoginUser(), 1, 1,
getDate(), "token"));
-
-
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ACCESS_TOKEN,
1,
- ACCESS_TOKEN_UPDATE, baseServiceLogger)).thenReturn(true);
-
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN,
null, 0,
- baseServiceLogger)).thenReturn(true);
// Given Token
when(accessTokenDao.queryById(1)).thenReturn(getEntity());
when(accessTokenDao.updateById(any())).thenReturn(true);
@@ -211,14 +213,10 @@ public class AccessTokenServiceTest {
assertThrowsServiceException(Status.ACCESS_TOKEN_NOT_EXIST,
() -> accessTokenService.updateToken(getLoginUser(), 2,
Integer.MAX_VALUE, getDate(), "token"));
- // resource perm check
+ // old token owner check
User user = getLoginUser();
user.setUserType(UserType.GENERAL_USER);
user.setId(2);
-
when(resourcePermissionCheckService.operationPermissionCheck(AuthorizationType.ACCESS_TOKEN,
2,
- ACCESS_TOKEN_UPDATE, baseServiceLogger)).thenReturn(true);
-
when(resourcePermissionCheckService.resourcePermissionCheck(AuthorizationType.ACCESS_TOKEN,
null, 2,
- baseServiceLogger)).thenReturn(true);
assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
() -> accessTokenService.updateToken(user, 1,
Integer.MAX_VALUE, getDate(), "token"));
@@ -229,6 +227,17 @@ public class AccessTokenServiceTest {
() -> accessTokenService.updateToken(getLoginUser(), 1,
Integer.MAX_VALUE, getDate(), "token"));
}
+ @Test
+ public void testUpdateTokenToOtherUserDeniedForGeneralUser() {
+ User user = getGeneralUser();
+ when(accessTokenDao.queryById(1)).thenReturn(getEntity());
+
when(accessTokenDao.updateById(any(AccessToken.class))).thenReturn(true);
+
+ assertThrowsServiceException(Status.USER_NO_OPERATION_PERM,
+ () -> accessTokenService.updateToken(user, 1, 2, getDate(),
"token"));
+ Mockito.verify(accessTokenDao,
Mockito.never()).updateById(any(AccessToken.class));
+ }
+
private User getLoginUser() {
User loginUser = new User();
loginUser.setId(1);
@@ -236,6 +245,13 @@ public class AccessTokenServiceTest {
return loginUser;
}
+ private User getGeneralUser() {
+ User loginUser = new User();
+ loginUser.setId(1);
+ loginUser.setUserType(UserType.GENERAL_USER);
+ return loginUser;
+ }
+
/**
* create entity
*/
@@ -249,16 +265,6 @@ public class AccessTokenServiceTest {
return accessToken;
}
- /**
- * entity list
- */
- private List<AccessToken> getList() {
-
- List<AccessToken> list = new ArrayList<>();
- list.add(getEntity());
- return list;
- }
-
/**
* get dateStr
*/
