Refactor tunnel issue in UI #2551
Conversation
monkey/monkey_island/cc/ui/src/components/report-components/SecurityReport.js
Outdated
Show resolved
Hide resolved
52c120c to
ca0fb5c
Compare
|
Updated screenshot: |
monkey/monkey_island/cc/ui/src/components/report-components/SecurityReport.js
Outdated
Show resolved
Hide resolved
| @@ -526,6 +537,45 @@ class ReportPageComponent extends AuthComponent { | |||
| return <ul>{issuesDivArray}</ul>; | |||
| }; | |||
|
|
|||
| createTunnelingIssueComponent(agents, machines) { | |||
There was a problem hiding this comment.
Can we move this to the TunnelIssue.js?
| let tunnelingIssues = []; | ||
| for (let agent of agents) { | ||
| if (!islandIPs.includes(agent.cc_server.ip)) { | ||
| let agentMachine = getMachineByAgent(agent, machines); | ||
| if (agentMachine !== null) { | ||
| let agentMachineHostname = getMachineHostname(agentMachine) | ||
|
|
||
| let agentTunnelMachineHostname = agent.cc_server.ip; | ||
| let agentTunnelMachine = getMachineFromIP(agent.cc_server.ip, machines) | ||
| if (agentTunnelMachine !== null) { | ||
| agentTunnelMachineHostname = getMachineHostname(agentTunnelMachine); | ||
| } | ||
|
|
||
| tunnelingIssues.push({ | ||
| 'agent_machine': agentMachineHostname, | ||
| 'agent_tunnel': agentTunnelMachineHostname | ||
| }); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
This logic looks like it belongs in the tunnelIssueReport.js component
| @@ -356,6 +361,11 @@ class ReportPageComponent extends AuthComponent { | |||
| overviews.push(this.getIssueOverview(this.IssueDescriptorEnum[issues[i]])); | |||
| } | |||
| } | |||
|
|
|||
| if (this.tunnelingIssueExists === true) { | |||
| overviews.push(this.getIssueOverview(this.IssueDescriptorEnum['tunnel'])) | |||
There was a problem hiding this comment.
Always push the tunneling issue overview. The overview will return <></> if no tunneling happened
| @@ -444,6 +454,7 @@ class ReportPageComponent extends AuthComponent { | |||
| <h3>Machine related recommendations</h3> : null} | |||
| <div> | |||
| {this.generateIssues(this.state.report.recommendations.issues)} | |||
| {this.tunnelingIssueComponent} | |||
There was a problem hiding this comment.
We don't need to store this in the state. We can just call tunnelIssueReport component here and pass machines and agents. If there's no tunneling issue, the component can return <></>
| 'tunnel': { | ||
| [this.issueContentTypes.OVERVIEW]: tunnelIssueOverview, | ||
| [this.issueContentTypes.REPORT]: tunnelIssueReport, | ||
| [this.issueContentTypes.TYPE]: this.issueTypes.WARNING |
There was a problem hiding this comment.
Remove this whole thing, if we turn overview and report into standalone components, they can determine by machines and agents if they should render or not
|
The report is already not in a great shape. But at least it has it's own abstraction layer and we shouldn't cram the logic of specific issues into it. Tunneling overview and report components can decide, based on agents and machines, whether they should return data or an empty component. |
What does this PR do?
Fixes #2507
PR Checklist
Was the CHANGELOG.md updated to reflect the changes?Was the documentation framework updated to reflect the changes?Testing Checklist
Added relevant unit tests?