[client] more error handling changes for BWIDM Auth

4 files changed, 110 insertions, 84 deletions

diff --git a/dozentenmodul/src/main/java/auth/BWIDMAuthenticator.java b/dozentenmodul/src/main/java/auth/BWIDMAuthenticator.java
index e4da3cc3..40c1d2eb 100644
--- a/dozentenmodul/src/main/java/auth/BWIDMAuthenticator.java
+++ b/dozentenmodul/src/main/java/auth/BWIDMAuthenticator.java
@@ -1,5 +1,11 @@
 package auth;
 
+import java.io.IOException;
+import java.net.MalformedURLException;
+import java.net.URISyntaxException;
+
+import org.apache.http.ParseException;
+import org.apache.http.client.ClientProtocolException;
 import org.apache.log4j.Logger;
 import org.apache.thrift.TException;
 import org.openslx.bwlp.thrift.iface.TAuthenticationException;
@@ -7,6 +13,8 @@ import org.openslx.bwlp.thrift.iface.TInvalidTokenException;
 import org.openslx.bwlp.thrift.iface.UserInfo;
 import org.openslx.thrifthelper.ThriftManager;
 
+import com.google.gson.JsonSyntaxException;
+
 import util.ShibbolethECP;
 import util.ShibbolethECP.ReturnCode;
 
