-
Notifications
You must be signed in to change notification settings - Fork 6
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 an email sender policy #91
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export * from "./regex-pattern"; | ||
export * from "./stage"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
import { RegexPattern } from "./regex-pattern"; | ||
|
||
describe("the regex patterns", () => { | ||
it("should successfully regex against valid ARNs", () => { | ||
const regex = new RegExp(RegexPattern.ARN); | ||
expect(regex.test("fooooo")).toBeFalsy(); | ||
expect(regex.test("arn:aws:rds:us-east-2:123456789012:db:my-mysql-instance-1")).toBeTruthy(); | ||
}); | ||
|
||
it("should successfully regex against valid s3 ARNs only", () => { | ||
const regex = new RegExp(RegexPattern.S3ARN); | ||
expect(regex.test("fooooo")).toBeFalsy(); | ||
expect(regex.test("arn:aws:rds:us-east-2:123456789012:db:my-mysql-instance-1")).toBeFalsy(); | ||
expect(regex.test("arn:aws:s3:::examplebucket/my-data/sales-export-2019-q4.json")).toBeTruthy(); | ||
}); | ||
|
||
it("should successfully regex against valid emails only", () => { | ||
const regex = new RegExp(RegexPattern.GUARDIAN_EMAIL); | ||
expect(regex.test("email.address.com")).toBeFalsy(); | ||
expect(regex.test("email@gmail.com")).toBeFalsy(); | ||
|
||
expect(regex.test("email@theguardian.com")).toBeTruthy(); | ||
expect(regex.test("another.email@theguardian.com")).toBeTruthy(); | ||
|
||
expect(regex.test("another.@theguardian.com")).toBeFalsy(); | ||
expect(regex.test("another1@theguardian.com")).toBeFalsy(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
const arnRegex = "arn:aws:[a-z0-9]*:[a-z0-9\\-]*:[0-9]{12}:.*"; | ||
|
||
const s3BucketRegex = "(?!^(\\d{1,3}\\.){3}\\d{1,3}$)(^[a-z0-9]([a-z0-9-]*(\\.[a-z0-9])?)*$(?<!\\-))"; | ||
const s3ArnRegex = `arn:aws:s3:::${s3BucketRegex}*`; | ||
|
||
const emailRegex = "^[a-zA-Z]+(\\.[a-zA-Z]+)*@theguardian.com$"; | ||
|
||
export const RegexPattern = { | ||
ARN: arnRegex, | ||
S3ARN: s3ArnRegex, | ||
GUARDIAN_EMAIL: emailRegex, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,113 @@ | ||
// Jest Snapshot v1, https://goo.gl/fbAQLP | ||
|
||
exports[`GuSESSenderPolicy should add a parameter to the stack for the sending email address 1`] = ` | ||
Object { | ||
"Parameters": Object { | ||
"EmailSenderAddress": Object { | ||
"AllowedPattern": "^[a-zA-Z]+(\\\\.[a-zA-Z]+)*@theguardian.com$", | ||
"ConstraintDescription": "Must be an @theguardian.com email address", | ||
"Type": "String", | ||
}, | ||
"Stack": Object { | ||
"Default": "deploy", | ||
"Description": "Name of this stack", | ||
"Type": "String", | ||
}, | ||
"Stage": Object { | ||
"AllowedValues": Array [ | ||
"CODE", | ||
"PROD", | ||
], | ||
"Default": "CODE", | ||
"Description": "Stage name", | ||
"Type": "String", | ||
}, | ||
}, | ||
"Resources": Object { | ||
"GuSESSenderPolicy2E2A75A2": Object { | ||
"Properties": Object { | ||
"PolicyDocument": Object { | ||
"Statement": Array [ | ||
Object { | ||
"Action": "ses:SendEmail", | ||
"Effect": "Allow", | ||
"Resource": Object { | ||
"Fn::Join": Array [ | ||
"", | ||
Array [ | ||
"arn:aws:ses:", | ||
Object { | ||
"Ref": "AWS::Region", | ||
}, | ||
":", | ||
Object { | ||
"Ref": "AWS::AccountId", | ||
}, | ||
":identity/", | ||
Object { | ||
"Ref": "EmailSenderAddress", | ||
}, | ||
], | ||
], | ||
}, | ||
}, | ||
], | ||
"Version": "2012-10-17", | ||
}, | ||
"PolicyName": "GuSESSenderPolicy2E2A75A2", | ||
"Roles": Array [ | ||
Object { | ||
"Ref": "TestRole6C9272DF", | ||
}, | ||
], | ||
}, | ||
"Type": "AWS::IAM::Policy", | ||
}, | ||
"TestRole6C9272DF": Object { | ||
"Properties": Object { | ||
"AssumeRolePolicyDocument": Object { | ||
"Statement": Array [ | ||
Object { | ||
"Action": "sts:AssumeRole", | ||
"Effect": "Allow", | ||
"Principal": Object { | ||
"Service": Object { | ||
"Fn::Join": Array [ | ||
"", | ||
Array [ | ||
"ec2.", | ||
Object { | ||
"Ref": "AWS::URLSuffix", | ||
}, | ||
], | ||
], | ||
}, | ||
}, | ||
}, | ||
], | ||
"Version": "2012-10-17", | ||
}, | ||
"Tags": Array [ | ||
Object { | ||
"Key": "App", | ||
"Value": "testing", | ||
}, | ||
Object { | ||
"Key": "Stack", | ||
"Value": Object { | ||
"Ref": "Stack", | ||
}, | ||
}, | ||
Object { | ||
"Key": "Stage", | ||
"Value": Object { | ||
"Ref": "Stage", | ||
}, | ||
}, | ||
], | ||
}, | ||
"Type": "AWS::IAM::Role", | ||
}, | ||
}, | ||
} | ||
`; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import "@aws-cdk/assert/jest"; | ||
import { SynthUtils } from "@aws-cdk/assert"; | ||
import { attachPolicyToTestRole, simpleGuStackForTesting } from "../../../../test/utils"; | ||
import { GuSESSenderPolicy } from "./ses"; | ||
|
||
describe("GuSESSenderPolicy", () => { | ||
it("should have a policy that reads from a parameter", () => { | ||
const stack = simpleGuStackForTesting(); | ||
attachPolicyToTestRole(stack, new GuSESSenderPolicy(stack)); | ||
|
||
expect(stack).toHaveResource("AWS::IAM::Policy", { | ||
PolicyDocument: { | ||
Version: "2012-10-17", | ||
Statement: [ | ||
{ | ||
Action: "ses:SendEmail", | ||
Effect: "Allow", | ||
Resource: { | ||
"Fn::Join": [ | ||
"", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This empty string is a cdk oddity isn't it? I was wondering if there was something to fix on our side, but it looks fine on our side There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, it's super odd! I'd be reluctant to patch it on our side as this is auto-generated output; any patch we add would need to be compatible with future versions of the aws-cdk library. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, I don't think we should introduce a workaround for this. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We might be able to use one of these functions https://docs.aws.amazon.com/cdk/api/latest/docs/@aws-cdk_core.Arn.html. |
||
[ | ||
"arn:aws:ses:", | ||
{ | ||
Ref: "AWS::Region", | ||
}, | ||
":", | ||
{ | ||
Ref: "AWS::AccountId", | ||
}, | ||
":identity/", | ||
{ | ||
Ref: "EmailSenderAddress", | ||
}, | ||
], | ||
], | ||
}, | ||
}, | ||
], | ||
}, | ||
}); | ||
}); | ||
|
||
it("should add a parameter to the stack for the sending email address", () => { | ||
const stack = simpleGuStackForTesting(); | ||
attachPolicyToTestRole(stack, new GuSESSenderPolicy(stack)); | ||
expect(SynthUtils.toCloudFormation(stack)).toMatchSnapshot(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { Effect, PolicyStatement } from "@aws-cdk/aws-iam"; | ||
import type { GuStack } from "../../core"; | ||
import { GuGuardianEmailSenderParameter } from "../../core"; | ||
import type { GuPolicyProps } from "./base-policy"; | ||
import { GuPolicy } from "./base-policy"; | ||
|
||
export class GuSESSenderPolicy extends GuPolicy { | ||
constructor(scope: GuStack, id: string = "GuSESSenderPolicy", props?: GuPolicyProps) { | ||
super(scope, id, { ...props }); | ||
|
||
const emailSenderParam = new GuGuardianEmailSenderParameter(scope); | ||
|
||
this.addStatements( | ||
// see https://docs.aws.amazon.com/ses/latest/DeveloperGuide/sending-authorization-policies.html | ||
new PolicyStatement({ | ||
effect: Effect.ALLOW, | ||
actions: ["ses:SendEmail"], | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I remember there was one example from the datalake where they had both this and SendRawEmail. I was reading up on the difference in the boto3 docs and SendRawEmail is what you need to use to send an email with an attachment, for example. Do you think it's overkill to include it here as well? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, was meant to include something about this in the description. It looks like the datalake's permissions are too wide. That is, it's only calling We can always add it later if needed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair enough! |
||
resources: [`arn:aws:ses:${scope.region}:${scope.account}:identity/${emailSenderParam.valueAsString}`], | ||
}) | ||
); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Encapsulation and unit tests ftw 👍