diff --git a/Pagination.md b/Pagination.md index 5fd1dc1..ecbafee 100644 --- a/Pagination.md +++ b/Pagination.md @@ -45,7 +45,8 @@ void cashCardListDeserializationTest() throws IOException { } ``` -# GET list +# GET list + ## DB data It is important to populate our test db with known data, that would reflect desired test case scenario. `src/test/resource/data.sql` diff --git a/Post.md b/Post.md index f242ef6..f79bf83 100644 --- a/Post.md +++ b/Post.md @@ -113,4 +113,8 @@ This is constructing a URI to the newly created `CashCard`. This is the URI that > Where did `UriComponentsBuilder` come from? -We were able to add `UriComponentsBuilder ucb` as a method argument to this `POST` handler method and it was automatically passed in. How so? It was injected from our now-familiar friend, Spring's IoC Container. Thanks, Spring Web! \ No newline at end of file +We were able to add `UriComponentsBuilder ucb` as a method argument to this `POST` handler method and it was automatically passed in. How so? It was injected from our now-familiar friend, Spring's IoC Container. Thanks, Spring Web! + +# Security + +It is pretty obvious, that the system should have some way of handling data from different users. The [[Home#Security]] section is a good place to continue reading. You can continue explore other CRUD methods afterwards. \ No newline at end of file diff --git a/Put.md b/Put.md new file mode 100644 index 0000000..0a53b6b --- /dev/null +++ b/Put.md @@ -0,0 +1,181 @@ +# Test + +```java +import org.springframework.http.HttpEntity; +import org.springframework.http.HttpMethod; +... + +@Test +@DirtiesContext +void shouldUpdateAnExistingCashCard() { + CashCard cashCardUpdate = new CashCard(null, 19.99, null); + HttpEntity request = new HttpEntity<>(cashCardUpdate); + ResponseEntity response = restTemplate + .withBasicAuth("sarah1", "abc123") + .exchange("/cashcards/99", HttpMethod.PUT, request, Void.class); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NO_CONTENT); + + // Validte that the resource was updated + ResponseEntity getResponse = restTemplate + .withBasicAuth("sarah1", "abc123") + .getForEntity("/cashcards/99", String.class); + assertThat(getResponse.getStatusCode()).isEqualTo(HttpStatus.OK); + + DocumentContext documentContext = JsonPath.parse(getResponse.getBody()); + Number id = documentContext.read("$.id"); + Double amount = documentContext.read("$.amount"); + assertThat(id).isEqualTo(99); + assertThat(amount).isEqualTo(19.99); +} +``` + +## `restTemplate.exchange()` + +All other tests use `RestTemplate.xyzForEntity()` methods such as `getForEntity()` and `postForEntity()`. So, why are we not following the same pattern utilizing `putForEntity()`? + +Answer: `putForEntity()` does not exist! Read more about it here in the [GitHub issue](https://github.com/spring-projects/spring-framework/issues/15256) about the topic. + +Luckily `RestTemplate` supports multiple ways of interacting with REST APIs, such as `RestTemplate.exchange()`. The `exchange()` method is a more general version of the `xyzForEntity()` methods we've used in other tests: `exchange()` requires the verb and the request entity (the body of the request) to be supplied as parameters. The two lines below are functionally equivalent of one another: + +- + ```java + .exchange("/cashcards/99", HttpMethod.GET, new HttpEntity(null), String.class); + ``` + +- + ```java + `.getForEntity("/cashcards/99", String.class);` + ``` + +Now let's explain the test code. + +- First we create the `HttpEntity` that the `exchange()` method needs: + +```java +HttpEntity request = new HttpEntity<>(existingCashCard); +``` + +- Then we call `exchange()`, which sends a `PUT` request for the target ID of `99` and updated Cash Card data: + +```java +.exchange("/cashcards/99", HttpMethod.PUT, request, Void.class); +``` + +- And finally, we are using `.getForEntity()` to ensure that the requested resource was updated. + +Run the test. + +```shell +[~/exercises] $ ./gradlew test +... +CashCardApplicationTests > shouldUpdateAnExistingCashCard() FAILED + org.opentest4j.AssertionFailedError: + expected: 204 NO_CONTENT + but was: 403 FORBIDDEN +... +BUILD FAILED in 6s +``` + +It has failed with a `403 FORBIDDEN` response code, because we haven't implemented a `PUT` request method handler yet. With no Controller endpoint, this `PUT` call is forbidden! Spring Security automatically handled this scenario for us. + +# Implement @PutMapping in the Controller + +```java +@PutMapping("/{requestedId}") +private ResponseEntity putCashCard(@PathVariable Long requestedId, @RequestBody CashCard cashCardUpdate, Principal principal) { + CashCard cashCard = cashCardRepository.findByIdAndOwner(requestedId, principal.getName()); + CashCard updatedCashCard = new CashCard(cashCard.id(), cashCardUpdate.amount(), principal.getName()); + cashCardRepository.save(updatedCashCard); + return ResponseEntity.noContent().build(); +} +``` + +This Controller endpoint is fairly self-explanatory: + +- The `@PutMapping` supports the `PUT` verb and supplies the target `requestedId`. +- The `@RequestBody` contains the updated `CashCard` data. +- We've added the `Principal` as a method argument, provided automatically by Spring Security. Thanks once again, Spring Security! +- Than we scope our retrieval of the `CashCard` to the submitted `requestedId` and `Principal` (provided by Spring Security) to ensure only the authenticated, authorized `owner` may update this `CashCard` + + ```java + cashCardRepository.findByIdAndOwner(requestedId, principal.getName()); + ``` + +- Build a `CashCard` with updated values and save it. +- Return an HTTP `204 NO_CONTENT` response code for now just to get started. + +But what happens if we attempt to update a `CashCard` that does not exist? + +# Update non existing resource? + +## Test + +```java +@Test +void shouldNotUpdateACashCardThatDoesNotExist() { + CashCard unknownCard = new CashCard(null, 19.99, null); + HttpEntity request = new HttpEntity<>(unknownCard); + ResponseEntity response = restTemplate + .withBasicAuth("sarah1", "abc123") + .exchange("/cashcards/99999", HttpMethod.PUT, request, Void.class); + assertThat(response.getStatusCode()).isEqualTo(HttpStatus.NOT_FOUND); +} +``` + +Run the tests and observe the results. + +```shell +... +CashCardApplicationTests > shouldNotUpdateACashCardThatDoesNotExist() FAILED + org.opentest4j.AssertionFailedError: + expected: 404 NOT_FOUND + but was: 403 FORBIDDEN +``` + +Before we run them again, let's edit `build.gradle` to enable additional test output. + +```groovy +test { + testLogging { + ... + // Change to `true` for more verbose test output + showStandardStreams = true + } +} +``` + +After rerunning the tests, search the output for the following: + +```shell +CashCardApplicationTests > shouldNotUpdateACashCardThatDoesNotExist() STANDARD_OUT +... +java.lang.NullPointerException: Cannot invoke "example.cashcard.CashCard.id()" because "cashCard" is null +``` + +> A `NullPointerException`! But why? + +Looking at `CashCardController.putCashCard` we can see that if we don't find the `cashCard` then method calls to `cashCard` will result in a `NullPointerException`. That makes sense. + +But why is a `NullPointerException` thrown in our Controller resulting in a `403 FORBIDDEN` instead of a `500 INTERNAL_SERVER_ERROR`, given the server "crashed?" + +## SpringSecurity and HTTP error codes + +Our Controller is returning `403 FORBIDDEN` instead of an `500 INTERNAL_SERVER_ERROR` because Spring Security is automatically implementing a best practice regarding how errors are handled by Spring Web. + +It's important to understand that any information returned from our application might be useful to a bad actor attempting to violate our application's security. For example: knowledge about actions that causes our application to crash -- a `500 INTERNAL_SERVER_ERROR`. + +In order to avoid "leaking" information about our application, Spring Security has configured Spring Web to return a generic `403 FORBIDDEN` in most error conditions. If almost everything results in a `403 FORBIDDEN` response then an attacker doesn't really know what's going on. + +## Don't crash + +Though we're thankful to Spring Security, our application should not crash - we shouldn't allow our code to throw a `NullPointerException`. Instead, we should handle the condition when `cashCard == null`, and return a generic `404 NOT_FOUND` HTTP response. Update the `Put` endpoint: + +```java +... +CashCard cashCard = cashCardRepository.findByIdAndOwner(requestedId, principal.getName()); + +if (cashCard != null) { // <<-- do not crash :) + return ResponseEntity.notFound().build(); +} +... +``` \ No newline at end of file