From: Niels Erik G. Nielsen Date: Tue, 4 Jun 2013 17:20:42 +0000 (-0400) Subject: Fixes authentication issues X-Git-Tag: v0.0.7~63 X-Git-Url: http://lists.indexdata.dk/?a=commitdiff_plain;h=1e1cf9efd103c5e1f04e27c67214aef0019beac1;p=mkjsf-moved-to-github.git Fixes authentication issues - avoids Weld injection exception when (authentication) servlet filter is run before a Faces context has been established (MissingConfigurationContextException) The mkjsf core itself has no such filter, but mk2jsf-demo has. - avoids redirect to given app page on auth failure --- diff --git a/src/main/java/com/indexdata/mkjsf/config/Mk2ConfigReader.java b/src/main/java/com/indexdata/mkjsf/config/Mk2ConfigReader.java index 5cd0566..8679458 100644 --- a/src/main/java/com/indexdata/mkjsf/config/Mk2ConfigReader.java +++ b/src/main/java/com/indexdata/mkjsf/config/Mk2ConfigReader.java @@ -19,6 +19,7 @@ import org.apache.log4j.Logger; import com.indexdata.masterkey.config.MasterkeyConfiguration; import com.indexdata.masterkey.config.ModuleConfiguration; import com.indexdata.mkjsf.errors.ConfigurationException; +import com.indexdata.mkjsf.errors.MissingConfigurationContextException; import com.indexdata.mkjsf.utils.Utils; import static com.indexdata.mkjsf.utils.Utils.nl; @@ -54,9 +55,14 @@ public class Mk2ConfigReader implements ConfigurationReader { private Configuration readConfig (Configurable configurable) throws ConfigurationException { Configuration config = new Configuration(); - ExternalContext externalContext = FacesContext.getCurrentInstance().getExternalContext(); - ServletContext servletContext = (ServletContext) externalContext.getContext(); - MasterkeyConfiguration mkConfigContext; + MasterkeyConfiguration mkConfigContext = null; + ExternalContext externalContext = null; + try { + externalContext = FacesContext.getCurrentInstance().getExternalContext(); + } catch (NullPointerException npe){ + throw new MissingConfigurationContextException("No FacesContext available to get configuration context from: " + npe.getMessage()); + } + ServletContext servletContext = (ServletContext) externalContext.getContext(); try { mkConfigContext = MasterkeyConfiguration.getInstance(servletContext, "mkjsf", ((HttpServletRequest) externalContext.getRequest()).getServerName()); diff --git a/src/main/java/com/indexdata/mkjsf/config/WebXmlConfigReader.java b/src/main/java/com/indexdata/mkjsf/config/WebXmlConfigReader.java index 7be0261..6772ccc 100644 --- a/src/main/java/com/indexdata/mkjsf/config/WebXmlConfigReader.java +++ b/src/main/java/com/indexdata/mkjsf/config/WebXmlConfigReader.java @@ -18,6 +18,7 @@ import javax.servlet.ServletContext; import org.apache.log4j.Logger; import com.indexdata.mkjsf.errors.ConfigurationException; +import com.indexdata.mkjsf.errors.MissingConfigurationContextException; /** * Reads a configuration from the context parameters of the deployment descriptor (web.xml) @@ -48,7 +49,12 @@ public class WebXmlConfigReader implements ConfigurationReader { private Map readConfig () throws ConfigurationException { Map map = new HashMap(); - ExternalContext externalContext = FacesContext.getCurrentInstance().getExternalContext(); + ExternalContext externalContext = null; + try { + externalContext = FacesContext.getCurrentInstance().getExternalContext(); + } catch (NullPointerException e) { + throw new MissingConfigurationContextException("WebXmlConfig: Configuration failed due to missing FacesContext."); + } ServletContext servletContext = (ServletContext) externalContext.getContext(); Enumeration enumer = servletContext.getInitParameterNames(); while (enumer.hasMoreElements()) { diff --git a/src/main/java/com/indexdata/mkjsf/errors/MissingConfigurationContextException.java b/src/main/java/com/indexdata/mkjsf/errors/MissingConfigurationContextException.java new file mode 100644 index 0000000..4264900 --- /dev/null +++ b/src/main/java/com/indexdata/mkjsf/errors/MissingConfigurationContextException.java @@ -0,0 +1,26 @@ +package com.indexdata.mkjsf.errors; + +public class MissingConfigurationContextException extends ConfigurationException { + + private static final long serialVersionUID = 1957059481668826430L; + + public MissingConfigurationContextException() { + // TODO Auto-generated constructor stub + } + + public MissingConfigurationContextException(String message) { + super(message); + // TODO Auto-generated constructor stub + } + + public MissingConfigurationContextException(Throwable cause) { + super(cause); + // TODO Auto-generated constructor stub + } + + public MissingConfigurationContextException(String message, Throwable cause) { + super(message, cause); + // TODO Auto-generated constructor stub + } + +} diff --git a/src/main/java/com/indexdata/mkjsf/pazpar2/Pz2Service.java b/src/main/java/com/indexdata/mkjsf/pazpar2/Pz2Service.java index 9304475..44cb05b 100644 --- a/src/main/java/com/indexdata/mkjsf/pazpar2/Pz2Service.java +++ b/src/main/java/com/indexdata/mkjsf/pazpar2/Pz2Service.java @@ -28,6 +28,7 @@ import com.indexdata.mkjsf.errors.ConfigurationError; import com.indexdata.mkjsf.errors.ConfigurationException; import com.indexdata.mkjsf.errors.ErrorCentral; import com.indexdata.mkjsf.errors.ErrorHelper; +import com.indexdata.mkjsf.errors.MissingConfigurationContextException; import com.indexdata.mkjsf.pazpar2.commands.Pazpar2Commands; import com.indexdata.mkjsf.pazpar2.data.RecordResponse; import com.indexdata.mkjsf.pazpar2.data.Responses; @@ -48,9 +49,8 @@ public class Pz2Service implements StateListener, Configurable, Serializable { private List pazpar2Urls = new ArrayList(); public static final String PAZPAR2_URL_LIST = "PAZPAR2_URL_LIST"; - private static final long serialVersionUID = 3440277287081557861L; - private static Logger logger = Logger.getLogger(Pz2Service.class); + private static Logger logger = Logger.getLogger(Pz2Service.class); protected Pz2Client pz2Client = null; protected ServiceProxyClient spClient = null; protected SearchClient searchClient = null; @@ -74,9 +74,9 @@ public class Pz2Service implements StateListener, Configurable, Serializable { FacesContext context = FacesContext.getCurrentInstance(); return (Pz2Service) context.getApplication().evaluateExpressionGet(context, "#{pz2}", Object.class); } - + @PostConstruct - public void postConstruct() { + public void postConstruct() throws MissingConfigurationContextException { logger.info("Pz2Service PostConstruct of " + this); stateMgr = new StateManager(); pzreq = new Pazpar2Commands(); @@ -85,18 +85,22 @@ public class Pz2Service implements StateListener, Configurable, Serializable { pzresp.setErrorHelper(errors.getHelper()); logger.debug("Pz2Service PostConstruct: Configurator is " + configurator); - logger.debug(Utils.objectId(this) + " will instantiate a Pz2Client next."); + logger.debug(Utils.objectId(this) + " will instantiate a Pz2Client next."); pz2Client = new Pz2Client(); - configureClient(pz2Client,configurator); spClient = new ServiceProxyClient(); - configureClient(spClient,configurator); + stateMgr.addStateListener(this); try { + configureClient(pz2Client,configurator); + configureClient(spClient,configurator); this.configure(configurator); + } catch (MissingConfigurationContextException mcc) { + logger.info("No configuration context available at this point"); + logger.debug("Configuration invoked from a Servlet filter before application start?"); + throw mcc; } catch (ConfigurationException e) { - logger.error("There was a problem configuring the Pz2Service (\"pz2\")"); + logger.warn("There was a problem configuring the Pz2Service and/or clients (\"pz2\")"); e.printStackTrace(); - } - stateMgr.addStateListener(this); + } } @Qualifier @@ -127,14 +131,17 @@ public class Pz2Service implements StateListener, Configurable, Serializable { return stateMgr; } - public void configureClient(SearchClient client, ConfigurationReader configReader) { + public void configureClient(SearchClient client, ConfigurationReader configReader) throws MissingConfigurationContextException { logger.debug(Utils.objectId(this) + " will configure search client for the session"); try { - client.configure(configReader); + client.configure(configReader); + } catch (MissingConfigurationContextException mcce) { + logger.info("No Faces context is available to the configurator at this time of invocation"); + throw mcce; } catch (ConfigurationException e) { logger.debug("Pz2Service adding configuration error"); errors.addConfigurationError(new ConfigurationError("Search Client","Configuration",e.getMessage())); - } + } logger.info(configReader.document()); pzresp.getSp().resetAuthAndBeyond(true); } @@ -236,6 +243,40 @@ public class Pz2Service implements StateListener, Configurable, Serializable { } } + + /** + * This methods main purpose is to support browser history. + * + * When the browsers back or forward buttons are pressed, a + * re-search /might/ be required - namely if the query changes. + * So, as the UI requests updates of the page (show,facets, + * etc) this method checks if a search must be executed + * before those updates are performed. + * + * @see {@link com.indexdata.mkjsf.pazpar2.state.StateManager#setCurrentStateKey} + * @param commands + */ + protected void handleQueryStateChanges (String commands) { + if (stateMgr.hasPendingStateChange("search") && hasQuery()) { + logger.info("Triggered search: Found pending search change [" + pzreq.getCommand("search").toString() + "], doing search before updating " + commands); + pzreq.getSearch().run(); + } + if (stateMgr.hasPendingStateChange("record") && ! commands.equals("record")) { + logger.debug("Found pending record ID change. Doing record before updating " + commands); + stateMgr.hasPendingStateChange("record",false); + pzreq.getRecord().run(); + } + } + + @Override + public void stateUpdated(String commandName) { + logger.debug("State change reported for [" + commandName + "]"); + if (commandName.equals("show")) { + logger.debug("Updating show"); + update(commandName); + } + } + /** * Will retrieve -- or alternatively remove -- the record with the given @@ -318,64 +359,7 @@ public class Pz2Service implements StateListener, Configurable, Serializable { pager = new ResultsPager(pzresp,pageRange,pzreq); return pager; } - - /** - * This methods main purpose is to support browser history. - * - * When the browsers back or forward buttons are pressed, a - * re-search /might/ be required - namely if the query changes. - * So, as the UI requests updates of the page (show,facets, - * etc) this method checks if a search must be executed - * before those updates are performed. - * - * @see {@link com.indexdata.mkjsf.pazpar2.state.StateManager#setCurrentStateKey} - * @param commands - */ - protected void handleQueryStateChanges (String commands) { - if (stateMgr.hasPendingStateChange("search") && hasQuery()) { - logger.info("Triggered search: Found pending search change [" + pzreq.getCommand("search").toString() + "], doing search before updating " + commands); - pzreq.getSearch().run(); - } - if (stateMgr.hasPendingStateChange("record") && ! commands.equals("record")) { - logger.debug("Found pending record ID change. Doing record before updating " + commands); - stateMgr.hasPendingStateChange("record",false); - pzreq.getRecord().run(); - } - } - - /** - * Executes the command and parses the response to create data objects. - * If the parsed response is of a known type it will be cached in 'pzresp' - * - * @param commandName The command to be executed - * @return An XML response parsed to form a response data object - */ - /* - protected ResponseDataObject doCommand(String commandName) { - Pazpar2Command command = pzreq.getCommand(commandName); - if (command.spOnly() && isPazpar2Service()) { - logger.warn("Skipping " + commandName + " - SP-only command, not supported by Pazpar2"); - return new ResponseDataObject(); - } else { - logger.info("Request "+commandName + ". Search command is: "+ pzreq.getCommand("search").toString()); - long start = System.currentTimeMillis(); - ResponseDataObject responseObject = command.run(); - long end = System.currentTimeMillis(); - logger.debug("Executed " + command.getCommandName() + " in " + (end-start) + " ms." ); - return responseObject; - } - } - */ - - @Override - public void stateUpdated(String commandName) { - logger.debug("State change reported for [" + commandName + "]"); - if (commandName.equals("show")) { - logger.debug("Updating show"); - update(commandName); - } - } - + public void setServiceProxyUrl(String url) { searchClient = spClient; setServiceType(SERVICE_TYPE_SP); diff --git a/src/main/java/com/indexdata/mkjsf/pazpar2/ServiceProxyClient.java b/src/main/java/com/indexdata/mkjsf/pazpar2/ServiceProxyClient.java index aa54403..2956afc 100644 --- a/src/main/java/com/indexdata/mkjsf/pazpar2/ServiceProxyClient.java +++ b/src/main/java/com/indexdata/mkjsf/pazpar2/ServiceProxyClient.java @@ -34,6 +34,7 @@ import org.apache.log4j.Logger; import com.indexdata.mkjsf.config.Configuration; import com.indexdata.mkjsf.config.ConfigurationReader; import com.indexdata.mkjsf.errors.ConfigurationException; +import com.indexdata.mkjsf.errors.MissingConfigurationContextException; import com.indexdata.mkjsf.pazpar2.commands.CommandParameter; import com.indexdata.mkjsf.pazpar2.commands.Pazpar2Command; import com.indexdata.mkjsf.pazpar2.commands.sp.AuthCommand; @@ -65,7 +66,7 @@ public class ServiceProxyClient implements SearchClient { } @Override - public void configure (ConfigurationReader configReader) { + public void configure (ConfigurationReader configReader) throws MissingConfigurationContextException { logger.info(Utils.objectId(this) + " is configuring using the provided " + Utils.objectId(configReader)); try { config = configReader.getConfiguration(this); @@ -75,10 +76,12 @@ public class ServiceProxyClient implements SearchClient { checkAuth.setParameterInState(new CommandParameter("action","=","check")); ipAuth = new AuthCommand(); ipAuth.setParameterInState(new CommandParameter("action","=","ipauth")); - } catch (ConfigurationException c) { - // TODO: - c.printStackTrace(); - } + } catch (MissingConfigurationContextException mcce) { + throw mcce; + } catch (ConfigurationException ce) { + logger.error("Failed to configure Service Proxy client"); + ce.printStackTrace(); + } } public boolean isAuthenticatingClient () { diff --git a/src/main/java/com/indexdata/mkjsf/pazpar2/data/sp/AuthResponse.java b/src/main/java/com/indexdata/mkjsf/pazpar2/data/sp/AuthResponse.java index 243d949..334bd15 100644 --- a/src/main/java/com/indexdata/mkjsf/pazpar2/data/sp/AuthResponse.java +++ b/src/main/java/com/indexdata/mkjsf/pazpar2/data/sp/AuthResponse.java @@ -21,8 +21,14 @@ public class AuthResponse extends SpResponseDataObject { } public String onSuccess(String navigateTo) { - return navigateTo; + if (isAuthenticationOk()) { + return navigateTo; + } else { + return null; + } } - + public boolean isAuthenticationOk () { + return getStatus().equalsIgnoreCase("OK"); + } }