Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Manual merge/develop 226 #316

Merged
merged 24 commits into from Apr 9, 2019
Merged

Manual merge/develop 226 #316

merged 24 commits into from Apr 9, 2019

Conversation

vifor2
Copy link
Contributor

@vifor2 vifor2 commented Apr 9, 2019

Feature functional although it could use some improvements (hence the EXPERIMENTAL disclaimer when using --help), but since there is less than 1 week before the end of the project @zer0x64 and me would like to get this merged ASAP! Also to cease the merge conflicts (pl0x).

This PR but less chaotic.
Implements this issue.

Scroll down for TL;DR

Changes done within this PR :

  • User can input --result-format with either json or sqlite as args.
  • Imports of scoutsuite-results/scoutsuite_results.js and scoutsuite-results/scoutsuite_exceptions.js are no longer in report.html by default, they are instead in a partial that is added if the user chose json in the previous point.
  • scoutsuite.js now checks whether an element with the id json_format or sqlite_format exists to know which functions to call.
  • When choosing SQLite all of the user's data is saved as a .db file instead of a .js file, these files being more compact as well as readable by a server.
  • User can input arguments related to the server (reminder: we have decided that when the user selects SQLite he must then enter a second command to run the server, it will not be launched automatically).
  • Exposed all of the data saved in the .db file to our client through CherryPy.
  • Modified all .js files to follow standardJS, there are still some changes to do.
  • All data except resource pages other than the first one are fetched at the start and properly viewable within the report.
  • Added Back and Next buttons to change page, pressing one of those buttons disables it if you're at the page limit afterwards ([0, totalResources / pageSize - 1]), it removes the data of the previous resource from memory (run_results) and add the new page to it, it then overwrites the current template with the same one but with the new data within it.
  • Page size is currently hard-coded it would be nice to give the option to the user of choosing the size but Id rather get this issue done and merged beforehand. Same with more buttons to go to first page, last page, page at a specific index, modal that tells the user to make sure his port 8000 is freed and the server is running, etc.
  • I know that when we talked I said Id keep the paging buttons on both result formats but for now the buttons are disabled when viewing a report in JSON format (it should look as if nothing had happened) while the SQLite one have 2 buttons.
  • Merged develop and it's new architecture into this branch.

