From ccb42d2311f7a6a06328204913c8d6fcdd947e83 Mon Sep 17 00:00:00 2001 From: "Peter Marsh (He/Him)" Date: Tue, 31 Jan 2023 10:17:14 +0000 Subject: [PATCH 1/2] removed username from newOrderParams --- .../example/demo/matcher/MatcherController.java | 14 +++++++------- .../demo/matcher/models/NewOrderParams.java | 1 - .../{MakeOrderReturn.java => NewOrderReturn.java} | 2 +- .../PrivateEndpointsWithTokenTest.java | 3 +-- 4 files changed, 9 insertions(+), 11 deletions(-) rename src/main/java/com/example/demo/matcher/models/{MakeOrderReturn.java => NewOrderReturn.java} (94%) diff --git a/src/main/java/com/example/demo/matcher/MatcherController.java b/src/main/java/com/example/demo/matcher/MatcherController.java index df4eefc..9056bcf 100644 --- a/src/main/java/com/example/demo/matcher/MatcherController.java +++ b/src/main/java/com/example/demo/matcher/MatcherController.java @@ -92,28 +92,28 @@ public ResponseEntity> tradebook() { } @PostMapping(value="/private/make/order") - public ResponseEntity makeOrder(@Valid @RequestBody NewOrderParams newOrderParams, @RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { + public ResponseEntity makeOrder(@Valid @RequestBody NewOrderParams newOrderParams, @RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { + String username; try{ - if (!getUsernameFromAuthHeader(authHeader).equals(newOrderParams.getUsername())) - return new ResponseEntity<>(HttpStatus.UNAUTHORIZED); + username = getUsernameFromAuthHeader(authHeader); } catch (Exception e) { return new ResponseEntity<>(HttpStatus.BAD_REQUEST); } OrderObj newOrder = new OrderObj( - userService.getUser(newOrderParams.getUsername()), + userService.getUser(username), BigDecimal.valueOf(newOrderParams.getPrice()), BigDecimal.valueOf(newOrderParams.getQuantity()), newOrderParams.getAction().equals("buy") ? OrderAction.BUY : OrderAction.SELL - ); + ); matcher.match(newOrder); - return ResponseEntity.ok(new MakeOrderReturn( + return ResponseEntity.ok(new NewOrderReturn( orderService.getOrderbook(OrderAction.BUY), orderService.getOrderbook(OrderAction.SELL), - orderService.getOrderbook(OrderAction.BUY, newOrderParams.getUsername()), + orderService.getOrderbook(OrderAction.BUY, username), orderService.getOrderbook(OrderAction.SELL, newOrder.getUser().getUsername()), tradeService.getRecent(), orderService.getOrderDepth(OrderAction.BUY), diff --git a/src/main/java/com/example/demo/matcher/models/NewOrderParams.java b/src/main/java/com/example/demo/matcher/models/NewOrderParams.java index 81797f2..7e29366 100644 --- a/src/main/java/com/example/demo/matcher/models/NewOrderParams.java +++ b/src/main/java/com/example/demo/matcher/models/NewOrderParams.java @@ -11,7 +11,6 @@ @ToString @AllArgsConstructor @Getter public class NewOrderParams { - String username; @Min(value = OrderObj.minPrice) @Max(value = OrderObj.maxPrice) double price; @Min(value = OrderObj.minQuantity) @Max(value = OrderObj.maxQuantity) diff --git a/src/main/java/com/example/demo/matcher/models/MakeOrderReturn.java b/src/main/java/com/example/demo/matcher/models/NewOrderReturn.java similarity index 94% rename from src/main/java/com/example/demo/matcher/models/MakeOrderReturn.java rename to src/main/java/com/example/demo/matcher/models/NewOrderReturn.java index cf84f12..f766701 100644 --- a/src/main/java/com/example/demo/matcher/models/MakeOrderReturn.java +++ b/src/main/java/com/example/demo/matcher/models/NewOrderReturn.java @@ -7,7 +7,7 @@ import java.util.List; @AllArgsConstructor @ToString @Getter -public class MakeOrderReturn { +public class NewOrderReturn { private final List buy; private final List sell; private final List buyPrivate; diff --git a/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java b/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java index 0d92bf3..19d13e0 100644 --- a/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java +++ b/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java @@ -9,7 +9,6 @@ import com.example.demo.security.service.UserService; import com.example.demo.security.token.JwtTokenUtil; import com.example.demo.security.userInfo.AppUser; -import org.junit.Before; import org.junit.jupiter.api.*; import org.junit.runner.RunWith; import org.mockito.Mockito; @@ -180,7 +179,7 @@ void ItShouldAllowCreatingAnOrderWithValidJWT() throws Exception { .andReturn(); // check result - MakeOrderReturn expectedReturn = new MakeOrderReturn( + NewOrderReturn expectedReturn = new NewOrderReturn( List.of(expectedOrderbookItem), List.of(), List.of(expectedOrderbookItem), From 8820e7b664bc0b434989fccaf511f6acab1a8838 Mon Sep 17 00:00:00 2001 From: "Peter Marsh (He/Him)" Date: Tue, 31 Jan 2023 10:36:27 +0000 Subject: [PATCH 2/2] removed usernames from private access urls and adjusted tests accordingle --- build.gradle | 11 ++-- .../demo/matcher/MatcherController.java | 12 ++-- .../NewOrderValidationTest.java | 10 ++-- .../PrivateEndpointsWithTokenTest.java | 56 ++----------------- ...ndpointsAndLockedPrivateEndpointsTest.java | 2 - 5 files changed, 23 insertions(+), 68 deletions(-) diff --git a/build.gradle b/build.gradle index 94870af..eec7df9 100644 --- a/build.gradle +++ b/build.gradle @@ -43,7 +43,10 @@ test { jacocoTestReport { dependsOn test } -buildScan { - termsOfServiceUrl = 'https://gradle.com/terms-of-service' - termsOfServiceAgree = 'yes' -} \ No newline at end of file + +if (hasProperty('buildScan')) { + buildScan { + termsOfServiceUrl = 'https://gradle.com/terms-of-service' + termsOfServiceAgree = 'yes' + } +} diff --git a/src/main/java/com/example/demo/matcher/MatcherController.java b/src/main/java/com/example/demo/matcher/MatcherController.java index 9056bcf..50ca6ce 100644 --- a/src/main/java/com/example/demo/matcher/MatcherController.java +++ b/src/main/java/com/example/demo/matcher/MatcherController.java @@ -46,11 +46,10 @@ private String getUsernameFromAuthHeader(String authHeader) { return jwtTokenUtil.getSubject(token).split(",")[1]; } - @GetMapping(value = "/private/orderbook/buy/{username}") - public ResponseEntity> orderbook_buy(@PathVariable String username, @RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { + @GetMapping(value = "/private/orderbook/buy") + public ResponseEntity> orderbook_buy(@RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { try { - if (!getUsernameFromAuthHeader(authHeader).equals(username)) - return new ResponseEntity<>(HttpStatus.UNAUTHORIZED); + String username = getUsernameFromAuthHeader(authHeader); return ResponseEntity.ok(orderService.getOrderbook(OrderAction.BUY, username)); } catch(Exception e) { @@ -65,10 +64,9 @@ public ResponseEntity> orderbook_sell() { } @GetMapping(value = "/private/orderbook/sell/{username}") - public ResponseEntity> orderbook_sell(@PathVariable String username, @RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { + public ResponseEntity> orderbook_sell(@RequestHeader(HttpHeaders.AUTHORIZATION) String authHeader) { try { - if (!getUsernameFromAuthHeader(authHeader).equals(username)) - return new ResponseEntity<>(HttpStatus.UNAUTHORIZED); + String username = getUsernameFromAuthHeader(authHeader); return ResponseEntity.ok(orderService.getOrderbook(OrderAction.SELL, username)); } catch(Exception e) { diff --git a/src/test/java/com/example/demo/WebAndSecurity/NewOrderValidationTest.java b/src/test/java/com/example/demo/WebAndSecurity/NewOrderValidationTest.java index 7961690..c44c416 100644 --- a/src/test/java/com/example/demo/WebAndSecurity/NewOrderValidationTest.java +++ b/src/test/java/com/example/demo/WebAndSecurity/NewOrderValidationTest.java @@ -69,7 +69,7 @@ public void setup() throws Exception { @Test void ItShouldCheckNewOrderPriceIsNotTooSmall() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - "fakeUsername", -1, 1, "buy" + -1, 1, "buy" ); // API call @@ -87,7 +87,7 @@ void ItShouldCheckNewOrderPriceIsNotTooSmall() throws Exception { @Test void ItShouldCheckNewOrderPriceIsNotTooLarge() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - "fakeUsername", 1000000001, 1, "buy" + 1000000001, 1, "buy" ); // API call @@ -105,7 +105,7 @@ void ItShouldCheckNewOrderPriceIsNotTooLarge() throws Exception { @Test void ItShouldCheckNewOrderQuantityIsNotTooSmall() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - "fakeUsername", 1, -1, "buy" + 1, -1, "buy" ); // API call @@ -123,7 +123,7 @@ void ItShouldCheckNewOrderQuantityIsNotTooSmall() throws Exception { @Test void ItShouldCheckNewOrderQuantityIsNotTooLarge() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - "fakeUsername", 1, 1000000001, "buy" + 1, 1000000001, "buy" ); // API call @@ -141,7 +141,7 @@ void ItShouldCheckNewOrderQuantityIsNotTooLarge() throws Exception { @Test void ItShouldCheckNewOrderActionIsBuyOrSell() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - "fakeUsername", 1, 1, "badAction" + 1, 1, "badAction" ); // API call diff --git a/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java b/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java index 19d13e0..c4b7941 100644 --- a/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java +++ b/src/test/java/com/example/demo/WebAndSecurity/PrivateEndpointsWithTokenTest.java @@ -30,6 +30,7 @@ import java.util.stream.Collectors; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.doReturn; import static org.springframework.security.test.web.servlet.setup.SecurityMockMvcConfigurers.springSecurity; @@ -96,10 +97,10 @@ static List testOrderbook1() { @Test public void itShouldAllowAccessToPrivateBuyOrdersWithValidJWT() throws Exception { // mock orderService - doReturn(testOrderbook1()).when(orderService).getOrderbook(OrderAction.BUY, testUser1.getUsername()); + doReturn(testOrderbook1()).when(orderService).getOrderbook(eq(OrderAction.BUY), anyString()); MvcResult result = mvc.perform( - MockMvcRequestBuilders.get("/private/orderbook/buy/" + testUser1.getUsername()) + MockMvcRequestBuilders.get("/private/orderbook/buy/") .header("Authorization", "Bearer: " + FAKE_JWT)) .andReturn(); @@ -110,7 +111,7 @@ public void itShouldAllowAccessToPrivateBuyOrdersWithValidJWT() throws Exception @Test public void itShouldAllowAccessToPrivateSellOrdersWithValidJWT() throws Exception { // mock orderService - doReturn(testOrderbook1()).when(orderService).getOrderbook(OrderAction.SELL, testUser1.getUsername()); + doReturn(testOrderbook1()).when(orderService).getOrderbook(eq(OrderAction.SELL), anyString()); MvcResult result = mvc.perform( MockMvcRequestBuilders.get("/private/orderbook/sell/" + testUser1.getUsername()) @@ -121,32 +122,6 @@ public void itShouldAllowAccessToPrivateSellOrdersWithValidJWT() throws Exceptio assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()); } - @Test - public void itShouldNotAllowAccessToPrivateBuyOrdersWithValidJWTButWrongAccount() throws Exception { - // mock orderService - doReturn(testOrderbook1()).when(orderService).getOrderbook(OrderAction.BUY, testUser2.getUsername()); - - MvcResult result = mvc.perform( - MockMvcRequestBuilders.get("/private/orderbook/buy/" + testUser2.getUsername()) - .header("Authorization", "Bearer: " + FAKE_JWT)) - .andReturn(); - - assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value()); - } - - @Test - public void itShouldNotAllowAccessToPrivateSellOrdersWithValidJWTButWrongAccount() throws Exception { - // mock orderService - doReturn(testOrderbook1()).when(orderService).getOrderbook(OrderAction.BUY, testUser2.getUsername()); - - MvcResult result = mvc.perform( - MockMvcRequestBuilders.get("/private/orderbook/sell/" + testUser2.getUsername()) - .header("Authorization", "Bearer: " + FAKE_JWT)) - .andReturn(); - - assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value()); - } - @Test void ItShouldAllowCreatingAnOrderWithValidJWT() throws Exception { OrderObj newOrder = TestUtils.makeOrder(testUser1, 1 ,1, "b"); @@ -154,7 +129,6 @@ void ItShouldAllowCreatingAnOrderWithValidJWT() throws Exception { NewOrderParams newOrderParams = new NewOrderParams( - newOrder.getUser().getUsername(), newOrder.getPrice().doubleValue(), newOrder.getQuantity().doubleValue(), "buy" @@ -164,8 +138,8 @@ void ItShouldAllowCreatingAnOrderWithValidJWT() throws Exception { OrderbookItem expectedOrderbookItem = new OrderbookItem(newOrder.getPrice(), newOrder.getQuantity()); doReturn(List.of(expectedOrderbookItem)).when(orderService).getOrderbook(OrderAction.BUY); doReturn(List.of()).when(orderService).getOrderbook(OrderAction.SELL); - doReturn(List.of(expectedOrderbookItem)).when(orderService).getOrderbook(OrderAction.BUY, newOrderParams.getUsername()); - doReturn(List.of()).when(orderService).getOrderbook(OrderAction.SELL, newOrderParams.getUsername()); + doReturn(List.of(expectedOrderbookItem)).when(orderService).getOrderbook(eq(OrderAction.BUY), anyString()); + doReturn(List.of()).when(orderService).getOrderbook(eq(OrderAction.SELL), anyString()); doReturn(List.of()).when(tradeService).getRecent(); doReturn(List.of(expectedOrderbookItem)).when(orderService).getOrderDepth(OrderAction.BUY); doReturn(List.of()).when(orderService).getOrderDepth(OrderAction.SELL); @@ -192,22 +166,4 @@ void ItShouldAllowCreatingAnOrderWithValidJWT() throws Exception { assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.OK.value()); assertThat(result.getResponse().getContentAsString()).isEqualTo(TestUtils.asJsonString(expectedReturn)); } - - // Test new order post request validation - - @Test - void ItShouldCheckNewOrderHasSameUsernameAsJWT() throws Exception { - NewOrderParams newOrderParams = new NewOrderParams( - "anotherUsername", 1, 1, "buy" - ); - - // API call - MvcResult result = mvc.perform(MockMvcRequestBuilders.post("/private/make/order") - .header("Authorization", "Bearer: " + FAKE_JWT) - .contentType(MediaType.APPLICATION_JSON) - .content(TestUtils.asJsonString(newOrderParams))) - .andReturn(); - - assertThat(result.getResponse().getStatus()).isEqualTo(HttpStatus.UNAUTHORIZED.value()); - } } diff --git a/src/test/java/com/example/demo/WebAndSecurity/PublicEndpointsAndLockedPrivateEndpointsTest.java b/src/test/java/com/example/demo/WebAndSecurity/PublicEndpointsAndLockedPrivateEndpointsTest.java index 34e0330..2dae73e 100644 --- a/src/test/java/com/example/demo/WebAndSecurity/PublicEndpointsAndLockedPrivateEndpointsTest.java +++ b/src/test/java/com/example/demo/WebAndSecurity/PublicEndpointsAndLockedPrivateEndpointsTest.java @@ -157,7 +157,6 @@ void itShouldNotAllowCreationOfOrdersWithoutAnyJWT() throws Exception { OrderObj newOrder = TestUtils.makeOrder(1 ,1, "b"); NewOrderParams newOrderParams = new NewOrderParams( - newOrder.getUser().getUsername(), newOrder.getPrice().doubleValue(), newOrder.getQuantity().doubleValue(), "buy" @@ -198,7 +197,6 @@ void itShouldNotAllowCreationOfOrdersWithoutValidJWT() throws Exception { OrderObj newOrder = TestUtils.makeOrder(1 ,1, "b"); NewOrderParams newOrderParams = new NewOrderParams( - newOrder.getUser().getUsername(), newOrder.getPrice().doubleValue(), newOrder.getQuantity().doubleValue(), "buy"