From ecc8a7d5a37b4f6f435b294e6041ec6b1dea923c Mon Sep 17 00:00:00 2001
From: srosse <stephane.rosse@frentix.com>
Date: Thu, 15 Oct 2020 09:05:17 +0200
Subject: [PATCH] OO-4979: validate nickname in Shibboleth registration
 workflow

---
 .../olat/shibboleth/ShibbolethDispatcher.java | 16 ++++----
 .../ShibbolethRegistrationForm.java           | 38 +++++++++++++------
 ...bbolethRegistrationUserPropertiesFrom.java |  4 +-
 .../database/mysql/setupDatabase.sql          |  2 +
 .../database/oracle/setupDatabase.sql         |  2 +-
 .../postgresql/alter_15_2_x_to_15_2_6.sql     |  4 ++
 .../database/postgresql/setupDatabase.sql     |  2 +
 7 files changed, 45 insertions(+), 23 deletions(-)

diff --git a/src/main/java/org/olat/shibboleth/ShibbolethDispatcher.java b/src/main/java/org/olat/shibboleth/ShibbolethDispatcher.java
index fe2eb308971..5731e27f0be 100644
--- a/src/main/java/org/olat/shibboleth/ShibbolethDispatcher.java
+++ b/src/main/java/org/olat/shibboleth/ShibbolethDispatcher.java
@@ -26,7 +26,8 @@
 package org.olat.shibboleth;
 
 import java.io.IOException;
-import java.io.UnsupportedEncodingException;
+import java.nio.charset.StandardCharsets;
+import java.util.Collection;
 import java.util.Enumeration;
 import java.util.HashMap;
 import java.util.Map;
