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

Add sendgrid username/password authentication #1297

Merged
merged 1 commit into from Jun 29, 2018
Merged

Add sendgrid username/password authentication #1297

merged 1 commit into from Jun 29, 2018

Conversation

@c-w
Copy link
Contributor

@c-w c-w commented Jun 27, 2018

This change fixes an issue where the Azure PaaS deployments were unable
to send mail via Sendgrid.

The issue was caused by the fact that the Sendgrid API assumes
authentication via an API key, but in the ARM template we only have
access to the Sendgrid account admin username and password. Due to the
limitations of the ARM template format, it's difficult to automatically
generate the API key outside of the OKpy application during the
deployment: we'd have to spin up an Azure Container Instances
deployment, generate the key in the container, upload it to a storage
like Key Vault and then access the key from the OKpy application.
Reading configuration from a storage like Key Vault instead of
environment variables would be a non-trivial change to OKpy so this
commit implements a more pragmatic work-around: generate a new API key
on the fly at runtime given the Sendgrid admin credentials. The key is
cached for the lifetime of the server so this shouldn't add overhead
past the first email sent.

Note that the Sendgrid API has a limit on 100 API keys so we always
ensure to auto-create at most one API key for OKpy and delete any old
copies of the key before creating a new one (this would happen when the
OKpy server is restarted since any auto-created API key only is
persisted in the server memory).

Another nice property of this change is that it's now possible to
authenticate to Sendgrid in OKpy in the same way as we do in AutoPY.

This change fixes an issue where the Azure PaaS deployments were unable
to send mail via Sendgrid.

The issue was caused by the fact that the Sendgrid API assumes
authentication via an API key, but in the ARM template we only have
access to the Sendgrid account admin username and password. Due to the
limitations of the ARM template format, it's difficult to automatically
generate the API key outside of the OKpy application during the
deployment: we'd have to spin up an Azure Container Instances
deployment, generate the key in the container, upload it to a storage
like Key Vault and then access the key from the OKpy application.
Reading configuration from a storage like Key Vault instead of
environment variables would be a non-trivial change to OKpy so this
commit implements a more pragmatic work-around: generate a new API key
on the fly at runtime given the Sendgrid admin credentials. The key is
cached for the lifetime of the server so this shouldn't add overhead
past the first email sent.

Note that the Sendgrid API has a limit on 100 API keys so we always
ensure to auto-create at most one API key for OKpy and delete any old
copies of the key before creating a new one (this would happen when the
OKpy server is restarted since any auto-created API key only is
persisted in the server memory).

Another nice property of this change is that it's now possible to
authenticate to Sendgrid in OKpy in the same way as we do in AutoPY.
@c-w
Copy link
Contributor Author

@c-w c-w commented Jun 27, 2018

CC @marrobi since this affects the ARM templates.

Copy link
Member

@colinschoen colinschoen left a comment

LGTM Wow this is great.

if not response.ok:
raise ValueError('unable to list sendgrid api keys')

existing_key = next((key['api_key_id']
Copy link
Member

@colinschoen colinschoen Jun 27, 2018

Nice :)

@colinschoen colinschoen merged commit ed72d06 into okpy:master Jun 29, 2018
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants