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

feat: Refactor Inbound package to provide access to SendGrid's pre-processing #443

Merged
merged 16 commits into from Feb 9, 2022

Conversation

qhenkart
Copy link
Contributor

@qhenkart qhenkart commented Oct 9, 2021

Resolves #439
Resolves #444

The inbound package as it is currently written cannot work in a reasonable fashion for several reasons

Primarily the package currently relies on the email field to extract attachments and email body https://github.com/sendgrid/sendgrid-go/blob/main/helpers/inbound/inbound.go#L35. However the email field is empty unless you select the RAW setting in SendGrid. This means that without the RAW setting, you will only have headers and no access to other fields

SendGrid provides a large set of fields that should be utilized, verified and checked that this library does not give access to. and the RAW setting should not be required to use this library

I have maintained the previous field names and functionality so there should be no breaking change. I have added a number of features that SendGrid already provides https://docs.sendgrid.com/for-developers/parsing-email/setting-up-the-inbound-parse-webhook

email := inbound.Parse skips the attachments but still provides the informative fields provided by SendGrid
email := inbound.ParseWithAttachments includes the attachment files

email.TextBody now reflects the parsed text/plain field
email.ParsedValues now provide access to all fields preprocessed by SendGrid
email.ParsedAttachments now provide access to the actual files, that is matched and attached to the information processed by SendGrid

email.Envelope is unmarshalled and provides email.Envelope.From which is the actual sender's email address without additional characters

email.Validate() will ensure that all of the DKIM and SPF values are passing to avoid spoofing and phishing attacks.

This PR provides functionality that matches the Sendgrid inbound capabilities listed in the official documentation. We are using it in production and it works great.

Check out the examples here

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the Contribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file a support ticket, or create a GitHub Issue in this repository.

@qhenkart qhenkart changed the title <fix> Refactor Inbound package to provide access to SendGrid's pre-processing fix: Refactor Inbound package to provide access to SendGrid's pre-processing Oct 9, 2021
@qhenkart qhenkart changed the title fix: Refactor Inbound package to provide access to SendGrid's pre-processing fix/feat: Refactor Inbound package to provide access to SendGrid's pre-processing Oct 9, 2021
@qhenkart
Copy link
Contributor Author

I have updated the code and documentation to patch #444, and to warn people away from the email.Headers field

@shwetha-manvinkurke shwetha-manvinkurke changed the title fix/feat: Refactor Inbound package to provide access to SendGrid's pre-processing feat: Refactor Inbound package to provide access to SendGrid's pre-processing Oct 18, 2021
helpers/inbound/README.md Outdated Show resolved Hide resolved
helpers/inbound/README.md Outdated Show resolved Hide resolved
helpers/inbound/README.md Outdated Show resolved Hide resolved
helpers/inbound/inbound.go Outdated Show resolved Hide resolved
helpers/inbound/inbound.go Show resolved Hide resolved
helpers/inbound/inbound.go Show resolved Hide resolved
helpers/inbound/inbound_test.go Show resolved Hide resolved
@qhenkart
Copy link
Contributor Author

sorry for the delay, I have not had a change to return to this and respond or look into the review issues. I hope to have some time to get to it in the next week or so

@mewis
Copy link

mewis commented Jan 7, 2022

Hi @qhenkart.
What is the progress of this? Is there anyway I can help as I have also come across the issue with outlook?

Thank you

@qhenkart
Copy link
Contributor Author

@mewis thank you for the reminder. I've put aside some time to fix up the minor updates, and I will handle any continued code review updates faster. Keep an eye out for the approval

@ryskiz
Copy link

ryskiz commented Feb 9, 2022

Any updates on this @shwetha-manvinkurke @qhenkart ?

This was referenced Feb 9, 2022
@shwetha-manvinkurke shwetha-manvinkurke merged commit 5746b4b into sendgrid:main Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants