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

Send user-confirmation emails with SES #281

Closed
douglasnaphas opened this issue Feb 20, 2021 · 10 comments
Closed

Send user-confirmation emails with SES #281

douglasnaphas opened this issue Feb 20, 2021 · 10 comments

Comments

@douglasnaphas
Copy link
Owner

douglasnaphas commented Feb 20, 2021

Emails sent through Cognito (the default, my current setup) have like a 50 per day limit.

@douglasnaphas
Copy link
Owner Author

This has example code for getting this working using the Cfn-layer classes.

douglasnaphas added a commit that referenced this issue Feb 20, 2021
gh-281

When I tried to sign up using the hosted UI, it failed. I think this is
because Cognito couldn't confirm my users, because they didn't have
email addresses collected.

I got a failure message as shown in gh-281, but when I look in the user
pool in the AWS console, the users are there with a status of
"UNCONFIRMED."

I am expecting this change to delete and re-create the user pool,
because attributes cannot change from required to not-required after
user pool creation.
@douglasnaphas
Copy link
Owner Author

douglasnaphas commented Feb 21, 2021

SES Mailbox Simulator?

Based on this, maybe I should use the SES Mailbox Simulator in part of itest to make sure that sign-up works.

Update: This is probably not worth doing. I can't use the Simulator to click on the links that go out with the confirmation emails.

@douglasnaphas
Copy link
Owner Author

douglasnaphas commented Feb 22, 2021

us-east-1 stack needed

I'll need a separate stack, same repo, fixed in us-east-1.

As noted in the comments on aws/aws-cdk#6768,

SES integration is only available in us-east-1, us-west-2, eu-west-1

Other resources, like the Lambda@Edge functions to add security headers and the ACM cert to be sent along with the custom domain for the CloudFront distro, need to live in us-east-1 as well.

@douglasnaphas
Copy link
Owner Author

douglasnaphas commented Feb 22, 2021

