Skip to content

Commit 5b2a849

Browse files
author
lawson89
committed
add logging and cleanup for course loading
detect is session is setup correctly when hitting start.mvcand if not redirect to login
1 parent 9b453ed commit 5b2a849

File tree

2 files changed

+63
-47
lines changed

2 files changed

+63
-47
lines changed

‎java/org/owasp/webgoat/controller/Start.java‎

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@
66
package org.owasp.webgoat.controller;
77

88
import javax.servlet.http.HttpServletRequest;
9+
import javax.servlet.http.HttpSession;
10+
import org.owasp.webgoat.session.Course;
11+
import org.owasp.webgoat.session.WebSession;
12+
import org.slf4j.Logger;
13+
import org.slf4j.LoggerFactory;
914
import org.springframework.stereotype.Controller;
1015
import org.springframework.web.bind.annotation.RequestMapping;
1116
import org.springframework.web.bind.annotation.RequestMethod;
@@ -19,18 +24,38 @@
1924
@Controller
2025
public class Start {
2126

27+
final Logger logger = LoggerFactory.getLogger(Start.class);
28+
2229
private static final String WELCOMED = "welcomed";
2330

2431
@RequestMapping(value = "start.mvc", method = {RequestMethod.GET, RequestMethod.POST})
2532
public ModelAndView start(HttpServletRequest request,
2633
@RequestParam(value = "error", required = false) String error,
2734
@RequestParam(value = "logout", required = false) String logout) {
2835

29-
//@TODO put stuff here the main page needs to access
3036
ModelAndView model = new ModelAndView();
31-
model.setViewName("main_new");
37+
// make sure session is set up correctly
38+
// if not redirect user to login
39+
if (checkWebSession(request.getSession()) == false) {
40+
model.setViewName("redirect:/login.mvc");
41+
return model;
42+
}
3243

44+
// if everything ok then go to webgoat UI
45+
model.setViewName("main_new");
3346
return model;
47+
}
3448

49+
public boolean checkWebSession(HttpSession session) {
50+
Object o = session.getAttribute(WebSession.SESSION);
51+
if (o == null) {
52+
logger.error("No valid WebSession object found, has session timed out? [" + session.getId() + "]");
53+
return false;
54+
}
55+
if (!(o instanceof WebSession)) {
56+
logger.error("Invalid WebSession object found, this is probably a bug! [" + o.getClass() + " | " + session.getId() + "]");
57+
return false;
58+
}
59+
return true;
3560
}
3661
}

‎java/org/owasp/webgoat/session/Course.java‎

Lines changed: 36 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,12 @@
77
import java.util.Iterator;
88
import java.util.List;
99
import java.util.Set;
10-
import java.util.Vector;
1110
import java.util.LinkedList;
1211
import javax.servlet.ServletContext;
1312
import org.owasp.webgoat.HammerHead;
1413
import org.owasp.webgoat.lessons.AbstractLesson;
1514
import org.owasp.webgoat.lessons.Category;
15+
import org.owasp.webgoat.util.WebGoatI18N;
1616
import org.slf4j.Logger;
1717
import org.slf4j.LoggerFactory;
1818

