From b2d620ffa787b95074d7cf479b6610b7327dd388 Mon Sep 17 00:00:00 2001
From: Thomas Lenz <thomas.lenz@egiz.gv.at>
Date: Thu, 4 Jul 2019 12:51:23 +0200
Subject: fix some small bugs in SL2.0 module

---
 .../impl/idp/auth/modules/ModuleRegistration.java  | 32 +++++++-------
 .../eaaf/core/impl/utils/HttpClientFactory.java    | 51 ++++++++++++++++++----
 .../eaaf/core/impl/utils/IHttpClientFactory.java   | 21 +++++++++
 .../sl20/AbstractSL20AuthenticationModulImpl.java  | 47 +++++++++++++++-----
 .../tasks/AbstractCreateQualeIDRequestTask.java    | 12 +++--
 .../auth/sl20/utils/SL20HttpBindingUtils.java      |  4 +-
 .../auth/sl20/utils/SL20JSONExtractorUtils.java    |  7 ++-
 7 files changed, 133 insertions(+), 41 deletions(-)
 create mode 100644 eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/IHttpClientFactory.java

diff --git a/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/idp/auth/modules/ModuleRegistration.java b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/idp/auth/modules/ModuleRegistration.java
index 356744e8..f35b6032 100644
--- a/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/idp/auth/modules/ModuleRegistration.java
+++ b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/idp/auth/modules/ModuleRegistration.java
@@ -59,7 +59,7 @@ public class ModuleRegistration {
 
 	private static ModuleRegistration instance = new ModuleRegistration();
 
-	private List<AuthModule> priorizedModules = new ArrayList<>();
+	private final List<AuthModule> priorizedModules = new ArrayList<>();
 
 	@Autowired
 	private ApplicationContext ctx;
@@ -67,7 +67,7 @@ public class ModuleRegistration {
 	@Autowired
 	private ProcessEngine processEngine;
 
-	private Logger log = LoggerFactory.getLogger(getClass());
+	private final Logger log = LoggerFactory.getLogger(getClass());
 
 	public static ModuleRegistration getInstance() {
 		return instance;
@@ -86,6 +86,8 @@ public class ModuleRegistration {
 
 		// order modules according to their priority
 		sortModules();
+		
+		instance = this;
 	}
 	
 	/**
@@ -93,10 +95,10 @@ public class ModuleRegistration {
 	 */
 	private void initServiceLoaderModules() {
 		log.info("Looking for auth modules.");
-		ServiceLoader<AuthModule> loader = ServiceLoader.load(AuthModule.class);
-		Iterator<AuthModule> modules = loader.iterator();
+		final ServiceLoader<AuthModule> loader = ServiceLoader.load(AuthModule.class);
+		final Iterator<AuthModule> modules = loader.iterator();
 		while (modules.hasNext()) {
-			AuthModule module = modules.next();
+			final AuthModule module = modules.next();
 			log.info("Detected module {}", module.getClass().getName());
 			registerModuleProcessDefinitions(module);
 			priorizedModules.add(module);
@@ -108,8 +110,8 @@ public class ModuleRegistration {
 	 */
 	private void initSpringModules() {
 		log.debug("Discovering Spring modules.");
-		Map<String, AuthModule> modules = ctx.getBeansOfType(AuthModule.class);
-		for (AuthModule module : modules.values()) {
+		final Map<String, AuthModule> modules = ctx.getBeansOfType(AuthModule.class);
+		for (final AuthModule module : modules.values()) {
 			registerModuleProcessDefinitions(module);
 			priorizedModules.add(module);
 		}
@@ -122,15 +124,15 @@ public class ModuleRegistration {
 	 *            the module.
 	 */
 	private void registerModuleProcessDefinitions(AuthModule module) {
-		for (String uri : module.getProcessDefinitions()) {
-			Resource resource = ctx.getResource(uri);
+		for (final String uri : module.getProcessDefinitions()) {
+			final Resource resource = ctx.getResource(uri);
 			if (resource.isReadable()) {
 				log.info("Registering process definition '{}'.", uri);
 				try (InputStream processDefinitionInputStream = resource.getInputStream()) {
 					processEngine.registerProcessDefinition(processDefinitionInputStream);
-				} catch (IOException e) {
+				} catch (final IOException e) {
 					log.error("Process definition '{}' could NOT be read.", uri, e);
-				} catch (ProcessDefinitionParserException e) {
+				} catch (final ProcessDefinitionParserException e) {
 					log.error("Error while parsing process definition '{}'", uri, e);
 				}
 			} else {
@@ -146,8 +148,8 @@ public class ModuleRegistration {
 		Collections.sort(priorizedModules, new Comparator<AuthModule>() {
 			@Override
 			public int compare(AuthModule thisAuthModule, AuthModule otherAuthModule) {
-				int thisOrder = thisAuthModule.getPriority();
-				int otherOrder = otherAuthModule.getPriority();
+				final int thisOrder = thisAuthModule.getPriority();
+				final int otherOrder = otherAuthModule.getPriority();
 				return (thisOrder < otherOrder ? 1 : (thisOrder == otherOrder ? 0 : -1));
 			}
 		});
@@ -162,8 +164,8 @@ public class ModuleRegistration {
 	 * @return the process id or {@code null}
 	 */
 	public String selectProcess(ExecutionContext context) {
-		for (AuthModule module : priorizedModules) {
-			String id = module.selectProcess(context);
+		for (final AuthModule module : priorizedModules) {
+			final String id = module.selectProcess(context);
 			if (StringUtils.isNotEmpty(id)) {
 				log.debug("Process with id '{}' selected, for context '{}'.", id, context);
 				return id;
diff --git a/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/HttpClientFactory.java b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/HttpClientFactory.java
index 926b2bd5..ee12b9e4 100644
--- a/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/HttpClientFactory.java
+++ b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/HttpClientFactory.java
@@ -6,14 +6,21 @@ import javax.annotation.PostConstruct;
 import javax.net.ssl.HostnameVerifier;
 import javax.net.ssl.SSLContext;
 
+import org.apache.http.HttpRequest;
+import org.apache.http.HttpResponse;
+import org.apache.http.ProtocolException;
+import org.apache.http.client.RedirectStrategy;
 import org.apache.http.client.config.RequestConfig;
+import org.apache.http.client.methods.HttpUriRequest;
 import org.apache.http.conn.socket.LayeredConnectionSocketFactory;
 import org.apache.http.conn.ssl.NoopHostnameVerifier;
 import org.apache.http.conn.ssl.SSLConnectionSocketFactory;
 import org.apache.http.impl.client.CloseableHttpClient;
+import org.apache.http.impl.client.DefaultRedirectStrategy;
 import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.impl.client.HttpClients;
 import org.apache.http.impl.conn.PoolingHttpClientConnectionManager;
+import org.apache.http.protocol.HttpContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -22,7 +29,7 @@ import org.springframework.stereotype.Service;
 import at.gv.egiz.eaaf.core.api.idp.IConfiguration;
 
 @Service
-public class HttpClientFactory {
+public class HttpClientFactory implements IHttpClientFactory {
 	private static final Logger log = LoggerFactory.getLogger(HttpClientFactory.class);	
 	@Autowired(required=true) private IConfiguration basicConfig;
 	
@@ -35,24 +42,47 @@ public class HttpClientFactory {
 	public static final String PROP_CONFIG_CLIENT_HTTP_SSL_HOSTNAMEVERIFIER_TRUSTALL = "client.http.ssl.hostnameverifier.trustall";
 	
 	// default configuration values
-	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_SOCKET = "300";
-	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_CONNECTION = "300";
-	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_REQUEST = "1500";
+	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_SOCKET = "15";
+	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_CONNECTION = "15";
+	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_TIMEOUT_REQUEST = "30";
 	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_POOL_MAXTOTAL = "500";
 	public static final String DEFAULT_CONFIG_CLIENT_HTTP_CONNECTION_POOL_MAXPERROUTE = "100";
 	
 	private HttpClientBuilder httpClientBuilder = null;
 	
-	/**
-	 * Return an instance of a Apache HTTP client
-	 * 
-	 * @return
+	/* (non-Javadoc)
+	 * @see at.gv.egiz.eaaf.core.impl.utils.IHttpClientFactory#getHttpClient()
 	 */
+	@Override
 	public CloseableHttpClient getHttpClient() {
-		return httpClientBuilder.build();
+		return getHttpClient(true);
 		
 	}
 	
+	@Override
+	public CloseableHttpClient getHttpClient(boolean followRedirects) {
+		RedirectStrategy redirectStrategy = new DefaultRedirectStrategy(); 
+		if (!followRedirects)
+			redirectStrategy = new RedirectStrategy() {
+				
+				@Override
+				public boolean isRedirected(HttpRequest request, HttpResponse response, HttpContext context)
+						throws ProtocolException {
+					return false;
+				}
+				
+				@Override
+				public HttpUriRequest getRedirect(HttpRequest request, HttpResponse response, HttpContext context)
+						throws ProtocolException {
+					return null;
+				}
+			};
+		 
+		return httpClientBuilder
+				.setRedirectStrategy(redirectStrategy)
+				.build();			
+									
+	}
 	
 	@PostConstruct
 	private void initalize() {
@@ -116,6 +146,9 @@ public class HttpClientFactory {
 		log.info("HTTP client-builder successfuly initialized");
 		
 	}
+
+
+
 	
 	
 }
diff --git a/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/IHttpClientFactory.java b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/IHttpClientFactory.java
new file mode 100644
index 00000000..1975fb52
--- /dev/null
+++ b/eaaf_core/src/main/java/at/gv/egiz/eaaf/core/impl/utils/IHttpClientFactory.java
@@ -0,0 +1,21 @@
+package at.gv.egiz.eaaf.core.impl.utils;
+
+import org.apache.http.impl.client.CloseableHttpClient;
+
+public interface IHttpClientFactory {
+
+	/**
+	 * Return an instance of a Apache HTTP client that follows http redirects automatically
+	 * 
+	 * @return
+	 */
+	CloseableHttpClient getHttpClient();
+
+	/**
+	 * Return an instance of a Apache HTTP client
+	 * @param followRedirects 
+	 * @return
+	 */
+	CloseableHttpClient getHttpClient(boolean followRedirects);
+	
+}
\ No newline at end of file
diff --git a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/AbstractSL20AuthenticationModulImpl.java b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/AbstractSL20AuthenticationModulImpl.java
index a5bbf03f..64739dd8 100644
--- a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/AbstractSL20AuthenticationModulImpl.java
+++ b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/AbstractSL20AuthenticationModulImpl.java
@@ -61,31 +61,58 @@ public abstract class AbstractSL20AuthenticationModulImpl implements AuthModule
 	public String selectProcess(ExecutionContext context) {
 		final ISPConfiguration spConfig = (ISPConfiguration) context.get(EAAFConstants.PROCESSCONTEXT_SP_CONFIG);
 		
+		if (spConfig == null) {
+			log.error("Suspect state. NO SP CONFIGURATION IN CONTEXT!");
+			throw new RuntimeException("Suspect state. NO SP CONFIGURATION IN CONTEXT!");
+			
+		}
+		
 		final String sl20ClientTypeHeader = (String) context.get(SL20Constants.HTTP_HEADER_SL20_CLIENT_TYPE.toLowerCase());
 		final String sl20VDATypeHeader = (String)  context.get(SL20Constants.HTTP_HEADER_SL20_VDA_TYPE.toLowerCase());
 		
-		if (spConfig != null && 
-				StringUtils.isNotEmpty(spConfig.getConfigurationValue(getConfigPropertyNameEnableModule())) &&
-					Boolean.valueOf(spConfig.getConfigurationValue(getConfigPropertyNameEnableModule()))) {
-			log.debug("SL2.0 is enabled for " + spConfig.getUniqueIdentifier());
-			log.trace(SL20Constants.HTTP_HEADER_SL20_CLIENT_TYPE +  ": " + sl20ClientTypeHeader);			
-			log.trace(SL20Constants.HTTP_HEADER_SL20_VDA_TYPE +  ": " + sl20VDATypeHeader);
-			return getProcessName();
+		if (authConfig.getBasicConfigurationBoolean(getGeneralConfigPropertyNameEnableModule(), getGeneralConfigPropertyNameEnableModuleDefault())) {
+			if (spConfig != null && 
+					StringUtils.isNotEmpty(spConfig.getConfigurationValue(getSPConfigPropertyNameEnableModule())) &&
+						Boolean.valueOf(spConfig.getConfigurationValue(getSPConfigPropertyNameEnableModule()))) {
+				log.debug("SL2.0 is enabled for " + spConfig.getUniqueIdentifier());
+				log.trace(SL20Constants.HTTP_HEADER_SL20_CLIENT_TYPE +  ": " + sl20ClientTypeHeader);			
+				log.trace(SL20Constants.HTTP_HEADER_SL20_VDA_TYPE +  ": " + sl20VDATypeHeader);
+				return getProcessName();
+			
+			} else {
+				log.trace("SL2.0 is NOT enabled for " + spConfig.getUniqueIdentifier());
+				return null;
+			
+			}
 			
 		} else {
-			log.trace("SL2.0 is NOT enabled for " + spConfig.getUniqueIdentifier());
+			log.trace("SL2.0 is NOT enabled with property: {}", getGeneralConfigPropertyNameEnableModule());
 			return null;
-			
+		
 		}
 						
 	}
 
+	/**
+	 * Get the general configuration-key that holds the enabled key for this authentication module 
+	 * 
+	 * @return
+	 */
+	public abstract String getGeneralConfigPropertyNameEnableModule();
+
+	/**
+	 * Get the default value of the general configuration-key that holds the enabled key for this authentication module 
+	 * 
+	 * @return
+	 */
+	public abstract boolean getGeneralConfigPropertyNameEnableModuleDefault();
+	
 	/**
 	 * Get the SP specific configuration-key that holds the enabled key for this authentication module 
 	 * 
 	 * @return configuration key for SP configuration
 	 */
-	public abstract String getConfigPropertyNameEnableModule();
+	public abstract String getSPConfigPropertyNameEnableModule();
 	
 	/**
 	 * Get the name of this specific SL2.0 process
diff --git a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/tasks/AbstractCreateQualeIDRequestTask.java b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/tasks/AbstractCreateQualeIDRequestTask.java
index 8939e61d..736ba077 100644
--- a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/tasks/AbstractCreateQualeIDRequestTask.java
+++ b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/tasks/AbstractCreateQualeIDRequestTask.java
@@ -28,7 +28,7 @@ import at.gv.egiz.eaaf.core.api.idp.process.ExecutionContext;
 import at.gv.egiz.eaaf.core.exceptions.EAAFAuthenticationException;
 import at.gv.egiz.eaaf.core.exceptions.TaskExecutionException;
 import at.gv.egiz.eaaf.core.impl.idp.auth.modules.AbstractAuthServletTask;
-import at.gv.egiz.eaaf.core.impl.utils.HttpClientFactory;
+import at.gv.egiz.eaaf.core.impl.utils.IHttpClientFactory;
 import at.gv.egiz.eaaf.core.impl.utils.KeyValueUtils;
 import at.gv.egiz.eaaf.core.impl.utils.Random;
 import at.gv.egiz.eaaf.core.impl.utils.TransactionIDUtils;
@@ -46,7 +46,7 @@ import at.gv.egiz.eaaf.modules.auth.sl20.utils.SL20JSONExtractorUtils;
 public abstract class AbstractCreateQualeIDRequestTask extends AbstractAuthServletTask {
 	private static final Logger log = LoggerFactory.getLogger(AbstractCreateQualeIDRequestTask.class);
 	
-	@Autowired(required=true) private HttpClientFactory httpClientFactory;
+	@Autowired(required=true) private IHttpClientFactory httpClientFactory;
 	
 	@Override 
 	public void execute(ExecutionContext executionContext, HttpServletRequest request, HttpServletResponse response)
@@ -60,6 +60,12 @@ public abstract class AbstractCreateQualeIDRequestTask extends AbstractAuthServl
 				//get service-provider configuration
 				final ISPConfiguration oaConfig = pendingReq.getServiceProviderConfiguration();
 				
+				if (oaConfig == null) {
+					log.warn("No SP configuration in pendingReq!");
+					throw new RuntimeException("Suspect state. NO SP CONFIGURATION IN PendingRequest!");
+					
+				}
+				
 				//get basic configuration parameters
 				final String vdaQualeIDUrl = extractVDAURLForSpecificOA(oaConfig, executionContext);				
 				if (StringUtils.isEmpty(vdaQualeIDUrl)) {
@@ -98,7 +104,7 @@ public abstract class AbstractCreateQualeIDRequestTask extends AbstractAuthServl
 				log.trace("Request VDA via SL20 with: " + Base64Url.encode(sl20Req.toString().getBytes()));
 				
 				//request VDA
-				final HttpResponse httpResp = httpClientFactory.getHttpClient().execute(httpReq);
+				final HttpResponse httpResp = httpClientFactory.getHttpClient(false).execute(httpReq);
 								
 				//parse response
 				log.info("Receive response from VDA ... ");
diff --git a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20HttpBindingUtils.java b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20HttpBindingUtils.java
index 39f2515d..4d8cabb7 100644
--- a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20HttpBindingUtils.java
+++ b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20HttpBindingUtils.java
@@ -10,10 +10,10 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.http.client.utils.URIBuilder;
-import org.apache.http.entity.ContentType;
 import org.jose4j.base64url.Base64Url;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.springframework.http.MediaType;
 
 import com.fasterxml.jackson.databind.JsonNode;
 
@@ -45,7 +45,7 @@ public class SL20HttpBindingUtils {
 			final byte[] content = writer.toString().getBytes("UTF-8");
 			httpResp.setStatus(HttpServletResponse.SC_OK);
 			httpResp.setContentLength(content.length);
-			httpResp.setContentType(ContentType.APPLICATION_JSON.toString());						
+			httpResp.setContentType(MediaType.APPLICATION_JSON_UTF8_VALUE);						
 			httpResp.getOutputStream().write(content);
 											
 		} else {
diff --git a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20JSONExtractorUtils.java b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20JSONExtractorUtils.java
index 901eff51..314dde17 100644
--- a/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20JSONExtractorUtils.java
+++ b/eaaf_modules/eaaf_module_auth_sl20/src/main/java/at/gv/egiz/eaaf/modules/auth/sl20/utils/SL20JSONExtractorUtils.java
@@ -291,15 +291,18 @@ public class SL20JSONExtractorUtils {
 	public static JsonNode getSL20ContainerFromResponse(HttpResponse httpResp) throws SLCommandoParserException {
 		try {
 			JsonNode sl20Resp = null;
-			if (httpResp.getStatusLine().getStatusCode() == 307) {
+			if (httpResp.getStatusLine().getStatusCode() == 303 || httpResp.getStatusLine().getStatusCode() == 307) {
 				final Header[] locationHeader = httpResp.getHeaders("Location");
 				if (locationHeader == null)
 					throw new SLCommandoParserException("Find Redirect statuscode but not Location header");
 			
 				final String sl20RespString = new URIBuilder(locationHeader[0].getValue()).getQueryParams().get(0).getValue();
-				sl20Resp =  mapper.getMapper().readTree(Base64Url.encode((sl20RespString.getBytes())));
+				sl20Resp =  mapper.getMapper().readTree(Base64Url.decode(sl20RespString));
 			
 			} else if (httpResp.getStatusLine().getStatusCode() == 200) {
+				if (httpResp.getEntity().getContentType() == null)
+					throw new SLCommandoParserException("SL20 response contains NO ContentType");					
+				
 				if (!httpResp.getEntity().getContentType().getValue().startsWith("application/json"))
 					throw new SLCommandoParserException("SL20 response with a wrong ContentType: " + httpResp.getEntity().getContentType().getValue());					
 				sl20Resp = parseSL20ResultFromResponse(httpResp.getEntity());
-- 
cgit v1.2.3