Attempts to fix security errors around using unsanitized flow and screen inputs
#547
Conversation
| * } | ||
| * <p> | ||
| * /** | ||
| * Checks if current screen condition is met. |
There was a problem hiding this comment.
Hmmm... typo, I think. I will revert this change.
| String flowName; | ||
| String screenName; | ||
| ScreenNavigationConfiguration screenNavigationConfiguration; | ||
| } |
There was a problem hiding this comment.
If we like this, I can put it out into another class. The problem I'm trying to solve is that you can't get the flow or the screen name from the screenNavigationConfiguration object, so I thought I'd do a light weight wrapping of it with that data, for convenience. If you can think of a better way, let me know.
It's hard to change the ScreenNavigationConfiguration object because it's tightly coupled to the yaml format.
|
This doesn't appear to fix the issue, though I'm not particularly worried about these pieces of code being a security vulnerability, given the code path. |
|
I think the last change fixes the security alerts. Question is, do we like this change? |
| @@ -29,4 +32,13 @@ public class FlowConfiguration { | |||
| public ScreenNavigationConfiguration getScreenNavigation(String screenName) { | |||
| return flow.get(screenName); | |||
| } | |||
|
|
|||
| public void setFlow(Map<String, ScreenNavigationConfiguration> screenMap) { | |||
There was a problem hiding this comment.
This method will get called by SnakeYAML when it builds the flow configuration from the yaml file. This allows us to inject information, if we want.
Issue tracking number 🔗
Description of change ✍️
Creates a way to use
flowandscreenfor redirects in a way that they are first validated to be safe values to use.The
getScreenConfigwas already ensuring they were good values, but that doesn't stop the code scanning for thinking they are unsafe.Priority 🥇
Effect on other applications using FFB 🌊
Testing
✅ Checklist before requesting a review
style?