@@ -194,25 +195,23 @@ public class ShibbolethDispatcher implements Dispatcher{
 	private Map<String, String> getShibbolethAttributesFromRequest(HttpServletRequest req) {
 		Map<String, String> attributesMap = new HashMap<>();
 		Enumeration<String> headerEnum = req.getHeaderNames();
+		Collection<String> attributeNames = shibbolethModule.getShibbolethAttributeNames();
 		while(headerEnum.hasMoreElements()) {
 			String attributeName = headerEnum.nextElement();
 			String attributeValue = req.getHeader(attributeName);
 
 			try {
-				attributeValue = new String(attributeValue.getBytes("ISO-8859-1"), "UTF-8");
-				if (shibbolethModule.getShibbolethAttributeNames().contains(attributeName)) {
+				attributeValue = new String(attributeValue.getBytes(StandardCharsets.ISO_8859_1), StandardCharsets.UTF_8);
+				if (attributeNames.contains(attributeName)) {
 					attributesMap.put(attributeName, attributeValue);
 				}
-			} catch (UnsupportedEncodingException e) {
+			} catch (Exception e) {
 				//bad luck
 				throw new AssertException("ISO-8859-1, or UTF-8 Encoding not supported",e);
 			}
 		}
 
-		if(log.isDebugEnabled()){
-			log.debug("Shib attribute Map: \n\n"+attributesMap.toString()+"\n\n");
-		}
-
+		log.debug("Shib attribute Map:  \n\n{}\n\n", attributesMap);
 		return attributesMap;
 	}
 	
@@ -287,7 +286,6 @@ public class ShibbolethDispatcher implements Dispatcher{
 				default: userMsg = transl.translate("error.shibboleth.generic"); break;
 			}
 			showMessage(ureq,"org.opensaml.SAMLException: " + e.getMessage(), e, userMsg, ((ShibbolethException)e).getContactPersonEmail());
-			return;
 		} else {
 		  try {
 			  ChiefController msgcc = MsgFactory.createMessageChiefController(ureq,
diff --git a/src/main/java/org/olat/shibboleth/ShibbolethRegistrationForm.java b/src/main/java/org/olat/shibboleth/ShibbolethRegistrationForm.java
index 43e44caebbf..46d6f342ec3 100644
--- a/src/main/java/org/olat/shibboleth/ShibbolethRegistrationForm.java
+++ b/src/main/java/org/olat/shibboleth/ShibbolethRegistrationForm.java
@@ -25,7 +25,10 @@
 
 package org.olat.shibboleth;
 
+import java.util.List;
+
 import org.olat.admin.user.imp.TransientIdentity;
+import org.olat.basesecurity.BaseSecurity;
 import org.olat.core.gui.UserRequest;
 import org.olat.core.gui.components.form.flexible.FormItemContainer;
 import org.olat.core.gui.components.form.flexible.elements.TextElement;
@@ -33,6 +36,7 @@ import org.olat.core.gui.components.form.flexible.impl.FormBasicController;
 import org.olat.core.gui.control.Controller;
 import org.olat.core.gui.control.Event;
 import org.olat.core.gui.control.WindowControl;
+import org.olat.core.id.Identity;
 import org.olat.core.util.StringHelper;
 import org.olat.core.util.Util;
 import org.olat.login.validation.SyntaxValidator;
@@ -57,6 +61,8 @@ public class ShibbolethRegistrationForm extends FormBasicController {
 
 	private final SyntaxValidator usernameSyntaxValidator;
 
+	@Autowired
+	private BaseSecurity securityManager;
 	@Autowired
 	private UsernameValidationRulesFactory usernameRulesFactory;
 
@@ -71,23 +77,33 @@ public class ShibbolethRegistrationForm extends FormBasicController {
 	@Override
 	protected boolean validateFormLogic(UserRequest ureq) {
 		// validate if username does match the syntactical login requirements
+		boolean allOk = super.validateFormLogic(ureq);
+		
 		usernameEl.clearError();
 		String username = usernameEl.getValue();
-		if (!StringHelper.containsNonWhitespace(username)) {
+		if (StringHelper.containsNonWhitespace(username)) {
+			TransientIdentity newIdentity = new TransientIdentity();
+			newIdentity.setName(username);
+			ValidationResult validationResult = usernameSyntaxValidator.validate(username, newIdentity);
+			if (!validationResult.isValid()) {
+				String descriptions = validationResult.getInvalidDescriptions().get(0).getText(getLocale());
+				usernameEl.setErrorKey("error.username.invalid", new String[] { descriptions });
+				allOk &= false;
+			} else if(!isNickNameUnique(username)) {
+				usernameEl.setErrorKey("sm.error.username_in_use", null);
+				allOk &= false;
+			}
+		} else {
 			usernameEl.setErrorKey("form.legende.mandatory", null);
 			return false;
 		} 
 		
-		TransientIdentity newIdentity = new TransientIdentity();
-		newIdentity.setName(username);
-		ValidationResult validationResult = usernameSyntaxValidator.validate(username, newIdentity);
-		if (!validationResult.isValid()) {
-			String descriptions = validationResult.getInvalidDescriptions().get(0).getText(getLocale());
-			usernameEl.setErrorKey("error.username.invalid", new String[] { descriptions });
-			return false;
-		}
-		
-		return true;
+		return allOk;
+	}
+	
+	private boolean isNickNameUnique(String val) {
+		List<Identity> identities = securityManager.findIdentitiesByNickName(val);
+		return identities.isEmpty();
 	}
 
 	/**
diff --git a/src/main/java/org/olat/shibboleth/ShibbolethRegistrationUserPropertiesFrom.java b/src/main/java/org/olat/shibboleth/ShibbolethRegistrationUserPropertiesFrom.java
index 7b2e0e3b144..96c3a5cb841 100644
--- a/src/main/java/org/olat/shibboleth/ShibbolethRegistrationUserPropertiesFrom.java
+++ b/src/main/java/org/olat/shibboleth/ShibbolethRegistrationUserPropertiesFrom.java
@@ -94,7 +94,7 @@ public class ShibbolethRegistrationUserPropertiesFrom extends FormBasicControlle
 
 	@Override
 	protected boolean validateFormLogic(UserRequest ureq) {
-		boolean allOk = true;
+		boolean allOk = super.validateFormLogic(ureq);
 
 		// validate each user field
 		for (UserPropertyHandler userPropertyHandler : userPropertyHandlers) {
@@ -104,7 +104,7 @@ public class ShibbolethRegistrationUserPropertiesFrom extends FormBasicControlle
 			}
 		}
 
-		return allOk & super.validateFormLogic(ureq);
+		return allOk;
 	}
 
 	@Override
diff --git a/src/main/resources/database/mysql/setupDatabase.sql b/src/main/resources/database/mysql/setupDatabase.sql
index 3346dc40417..9dbc39d24ae 100644
--- a/src/main/resources/database/mysql/setupDatabase.sql
+++ b/src/main/resources/database/mysql/setupDatabase.sql
@@ -3576,6 +3576,8 @@ create index idx_user_instid_idx on o_user (u_institutionaluseridentifier);
 create index idx_user_instemail_idx on o_user (u_institutionalemail);
 create index idx_user_creationdate_idx on o_user (creationdate);
 
+alter table o_user add constraint iuni_user_nickname_idx unique (u_nickname);
+
 alter table o_user add constraint user_to_ident_idx foreign key (fk_identity) references o_bs_identity(id);
 alter table o_user add constraint idx_un_user_to_ident_idx UNIQUE (fk_identity);
 
diff --git a/src/main/resources/database/oracle/setupDatabase.sql b/src/main/resources/database/oracle/setupDatabase.sql
index af453776cbf..3b2b5126f78 100644
--- a/src/main/resources/database/oracle/setupDatabase.sql
+++ b/src/main/resources/database/oracle/setupDatabase.sql
@@ -3536,7 +3536,7 @@ create index idx_user_instid_idx on o_user (u_institutionaluseridentifier);
 create index idx_user_instemail_idx on o_user (u_institutionalemail);
 create index idx_user_creationdate_idx on o_user (creationdate);
 
-create index propvalue_idx on o_userproperty (propvalue);
+alter table o_user add constraint iuni_user_nickname_idx unique (u_nickname);
 
 alter table o_user add constraint user_to_ident_idx foreign key (fk_identity) references o_bs_identity(id);
 create index idx_user_to_ident_idx on o_user (fk_identity);
diff --git a/src/main/resources/database/postgresql/alter_15_2_x_to_15_2_6.sql b/src/main/resources/database/postgresql/alter_15_2_x_to_15_2_6.sql
index a9b2e62bc11..c54b86634ad 100644
--- a/src/main/resources/database/postgresql/alter_15_2_x_to_15_2_6.sql
+++ b/src/main/resources/database/postgresql/alter_15_2_x_to_15_2_6.sql
@@ -1,2 +1,6 @@
 -- User
 alter table o_user add column u_linkedin varchar(255);
+
+-- Ensure the constraint exists
+alter table o_user drop constraint if exists iuni_user_nickname_idx;
+alter table o_user add constraint iuni_user_nickname_idx unique (u_nickname);
diff --git a/src/main/resources/database/postgresql/setupDatabase.sql b/src/main/resources/database/postgresql/setupDatabase.sql
index 64742e3dec1..25b232a5bfc 100644
--- a/src/main/resources/database/postgresql/setupDatabase.sql
+++ b/src/main/resources/database/postgresql/setupDatabase.sql
@@ -3424,6 +3424,8 @@ create index xx_idx_institutionalemail_low_text on o_user(lower(u_institutionale
 create index xx_idx_username_low_text on o_bs_identity(lower(name) text_pattern_ops);
 create index xx_idx_nickname_low_text on o_user(lower(u_nickname) text_pattern_ops);
 
+alter table o_user add constraint iuni_user_nickname_idx unique (u_nickname);
+
 create index propvalue_idx on o_userproperty (propvalue);
 
 alter table o_user add constraint user_to_ident_idx foreign key (fk_identity) references o_bs_identity(id);
-- 
GitLab