From: Mathieu Baudier <mbaudier@argeo.org>
Date: Thu, 30 Jun 2011 20:11:15 +0000 (+0000)
Subject: Fix issue with username case in LDAP
X-Git-Tag: argeo-commons-2.1.30~1207
X-Git-Url: https://git.argeo.org/?a=commitdiff_plain;h=118878eb12c8e142da7648cae3880754b34798b2;p=lgpl%2Fargeo-commons.git

Fix issue with username case in LDAP
Improve log in mechanism in RAP

git-svn-id: https://svn.argeo.org/commons/trunk@4643 4cfe0d0a-d680-48aa-b62c-e0a02a3f76cc
---

diff --git a/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/Error.java b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/Error.java
index 96724adff..04971d155 100644
--- a/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/Error.java
+++ b/eclipse/runtime/org.argeo.eclipse.ui/src/main/java/org/argeo/eclipse/ui/Error.java
@@ -26,6 +26,11 @@ public class Error extends TitleAreaDialog {
 	private final Throwable exception;
 
 	public static void show(String message, Throwable e) {
+		// rethrow ThreaDeath in order to make sure that RAP will properly clean
+		// up the UI thread
+		if (e instanceof ThreadDeath)
+			throw (ThreadDeath) e;
+		
 		new Error(getDisplay().getActiveShell(), message, e).open();
 	}
 
@@ -36,7 +41,11 @@ public class Error extends TitleAreaDialog {
 	/** Tries to find a display */
 	private static Display getDisplay() {
 		try {
-			return PlatformUI.getWorkbench().getDisplay();
+			Display display = PlatformUI.getWorkbench().getDisplay();
+			if (display != null)
+				return display;
+			else
+				return Display.getDefault();
 		} catch (Exception e) {
 			return Display.getCurrent();
 		}
diff --git a/security/modules/org.argeo.security.dao.ldap/META-INF/spring/ldap.xml b/security/modules/org.argeo.security.dao.ldap/META-INF/spring/ldap.xml
index 33dc554a8..1f2117d1a 100644
--- a/security/modules/org.argeo.security.dao.ldap/META-INF/spring/ldap.xml
+++ b/security/modules/org.argeo.security.dao.ldap/META-INF/spring/ldap.xml
@@ -33,30 +33,29 @@
 		<property name="userDetailsContextMapper" ref="jcrUserDetailsContextMapper" />
 	</bean>
 
+<!-- 	<bean id="ldapAuthenticator" -->
+<!-- 		class="org.springframework.security.providers.ldap.authenticator.BindAuthenticator"> -->
+<!-- 		<constructor-arg ref="contextSource" /> -->
+<!-- 		<property name="userDnPatterns"> -->
+<!-- 			<list> -->
+<!-- 				<value><![CDATA[${argeo.ldap.usernameAttribute}={0},${argeo.ldap.userBase}]]></value> -->
+<!-- 			</list> -->
+<!-- 		</property> -->
+<!-- 	</bean> -->
+
+	<!-- PasswordComparisonAuthenticator doesn't work with SSHA -->
 	<bean id="ldapAuthenticator"
-		class="org.springframework.security.providers.ldap.authenticator.BindAuthenticator">
+		class="org.springframework.security.providers.ldap.authenticator.PasswordComparisonAuthenticator">
 		<constructor-arg ref="contextSource" />
 		<property name="userDnPatterns">
 			<list>
 				<value><![CDATA[${argeo.ldap.usernameAttribute}={0},${argeo.ldap.userBase}]]></value>
 			</list>
 		</property>
+		<property name="passwordAttributeName" value="${argeo.ldap.passwordAttribute}" />
+		<property name="passwordEncoder" ref="passwordEncoder" />
 	</bean>
 
-	<!-- DOESN'T WORK WITH SSHA -->
-	<!-- <bean id="passwordComparisonAuthenticator" -->
-	<!-- class="org.springframework.security.providers.ldap.authenticator.PasswordComparisonAuthenticator"> -->
-	<!-- <constructor-arg ref="contextSource" /> -->
-	<!-- <property name="userDnPatterns"> -->
-	<!-- <list> -->
-	<!-- <value><![CDATA[${argeo.ldap.usernameAttribute}={0},${argeo.ldap.userBase}]]></value> -->
-	<!-- </list> -->
-	<!-- </property> -->
-	<!-- <property name="passwordAttributeName" value="${argeo.ldap.passwordAttribute}" 
-		/> -->
-	<!-- <property name="passwordEncoder" ref="passwordEncoder" /> -->
-	<!-- </bean> -->
-
 	<!-- USER DETAILS -->
 	<bean id="userAdminDao" class="org.argeo.security.ldap.ArgeoSecurityDaoLdap">
 		<constructor-arg ref="contextSource" />
diff --git a/security/plugins/org.argeo.security.ui.rap/branding/default.htm b/security/plugins/org.argeo.security.ui.rap/branding/default.htm
new file mode 100644
index 000000000..67c89f4fe
--- /dev/null
+++ b/security/plugins/org.argeo.security.ui.rap/branding/default.htm
@@ -0,0 +1,14 @@
+<html>
+<head></head>
+<body>
+<center>
+<table height="100%">
+<tr>
+	<td style="vertical-align:middle">
+		<a style="font-family:sans-serif;color:#0066CC;text-decoration:none;" href="./secureWebUi">Login...</a>
+	</td>
+</tr>
+</table>
+</center>
+</body>
+</html>
\ No newline at end of file
diff --git a/security/plugins/org.argeo.security.ui.rap/branding/favicon.ico b/security/plugins/org.argeo.security.ui.rap/branding/favicon.ico
new file mode 100644
index 000000000..213cdf73f
Binary files /dev/null and b/security/plugins/org.argeo.security.ui.rap/branding/favicon.ico differ
diff --git a/security/plugins/org.argeo.security.ui.rap/build.properties b/security/plugins/org.argeo.security.ui.rap/build.properties
index b840a9456..572b0b491 100644
--- a/security/plugins/org.argeo.security.ui.rap/build.properties
+++ b/security/plugins/org.argeo.security.ui.rap/build.properties
@@ -1,4 +1,5 @@
 bin.includes = plugin.xml,\
-               META-INF/
+               META-INF/,\
+               branding/
 source.. = src/main/java/
 output.. = target/classes/
diff --git a/security/plugins/org.argeo.security.ui.rap/plugin.xml b/security/plugins/org.argeo.security.ui.rap/plugin.xml
index 9d6135fea..7e192647d 100644
--- a/security/plugins/org.argeo.security.ui.rap/plugin.xml
+++ b/security/plugins/org.argeo.security.ui.rap/plugin.xml
@@ -22,4 +22,16 @@
       </application>
    </extension>
 
+	<extension
+         point="org.eclipse.rap.ui.branding">
+    	<branding
+			id="org.argeo.security.ui.rap.branding"
+            servletName="secureWebUi"
+            defaultEntrypointId="org.argeo.security.ui.rap.secureEntryPoint"
+            title="Argeo Secure Web UI"
+            favicon="branding/favicon.ico"
+            body="branding/default.htm">
+      	</branding>
+   	</extension>
+
 </plugin>
diff --git a/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java b/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java
index 4d85cc869..8e377e099 100644
--- a/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java
+++ b/security/plugins/org.argeo.security.ui.rap/src/main/java/org/argeo/security/ui/rap/SecureEntryPoint.java
@@ -7,6 +7,7 @@ import javax.security.auth.login.LoginException;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+import org.argeo.ArgeoException;
 import org.eclipse.equinox.security.auth.ILoginContext;
 import org.eclipse.jface.dialogs.MessageDialog;
 import org.eclipse.rwt.RWT;
@@ -51,37 +52,60 @@ public class SecureEntryPoint implements IEntryPoint {
 		final ILoginContext loginContext = SecureRapActivator
 				.createLoginContext();
 		Subject subject = null;
-		tryLogin: while (subject == null) {
+		tryLogin: while (subject == null && !display.isDisposed()) {
 			try {
 				loginContext.login();
 				subject = loginContext.getSubject();
 			} catch (LoginException e) {
-				if (e.getCause() != null) {
-					Throwable firstCause = e.getCause();
-					// log.error("Cause", firstCause);
-					if (firstCause instanceof LoginException
-							&& firstCause.getCause() != null) {
-						Throwable secondCause = firstCause.getCause();
-						if (secondCause instanceof BadCredentialsException) {
-							MessageDialog.openInformation(
-									display.getActiveShell(),
-									"Bad Credentials",
-									"Your credentials are incorrect");
-							// retry login
-							continue tryLogin;
-						} else if (secondCause instanceof ThreadDeath) {
-							// rethrow thread death caused by dialog UI timeout
-							throw (ThreadDeath) secondCause;
-						}
-
-					} else if (firstCause instanceof ThreadDeath) {
-						throw (ThreadDeath) firstCause;
-					}
+				BadCredentialsException bce = wasCausedByBadCredentials(e);
+				if (bce != null) {
+					MessageDialog.openInformation(display.getActiveShell(),
+							"Bad Credentials", bce.getMessage());
+					// retry login
+					continue tryLogin;
+				}
+
+				// check thread death
+				ThreadDeath td = wasCausedByThreadDeath(e);
+				if (td != null) {
+					display.dispose();
+					throw td;
+				}
+
+				// if (e.getCause() != null) {
+				// Throwable firstCause = e.getCause();
+				// // log.error("Cause", firstCause);
+				// if (firstCause instanceof LoginException
+				// && firstCause.getCause() != null) {
+				// Throwable secondCause = firstCause.getCause();
+				// if (secondCause instanceof BadCredentialsException) {
+				// MessageDialog.openInformation(
+				// display.getActiveShell(),
+				// "Bad Credentials",
+				// "Your credentials are incorrect");
+				// // retry login
+				// continue tryLogin;
+				// } else if (secondCause instanceof ThreadDeath) {
+				// // rethrow thread death caused by dialog UI timeout
+				// throw (ThreadDeath) secondCause;
+				// }
+				//
+				// } else if (firstCause instanceof ThreadDeath) {
+				// throw (ThreadDeath) firstCause;
+				// }
+				// }
+
+				if (!display.isDisposed()) {
+					org.argeo.eclipse.ui.Error.show(
+							"Unexpected exception during authentication", e);
+					// this was not just bad credentials or death thread
+					RWT.getRequest().getSession().setMaxInactiveInterval(1);
+					display.dispose();
+					return -1;
+				} else {
+					throw new ArgeoException(
+							"Unexpected exception during authentication", e);
 				}
-				// this was not just bad credentials returns
-				RWT.getRequest().getSession().setMaxInactiveInterval(1);
-				display.dispose();
-				return -1;
 			}
 		}
 
@@ -99,13 +123,6 @@ public class SecureEntryPoint implements IEntryPoint {
 			public void run() {
 				log.debug("Display disposed");
 				logout(loginContext, username);
-				// invalidate session
-				//RWT.getRequest().getSession().setMaxInactiveInterval(1);
-				try {
-					Thread.sleep(2000);
-				} catch (InterruptedException e1) {
-					// silent
-				}
 			}
 		});
 
@@ -121,6 +138,31 @@ public class SecureEntryPoint implements IEntryPoint {
 		return processReturnCode(returnCode);
 	}
 
+	/** Recursively look for {@link BadCredentialsException} in the root causes. */
+	private BadCredentialsException wasCausedByBadCredentials(Throwable t) {
+		if (t instanceof BadCredentialsException)
+			return (BadCredentialsException) t;
+
+		if (t.getCause() != null)
+			return wasCausedByBadCredentials(t.getCause());
+		else
+			return null;
+	}
+
+	/**
+	 * If there is a {@link ThreadDeath} in the root causes, rethrow it
+	 * (important for RAP cleaning mechanism)
+	 */
+	protected ThreadDeath wasCausedByThreadDeath(Throwable t) {
+		if (t instanceof ThreadDeath)
+			return (ThreadDeath) t;
+
+		if (t.getCause() != null)
+			return wasCausedByThreadDeath(t.getCause());
+		else
+			return null;
+	}
+
 	protected void logout(ILoginContext secureContext, String username) {
 		try {
 			secureContext.logout();
diff --git a/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/dialogs/AbstractLoginDialog.java b/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/dialogs/AbstractLoginDialog.java
index 92ca3d85d..fecb80afc 100644
--- a/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/dialogs/AbstractLoginDialog.java
+++ b/security/plugins/org.argeo.security.ui/src/main/java/org/argeo/security/ui/dialogs/AbstractLoginDialog.java
@@ -124,10 +124,10 @@ public abstract class AbstractLoginDialog extends TrayDialog implements
 						// when the OSGi runtime is shut down
 						try {
 							Thread.sleep(100);
-							if (display.isDisposed()) {
-								log.warn("Display is disposed, killing login dialog thread");
-								throw new ThreadDeath();
-							}
+							// if (display.isDisposed()) {
+							// log.warn("Display is disposed, killing login dialog thread");
+							// throw new ThreadDeath();
+							// }
 						} catch (final Exception e) {
 							// do nothing
 						}
@@ -147,6 +147,7 @@ public abstract class AbstractLoginDialog extends TrayDialog implements
 			}, true, new NullProgressMonitor(), Display.getDefault());
 		} catch (ThreadDeath e) {
 			isCancelled = true;
+			log.debug("Thread " + Thread.currentThread().getId() + " died");
 			throw e;
 		} catch (Exception e) {
 			isCancelled = true;
@@ -157,12 +158,12 @@ public abstract class AbstractLoginDialog extends TrayDialog implements
 		} finally {
 			// so that the modal thread dies
 			processCallbacks = true;
-			try {
-				// wait for the modal context thread to gracefully exit
-				modalContextThread.join(1000);
-			} catch (InterruptedException ie) {
-				// silent
-			}
+			// try {
+			// // wait for the modal context thread to gracefully exit
+			// modalContextThread.join();
+			// } catch (InterruptedException ie) {
+			// // silent
+			// }
 			modalContextThread = null;
 		}
 	}
diff --git a/security/plugins/pom.xml b/security/plugins/pom.xml
index b24f6fc27..775893108 100644
--- a/security/plugins/pom.xml
+++ b/security/plugins/pom.xml
@@ -27,6 +27,7 @@
 				<includes>
 					<include>plugin.xml</include>
 					<include>META-INF/**</include>
+					<include>branding/**</include>
 					<include>jaas/**</include>
 					<include>icons/**</include>
 				</includes>
diff --git a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/core/KeyBasedSystemExecutionService.java b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/core/KeyBasedSystemExecutionService.java
index 07ae04653..b5791c587 100644
--- a/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/core/KeyBasedSystemExecutionService.java
+++ b/security/runtime/org.argeo.security.core/src/main/java/org/argeo/security/core/KeyBasedSystemExecutionService.java
@@ -17,6 +17,8 @@ public class KeyBasedSystemExecutionService extends AbstractSystemExecution
 	public void execute(Runnable runnable) {
 		try {
 			wrapWithSystemAuthentication(Executors.callable(runnable)).call();
+		} catch (RuntimeException e) {
+			throw e;
 		} catch (Exception e) {
 			throw new ArgeoException(
 					"Exception when running system authenticated task", e);
diff --git a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java
index 5c6a88585..537e01763 100644
--- a/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java
+++ b/security/runtime/org.argeo.security.ldap/src/main/java/org/argeo/security/ldap/jcr/JcrUserDetailsContextMapper.java
@@ -22,6 +22,7 @@ import org.argeo.jcr.JcrUtils;
 import org.argeo.security.jcr.JcrUserDetails;
 import org.springframework.ldap.core.DirContextAdapter;
 import org.springframework.ldap.core.DirContextOperations;
+import org.springframework.security.BadCredentialsException;
 import org.springframework.security.GrantedAuthority;
 import org.springframework.security.context.SecurityContextHolder;
 import org.springframework.security.providers.encoding.PasswordEncoder;
@@ -108,13 +109,25 @@ public class JcrUserDetailsContextMapper implements UserDetailsContextMapper,
 
 	/** @return path to the user home node */
 	protected String mapLdapToJcr(String username, DirContextOperations ctx) {
+		String usernameLdap = ctx.getStringAttribute(usernameAttribute);
+		// log.debug("username=" + username + ", usernameLdap=" + usernameLdap);
+		if (!username.equals(usernameLdap)) {
+			String msg = "Provided username '" + username
+					+ "' is different from username stored in LDAP '"
+					+ usernameLdap+"'";
+			// we log it because the exception may not be displayed
+			log.error(msg);
+			throw new BadCredentialsException(msg);
+		}
+
 		try {
+
 			Node userHome = JcrUtils.getUserHome(session, username);
 			if (userHome == null)
 				userHome = JcrUtils.createUserHome(session, homeBasePath,
 						username);
 			String userHomePath = userHome.getPath();
-			Node userProfile;  // = userHome.getNode(ARGEO_PROFILE);
+			Node userProfile; // = userHome.getNode(ARGEO_PROFILE);
 			if (userHome.hasNode(ARGEO_PROFILE)) {
 				userProfile = userHome.getNode(ARGEO_PROFILE);
 			} else {