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 2023/02/27 08:01:12 UTC
[kylin] 32/34: KYLIN-5457 fix ldap authorize user group project permissions
This is an automated email from the ASF dual-hosted git repository.
xxyu pushed a commit to branch kylin5
in repository https://gitbox.apache.org/repos/asf/kylin.git
commit 1cebc04393638587e22631016ceb3cbf9bf6adba
Author: Jiale He <35...@users.noreply.github.com>
AuthorDate: Thu Jan 5 18:49:21 2023 +0800
KYLIN-5457 fix ldap authorize user group project permissions
* KYLIN-5457 fix ldap authorize user group project permissions
* KYLIN-5457 fix OpenUserGroupService
---
.../kylin/rest/service/LdapUserGroupService.java | 13 ++++
.../kylin/rest/service/OpenUserGroupService.java | 15 +++-
.../kylin/rest/service/LdapUserServiceTest.java | 79 ++++++++++++----------
.../kylin/rest/service/OpenUserServiceTest.java | 25 ++++++-
4 files changed, 92 insertions(+), 40 deletions(-)
diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/service/LdapUserGroupService.java b/src/common-service/src/main/java/org/apache/kylin/rest/service/LdapUserGroupService.java
index de41aff9de..47a1dd17c4 100644
--- a/src/common-service/src/main/java/org/apache/kylin/rest/service/LdapUserGroupService.java
+++ b/src/common-service/src/main/java/org/apache/kylin/rest/service/LdapUserGroupService.java
@@ -233,4 +233,17 @@ public class LdapUserGroupService extends NUserGroupService {
public String getUuidByGroupName(String groupName) {
return groupName;
}
+
+ @Override
+ public boolean exists(String name) {
+ return getAllUserGroups().contains(name);
+ }
+
+ @Override
+ public Set<String> listUserGroups(String username) {
+ return getAllUserGroups().stream()
+ .filter(group -> getGroupMembersByName(group).stream()
+ .anyMatch(user -> StringUtils.equalsIgnoreCase(username, user.getUsername())))
+ .collect(Collectors.toSet());
+ }
}
diff --git a/src/common-service/src/main/java/org/apache/kylin/rest/service/OpenUserGroupService.java b/src/common-service/src/main/java/org/apache/kylin/rest/service/OpenUserGroupService.java
index cb751efde3..d26e91e85d 100644
--- a/src/common-service/src/main/java/org/apache/kylin/rest/service/OpenUserGroupService.java
+++ b/src/common-service/src/main/java/org/apache/kylin/rest/service/OpenUserGroupService.java
@@ -20,11 +20,12 @@ package org.apache.kylin.rest.service;
import java.util.List;
import java.util.Locale;
+import java.util.Set;
import java.util.stream.Collectors;
import org.apache.commons.lang.StringUtils;
-import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.common.annotation.ThirdPartyDependencies;
+import org.apache.kylin.common.msg.MsgPicker;
import org.apache.kylin.metadata.user.ManagedUser;
import org.apache.kylin.metadata.usergroup.UserGroup;
@@ -80,4 +81,16 @@ public abstract class OpenUserGroupService extends NUserGroupService {
.collect(Collectors.toList());
}
+ @Override
+ public boolean exists(String name) {
+ return getAllUserGroups().contains(name);
+ }
+
+ @Override
+ public Set<String> listUserGroups(String username) {
+ return getAllUserGroups().stream()
+ .filter(group -> getGroupMembersByName(group).stream()
+ .anyMatch(user -> StringUtils.equalsIgnoreCase(username, user.getUsername())))
+ .collect(Collectors.toSet());
+ }
}
diff --git a/src/common-service/src/test/java/org/apache/kylin/rest/service/LdapUserServiceTest.java b/src/common-service/src/test/java/org/apache/kylin/rest/service/LdapUserServiceTest.java
index 1b07ad2985..b43d62b638 100644
--- a/src/common-service/src/test/java/org/apache/kylin/rest/service/LdapUserServiceTest.java
+++ b/src/common-service/src/test/java/org/apache/kylin/rest/service/LdapUserServiceTest.java
@@ -25,8 +25,8 @@ import static org.springframework.security.test.web.servlet.response.SecurityMoc
import static org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers.unauthenticated;
import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity;
-import java.io.FileInputStream;
import java.io.IOException;
+import java.nio.file.Files;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
@@ -53,9 +53,7 @@ import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
-import org.junit.Rule;
import org.junit.Test;
-import org.junit.rules.ExpectedException;
import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.Mockito;
@@ -79,7 +77,6 @@ import org.springframework.web.context.WebApplicationContext;
import com.google.common.cache.Cache;
import com.google.common.collect.ImmutableMap;
-import com.google.common.collect.Lists;
import com.unboundid.ldap.listener.InMemoryDirectoryServer;
import com.unboundid.ldap.listener.InMemoryDirectoryServerConfig;
import com.unboundid.ldap.listener.InMemoryListenerConfig;
@@ -119,14 +116,11 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
@Qualifier("userGroupService")
LdapUserGroupService userGroupService;
- @Rule
- public ExpectedException thrown = ExpectedException.none();
-
@BeforeClass
public static void setupResource() throws Exception {
staticCreateTestMetadata();
Properties ldapConfig = new Properties();
- ldapConfig.load(new FileInputStream(new ClassPathResource(LDAP_CONFIG).getFile()));
+ ldapConfig.load(Files.newInputStream(new ClassPathResource(LDAP_CONFIG).getFile().toPath()));
final KylinConfig kylinConfig = getTestConfig();
overwriteSystemPropBeforeClass("kylin.security.ldap.max-page-size", "1");
ldapConfig.forEach((k, v) -> kylinConfig.setProperty(k.toString(), v.toString()));
@@ -146,7 +140,7 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
}
@AfterClass
- public static void cleanupResource() throws Exception {
+ public static void cleanupResource() {
directoryServer.shutDown(true);
staticCleanupTestMetadata();
}
@@ -181,26 +175,22 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
@Test
public void testCreateUser() {
- thrown.expect(UnsupportedOperationException.class);
- ldapUserService.createUser(null);
+ Assert.assertThrows(UnsupportedOperationException.class, () -> ldapUserService.createUser(null));
}
@Test
public void testUpdateUser() {
- thrown.expect(UnsupportedOperationException.class);
- ldapUserService.updateUser(null);
+ Assert.assertThrows(UnsupportedOperationException.class, () -> ldapUserService.updateUser(null));
}
@Test
public void testDeleteUser() {
- thrown.expect(UnsupportedOperationException.class);
- ldapUserService.deleteUser("ben");
+ Assert.assertThrows(UnsupportedOperationException.class, () -> ldapUserService.deleteUser("ben"));
}
@Test
public void testChangePassword() {
- thrown.expect(UnsupportedOperationException.class);
- ldapUserService.changePassword("old", "new");
+ Assert.assertThrows(UnsupportedOperationException.class, () -> ldapUserService.changePassword("old", "new"));
}
@Test
@@ -216,8 +206,8 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
}
@Test
- public void testListUsers() throws Exception {
- Set<String> users = ldapUserService.listUsers().stream().map(x -> x.getUsername()).collect(toSet());
+ public void testListUsers() {
+ Set<String> users = ldapUserService.listUsers().stream().map(ManagedUser::getUsername).collect(toSet());
Assert.assertEquals(6, users.size());
List<ManagedUser> managedUserList = ldapUserService.listUsers();
for (val user : managedUserList) {
@@ -231,7 +221,7 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
}
@Test
- public void testListSuperAdminUsers() throws Exception {
+ public void testListSuperAdminUsers() {
getTestConfig().setProperty("kylin.security.acl.super-admin-username", "jenny");
Assert.assertEquals("jenny", ldapUserService.listSuperAdminUsers().get(0));
}
@@ -239,14 +229,15 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
@Test
public void testLoadUserByUsername() {
Assert.assertTrue(ldapUserService.loadUserByUsername("jenny").getAuthorities().stream()
- .map(x -> x.getAuthority()).collect(toSet()).contains("ROLE_ADMIN"));
+ .map(GrantedAuthority::getAuthority).collect(toSet()).contains("ROLE_ADMIN"));
}
@Test
public void testCompleteUserInfoInternal() {
ManagedUser user = new ManagedUser("oliver", "", false);
ldapUserService.completeUserInfoInternal(user);
- Set<String> authorities = user.getAuthorities().stream().map(x -> x.getAuthority()).collect(toSet());
+ Set<String> authorities = user.getAuthorities().stream().map(SimpleGrantedAuthority::getAuthority)
+ .collect(toSet());
Assert.assertFalse(authorities.contains("ROLE_ADMIN"));
Assert.assertTrue(authorities.contains("itpeople"));
}
@@ -261,34 +252,33 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
}
@Test
- public void testOnNewUserAdded() throws Exception {
+ public void testOnNewUserAdded() {
Assert.assertTrue(ldapUserService.userExists("rick"));
ldapUserService.onUserAuthenticated("rick");
Assert.assertTrue(ldapUserService.userExists("rick"));
}
@Test
- public void testOnUserWithoutPassword() throws Exception {
+ public void testOnUserWithoutPassword() {
ldapUserService.onUserAuthenticated("ricky");
Assert.assertTrue(ldapUserService.userExists("ricky"));
}
@Test
public void testAddGroup() {
- thrown.expect(UnsupportedOperationException.class);
- userGroupService.addGroup("gg");
+ Assert.assertThrows(UnsupportedOperationException.class, () -> userGroupService.addGroup("gg"));
}
@Test
public void testUpdateUserGroup() {
- thrown.expect(UnsupportedOperationException.class);
- userGroupService.modifyGroupUsers("gg", Lists.newArrayList());
+ List<String> emptyUserGroup = Collections.emptyList();
+ Assert.assertThrows(UnsupportedOperationException.class,
+ () -> userGroupService.modifyGroupUsers("gg", emptyUserGroup));
}
@Test
public void testDeleteUserGroup() {
- thrown.expect(UnsupportedOperationException.class);
- userGroupService.deleteGroup("gg");
+ Assert.assertThrows(UnsupportedOperationException.class, () -> userGroupService.deleteGroup("gg"));
}
@Test
@@ -360,7 +350,7 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
public void testGroupNameByUuidAndUuidByGroupName() throws IOException {
List<UserGroupResponseKI> userGroupResponse = userGroupService.getUserGroupResponse(
userGroupService.getAllUserGroups().stream().map(UserGroup::new).collect(Collectors.toList()));
- userGroupResponse.stream().forEach(response -> {
+ userGroupResponse.forEach(response -> {
String groupName = response.getGroupName();
String uuidByGroupName = userGroupService.getUuidByGroupName(response.getGroupName());
Assert.assertEquals(groupName, uuidByGroupName);
@@ -372,7 +362,7 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
}
@Test
- public void testGetDnMapperMap() throws Exception {
+ public void testGetDnMapperMap() {
String cacheKey = ReflectionTestUtils.getField(LdapUserService.class, "LDAP_VALID_DN_MAP_KEY").toString();
Cache cache = (Cache) ReflectionTestUtils.getField(LdapUserService.class, "LDAP_VALID_DN_MAP_CACHE");
cache.invalidate(cacheKey);
@@ -384,8 +374,8 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
public void testSameNameUserInvalidation() {
Assert.assertFalse(
ldapUserService.listUsers().stream().map(ManagedUser::getUsername).collect(toSet()).contains("user"));
- Assert.assertFalse(userGroupService.getUserAndUserGroup().entrySet().stream().map(Map.Entry::getValue)
- .map(HashSet::new).map(set -> set.contains("user")).reduce(Boolean::logicalOr).get().booleanValue());
+ Assert.assertFalse(userGroupService.getUserAndUserGroup().values().stream().map(HashSet::new)
+ .map(set -> set.contains("user")).reduce(Boolean::logicalOr).get());
}
@Test
@@ -429,12 +419,29 @@ public class LdapUserServiceTest extends NLocalFileMetadataTestCase {
getTestConfig().setProperty("kylin.security.acl.data-permission-default-enabled", "false");
userAclService.syncAdminUserAcl(Collections.emptyList(), false);
Assert.assertNull(userAclManager.get("jenny"));
- userAclService.syncAdminUserAcl(Arrays.asList("jenny"), true);
+ userAclService.syncAdminUserAcl(Collections.singletonList("jenny"), true);
Assert.assertTrue(userAclManager.get("jenny").hasPermission(AclPermission.DATA_QUERY));
getTestConfig().setProperty("kylin.security.acl.super-admin-username", "");
userAclManager.delete("jenny");
- userAclService.syncAdminUserAcl(Arrays.asList("jenny"), true);
+ userAclService.syncAdminUserAcl(Collections.singletonList("jenny"), true);
Assert.assertFalse(userAclManager.get("jenny").hasPermission(AclPermission.DATA_QUERY));
}
+
+ @Test
+ public void testUserGroupExists() {
+ Assert.assertTrue(userGroupService.exists("admin"));
+ Assert.assertFalse(userGroupService.exists("not_exist_group"));
+ }
+
+ @Test
+ public void testListUserGroupsByUsername() {
+ Assert.assertTrue(ldapUserService.userExists("johnny"));
+ Set<String> johnnyGroups = userGroupService.listUserGroups("johnny");
+ Assert.assertFalse(johnnyGroups.isEmpty());
+
+ Assert.assertFalse(ldapUserService.userExists("not_exist_user"));
+ Set<String> notExistUser = userGroupService.listUserGroups("not_exist_user");
+ Assert.assertTrue(notExistUser.isEmpty());
+ }
}
diff --git a/src/common-service/src/test/java/org/apache/kylin/rest/service/OpenUserServiceTest.java b/src/common-service/src/test/java/org/apache/kylin/rest/service/OpenUserServiceTest.java
index 85852ff3ab..c8ad9e6b68 100644
--- a/src/common-service/src/test/java/org/apache/kylin/rest/service/OpenUserServiceTest.java
+++ b/src/common-service/src/test/java/org/apache/kylin/rest/service/OpenUserServiceTest.java
@@ -18,12 +18,13 @@
package org.apache.kylin.rest.service;
-import java.io.FileInputStream;
+import java.nio.file.Files;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Properties;
+import java.util.Set;
import org.apache.kylin.common.KylinConfig;
import org.apache.kylin.common.scheduler.EventBusFactory;
@@ -96,7 +97,8 @@ public class OpenUserServiceTest extends NLocalFileMetadataTestCase {
public static void setupResource() throws Exception {
staticCreateTestMetadata();
Properties ldapConfig = new Properties();
- ldapConfig.load(new FileInputStream(new ClassPathResource("ut_custom/custom-config.properties").getFile()));
+ ldapConfig.load(
+ Files.newInputStream(new ClassPathResource("ut_custom/custom-config.properties").getFile().toPath()));
final KylinConfig kylinConfig = getTestConfig();
ldapConfig.forEach((k, v) -> kylinConfig.setProperty(k.toString(), v.toString()));
@@ -247,7 +249,7 @@ public class OpenUserServiceTest extends NLocalFileMetadataTestCase {
@Test
public void testDoAfterListAdminUsers() {
- List adminUserList = Arrays.asList("admin", "sunny");
+ List<String> adminUserList = Arrays.asList("admin", "sunny");
val adminUserAspect = SpringContext.getBean(AdminUserAspect.class);
adminUserAspect.doAfterListAdminUsers(adminUserList);
Assert.assertTrue(((List) ReflectionTestUtils.getField(adminUserAspect, "adminUserList")).contains("sunny"));
@@ -276,4 +278,21 @@ public class OpenUserServiceTest extends NLocalFileMetadataTestCase {
userAclService.syncAdminUserAcl();
Assert.assertTrue(userAclService.hasUserAclPermission("admin", AclPermission.DATA_QUERY));
}
+
+ @Test
+ public void testUserGroupExists() {
+ Assert.assertTrue(userGroupService.exists("ROLE_ADMIN"));
+ Assert.assertFalse(userGroupService.exists("not_exist_group"));
+ }
+
+ @Test
+ public void testListUserGroupsByUsername() {
+ Assert.assertTrue(userService.userExists("test"));
+ Set<String> testGroups = userGroupService.listUserGroups("test");
+ Assert.assertFalse(testGroups.isEmpty());
+
+ Assert.assertFalse(userService.userExists("not_exist_user"));
+ Set<String> notExistUser = userGroupService.listUserGroups("not_exist_user");
+ Assert.assertTrue(notExistUser.isEmpty());
+ }
}