cisagov / gophish-tools

Helpful tools for interacting with a GoPhish phishing instance
Creative Commons Zero v1.0 Universal
42 stars 6 forks source link

Add function to export user report data from Gophish #33

Closed JCantu248 closed 3 years ago

JCantu248 commented 3 years ago

🗣 Description

Function was added to gather user report data for all campaigns associated with a given assessment. The data is obtained by using the GoPhish API, and written to a JSON file for later use by PCA operators. Each campaign is written to its own JSON file.

💭 Motivation and context

As part of the PCA team's effort to develop Live PCA, it is requested that user report data (that is targeted user clicking on links) be obtained from GoPhish and collected into a format that allows for import into the PCA database. This will allow for archival storage of assessment and campaign data.

🧪 Testing

Code base was tested in the Staging COOL environment. Two new campaigns were set up in GoPhish running in Staging COOL, and after their completion, the GoPhish Export command was run with the appropriate assessment name. A JSON for each campaign was generated, with properly formatted JSON.

Unfortunately since we don't have corresponding assessment and campaign documents from GoPhish loaded into the PCA DB, I wasnt able to test importing the generating JSON. What I did instead was wrote the first_report datetime into a known good and valid user report JSON file, and forced an overwrite of the document. I did this to make sure that the datetime format I was using for first_report was matching what the importer and PCA DB were expecting.

📷 Screenshots (if appropriate)

✅ Checklist

BenBreaksThings commented 3 years ago

Chiming in out of turn here, but many of the items above are characterized as recommendations or are instances of refactoring. Not that the code is broken, just preferential construct. Can we adopt a more ops focused mindset here and accept that the working code that isn't doing anything dangerous need not be prevented from progressing due to JSON alphabetizing?

felddy commented 3 years ago

Chiming in out of turn here, but many of the items above are characterized as recommendations or are instances of refactoring. Not that the code is broken, just preferential construct. Can we adopt a more ops focused mindset here and accept that the working code that isn't doing anything dangerous need not be prevented from progressing due to JSON alphabetizing?

It's all good. All chiming is welcome, at all times.

We do indeed have preferential constructs, and they're usually not just whims. We strive to give our codebase as much longevity as possible. This means that we hope that it will continue to be in useful long after we're done writing it. The more readable, and uniform we can make our projects, the better the chance they will survive changes in maintainers and owners.

It's ok to disagree with a suggestion but we'd ask that you provide your reasoning when you do. Our coding best-practices are always evolving and we're always looking to improve them.

I don't think any of the conversations and suggestions above are out of the ordinary. This is actually less of a barrage than I usually get on my PRs. 😅

In any case, thank you for contributing and running the gauntlet with us. It's not always fun, but the project will be better for it.

dav3r commented 3 years ago

@JCantu248 - I just noticed that you still need to pull in the latest changes from develop. Please take care of that when you can.