You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@openmeetings.apache.org by so...@apache.org on 2020/04/25 08:31:57 UTC

[openmeetings] branch master updated: [OPENMEETINGS-2294] LDAP and OAuth users should be imported without duplicates

This is an automated email from the ASF dual-hosted git repository.

solomax pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/openmeetings.git


The following commit(s) were added to refs/heads/master by this push:
     new cc2d0bd  [OPENMEETINGS-2294] LDAP and OAuth users should be imported without duplicates
cc2d0bd is described below

commit cc2d0bd198d53e748295f14096e7b669a97ac587
Author: Maxim Solodovnik <so...@gmail.com>
AuthorDate: Sat Apr 25 15:31:40 2020 +0700

    [OPENMEETINGS-2294] LDAP and OAuth users should be imported without duplicates
---
 .../openmeetings/db/entity/server/LdapConfig.java  |   2 +-
 .../apache/openmeetings/backup/BackupImport.java   | 108 +++++++++++++++++----
 .../apache/openmeetings/backup/TestImportUser.java |  49 +++++++++-
 .../openmeetings/backup/user/ldap/ldapconfigs.xml  |  37 +++++++
 .../apache/openmeetings/backup/user/ldap/users.xml |  49 ++++++++++
 5 files changed, 220 insertions(+), 25 deletions(-)

