Skip to content
This repository was archived by the owner on May 14, 2021. It is now read-only.

Improvements for AssetDetails, Tags & Unique Identifiers + Adhoc Reports#43

Merged
gschneider-r7 merged 4 commits into
rapid7:masterfrom
asecurityteam:master
Nov 20, 2017
Merged

Improvements for AssetDetails, Tags & Unique Identifiers + Adhoc Reports#43
gschneider-r7 merged 4 commits into
rapid7:masterfrom
asecurityteam:master

Conversation

@fruechel

Copy link
Copy Markdown
Contributor

Tags & Unique Identifiers are now supported similar to the Ruby client. Adhoc reports can be generated in more than XML.

Description

See commit messages for more details. They outline each change.

Motivation and Context

These changes are needed for our use case of Nexpose. All changes are backwards-compatible improvements which is why they are in a PR.

How Has This Been Tested?

No additional tests have been added but all changes have been verified and are being used by our code. All old tests still work (which is expected given that all changes are backwards-compatible). Changes have only been tested in Python 3. The only potential problem is the new CSV parsing function which has been explicitly tested & verified for Python 2 as well.

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have updated the documentation accordingly (if changes are required).
  • I have added tests to cover my changes.
  • All new and existing tests passed.

There's no docs for any of the areas I touched so I didn't add any either. Same goes for tests.

GetAssetDetails can sometimes throw a 500 on one of the details URLs
(e.g. assessment). This change allows those errors to be skipped. In
addition, if the field `assessment` isn't present when creating the
asset object, this is now handled properly. Future changes might want to
expand this out to other fields as well.
Ensure tags are fetched for AssetDetails as well & ensure both Tag &
UniqueIdentifier objects are created on the AssetDetails object.
Adhoc reports can now be generated in different formats using different
templates. Support for CSV is included as well, other formats can be
requested but need to be parsed manually. The change retains backwards
compatibility.
Comment thread nexpose/nexpose.py
body = ''.join(data[4:-1])
boundary_bottom = data[-1]
if boundary_top != boundary_bottom[:-2]:
raise ValueError("Invalid boundary")

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I replaced the assertions as per Python recommendation: asserts could be compiled out so throwing exceptions is the preferred method here.

@gschneider-r7

Copy link
Copy Markdown
Contributor

I fixed (well, more like bandaid) the flake8 issues on master so if you rebase travis-ci should pass on this PR.

@gschneider-r7

Copy link
Copy Markdown
Contributor

Also as far as reporting goes, let me know if you have any feedback on #37

@fruechel

Copy link
Copy Markdown
Contributor Author

I merged in master, seems this worked. As for #37 it looks like we should combine the changes since they touch similar parts. I'll have a closer look. If you want I can remove the report commit from this PR and we work on #37 to incorporate both changes. Does that work for you?

@gschneider-r7

Copy link
Copy Markdown
Contributor

I don't mind having your report changes here as well since this PR could probably get into a released version sooner than #37. Feel free to PR any suggested changes to the report_configs branch, then I can review and merge to #37.

@fruechel

Copy link
Copy Markdown
Contributor Author

Does this mean we can merge these changes into master or do you need anything from me before making that merge?

Comment thread nexpose/nexpose.py
<AdhocReportConfig format="{format}" template-id="{template_id}">
<Filters>
<filter type="scan" id="{0}" />
<filter type="scan" id="{scan_id}" />

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

where does scan_id come from?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It comes from id as part of the method signature. It's always been there, I just renamed it for clarity.

@gschneider-r7 gschneider-r7 merged commit 140debc into rapid7:master Nov 20, 2017
@gschneider-r7

Copy link
Copy Markdown
Contributor

These changes are published in v0.1.7, available on pypi now.

@fruechel

Copy link
Copy Markdown
Contributor Author

That was really fast, thank you!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants