You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by xx...@apache.org on 2020/10/10 03:32:47 UTC
[kylin] branch master updated: User info update logic is not correct
This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kylin.git
The following commit(s) were added to refs/heads/master by this push:
new 443c252 User info update logic is not correct
443c252 is described below
commit 443c2523e27e86ed397c526f741db62a805b95c4
Author: yangjiang <ya...@ebay.com>
AuthorDate: Wed Sep 30 11:20:04 2020 +0800
User info update logic is not correct
---
.../kylin/rest/controller/UserController.java | 1 +
.../rest/security/KylinAuthenticationProvider.java | 26 +++++++++++++++++-----
.../kylin/rest/security/KylinUserManager.java | 4 ++++
.../kylin/rest/service/KylinUserGroupService.java | 3 +++
.../kylin/rest/service/KylinUserService.java | 7 +++++-
.../org/apache/kylin/rest/service/UserService.java | 2 ++
6 files changed, 36 insertions(+), 7 deletions(-)
diff --git a/server-base/src/main/java/org/apache/kylin/rest/controller/UserController.java b/server-base/src/main/java/org/apache/kylin/rest/controller/UserController.java
index 6d62b38..83f6a29 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/controller/UserController.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/controller/UserController.java
@@ -217,6 +217,7 @@ public class UserController extends BasicController {
throw new BadRequestException("pwd update error");
}
+ existing = userService.copyForWrite(existing);
existing.setPassword(pwdEncode(user.getNewPassword()));
existing.setDefaultPassword(false);
logger.info("update password for user {}", user);
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/KylinAuthenticationProvider.java b/server-base/src/main/java/org/apache/kylin/rest/security/KylinAuthenticationProvider.java
index 590c15a..ed072a3 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/security/KylinAuthenticationProvider.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/KylinAuthenticationProvider.java
@@ -26,6 +26,7 @@ import java.util.Arrays;
import javax.annotation.PostConstruct;
import org.apache.kylin.common.KylinConfig;
+import org.apache.kylin.rest.constant.Constant;
import org.apache.kylin.rest.service.UserService;
import org.slf4j.Logger;
@@ -37,6 +38,7 @@ import org.springframework.cache.CacheManager;
import org.springframework.security.authentication.AuthenticationProvider;
import org.springframework.security.core.Authentication;
import org.springframework.security.core.AuthenticationException;
+import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.context.SecurityContextHolder;
import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
@@ -113,8 +115,8 @@ public class KylinAuthenticationProvider implements AuthenticationProvider {
logger.debug("User {} authorities : {}", username, user.getAuthorities());
if (!userService.userExists(username)) {
userService.createUser(user);
- } else if (needUpdateUser(user, username)) {
- userService.updateUser(user);
+ } else {
+ updateUserIfNeeded(user, username);
}
cacheManager.getCache(USER_CACHE).put(userKey, authed);
@@ -129,10 +131,22 @@ public class KylinAuthenticationProvider implements AuthenticationProvider {
return authed;
}
- // in case ldap users changing.
- private boolean needUpdateUser(ManagedUser user, String username) {
- return KylinConfig.getInstanceFromEnv().getSecurityProfile().equals("ldap")
- && !userService.loadUserByUsername(username).equals(user);
+ private void updateUserIfNeeded(ManagedUser newUser, String username) {
+ String securityProfile = KylinConfig.getInstanceFromEnv().getSecurityProfile();
+ // in case ldap users changing.
+ if (securityProfile.equals("ldap") || securityProfile.equals("saml")) {
+ UserDetails existingUser = userService.loadUserByUsername(username);
+ SimpleGrantedAuthority groupAllUsersAuthority = new SimpleGrantedAuthority(Constant.GROUP_ALL_USERS);
+ if (existingUser.getAuthorities().contains(groupAllUsersAuthority)) {
+ if (!newUser.getAuthorities().contains(groupAllUsersAuthority)) {
+ newUser.getAuthorities().add(groupAllUsersAuthority);
+ }
+ }
+ if (!existingUser.equals(newUser)) {
+ logger.info("Going to update user info for {}", username);
+ userService.updateUser(newUser);
+ }
+ }
}
@Override
diff --git a/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java b/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
index afa78b0..fef556f 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/security/KylinUserManager.java
@@ -139,4 +139,8 @@ public class KylinUserManager {
return userMap.containsKey(username);
}
}
+
+ public ManagedUser copyForWrite(ManagedUser user) {
+ return crud.copyForWrite(user);
+ }
}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserGroupService.java b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserGroupService.java
index f5563c9..7af8d76 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserGroupService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserGroupService.java
@@ -154,6 +154,7 @@ public class KylinUserGroupService extends UserGroupService {
List<ManagedUser> managedUsers = userService.listUsers();
for (ManagedUser managedUser : managedUsers) {
if (managedUser.getAuthorities().contains(new SimpleGrantedAuthority(name))) {
+ managedUser = userService.copyForWrite(managedUser);
managedUser.removeAuthorities(name);
userService.updateUser(managedUser);
}
@@ -181,12 +182,14 @@ public class KylinUserGroupService extends UserGroupService {
for (String in : moveInUsers) {
ManagedUser managedUser = (ManagedUser) userService.loadUserByUsername(in);
+ managedUser = userService.copyForWrite(managedUser);
managedUser.addAuthorities(groupName);
userService.updateUser(managedUser);
}
for (String out : moveOutUsers) {
ManagedUser managedUser = (ManagedUser) userService.loadUserByUsername(out);
+ managedUser = userService.copyForWrite(managedUser);
managedUser.removeAuthorities(groupName);
userService.updateUser(managedUser);
}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
index 1f93fc2..2d3151e 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/KylinUserService.java
@@ -38,6 +38,7 @@ import org.apache.kylin.rest.msg.MsgPicker;
import org.apache.kylin.rest.security.KylinUserManager;
import org.apache.kylin.rest.security.ManagedUser;
import org.apache.kylin.rest.util.AclEvaluate;
+import org.apache.kylin.shaded.com.google.common.base.Preconditions;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.springframework.beans.factory.annotation.Autowired;
@@ -47,7 +48,6 @@ import org.springframework.security.core.userdetails.UserDetails;
import org.springframework.security.core.userdetails.UsernameNotFoundException;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
-import org.apache.kylin.shaded.com.google.common.base.Preconditions;
public class KylinUserService implements UserService {
@@ -210,6 +210,11 @@ public class KylinUserService implements UserService {
}
@Override
+ public ManagedUser copyForWrite(ManagedUser user) {
+ return getKylinUserManager().copyForWrite(user);
+ }
+
+ @Override
public void completeUserInfo(ManagedUser user) {
}
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
index 1734be2..119e25a 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/UserService.java
@@ -34,6 +34,8 @@ public interface UserService extends UserDetailsManager {
List<String> listAdminUsers() throws IOException;
+ ManagedUser copyForWrite(ManagedUser user);
+
//For performance consideration, list all users may be incomplete(eg. not load user's authorities until authorities has benn used).
//So it's an extension point that can complete user's information latter.
//loadUserByUsername() has guarantee that the return user is complete.