From ddf6ac9bdbb8f478f8cbd6e7384a815997bb136a Mon Sep 17 00:00:00 2001
From: Nanne Baars <nanne.baars@owasp.org>
Date: Sun, 3 Nov 2019 18:27:03 +0100
Subject: [PATCH] Improve handling of missing parameters, now returns HTTP/401
 (#698)

---
 .../owasp/webgoat/jwt/JWTRefreshEndpoint.java | 31 +++++++++++++------
 .../webgoat/jwt/JWTRefreshEndpointTest.java   | 18 +++++++++++
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/webgoat-lessons/jwt/src/main/java/org/owasp/webgoat/jwt/JWTRefreshEndpoint.java b/webgoat-lessons/jwt/src/main/java/org/owasp/webgoat/jwt/JWTRefreshEndpoint.java
index 74979f359..537bf1253 100644
--- a/webgoat-lessons/jwt/src/main/java/org/owasp/webgoat/jwt/JWTRefreshEndpoint.java
+++ b/webgoat-lessons/jwt/src/main/java/org/owasp/webgoat/jwt/JWTRefreshEndpoint.java
@@ -35,6 +35,8 @@ import org.springframework.web.bind.annotation.*;
 import java.util.*;
 import java.util.concurrent.TimeUnit;
 
+import static org.springframework.http.ResponseEntity.ok;
+
 /**
  * @author nbaars
  * @since 4/23/17.
@@ -49,12 +51,15 @@ public class JWTRefreshEndpoint extends AssignmentEndpoint {
 
     @PostMapping(value = "/JWT/refresh/login", consumes = MediaType.APPLICATION_JSON_VALUE, produces = MediaType.APPLICATION_JSON_VALUE)
     @ResponseBody
-    public ResponseEntity follow(@RequestBody Map<String, Object> json) {
+    public ResponseEntity follow(@RequestBody(required = false) Map<String, Object> json) {
+        if (json == null) {
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
         String user = (String) json.get("user");
         String password = (String) json.get("password");
 
         if ("Jerry".equals(user) && PASSWORD.equals(password)) {
-            return ResponseEntity.ok(createNewTokens(user));
+            return ok(createNewTokens(user));
         }
         return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
     }
@@ -78,25 +83,33 @@ public class JWTRefreshEndpoint extends AssignmentEndpoint {
 
     @PostMapping("/JWT/refresh/checkout")
     @ResponseBody
-    public AttackResult checkout(@RequestHeader("Authorization") String token) {
+    public ResponseEntity<?> checkout(@RequestHeader(value = "Authorization", required = false) String token) {
+        if (token == null) {
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
         try {
             Jwt jwt = Jwts.parser().setSigningKey(JWT_PASSWORD).parse(token.replace("Bearer ", ""));
             Claims claims = (Claims) jwt.getBody();
             String user = (String) claims.get("user");
             if ("Tom".equals(user)) {
-                return trackProgress(success().build());
+                return ok(trackProgress(success().build()));
             }
-            return trackProgress(failed().feedback("jwt-refresh-not-tom").feedbackArgs(user).build());
+            return ok(trackProgress(failed().feedback("jwt-refresh-not-tom").feedbackArgs(user).build()));
         } catch (ExpiredJwtException e) {
-            return trackProgress(failed().output(e.getMessage()).build());
+            return ok(trackProgress(failed().output(e.getMessage()).build()));
         } catch (JwtException e) {
-            return trackProgress(failed().feedback("jwt-invalid-token").build());
+            return ok(trackProgress(failed().feedback("jwt-invalid-token").build()));
         }
     }
 
     @PostMapping("/JWT/refresh/newToken")
     @ResponseBody
-    public ResponseEntity newToken(@RequestHeader("Authorization") String token, @RequestBody Map<String, Object> json) {
+    public ResponseEntity newToken(@RequestHeader(value = "Authorization", required = false) String token,
+                                   @RequestBody(required = false) Map<String, Object> json) {
+        if (token == null || json == null) {
+            return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
+        }
+
         String user;
         String refreshToken;
         try {
@@ -112,7 +125,7 @@ public class JWTRefreshEndpoint extends AssignmentEndpoint {
             return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
         } else if (validRefreshTokens.contains(refreshToken)) {
             validRefreshTokens.remove(refreshToken);
-            return ResponseEntity.ok(createNewTokens(user));
+            return ok(createNewTokens(user));
         } else {
             return ResponseEntity.status(HttpStatus.UNAUTHORIZED).build();
         }
diff --git a/webgoat-lessons/jwt/src/test/java/org/owasp/webgoat/jwt/JWTRefreshEndpointTest.java b/webgoat-lessons/jwt/src/test/java/org/owasp/webgoat/jwt/JWTRefreshEndpointTest.java
index 89d368763..0b66fb49b 100644
--- a/webgoat-lessons/jwt/src/test/java/org/owasp/webgoat/jwt/JWTRefreshEndpointTest.java
+++ b/webgoat-lessons/jwt/src/test/java/org/owasp/webgoat/jwt/JWTRefreshEndpointTest.java
@@ -195,4 +195,22 @@ public class JWTRefreshEndpointTest extends LessonTest {
                 .content(objectMapper.writeValueAsString(refreshJson)))
                 .andExpect(status().isUnauthorized());
     }
+
+    @Test
+    public void noTokenWhileCheckoutShouldReturn401() throws Exception {
+        mockMvc.perform(MockMvcRequestBuilders.post("/JWT/refresh/checkout"))
+                .andExpect(status().isUnauthorized());
+    }
+
+    @Test
+    public void noTokenWhileRequestingNewTokenShouldReturn401() throws Exception {
+        mockMvc.perform(MockMvcRequestBuilders.post("/JWT/refresh/newToken"))
+                .andExpect(status().isUnauthorized());
+    }
+
+    @Test
+    public void noTokenWhileLoginShouldReturn401() throws Exception {
+        mockMvc.perform(MockMvcRequestBuilders.post("/JWT/refresh/login"))
+                .andExpect(status().isUnauthorized());
+    }
 }
\ No newline at end of file