TODO

  • Make a us-east-1 stack, same repo, with a resource in it.
  • Confirm what happens when I pass in an env var to the ue1 stack. Maybe I should be using CloudFormation parameters, or inputs, or whatever the CloudFormation concept is. Update: they're parameters, explained here and here, and maybe I'll use them instead of environment variables. Actually the docs linked just now say In general, we recommend against using AWS CloudFormation parameters with the AWS CDK, so I'll go with environment variables. Update 2: I'm using stack props. I'm getting the info I need through async SDK calls in bin/, and passing them down through stack props. I think this is easier to follow than env vars or context values.
  • Condition an output value for the ue1 stack on an ssm param.
  • Change the ssm param, re-run the build, and observe the output (which is from the env var input) changing.
  • Use a correct object type for interface MadLiberationUe1Props.
  • Verify a dev-env from-address. confirm-dev-1@passover.lol is now verified in the doug account, us-east-1.
  • Observe the build failing due to an unverified email address.
  • Set a verified email address, and observe the build failing due to a non-DKIM domain.
  • Figure out in bin/ which stack is being deployed. I only want to query the SES-related SSM params when deploying webapp. Not necessary, because I realized I was using getParameters wrong.
  • Deploy to douglas us-west-1, to see if using a non-sandboxed account resolves this issue. If that fixes the issue, fail if a sandboxed account is provided in bin/ (may want to do this anyway--I shouldn't deploy with a sandboxed account, at least not in prod). It doesn't.
  • Set SES-related SSM params in douglas us-east-1.
  • Validate that I can sign up and get an email from verify@passover.lol.
  • Lock the SES verification sender identity to the Cognito user pool (webapp) identity.
  • Put in a check in bin/ that the SES from-region, if provided, is supported for SES-Cognito integration. us-east-1, us-west-2, and eu-west-1 are supported. Remember that this region must be the webapp region.
  • Set up DKIM for the domain used in dev, and observe the build succeeding.
  • Use the SES workaround to verify with an SES address.
  • Confirm that new user verification works with an SES address.
  • Propagate changes to the main Action.
  • Verify an email address for the prod account.
  • Verify a sender domain for the prod account.
  • Observe email verification working in the prod account.
  • Request a service limit increase for SES in the prod account. This will close Get my SES quota raised #286. Also request to be taken out of the sandbox.
  • Check the default quota for send-rate for SES. Make sure it's acceptable for dev purposes.
  • Just make douglas the Mad Liberation prod account. It's fine. It already has its SES sending limit increased.

@douglasnaphas
Copy link
Owner Author

SES sandbox and from-address verification force manual steps

SES's requirements that new users be confirmed by text message or email threatens to make Turnkey non-repeatable without manual intervention.

Specifically, to use SES to send emails, I need to verify the from-address for the verification emails, which requires (I believe) setting up an email address outside of the CDK. Also, to send emails with SES, I need to manually create a support case requesting breakout from the SES sandbox.

Options

See if text-message confirmation can be repeatable

Maybe I can verify through text messages, though getting a phone number may require at least as much manual effort as getting an email address.

Find a way to repeatably obtain email addresses

Dynamically bypass confirmation based on SSM parameters

This doc mentions using a Pre Sign-up Lambda trigger to confirm users without sending email. This might be a good option for Turnkey deployments that are not going to face real customers, and maybe even for full production apps.

Preferred option

I like the Dynamically bypass confirmation based on SSM parameters option the best. The text-message option is too different from the desired production setup, and the repeatably-obtain-emails option probably isn't supported. The text-message option probably isn't supported in a repeatable fashion, either.

Dynamic deploys

In any case, it is probably time to figure out how to branch at deployment time based on SSM parameters.

@douglasnaphas
Copy link
Owner Author

Default setup (no SSM param data for this behavior found): pre-signup trigger confirms user. (Verify this) No working password recovery flow available. Or maybe just send emails with Cognito.

Email address provided through SSM: Use the address provided. Maybe even check at deploy time to confirm it’s verified with SES.

douglasnaphas added a commit that referenced this issue Feb 23, 2021
gh-281

Some resources, like an SES identity used with Cognito, have to be in
us-east-1.
douglasnaphas added a commit that referenced this issue Feb 23, 2021
gh-281

I'm also curious to see what happens on deploy now that there are two
stacks in the app.
douglasnaphas added a commit that referenced this issue Feb 23, 2021
gh-281

I got an error:

Since this app includes more than a single stack, specify which stacks
to use (wildcards are supported) or specify `--all`
Stacks: DouglasnaphasMadliberation281-ses-ue1
DouglasnaphasMadliberation281-ses-webapp
douglasnaphas added a commit that referenced this issue Feb 23, 2021
gh-281

This is part of testing out the syntax for using environment variables
in stacks.
douglasnaphas added a commit that referenced this issue Feb 24, 2021
douglasnaphas added a commit that referenced this issue Feb 25, 2021
gh-281

I changed what I expected to be the param name that I'm conditioning an
output on, and the output didn't change. I think the ue1 stack is
deploying not in us-east-1.

I'm also outputting the param that ue1 is attempting to condition on.
SSM params that are conditioned on in a stack should always be output by
the stack.
douglasnaphas added a commit that referenced this issue Feb 28, 2021
gh-281

This is because constructors cannot be async.
douglasnaphas added a commit that referenced this issue Feb 28, 2021
gh-281

I got an error "Missing region in config."
douglasnaphas added a commit that referenced this issue Mar 5, 2021
gh-281

I'm fetching multiple parameters with the SDK in bin (the from address
and the from region), so I need to iterate over the returned set to find
each.
douglasnaphas added a commit that referenced this issue Mar 5, 2021
gh-281

This applies only if a from-address is provided, but not verified with
AWS.
douglasnaphas added a commit that referenced this issue Mar 6, 2021
gh-281

This is to complete the TODO:

Set a verified email address, and observe the build failing due to a
non-DKIM domain.
douglasnaphas added a commit that referenced this issue Mar 6, 2021
gh-281

This is for the TODO:

> Deploy to douglas us-west-1, to see if using a non-sandboxed account
> resolves this issue. If that fixes the issue, fail if a sandboxed
> account is provided in bin/ (may want to do this anyway--I shouldn't
> deploy with a sandboxed account, at least not in prod).
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

