From 0a19ed7d86ee7775de0d08688d8962ae7b73c0af Mon Sep 17 00:00:00 2001 From: j2blake Date: Tue, 31 Jan 2012 21:25:27 +0000 Subject: [PATCH] NIHVIVO-2694 Change how the AuthenticatorFactory is structured, so it will be easier to change on the fly. --- .../authenticate/Authenticator.java | 35 +++++++++----- .../authenticate/BasicAuthenticator.java | 20 +++++++- .../authenticate/AuthenticatorStub.java | 48 +++++-------------- .../authenticate/ProgramLoginTest.java | 6 ++- .../controller/edit/AuthenticateTest.java | 13 +++-- 5 files changed, 65 insertions(+), 57 deletions(-) diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java index dce2bf46a..fb86eb1d1 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/Authenticator.java @@ -8,6 +8,7 @@ import java.util.List; import javax.mail.internet.AddressException; import javax.mail.internet.InternetAddress; +import javax.servlet.ServletContext; import javax.servlet.http.HttpServletRequest; import org.apache.commons.codec.binary.Hex; @@ -27,23 +28,33 @@ public abstract class Authenticator { // ---------------------------------------------------------------------- // The factory // - // Unit tests can replace the factory to get a stub class instead. - // Note: this can only work because the factory value is not final. + // Each Authenticator instance is used for a single request, so we store + // a factory in the context that can create these instances. // ---------------------------------------------------------------------- - public static interface AuthenticatorFactory { - Authenticator newInstance(HttpServletRequest request); + private static final String FACTORY_ATTRIBUTE_NAME = AuthenticatorFactory.class + .getName(); + + public interface AuthenticatorFactory { + Authenticator getInstance(HttpServletRequest request); } - private static AuthenticatorFactory factory = new AuthenticatorFactory() { - @Override - public Authenticator newInstance(HttpServletRequest request) { - return new BasicAuthenticator(request); - } - }; - + /** + * Ask the currently configured AuthenticatorFactory to give us an + * Authenticator for this request. + * + * If there is no factory, configure a Basic one. + */ public static Authenticator getInstance(HttpServletRequest request) { - return factory.newInstance(request); + ServletContext ctx = request.getSession().getServletContext(); + Object attribute = ctx.getAttribute(FACTORY_ATTRIBUTE_NAME); + if (! (attribute instanceof AuthenticatorFactory)) { + attribute = new BasicAuthenticator.Factory(); + ctx.setAttribute(FACTORY_ATTRIBUTE_NAME, attribute); + } + AuthenticatorFactory factory = (AuthenticatorFactory) attribute; + + return factory.getInstance(request); } // ---------------------------------------------------------------------- diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java index ff299596e..347d6dc86 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/BasicAuthenticator.java @@ -37,6 +37,21 @@ import edu.cornell.mannlib.vitro.webapp.search.indexing.IndexBuilder; public class BasicAuthenticator extends Authenticator { private static final Log log = LogFactory.getLog(BasicAuthenticator.class); + // ---------------------------------------------------------------------- + // The factory + // ---------------------------------------------------------------------- + + public static class Factory implements AuthenticatorFactory { + @Override + public Authenticator getInstance(HttpServletRequest req) { + return new BasicAuthenticator(req); + } + } + + // ---------------------------------------------------------------------- + // The authenticator + // ---------------------------------------------------------------------- + private final HttpServletRequest request; public BasicAuthenticator(HttpServletRequest request) { @@ -134,10 +149,11 @@ public class BasicAuthenticator extends Authenticator { setSessionTimeoutLimit(userAccount, session); recordInUserSessionMap(userAccount.getUri(), session); notifyOtherUsers(userAccount.getUri(), session); - + if (IsRootUser.isRootUser(RequestIdentifiers .getIdBundleForRequest(request))) { - IndexBuilder.checkIndexOnRootLogin(request); } + IndexBuilder.checkIndexOnRootLogin(request); + } } /** diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java index 3dd4fabd0..f97d55032 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/AuthenticatorStub.java @@ -2,7 +2,6 @@ package edu.cornell.mannlib.vitro.webapp.controller.authenticate; -import java.lang.reflect.Field; import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -19,47 +18,22 @@ import edu.cornell.mannlib.vitro.webapp.beans.UserAccount; * put it into place. */ public class AuthenticatorStub extends Authenticator { + public static final String FACTORY_ATTRIBUTE_NAME = AuthenticatorFactory.class + .getName(); + // ---------------------------------------------------------------------- - // factory + // factory - store this in the context. + // + // Creates a single instance of the stub and returns it for all requests. // ---------------------------------------------------------------------- - /** - * Create a single instance of the stub. Force our factory into the - * Authenticator, so each request for an instance returns that one. - * - * Call this at the top of each unit test, so you get fresh instance for - * each test. - */ - public static AuthenticatorStub setup() throws SecurityException, - NoSuchFieldException, IllegalArgumentException, - IllegalAccessException { - AuthenticatorStub authenticator = new AuthenticatorStub(); - - Field factoryField = Authenticator.class.getDeclaredField("factory"); - factoryField.setAccessible(true); - Authenticator.AuthenticatorFactory factory = new AuthenticatorStub.AuthenticatorFactory( - authenticator); - factoryField.set(null, factory); - - return authenticator; - } - - /** - * This factory holds a single instance of the stub, and hands it out each - * time we request an "newInstance". - */ - private static class AuthenticatorFactory implements - Authenticator.AuthenticatorFactory { - private final AuthenticatorStub authenticator; - - public AuthenticatorFactory(AuthenticatorStub authenticator) { - this.authenticator = authenticator; - } + public static class Factory implements Authenticator.AuthenticatorFactory { + private final AuthenticatorStub instance = new AuthenticatorStub(); @Override - public Authenticator newInstance(HttpServletRequest request) { - authenticator.setRequest(request); - return authenticator; + public AuthenticatorStub getInstance(HttpServletRequest request) { + instance.setRequest(request); + return instance; } } diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/ProgramLoginTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/ProgramLoginTest.java index 666b4b208..4e2354576 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/ProgramLoginTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/authenticate/ProgramLoginTest.java @@ -48,6 +48,7 @@ public class ProgramLoginTest extends AbstractTestClass { private static final UserAccount OLD_USER = createUserAccount(OLD_USER_URI, OLD_USER_NAME, OLD_USER_PASSWORD, 10); + private AuthenticatorStub.Factory authenticatorFactory; private AuthenticatorStub authenticator; private ServletContextStub servletContext; private ServletConfigStub servletConfig; @@ -64,11 +65,14 @@ public class ProgramLoginTest extends AbstractTestClass { @Before public void setup() throws Exception { - authenticator = AuthenticatorStub.setup(); + authenticatorFactory = new AuthenticatorStub.Factory(); + authenticator = authenticatorFactory.getInstance(request); authenticator.addUser(NEW_USER); authenticator.addUser(OLD_USER); servletContext = new ServletContextStub(); + servletContext.setAttribute(AuthenticatorStub.FACTORY_ATTRIBUTE_NAME, + authenticatorFactory); servletConfig = new ServletConfigStub(); servletConfig.setServletContext(servletContext); diff --git a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java index 97c6da94f..10e937af9 100644 --- a/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java +++ b/webapp/test/edu/cornell/mannlib/vitro/webapp/controller/edit/AuthenticateTest.java @@ -10,7 +10,6 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; -import java.net.URL; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; @@ -52,7 +51,7 @@ import edu.cornell.mannlib.vitro.webapp.controller.login.LoginProcessBean.State; /** */ public class AuthenticateTest extends AbstractTestClass { - + private AuthenticatorStub.Factory authenticatorFactory; private AuthenticatorStub authenticator; private ServletContextStub servletContext; private WebappDaoFactoryStub webappDaoFactory; @@ -120,7 +119,8 @@ public class AuthenticateTest extends AbstractTestClass { @Before public void setup() throws Exception { - authenticator = AuthenticatorStub.setup(); + authenticatorFactory = new AuthenticatorStub.Factory(); + authenticator = authenticatorFactory.getInstance(request); authenticator.addUser(createUserFromUserInfo(NEW_DBA)); authenticator.addUser(createUserFromUserInfo(OLD_DBA)); authenticator.addUser(createUserFromUserInfo(OLD_SELF)); @@ -148,12 +148,14 @@ public class AuthenticateTest extends AbstractTestClass { servletContext = new ServletContextStub(); servletContext.setAttribute("webappDaoFactory", webappDaoFactory); + servletContext.setAttribute(AuthenticatorStub.FACTORY_ATTRIBUTE_NAME, + authenticatorFactory); setLoggerLevel(ServletPolicyList.class, Level.WARN); ServletPolicyList.addPolicy(servletContext, new PermissionsPolicy()); PermissionRegistry.createRegistry(servletContext, Collections.singleton(SimplePermission.SEE_SITE_ADMIN_PAGE)); - + servletConfig = new ServletConfigStub(); servletConfig.setServletContext(servletContext); @@ -162,7 +164,8 @@ public class AuthenticateTest extends AbstractTestClass { request = new HttpServletRequestStub(); request.setSession(session); - request.setRequestUrlByParts("http://this.that", "/vivo", "/authenticate", null); + request.setRequestUrlByParts("http://this.that", "/vivo", + "/authenticate", null); request.setMethod("POST"); response = new HttpServletResponseStub();