adding SES Bulk Templated Policy Template #715

simalexan · 2018-12-12T01:58:16Z

Description of changes:
Add SESBulkTemplatedCrudPolicy which grants the user access to:

ses:SendTemplatedEmail
ses:SendBulkTemplatedEmail

Use case:
As a developer I'd like to be able to instantly deploy a serverless component from SAR that can send templated emails and bulk templated emails.
I have published three SAR components for sending emails, two of which are now labeled as having Custom IAM roles, as there isn't a policy template that enables them to use ses:SendTemplatedEmail and ses:SendBulkTemplatedEmail.

Here are the links to SAR components:

Thank you for taking this into consideration. I would be interested in adding more in the future.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

jlhood

Hi Aleks! Thanks for the contribution. I'm not confident that SES policies can be scoped to a resource type from reading their docs. If you're adding this to unblock publishing an app to SAR, note that you can now publish apps containing Lambda functions with custom IAM policies. The catch is that consumers will have to opt in for them to show up in search and will have to give explicit acknowledgements before deployment. You can read more about that here:

https://docs.aws.amazon.com/serverlessrepo/latest/devguide/acknowledging-application-capabilities.html

jlhood · 2018-12-12T18:36:16Z

samtranslator/policy_templates_data/policy_templates.json

+            ],
+            "Resource": {
+              "Fn::Sub": [
+                "arn:${AWS::Partition}:ses:${AWS::Region}:${AWS::AccountId}:identity/${identityName}",


Are you sure this works? I read this in the SES IAM docs:

Amazon SES has no service-defined resources that can be used as the Resource element of an IAM policy statement.

Yes, that is correct, you don't need any resource, but there are 2 things that made me include this Resource section:

The previous SESCrudPolicy, which is just for sending regular SES emails, requires an identity as well. It's the previous policy template just above this one. It is required for this Policy Template. I didn't intend to add it, but when I saw it there, as it was one the first ones added here, I just added it for consistency.

This identity represents the verified identity (email or domain) from which you want to be allowed to send emails from.

I have no problem from removing this section if necessary. The SESCrudPolicy is working, so I think this one will too.

Thanks for pointing out the SESCrudPolicy. It's interesting that the SES IAM docs say SES has no service-defined resources. I'll leave some feedback on their docs page so we can get that corrected.

simalexan · 2018-12-12T22:28:59Z

Hi James! Hope you're doing well!

I'm sorry, I don't want to add more work to your plate for review. I did actually publish three components, where two of them are now represented as having "Custom IAM Roles", as mentioned in the use case above and you can find the links to them there. Did publish a few components now that are as Custom Roles, one was the AppSync Crud just as the Custom IAM roles were announced before re:Invent.

So, I was a bit confused that a policy template for sending SES Templated Emails or Bulk Templated Emails wasn't in any of the existing Policy Templates. I guess that you are classifying which policy templates should be Custom based on the level of security implications possible.

I hope that this Template Policy for sending templated emails is ok, especially since sending emails to other unverified emails is already secured by the AWS SES platform. You can't send or spam someone until you ask for a permission to to raise your SES limits from the AWS Console, to send to unverified emails directly. Which I guess are processed manually (not sure, tbh!)

Thank you again for taking this into consideration! Sorry for taking your time!

simalexan · 2018-12-12T22:42:14Z

Last but not least, this is not urgent or so for me. I've also chosen this template policy numbering in order of the reviewed existing PRs with new Policy Templates. KitchenSinkFunctionRolePolicy47

jlhood · 2018-12-13T01:30:24Z

Sorry, I clearly looked at this too quickly this morning and missed a lot of the details you'd provided. Sorry if I created confusion. Let me clarify:

I guess that you are classifying which policy templates should be Custom based on the level of security implications possible.

No, we're still encouraging people to contribute SAM policy templates and are not drawing any lines regarding which are more or less secure. It's unfortunately common for developers to use Resource: '*' out of convenience so we enforce that policy templates should specify a specific resource unless a specific resource is actually unsupported for the given action. We've received positive customer feedback on policy templates as a more convenient way to help developers to do the right thing from a security standpoint so keep them coming!

From a SAR standpoint, we also still encourage using policy templates as they help increase discoverability of your apps since they'll show up to consumers by default in search if they don't require any IAM or resource policy capabilities.

Again, sorry for the confusion and really appreciate you submitting these policy templates. Look forward to more from you in the future!

jlhood

The Travis build is failing. Can you fix? Then we should be able to get this merged in.

keetonian

Moved things around in the tests so that they passed. Thank you for submitting this!

codecov-io · 2018-12-13T20:47:30Z

Codecov Report

Merging #715 into develop will increase coverage by 0.01%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop     #715      +/-   ##
===========================================
+ Coverage    94.17%   94.19%   +0.01%     
===========================================
  Files           67       67              
  Lines         2678     2686       +8     
  Branches       478      478              
===========================================
+ Hits          2522     2530       +8     
  Misses          80       80              
  Partials        76       76

Impacted Files	Coverage Δ
samtranslator/model/sam_resources.py	`95.72% <0%> (ø)`	⬆️
samtranslator/model/__init__.py	`96.85% <0%> (+0.16%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc9a3b7...cad8cb1. Read the comment docs.

simalexan · 2018-12-13T20:58:19Z

@keetonian Thank you very much! I was going to get it after finishing work. But am very very grateful! Much appreciated!

simalexan · 2018-12-13T20:58:55Z

@jlhood your colleague finished it up for me, kudos to @keetonian ! :)

simalexan · 2018-12-13T21:21:00Z

Also, didn't know that the tests were actually checking up on the order of policy templates. What is the goal of such a specified order?

jlhood · 2018-12-13T21:24:01Z

@simalexan I believe the tests are just checking that the output json of the translator matches the expected output. Since the policies are in JSON arrays, the order has to be the same for them to be equivalent.

keetonian · 2018-12-13T21:24:49Z

There isn't a goal- it's just how SAM currently works. Policy templates are added as policies with a set name and an incrementing number. This test, I think, could do a better job of testing these policy templates though.

jlhood

Thanks for fixing the tests @keetonian!

simalexan · 2018-12-13T21:27:01Z

@jlhood @keetonian Sure, I assumed so, just wanted to understand the reasoning and if I can help maybe with some improvement later on. Appreciate your work and help very much!
Thank you both again!

adding SES Bulk Templated Policy Templates

bd53f37

jlhood reviewed Dec 12, 2018

View reviewed changes

jlhood approved these changes Dec 13, 2018

View reviewed changes

jlhood suggested changes Dec 13, 2018

View reviewed changes

keetonian added 6 commits December 13, 2018 12:16

Update all_policy_templates.json

c796059

Update all_policy_templates.json

c45e848

Update all_policy_templates.json

5630a7a

Update policy_templates.json

d02600f

Update all_policy_templates.yaml

074ed30

Update all_policy_templates.yaml

cad8cb1

keetonian approved these changes Dec 13, 2018

View reviewed changes

jlhood approved these changes Dec 13, 2018

View reviewed changes

jlhood merged commit 5a4947b into aws:develop Dec 13, 2018

brettstack mentioned this pull request Mar 6, 2019

chore: merge release/v1.10.0 into master #843

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adding SES Bulk Templated Policy Template #715

adding SES Bulk Templated Policy Template #715

simalexan commented Dec 12, 2018

jlhood left a comment

jlhood Dec 12, 2018

simalexan Dec 12, 2018

jlhood Dec 13, 2018

simalexan commented Dec 12, 2018

simalexan commented Dec 12, 2018

jlhood commented Dec 13, 2018 •

edited

Loading

jlhood left a comment

keetonian left a comment

codecov-io commented Dec 13, 2018

simalexan commented Dec 13, 2018

simalexan commented Dec 13, 2018

simalexan commented Dec 13, 2018

jlhood commented Dec 13, 2018

keetonian commented Dec 13, 2018

jlhood left a comment

simalexan commented Dec 13, 2018

adding SES Bulk Templated Policy Template #715

adding SES Bulk Templated Policy Template #715

Conversation

simalexan commented Dec 12, 2018

jlhood left a comment

Choose a reason for hiding this comment

jlhood Dec 12, 2018

Choose a reason for hiding this comment

simalexan Dec 12, 2018

Choose a reason for hiding this comment

jlhood Dec 13, 2018

Choose a reason for hiding this comment

simalexan commented Dec 12, 2018

simalexan commented Dec 12, 2018

jlhood commented Dec 13, 2018 • edited Loading

jlhood left a comment

Choose a reason for hiding this comment

keetonian left a comment

Choose a reason for hiding this comment

codecov-io commented Dec 13, 2018

Codecov Report

simalexan commented Dec 13, 2018

simalexan commented Dec 13, 2018

simalexan commented Dec 13, 2018

jlhood commented Dec 13, 2018

keetonian commented Dec 13, 2018

jlhood left a comment

Choose a reason for hiding this comment

simalexan commented Dec 13, 2018

jlhood commented Dec 13, 2018 •

edited

Loading