From 1a970e62627c58639f5243747029b5e90a87dfa6 Mon Sep 17 00:00:00 2001 From: jeb228 Date: Fri, 10 Dec 2010 16:30:22 +0000 Subject: [PATCH] NIHVIVO-151 handle the case where we come from a bookmark of the login link -- a "return" parameter with no referrer. --- .../authenticate/LoginRedirector.java | 36 ++++------- .../webapp/controller/edit/Authenticate.java | 62 +++++++++++++------ .../controller/edit/AuthenticateTest.java | 45 +++++++++----- 3 files changed, 83 insertions(+), 60 deletions(-) diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/LoginRedirector.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/LoginRedirector.java index 3cb8116c1..363d98976 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/LoginRedirector.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/authenticate/LoginRedirector.java @@ -30,7 +30,6 @@ public class LoginRedirector { private final HttpSession session; private final String uriOfAssociatedIndividual; - private final String loginProcessPage; private final String afterLoginPage; public LoginRedirector(HttpServletRequest request, @@ -43,7 +42,6 @@ public class LoginRedirector { LoginProcessBean processBean = LoginProcessBean.getBean(request); log.debug("process bean is: " + processBean); - loginProcessPage = processBean.getLoginPageUrl(); afterLoginPage = processBean.getAfterLoginUrl(); } @@ -80,18 +78,15 @@ public class LoginRedirector { + "but the system contains no profile for you."); response.sendRedirect(getApplicationHomePageUrl()); } else { - if (hasSomeplaceToGoAfterLogin()) { - log.debug("Returning to requested page: " + afterLoginPage); - response.sendRedirect(afterLoginPage); - } else if (loginProcessPage == null) { - log.debug("Don't know what to do. Go home."); - response.sendRedirect(getApplicationHomePageUrl()); - } else if (isLoginPage(loginProcessPage)) { + if (isLoginPage(afterLoginPage)) { log.debug("Coming from /login. Going to site admin page."); response.sendRedirect(getSiteAdminPageUrl()); + } else if (null != afterLoginPage) { + log.debug("Returning to requested page: " + afterLoginPage); + response.sendRedirect(afterLoginPage); } else { - log.debug("Coming from a login widget. Going back there."); - response.sendRedirect(loginProcessPage); + log.debug("Don't know what to do. Go home."); + response.sendRedirect(getApplicationHomePageUrl()); } } LoginProcessBean.removeBean(request); @@ -103,18 +98,15 @@ public class LoginRedirector { public void redirectCancellingUser() throws IOException { try { - if (hasSomeplaceToGoAfterLogin()) { - log.debug("Returning to requested page: " + afterLoginPage); - response.sendRedirect(afterLoginPage); - } else if (loginProcessPage == null) { - log.debug("Don't know what to do. Go home."); - response.sendRedirect(getApplicationHomePageUrl()); - } else if (isLoginPage(loginProcessPage)) { + if (isLoginPage(afterLoginPage)) { log.debug("Coming from /login. Going to home."); response.sendRedirect(getApplicationHomePageUrl()); + } else if (null != afterLoginPage) { + log.debug("Returning to requested page: " + afterLoginPage); + response.sendRedirect(afterLoginPage); } else { - log.debug("Coming from a login widget. Going back there."); - response.sendRedirect(loginProcessPage); + log.debug("Don't know what to do. Go home."); + response.sendRedirect(getApplicationHomePageUrl()); } LoginProcessBean.removeBean(request); } catch (IOException e) { @@ -131,10 +123,6 @@ public class LoginRedirector { response.sendRedirect(getApplicationHomePageUrl()); } - private boolean hasSomeplaceToGoAfterLogin() { - return afterLoginPage != null; - } - private boolean isMerelySelfEditor() { return LoginStatusBean.getBean(session).isLoggedInExactly( LoginStatusBean.NON_EDITOR); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java index 278f5d9af..d9eaad176 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/Authenticate.java @@ -135,36 +135,60 @@ public class Authenticate extends VitroHttpServlet { /** * If they supply an after-login page, record it and use the Login page for - * the process. + * the process. Note that we expect it to be URL-encoded. * - * If they supply a return flag, record the referrer as the after-login page - * and use the Login page for the process. + * If they supply a return flag, record the current page as the after-login + * page and use the Login page for the process. * * Otherwise, use the current page for the process. + * + * The "current page" is the referrer, unless there is no referrer for some + * reason. In that case, pretend it's the login page. */ private void recordLoginProcessPages(HttpServletRequest request) { LoginProcessBean bean = LoginProcessBean.getBean(request); - String afterLoginUrl = request.getParameter(PARAMETER_AFTER_LOGIN); + String afterLoginUrl = decodeAfterLoginParameter(request); + boolean doReturn = isReturnParameterSet(request); + String referrer = whereDidWeComeFrom(request); + if (afterLoginUrl != null) { - try { - String decoded = URLDecoder.decode(afterLoginUrl, "UTF-8"); - bean.setAfterLoginUrl(decoded); - } catch (UnsupportedEncodingException e) { - log.error("Really? No UTF-8 encoding?"); - } - } - - String returnParameter = request.getParameter(PARAMETER_RETURN); - if (returnParameter != null) { - String referrer = request.getHeader("referer"); + bean.setAfterLoginUrl(afterLoginUrl); + bean.setLoginPageUrl(request.getContextPath() + Controllers.LOGIN); + } else if (doReturn) { bean.setAfterLoginUrl(referrer); - } - - if (bean.getAfterLoginUrl() != null) { bean.setLoginPageUrl(request.getContextPath() + Controllers.LOGIN); } else { - bean.setLoginPageUrl(request.getHeader("referer")); + bean.setAfterLoginUrl(referrer); + bean.setLoginPageUrl(referrer); + } + } + + private String decodeAfterLoginParameter(HttpServletRequest request) { + String parm = request.getParameter(PARAMETER_AFTER_LOGIN); + if (parm == null) { + return null; + } else { + try { + return URLDecoder.decode(parm, "UTF-8"); + } catch (UnsupportedEncodingException e) { + log.error("No UTF-8 encoding? Really?", e); + return parm; + } + } + } + + private boolean isReturnParameterSet(HttpServletRequest request) { + return (null != request.getParameter(PARAMETER_RETURN)); + } + + /** If no referrer, say we were on the login page. */ + private String whereDidWeComeFrom(HttpServletRequest request) { + String referrer = request.getHeader("referer"); + if (referrer != null) { + return referrer; + } else { + return request.getContextPath() + Controllers.LOGIN; } } 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 ebcbc6323..49d735cea 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 @@ -183,8 +183,12 @@ public class AuthenticateTest extends AbstractTestClass { private static final HowDidWeGetHere FROM_WIDGET = new HowDidWeGetHere( null, false, URL_WIDGET); - private static final HowDidWeGetHere FROM_LOGIN = new HowDidWeGetHere(null, - false, URL_LOGIN); + private static final HowDidWeGetHere FROM_LOGIN = new HowDidWeGetHere( + null, false, URL_LOGIN); + + /** "return" parameter with no referrer - like coming from the login page. */ + private static final HowDidWeGetHere FROM_BOOKMARK_OF_LINK = new HowDidWeGetHere( + null, true, null); // --------- All sets of test data ---------- @@ -202,26 +206,28 @@ public class AuthenticateTest extends AbstractTestClass { { OLD_DBA, FROM_FORCED, new WhereTo(URL_LOGIN, URL_RESTRICTED, null) }, // 4 { OLD_DBA, FROM_LINK, new WhereTo(URL_LOGIN, URL_LINK, null) }, // 5 + { OLD_DBA, FROM_BOOKMARK_OF_LINK, + new WhereTo(URL_LOGIN, URL_SITE_ADMIN, null) }, // 6 { OLD_DBA, FROM_WIDGET, - new WhereTo(URL_WIDGET, URL_WIDGET, null) }, // 6 + new WhereTo(URL_WIDGET, URL_WIDGET, null) }, // 7 { OLD_DBA, FROM_LOGIN, - new WhereTo(URL_LOGIN, URL_SITE_ADMIN, null) }, // 7 + new WhereTo(URL_LOGIN, URL_SITE_ADMIN, null) }, // 8 { OLD_SELF, FROM_FORCED, - new WhereTo(URL_LOGIN, URL_SELF_PROFILE, null) }, // 8 - { OLD_SELF, FROM_LINK, new WhereTo(URL_LOGIN, URL_SELF_PROFILE, null) }, // 9 + { OLD_SELF, FROM_LINK, + new WhereTo(URL_LOGIN, URL_SELF_PROFILE, null) }, // 10 { OLD_SELF, FROM_WIDGET, - new WhereTo(URL_WIDGET, URL_SELF_PROFILE, null) }, // 10 + new WhereTo(URL_WIDGET, URL_SELF_PROFILE, null) }, // 11 { OLD_SELF, FROM_LOGIN, - new WhereTo(URL_LOGIN, URL_SELF_PROFILE, null) }, // 11 + new WhereTo(URL_LOGIN, URL_SELF_PROFILE, null) }, // 12 { NEW_STRANGER, FROM_FORCED, - new WhereTo(URL_LOGIN, URL_HOME, URL_RESTRICTED) }, // 12 + new WhereTo(URL_LOGIN, URL_HOME, URL_RESTRICTED) }, // 13 { NEW_STRANGER, FROM_LINK, - new WhereTo(URL_LOGIN, URL_HOME, URL_LINK) }, // 13 + new WhereTo(URL_LOGIN, URL_HOME, URL_LINK) }, // 14 { NEW_STRANGER, FROM_WIDGET, - new WhereTo(URL_WIDGET, URL_HOME, URL_WIDGET) }, // 14 + new WhereTo(URL_WIDGET, URL_HOME, URL_WIDGET) }, // 15 { NEW_STRANGER, FROM_LOGIN, - new WhereTo(URL_LOGIN, URL_HOME, URL_HOME) } // 15 + new WhereTo(URL_LOGIN, URL_HOME, URL_HOME) } // 16 }; return Arrays.asList(data); } @@ -503,7 +509,7 @@ public class AuthenticateTest extends AbstractTestClass { processBean.setAfterLoginUrl(urlBundle.referrer); processBean.setLoginPageUrl(URL_LOGIN); } else { - processBean.setAfterLoginUrl(null); + processBean.setAfterLoginUrl(urlBundle.referrer); processBean.setLoginPageUrl(urlBundle.referrer); } LoginProcessBean.setBean(request, processBean); @@ -562,10 +568,15 @@ public class AuthenticateTest extends AbstractTestClass { assertEquals("username", username, bean.getUsername()); // This should represent the URL bundle, every time. - String expectedAfterLoginUrl = (urlBundle.returnParameterSet) ? urlBundle.referrer - : urlBundle.afterLoginUrl; - assertEquals("after login URL", expectedAfterLoginUrl, - bean.getAfterLoginUrl()); + if (urlBundle.afterLoginUrl != null) { + assertEquals("after login URL", urlBundle.afterLoginUrl, + bean.getAfterLoginUrl()); + } else if (urlBundle.referrer != null) { + assertEquals("after login URL", urlBundle.referrer, + bean.getAfterLoginUrl()); + } else { + assertEquals("after login URL", URL_LOGIN, bean.getAfterLoginUrl()); + } } /** What logins were completed in this test? */