From 9cedcb5005ea458ce62e1cce5469edb3603f1d6a Mon Sep 17 00:00:00 2001
From: uhensler <none@none>
Date: Tue, 14 Nov 2017 15:37:01 +0100
Subject: [PATCH] OO-2981: Remove invalid authentications after the change if
an email address
---
.../user/ChangeEMailExecuteController.java | 2 +
.../bulkChange/UserBulkChangeManager.java | 2 +
.../admin/user/imp/UserImportController.java | 12 +++++
.../org/olat/basesecurity/BaseSecurity.java | 12 +++++
.../basesecurity/BaseSecurityManager.java | 29 ++++++++++++
.../org/olat/user/ProfileFormController.java | 5 ++-
.../org/olat/user/restapi/UserWebService.java | 4 +-
.../basesecurity/BaseSecurityManagerTest.java | 45 +++++++++++++++++++
8 files changed, 109 insertions(+), 2 deletions(-)
diff --git a/src/main/java/de/bps/olat/user/ChangeEMailExecuteController.java b/src/main/java/de/bps/olat/user/ChangeEMailExecuteController.java
index f6ae3d44dc2..2922f57455a 100644
--- a/src/main/java/de/bps/olat/user/ChangeEMailExecuteController.java
+++ b/src/main/java/de/bps/olat/user/ChangeEMailExecuteController.java
@@ -107,6 +107,7 @@ public class ChangeEMailExecuteController extends ChangeEMailController implemen
Identity identity = BaseSecurityManager.getInstance().loadIdentityByKey(tempKey.getIdentityKey());
if (identity != null) {
+ String oldEmail = identity.getUser().getEmail();
identity.getUser().setProperty("email", mails.get("changedEMail"));
// if old mail address closed then set the new mail address
// unclosed
@@ -122,6 +123,7 @@ public class ChangeEMailExecuteController extends ChangeEMailController implemen
// remove keys
identity.getUser().setProperty("emchangeKey", null);
userRequest.getUserSession().removeEntryFromNonClearedStore(ChangeEMailController.CHANGE_EMAIL_ENTRY);
+ BaseSecurityManager.getInstance().deleteInvalidAuthenticationsByEmail(oldEmail);
} else {
// error message
wControl.setWarning(pT.translate("error.change.email.unexpected", new String[] { mails.get("currentEMail"), mails.get("changedEMail") }));
diff --git a/src/main/java/org/olat/admin/user/bulkChange/UserBulkChangeManager.java b/src/main/java/org/olat/admin/user/bulkChange/UserBulkChangeManager.java
index a8c4c1a2ef1..5eef9ece5ec 100644
--- a/src/main/java/org/olat/admin/user/bulkChange/UserBulkChangeManager.java
+++ b/src/main/java/org/olat/admin/user/bulkChange/UserBulkChangeManager.java
@@ -133,6 +133,7 @@ public class UserBulkChangeManager implements InitializingBean {
//reload identity from cache, to prevent stale object
identity = securityManager.loadIdentityByKey(identity.getKey());
User user = identity.getUser();
+ String oldEmail = user.getEmail();
String errorDesc = "";
boolean updateError = false;
// change pwd
@@ -245,6 +246,7 @@ public class UserBulkChangeManager implements InitializingBean {
notUpdatedIdentities.add(errorOutput);
} else {
userManager.updateUserFromIdentity(identity);
+ securityManager.deleteInvalidAuthenticationsByEmail(oldEmail);
changedIdentities.add(identity);
log.audit("User::" + addingIdentity.getName() + " successfully changed account data for user::" + identity.getName() + " in bulk change", null);
}
diff --git a/src/main/java/org/olat/admin/user/imp/UserImportController.java b/src/main/java/org/olat/admin/user/imp/UserImportController.java
index 4ca3d728a20..a763f9c8db3 100644
--- a/src/main/java/org/olat/admin/user/imp/UserImportController.java
+++ b/src/main/java/org/olat/admin/user/imp/UserImportController.java
@@ -38,6 +38,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import org.apache.velocity.VelocityContext;
import org.olat.basesecurity.Authentication;
import org.olat.basesecurity.BaseSecurity;
+import org.olat.basesecurity.BaseSecurityManager;
import org.olat.core.commons.persistence.DB;
import org.olat.core.commons.persistence.DBFactory;
import org.olat.core.gui.UserRequest;
@@ -204,8 +205,10 @@ public class UserImportController extends BasicController {
Identity identity;
if(updateUsers != null && updateUsers.booleanValue()) {
identity = userToUpdate.getIdentity(true);
+ String oldEmail = loadEmail(identity);
if(um.updateUserFromIdentity(identity)) {
report.incrementUpdatedUser();
+ securityManager.deleteInvalidAuthenticationsByEmail(oldEmail);
}
} else {
identity = userToUpdate.getIdentity();
@@ -238,6 +241,15 @@ public class UserImportController extends BasicController {
return userToUpdate.getIdentity();
}
+ private String loadEmail(Identity updatedIdentity) {
+ String email = null;
+ Identity oldIdentity = BaseSecurityManager.getInstance().loadIdentityByKey(updatedIdentity.getKey());
+ if (oldIdentity != null) {
+ email = oldIdentity.getUser().getEmail();
+ }
+ return email;
+ }
+
@Override
protected void doDispose() {
// child controllers disposed by basic controller
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurity.java b/src/main/java/org/olat/basesecurity/BaseSecurity.java
index 6445099485a..4f62739aef7 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurity.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurity.java
@@ -402,6 +402,18 @@ public interface BaseSecurity {
*/
public void deleteAuthentication(Authentication authentication);
+ /**
+ * Deletes invalid authentications with the specified email address in the field
+ * username. An authentication is invalid if no unique user with that email
+ * exists. Use this method to clean old authentications after the change of the
+ * email address of a user to avoid duplicate authentications. Because a user
+ * can have an email address as his username, the authentication of the OLAT
+ * authentication provider is never deleted.
+ *
+ * @param email
+ */
+ public void deleteInvalidAuthenticationsByEmail(String email);
+
/**
*
* @param authentication
diff --git a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
index 20ce0a75a79..1802c4acb63 100644
--- a/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
+++ b/src/main/java/org/olat/basesecurity/BaseSecurityManager.java
@@ -27,6 +27,7 @@
package org.olat.basesecurity;
import java.util.ArrayList;
+import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.Date;
@@ -53,6 +54,7 @@ import org.olat.core.commons.persistence.DB;
import org.olat.core.commons.persistence.DBFactory;
import org.olat.core.commons.persistence.DBQuery;
import org.olat.core.commons.persistence.PersistenceHelper;
+import org.olat.core.commons.services.webdav.manager.WebDAVAuthManager;
import org.olat.core.gui.translator.Translator;
import org.olat.core.id.Identity;
import org.olat.core.id.ModifiedInfo;
@@ -65,6 +67,7 @@ import org.olat.core.logging.OLog;
import org.olat.core.logging.Tracing;
import org.olat.core.util.Encoder;
import org.olat.core.util.Encoder.Algorithm;
+import org.olat.core.util.StringHelper;
import org.olat.core.util.Util;
import org.olat.core.util.coordinate.CoordinatorManager;
import org.olat.core.util.coordinate.SyncerCallback;
@@ -1512,6 +1515,32 @@ public class BaseSecurityManager implements BaseSecurity {
log.error("", e);
}
}
+
+ @Override
+ public void deleteInvalidAuthenticationsByEmail(String email) {
+ if (!StringHelper.containsNonWhitespace(email)) return;
+
+ // If a user with this email exists the email is valid.
+ Identity identity = UserManager.getInstance().findUniqueIdentityByEmail(email);
+ if (identity != null) return;
+
+ StringBuilder sb = new StringBuilder();
+ sb.append("delete from ").append(AuthenticationImpl.class.getName()).append(" as auth");
+ sb.append(" where auth.authusername=:authusername");
+ sb.append(" and auth.provider in (:providers)");
+
+ List<String> providers = Arrays.asList(
+ WebDAVAuthManager.PROVIDER_HA1_EMAIL,
+ WebDAVAuthManager.PROVIDER_HA1_INSTITUTIONAL_EMAIL,
+ WebDAVAuthManager.PROVIDER_WEBDAV_EMAIL,
+ WebDAVAuthManager.PROVIDER_WEBDAV_INSTITUTIONAL_EMAIL);
+
+ dbInstance.getCurrentEntityManager()
+ .createQuery(sb.toString())
+ .setParameter("authusername", email)
+ .setParameter("providers", providers)
+ .executeUpdate();
+ }
/**
* @see org.olat.basesecurity.Manager#findAuthenticationByAuthusername(java.lang.String, java.lang.String)
diff --git a/src/main/java/org/olat/user/ProfileFormController.java b/src/main/java/org/olat/user/ProfileFormController.java
index 1bc8bec9bb6..53e47229898 100644
--- a/src/main/java/org/olat/user/ProfileFormController.java
+++ b/src/main/java/org/olat/user/ProfileFormController.java
@@ -107,6 +107,8 @@ public class ProfileFormController extends FormBasicController {
@Autowired
private UserManager userManager;
@Autowired
+ private BaseSecurityManager securityManager;
+ @Autowired
private RegistrationManager rm;
@Autowired
private MailManager mailManager;
@@ -486,7 +488,8 @@ public class ProfileFormController extends FormBasicController {
TemporaryKey tempKey = rm.loadTemporaryKeyByRegistrationKey(key);
if (tempKey != null) {
rm.deleteTemporaryKey(tempKey);
- }
+ }
+ securityManager.deleteInvalidAuthenticationsByEmail(currentEmail);
} else {
emailChanged = true;
// change email address to old address until it is verified
diff --git a/src/main/java/org/olat/user/restapi/UserWebService.java b/src/main/java/org/olat/user/restapi/UserWebService.java
index 5ca79548c26..8c8b61cd337 100644
--- a/src/main/java/org/olat/user/restapi/UserWebService.java
+++ b/src/main/java/org/olat/user/restapi/UserWebService.java
@@ -780,8 +780,10 @@ public class UserWebService {
retrievedIdentity = baseSecurity.setExternalId(retrievedIdentity, user.getExternalId());
retrievedUser = retrievedIdentity.getUser();
}
+ String oldEmail = retrievedUser.getEmail();
post(retrievedUser, user, getLocale(request));
UserManager.getInstance().updateUser(retrievedUser);
+ BaseSecurityManager.getInstance().deleteInvalidAuthenticationsByEmail(oldEmail);
return Response.ok(get(retrievedIdentity, true, true)).build();
}
@@ -794,7 +796,7 @@ public class UserWebService {
return Response.serverError().status(Status.INTERNAL_SERVER_ERROR).build();
}
}
-
+
private List<ErrorVO> validateUser(User user, UserVO userVo, HttpServletRequest request) {
UserManager um = UserManager.getInstance();
diff --git a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
index 030c1185589..fce34a0899c 100644
--- a/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
+++ b/src/test/java/org/olat/basesecurity/BaseSecurityManagerTest.java
@@ -36,6 +36,7 @@ import java.util.UUID;
import org.junit.Assert;
import org.junit.Test;
import org.olat.core.commons.persistence.DB;
+import org.olat.core.commons.services.webdav.manager.WebDAVAuthManager;
import org.olat.core.id.Identity;
import org.olat.core.id.Roles;
import org.olat.core.id.User;
@@ -971,4 +972,48 @@ public class BaseSecurityManagerTest extends OlatTestCase {
Assert.assertEquals(identity, reloadedId);
dbInstance.commitAndCloseSession();
}
+
+ @Test
+ public void deleteInvalidAuthenticationsByEmail() {
+ Identity identity = JunitTestHelper.createAndPersistIdentityAsUser("auth-del-email-" + UUID.randomUUID().toString());
+ User user = identity.getUser();
+ String email = user.getEmail();
+ securityManager.createAndPersistAuthentication(identity, "OLAT", email, "secret", Encoder.Algorithm.sha512);
+ securityManager.createAndPersistAuthentication(identity, "del-mail", email, "secret", Encoder.Algorithm.sha512);
+ securityManager.createAndPersistAuthentication(identity, WebDAVAuthManager.PROVIDER_HA1_EMAIL, email, "secret", Encoder.Algorithm.sha512);
+ securityManager.createAndPersistAuthentication(identity, WebDAVAuthManager.PROVIDER_HA1_INSTITUTIONAL_EMAIL, email, "secret", Encoder.Algorithm.sha512);
+ securityManager.createAndPersistAuthentication(identity, WebDAVAuthManager.PROVIDER_WEBDAV_EMAIL, email, "secret", Encoder.Algorithm.sha512);
+ securityManager.createAndPersistAuthentication(identity, WebDAVAuthManager.PROVIDER_WEBDAV_INSTITUTIONAL_EMAIL, email, "secret", Encoder.Algorithm.sha512);
+ dbInstance.commitAndCloseSession();
+
+ // User with email address exists: The authentications are valid.
+ securityManager.deleteInvalidAuthenticationsByEmail(email);
+ dbInstance.commitAndCloseSession();
+
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_HA1_EMAIL));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_HA1_INSTITUTIONAL_EMAIL));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_WEBDAV_EMAIL));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_WEBDAV_INSTITUTIONAL_EMAIL));
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, "OLAT"));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(identity.getName(), "OLAT"));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, "del-mail"));
+
+ // Email of the user changed: The authentications are not valid any longer.
+ user.setProperty(UserConstants.EMAIL, "new@trashcmail.com");
+ user.setProperty(UserConstants.INSTITUTIONALEMAIL, "new@trashcmail.com");
+ userManager.updateUser(user);
+ dbInstance.commitAndCloseSession();
+
+ securityManager.deleteInvalidAuthenticationsByEmail(email);
+ dbInstance.commitAndCloseSession();
+
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_HA1_EMAIL));
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_HA1_INSTITUTIONAL_EMAIL));
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_WEBDAV_EMAIL));
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, WebDAVAuthManager.PROVIDER_WEBDAV_INSTITUTIONAL_EMAIL));
+ Assert.assertNull(securityManager.findAuthenticationByAuthusername(email, "OLAT"));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(identity.getName(), "OLAT"));
+ Assert.assertNotNull(securityManager.findAuthenticationByAuthusername(email, "del-mail"));
+ }
+
}
--
GitLab