From 2d7679cddad5fec3d1c5bc358e89c012438f04e4 Mon Sep 17 00:00:00 2001
From: lawson89 <>
Date: Thu, 21 Aug 2014 09:05:12 -0400
Subject: [PATCH] fix various session issues and cleanup main change is to
force spring security to always send user to welcome.mvc after login which
gets their session setup properly before redirecting to start.mvc
---
java/org/owasp/webgoat/HammerHead.java | 12 +--
.../owasp/webgoat/service/BaseService.java | 7 +-
.../org/owasp/webgoat/session/WebSession.java | 3 +
webapp/META-INF/context.xml | 2 +-
webapp/WEB-INF/mvc-dispatcher-servlet.xml | 73 +++++++++++--------
webapp/WEB-INF/spring-security.xml | 6 +-
6 files changed, 62 insertions(+), 41 deletions(-)
diff --git a/java/org/owasp/webgoat/HammerHead.java b/java/org/owasp/webgoat/HammerHead.java
index a39287b2c..da38d28f1 100644
--- a/java/org/owasp/webgoat/HammerHead.java
+++ b/java/org/owasp/webgoat/HammerHead.java
@@ -182,9 +182,10 @@ public class HammerHead extends HttpServlet {
clientBrowser = userAgent;
}
request.setAttribute("client.browser", clientBrowser);
- request.getSession().setAttribute(WebSession.SESSION, mySession);
+ // removed - this is being done in updateSession call
+ //request.getSession().setAttribute(WebSession.SESSION, mySession);
// not sure why this is being set in the session?
- request.getSession().setAttribute(WebSession.COURSE, mySession.getCourse());
+ //request.getSession().setAttribute(WebSession.COURSE, mySession.getCourse());
String viewPage = getViewPage(mySession);
logger.debug("Forwarding to view: " + viewPage);
logger.debug("Screen: " + screen);
@@ -374,7 +375,8 @@ public class HammerHead extends HttpServlet {
protected WebSession updateSession(HttpServletRequest request, HttpServletResponse response, ServletContext context)
throws IOException {
HttpSession hs;
- hs = request.getSession(true);
+ // session should already be created by spring security
+ hs = request.getSession(false);
logger.debug("HH Entering Session_id: " + hs.getId());
// dumpSession( hs );
@@ -384,6 +386,7 @@ public class HammerHead extends HttpServlet {
if ((o != null) && o instanceof WebSession) {
session = (WebSession) o;
+ hs.setAttribute(WebSession.COURSE, session.getCourse());
} else {
// Create new custom session and save it in the HTTP session
logger.warn("HH Creating new WebSession");
@@ -394,13 +397,12 @@ public class HammerHead extends HttpServlet {
hs.setAttribute(WebSession.SESSION, session);
// reset timeout
hs.setMaxInactiveInterval(sessionTimeoutSeconds);
-
}
+ session.update(request, response, this.getServletName());
// update last attack request info (cookies, parms)
// this is so the REST services can have access to them via the session
session.updateLastAttackRequestInfo(request);
- session.update(request, response, this.getServletName());
// to authenticate
logger.debug("HH Leaving Session_id: " + hs.getId());
diff --git a/java/org/owasp/webgoat/service/BaseService.java b/java/org/owasp/webgoat/service/BaseService.java
index ea067cfaa..94513d480 100644
--- a/java/org/owasp/webgoat/service/BaseService.java
+++ b/java/org/owasp/webgoat/service/BaseService.java
@@ -67,8 +67,11 @@ public abstract class BaseService {
public WebSession getWebSession(HttpSession session) {
WebSession ws;
Object o = session.getAttribute(WebSession.SESSION);
- if (o == null || !(o instanceof WebSession)) {
- throw new IllegalArgumentException("No valid session object found, has session timed out? [" + session.getId() + "]");
+ if (o == null) {
+ throw new IllegalArgumentException("No valid WebSession object found, has session timed out? [" + session.getId() + "]");
+ }
+ if (!(o instanceof WebSession)) {
+ throw new IllegalArgumentException("Invalid WebSession object found, this is probably a bug! [" + o.getClass() + " | " + session.getId() + "]");
}
ws = (WebSession) o;
return ws;
diff --git a/java/org/owasp/webgoat/session/WebSession.java b/java/org/owasp/webgoat/session/WebSession.java
index e5729c252..a9c92896f 100644
--- a/java/org/owasp/webgoat/session/WebSession.java
+++ b/java/org/owasp/webgoat/session/WebSession.java
@@ -782,6 +782,8 @@ public class WebSession {
// System.out.println("Previous Screen 1: " + previousScreen );
// FIXME: requires ?Logout=true
// FIXME: doesn't work right -- no reauthentication
+ // REMOVED - we have explicit logout now via spriing security
+ /*
if (myParser.getRawParameter(LOGOUT, null) != null) {
System.out.println("Logout " + request.getUserPrincipal());
eatCookies();
@@ -789,6 +791,7 @@ public class WebSession {
currentScreen = WELCOME;
previousScreen = ERROR;
}
+ */
// There are several scenarios where we want the first lesson to be loaded
// 1) Previous screen is Welcome - Start of the course
diff --git a/webapp/META-INF/context.xml b/webapp/META-INF/context.xml
index 5bee3dc30..658058885 100644
--- a/webapp/META-INF/context.xml
+++ b/webapp/META-INF/context.xml
@@ -1,2 +1,2 @@
-
+
diff --git a/webapp/WEB-INF/mvc-dispatcher-servlet.xml b/webapp/WEB-INF/mvc-dispatcher-servlet.xml
index 3dd79b210..943637baa 100644
--- a/webapp/WEB-INF/mvc-dispatcher-servlet.xml
+++ b/webapp/WEB-INF/mvc-dispatcher-servlet.xml
@@ -1,50 +1,59 @@
-
+
-
+
-
-
+
+
-
-
+
+
-
-
+
+
-
-
-
+
+
+
+
+
+
+
+
+
+
+
+
-
+
\ No newline at end of file
diff --git a/webapp/WEB-INF/spring-security.xml b/webapp/WEB-INF/spring-security.xml
index 13d343b5e..50f249aa6 100644
--- a/webapp/WEB-INF/spring-security.xml
+++ b/webapp/WEB-INF/spring-security.xml
@@ -14,6 +14,9 @@
+
+
+
@@ -26,7 +29,8 @@
default-target-url="/welcome.mvc"
authentication-failure-url="/login.mvc?error"
username-parameter="username"
- password-parameter="password" />
+ password-parameter="password"
+ always-use-default-target="true"/>