[LdapUser] Added doc-comments, some TODOs, restructured connection error checking

2 files changed, 35 insertions, 23 deletions

diff --git a/src/main/java/org/openslx/imagemaster/db/LDAPUser.java b/src/main/java/org/openslx/imagemaster/db/LdapUser.java
index 2d17b08..17bf65e 100644
--- a/src/main/java/org/openslx/imagemaster/db/LDAPUser.java
+++ b/src/main/java/org/openslx/imagemaster/db/LdapUser.java
@@ -13,12 +13,17 @@ import org.apache.directory.api.ldap.model.exception.LdapException;
 import org.apache.directory.api.ldap.model.message.SearchScope;
 import org.apache.directory.ldap.client.api.LdapConnectionConfig;
 import org.apache.directory.ldap.client.api.LdapNetworkConnection;
+import org.apache.log4j.Logger;
 import org.openslx.imagemaster.session.User;
+import org.openslx.imagemaster.thrift.iface.AuthenticationError;
 import org.openslx.imagemaster.thrift.iface.AuthenticationException;
 import org.openslx.imagemaster.util.Sha512Crypt;
 
-/*
+/**
  * This TrustManager is used to accept custom certificates.
+ * TODO: Once we are talking to the real server(s), we should
+ * actually verify the cert, or we could just stop using ssl
+ * altogether.
  */
 class MyTrustManager implements X509TrustManager {
 
@@ -37,9 +42,16 @@ class MyTrustManager implements X509TrustManager {
 	
 }
 
-public class LDAPUser extends User {
-
-	protected LDAPUser(int userId, String username, String password, String organization,
+/**
+ * Represents a user instance that was queries (primarily) from LDAP.
+ * Additional information that is not provided by the LDAP server might
+ * be fetched from other sources, like the local database.
+ */
+public class LdapUser extends User
+{
+	private static final Logger log = Logger.getLogger( LdapUser.class );
+	
+	protected LdapUser(int userId, String username, String password, String organization,
 			String firstName, String lastName, String eMail,
 			String satelliteAddress) {
 		super(userId, username, password, organization, firstName, lastName, eMail,
@@ -48,13 +60,14 @@ public class LDAPUser extends User {
 	
 	/**
 	 * Query LDAP for user with given login
-	 * @param login (user@organization)
+	 * @param login  Login of user in the form "user@organization.com"
 	 * @return instance of LDAPUser for matching entry from LDAP, or null if not found
 	 */
 	@SuppressWarnings("finally")
-	public static LDAPUser forLogin( final String login, final String password ) throws AuthenticationException {
+	public static LdapUser forLogin( final String login, final String password ) throws AuthenticationException {
 		String username, organization, firstName, lastName, eMail, satelliteAddress; 
 		
+		// TODO: Read connection info from config file
 		LdapConnectionConfig ldapConfig = new LdapConnectionConfig();
 		ldapConfig.setTrustManagers(new MyTrustManager());
 		ldapConfig.setLdapPort(636);
@@ -64,31 +77,33 @@ public class LDAPUser extends User {
 		LdapNetworkConnection connection = new LdapNetworkConnection( ldapConfig );
 		
 		// bind connection
+		// TODO: Hard coded stuff here too. binddn, search query etc. need to be configurable
 		try {
-			connection.bind("uid=" + login + ",ou=people,dc=uni-freiburg,dc=de", password);
+			if ( connection.connect() )
+				connection.bind("uid=" + login + ",ou=people,dc=uni-freiburg,dc=de", password);
 		} catch (LdapException e1) {
+			log.warn( "Connection to LDAP failed: " + e1.getMessage() );
+		}
+		
+		if ( !connection.isConnected() ) {
 			try {
 				connection.unBind();
 				connection.close();
 			} catch (LdapException | IOException e) {
-			} finally {
-				AuthenticationException ae = new AuthenticationException();
-				ae.message = "Could not bind to LDAP server. Invalid credentials.";
-				throw ae;
+				// Not doing anything here, as ldap already failed...
 			}
+			throw new AuthenticationException( AuthenticationError.GENERIC_ERROR, "Could not connect to LDAP server." );
 		}
 		
 		// test authorization
-		if (!connection.isConnected() || !connection.isAuthenticated()) {
+		if ( !connection.isAuthenticated() ) {
 			try {
 				connection.unBind();
 				connection.close();
 			} catch (LdapException | IOException e) {
-			} finally {
-				AuthenticationException ae = new AuthenticationException();
-				ae.message = "Could not connect / authenticate to LDAP server. Invalid credentials?";
-				throw ae;
+				// Failing disconnect... Can't do much about it, just go on
 			}
+			throw new AuthenticationException( AuthenticationError.INVALID_CREDENTIALS, "Could not authenticate to LDAP server. Invalid credentials?" );
 		}
 		
 		// make search query
@@ -108,10 +123,7 @@ public class LDAPUser extends User {
 			if (dbSatellite != null) {
 				satelliteAddress = dbSatellite.getAddress();
 			} else {
-				/*
-				 * Organization is not known..
-				 * TODO: Handle this
-				 */
+				// TODO: Organization is not known.. Handle this
 				satelliteAddress = "addressNotKown";
 			}
 		} catch (LdapException | CursorException e1) {
@@ -129,6 +141,6 @@ public class LDAPUser extends User {
 				return null;
 			}
 		}
-		return new LDAPUser(0, username, Sha512Crypt.Sha512_crypt(password, null, 0), organization, firstName, lastName, eMail, satelliteAddress);
+		return new LdapUser(0, username, Sha512Crypt.Sha512_crypt(password, null, 0), organization, firstName, lastName, eMail, satelliteAddress);
 	}
 }
diff --git a/src/main/java/org/openslx/imagemaster/session/Authenticator.java b/src/main/java/org/openslx/imagemaster/session/Authenticator.java
index d1cca94..3f86fad 100644
--- a/src/main/java/org/openslx/imagemaster/session/Authenticator.java
+++ b/src/main/java/org/openslx/imagemaster/session/Authenticator.java
@@ -2,7 +2,7 @@ package org.openslx.imagemaster.session;
 
 import org.apache.log4j.Logger;
 import org.openslx.imagemaster.db.DbUser;
-import org.openslx.imagemaster.db.LDAPUser;
+import org.openslx.imagemaster.db.LdapUser;
 import org.openslx.imagemaster.thrift.iface.AuthenticationError;
 import org.openslx.imagemaster.thrift.iface.AuthenticationException;
 
@@ -27,7 +27,7 @@ public class Authenticator
 //		}
 //		log.debug( "Login successful: " + username );
 		
-		LDAPUser user = LDAPUser.forLogin( username, password ); // throws exception if credentials are invalid
+		LdapUser user = LdapUser.forLogin( username, password ); // throws exception if credentials are invalid
 		if (user == null) {
 			log.debug( "Login failed: " + username);
 			throw new AuthenticationException( AuthenticationError.INVALID_CREDENTIALS, "Invalid username or password!" );