Known issues :

  • Works with Firefox but not with Chrome, will discuss about this issue tomorrow with @j4v : jquery-3.3.1.min.js:2 Access to XMLHttpRequest at 'file:///home/vifor2/Documents/ScoutSuite/scoutsuite-report/inc-scoutsuite/sqlite.js' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https.
  • Items in the navbar are not in alphabetical order although they are in the JavaScript object (I'm bamboozled).
  • Constant error when used with Firefox although it doesn't seem to actually cause any problem : XML Parsing Error: syntax error Location: file:///home/vifor2/Documents/ScoutSuite/scoutsuite-report/inc-scoutsuite/sqlite.js Line Number 1, Column 1:
  • Initial data loading is a bit long compared to other providers because of the 16 regions per resource.

TL;DR

Example with Azure

While in /ScoutSuite open two terminals, in the first one type in azure --cli --serve and in the second one your usual command followed by --result-format sqlite, you can then view the report has usually excepted for the fact that you'll have buttons to load in/out new pages.

Example with GCP

Same as concept as Azure but replace the first command with gcp --user-account --serve

Example with AWS

Same concept as previous ones but replace the first command with aws --serve

Or you can only use one terminal, --no-browser and the 2 commands in the right order...

@vifor2 vifor2 added enhancement New feature or request component-core Affects core component-UI Affects UI cleanup Code cleanup labels Apr 9, 2019
@vifor2 vifor2 added this to the Iteration #6 milestone Apr 9, 2019
@vifor2 vifor2 self-assigned this Apr 9, 2019
@vifor2 vifor2 added this to In progress in Scout Suite via automation Apr 9, 2019
Copy link
Contributor

@zer0x64 zer0x64 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

rip my commit :'(

@codecov-io
Copy link

Codecov Report

Merging #316 into develop will decrease coverage by 0.31%.
The diff coverage is 26.16%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #316      +/-   ##
===========================================
- Coverage    34.61%   34.29%   -0.32%     
===========================================
  Files          176      179       +3     
  Lines         5764     5974     +210     
===========================================
+ Hits          1995     2049      +54     
- Misses        3769     3925     +156
Impacted Files Coverage Δ
ScoutSuite/providers/aws/provider.py 9.54% <0%> (-0.03%) ⬇️
ScoutSuite/core/cli_parser.py 13.15% <0%> (-0.55%) ⬇️
ScoutSuite/providers/azure/provider.py 33.33% <0%> (-1.45%) ⬇️
ScoutSuite/providers/base/provider.py 10.04% <0%> (ø) ⬆️
ScoutSuite/providers/gcp/provider.py 12.82% <0%> (-0.12%) ⬇️
ScoutSuite/__init__.py 100% <100%> (ø) ⬆️
ScoutSuite/output/report_file.py 100% <100%> (ø)
ScoutSuite/output/utils.py 21.42% <14.81%> (-0.53%) ⬇️
ScoutSuite/output/html.py 21.78% <20.51%> (-0.84%) ⬇️
ScoutSuite/output/result_encoder.py 24% <24%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33841dd...20a7cc8. Read the comment docs.

@codecov-io
Copy link

codecov-io commented Apr 9, 2019

Codecov Report

Merging #316 into develop will decrease coverage by 0.31%.
The diff coverage is 26.25%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #316      +/-   ##
===========================================
- Coverage    34.61%   34.29%   -0.32%     
===========================================
  Files          176      179       +3     
  Lines         5764     5974     +210     
===========================================
+ Hits          1995     2049      +54     
- Misses        3769     3925     +156
Impacted Files Coverage Δ
ScoutSuite/providers/aws/provider.py 9.54% <0%> (-0.03%) ⬇️
ScoutSuite/core/cli_parser.py 13.15% <0%> (-0.55%) ⬇️
ScoutSuite/providers/azure/provider.py 33.33% <0%> (-1.45%) ⬇️
ScoutSuite/providers/base/provider.py 10.04% <0%> (ø) ⬆️
ScoutSuite/providers/gcp/provider.py 12.82% <0%> (-0.12%) ⬇️
ScoutSuite/__init__.py 100% <100%> (ø) ⬆️
ScoutSuite/output/report_file.py 100% <100%> (ø)
ScoutSuite/output/utils.py 21.42% <14.81%> (-0.53%) ⬇️
ScoutSuite/output/html.py 21.78% <21.05%> (-0.84%) ⬇️
ScoutSuite/output/result_encoder.py 24% <24%> (ø)
... and 9 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 33841dd...a1be1b7. Read the comment docs.


/**
* Acts like getResourcePageSqlite but when we're using regions, made a separate function since the order of
* the variables are different and it was getting confusing
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand why we need a separate method for services with regions. It seems like this won't scale well with other providers. For example, GCP has projects, which I assume is currently not supported.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made separate functions to limit the amount of if/elsr clauses. But yeah, more needs to be done for sure.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant why do we need specific pieces of code for both cases. The comment is a bit unclear, what variables are different? And how are the projects handled?

ScoutSuite/output/html.py Outdated Show resolved Hide resolved
@@ -25,7 +25,7 @@ class BaseProvider(object):
"""

def __init__(self, report_dir=None, timestamp=None, services=None, skipped_services=None, thread_config=4,
**kwargs):
result_format='json', **kwargs):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does the provider need to know about the result format?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's @vifor2 works.
That might have to do with the report generation, so it ends up written somewhere in the javascript or HTML that the report is supposed to be hooked to a server. I see it stored in the provider object at least in Azure, though I don't know if it's used anywhere, and in the base provider it is not used anywhere. Might just be leftovers

Copy link
Contributor Author

@vifor2 vifor2 Apr 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah depeding on the type of report, I attach or not certain elements to the report, e.g. the script sqlite.js is not attached if creating a json report. This also helps me determine which functions to run in scoutsuite.js

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does the provider have to do with this though? :/

Copy link
Contributor

@Aboisier Aboisier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great job guys! It seems like there could be some improvement in the cleanness department, but it's a great proof of concept 🥇

EDIT:
I checked out the branch in order to test. Is the JSON format supposed to work on chrome? I'm getting a cors error.

jquery-3.3.1.min.js:2 Access to XMLHttpRequest at 'file:///C:/ScoutSuite/scoutsuite-report/scoutsuite-results/scoutsuite_results-aws.js' from origin 'null' has been blocked by CORS policy: Cross origin requests are only supported for protocol schemes: http, data, chrome, chrome-extension, https.

EDITEDIT:
The CORS problem can be fixed by starting chrome with the --allow-file-access-from-files flag.

I'm getting this error log when scanning.

2019-04-09 10:47:24 SURFACE-ANTOINE scout[6792] INFO Applying exceptions
2019-04-09 10:47:24 SURFACE-ANTOINE scout[6792] ERROR scout.py L186: Failed to load exceptions: 'NoneType' object has no attribute 'replace'

@zer0x64
Copy link
Contributor

zer0x64 commented Apr 9, 2019

@Aboisier Yeah, CORS crashing on chrome is a known issue(we talked about it with @j4v last week). Nice job finding the flag, we'll add it to the doc!

Aboisier and others added 8 commits April 9, 2019 13:43
Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
…vpcs.security_groups.resource_list.html

Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
Co-Authored-By: vifor2 <fortin.vincent@hotmail.com>
@zer0x64
Copy link
Contributor

zer0x64 commented Apr 9, 2019

I'm getting this error log when scanning.

2019-04-09 10:47:24 SURFACE-ANTOINE scout[6792] INFO Applying exceptions
2019-04-09 10:47:24 SURFACE-ANTOINE scout[6792] ERROR scout.py L186: Failed to load exceptions: 'NoneType' object has no attribute 'replace'

This is on develop too, and happens when there are exceptions(to enter the if execptions:) and the profile is None, so on both GCP and Azure.

@vifor2 vifor2 merged commit a8373cc into develop Apr 9, 2019
Scout Suite automation moved this from In progress to Done Apr 9, 2019
@vifor2 vifor2 deleted the manual-merge/develop-226 branch April 9, 2019 20:33
@x4v13r64
Copy link
Collaborator

This is on develop too, and happens when there are exceptions(to enter the if execptions:) and the profile is None, so on both GCP and Azure.

Should be fixed in #327.

x4v13r64 added a commit that referenced this pull request Apr 10, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Code cleanup component-core Affects core component-UI Affects UI enhancement New feature or request
Projects
No open projects
Scout Suite
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants