cisagov / tpt-reports

Process to build and distribute Technical Phishing Test (TPT) reports
Creative Commons Zero v1.0 Universal
4 stars 0 forks source link

Add validation for DOMAIN_TESTED argument #37

Closed JCantu248 closed 12 months ago

JCantu248 commented 1 year ago

πŸ—£ Description

Add code to validate the DOMAIN_TESTED argument is a valid email domain.

πŸ’­ Motivation and context

This code will validate user input for sanity checks, to ensure the supplied email domain is in the correct format. This PR resolves issue https://github.com/cisagov/tpt-reports/issues/36

πŸ§ͺ Testing

Testing on usage arguments, with valid and invalid email formatted string for DOMAIN_TESTED argument.

βœ… Pre-approval checklist

βœ… Pre-merge checklist

βœ… Post-merge checklist

schmelz21 commented 1 year ago

Requirement Update - It was determined that the argument is strictly an email domain, and does not necessarily linked to a web/request url. So the goal of this ticket is to create a validation on the string only.

dav3r commented 1 year ago

I see that you have linked this PR with issue #36. Typically, we also like to explicitly put something like "This PR resolves #36." in the PR's "Motivation and context" section above.

coveralls commented 1 year ago

Pull Request Test Coverage Report for Build 6199486455


Totals Coverage Status
Change from base Build 6199482471: 0.0%
Covered Lines: 0
Relevant Lines: 0

πŸ’› - Coveralls
dav3r commented 1 year ago

One thing I just noticed is that you have the checkbox for post-merge item "Create a release" checked. That shouldn't be checked yet since this PR hasn't been merged yet. Also, you have not been using bump_version.sh (or any other mechanism, but that is the easiest one) to manage the version string for this repo. Once you start doing that, then it will make sense for you to start creating releases after each PR is merged.