@@ -36,10 +44,30 @@ public class BWIDMAuthenticator implements BaseAuthenticator {
 	@Override
 	public void login(String username, String password,
 			AuthenticatorCallback callback) throws TAuthenticationException {
-		// sanity check on the ecpUrl, should have been set
-		
-		ReturnCode ret = ShibbolethECP.doLogin(this.ecpUrl, username, password);
+		// try to login
+		ReturnCode ret = null;
+		try {
+			ret = ShibbolethECP.doLogin(this.ecpUrl, username, password);
+		} catch (JsonSyntaxException e) {
+			LOGGER.error("Could not parse JSON response from the service provider: ", e);
+		} catch (ClientProtocolException e) {
+			LOGGER.error("HTTP client protocol error: ", e);
+		} catch (ParseException e) {
+			LOGGER.error("Error parsing the raw response body from the service provider: ", e);
+		} catch (MalformedURLException e) {
+			LOGGER.error("Bad syntax of the registration URL returned by the service provider: ", e);
+		} catch (URISyntaxException e) {
+			LOGGER.error("Bad syntax of the URL to the identity provider: ", e);
+		} catch (IOException e) {
+			LOGGER.error("IO Error while communicating with the service provider: ", e);
+		}
+		// if ret is still null, some exception happened, so abort.
+		if (ret == null) {
+			LOGGER.error("Error during the ECP authentication process.");
+			return;
+		}
 
+		// else, we do have a valid ReturnCode
 		if (ret == ReturnCode.NO_ERROR) {
 			UserInfo userInfo;
 			try {
@@ -51,7 +79,11 @@ public class BWIDMAuthenticator implements BaseAuthenticator {
 				LOGGER.error("Thrift transport error, see trace: ", e);
 				return;
 			}
-			callback.postLogin(userInfo);
+			callback.postLogin(ReturnCode.NO_ERROR, userInfo);
+		} else {
+			// else just return the ReturnCode to the GUI
+			// it should then show a corresponding error message!
+			callback.postLogin(ret, null);
 		}
 	}
 }
diff --git a/dozentenmodul/src/main/java/auth/BaseAuthenticator.java b/dozentenmodul/src/main/java/auth/BaseAuthenticator.java
index d52c5343..f22577d8 100644
--- a/dozentenmodul/src/main/java/auth/BaseAuthenticator.java
+++ b/dozentenmodul/src/main/java/auth/BaseAuthenticator.java
@@ -3,6 +3,9 @@ package auth;
 import org.openslx.bwlp.thrift.iface.TAuthenticationException;
 import org.openslx.bwlp.thrift.iface.UserInfo;
 
+import util.ShibbolethECP.ReturnCode;
+import edu.kit.scc.dei.ecplean.ECPAuthenticationException;
+
 /**
  * @author Jonathan Bauer
  *
@@ -16,7 +19,7 @@ public interface BaseAuthenticator {
 	 * corresponding message to the user.
 	 */
 	interface AuthenticatorCallback {
-		void postLogin(UserInfo user);
+		void postLogin(ReturnCode returnCode, UserInfo user);
 	}
 	/**
 	 * Definition of the generic login method. 
diff --git a/dozentenmodul/src/main/java/gui/core/LoginGUI.java b/dozentenmodul/src/main/java/gui/core/LoginGUI.java
index 6ca06518..00fb5310 100644
--- a/dozentenmodul/src/main/java/gui/core/LoginGUI.java
+++ b/dozentenmodul/src/main/java/gui/core/LoginGUI.java
@@ -5,15 +5,18 @@ import java.util.Comparator;
 import java.util.List;
 
 import org.apache.log4j.Logger;
-import org.apache.thrift.TException;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.events.KeyEvent;
+import org.eclipse.swt.events.KeyListener;
 import org.eclipse.swt.events.SelectionAdapter;
 import org.eclipse.swt.events.SelectionEvent;
 import org.eclipse.swt.widgets.Shell;
+import org.openslx.bwlp.dozmod.thrift.OrganizationList;
 import org.openslx.bwlp.thrift.iface.Organization;
 import org.openslx.bwlp.thrift.iface.TAuthenticationException;
 import org.openslx.bwlp.thrift.iface.UserInfo;
-import org.openslx.thrifthelper.ThriftManager;
 
+import util.ShibbolethECP.ReturnCode;
 import auth.BWIDMAuthenticator;
 import auth.BaseAuthenticator.AuthenticatorCallback;
 import edu.kit.scc.dei.ecplean.ECPAuthenticationException;
@@ -38,16 +41,12 @@ public class LoginGUI extends LoginComposite {
 		super(mainShell);
 
 		// entries in the combo
-		List<Organization> orgs = null;
-		try {
-			orgs = ThriftManager.getMasterClient().getOrganizations();
-		} catch (TException e) {
-			LOGGER.error(
-					"Could not fetch the IdP list from the masterserver! See trace:", e);
-			// in this case, we can just call the default fillIdPCombo method of the
-			// superclass
+		List<Organization> orgs = OrganizationList.getAll();
+		if (orgs == null) {
+			LOGGER.error("No organizations received from the cache.");
 			idpCombo.add("No entries");
 		}
+		
 		// all fine, lets sort it
 		Collections.sort(orgs, new Comparator<Organization>() {
 			public int compare(Organization o1, Organization o2) {
@@ -55,6 +54,7 @@ public class LoginGUI extends LoginComposite {
 			}
 		});
 		for (Organization o : orgs) {
+			if (o.getEcpUrl() == null | o.getEcpUrl().isEmpty()) continue;
 			idpCombo.add(o.displayName);
 			idpCombo.setData(o.displayName, o);
 		}
@@ -64,12 +64,7 @@ public class LoginGUI extends LoginComposite {
 		loginButton.addSelectionListener(new SelectionAdapter() {
 			@Override
 			public void widgetSelected(SelectionEvent e) {
-				try {
 					doLogin(loginType);
-				} catch (ECPAuthenticationException ae) {
-					LOGGER.error("Authentication error, see trace: ", ae);
-					GuiManager.showMessage(ae.getMessage());
-				}
 			}
 		});
 
@@ -111,6 +106,19 @@ public class LoginGUI extends LoginComposite {
 				loginType = LOGIN_TYPE.SAT;
 			}
 		});
+		passwordText.addKeyListener(new KeyListener() {
+			
+			@Override
+			public void keyReleased(KeyEvent e) {
+				// TODO Auto-generated method stub
+				if (e.keyCode == SWT.CR) {
+					doLogin(loginType);
+				}
+			}
+			
+			@Override
+			public void keyPressed(KeyEvent e) {}
+		});
 	}
 
 	/**
@@ -118,7 +126,7 @@ public class LoginGUI extends LoginComposite {
 	 * authentication mechanism corresponding to the selected authButton
 	 * @throws ECPAuthenticationException 
 	 */
-	private void doLogin(LOGIN_TYPE loginType) throws ECPAuthenticationException {
+	private void doLogin(LOGIN_TYPE loginType) {
 		// here we only check for the fields
 		username = usernameText.getText();
 		password = passwordText.getText();
@@ -153,15 +161,24 @@ public class LoginGUI extends LoginComposite {
 			try {
 				bwidmAuth.login(username, password, new AuthenticatorCallback() {
 					@Override
-					public void postLogin(UserInfo user) {
-						LOGGER.info(user.firstName + " " + user.lastName);
-						if (user != null)
+					public void postLogin(ReturnCode returnCode, UserInfo user) {
+						// handle errors first
+						if (returnCode != ReturnCode.NO_ERROR && user == null) {
+							switch(returnCode) {
+							case IDP_ERROR:
+								GuiManager.showMessage("");
+								break;
+							default:
+								GuiManager.showMessage("Internal error!");
+								break;
+							}
+						} else
 							GuiManager.addContent(new DisclaimerComposite(getShell()));
 					}
 				});
 			} catch (TAuthenticationException e) {
-				LOGGER.error("Authentication error, see trace: ", e);
-				GuiManager.showMessage(BAD_AUTH);
+				//LOGGER.error("Authentication error, see trace: ", e);
+				GuiManager.showMessage(e.getMessage());
 				return;
 			}
 			break;
diff --git a/dozentenmodul/src/main/java/util/ShibbolethECP.java b/dozentenmodul/src/main/java/util/ShibbolethECP.java
index 67936906..7844a51e 100644
--- a/dozentenmodul/src/main/java/util/ShibbolethECP.java
+++ b/dozentenmodul/src/main/java/util/ShibbolethECP.java
@@ -35,23 +35,23 @@ public class ShibbolethECP {
 	private static final Gson GSON = new GsonBuilder().create();
 	
 	/**
-	 * 
+	 * ServiceProviderResponse Object representing the last response we received
 	 */
-	private static ServiceProviderResponse _lastResponse = null;
+	private static ServiceProviderResponse lastResponse = null;
 	/**
 	 * URL for bwLehrpool registration  
 	 */
-	private static URL _registrationUrl = null;
+	private static URL registrationUrl = null;
 	/**
 	 * Return codes
 	 */
 	public static enum ReturnCode {
 		NO_ERROR(0, "Authentication against the identity provider and request of the service provider resource worked."),
-	  ERROR_IDP(1, "Authentication against the identity provider failed."),
-	  ERROR_UNREG(2, "User not registered to use bwLehrpool"),
+	  IDP_ERROR(1, "Authentication against the identity provider failed."),
+	  UNREG_ERROR(2, "User not registered to use bwLehrpool."),
 	  ERROR_SP(3, "Invalid resource of the service provider."),
 	  ERROR_URL(4, "Invalid URL received from master server."),
-		ERROR_OTHER(5, "Internal class error.");
+		ERROR_OTHER(5, "Internal error.");
 
 	  private final int id;
 	  private final String msg;
@@ -87,7 +87,7 @@ public class ShibbolethECP {
 	}
 
 	public static ServiceProviderResponse getResponse() {
-		return _lastResponse;
+		return lastResponse;
 	}
 	/**
 	 * Fetches the resource
@@ -102,7 +102,9 @@ public class ShibbolethECP {
 	 * 						true if login worked, false otherwise.
 	 * @throws TAuthenticationException
 	 */
-	public static ReturnCode doLogin(final String idpUrl, final String user, final String pass) throws TAuthenticationException {
+	public static ReturnCode doLogin(final String idpUrl, final String user, final String pass)
+			throws TAuthenticationException, URISyntaxException, ClientProtocolException, IOException,
+			ParseException, JsonSyntaxException, MalformedURLException {
 
 		// first lets do some sanity checks
 		if (BWLP_SP == null) {
@@ -123,76 +125,48 @@ public class ShibbolethECP {
 		}
 
 		// now init the authenticator for that idp and our static sp
-    ECPAuthenticator auth = null;
-		try {
-			auth = new ECPAuthenticator(user, pass, new URI(idpUrl), BWLP_SP);
-		} catch (URISyntaxException e) {
-			LOGGER.error("Bad URI syntax, see trace: ", e);
-			return ReturnCode.ERROR_OTHER;
-		}
+		final ECPAuthenticator auth = new ECPAuthenticator(user, pass, new URI(idpUrl), BWLP_SP);
+		
     try {
 			auth.authenticate();
     } catch (ECPAuthenticationException e) {
 			LOGGER.error("ECP Authentication Exception, see trace: ", e);
-			throw new TAuthenticationException(AuthenticationError.GENERIC_ERROR, "ECP client failed to authenticate.");
+			throw new TAuthenticationException(AuthenticationError.GENERIC_ERROR, e.getMessage());
     }
-    // here test again for the SPURL
-    HttpGet testSp = new HttpGet(BWLP_SP);
-    HttpResponse response = null;
-    try {
-			response = auth.getHttpClient().execute(testSp);
-		} catch (ClientProtocolException e) {
-			LOGGER.error("Bad protocol, see trace: ", e);
-			return ReturnCode.ERROR_OTHER;
-		} catch (IOException e) {
-			LOGGER.error("I/O error, see trace: ", e);
-			return ReturnCode.ERROR_OTHER;
-		}
+    // here test again for the SP's URL
+    final HttpGet testSp = new HttpGet(BWLP_SP);
+    final HttpResponse response = auth.getHttpClient().execute(testSp);
+
     LOGGER.debug("SP request returned: " + response.getStatusLine());
-    String responseBody = null;
-    try {
-			responseBody = EntityUtils.toString(response.getEntity());
-		} catch (ParseException e) {
-			LOGGER.error("Parsing error, see trace: ", e);
-			return ReturnCode.ERROR_OTHER;
-		} catch (IOException e) {
-			LOGGER.error("I/O error, see trace: ", e);
-			return ReturnCode.ERROR_OTHER;
-		}
-    _lastResponse = null;
-    try {
-    	_lastResponse = GSON.fromJson(responseBody, ServiceProviderResponse.class);
-    } catch (JsonSyntaxException e) {
-    	LOGGER.error("Bad JSON syntax, see trace: ", e);
-    	return ReturnCode.ERROR_SP;
-    }
+    final String responseBody = EntityUtils.toString(response.getEntity());
+    
+    lastResponse = GSON.fromJson(responseBody, ServiceProviderResponse.class);
+
     // TODO: here we will need to parse the answer accordingly.
     // no errors, meaning everything worked fine.
-    if (_lastResponse.status.equals("unregistered")) {
-    	try {
-				_registrationUrl = new URL(_lastResponse.url);
-			} catch (MalformedURLException e) {
-				LOGGER.error("URL returned by masterserver is malformed, see trace: " + e);
-				return ReturnCode.ERROR_URL;
-			}
-    	return ReturnCode.ERROR_UNREG;
+    if (lastResponse.status.equals("unregistered")) {
+			registrationUrl = new URL(lastResponse.url);
+			return ReturnCode.UNREG_ERROR;
     }
     // TODO the rest of the cases...
-    if (_lastResponse.status.equals("error")) {
-    	LOGGER.error("Server side error: " + _lastResponse.error);
+    if (lastResponse.status.equals("error")) {
+    	LOGGER.error("Server side error: " + lastResponse.error);
     	return ReturnCode.ERROR_OTHER;
     }
-    if (_lastResponse.status.equals("anonymous")) {
+    if (lastResponse.status.equals("anonymous")) {
     	LOGGER.error("IdP did not forward user account information to SP. Contact developper.");
     	return ReturnCode.ERROR_OTHER;
     }
-    if (_lastResponse.status.equals("ok")) {
+    if (lastResponse.status.equals("ok")) {
     	return ReturnCode.NO_ERROR;
     }
     // still here? then something else went wrong
     return ReturnCode.ERROR_OTHER;
 	}
+	/**
+	 * @return Registration URL given by the SP.
+	 */
 	public static URL getRegistrationUrl() {
-		return _registrationUrl;
+		return registrationUrl;
 	}
 }