The params where I am communicating the SES verification data (from
address and from-region) do not seem to be finding their way into the
stacks.

After the last deployment, even though I had SSM params
DouglasnaphasMadliberation281-ses-sesEmailVerificationFromAddress and
DouglasnaphasMadliberation281-ses-sesEmailVerificationFromRegion set in
us-west-1, it behaved as though I didn't. It's using Cognito for email
signup verification, when those params seek to tell it to use SES.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

I think my AWS SDK calls are failing in bin/. I think they need a
region, because I think the SDK uses the env var AWS_REGION, whereas the
CDK uses AWS_DEFAULT_REGION. My SDK calls are async and, intentionally,
do not fail if the SDK calls are errors (the SDK calls are seeking info
that may not be there).

This commit seeks to make SDK calls using whatever region is being used
for the app.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

The last build failed one the ue1 deploy with:

Run npx cdk bootstrap
us-east-1
error: sesEmailVerificationFromAddress expected, but not set
to fix this, set this param:
DouglasnaphasMadliberation281-ses-sesEmailVerificationFromAddress
or leave it unset, and unset this one:
DouglasnaphasMadliberation281-ses-sesEmailVerificationFromRegion
Subprocess exited with error

This doesn't make sense, because neither of those params are set for
us-east-1.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

It turns out getParameters does not populate err if the requested params
are all unset. It populates data.InvalidParameters, and leaves
data.Parameters as an empty array.

This change handles the case where Parameters is an empty array. This is
not an error case for my app--it just means that the default behavior
for the unset parameters will be used.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

When I query SES v2 for info, like verification status and DKIM details,
regarding the from-address, I need to use the region where the SES
identity is, not the region where the webapp stack is being deployed.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
@douglasnaphas
Copy link
Owner Author

I might need to lock the Cognito user pool and the SES identity to the same region, and restrict that region to us-east-1, us-west-2, or eu-west-1, because of this problem, which I have not been able to get past.

douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

The CloudFront domainName property is deprecated in favor of
distributionDomainName, which I was already also using.

https://github.com/aws/aws-cdk/blob/349a6e2bfaa72440deb3767fb1e28e38cc4d73ef/packages/%40aws-cdk/aws-cloudfront/lib/distribution.ts#L26

The real reason for this commit is to trigger a re-build, since I have
now deleted the stack in us-west-1 that was causing name collisions on
the CloudFront Origin Request Policies. ORPs need globally unique names,
and the name was being assigned the same value (confirmed by examining
the templates for the stacks in the CloudFormation console) in the two
regions.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

As far as I can tell, cross-region Cognito-SES access does not work,
even when SES is in a region supported for Cognito-SES access, and even
when the sender is production-enabled.

The more important reason for this commit is that I just set the SSM
params to tell webapp to use SES for sending emails.

I have been committing, rather than clicking "re-run jobs" in GitHub
actions, to re-trigger builds, because it seems that only the console
output from the most recent run of a build for a given commit shows up.
@douglasnaphas
Copy link
Owner Author

If I can get SES signup verification working, I may, at least for Mad Liberation, abandon the notion of non-us-east-1 deployments, and just assume all deployments will be us-east-1. This will be quicker, and I am having lots of issues with cross-region deployment.

@douglasnaphas
Copy link
Owner Author

SES signup verification works in the douglas account, us-east-1, same region for the user pool and SES identity.

Screen Shot 2021-03-06 at 11 23 03 PM

Screen Shot 2021-03-06 at 11 24 21 PM

douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

Mad Liberation is us-east-1-only now. Multi-region has no value for the
app, especially three weeks before Passover.
douglasnaphas added a commit that referenced this issue Mar 7, 2021
gh-281

I don't currently have multiple stacks in the app, but I might in the
future, especially once I get around to adding web sockets.
@douglasnaphas
Copy link
Owner Author

Done, as verified in the legacy passover.lol account. Closing.

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

No branches or pull requests

1 participant