From 4b06e2cc0cb5f381db1008d53f48a05ccf51b072 Mon Sep 17 00:00:00 2001 From: Rick Herrick <jrherrick@wustl.edu> Date: Wed, 13 Jul 2016 16:35:42 -0500 Subject: [PATCH] XNAT-4327 Removed additional code that set CSRF token. All token sets should now be done only in session event publisher at the point when the session is actually created. --- build.gradle | 2 +- .../xnat/restlet/guard/XnatSecureGuard.java | 5 --- .../restlet/resources/SecureResource.java | 2 - .../xnat/restlet/resources/UserSession.java | 3 -- .../org/nrg/xnat/security/OnXnatLogin.java | 45 +++++++++---------- .../XnatBasicAuthenticationFilter.java | 34 +++++++------- 6 files changed, 40 insertions(+), 51 deletions(-) diff --git a/build.gradle b/build.gradle index f2497166..bc56306d 100644 --- a/build.gradle +++ b/build.gradle @@ -390,7 +390,7 @@ dependencies { compile "net.sourceforge.saxon:saxon:${vSaxon}" compile "xalan:xalan:2.7.2" - compile "nl.bitwalker:UserAgentUtils:1.2.4" + compile "eu.bitwalker:UserAgentUtils:1.20" compile "com.twmacinta:fast-md5:2.7.1" compile "com.h2database:h2:1.4.191" compile "com.lowagie:itext:4.2.1" diff --git a/src/main/java/org/nrg/xnat/restlet/guard/XnatSecureGuard.java b/src/main/java/org/nrg/xnat/restlet/guard/XnatSecureGuard.java index ea38961e..ebbcdb05 100644 --- a/src/main/java/org/nrg/xnat/restlet/guard/XnatSecureGuard.java +++ b/src/main/java/org/nrg/xnat/restlet/guard/XnatSecureGuard.java @@ -30,8 +30,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpSession; -import java.util.UUID; public class XnatSecureGuard extends Filter { private static final Logger logger = LoggerFactory.getLogger(XnatSecureGuard.class); @@ -109,14 +107,11 @@ public class XnatSecureGuard extends Filter { if (challengeResponse != null) { user = authenticateBasic(challengeResponse); if (user != null) { - httpRequest.getSession().setAttribute("XNAT_CSRF", UUID.randomUUID().toString()); return true; } } else if (!XDAT.getSiteConfigPreferences().getRequireLogin()) { try { - HttpSession session = httpRequest.getSession(); - session.removeAttribute("loggedin"); user=Users.getGuest(); if (user!=null) { return true; diff --git a/src/main/java/org/nrg/xnat/restlet/resources/SecureResource.java b/src/main/java/org/nrg/xnat/restlet/resources/SecureResource.java index 08b451f0..49409e2b 100644 --- a/src/main/java/org/nrg/xnat/restlet/resources/SecureResource.java +++ b/src/main/java/org/nrg/xnat/restlet/resources/SecureResource.java @@ -133,8 +133,6 @@ public abstract class SecureResource extends Resource { public String requested_format = null; public String filepath = null; - protected String csrfToken = null; - private final SerializerService _serializer; public SecureResource(Context context, Request request, Response response) { diff --git a/src/main/java/org/nrg/xnat/restlet/resources/UserSession.java b/src/main/java/org/nrg/xnat/restlet/resources/UserSession.java index 6d0ba78e..026536bc 100644 --- a/src/main/java/org/nrg/xnat/restlet/resources/UserSession.java +++ b/src/main/java/org/nrg/xnat/restlet/resources/UserSession.java @@ -21,8 +21,6 @@ import org.restlet.resource.ResourceException; import org.restlet.resource.StringRepresentation; import org.restlet.resource.Variant; -import java.util.UUID; - public class UserSession extends SecureResource { protected UserI user = null; @@ -33,7 +31,6 @@ public class UserSession extends SecureResource { // copy the user from the request into the session getHttpSession().setAttribute("userHelper", UserHelper.getUserHelperService(user)); - getHttpSession().setAttribute("XNAT_CSRF", UUID.randomUUID().toString()); } @Override diff --git a/src/main/java/org/nrg/xnat/security/OnXnatLogin.java b/src/main/java/org/nrg/xnat/security/OnXnatLogin.java index f8e20eff..09d5d5a9 100644 --- a/src/main/java/org/nrg/xnat/security/OnXnatLogin.java +++ b/src/main/java/org/nrg/xnat/security/OnXnatLogin.java @@ -10,28 +10,25 @@ */ package org.nrg.xnat.security; -import java.io.IOException; -import java.util.UUID; - -import javax.servlet.ServletException; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; import org.nrg.xdat.XDAT; import org.nrg.xdat.security.helpers.UserHelper; -import org.nrg.xdat.security.helpers.Users; import org.nrg.xdat.turbine.utils.AccessLogger; import org.nrg.xft.XFTItem; import org.nrg.xft.event.EventUtils; import org.nrg.xft.security.UserI; import org.nrg.xft.utils.SaveItemHelper; import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContext; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.authentication.SavedRequestAwareAuthenticationSuccessHandler; +import javax.servlet.ServletException; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.io.IOException; +import java.util.Calendar; +import java.util.Date; + public class OnXnatLogin extends SavedRequestAwareAuthenticationSuccessHandler { protected final Log logger = LogFactory.getLog(getClass()); @@ -47,20 +44,20 @@ public class OnXnatLogin extends SavedRequestAwareAuthenticationSuccessHandler { try{ final UserI user = XDAT.getUserDetails(); - request.getSession().setAttribute("XNAT_CSRF", UUID.randomUUID().toString()); - - java.util.Date today = java.util.Calendar.getInstance(java.util.TimeZone.getDefault()).getTime(); - XFTItem item = XFTItem.NewItem("xdat:user_login",user); - item.setProperty("xdat:user_login.user_xdat_user_id", user.getID()); - item.setProperty("xdat:user_login.login_date",today); - item.setProperty("xdat:user_login.ip_address", AccessLogger.GetRequestIp(request)); - item.setProperty("xdat:user_login.session_id", request.getSession().getId()); - SaveItemHelper.authorizedSave(item,null,true,false, EventUtils.DEFAULT_EVENT(user,null)); - - request.getSession().setAttribute("userHelper", UserHelper.getUserHelperService(user)); - } - catch(Exception e){ - logger.error(e); + + if (user != null) { + Date today = Calendar.getInstance(java.util.TimeZone.getDefault()).getTime(); + XFTItem item = XFTItem.NewItem("xdat:user_login", user); + item.setProperty("xdat:user_login.user_xdat_user_id", user.getID()); + item.setProperty("xdat:user_login.login_date", today); + item.setProperty("xdat:user_login.ip_address", AccessLogger.GetRequestIp(request)); + item.setProperty("xdat:user_login.session_id", request.getSession().getId()); + SaveItemHelper.authorizedSave(item, null, true, false, EventUtils.DEFAULT_EVENT(user, null)); + + request.getSession().setAttribute("userHelper", UserHelper.getUserHelperService(user)); + } + } catch (Exception e) { + logger.error(e); } super.onAuthenticationSuccess(request, response, authentication); } diff --git a/src/main/java/org/nrg/xnat/security/XnatBasicAuthenticationFilter.java b/src/main/java/org/nrg/xnat/security/XnatBasicAuthenticationFilter.java index 3f78e5c3..21d71b33 100644 --- a/src/main/java/org/nrg/xnat/security/XnatBasicAuthenticationFilter.java +++ b/src/main/java/org/nrg/xnat/security/XnatBasicAuthenticationFilter.java @@ -39,8 +39,9 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Calendar; +import java.util.Date; import java.util.Map; -import java.util.UUID; public class XnatBasicAuthenticationFilter extends BasicAuthenticationFilter { @@ -152,23 +153,24 @@ public class XnatBasicAuthenticationFilter extends BasicAuthenticationFilter { Authentication authResult) throws IOException { try { final UserI user = XDAT.getUserDetails(); - Object lock = locks.get(user.getID()); - if (lock == null) { - locks.put(user.getID(), new Object()); - lock = locks.get(user.getID()); - } + if (user != null) { + Object lock = locks.get(user.getID()); + if (lock == null) { + locks.put(user.getID(), new Object()); + lock = locks.get(user.getID()); + } - synchronized (lock) { - java.util.Date today = java.util.Calendar.getInstance(java.util.TimeZone.getDefault()).getTime(); - XFTItem item = XFTItem.NewItem("xdat:user_login", user); - item.setProperty("xdat:user_login.user_xdat_user_id", user.getID()); - item.setProperty("xdat:user_login.login_date", today); - item.setProperty("xdat:user_login.ip_address", AccessLogger.GetRequestIp(request)); - item.setProperty("xdat:user_login.session_id", request.getSession().getId()); - SaveItemHelper.authorizedSave(item, null, true, false, EventUtils.DEFAULT_EVENT(user, null)); + synchronized (lock) { + Date today = Calendar.getInstance(java.util.TimeZone.getDefault()).getTime(); + XFTItem item = XFTItem.NewItem("xdat:user_login", user); + item.setProperty("xdat:user_login.user_xdat_user_id", user.getID()); + item.setProperty("xdat:user_login.login_date", today); + item.setProperty("xdat:user_login.ip_address", AccessLogger.GetRequestIp(request)); + item.setProperty("xdat:user_login.session_id", request.getSession().getId()); + SaveItemHelper.authorizedSave(item, null, true, false, EventUtils.DEFAULT_EVENT(user, null)); + } + request.getSession().setAttribute("userHelper", UserHelper.getUserHelperService(user)); } - request.getSession().setAttribute("XNAT_CSRF", UUID.randomUUID().toString()); - request.getSession().setAttribute("userHelper", UserHelper.getUserHelperService(user)); } catch (Exception e) { logger.error(e); } -- GitLab