diff --git a/openmeetings-db/src/main/java/org/apache/openmeetings/db/entity/server/LdapConfig.java b/openmeetings-db/src/main/java/org/apache/openmeetings/db/entity/server/LdapConfig.java
index d13407a..1ce2585 100644
--- a/openmeetings-db/src/main/java/org/apache/openmeetings/db/entity/server/LdapConfig.java
+++ b/openmeetings-db/src/main/java/org/apache/openmeetings/db/entity/server/LdapConfig.java
@@ -53,7 +53,7 @@ public class LdapConfig extends HistoricalEntity {
 	@Id
 	@GeneratedValue(strategy = GenerationType.IDENTITY)
 	@Column(name = "id")
-	@XmlTransient
+	@XmlElement(name = "id", required = false)
 	private Long id;
 
 	@Column(name = "name")
diff --git a/openmeetings-install/src/main/java/org/apache/openmeetings/backup/BackupImport.java b/openmeetings-install/src/main/java/org/apache/openmeetings/backup/BackupImport.java
index 891fb88..72ba1e9 100644
--- a/openmeetings-install/src/main/java/org/apache/openmeetings/backup/BackupImport.java
+++ b/openmeetings-install/src/main/java/org/apache/openmeetings/backup/BackupImport.java
@@ -149,10 +149,13 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.Objects;
+import java.util.Set;
 import java.util.TreeMap;
 import java.util.function.BiConsumer;
 import java.util.function.Consumer;
@@ -342,6 +345,8 @@ public class BackupImport {
 	@Autowired
 	private DocumentConverter docConverter;
 
+	private final Map<Long, Long> ldapMap = new HashMap<>();
+	private final Map<Long, Long> oauthMap = new HashMap<>();
 	private final Map<Long, Long> userMap = new HashMap<>();
 	private final Map<Long, Long> groupMap = new HashMap<>();
 	private final Map<Long, Long> calendarMap = new HashMap<>();
@@ -392,15 +397,7 @@ public class BackupImport {
 
 	public void performImport(InputStream is, ProgressHolder progressHolder) throws Exception {
 		progressHolder.setProgress(0);
-		userMap.clear();
-		groupMap.clear();
-		calendarMap.clear();
-		appointmentMap.clear();
-		roomMap.clear();
-		messageFolderMap.clear();
-		userContactMap.clear();
-		fileMap.clear();
-		hashMap.clear();
+		cleanup();
 		messageFolderMap.put(INBOX_FOLDER_ID, INBOX_FOLDER_ID);
 		messageFolderMap.put(SENT_FOLDER_ID, SENT_FOLDER_ID);
 		messageFolderMap.put(TRASH_FOLDER_ID, TRASH_FOLDER_ID);
@@ -475,9 +472,27 @@ public class BackupImport {
 		log.info("File explorer item import complete, clearing temp files");
 
 		FileUtils.deleteDirectory(f);
+		cleanup();
 		progressHolder.setProgress(100);
 	}
 
+	void cleanup() {
+		ldapMap.clear();
+		oauthMap.clear();
+		userMap.clear();
+		groupMap.clear();
+		calendarMap.clear();
+		appointmentMap.clear();
+		roomMap.clear();
+		messageFolderMap.clear();
+		userContactMap.clear();
+		fileMap.clear();
+		hashMap.clear();
+		messageFolderMap.put(INBOX_FOLDER_ID, INBOX_FOLDER_ID);
+		messageFolderMap.put(SENT_FOLDER_ID, SENT_FOLDER_ID);
+		messageFolderMap.put(TRASH_FOLDER_ID, TRASH_FOLDER_ID);
+	}
+
 	static BackupVersion getVersion(File base) {
 		List<BackupVersion> list = new ArrayList<>(1);
 		readList(base, "version.xml", VERSION_LIST_NODE, VERSION_NODE, BackupVersion.class, v -> list.add(v), true);
@@ -570,11 +585,15 @@ public class BackupImport {
 			if (Strings.isEmpty(c.getName()) || "local DB [internal]".equals(c.getName())) {
 				return;
 			}
+			Long oldId = c.getId();
 			c.setId(null);
 			c = ldapConfigDao.update(c, null);
 			if (defaultLdapId[0] == null) {
 				defaultLdapId[0] = c.getId();
 			}
+			if (oldId != null) {
+				ldapMap.put(oldId, c.getId());
+			}
 		});
 		return defaultLdapId[0];
 	}
@@ -586,8 +605,12 @@ public class BackupImport {
 		log.info("Ldap config import complete, starting OAuth2 server import");
 		readList(base, "oauth2servers.xml", OAUTH_LIST_NODE, OAUTH_NODE, OAuthServer.class
 				, s -> {
+					Long oldId = s.getId();
 					s.setId(null);
-					auth2Dao.update(s, null);
+					s = auth2Dao.update(s, null);
+					if (oldId != null) {
+						oauthMap.put(oldId, s.getId());
+					}
 				}, false);
 	}
 
@@ -599,14 +622,13 @@ public class BackupImport {
 		String jNameTimeZone = getDefaultTimezone();
 		//add existent emails from database
 		List<User>  users = userDao.getAllUsers();
-		final Map<String, Integer> userEmailMap = new HashMap<>();
-		final Map<String, Integer> userLoginMap = new HashMap<>();
+		final Set<String> userEmails = new HashSet<>();
+		final Set<UserKey> userLogins = new HashSet<>();
 		for (User u : users){
-			if (u.getAddress() == null || u.getAddress().getEmail() == null || User.Type.USER != u.getType()) {
-				continue;
+			if (u.getAddress() != null && !Strings.isEmpty(u.getAddress().getEmail())) {
+				userEmails.add(u.getAddress().getEmail());
 			}
-			userEmailMap.put(u.getAddress().getEmail(), Integer.valueOf(-1));
-			userLoginMap.put(u.getLogin(), Integer.valueOf(-1));
+			userLogins.add(new UserKey(u));
 		}
 		Class<User> eClazz = User.class;
 		JAXBContext jc = JAXBContext.newInstance(eClazz);
@@ -620,19 +642,33 @@ public class BackupImport {
 			}
 			// check that email is unique
 			if (u.getAddress() != null && u.getAddress().getEmail() != null && User.Type.USER == u.getType()) {
-				if (userEmailMap.containsKey(u.getAddress().getEmail())) {
+				if (userEmails.contains(u.getAddress().getEmail())) {
 					log.warn("Email is duplicated for user {}", u);
 					String updateEmail = String.format("modified_by_import_<%s>%s", randomUUID(), u.getAddress().getEmail());
 					u.getAddress().setEmail(updateEmail);
 				}
-				userEmailMap.put(u.getAddress().getEmail(), Integer.valueOf(userEmailMap.size()));
+				userEmails.add(u.getAddress().getEmail());
 			}
-			if (userLoginMap.containsKey(u.getLogin())) {
+			if (u.getType() == User.Type.LDAP) {
+				if (u.getDomainId() != null && ldapMap.containsKey(u.getDomainId())) {
+					u.setDomainId(ldapMap.get(u.getDomainId()));
+				} else {
+					log.error("Unable to find Domain for ID: {}", u.getDomainId());
+				}
+			}
+			if (u.getType() == User.Type.OAUTH) {
+				if (u.getDomainId() != null && oauthMap.containsKey(u.getDomainId())) {
+					u.setDomainId(oauthMap.get(u.getDomainId()));
+				} else {
+					log.error("Unable to find Domain for ID: {}", u.getDomainId());
+				}
+			}
+			if (userLogins.contains(new UserKey(u))) {
 				log.warn("LOGIN is duplicated for USER {}", u);
 				String updateLogin = String.format("modified_by_import_<%s>%s", randomUUID(), u.getLogin());
 				u.setLogin(updateLogin);
 			}
-			userLoginMap.put(u.getLogin(), Integer.valueOf(userLoginMap.size()));
+			userLogins.add(new UserKey(u));
 			if (u.getGroupUsers() != null) {
 				for (Iterator<GroupUser> iter = u.getGroupUsers().iterator(); iter.hasNext();) {
 					GroupUser gu = iter.next();
@@ -1279,4 +1315,36 @@ public class BackupImport {
 			}
 		}
 	}
+
+	private static class UserKey {
+		private final String login;
+		private final User.Type type;
+		private final Long domainId;
+
+		UserKey(User u) {
+			this.login = u.getLogin();
+			this.type = u.getType();
+			this.domainId = u.getDomainId();
+		}
+
+		@Override
+		public int hashCode() {
+			return Objects.hash(domainId, login, type);
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if (this == obj) {
+				return true;
+			}
+			if (obj == null) {
+				return false;
+			}
+			if (getClass() != obj.getClass()) {
+				return false;
+			}
+			UserKey other = (UserKey) obj;
+			return Objects.equals(domainId, other.domainId) && Objects.equals(login, other.login) && type == other.type;
+		}
+	}
 }
diff --git a/openmeetings-web/src/test/java/org/apache/openmeetings/backup/TestImportUser.java b/openmeetings-web/src/test/java/org/apache/openmeetings/backup/TestImportUser.java
index e493689..1075983 100644
--- a/openmeetings-web/src/test/java/org/apache/openmeetings/backup/TestImportUser.java
+++ b/openmeetings-web/src/test/java/org/apache/openmeetings/backup/TestImportUser.java
@@ -22,12 +22,19 @@ import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
 
 import java.io.File;
+import java.util.List;
 
+import org.apache.openmeetings.db.dao.server.LdapConfigDao;
+import org.apache.openmeetings.db.entity.server.LdapConfig;
 import org.apache.openmeetings.db.entity.user.User;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
+import org.springframework.beans.factory.annotation.Autowired;
 
 public class TestImportUser extends AbstractTestImport {
+	@Autowired
+	private LdapConfigDao ldapDao;
+
 	@Test
 	public void importUserNE() throws Exception {
 		Assertions.assertThrows(BackupException.class, () -> {
@@ -39,8 +46,8 @@ public class TestImportUser extends AbstractTestImport {
 	@Test
 	public void importUsers() throws Exception {
 		long userCount = userDao.count();
-		File configs = new File(getClass().getClassLoader().getResource("org/apache/openmeetings/backup/user/users.xml").toURI());
-		backupImport.importUsers(configs.getParentFile());
+		File users = new File(getClass().getClassLoader().getResource("org/apache/openmeetings/backup/user/users.xml").toURI());
+		backupImport.importUsers(users.getParentFile());
 		assertEquals(userCount + 8, userDao.count(), "Users should be added");
 		User ext = userDao.getExternalUser("234", "TheBestCms");
 		assertNotNull(ext, "External user should be imported");
@@ -49,8 +56,42 @@ public class TestImportUser extends AbstractTestImport {
 	@Test
 	public void importNoLoginDeleted() throws Exception {
 		long userCount = userDao.count();
-		File configs = new File(getClass().getClassLoader().getResource("org/apache/openmeetings/backup/user/skip/users.xml").toURI());
-		backupImport.importUsers(configs.getParentFile());
+		File users = new File(getClass().getClassLoader().getResource("org/apache/openmeetings/backup/user/skip/users.xml").toURI());
+		backupImport.importUsers(users.getParentFile());
 		assertEquals(userCount, userDao.count(), "No records should be added");
 	}
+
+	@Test
+	public void importLdap() throws Exception {
+		final String login = "omLdap2294";
+		// OPENMEETINGS-2294
+		//clean-up
+		for (LdapConfig cfg : ldapDao.get(0, Integer.MAX_VALUE)) {
+			ldapDao.delete(cfg, null);
+		}
+		File users = new File(getClass().getClassLoader().getResource("org/apache/openmeetings/backup/user/ldap/users.xml").toURI());
+		backupImport.cleanup();
+		backupImport.importLdap(users.getParentFile());
+		List<LdapConfig> ldapConfigs = ldapDao.get("om_2294_ldap", 0, 100, null);
+		assertEquals(1, ldapConfigs.size(), "There should be exactly one config");
+		LdapConfig ldap = ldapConfigs.get(0);
+
+		//clean-up
+		User u = userDao.getByLogin(login, User.Type.LDAP, ldap.getId());
+		if (u != null) {
+			userDao.purge(u, null);
+		}
+
+		//will create existing user:
+		u = getUser();
+		u.setLogin(login);
+		u.setType(User.Type.LDAP);
+		u.setDomainId(ldap.getId());
+		userDao.update(u, null);
+
+		backupImport.importUsers(users.getParentFile());
+		u = userDao.getByLogin(login, User.Type.LDAP, ldap.getId());
+		assertNotNull(u, "User should be imported");
+	}
+
 }
diff --git a/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/ldapconfigs.xml b/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/ldapconfigs.xml
new file mode 100644
index 0000000..9962f54
--- /dev/null
+++ b/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/ldapconfigs.xml
@@ -0,0 +1,37 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+
+-->
+<!-- ###############################################
+This File is auto-generated by the Backup Tool 
+you should use the BackupPanel to modify or change this file 
+see https://openmeetings.apache.org/Upgrade.html for Details 
+###############################################
+ --><root>
+   <ldapconfigs>
+      <ldapconfig>
+         <id><![CDATA[2294]]></id>
+         <name><![CDATA[om_2294_ldap]]></name>
+         <configFileName><![CDATA[ldap_conf]]></configFileName>
+         <addDomainToUserName><![CDATA[false]]></addDomainToUserName>
+         <domain><![CDATA[test]]></domain>
+         <isActive><![CDATA[true]]></isActive>
+      </ldapconfig>
+   </ldapconfigs>
+</root>
\ No newline at end of file
diff --git a/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/users.xml b/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/users.xml
new file mode 100644
index 0000000..39b9748
--- /dev/null
+++ b/openmeetings-web/src/test/resources/org/apache/openmeetings/backup/user/ldap/users.xml
@@ -0,0 +1,49 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing,
+  software distributed under the License is distributed on an
+  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+  KIND, either express or implied.  See the License for the
+  specific language governing permissions and limitations
+  under the License.
+
+-->
+<!-- ###############################################
+This File is auto-generated by the Backup Tool 
+you should use the BackupPanel to modify or change this file 
+see http://openmeetings.apache.org/Upgrade.html for Details 
+###############################################
+ --><root>
+   <users>
+      <user>
+         <user_id><![CDATA[2294]]></user_id>
+         <age class="java.util.Date"><![CDATA[1502426202010]]></age>
+         <login><![CDATA[omLdap2294]]></login>
+         <firstname><![CDATA[Totvit]]></firstname>
+         <lastname><![CDATA[Gilzeke]]></lastname>
+         <pass><![CDATA[1000:CJcOXllfzJK/CC3xWdVT6PrgW0y0FtGNhwKQPVREn6d6pBDqxtI973Yj6d2VUkpP+i2Z2I9IVvDZjOi+DSaaslmaXUV5mM89eLEqy631vcsgHVWmzsAphtdpuTUkri07hE6KKGROLkmfFtD+K0v+gLyQFA0H2M11DKF9suLc5W4=:xdqnah+D81hn7IRReRdMzA0H37mfIyTM+itARUQWKCHAAzM2dgjaA8E4YKV6+NO+p1q9gWGRbNtHRMnXXGfWhY8D5pE3P7mvSxiy0UbYXcICzl1ZT8q1hg+vNDJBMIuAx9/FHd640847yYpPM2VymMOT1rXJW3T9NXR2qibSwPKmKajy6eIzf9dRPvnLDFGOEddpfUAydPIbRjwsVxH/XzZbYR9BJ6S+7/RlK1bIdEWVjb8QWEB1VAO/LMH0gB7nNsAtjhvAZ+uOWPIYyCPBX6wmi4keb4RyOE [...]
+         <regdate class="java.util.Date"><![CDATA[1502426202011]]></regdate>
+         <title_id><![CDATA[1]]></title_id>
+         <pictureuri><![CDATA[profile.jpg]]></pictureuri>
+         <deleted><![CDATA[false]]></deleted>
+         <language_id><![CDATA[1]]></language_id>
+         <timeZoneId><![CDATA[Europe/Berlin]]></timeZoneId>
+         <forceTimeZoneCheck><![CDATA[false]]></forceTimeZoneCheck>
+         <sendSMS>false</sendSMS>
+         <showContactData><![CDATA[false]]></showContactData>
+         <showContactDataToContacts><![CDATA[false]]></showContactDataToContacts>
+         <type><![CDATA[ldap]]></type>
+         <domainId><![CDATA[2294]]></domainId>
+      </user>
+   </users>
+</root>