Skip to content

Commit

Permalink
Add validation for a few inputs (like flow and uuid) in a few spots. (#…
Browse files Browse the repository at this point in the history 
…548)

* Add validation for a few inputs (like flow and uuid) in a few spots.

* Add a couple of tests for bad uuids and errors being created.
  • Loading branch information
@bseeger
bseeger committed Apr 26, 2024
1 parent fb28ce5 commit ecda742
Show file tree
Hide file tree
Showing 4 changed files with 154 additions and 79 deletions.
16 changes: 8 additions & 8 deletions src/main/java/formflow/library/FormFlowController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,13 @@
import formflow.library.data.SubmissionRepositoryService;
import formflow.library.data.UserFileRepositoryService;
import jakarta.servlet.http.HttpSession;

import java.util.List;
import java.util.Optional;
import java.util.UUID;
import java.util.Map;
import java.util.HashMap;

import lombok.extern.slf4j.Slf4j;
import org.springframework.context.MessageSource;
import org.springframework.http.HttpStatus;
Expand All @@ -36,8 +38,8 @@ public abstract class FormFlowController {
public static final String SUBMISSION_MAP_NAME = "submissionMap";

FormFlowController(SubmissionRepositoryService submissionRepositoryService, UserFileRepositoryService userFileRepositoryService,
List<FlowConfiguration> flowConfigurations, FormFlowConfigurationProperties formFlowConfigurationProperties,
MessageSource messageSource) {
List<FlowConfiguration> flowConfigurations, FormFlowConfigurationProperties formFlowConfigurationProperties,
MessageSource messageSource) {
this.submissionRepositoryService = submissionRepositoryService;
this.userFileRepositoryService = userFileRepositoryService;
this.flowConfigurations = flowConfigurations;
Expand Down Expand Up @@ -72,10 +74,10 @@ protected Submission saveToRepository(Submission submission, String subflowName)

/**
* Gets the {@link FlowConfiguration} object for a given flow
* @throws ResponseStatusException when FlowConfigurations are not found.
*
* @param flow {@link String} of a flow name.
* @return Returns a {@link FlowConfiguration} object.
* @throws ResponseStatusException when FlowConfigurations are not found.
*/
protected FlowConfiguration getFlowConfigurationByName(String flow) {
List<FlowConfiguration> flowConfigurationList = flowConfigurations.stream().filter(
Expand Down Expand Up @@ -103,17 +105,15 @@ protected Boolean doesFlowExist(String flow) {
/**
* Throws a {@link ResponseStatusException} when called that includes the status, {@code HttpStatus.NOT_FOUND}, and an error message.
*
* @param flow {@link String} of the flow name.
* @param screen Screen name of a flow
* @param flow {@link String} of the flow name.
* @param screen Screen name of a flow
* @param message Message about the request issue
*
* @throws ResponseStatusException Throws a {@link ResponseStatusException} when called.
*/
protected static void throwNotFoundError(String flow, String screen, String message) {
throw new ResponseStatusException(HttpStatus.NOT_FOUND,
String.format("There was a problem with the request (flow: %s, screen: %s): %s",
flow, screen, message));

}

/**
Expand Down Expand Up @@ -143,8 +143,8 @@ public Submission findOrCreateSubmission(HttpSession httpSession, String flow) {
*
* @param session The {@link HttpSession} the user is in
* @param flow The flow to look up the submission ID for
* @throws ResponseStatusException if {@code throwNotFoundError()} is called.
* @return The submission id if it exists for the given flow, else null
* @throws ResponseStatusException if {@code throwNotFoundError()} is called.
*/
public static UUID getSubmissionIdForFlow(HttpSession session, String flow) {
if (session == null) {
Expand Down
52 changes: 39 additions & 13 deletions src/main/java/formflow/library/ScreenController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ ModelAndView getScreen(
@PathVariable(name = "flow") String requestFlow,
@PathVariable(name = "screen") String requestScreen,
@RequestParam(required = false) Map<String, String> query_params,
@RequestParam(value = "uuid", required = false) String uuid,
@RequestParam(value = "uuid", required = false) String requestUuid,
RedirectAttributes redirectAttributes,
HttpSession httpSession,
HttpServletRequest request,
Expand All @@ -126,6 +126,11 @@ ModelAndView getScreen(

Submission submission = findOrCreateSubmission(httpSession, flow);

String uuid = null;
if (requestUuid != null && !requestUuid.isBlank()) {
uuid = getValidatedIterationUuid(submission, currentScreen, requestUuid);
}

if (shouldRedirectDueToLockedSubmission(screen, submission, flow)) {
String lockedSubmissionRedirectUrl = getLockedSubmissionRedirectUrl(flow, redirectAttributes, locale);
return new ModelAndView("redirect:" + lockedSubmissionRedirectUrl);
Expand All @@ -137,6 +142,7 @@ ModelAndView getScreen(
if (uuid == null) {
return new ModelAndView(String.format("redirect:/flow/%s/%s", flow, nextViewableScreen));
} else {

return new ModelAndView(String.format("redirect:/flow/%s/%s/%s", flow, nextViewableScreen, uuid));
}
}
Expand Down Expand Up @@ -340,15 +346,15 @@ private ModelAndView handlePost(
*
* @param requestFlow The current flow name, not null
* @param requestScreen The current screen name in the subflow, not null
* @param uuid The uuid of a subflow entry, not null
* @param requestUuid The uuid of a subflow entry, not null
* @param httpSession The current httpSession, not null
* @return the screen template with model data
*/
@GetMapping({FLOW_SCREEN_PATH + "/{uuid}", FLOW_SCREEN_PATH + "/{uuid}/edit"})
ModelAndView getSubflowScreen(
@PathVariable(name = "flow") String requestFlow,
@PathVariable(name = "screen") String requestScreen,
@PathVariable String uuid,
@PathVariable(name = "uuid") String requestUuid,
HttpSession httpSession,
HttpServletRequest request,
RedirectAttributes redirectAttributes,
Expand All @@ -359,14 +365,15 @@ ModelAndView getSubflowScreen(
request.getRequestURI().toLowerCase(),
requestFlow,
requestScreen,
uuid);
requestUuid);
// Checks if screen and flow exist
ScreenConfig screenConfig = getScreenConfig(requestFlow, requestScreen);
ScreenNavigationConfiguration currentScreen = screenConfig.getScreenNavigationConfiguration();
String flow = screenConfig.getFlowName();
String screen = currentScreen.getName();

Submission submission = getSubmissionFromSession(httpSession, flow);
String uuid = getValidatedIterationUuid(submission, currentScreen, requestUuid);

if (shouldRedirectDueToLockedSubmission(screen, submission, flow)) {
String lockedSubmissionRedirectUrl = getLockedSubmissionRedirectUrl(flow, redirectAttributes, locale);
Expand Down Expand Up @@ -551,7 +558,7 @@ ModelAndView deleteConfirmation(
/**
* Deletes a subflow's input data set, based on that set's uuid.
*
* @param flow The current flow name, not null
* @param requestFlow The current flow name, not null
* @param subflow The current subflow name, not null
* @param uuid Unique id associated with the subflow's data, not null
* @param httpSession The HTTP session if it exists, not null
Expand All @@ -560,7 +567,7 @@ ModelAndView deleteConfirmation(
*/
@PostMapping("{flow}/{subflow}/{uuid}/delete")
ModelAndView deleteSubflowIteration(
@PathVariable String flow,
@PathVariable(name = "flow") String requestFlow,
@PathVariable String subflow,
@PathVariable String uuid,
HttpSession httpSession,
Expand All @@ -571,11 +578,14 @@ ModelAndView deleteSubflowIteration(
log.info(
"POST deleteSubflowIteration (url: {}): flow: {}, uuid: {}",
request.getRequestURI().toLowerCase(),
flow,
requestFlow,
uuid);
// Checks to make sure flow exists
String subflowEntryScreen = getFlowConfigurationByName(flow).getSubflows().get(subflow)
// Checks to make sure flow exists; if it doesn't an error is thrown
FlowConfiguration flowConfiguration = getFlowConfigurationByName(requestFlow);
String subflowEntryScreen = flowConfiguration.getSubflows().get(subflow)
.getEntryScreen();
String flow = flowConfiguration.getName();

Submission submission = getSubmissionFromSession(httpSession, flow);
if (submission == null) {
throw new ResponseStatusException(HttpStatus.BAD_REQUEST);
Expand Down Expand Up @@ -608,7 +618,7 @@ ModelAndView deleteSubflowIteration(

String reviewScreen = getFlowConfigurationByName(flow).getSubflows().get(subflow)
.getReviewScreen();
return new ModelAndView(String.format("redirect:/flow/%s/" + reviewScreen, flow));
return new ModelAndView("redirect:/flow/%s/%s".formatted(flow, reviewScreen));
}

/**
Expand All @@ -625,9 +635,10 @@ ModelAndView navigation(
@PathVariable(name = "screen") String requestScreen,
HttpSession httpSession,
HttpServletRequest request,
@RequestParam(required = false) String uuid
@RequestParam(name = "uuid", required = false) String requestUuid
) {
log.info("GET navigation (url: {}): flow: {}, screen: {}", request.getRequestURI().toLowerCase(), requestFlow, requestScreen);
// Checks if the flow and screen exist
ScreenConfig screenConfig = getScreenConfig(requestFlow, requestScreen);
ScreenNavigationConfiguration currentScreen = screenConfig.getScreenNavigationConfiguration();
String flow = screenConfig.getFlowName();
Expand All @@ -636,11 +647,16 @@ ModelAndView navigation(
Submission submission = getSubmissionFromSession(httpSession, flow);
log.info(
"Current submission ID is: {} and current Session ID is: {}", submission.getId(), httpSession.getId());
// Checks if the screen and flow exist
if (submission == null) {
throwNotFoundError(flow, screen,
String.format("Submission not found in session for flow '{}', when navigating to '{}'", flow, screen));
}

String uuid = null;
if (requestUuid != null && !requestUuid.isBlank()) {
uuid = getValidatedIterationUuid(submission, currentScreen, requestUuid);
}

String nextScreen = getNextViewableScreen(flow, getNextScreenName(submission, currentScreen, uuid), uuid, submission);

boolean isCurrentScreenLastInSubflow = getScreenConfig(flow, nextScreen).getScreenNavigationConfiguration().getSubflow() == null;
Expand Down Expand Up @@ -684,7 +700,8 @@ private String getNextViewableScreen(String flow, String screen, String uuid, Su
}

private String getNextScreenName(Submission submission,
ScreenNavigationConfiguration currentScreen, String subflowUuid) {
ScreenNavigationConfiguration currentScreen,
String subflowUuid) {
NextScreen nextScreen;

List<NextScreen> nextScreens = getConditionalNextScreen(currentScreen, submission, subflowUuid);
Expand Down Expand Up @@ -908,4 +925,13 @@ private String getLockedSubmissionRedirectUrl(String flow, RedirectAttributes re
return String.format("/flow/%s/%s", flow, lockedSubmissionRedirectPage);
}

private String getValidatedIterationUuid(Submission submission, ScreenNavigationConfiguration screen, String uuidToVerify) {
Map<String, Object> iteration = submission.getSubflowEntryByUuid(screen.getSubflow(), uuidToVerify);
if (iteration == null) {
throwNotFoundError(submission.getFlow(), screen.getName(),
String.format("UUID ('%s') not found in iterations for subflow '%s' in flow '%s', when navigating to '%s'",
uuidToVerify, screen.getSubflow(), submission.getFlow(), screen.getName()));
}
return (String) iteration.get("uuid");
}
}
53 changes: 51 additions & 2 deletions src/test/java/formflow/library/controllers/ScreenControllerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import formflow.library.data.Submission;
import formflow.library.utilities.AbstractMockMvcTest;
import formflow.library.utilities.FormScreen;

import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -216,7 +217,7 @@ public void shouldHandleSubflowsWithAGetAndThenAPost() throws Exception {
.get("subflowWithAGetAndThenAPost");
assertThat((Boolean) iterationsAfterSecondPost.get(0).get("iterationIsComplete")).isTrue();
}

@Test
public void shouldHandleGoingFromANonSubflowPostScreenIntoASubflow() throws Exception {
setFlowInfoInSession(session, "testSubflowLogic", submission.getId());
Expand All @@ -228,7 +229,7 @@ public void shouldHandleGoingFromANonSubflowPostScreenIntoASubflow() throws Exce
);
String nextScreenUrl = "/flow/testSubflowLogic/testEntryScreen/navigation";
result.andExpect(redirectedUrl(nextScreenUrl));

while (Objects.requireNonNull(nextScreenUrl).contains("/navigation")) {
// follow redirects
nextScreenUrl = mockMvc.perform(get(nextScreenUrl).session(session))
Expand All @@ -240,6 +241,54 @@ public void shouldHandleGoingFromANonSubflowPostScreenIntoASubflow() throws Exce
FormScreen nextScreen = new FormScreen(mockMvc.perform(get(nextScreenUrl)));
assertThat(nextScreen.getTitle()).isEqualTo("Subflow Page");
}

@Test
public void shouldTriggerErrorWhenRequestedIterationUuidForNavigationIsBad() throws Exception {
setFlowInfoInSession(session, "yetAnotherTestFlow", submission.getId());
mockMvc.perform(post("/flow/yetAnotherTestFlow/subflowAddItem/new")
.session(session)
.params(new LinkedMultiValueMap<>(Map.of(
"textInput", List.of("textInputValue"),
"numberInput", List.of("10"))))
);
UUID testSubflowLogicUUID = ((Map<String, UUID>) session.getAttribute(SUBMISSION_MAP_NAME)).get("yetAnotherTestFlow");
Submission submissionAfterFirstPost = submissionRepositoryService.findById(testSubflowLogicUUID).get();
List<Map<String, Object>> iterationsAfterFirstPost = (List<Map<String, Object>>) submissionAfterFirstPost.getInputData()
.get("subflowWithAGetAndThenAPost");
String uuidString = (String) iterationsAfterFirstPost.get(0).get("uuid");

mockMvc.perform(get("/flow/yetAnotherTestFlow/getScreen/navigation?uuid=" + "1234-bad-1234").session(session)).andExpect(
status().isNotFound());

mockMvc.perform(get("/flow/yetAnotherTestFlow/getScreen/navigation?uuid=" + uuidString).session(session)).andExpect(
status().is3xxRedirection());

mockMvc.perform(get("/flow/yetAnotherTestFlow/getScreen/navigation").session(session)).andExpect(
status().is3xxRedirection());
}

@Test
public void shouldTriggerErrorWhenRequestedIterationUuidForGetIsBadInSubflow() throws Exception {
setFlowInfoInSession(session, "yetAnotherTestFlow", submission.getId());
mockMvc.perform(post("/flow/yetAnotherTestFlow/subflowAddItem/new")
.session(session)
.params(new LinkedMultiValueMap<>(Map.of(
"textInput", List.of("textInputValue"),
"firstNameSubflowPage2", List.of("FirstName"),
"numberInput", List.of("10"))))
);
UUID testSubflowLogicUUID = ((Map<String, UUID>) session.getAttribute(SUBMISSION_MAP_NAME)).get("yetAnotherTestFlow");
Submission submissionAfterFirstPost = submissionRepositoryService.findById(testSubflowLogicUUID).get();
List<Map<String, Object>> iterationsAfterFirstPost = (List<Map<String, Object>>) submissionAfterFirstPost.getInputData()
.get("subflowWithAGetAndThenAPost");
String uuidString = (String) iterationsAfterFirstPost.get(0).get("uuid");

mockMvc.perform(get("/flow/yetAnotherTestFlow/subflowAddItemPage2/" + "1234-bad-1234").session(session)).andExpect(
status().isNotFound());

mockMvc.perform(get("/flow/yetAnotherTestFlow/subflowAddItemPage2/" + uuidString).session(session)).andExpect(
status().is2xxSuccessful());
}
}

@Nested
Expand Down

0 comments on commit ecda742

Please sign in to comment.