From d98b8feaad491e3ac181e53168df39a3eb48a954 Mon Sep 17 00:00:00 2001 From: Michael Hoennig Date: Wed, 5 Oct 2022 08:04:44 +0200 Subject: [PATCH] add IBAN+BIC validation --- build.gradle | 1 + .../RestResponseEntityExceptionHandler.java | 24 ++-- .../HsOfficeBankAccountController.java | 5 + ...esponseEntityExceptionHandlerUnitTest.java | 22 ++- ...ceBankAccountControllerAcceptanceTest.java | 21 ++- ...HsOfficeBankAccountControllerRestTest.java | 125 ++++++++++++++++++ 6 files changed, 178 insertions(+), 20 deletions(-) create mode 100644 src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerRestTest.java diff --git a/build.gradle b/build.gradle index 72217dc7..621bbc34 100644 --- a/build.gradle +++ b/build.gradle @@ -60,6 +60,7 @@ dependencies { implementation 'com.vladmihalcea:hibernate-types-55:2.19.2' implementation 'org.openapitools:jackson-databind-nullable:0.2.3' implementation 'org.modelmapper:modelmapper:3.1.0' + implementation 'org.iban4j:iban4j:3.2.3-RELEASE' compileOnly 'org.projectlombok:lombok' testCompileOnly 'org.projectlombok:lombok' diff --git a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java b/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java index beef530c..3eb428ae 100644 --- a/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java +++ b/src/main/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandler.java @@ -2,6 +2,7 @@ package net.hostsharing.hsadminng.errors; import com.fasterxml.jackson.annotation.JsonFormat; import lombok.Getter; +import org.iban4j.Iban4jException; import org.springframework.core.NestedExceptionUtils; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.http.HttpStatus; @@ -54,6 +55,20 @@ public class RestResponseEntityExceptionHandler return errorResponse(request, HttpStatus.BAD_REQUEST, message); } + @ExceptionHandler(Iban4jException.class) + protected ResponseEntity handleIbanAndBicExceptions( + final Throwable exc, final WebRequest request) { + final var message = firstLine(NestedExceptionUtils.getMostSpecificCause(exc).getMessage()); + return errorResponse(request, HttpStatus.BAD_REQUEST, message); + } + + @ExceptionHandler(Throwable.class) + protected ResponseEntity handleOtherExceptions( + final Throwable exc, final WebRequest request) { + final var message = firstLine(NestedExceptionUtils.getMostSpecificCause(exc).getMessage()); + return errorResponse(request, httpStatus(message).orElse(HttpStatus.INTERNAL_SERVER_ERROR), message); + } + private String userReadableEntityClassName(final String exceptionMessage) { final var regex = "(net.hostsharing.hsadminng.[a-z0-9_.]*.[A-Za-z0-9_$]*Entity) "; final var pattern = Pattern.compile(regex); @@ -61,7 +76,7 @@ public class RestResponseEntityExceptionHandler if (matcher.find()) { final var entityName = matcher.group(1); final var entityClass = resolveClassOrNull(entityName); - if (entityClass != null ) { + if (entityClass != null) { return (entityClass.isAnnotationPresent(DisplayName.class) ? exceptionMessage.replace(entityName, entityClass.getAnnotation(DisplayName.class).value()) : exceptionMessage.replace(entityName, entityClass.getSimpleName())) @@ -80,13 +95,6 @@ public class RestResponseEntityExceptionHandler } } - @ExceptionHandler(Throwable.class) - protected ResponseEntity handleOtherExceptions( - final Throwable exc, final WebRequest request) { - final var message = firstLine(NestedExceptionUtils.getMostSpecificCause(exc).getMessage()); - return errorResponse(request, httpStatus(message).orElse(HttpStatus.INTERNAL_SERVER_ERROR), message); - } - private Optional httpStatus(final String message) { if (message.startsWith("ERROR: [")) { for (HttpStatus status : HttpStatus.values()) { diff --git a/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountController.java b/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountController.java index 81f15555..130f934f 100644 --- a/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountController.java +++ b/src/main/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountController.java @@ -5,6 +5,8 @@ import net.hostsharing.hsadminng.context.Context; import net.hostsharing.hsadminng.hs.office.generated.api.v1.api.HsOfficeBankAccountsApi; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeBankAccountInsertResource; import net.hostsharing.hsadminng.hs.office.generated.api.v1.model.HsOfficeBankAccountResource; +import org.iban4j.BicUtil; +import org.iban4j.IbanUtil; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.http.ResponseEntity; import org.springframework.transaction.annotation.Transactional; @@ -49,6 +51,9 @@ public class HsOfficeBankAccountController implements HsOfficeBankAccountsApi { context.define(currentUser, assumedRoles); + IbanUtil.validate(body.getIban()); + BicUtil.validate(body.getBic()); + final var entityToSave = map(body, HsOfficeBankAccountEntity.class); entityToSave.setUuid(UUID.randomUUID()); diff --git a/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerUnitTest.java b/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerUnitTest.java index 85f645d9..f2a1d66d 100644 --- a/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerUnitTest.java +++ b/src/test/java/net/hostsharing/hsadminng/errors/RestResponseEntityExceptionHandlerUnitTest.java @@ -2,6 +2,8 @@ package net.hostsharing.hsadminng.errors; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.ExtendWith; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.mockito.junit.jupiter.MockitoExtension; import org.springframework.dao.DataIntegrityViolationException; import org.springframework.orm.jpa.JpaObjectRetrievalFailureException; @@ -9,7 +11,6 @@ import org.springframework.orm.jpa.JpaSystemException; import org.springframework.web.context.request.WebRequest; import javax.persistence.EntityNotFoundException; - import java.util.NoSuchElementException; import static org.assertj.core.api.Assertions.assertThat; @@ -147,6 +148,25 @@ class RestResponseEntityExceptionHandlerUnitTest { assertThat(errorResponse.getBody().getMessage()).isEqualTo("some error message"); } + @ParameterizedTest + @ValueSource(classes = { + org.iban4j.InvalidCheckDigitException.class, + org.iban4j.IbanFormatException.class, + org.iban4j.BicFormatException.class }) + void handlesIbanAndBicExceptions(final Class givenExceptionClass) + throws Exception { + // given + final var givenException = givenExceptionClass.getConstructor(String.class).newInstance("given error message"); + final var givenWebRequest = mock(WebRequest.class); + + // when + final var errorResponse = exceptionHandler.handleIbanAndBicExceptions(givenException, givenWebRequest); + + // then + assertThat(errorResponse.getStatusCodeValue()).isEqualTo(400); + assertThat(errorResponse.getBody().getMessage()).isEqualTo("given error message"); + } + @Test void handleOtherExceptionsWithoutErrorCode() { // given diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java index a6e9c1c6..bb6b1f37 100644 --- a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerAcceptanceTest.java @@ -115,7 +115,7 @@ class HsOfficeBankAccountControllerAcceptanceTest { @Nested @Accepts({ "bankaccount:C(Create)" }) - class AddBankAccount { + class CreateBankAccount { @Test void globalAdmin_withoutAssumedRole_canAddBankAccount() { @@ -127,11 +127,11 @@ class HsOfficeBankAccountControllerAcceptanceTest { .header("current-user", "superuser-alex@hostsharing.net") .contentType(ContentType.JSON) .body(""" - { - "holder": "new test holder", - "iban": "DE88100900001234567892", - "bic": "BEVODEBB" - } + { + "holder": "new test holder", + "iban": "DE88100900001234567892", + "bic": "BEVODEBB" + } """) .port(port) .when() @@ -195,8 +195,7 @@ class HsOfficeBankAccountControllerAcceptanceTest { } @Test - @Accepts({ "bankaccount:X(Access Control)" }) - @Disabled("TODO: not implemented yet") + @Disabled("TODO: not implemented yet - also add Accepts annotation when done") void bankaccountAdminUser_canGetRelatedBankAccount() { context.define("superuser-alex@hostsharing.net"); final var givenBankAccountUuid = bankAccountRepo.findByOptionalHolderLike("first").get(0).getUuid(); @@ -212,9 +211,9 @@ class HsOfficeBankAccountControllerAcceptanceTest { .contentType("application/json") .body("", lenientlyEquals(""" { - "label": "first bankaccount", - "emailAddresses": "bankaccount-admin@firstbankaccount.example.com", - "phoneNumbers": "+49 123 1234567" + "holder": "...", + "iban": "...", + "bic": "..." } """)); // @formatter:on } diff --git a/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerRestTest.java b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerRestTest.java new file mode 100644 index 00000000..60885cc9 --- /dev/null +++ b/src/test/java/net/hostsharing/hsadminng/hs/office/bankaccount/HsOfficeBankAccountControllerRestTest.java @@ -0,0 +1,125 @@ +package net.hostsharing.hsadminng.hs.office.bankaccount; + +import net.hostsharing.hsadminng.context.Context; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.EnumSource; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.test.autoconfigure.web.servlet.WebMvcTest; +import org.springframework.boot.test.mock.mockito.MockBean; +import org.springframework.http.MediaType; +import org.springframework.test.web.servlet.MockMvc; +import org.springframework.test.web.servlet.request.MockMvcRequestBuilders; + +import static org.hamcrest.Matchers.is; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.jsonPath; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@WebMvcTest(HsOfficeBankAccountController.class) +class HsOfficeBankAccountControllerRestTest { + + @Autowired + MockMvc mockMvc; + + @MockBean + Context contextMock; + + @MockBean + HsOfficeBankAccountRepository bankAccountRepo; + + enum InvalidIbanTestCase { + TOO_SHORT("DE8810090000123456789", "[10090000123456789] length is 17, expected BBAN length is: 18"), + TOO_LONG("DE8810090000123456789123445", "[10090000123456789123445] length is 23, expected BBAN length is: 18"), + INVALID_CHARACTER("DE 8810090000123456789123445", "Iban's check digit should contain only digits."), + INVALID_CHECKSUM( + "DE88100900001234567893", + "[DE88100900001234567893] has invalid check digit: 88, expected check digit is: 61"); + + private final String givenIban; + private final String expectedIbanMessage; + + InvalidIbanTestCase(final String givenIban, final String expectedErrorMessage) { + this.givenIban = givenIban; + this.expectedIbanMessage = expectedErrorMessage; + } + + String givenIban() { + return givenIban; + } + + String expectedErrorMessage() { + return expectedIbanMessage; + } + } + + @ParameterizedTest + @EnumSource(InvalidIbanTestCase.class) + void invalidIbanBeRejected(final InvalidIbanTestCase testCase) throws Exception { + + // when + mockMvc.perform(MockMvcRequestBuilders + .post("/api/hs/office/bankaccounts") + .header("current-user", "superuser-alex@hostsharing.net") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "holder": "new test holder", + "iban": "%s", + "bic": "BEVODEBB" + } + """.formatted(testCase.givenIban())) + .accept(MediaType.APPLICATION_JSON)) + + // then + .andExpect(status().is4xxClientError()) + .andExpect(jsonPath("status", is(400))) + .andExpect(jsonPath("error", is("Bad Request"))) + .andExpect(jsonPath("message", is(testCase.expectedErrorMessage()))); + } + + enum InvalidBicTestCase { + TOO_SHORT("BEVODEB", "Bic length must be 8 or 11"), + TOO_LONG("BEVODEBBX", "Bic length must be 8 or 11"), + INVALID_CHARACTER("BEV-ODEB", "Bank code must contain only letters."); + + private final String givenBic; + private final String expectedErrorMessage; + + InvalidBicTestCase(final String givenBic, final String expectedErrorMessage) { + this.givenBic = givenBic; + this.expectedErrorMessage = expectedErrorMessage; + } + + String givenIban() { + return givenBic; + } + + String expectedErrorMessage() { + return expectedErrorMessage; + } + } + + @ParameterizedTest + @EnumSource(InvalidBicTestCase.class) + void invalidBicBeRejected(final InvalidBicTestCase testCase) throws Exception { + + // when + mockMvc.perform(MockMvcRequestBuilders + .post("/api/hs/office/bankaccounts") + .header("current-user", "superuser-alex@hostsharing.net") + .contentType(MediaType.APPLICATION_JSON) + .content(""" + { + "holder": "new test holder", + "iban": "DE88100900001234567892", + "bic": "%s" + } + """.formatted(testCase.givenIban())) + .accept(MediaType.APPLICATION_JSON)) + + // then + .andExpect(status().is4xxClientError()) + .andExpect(jsonPath("status", is(400))) + .andExpect(jsonPath("error", is("Bad Request"))) + .andExpect(jsonPath("message", is(testCase.expectedErrorMessage()))); + } +}