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

Handle multiple auth results #51

Merged
merged 2 commits into from
Apr 14, 2024
Merged

Handle multiple auth results #51

merged 2 commits into from
Apr 14, 2024

Conversation

moorereason
Copy link
Contributor

This PR attempts to resolve #45. From the main commit message:

The DMARC spec allows for zero or more DKIM results and one or more SPF
results. Prior to this commit, we only allowed for a single result for
each type. Update the dmarc.AuthResults struct to use a slice for each
type.

For reference, see the definition of AuthResultType in Appendix C of RFC 7489.

This change in the data model required updates to the Go templates. The
two HTML templates are almost exactly the same, so pull the common body
out into a separate const variable to ease maintenance.

Additional comments to help evaluate the code changes:

  • Modified the existing test xml files to add at least one test with multiple DKIM results.
  • On the HTML report, separate multiple results with a <br/> tag.
  • The text template got a little complicated. I'm not sure I'm happy with the control structures, but it should be fairly simple to understand. When dealing with additional DKIM results, print an additional line with only the DKIM result:
*   111.22.111.33 |    1 |       none |     pass |     fail |        abcde.com |      pass |        abcde.com |     pass | 
                  |      |            |          |          |    sendgrid.info |      pass |                  |          | 

The DMARC spec allows for zero or more DKIM results and one or more SPF
results.  Prior to this commit, we only allowed for a single result for
each type.  Update the dmarc.AuthResults struct to use a slice for each
type.

For reference, see the definition of AuthResultType in Appendix C of RFC
7489.

This change in the data model required updates to the Go templates.  The
two HTML templates are almost exactly the same, so pull the common body
out into a separate const variable to ease maintenance.

Fixes tierpod#45
@moorereason
Copy link
Contributor Author

I forgot to mention that this PR is a breaking change for the JSON output since the data model changes. If anyone has automatic processing of the JSON output, they'll need to update their code.

@tierpod
Copy link
Owner

tierpod commented Apr 14, 2024

Great! Thank you

breaking change for the JSON output

I'll mention it in UPGRADING.md for the next release 👍

@tierpod tierpod merged commit 96ba341 into tierpod:master Apr 14, 2024
@moorereason moorereason deleted the iss45 branch May 6, 2024 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

incorrect parsing Auth results DKIM
2 participants