fix: correct number of solved assignments in report card (#2065)

* fix: correct number of solved assignments in report card

Filter the list of assignments to accurately count the number of solved assignments.

Closes: gh-2063

* chore: remove scoreboard code

This is added when we run a CTF challenge during OWASP AppSecEU in 2017. We can remove this code.

Closes: gh-2064

src/it/java/org/owasp/webgoat/integration/ChallengeIntegrationTest.java

@@ -4,12 +4,9 @@
  */
 package org.owasp.webgoat.integration;
 
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
 import io.restassured.RestAssured;
 import java.util.Arrays;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import org.assertj.core.api.Assertions;
 import org.junit.jupiter.api.Test;
@@ -57,19 +54,6 @@ public class ChallengeIntegrationTest extends IntegrationTest {
     checkAssignment(webGoatUrlConfig.url("challenge/flag/1"), params, true);
 
     checkResults("Challenge1");
-
-      List<String> capturefFlags =
-        RestAssured.given()
-            .when()
-            .relaxedHTTPSValidation()
-            .cookie("JSESSIONID", getWebGoatCookie())
-            .get(webGoatUrlConfig.url("scoreboard-data"))
-            .then()
-            .statusCode(200)
-            .extract()
-            .jsonPath()
-            .get("find { it.username == \"" + this.getUser() + "\" }.flagsCaptured");
-    assertTrue(capturefFlags.contains("Admin lost password"));
   }
 
   @Test
@@ -99,19 +83,6 @@ public class ChallengeIntegrationTest extends IntegrationTest {
     checkAssignment(webGoatUrlConfig.url("challenge/flag/5"), params, true);
 
     checkResults("Challenge5");
-
-      List<String> capturefFlags =
-        RestAssured.given()
-            .when()
-            .relaxedHTTPSValidation()
-            .cookie("JSESSIONID", getWebGoatCookie())
-            .get(webGoatUrlConfig.url("scoreboard-data"))
-            .then()
-            .statusCode(200)
-            .extract()
-            .jsonPath()
-            .get("find { it.username == \"" + this.getUser() + "\" }.flagsCaptured");
-    assertTrue(capturefFlags.contains("Without password"));
   }
 
   @Test
@@ -162,7 +133,9 @@ public class ChallengeIntegrationTest extends IntegrationTest {
             .when()
             .relaxedHTTPSValidation()
             .cookie("JSESSIONID", getWebGoatCookie())
-            .get(webGoatUrlConfig.url("challenge/7/reset-password/{link}"), "375afe1104f4a487a73823c50a9292a2")
+            .get(
+                webGoatUrlConfig.url("challenge/7/reset-password/{link}"),
+                "375afe1104f4a487a73823c50a9292a2")
             .then()
             .statusCode(HttpStatus.ACCEPTED.value())
             .extract()

src/main/java/org/owasp/webgoat/container/MvcConfiguration.java

@@ -53,7 +53,6 @@ public class MvcConfiguration implements WebMvcConfigurer {
     registry.addViewController("/login").setViewName("login");
     registry.addViewController("/lesson_content").setViewName("lesson_content");
     registry.addViewController("/start.mvc").setViewName("main_new");
-    registry.addViewController("/scoreboard").setViewName("scoreboard");
   }
 
   @Bean

src/main/java/org/owasp/webgoat/container/users/LessonProgress.java

@@ -84,6 +84,6 @@ public class LessonProgress {
   }
 
   long numberOfSolvedAssignments() {
-    return assignments.size();
+    return assignments.stream().filter(AssignmentProgress::isSolved).count();
   }
 }

src/main/java/org/owasp/webgoat/container/users/Scoreboard.java

@@ -1,83 +0,0 @@
-/*
- * SPDX-FileCopyrightText: Copyright © 2017 WebGoat authors
- * SPDX-License-Identifier: GPL-2.0-or-later
- */
-package org.owasp.webgoat.container.users;
-
-import java.util.List;
-import java.util.Optional;
-import java.util.stream.Collectors;
-import lombok.AllArgsConstructor;
-import lombok.Getter;
-import org.owasp.webgoat.container.i18n.PluginMessages;
-import org.owasp.webgoat.container.lessons.Lesson;
-import org.owasp.webgoat.container.session.Course;
-import org.springframework.web.bind.annotation.GetMapping;
-import org.springframework.web.bind.annotation.RestController;
-
-/**
- * Temp endpoint just for the CTF.
- *
- * @author nbaars
- * @since 3/23/17.
- */
-@RestController
-@AllArgsConstructor
-public class Scoreboard {
-
-  private final UserProgressRepository userTrackerRepository;
-  private final UserRepository userRepository;
-  private final Course course;
-  private final PluginMessages pluginMessages;
-
-  @AllArgsConstructor
-  @Getter
-  private class Ranking {
-    private String username;
-    private List<String> flagsCaptured;
-  }
-
-  @GetMapping("/scoreboard-data")
-  public List<Ranking> getRankings() {
-    return userRepository.findAll().stream()
-        .filter(user -> !user.getUsername().startsWith("csrf-"))
-        .map(
-            user ->
-                new Ranking(
-                    user.getUsername(),
-                    challengesSolved(userTrackerRepository.findByUser(user.getUsername()))))
-        .sorted((o1, o2) -> o2.getFlagsCaptured().size() - o1.getFlagsCaptured().size())
-        .collect(Collectors.toList());
-  }
-
-  private List<String> challengesSolved(UserProgress userTracker) {
-    List<String> challenges =
-        List.of(
-            "Challenge1",
-            "Challenge2",
-            "Challenge3",
-            "Challenge4",
-            "Challenge5",
-            "Challenge6",
-            "Challenge7",
-            "Challenge8",
-            "Challenge9");
-    return challenges.stream()
-        .map(userTracker::getLessonProgress)
-        .flatMap(Optional::stream)
-        .filter(LessonProgress::isLessonSolved)
-        .map(LessonProgress::getLessonName)
-        .map(this::toLessonTitle)
-        .toList();
-  }
-
-  private String toLessonTitle(String id) {
-    String titleKey =
-        course.getLessons().stream()
-            .filter(l -> l.getId().equals(id))
-            .findFirst()
-            .map(Lesson::getTitle)
-            .orElse("No title");
-    return pluginMessages.getMessage(titleKey, titleKey);
-  }
-}

src/main/java/org/owasp/webgoat/container/users/UserProgress.java

@@ -41,7 +41,7 @@ public class UserProgress {
   }
 
   /**
-   * Returns an existing lesson tracker or create a new one based on the lesson
+   * Returns an existing lesson progress or create a new one based on the lesson
    *
    * @param lesson the lesson
    * @return a lesson tracker created if not already present
@@ -49,7 +49,7 @@ public class UserProgress {
   public LessonProgress getLessonProgress(Lesson lesson) {
     Optional<LessonProgress> progress =
         lessonProgress.stream().filter(l -> l.getLessonName().equals(lesson.getId())).findFirst();
-    if (!progress.isPresent()) {
+    if (progress.isEmpty()) {
       LessonProgress newLessonTracker = new LessonProgress(lesson);
       lessonProgress.add(newLessonTracker);
       return newLessonTracker;
@@ -58,16 +58,6 @@ public class UserProgress {
     }
   }
 
-  /**
-   * Query method for finding a specific lesson tracker based on id
-   *
-   * @param id the id of the lesson
-   * @return optional due to the fact we can only create a lesson tracker based on a lesson
-   */
-  public Optional<LessonProgress> getLessonProgress(String id) {
-    return lessonProgress.stream().filter(l -> l.getLessonName().equals(id)).findFirst();
-  }
-
   public void assignmentSolved(Lesson lesson, String assignmentName) {
     LessonProgress progress = getLessonProgress(lesson);
     progress.incrementAttempts();

src/main/resources/lessons/challenges/documentation/Challenge_introduction.adoc

@@ -4,7 +4,7 @@
 
 The challenges contain more a CTF like lessons where we do not provide any explanations what you need to do, no hints
 will be provided. You can use these challenges in a CTF style where you can run WebGoat on one server and all
-participants can join and hack the challenges. A scoreboard is available at link:scoreboard["scoreboard",window=_blank]
+participants can join and hack the challenges.
 
 :hardbreaks:
 In this CTF you will need to solve a couple of challenges, each challenge will give you a flag which you will

src/main/resources/webgoat/static/css/main.css

@@ -1172,46 +1172,10 @@ span.show-next-page, span.show-prev-page {
     width: 95% !important
 }
 
-/* scoreboard */
-div.scoreboard-title {
-    font-size: xx-large;
-}
-
-.scoreboard-table tr {
-}
-
-div.scoreboard-username {
-    background-color: #222;
-    color: aliceblue;
-    padding: 4px;
-    padding-left: 8px;
-    font-size: medium;
-    border-radius: 6px;
-}
-
 th.username {
     padding-bottom: 6px;
 }
 
-td.user-flags {
-    padding-left: 8px;
-    padding-bottom: 6px;
-}
-
-div.captured-flag {
-    border-radius: 6px;
-    background-color: #444;
-    color: white;
-    padding: 4px;
-    font-size: medium;
-    display: inline-block;
-}
-
-.scoreboard-page {
-    background-color: #e0dfdc;
-    padding: 20px;
-}
-
 .fa-flag {
     color: red
 }

src/main/resources/webgoat/static/js/goatApp/model/FlagModel.js

@@ -1,9 +0,0 @@
-define(['jquery',
-	'underscore',
-	'backbone'],
-	function($,
-		_,
-		Backbone) {
-	return Backbone.Model.extend({
-	});
-});

src/main/resources/webgoat/static/js/goatApp/model/FlagsCollection.js

@@ -1,13 +0,0 @@
-define(['jquery',
-	'underscore',
-	'backbone',
-	'goatApp/model/FlagModel'],
-	function($,
-		_,
-		Backbone,
-		FlagModel) {
-	return Backbone.Collection.extend({
-		url:'scoreboard-data',
-		model:FlagModel
-	});
-});

src/main/resources/webgoat/static/js/goatApp/scoreboardApp.js

@@ -1,16 +0,0 @@
-define(['underscore',
-        'goatApp/support/goatAsyncErrorHandler',
-        'goatApp/view/ScoreboardView'],
-    function (
-         _,
-         asyncErrorHandler,
-         ScoreboardView) {
-        'use strict'
-        class ScoreboardApp {
-            initApp() {
-                asyncErrorHandler.init();
-                this.scoreboard = new ScoreboardView();
-            }
-        }
-        return new ScoreboardApp();
-    });

src/main/resources/webgoat/static/js/goatApp/templates/scoreboard.html

@@ -1,14 +0,0 @@
-<table class="scoreboard-table">
-    <% _.each(rankings, function(userRanking, index) { %>
-    <tr>
-        <th class="username"> <div class="scoreboard-username"><%= index+1%> - <%=userRanking.username %> </div></th>
-        <td class="user-flags"> <% _.each(userRanking.flagsCaptured, function(flag) { %>
-
-             <div class="captured-flag">
-                 <i class="fa fa-flag" aria-hidden="true"></i>
-                 <%=flag%> </div>
-            <% }); %>
-        </td>
-    </tr>
-    <% }); %>
-</table>

src/main/resources/webgoat/static/js/goatApp/view/ScoreboardView.js

@@ -1,32 +0,0 @@
-define(['jquery',
-	'underscore',
-	'backbone',
-	'goatApp/model/FlagsCollection',
-	'text!templates/scoreboard.html'],
-function($,
-	_,
-	Backbone,
-	FlagsCollection,
-	ScoreboardTemplate) {
-	return Backbone.View.extend({
-		el:'#scoreboard',
-
-		initialize: function() {
-		    this.template = ScoreboardTemplate,
-		    this.collection = new FlagsCollection();
-		    this.listenTo(this.collection,'reset',this.render)
-		    this.collection.fetch({reset:true});
-		},
-
-		render: function() {
-			//this.$el.html('test');
-			var t = _.template(this.template);
-            this.$el.html(t({'rankings':this.collection.toJSON()}));
-            setTimeout(this.pollData.bind(this), 5000);
-		},
-
-		pollData: function() {
-		    this.collection.fetch({reset:true});
-		}
-	});
-});

src/main/resources/webgoat/static/js/scoreboard.js

@@ -1,44 +0,0 @@
-//main.js
-/*
-/js
-js/main.js << main file for require.js
---/libs/(jquery,backbone,etc.) << base libs
---/goatApp/ << base dir for goat application, js-wise
---/goatApp/model
---/goatApp/view
---/goatApp/support
---/goatApp/controller
-*/
-
-require.config({
-  baseUrl: "js/",
-  paths: {
-    jquery: 'libs/jquery.min',
-    jqueryvuln: 'libs/jquery-2.1.4.min',
-    jqueryuivuln: 'libs/jquery-ui-1.10.4',
-    jqueryui: 'libs/jquery-ui.min',
-    underscore: 'libs/underscore-min',
-    backbone: 'libs/backbone-min',
-    text: 'libs/text',
-    templates: 'goatApp/templates',
-    polyglot: 'libs/polyglot.min'
-  },
-
-  shim: {
-	"jqueryui": {
-	  exports:"$",
-	  deps: ['jquery']
-	},
-    underscore: {
-      exports: "_"
-    },
-    backbone: {
-      deps: ['underscore', 'jquery'],
-      exports: 'Backbone'
-    }
-  }
-});
-
-require(['underscore','backbone','goatApp/scoreboardApp'], function(_,Backbone,ScoreboardApp){
-    ScoreboardApp.initApp();
-});

src/main/resources/webgoat/templates/scoreboard.html

@@ -1,55 +0,0 @@
-<!DOCTYPE html>
-<html xmlns="http://www.w3.org/1999/xhtml" xmlns:th="http://www.thymeleaf.org"
-      xmlns:sec="http://www.thymeleaf.org/extras/spring-security">
-<head>
-    <meta http-equiv="Expires" CONTENT="-1"/>
-    <meta http-equiv="Pragma" CONTENT="no-cache"/>
-    <meta http-equiv="Cache-Control" CONTENT="no-cache"/>
-    <meta http-equiv="Cache-Control" CONTENT="no-store"/>
-
-    <!--  CSS -->
-    <link rel="shortcut icon" th:href="@{/css/img/favicon.ico}" type="image/x-icon"/>
-
-    <!-- Require.js used to load js asynchronously -->
-    <script src="js/libs/require.min.js" data-main="js/scoreboard.js"></script>
-    <!-- main css -->
-    <link rel="stylesheet" type="text/css" th:href="@{/css/main.css}"/>
-    <link rel="stylesheet" type="text/css" th:href="@{/plugins/bootstrap/css/bootstrap.min.css}"/>
-    <link rel="stylesheet" type="text/css" th:href="@{/css/font-awesome.min.css}"/>
-
-    <meta http-equiv="Content-Type" content="text/id; charset=ISO-8859-1"/>
-    <title>WebGoat</title>
-</head>
-<!--  <body class="scoreboard-page"> -->
-<body>
-<header id="header">
-        <!--logo start-->
-        <div class="brand">
-            <a th:href="@{/welcome.mvc}" class="logo"><span>Web</span>Goat</a>
-        </div>
-        <!--logo end-->
-        <div id="lesson-title-wrapper">
-			<h1 id="lesson-title">WebGoat challenges ranking</h1>
-        </div><!--lesson title end-->
-        <div class="user-nav pull-right" id="user-and-info-nav" style="margin-right: 75px;">
-        </div>
-    </header>
-<section id="container">
-<!--main content start-->
-    <section class="main-content-wrapper">
-        <section id="main-content">
-
-<div id="scoreboard-wrapper">
-    <div id="scoreboard">
-        <!-- will use _ template here -->
-    </div>
-</div>
-
-</section>
-</section>
-</section>
-
-</body>
-
-
-</html>

src/test/java/org/owasp/webgoat/container/users/UserProgressRepositoryTest.java

@@ -0,0 +1,89 @@
+/*
+ * SPDX-FileCopyrightText: Copyright © 2017 WebGoat authors
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+package org.owasp.webgoat.container.users;
+
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.List;
+import org.assertj.core.util.Lists;
+import org.junit.jupiter.api.Test;
+import org.owasp.webgoat.container.lessons.Assignment;
+import org.owasp.webgoat.container.lessons.Category;
+import org.owasp.webgoat.container.lessons.Lesson;
+import org.springframework.beans.factory.annotation.Autowired;
+import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
+import org.springframework.test.context.ActiveProfiles;
+
+@DataJpaTest
+@ActiveProfiles("webgoat-test")
+class UserProgressRepositoryTest {
+
+  private static class TestLesson extends Lesson {
+
+    @Override
+    public Category getDefaultCategory() {
+      return Category.CLIENT_SIDE;
+    }
+
+    @Override
+    public String getTitle() {
+      return "test";
+    }
+
+    @Override
+    public List<Assignment> getAssignments() {
+      var assignment1 = new Assignment("test1", "test1", Lists.newArrayList());
+      var assignment2 = new Assignment("test2", "test2", Lists.newArrayList());
+
+      return List.of(assignment1, assignment2);
+    }
+  }
+
+  private static final String USER = "user";
+  @Autowired private UserProgressRepository userProgressRepository;
+
+  @Test
+  void saveUserTracker() {
+    var userProgress = new UserProgress(USER);
+    userProgressRepository.save(userProgress);
+
+    userProgress = userProgressRepository.findByUser(USER);
+
+    assertThat(userProgress.getLessonProgress(new TestLesson())).isNotNull();
+  }
+
+  @Test
+  void solvedAssignmentsShouldBeSaved() {
+    var userProgress = new UserProgress(USER);
+    var lesson = new TestLesson();
+    userProgress.getLessonProgress(lesson);
+    userProgress.assignmentFailed(lesson);
+    userProgress.assignmentFailed(lesson);
+    userProgress.assignmentSolved(lesson, "test1");
+    userProgress.assignmentSolved(lesson, "test2");
+    userProgressRepository.saveAndFlush(userProgress);
+
+    userProgress = userProgressRepository.findByUser(USER);
+
+    assertThat(userProgress.numberOfAssignmentsSolved()).isEqualTo(2);
+  }
+
+  @Test
+  void saveAndLoadShouldHaveCorrectNumberOfAttempts() {
+    UserProgress userProgress = new UserProgress(USER);
+    TestLesson lesson = new TestLesson();
+    userProgress.getLessonProgress(lesson);
+    userProgress.assignmentFailed(lesson);
+    userProgress.assignmentFailed(lesson);
+    userProgressRepository.saveAndFlush(userProgress);
+
+    userProgress = userProgressRepository.findByUser(USER);
+    userProgress.assignmentFailed(lesson);
+    userProgress.assignmentFailed(lesson);
+    userProgressRepository.saveAndFlush(userProgress);
+
+    assertThat(userProgress.getLessonProgress(lesson).getNumberOfAttempts()).isEqualTo(4);
+  }
+}

src/test/java/org/owasp/webgoat/container/users/UserTrackerRepositoryTest.java

@@ -1,84 +0,0 @@
-/*
- * SPDX-FileCopyrightText: Copyright © 2017 WebGoat authors
- * SPDX-License-Identifier: GPL-2.0-or-later
- */
-package org.owasp.webgoat.container.users;
-
-import java.util.List;
-import org.assertj.core.api.Assertions;
-import org.assertj.core.util.Lists;
-import org.junit.jupiter.api.Test;
-import org.owasp.webgoat.container.lessons.Assignment;
-import org.owasp.webgoat.container.lessons.Category;
-import org.owasp.webgoat.container.lessons.Lesson;
-import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.test.autoconfigure.orm.jpa.DataJpaTest;
-import org.springframework.test.context.ActiveProfiles;
-
-@DataJpaTest
-@ActiveProfiles("webgoat-test")
-class UserTrackerRepositoryTest {
-
-  private class TestLesson extends Lesson {
-
-    @Override
-    public Category getDefaultCategory() {
-      return Category.CLIENT_SIDE;
-    }
-
-    @Override
-    public String getTitle() {
-      return "test";
-    }
-
-    @Override
-    public List<Assignment> getAssignments() {
-      Assignment assignment = new Assignment("test", "test", Lists.newArrayList());
-      return Lists.newArrayList(assignment);
-    }
-  }
-
-  @Autowired private UserProgressRepository userTrackerRepository;
-
-  @Test
-  void saveUserTracker() {
-    UserProgress userTracker = new UserProgress("test");
-
-    userTrackerRepository.save(userTracker);
-
-    userTracker = userTrackerRepository.findByUser("test");
-    Assertions.assertThat(userTracker.getLessonProgress("test")).isNotNull();
-  }
-
-  @Test
-  void solvedAssignmentsShouldBeSaved() {
-    UserProgress userTracker = new UserProgress("test");
-    TestLesson lesson = new TestLesson();
-    userTracker.getLessonProgress(lesson);
-    userTracker.assignmentFailed(lesson);
-    userTracker.assignmentFailed(lesson);
-    userTracker.assignmentSolved(lesson, "test");
-
-    userTrackerRepository.saveAndFlush(userTracker);
-
-    userTracker = userTrackerRepository.findByUser("test");
-    Assertions.assertThat(userTracker.numberOfAssignmentsSolved()).isEqualTo(1);
-  }
-
-  @Test
-  void saveAndLoadShouldHaveCorrectNumberOfAttempts() {
-    UserProgress userTracker = new UserProgress("test");
-    TestLesson lesson = new TestLesson();
-    userTracker.getLessonProgress(lesson);
-    userTracker.assignmentFailed(lesson);
-    userTracker.assignmentFailed(lesson);
-    userTrackerRepository.saveAndFlush(userTracker);
-
-    userTracker = userTrackerRepository.findByUser("test");
-    userTracker.assignmentFailed(lesson);
-    userTracker.assignmentFailed(lesson);
-    userTrackerRepository.saveAndFlush(userTracker);
-
-    Assertions.assertThat(userTracker.getLessonProgress(lesson).getNumberOfAttempts()).isEqualTo(4);
-  }
-}