From 7320122c03ae611264a465d62053e79603c11b5d Mon Sep 17 00:00:00 2001 From: "gaoda.xy" Date: Thu, 30 May 2024 17:52:57 +0800 Subject: [PATCH 1/5] fix approval --- .../odc/service/flow/ApprovalPermissionService.java | 7 ++++--- .../iam/auth/OrganizationAuthenticationInterceptor.java | 3 +++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java index debb764c0e..1338cdf325 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.Map; import java.util.Map.Entry; +import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; @@ -180,12 +181,12 @@ private Map> getUsersByFlowInstanceIdsAndStatus(@NonNull C if (approvalUserIds.isEmpty()) { return new HashMap<>(); } - Map userId2User = userRepository.findByUserIds(approvalUserIds).stream() + Map userId2User = userRepository.findByUserIdsAndEnabled(approvalUserIds, true).stream() .collect(Collectors.toMap(UserEntity::getId, userEntity -> userEntity)); Map> instanceId2Candidates = instanceId2UserIds.entrySet().stream().collect( - Collectors.toMap(Entry::getKey, entry -> entry.getValue().stream().map(userId2User::get).collect( - Collectors.toSet()))); + Collectors.toMap(Entry::getKey, entry -> entry.getValue().stream().map(userId2User::get) + .filter(Objects::nonNull).collect(Collectors.toSet()))); // map flow instance ids to users entity return instanceId2UserIds.entrySet().stream().collect( Collectors.toMap(entry -> approvalInstanceId2FlowInstanceId.get(entry.getKey()), diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java index f9847a567c..046fbbf34e 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java @@ -111,6 +111,9 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons "please set currentOrganizationId parameter or header in request"); } long actualOrganizationId = Long.valueOf(actual).longValue(); + if (!authenticationFacade.currentUser().isEnabled()) { + throw new AccessDeniedException(ErrorCodes.AccessDenied, "Current user is disabled"); + } if (authenticationFacade.currentOrganizationId() == actualOrganizationId && authenticationFacade.currentOrganization().getType() == OrganizationType.TEAM) { return true; From 96e717278a68c3184ef07b0caed986955a7f8e74 Mon Sep 17 00:00:00 2001 From: "gaoda.xy" Date: Fri, 31 May 2024 11:21:58 +0800 Subject: [PATCH 2/5] fix disabled user --- .../collaboration/project/ProjectService.java | 1 + .../collaboration/project/model/Project.java | 3 +++ .../odc/service/iam/UserService.java | 25 +++++++++++++++++++ ...OrganizationAuthenticationInterceptor.java | 3 --- 4 files changed, 29 insertions(+), 3 deletions(-) diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/ProjectService.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/ProjectService.java index 635a856f77..01b7aeaa5d 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/ProjectService.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/ProjectService.java @@ -461,6 +461,7 @@ private ProjectMember fromUserResourceRole(UserResourceRole userResourceRole) { member.setId(userResourceRole.getUserId()); member.setName(user.getName()); member.setAccountName(user.getAccountName()); + member.setUserEnabled(user.isEnabled()); return member; } diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/model/Project.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/model/Project.java index 912d9458dd..4551a9fc4c 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/model/Project.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/collaboration/project/model/Project.java @@ -122,6 +122,9 @@ public static class ProjectMember { @JsonProperty(access = Access.READ_ONLY) private String accountName; + @JsonProperty(access = Access.READ_ONLY) + private Boolean userEnabled; + @JsonProperty(access = Access.READ_ONLY) private String name; diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java index 76ae36164a..235cfbeb9d 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java @@ -49,6 +49,8 @@ import org.springframework.data.domain.Sort.Order; import org.springframework.data.jpa.domain.JpaSort; import org.springframework.data.jpa.domain.Specification; +import org.springframework.security.core.session.SessionInformation; +import org.springframework.security.core.session.SessionRegistry; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; @@ -174,6 +176,9 @@ public class UserService { @Autowired private UserOrganizationService userOrganizationService; + @Autowired + private SessionRegistry sessionRegistry; + private final PasswordEncoder passwordEncoder = new BCryptPasswordEncoder(); private final List> postPasswordChangeHooks = new ArrayList<>(); private final List> postUserDeleteHooks = new ArrayList<>(); @@ -676,6 +681,7 @@ private void acquireRolesAndRoleIds(@NotEmpty Collection users) { @PreAuthenticate(actions = "update", resourceType = "ODC_USER", indexOfIdParam = 0) public User update(long id, UpdateUserReq updateUserReq) { UserEntity userEntity = nullSafeGet(id); + boolean before = userEntity.isEnabled(); if (isBuiltin(userEntity)) { throw new UnsupportedException(ErrorCodes.IllegalOperation, new Object[] {"admin account"}, "Operation on admin account is not allowed"); @@ -725,6 +731,9 @@ public User update(long id, UpdateUserReq updateUserReq) { log.debug("User to role relation has been updated: {}", userRoleEntity); } } + if (before && !updateUserReq.isEnabled()) { + expireUserSessionsIfExisted(userEntity.getAccountName()); + } return detail(id); } @@ -734,6 +743,7 @@ public User setEnabled(long id, Boolean enabled) { PreConditions.validArgumentState(id != authenticationFacade.currentUserId(), ErrorCodes.BadRequest, new Object[] {"Can not enable or disable yourself"}, "Can not enable or disable yourself"); UserEntity userEntity = nullSafeGet(id); + boolean before = userEntity.isEnabled(); if (isBuiltin(userEntity)) { throw new UnsupportedException(ErrorCodes.IllegalOperation, new Object[] {"admin account"}, "Operation on admin account is not allowed"); @@ -743,9 +753,24 @@ public User setEnabled(long id, Boolean enabled) { userRepository.update(userEntity); User user = new User(userEntity); publishTriggerEventAfterTx(user, TriggerEvent.USER_UPDATED); + if (before && !enabled) { + expireUserSessionsIfExisted(user.getAccountName()); + } return user; } + private void expireUserSessionsIfExisted(String accountName) { + for (Object principal : sessionRegistry.getAllPrincipals()) { + if (principal instanceof User) { + User user = (User) principal; + if (accountName.equals(user.getAccountName())) { + sessionRegistry.getAllSessions(principal, false).forEach(SessionInformation::expireNow); + break; + } + } + } + } + /** * change password only refer to current user */ diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java index 046fbbf34e..f9847a567c 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/auth/OrganizationAuthenticationInterceptor.java @@ -111,9 +111,6 @@ public boolean preHandle(HttpServletRequest request, HttpServletResponse respons "please set currentOrganizationId parameter or header in request"); } long actualOrganizationId = Long.valueOf(actual).longValue(); - if (!authenticationFacade.currentUser().isEnabled()) { - throw new AccessDeniedException(ErrorCodes.AccessDenied, "Current user is disabled"); - } if (authenticationFacade.currentOrganizationId() == actualOrganizationId && authenticationFacade.currentOrganization().getType() == OrganizationType.TEAM) { return true; From 74e3c4bbd49a830533827574ba34fa9bed60194d Mon Sep 17 00:00:00 2001 From: "gaoda.xy" Date: Fri, 31 May 2024 14:50:33 +0800 Subject: [PATCH 3/5] fix --- .../odc/service/iam/UserService.java | 25 ------------------- 1 file changed, 25 deletions(-) diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java index 235cfbeb9d..76ae36164a 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/iam/UserService.java @@ -49,8 +49,6 @@ import org.springframework.data.domain.Sort.Order; import org.springframework.data.jpa.domain.JpaSort; import org.springframework.data.jpa.domain.Specification; -import org.springframework.security.core.session.SessionInformation; -import org.springframework.security.core.session.SessionRegistry; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.password.PasswordEncoder; import org.springframework.stereotype.Service; @@ -176,9 +174,6 @@ public class UserService { @Autowired private UserOrganizationService userOrganizationService; - @Autowired - private SessionRegistry sessionRegistry; - private final PasswordEncoder passwordEncoder = new BCryptPasswordEncoder(); private final List> postPasswordChangeHooks = new ArrayList<>(); private final List> postUserDeleteHooks = new ArrayList<>(); @@ -681,7 +676,6 @@ private void acquireRolesAndRoleIds(@NotEmpty Collection users) { @PreAuthenticate(actions = "update", resourceType = "ODC_USER", indexOfIdParam = 0) public User update(long id, UpdateUserReq updateUserReq) { UserEntity userEntity = nullSafeGet(id); - boolean before = userEntity.isEnabled(); if (isBuiltin(userEntity)) { throw new UnsupportedException(ErrorCodes.IllegalOperation, new Object[] {"admin account"}, "Operation on admin account is not allowed"); @@ -731,9 +725,6 @@ public User update(long id, UpdateUserReq updateUserReq) { log.debug("User to role relation has been updated: {}", userRoleEntity); } } - if (before && !updateUserReq.isEnabled()) { - expireUserSessionsIfExisted(userEntity.getAccountName()); - } return detail(id); } @@ -743,7 +734,6 @@ public User setEnabled(long id, Boolean enabled) { PreConditions.validArgumentState(id != authenticationFacade.currentUserId(), ErrorCodes.BadRequest, new Object[] {"Can not enable or disable yourself"}, "Can not enable or disable yourself"); UserEntity userEntity = nullSafeGet(id); - boolean before = userEntity.isEnabled(); if (isBuiltin(userEntity)) { throw new UnsupportedException(ErrorCodes.IllegalOperation, new Object[] {"admin account"}, "Operation on admin account is not allowed"); @@ -753,24 +743,9 @@ public User setEnabled(long id, Boolean enabled) { userRepository.update(userEntity); User user = new User(userEntity); publishTriggerEventAfterTx(user, TriggerEvent.USER_UPDATED); - if (before && !enabled) { - expireUserSessionsIfExisted(user.getAccountName()); - } return user; } - private void expireUserSessionsIfExisted(String accountName) { - for (Object principal : sessionRegistry.getAllPrincipals()) { - if (principal instanceof User) { - User user = (User) principal; - if (accountName.equals(user.getAccountName())) { - sessionRegistry.getAllSessions(principal, false).forEach(SessionInformation::expireNow); - break; - } - } - } - } - /** * change password only refer to current user */ From c91fc891858d6ef83a7aa748941495ffa97c4d75 Mon Sep 17 00:00:00 2001 From: "gaoda.xy" Date: Fri, 31 May 2024 15:13:38 +0800 Subject: [PATCH 4/5] disabled user cannot approve --- .../oceanbase/odc/service/flow/ApprovalPermissionService.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java b/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java index 1338cdf325..070c3bb365 100644 --- a/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java +++ b/server/odc-service/src/main/java/com/oceanbase/odc/service/flow/ApprovalPermissionService.java @@ -98,6 +98,9 @@ public boolean isApprovable(@NonNull Long approvalInstanceId) { public List getApprovableApprovalInstances() { long userId = authenticationFacade.currentUserId(); + if (!userService.nullSafeGet(userId).isEnabled()) { + return new ArrayList<>(); + } Set roleIds = userService.getCurrentUserRoleIds(); Set resourceRoleIdentifiers = userService.getCurrentUserResourceRoleIdentifiers(); Set unViewableStatuses = Collections.singleton(FlowNodeStatus.CREATED); From 5ae4cc4bb9ef5258daa1b14e0e23b1f21b47ff2a Mon Sep 17 00:00:00 2001 From: "gaoda.xy" Date: Fri, 31 May 2024 15:52:46 +0800 Subject: [PATCH 5/5] fix ut --- .../oceanbase/odc/service/flow/FlowInstanceServiceTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceServiceTest.java b/server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceServiceTest.java index fb8859a163..8c9468e2a0 100644 --- a/server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceServiceTest.java +++ b/server/integration-test/src/test/java/com/oceanbase/odc/service/flow/FlowInstanceServiceTest.java @@ -15,6 +15,7 @@ */ package com.oceanbase.odc.service.flow; +import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.when; @@ -70,6 +71,7 @@ import com.oceanbase.odc.metadb.flow.ServiceTaskInstanceRepository; import com.oceanbase.odc.metadb.flow.UserTaskInstanceCandidateRepository; import com.oceanbase.odc.metadb.flow.UserTaskInstanceRepository; +import com.oceanbase.odc.metadb.iam.UserEntity; import com.oceanbase.odc.metadb.task.TaskEntity; import com.oceanbase.odc.metadb.task.TaskRepository; import com.oceanbase.odc.plugin.task.api.datatransfer.model.DataTransferConfig; @@ -201,6 +203,9 @@ public void setUp() { when(riskLevelService.findDefaultRiskLevel()).thenReturn(getRiskLevel()); when(riskLevelService.list()).thenReturn(Arrays.asList(getRiskLevel(), getRiskLevel())); doNothing().when(databasePermissionHelper).checkPermissions(Mockito.anyCollection(), Mockito.anyCollection()); + UserEntity user = new UserEntity(); + user.setEnabled(true); + when(userService.nullSafeGet(anyLong())).thenReturn(user); } @Test