From eef19d232d23496161650fa04287353cac2449a4 Mon Sep 17 00:00:00 2001 From: john-tco <135222889+john-tco@users.noreply.github.com> Date: Tue, 19 Dec 2023 13:21:55 +0000 Subject: [PATCH] GAP-2349: Fix open redirects (#81) * initial * add tests * throw err when slug not in contentful * change log method * remove one liner insane variable * mv typecast --- .../service/GrantAdvertService.java | 40 +++++++++++-- .../web/GrantAdvertController.java | 12 +++- .../service/GrantAdvertServiceTest.java | 59 ++++++++++++++++++- .../web/GrantAdvertControllerTest.java | 31 +++++++++- 4 files changed, 135 insertions(+), 7 deletions(-) diff --git a/src/main/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertService.java b/src/main/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertService.java index 4130d119..7433cea6 100644 --- a/src/main/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertService.java +++ b/src/main/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertService.java @@ -1,10 +1,7 @@ package gov.cabinetoffice.gap.applybackend.service; -import com.contentful.java.cda.CDAArray; -import com.contentful.java.cda.CDAClient; -import com.contentful.java.cda.CDAEntry; -import com.contentful.java.cda.CDAResourceNotFoundException; +import com.contentful.java.cda.*; import gov.cabinetoffice.gap.applybackend.dto.api.GetGrantAdvertDto; import gov.cabinetoffice.gap.applybackend.dto.api.GetGrantMandatoryQuestionDto; import gov.cabinetoffice.gap.applybackend.enums.GrantAdvertStatus; @@ -20,6 +17,12 @@ import org.springframework.http.ResponseEntity; import org.springframework.stereotype.Service; +import java.net.URL; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Optional; + @Service @RequiredArgsConstructor @Slf4j @@ -86,6 +89,35 @@ public boolean advertExistsInContentful(final String advertSlug) { return advertExists; } + private String getGrantWebpageUrl(final CDAArray contentfulEntry){ + CDAEntry entry = ((CDAEntry) contentfulEntry.items().get(0)); + Map rawFields = entry.rawFields(); + Optional optionalUrl = ((Map) rawFields.get("grantWebpageUrl")).values().stream().findFirst(); + if(optionalUrl.isEmpty()){ + throw new NotFoundException("Grant webpage url not found"); + } + return optionalUrl.get(); + } + + public void validateGrantWebpageUrl(final String contentfulSlug, final String grantWebpageUrl) { + try { + final CDAArray contentfulEntry = contentfulDeliveryClient + .fetch(CDAEntry.class) + .withContentType("grantDetails") + .where("fields.label", contentfulSlug).all(); + + String url = this.getGrantWebpageUrl(contentfulEntry); + + if (!url.equals(grantWebpageUrl)) { + throw new NotFoundException("Grant webpage url does not match the url in contentful"); + } + } catch (CDAResourceNotFoundException error) { + log.error(String.format("Advert with slug %s not found in Contentful", contentfulSlug)); + throw error; + } + } + + public GrantAdvert getAdvertBySchemeId(String schemeId) { final GrantAdvert grantAdvert = grantAdvertRepository.findBySchemeId(Integer.parseInt(schemeId)) .orElseThrow(() -> new NotFoundException("Advert with schemeId " + schemeId + " not found")); diff --git a/src/main/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertController.java b/src/main/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertController.java index 598fb666..2769b7bf 100644 --- a/src/main/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertController.java +++ b/src/main/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertController.java @@ -27,6 +27,7 @@ import org.springframework.web.bind.annotation.RestController; import javax.validation.constraints.NotBlank; +import java.net.URL; @Slf4j @RequiredArgsConstructor @@ -99,7 +100,6 @@ public ResponseEntity generateGetGrantAdvertDtoFromSchemeId(@ return ResponseEntity.ok(grantAdvertService.generateGetGrantAdvertDto(advert, mandatoryQuestionsDto)); } - @GetMapping("{advertSlug}/exists-in-contentful") @Operation(summary = "Check whether a grant advert exists in Contentful with the given slug") @ApiResponses(value = { @@ -107,6 +107,7 @@ public ResponseEntity generateGetGrantAdvertDtoFromSchemeId(@ @ApiResponse(responseCode = "400", description = "Required path variable not provided in expected format", content = @Content(mediaType = "application/json")) }) + public ResponseEntity advertExistsInContentful(@PathVariable final String advertSlug) { final boolean advertExists = grantAdvertService.advertExistsInContentful(advertSlug); @@ -117,4 +118,13 @@ public ResponseEntity advertExistsInContentful(@Pa .build() ); } + + @GetMapping("/validate-grant-webpage-url") + @Operation(summary = "Check if a grant webpage url matches the grant-webpage-url attached to the slug") + public ResponseEntity validateGrantWebpageUrl( + @RequestParam @NotBlank String contentfulSlug,@RequestParam @NotBlank String grantWebpageUrl) { + grantAdvertService.validateGrantWebpageUrl(contentfulSlug, grantWebpageUrl); + + return ResponseEntity.ok("Success"); + } } diff --git a/src/test/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertServiceTest.java b/src/test/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertServiceTest.java index acb645a5..626513a8 100644 --- a/src/test/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertServiceTest.java +++ b/src/test/java/gov/cabinetoffice/gap/applybackend/service/GrantAdvertServiceTest.java @@ -22,6 +22,7 @@ import org.junit.jupiter.api.extension.ExtendWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.Mockito; import org.mockito.junit.jupiter.MockitoExtension; import java.util.HashMap; @@ -31,7 +32,10 @@ import java.util.UUID; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatNoException; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) @@ -52,6 +56,9 @@ class GrantAdvertServiceTest { @Mock private AbsQuery query; + @Mock + CDAEntry mockCDAEntry; + @Mock private FetchQuery fetchQuery; @@ -423,4 +430,54 @@ void getAdvertBySchemeId_throwsNotFoundException() { assertThrows(NotFoundException.class, () -> grantAdvertService.getAdvertBySchemeId(schemeId)); } } -} \ No newline at end of file + + @Nested + class validateGrantWebpageUrl { + @Test + void SuccessfullyValidatesWebpageUrl() { + final String advertSlug = "chargepoint-grant-for-homeowners-1"; + final String grantWebpageUrl = "https://example.domain.org/some/deeper/path"; + final Map entry = new HashMap<>(); + final Map entries = new HashMap<>(); + entry.put("0", grantWebpageUrl); + entries.put("grantWebpageUrl", entry); + when((mockCDAEntry).rawFields()).thenReturn(entries); + when(contentfulDeliveryClient.fetch(CDAEntry.class)) + .thenReturn(fetchQuery); + when(fetchQuery.withContentType("grantDetails")) + .thenReturn(fetchQuery); + when(fetchQuery.where("fields.label", advertSlug)) + .thenReturn(fetchQuery); + when(fetchQuery.all()) + .thenReturn(contentfulResults); + when(contentfulResults.items()) + .thenReturn(List.of(mockCDAEntry)); + + assertThatNoException().isThrownBy(() -> grantAdvertService.validateGrantWebpageUrl(advertSlug, grantWebpageUrl)); + } + + @Test + void throwsNotFound__WhenProvidedInvalidWebpageUrl() { + final String advertSlug = "chargepoint-grant-for-homeowners-1"; + final String grantWebpageUrl = "https://malicious.domain.org/path"; + final String contentfulGrantWebpageUrl = "https://example.domain.org/some/deeper/path"; + final Map entry = new HashMap<>(); + final Map entries = new HashMap<>(); + entry.put("0", contentfulGrantWebpageUrl); + entries.put("grantWebpageUrl", entry); + when((mockCDAEntry).rawFields()).thenReturn(entries); + when(contentfulDeliveryClient.fetch(CDAEntry.class)) + .thenReturn(fetchQuery); + when(fetchQuery.withContentType("grantDetails")) + .thenReturn(fetchQuery); + when(fetchQuery.where("fields.label", advertSlug)) + .thenReturn(fetchQuery); + when(fetchQuery.all()) + .thenReturn(contentfulResults); + when(contentfulResults.items()) + .thenReturn(List.of(mockCDAEntry)); + + assertThrows(NotFoundException.class, () -> grantAdvertService.validateGrantWebpageUrl(advertSlug, grantWebpageUrl)); + } + } +} diff --git a/src/test/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertControllerTest.java b/src/test/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertControllerTest.java index fdcda8fc..2bd88639 100644 --- a/src/test/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertControllerTest.java +++ b/src/test/java/gov/cabinetoffice/gap/applybackend/web/GrantAdvertControllerTest.java @@ -28,7 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertThrows; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; @ExtendWith(MockitoExtension.class) class GrantAdvertControllerTest { @@ -204,4 +204,33 @@ void generateGetGrantAdvertDtoFromSchemeId_ThrowsAnyOtherKindOfException() { assertThrows(IllegalArgumentException.class, () -> grantAdvertController.generateGetGrantAdvertDtoFromSchemeId(String.valueOf(schemeId))); } } + + @Nested + class validateGrantWebpageUrl { + @Test + void validatesGrantWebpageUrl_returnsSuccessWithValidArgs(){ + final String grantWebpageUrl = "https://www.example.com/external-advert"; + final String contentfulSlug = "internal-contentful-slug"; + doNothing().when(grantAdvertService).validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug); + ResponseEntity response = grantAdvertController.validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug); + + assertThat(response).isEqualTo(ResponseEntity.ok("Success")); + } + + @Test + void validatesGrantWebpageUrl_returnsNotFound(){ + final String grantWebpageUrl = "https://www.maliciousdomain.com/extenal"; + final String contentfulSlug = "internal-contentful-slug"; + doThrow(NotFoundException.class).when(grantAdvertService).validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug); + assertThrows(NotFoundException.class, ()-> grantAdvertController.validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug)); + } + + @Test + void validatesGrantWebpageUrl_ThrowsNotFoundException(){ + final String grantWebpageUrl = "https://www.maliciousdomain.com/extenal"; + final String contentfulSlug = "internal-contentful-slug"; + doThrow(NotFoundException.class).when(grantAdvertService).validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug); + assertThrows(NotFoundException.class, ()-> grantAdvertController.validateGrantWebpageUrl(grantWebpageUrl, contentfulSlug)); + } + } } \ No newline at end of file