@@ -51,15 +51,15 @@
5151
*/
5252
public class Course {
5353

54-
final Logger logger = LoggerFactory.getLogger(WebgoatProperties.class);
54+
final Logger logger = LoggerFactory.getLogger(Course.class);
5555

56-
private List<AbstractLesson> lessons = new LinkedList<AbstractLesson>();
56+
private final List<AbstractLesson> lessons = new LinkedList<AbstractLesson>();
5757

5858
private final static String PROPERTIES_FILENAME = HammerHead.propertiesPath;
5959

6060
private WebgoatProperties properties = null;
6161

62-
private List<String> files = new LinkedList<String>();
62+
private final List<String> files = new LinkedList<String>();
6363

6464
private WebgoatContext webgoatContext;
6565

@@ -82,11 +82,11 @@ public Course() {
8282
private static String getFileName(String s) {
8383
String fileName = new File(s).getName();
8484

85-
if (fileName.indexOf("/") != -1) {
85+
if (fileName.contains("/")) {
8686
fileName = fileName.substring(fileName.lastIndexOf("/"), fileName.length());
8787
}
8888

89-
if (fileName.indexOf(".") != -1) {
89+
if (fileName.contains(".")) {
9090
fileName = fileName.substring(0, fileName.indexOf("."));
9191
}
9292

@@ -102,7 +102,7 @@ private static String getFileName(String s) {
102102
* @return
103103
*/
104104
private static String getSourceFile(String className) {
105-
StringBuffer sb = new StringBuffer();
105+
StringBuilder sb = new StringBuilder();
106106

107107
sb.append(className.replace(".", "/"));
108108
sb.append(".java");
@@ -150,11 +150,7 @@ private static String getClassFile(String fileName, String path) {
150150
*/
151151
public List getCategories() {
152152
List<Category> categories = new ArrayList<Category>();
153-
Iterator iter = lessons.iterator();
154-
155-
while (iter.hasNext()) {
156-
AbstractLesson lesson = (AbstractLesson) iter.next();
157-
153+
for (AbstractLesson lesson : lessons) {
158154
if (!categories.contains(lesson.getCategory())) {
159155
categories.add(lesson.getCategory());
160156
}
@@ -181,8 +177,9 @@ public AbstractLesson getFirstLesson() {
181177
/**
182178
* Gets the lesson attribute of the Course object
183179
*
180+
* @param s
184181
* @param lessonId Description of the Parameter
185-
* @param role Description of the Parameter
182+
* @param roles
186183
* @return The lesson value
187184
*/
188185
public AbstractLesson getLesson(WebSession s, int lessonId, List<String> roles) {
@@ -205,21 +202,22 @@ public AbstractLesson getLesson(WebSession s, int lessonId, List<String> roles)
205202
}
206203

207204
public AbstractLesson getLesson(WebSession s, int lessonId, String role) {
208-
List<String> roles = new Vector<String>();
205+
List<String> roles = new ArrayList<String>();
209206
roles.add(role);
210207
return getLesson(s, lessonId, roles);
211208
}
212209

213210
public List getLessons(WebSession s, String role) {
214-
List<String> roles = new Vector<String>();
211+
List<String> roles = new ArrayList<String>();
215212
roles.add(role);
216213
return getLessons(s, roles);
217214
}
218215

219216
/**
220217
* Gets the lessons attribute of the Course object
221218
*
222-
* @param role Description of the Parameter
219+
* @param s
220+
* @param roles
223221
* @return The lessons value
224222
*/
225223
public List<AbstractLesson> getLessons(WebSession s, List<String> roles) {
@@ -245,10 +243,7 @@ public List<AbstractLesson> getLessons(WebSession s, List<String> roles) {
245243
private List<AbstractLesson> getLessons(Category category, List roles) {
246244
List<AbstractLesson> lessonList = new ArrayList<AbstractLesson>();
247245

248-
Iterator iter = lessons.iterator();
249-
while (iter.hasNext()) {
250-
AbstractLesson lesson = (AbstractLesson) iter.next();
251-
246+
for (AbstractLesson lesson : lessons) {
252247
if (lesson.getCategory().equals(category) && roles.contains(lesson.getRole())) {
253248
lessonList.add(lesson);
254249
}
@@ -260,7 +255,7 @@ private List<AbstractLesson> getLessons(Category category, List roles) {
260255
}
261256

262257
public List getLessons(WebSession s, Category category, String role) {
263-
List<String> roles = new Vector<String>();
258+
List<String> roles = new ArrayList<String>();
264259
roles.add(role);
265260
return getLessons(s, category, roles);
266261
}
@@ -288,7 +283,12 @@ public AbstractLesson getLesson(int lessonId) {
288283
* @param path
289284
*/
290285
private void loadFiles(ServletContext context, String path) {
286+
logger.debug("Loading files into cache, path: " + path);
291287
Set resourcePaths = context.getResourcePaths(path);
288+
if (resourcePaths == null) {
289+
logger.error("Unable to load file cache for courses, this is probably a bug or configuration issue");
290+
return;
291+
}
292292
Iterator itr = resourcePaths.iterator();
293293

294294
while (itr.hasNext()) {
@@ -308,10 +308,7 @@ private void loadFiles(ServletContext context, String path) {
308308
* @param path
309309
*/
310310
private void loadLessons(String path) {
311-
Iterator itr = files.iterator();
312-
313-
while (itr.hasNext()) {
314-
String file = (String) itr.next();
311+
for (String file : files) {
315312
String className = getClassFile(file, path);
316313

317314
if (className != null && !className.endsWith("_i")) {
@@ -330,7 +327,7 @@ private void loadLessons(String path) {
330327
}
331328
}
332329
} catch (Exception e) {
333-
// System.out.println("Warning: " + e.getMessage());
330+
logger.error("Error in loadLessons: ", e);
334331
}
335332
}
336333
}
@@ -341,26 +338,20 @@ private String getLanguageFromFileName(String first, String absoluteFile) {
341338
int p2 = absoluteFile.indexOf("/", p1 + 1);
342339
String langStr = absoluteFile.substring(p1 + 1, p2);
343340

344-
return new String(langStr);
341+
return langStr;
345342
}
346343

347344
/**
348345
* For each lesson, set the source file and lesson file
349346
*/
350347
private void loadResources() {
351-
Iterator lessonItr = lessons.iterator();
352-
353-
while (lessonItr.hasNext()) {
354-
AbstractLesson lesson = (AbstractLesson) lessonItr.next();
348+
for (AbstractLesson lesson : lessons) {
355349
String className = lesson.getClass().getName();
356350
String classFile = getSourceFile(className);
357351

358-
Iterator fileItr = files.iterator();
359-
360-
while (fileItr.hasNext()) {
361-
String absoluteFile = (String) fileItr.next();
352+
for (String absoluteFile : files) {
362353
String fileName = getFileName(absoluteFile);
363-
// System.out.println("Course: looking at file: " + absoluteFile);
354+
logger.debug("Course: looking at file: " + absoluteFile);
364355

365356
if (absoluteFile.endsWith(classFile)) {
366357
// System.out.println("Set source file for " + classFile);
@@ -369,20 +360,18 @@ private void loadResources() {
369360

370361
if (absoluteFile.startsWith("/lesson_plans") && absoluteFile.endsWith(".html")
371362
&& className.endsWith(fileName)) {
372-
// System.out.println("DEBUG: setting lesson plan file " + absoluteFile + " for
373-
// lesson " +
374-
// lesson.getClass().getName());
375-
// System.out.println("fileName: " + fileName + " == className: " + className );
363+
logger.debug("DEBUG: setting lesson plan file " + absoluteFile + " for lesson "
364+
+ lesson.getClass().getName());
365+
logger.debug("fileName: " + fileName + " == className: " + className);
376366
String language = getLanguageFromFileName("/lesson_plans", absoluteFile);
377367
lesson.setLessonPlanFileName(language, absoluteFile);
378-
this.webgoatContext.getWebgoatI18N().loadLanguage(language);
368+
WebGoatI18N.loadLanguage(language);
379369
}
380370
if (absoluteFile.startsWith("/lesson_solutions") && absoluteFile.endsWith(".html")
381371
&& className.endsWith(fileName)) {
382-
// System.out.println("DEBUG: setting lesson solution file " + absoluteFile + "
383-
// for lesson " +
384-
// lesson.getClass().getName());
385-
// System.out.println("fileName: " + fileName + " == className: " + className );
372+
logger.debug("DEBUG: setting lesson solution file " + absoluteFile + " for lesson "
373+
+ lesson.getClass().getName());
374+
logger.debug("fileName: " + fileName + " == className: " + className);
386375
lesson.setLessonSolutionFileName(absoluteFile);
387376
}
388377
}
@@ -392,10 +381,12 @@ private void loadResources() {
392381
/**
393382
* Description of the Method
394383
*
384+
* @param webgoatContext
395385
* @param path Description of the Parameter
396386
* @param context Description of the Parameter
397387
*/
398388
public void loadCourses(WebgoatContext webgoatContext, ServletContext context, String path) {
389+
logger.info("Loading courses: " + path);
399390
this.webgoatContext = webgoatContext;
400391
loadFiles(context, path);
401392
loadLessons(path);

0 commit comments

Comments
 (0)